Changing InjectionPoint.member to be swapped for a serializable instance lazily rather than eagerly. This is what Bob had originally requested, and only now do I learn my lesson (from Sam's memory leak). Bob=smart.
git-svn-id: https://google-guice.googlecode.com/svn/trunk@600 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/src/com/google/inject/internal/Errors.java b/src/com/google/inject/internal/Errors.java
index b82367a..7b4ada4 100644
--- a/src/com/google/inject/internal/Errors.java
+++ b/src/com/google/inject/internal/Errors.java
@@ -467,7 +467,7 @@
},
new Converter<Member>(Member.class) {
public String toString(Member member) {
- return MoreTypes.canonicalize(member).toString();
+ return MoreTypes.toString(member);
}
},
new Converter<Key>(Key.class) {
diff --git a/src/com/google/inject/internal/MoreTypes.java b/src/com/google/inject/internal/MoreTypes.java
index df4efa4..1695464 100644
--- a/src/com/google/inject/internal/MoreTypes.java
+++ b/src/com/google/inject/internal/MoreTypes.java
@@ -111,7 +111,7 @@
* according to {@link Object#equals(Object) Object.equals}. The returned
* member is {@link Serializable}.
*/
- public static Member canonicalize(Member member) {
+ public static Member serializableCopy(Member member) {
return member instanceof MemberImpl
? member
: new MemberImpl(member);
@@ -296,6 +296,24 @@
}
}
+ /**
+ * Formats a member as concise string, such as {@code java.util.ArrayList.size},
+ * {@code java.util.ArrayList<init>()} or {@code java.util.List.remove()}.
+ */
+ public static String toString(Member member) {
+ Class<? extends Member> memberType = memberType(member);
+
+ if (memberType == Method.class) {
+ return member.getDeclaringClass().getName() + "." + member.getName() + "()";
+ } else if (memberType == Field.class) {
+ return member.getDeclaringClass().getName() + "." + member.getName();
+ } else if (memberType == Constructor.class) {
+ return member.getDeclaringClass().getName() + ".<init>()";
+ } else {
+ throw new AssertionError();
+ }
+ }
+
public static String memberKey(Member member) {
checkNotNull(member, "member");
@@ -497,15 +515,7 @@
}
@Override public String toString() {
- if (memberType == Method.class) {
- return "method " + getDeclaringClass().getName() + "." + getName() + "()";
- } else if (memberType == Field.class) {
- return "field " + getDeclaringClass().getName() + "." + getName();
- } else if (memberType == Constructor.class) {
- return "constructor " + getDeclaringClass().getName() + "()";
- } else {
- throw new AssertionError();
- }
+ return MoreTypes.toString(this);
}
}
}
diff --git a/src/com/google/inject/spi/InjectionPoint.java b/src/com/google/inject/spi/InjectionPoint.java
index 573733b..6e2efca 100644
--- a/src/com/google/inject/spi/InjectionPoint.java
+++ b/src/com/google/inject/spi/InjectionPoint.java
@@ -20,6 +20,7 @@
import com.google.inject.Key;
import com.google.inject.internal.MoreTypes;
import com.google.inject.internal.ToStringBuilder;
+import java.io.ObjectStreamException;
import java.io.Serializable;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
@@ -38,7 +39,7 @@
private InjectionPoint(Member member, int paramterIndex,
boolean allowsNull, Key<T> key) {
- this.member = member == null ? null : MoreTypes.canonicalize(member);
+ this.member = member;
this.parameterIndex = paramterIndex;
this.allowsNull = allowsNull;
this.key = checkNotNull(key, "key");
@@ -60,11 +61,18 @@
return allowsNull;
}
- public String toString() {
- return new ToStringBuilder(InjectionPoint.class)
- .add("member", member)
+ @Override public String toString() {
+ ToStringBuilder builder = new ToStringBuilder(InjectionPoint.class)
.add("key", key)
- .toString();
+ .add("allowsNull", allowsNull);
+
+ if (member != null) {
+ builder.add("member", MoreTypes.toString(member));
+ }
+ if (parameterIndex != -1) {
+ builder.add("parameterIndex", parameterIndex);
+ }
+ return builder.toString();
}
public static <T> InjectionPoint<T> newInstance(
@@ -80,4 +88,9 @@
boolean allowsNull, Key<T> key) {
return new InjectionPoint<T>(member, parameterIndex, allowsNull, key);
}
+
+ private Object writeReplace() throws ObjectStreamException {
+ Member serializableMember = member != null ? MoreTypes.serializableCopy(member) : null;
+ return new InjectionPoint<T>(serializableMember, parameterIndex, allowsNull, key);
+ }
}
diff --git a/test/com/google/inject/AllTests.java b/test/com/google/inject/AllTests.java
index 61f87fd..1d9744f 100644
--- a/test/com/google/inject/AllTests.java
+++ b/test/com/google/inject/AllTests.java
@@ -53,6 +53,7 @@
suite.addTestSuite(ErrorMessagesTest.class);
suite.addTestSuite(GenericInjectionTest.class);
suite.addTestSuite(ImplicitBindingTest.class);
+ suite.addTestSuite(InjectionPointTest.class);
suite.addTestSuite(InjectorTest.class);
suite.addTestSuite(IntegrationTest.class);
suite.addTestSuite(KeyTest.class);
diff --git a/test/com/google/inject/ErrorMessagesTest.java b/test/com/google/inject/ErrorMessagesTest.java
index 3229720..49deb13 100644
--- a/test/com/google/inject/ErrorMessagesTest.java
+++ b/test/com/google/inject/ErrorMessagesTest.java
@@ -55,8 +55,7 @@
} catch (ProvisionException expected) {
assertContains(expected.getMessage(),
"Error at " + B.class.getName() + ".injectMe(ErrorMessagesTest.java:",
- "method " + B.class.getName() + ".injectMe() ",
- "is annotated with @", Green.class.getName() + "(), ",
+ B.class.getName() + ".injectMe() is annotated with @", Green.class.getName() + "(), ",
"but binding annotations should be applied to its parameters instead.");
}
@@ -66,8 +65,7 @@
} catch (ProvisionException expected) {
assertContains(expected.getMessage(),
"Error at " + C.class.getName() + ".<init>(ErrorMessagesTest.java:",
- "constructor " + C.class.getName() + "() ",
- "is annotated with @", Green.class.getName() + "(), ",
+ C.class.getName() + ".<init>() is annotated with @", Green.class.getName() + "(), ",
"but binding annotations should be applied to its parameters instead.");
}
}
diff --git a/test/com/google/inject/InjectionPointTest.java b/test/com/google/inject/InjectionPointTest.java
new file mode 100644
index 0000000..35a3acb
--- /dev/null
+++ b/test/com/google/inject/InjectionPointTest.java
@@ -0,0 +1,103 @@
+/**
+ * Copyright (C) 2008 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.inject;
+
+import com.google.inject.name.Named;
+import com.google.inject.name.Names;
+import com.google.inject.spi.InjectionPoint;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import junit.framework.TestCase;
+
+/**
+ * @author jessewilson@google.com (Jesse Wilson)
+ */
+public class InjectionPointTest extends TestCase {
+
+ public @Inject @Named("a") String foo;
+ public @Inject void bar(@Named("b") String param) {}
+
+ public static class Constructable {
+ @Inject public Constructable(@Named("c") String param) {}
+ }
+
+ public void testFieldInjectionPoint() throws NoSuchFieldException, IOException {
+ Field fooField = getClass().getField("foo");
+ InjectionPoint<String> injectionPoint
+ = InjectionPoint.newInstance(fooField, false, Key.get(String.class, Names.named("a")));
+
+ assertEquals("InjectionPoint["
+ + "key=Key[type=java.lang.String, annotation=@com.google.inject.name.Named(value=a)], "
+ + "allowsNull=false, "
+ + "member=com.google.inject.InjectionPointTest.foo]", injectionPoint.toString());
+ assertEquals(fooField, injectionPoint.getMember());
+ assertEquals(-1, injectionPoint.getParameterIndex());
+ assertEquals(Key.get(String.class, Names.named("a")), injectionPoint.getKey());
+ assertEquals(false, injectionPoint.allowsNull());
+ Asserts.assertSimilarWhenReserialized(injectionPoint);
+ }
+
+ public void testMethodInjectionPoint() throws NoSuchMethodException, IOException {
+ Method barMethod = getClass().getMethod("bar", String.class);
+ InjectionPoint<String> injectionPoint
+ = InjectionPoint.newInstance(barMethod, 0, false, Key.get(String.class, Names.named("b")));
+
+ assertEquals("InjectionPoint["
+ + "key=Key[type=java.lang.String, annotation=@com.google.inject.name.Named(value=b)], "
+ + "allowsNull=false, "
+ + "member=com.google.inject.InjectionPointTest.bar(), "
+ + "parameterIndex=0]", injectionPoint.toString());
+ assertEquals(barMethod, injectionPoint.getMember());
+ assertEquals(0, injectionPoint.getParameterIndex());
+ assertEquals(Key.get(String.class, Names.named("b")), injectionPoint.getKey());
+ assertEquals(false, injectionPoint.allowsNull());
+ Asserts.assertSimilarWhenReserialized(injectionPoint);
+ }
+
+ public void testConstructorInjectionPoint() throws NoSuchMethodException, IOException {
+ Constructor<?> constructor = Constructable.class.getConstructor(String.class);
+ InjectionPoint<String> injectionPoint
+ = InjectionPoint.newInstance(constructor, 0, true, Key.get(String.class, Names.named("c")));
+
+ assertEquals("InjectionPoint["
+ + "key=Key[type=java.lang.String, annotation=@com.google.inject.name.Named(value=c)], "
+ + "allowsNull=true, "
+ + "member=com.google.inject.InjectionPointTest$Constructable.<init>(), "
+ + "parameterIndex=0]", injectionPoint.toString());
+ assertEquals(constructor, injectionPoint.getMember());
+ assertEquals(0, injectionPoint.getParameterIndex());
+ assertEquals(Key.get(String.class, Names.named("c")), injectionPoint.getKey());
+ assertEquals(true, injectionPoint.allowsNull());
+ Asserts.assertSimilarWhenReserialized(injectionPoint);
+ }
+
+ public void testPlainKeyInjectionPoint() throws IOException {
+ InjectionPoint<String> injectionPoint
+ = InjectionPoint.newInstance(Key.get(String.class, Names.named("d")));
+
+ assertEquals("InjectionPoint["
+ + "key=Key[type=java.lang.String, annotation=@com.google.inject.name.Named(value=d)], "
+ + "allowsNull=true]", injectionPoint.toString());
+ assertNull(injectionPoint.getMember());
+ assertEquals(-1, injectionPoint.getParameterIndex());
+ assertEquals(Key.get(String.class, Names.named("d")), injectionPoint.getKey());
+ assertEquals(true, injectionPoint.allowsNull());
+ Asserts.assertSimilarWhenReserialized(injectionPoint);
+ }
+}
diff --git a/test/com/google/inject/NullableInjectionPointTest.java b/test/com/google/inject/NullableInjectionPointTest.java
index b12b4b3..52f4f38 100644
--- a/test/com/google/inject/NullableInjectionPointTest.java
+++ b/test/com/google/inject/NullableInjectionPointTest.java
@@ -16,7 +16,7 @@
catch (ProvisionException expected) {
assertContains(expected.getMessage(),
"null returned by binding at " + getClass().getName(),
- "parameter 0 of constructor " + FooConstructor.class.getName() + "() is not @Nullable");
+ "parameter 0 of " + FooConstructor.class.getName() + ".<init>() is not @Nullable");
}
}
@@ -28,7 +28,7 @@
catch (ProvisionException expected) {
assertContains(expected.getMessage(),
"null returned by binding at " + getClass().getName(),
- "parameter 0 of method " + FooMethod.class.getName() + ".setFoo() is not @Nullable");
+ "parameter 0 of " + FooMethod.class.getName() + ".setFoo() is not @Nullable");
}
}
@@ -40,7 +40,7 @@
catch (ProvisionException expected) {
assertContains(expected.getMessage(),
"null returned by binding at " + getClass().getName(),
- " but field " + FooField.class.getName() + ".foo is not @Nullable");
+ " but " + FooField.class.getName() + ".foo is not @Nullable");
}
}
diff --git a/test/com/google/inject/OptionalBindingTest.java b/test/com/google/inject/OptionalBindingTest.java
index 55a7816..a84d453 100644
--- a/test/com/google/inject/OptionalBindingTest.java
+++ b/test/com/google/inject/OptionalBindingTest.java
@@ -247,7 +247,7 @@
Guice.createInjector().getInstance(HasOptionalConstructor.class);
fail();
} catch (ProvisionException expected) {
- assertContains(expected.getMessage(), "OptionalBindingTest$HasOptionalConstructor() "
+ assertContains(expected.getMessage(), "OptionalBindingTest$HasOptionalConstructor.<init>() "
+ "is annotated @Inject(optional=true), but constructors cannot be optional.");
}
}