Tweaking error handling.
There was a bug where we weren't handling an ErrorsException that wasn't related to a passed-in Errors object. This Errors approach is good 'cause the messages are good. But it still isn't particularly easy to code against because we need to both pass-in an Errors object and throw an ErrorsException if a return value cannot be computed.
Also changing how error message merging works, so that if we get a problem looking up say, a cached constructor, we use the code that wants that constructor as a source.
git-svn-id: https://google-guice.googlecode.com/svn/trunk@557 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/src/com/google/inject/ClassBindingImpl.java b/src/com/google/inject/ClassBindingImpl.java
index a177607..b61ee4f 100644
--- a/src/com/google/inject/ClassBindingImpl.java
+++ b/src/com/google/inject/ClassBindingImpl.java
@@ -43,7 +43,7 @@
}
@Override void initialize(InjectorImpl injector, Errors errors) throws ErrorsException {
- lateBoundConstructor.bind(injector, getBoundClass());
+ lateBoundConstructor.bind(injector, getBoundClass(), errors);
}
public void accept(BindingVisitor<? super T> visitor) {
diff --git a/src/com/google/inject/ConstructorInjector.java b/src/com/google/inject/ConstructorInjector.java
index 4cff625..25dc08e 100644
--- a/src/com/google/inject/ConstructorInjector.java
+++ b/src/com/google/inject/ConstructorInjector.java
@@ -42,7 +42,8 @@
this.implementation = implementation;
constructionProxy = injector.reflection.getConstructionProxy(errors, implementation);
parameterInjectors = createParameterInjector(errors, injector, constructionProxy);
- List<SingleMemberInjector> memberInjectorsList = injector.getMemberInjectors(implementation);
+ List<SingleMemberInjector> memberInjectorsList
+ = injector.getMemberInjectors(implementation, errors);
memberInjectors = memberInjectorsList.toArray(
new SingleMemberInjector[memberInjectorsList.size()]);
}
diff --git a/src/com/google/inject/CreationTimeMemberInjector.java b/src/com/google/inject/CreationTimeMemberInjector.java
index 1c9519c..f34e82b 100644
--- a/src/com/google/inject/CreationTimeMemberInjector.java
+++ b/src/com/google/inject/CreationTimeMemberInjector.java
@@ -54,7 +54,7 @@
void validateOustandingInjections(Errors errors) {
for (Object toInject : outstandingInjections) {
try {
- injector.getMemberInjectors(toInject.getClass());
+ injector.getMemberInjectors(toInject.getClass(), errors);
} catch (ErrorsException e) {
errors.merge(e.getErrors());
}
diff --git a/src/com/google/inject/InjectorImpl.java b/src/com/google/inject/InjectorImpl.java
index e8629dd..3e9d1cb 100644
--- a/src/com/google/inject/InjectorImpl.java
+++ b/src/com/google/inject/InjectorImpl.java
@@ -464,8 +464,8 @@
static class LateBoundConstructor<T> implements InternalFactory<T> {
ConstructorInjector<T> constructorInjector;
- void bind(InjectorImpl injector, Class<T> implementation) throws ErrorsException {
- constructorInjector = injector.getConstructor(implementation);
+ void bind(InjectorImpl injector, Class<T> implementation, Errors errors) throws ErrorsException {
+ constructorInjector = injector.getConstructor(implementation, errors);
}
@SuppressWarnings("unchecked")
@@ -627,7 +627,7 @@
}
};
- public List<SingleMemberInjector> getMemberInjectors(Class<?> type)
+ public List<SingleMemberInjector> getMemberInjectors(Class<?> type, Errors errors)
throws ErrorsException {
Object injectorsOrError = injectors.get(type);
if (injectorsOrError instanceof List) {
@@ -635,7 +635,8 @@
List<SingleMemberInjector> result = (List<SingleMemberInjector>) injectorsOrError;
return result;
} else if (injectorsOrError instanceof Errors) {
- throw ((Errors) injectorsOrError).copy().toException();
+ errors.merge((Errors) injectorsOrError);
+ throw errors.toException();
} else {
throw new AssertionError();
}
@@ -894,7 +895,7 @@
final Map<Class<?>, Object> constructors = new ReferenceCache<Class<?>, Object>() {
@SuppressWarnings("unchecked")
protected Object create(Class<?> implementation) {
- Errors errors = new Errors(StackTraceElements.forType(implementation));
+ Errors errors = new Errors();
try {
ConstructorInjector result = new ConstructorInjector(
errors, InjectorImpl.this, implementation);
@@ -955,7 +956,7 @@
return;
}
- for (SingleMemberInjector injector : getMemberInjectors(o.getClass())) {
+ for (SingleMemberInjector injector : getMemberInjectors(o.getClass(), errors)) {
injector.inject(errors, context, o);
}
}
@@ -1067,10 +1068,12 @@
/** Gets a constructor function for a given implementation class. */
@SuppressWarnings("unchecked")
- <T> ConstructorInjector<T> getConstructor(Class<T> implementation) throws ErrorsException {
+ <T> ConstructorInjector<T> getConstructor(Class<T> implementation, Errors errors)
+ throws ErrorsException {
Object o = constructors.get(implementation);
if (o instanceof Errors) {
- throw ((Errors) o).copy().toException();
+ errors.merge((Errors) o);
+ throw errors.toException();
}
else if (o instanceof ConstructorInjector<?>) {
return (ConstructorInjector<T>) o;
@@ -1088,10 +1091,12 @@
List<InjectionPoint<?>> getModifiableFieldAndMethodInjectionsFor(Class<?> clazz)
throws ErrorsException {
+ Errors errors = new Errors();
List<InjectionPoint<?>> dependencies = Lists.newArrayList();
- for (SingleMemberInjector singleMemberInjector : getMemberInjectors(clazz)) {
+ for (SingleMemberInjector singleMemberInjector : getMemberInjectors(clazz, errors)) {
dependencies.addAll(singleMemberInjector.getDependencies());
}
+ errors.throwIfNecessary();
return dependencies;
}
diff --git a/src/com/google/inject/ProxyFactory.java b/src/com/google/inject/ProxyFactory.java
index f43cc9f..d0da0ca 100644
--- a/src/com/google/inject/ProxyFactory.java
+++ b/src/com/google/inject/ProxyFactory.java
@@ -24,7 +24,6 @@
import com.google.inject.internal.Errors;
import com.google.inject.internal.ErrorsException;
import com.google.inject.internal.ReferenceCache;
-import com.google.inject.internal.StackTraceElements;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
@@ -58,12 +57,12 @@
@SuppressWarnings("unchecked")
public <T> ConstructionProxy<T> get(Errors errors, Constructor<T> constructor)
throws ErrorsException {
- return (ConstructionProxy<T>) getConstructionProxy(constructor);
+ return (ConstructionProxy<T>) getConstructionProxy(constructor, errors);
}
Map<Constructor<?>, Object> constructionProxies = new ReferenceCache<Constructor<?>, Object>() {
protected Object create(Constructor<?> constructor) {
- Errors errors = new Errors(StackTraceElements.forMember(constructor));
+ Errors errors = new Errors();
try {
ConstructionProxy<?> result = createConstructionProxy(errors, constructor);
errors.throwIfNecessary();
@@ -74,14 +73,15 @@
}
};
- public ConstructionProxy<?> getConstructionProxy(Constructor<?> constructor)
+ public ConstructionProxy<?> getConstructionProxy(Constructor<?> constructor, Errors errors)
throws ErrorsException {
Object constructionProxyOrErrors = constructionProxies.get(constructor);
if (constructionProxyOrErrors instanceof ConstructionProxy<?>) {
return (ConstructionProxy<?>) constructionProxyOrErrors;
} else if (constructionProxyOrErrors instanceof Errors) {
- throw ((Errors) constructionProxyOrErrors).copy().toException();
+ errors.merge((Errors) constructionProxyOrErrors);
+ throw errors.toException();
} else {
throw new AssertionError();
}
diff --git a/src/com/google/inject/internal/Errors.java b/src/com/google/inject/internal/Errors.java
index f034abb..ee94dc9 100644
--- a/src/com/google/inject/internal/Errors.java
+++ b/src/com/google/inject/internal/Errors.java
@@ -332,8 +332,10 @@
List<InjectionPoint> injectionPoints = Lists.newArrayList();
injectionPoints.addAll(this.injectionPoints);
injectionPoints.addAll(message.getInjectionPoints());
- errors.add(new Message(message.getSource(), message.getMessage(), injectionPoints,
- message.getCause()));
+ Object source = message.getSource() != SourceProvider.UNKNOWN_SOURCE
+ ? message.getSource()
+ : this.source;
+ errors.add(new Message(source, message.getMessage(), injectionPoints, message.getCause()));
}
}
diff --git a/test/com/google/inject/BinderTest.java b/test/com/google/inject/BinderTest.java
index 655deb6..30c9878 100644
--- a/test/com/google/inject/BinderTest.java
+++ b/test/com/google/inject/BinderTest.java
@@ -92,23 +92,23 @@
}
}
- public void testGetInstanceOfAnAbstractClass() {
- Injector injector = Guice.createInjector();
+ public void testMissingDependency() {
try {
- injector.getInstance(Runnable.class);
- fail();
- } catch(ProvisionException expected) {
- assertContains(expected.getMessage(),
+ Guice.createInjector(new AbstractModule() {
+ public void configure() {
+ bind(NeedsRunnable.class);
+ }
+ });
+ } catch (CreationException e) {
+ assertEquals(1, e.getErrorMessages().size());
+ assertContains(e.getMessage(),
+ "1) Error at " + getClass().getName(),
"No implementation for java.lang.Runnable was bound.");
}
+ }
- try {
- injector.getInstance(Key.get(Date.class, Names.named("date")));
- fail();
- } catch(ProvisionException expected) {
- assertContains(expected.getMessage(), "No implementation for java.util.Date "
- + "annotated with @com.google.inject.name.Named(value=date) was bound.");
- }
+ static class NeedsRunnable {
+ @Inject Runnable runnable;
}
public void testDanglingConstantBinding() {
@@ -120,7 +120,9 @@
});
fail();
} catch (CreationException expected) {
- assertContains(expected.getMessage(), "Missing constant value.");
+ assertContains(expected.getMessage(),
+ "1) Error at " + getClass().getName(),
+ "Missing constant value. Please call to(...).");
}
}
@@ -260,6 +262,26 @@
assertSame(JustAClass.class, injector.getInstance(JustAClass.class).getClass());
}
+ public void testPartialInjectorGetInstance() {
+ Injector injector = Guice.createInjector();
+ try {
+ injector.getInstance(MissingParameter.class);
+ fail();
+ } catch (ProvisionException expected) {
+ assertContains(expected.getMessage(),
+ "1) Error at " + MissingParameter.class.getName() + ".<init>(BinderTest.java:",
+ "Could not find a suitable constructor in " + NoInjectConstructor.class.getName());
+ }
+ }
+
+ static class MissingParameter {
+ @Inject MissingParameter(NoInjectConstructor noInjectConstructor) {}
+ }
+
+ static class NoInjectConstructor {
+ private NoInjectConstructor() {}
+ }
+
@ProvidedBy(HasProvidedBy1Provider.class)
interface HasProvidedBy1 {}
diff --git a/test/com/googlecode/guice/BytecodeGenTest.java b/test/com/googlecode/guice/BytecodeGenTest.java
index d47d83f..2f54375 100644
--- a/test/com/googlecode/guice/BytecodeGenTest.java
+++ b/test/com/googlecode/guice/BytecodeGenTest.java
@@ -119,6 +119,26 @@
}
}
+ /** as loaded by another class loader */
+ private Class<ProxyTest> proxyTestClass;
+ private Class<ProxyTestImpl> realClass;
+ private Module testModule;
+
+ @SuppressWarnings("unchecked")
+ protected void setUp() throws Exception {
+ super.setUp();
+
+ ClassLoader testClassLoader = new TestVisibilityClassLoader();
+ proxyTestClass = (Class<ProxyTest>) testClassLoader.loadClass(ProxyTest.class.getName());
+ realClass = (Class<ProxyTestImpl>) testClassLoader.loadClass(ProxyTestImpl.class.getName());
+
+ testModule = new AbstractModule() {
+ public void configure() {
+ bind(proxyTestClass).to(realClass);
+ }
+ };
+ }
+
interface ProxyTest {
String sayHello();
}
@@ -141,33 +161,13 @@
}
}
- @SuppressWarnings("unchecked")
- public void testProxyClassLoading() {
- final ClassLoader testClassLoader = new TestVisibilityClassLoader();
+ public void testProxyClassLoading() throws Exception {
+ Object testObject = Guice.createInjector(interceptorModule, testModule)
+ .getInstance(proxyTestClass);
- final Class<ProxyTest> testAPIClazz;
- final Class<ProxyTest> testImplClazz;
-
- try {
- testAPIClazz = (Class<ProxyTest>) testClassLoader.loadClass(ProxyTest.class.getName());
- testImplClazz = (Class<ProxyTest>) testClassLoader.loadClass(ProxyTestImpl.class.getName());
- } catch (final Exception e) {
- throw new RuntimeException(e);
- }
-
- final Object testObject = Guice.createInjector(interceptorModule, new Module() {
- public void configure(Binder binder) {
- binder.bind(testAPIClazz).to(testImplClazz);
- }
- }).getInstance(testAPIClazz);
-
- try {
- // verify method interception still works
- Method m = testImplClazz.getMethod("sayHello");
- assertEquals("HELLO WORLD", m.invoke(testObject));
- } catch (final Exception e) {
- throw new RuntimeException(e);
- }
+ // verify method interception still works
+ Method m = realClass.getMethod("sayHello");
+ assertEquals("HELLO WORLD", m.invoke(testObject));
}
public void testProxyClassUnloading() {
