[picocontainer-scm] [scm-git][1/1] Enhance the message for AmbiguousComponentResolutionException to help with jsr-tck debugging.

Michael Rimov <git-yCVjj/[email protected]> Fri, 1 Feb 2013 17:51:00 -0600 (CST)
Newsgroups gmane.comp.java.picocontainer.cvs
Message-ID <[email protected]>
commit 1479c8a407c3dde368fde63824d070f20891548b
Author:     Michael Rimov <[email protected]>
AuthorDate: Fri, 1 Feb 2013 15:49:39 -0800
Commit:     Michael Rimov <[email protected]>
CommitDate: Fri, 1 Feb 2013 15:49:39 -0800

    Enhance the message for AmbiguousComponentResolutionException to help with jsr-tck debugging.

diff --git a/pico/container/src/java/org/picocontainer/injectors/AbstractInjector.java b/pico/container/src/java/org/picocontainer/injectors/AbstractInjector.java
index 6ca1990..8aa5c87 100644
--- a/pico/container/src/java/org/picocontainer/injectors/AbstractInjector.java
+++ b/pico/container/src/java/org/picocontainer/injectors/AbstractInjector.java
@@ -22,8 +22,10 @@ import org.picocontainer.parameters.ComponentParameter;
 
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Member;
+import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
 import java.util.Arrays;
@@ -336,6 +338,12 @@ public abstract class AbstractInjector<T> extends AbstractAdapter<T> implements
         private final Object[] ambiguousComponentKeys;
 		private AccessibleObject accessibleObject;
 		
+		/**
+		 * Zero-based parameter #
+		 */
+		private int parameterNumber = -1;
+		
+
 
         /**
          * Construct a new exception with the ambigous class type and the ambiguous component keys.
@@ -346,7 +354,7 @@ public abstract class AbstractInjector<T> extends AbstractAdapter<T> implements
         public AmbiguousComponentResolutionException(final Generic<?> ambiguousDependency, final Object[] keys) {
             super("");
             this.ambiguousDependency = ambiguousDependency;
-            this.ambiguousComponentKeys = new Class[keys.length];
+            this.ambiguousComponentKeys = new Object[keys.length];
             System.arraycopy(keys, 0, ambiguousComponentKeys, 0, keys.length);
         }
 
@@ -359,8 +367,31 @@ public abstract class AbstractInjector<T> extends AbstractAdapter<T> implements
             msg.append(component != null ? component : "<not-specified>");
             msg.append(" needs a '");
             msg.append(ambiguousDependency.toString());
-            msg.append("' injected via '");
-            msg.append(accessibleObject != null ? accessibleObject : "<unknown>");
+            msg.append("' injected");
+            if (parameterNumber > -1) {
+            	msg.append(" into parameter #" + parameterNumber);
+            	msg.append(" (zero based index) of");
+            } 
+            
+            if (parameterNumber == -1 && accessibleObject != null) {
+            	msg.append(" into");            	
+            }
+            
+            if (accessibleObject != null) {
+            	if (accessibleObject instanceof Field) {
+            		msg.append(" field '");
+            	} else if (accessibleObject instanceof Constructor) {
+            		msg.append(" constructor '");
+            	} else if (accessibleObject instanceof Method) {
+            		msg.append(" method '");
+            	} else {
+            		msg.append(" '");
+            	}
+            	msg.append(accessibleObject);
+            } else {
+            	msg.append(" through : <unknown>");
+            }
+            //msg.append(accessibleObject != null ? accessibleObject : "<unknown>");
             msg.append("', but there are too many choices to inject. These:");
             msg.append(Arrays.asList(getAmbiguousComponentKeys()));
             msg.append(", refer http://picocontainer.org/ambiguous-injectable-help.html");
@@ -381,6 +412,10 @@ public abstract class AbstractInjector<T> extends AbstractAdapter<T> implements
         public void setMember(AccessibleObject accessibleObject) {
             this.accessibleObject = accessibleObject;
         }
+
+		public void setParameterNumber(int parameterNumber) {
+			this.parameterNumber = parameterNumber;
+		}        
     }
 
     /**
diff --git a/pico/container/src/java/org/picocontainer/injectors/ConstructorInjection.java b/pico/container/src/java/org/picocontainer/injectors/ConstructorInjection.java
index baa0c58..da005b3 100644
--- a/pico/container/src/java/org/picocontainer/injectors/ConstructorInjection.java
+++ b/pico/container/src/java/org/picocontainer/injectors/ConstructorInjection.java
@@ -187,6 +187,7 @@ public class ConstructorInjection extends AbstractInjectionType {
             int lastSatisfiableConstructorSize = -1;
             Type unsatisfiedDependency = null;
             Constructor unsatisfiedConstructor = null;
+            int lastParameterTested = 0;
             for (final Constructor<T> sortedMatchingConstructor : sortedMatchingConstructors) {
             	try {
 	                boolean failedDependency = false;
@@ -201,6 +202,7 @@ public class ConstructorInjection extends AbstractInjectionType {
 	                
 	                // remember: all constructors with less arguments than the given parameters are filtered out already
 	                for (int j = 0; j < currentParameters.length; j++) {
+	                	lastParameterTested = j;
 	                    // check whether this constructor is satisfiable
 	                    Type expectedType = box(parameterTypes[j]);
 	                    NameBinding expectedNameBinding = new ParameterNameBinding(getParanamer(), sortedMatchingConstructor, j);
@@ -244,8 +246,10 @@ public class ConstructorInjection extends AbstractInjectionType {
 	                }
 
             	} catch (AmbiguousComponentResolutionException e) {
-                    // embellish with the constructor being injected into.
+                    // embellish with the constructor being injected into and
+            		// parameter # causing the problem
                     e.setMember(sortedMatchingConstructor);
+                    e.setParameterNumber(lastParameterTested);
                     throw e;
                 }            
             }
diff --git a/pico/container/src/test/org/picocontainer/DefaultPicoContainerTestCase.java b/pico/container/src/test/org/picocontainer/DefaultPicoContainerTestCase.java
index b709512..6f73544 100644
--- a/pico/container/src/test/org/picocontainer/DefaultPicoContainerTestCase.java
+++ b/pico/container/src/test/org/picocontainer/DefaultPicoContainerTestCase.java
@@ -219,7 +219,7 @@ public final class DefaultPicoContainerTestCase extends AbstractPicoContainerTes
 			assertEquals(
 					"class "
 							+ doc
-							+ " needs a 'java.util.Collection' injected via 'public org.picocontainer.DefaultPicoContainerTestCase$DependsOnCollection(java.util.Collection)', but there are too many choices to inject. These:[class java.util.ArrayList, class java.util.LinkedList], refer http://picocontainer.org/ambiguous-injectable-help.html",
+							+ " needs a 'java.util.Collection' injected into parameter #0 (zero based index) of constructor 'public org.picocontainer.DefaultPicoContainerTestCase$DependsOnCollection(java.util.Collection)', but there are too many choices to inject. These:[class java.util.ArrayList, class java.util.LinkedList], refer http://picocontainer.org/ambiguous-injectable-help.html",
 					expected.getMessage());
 		}
 	}
diff --git a/pico/container/src/test/org/picocontainer/injectors/AbstractInjectorTestCase.java b/pico/container/src/test/org/picocontainer/injectors/AbstractInjectorTestCase.java
index 3a2e434..e6b0122 100644
--- a/pico/container/src/test/org/picocontainer/injectors/AbstractInjectorTestCase.java
+++ b/pico/container/src/test/org/picocontainer/injectors/AbstractInjectorTestCase.java
@@ -10,7 +10,9 @@
 package org.picocontainer.injectors;
 
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
 
+import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Member;
@@ -26,8 +28,11 @@ import org.picocontainer.Parameter;
 import org.picocontainer.PicoCompositionException;
 import org.picocontainer.PicoContainer;
 import org.picocontainer.containers.EmptyPicoContainer;
+import org.picocontainer.injectors.AbstractInjector.AmbiguousComponentResolutionException;
 import org.picocontainer.monitors.NullComponentMonitor;
 
+import com.googlecode.jtype.Generic;
+
 @SuppressWarnings("serial")
 public class AbstractInjectorTestCase {
 
@@ -130,8 +135,7 @@ public class AbstractInjectorTestCase {
         }
     }
 
-
-
+    @SuppressWarnings("rawtypes")
     private static class MyAbstractInjector extends AbstractInjector {
 
         public MyAbstractInjector(Object key,
@@ -154,4 +158,107 @@ public class AbstractInjectorTestCase {
             return null;
         }
     }
+
+    
+    public static class TestObject {
+    	public String someField;
+    	
+    	public TestObject(String someValue) {
+    		
+    	}
+    	
+    	public void setSomething(String value) {
+    		
+    	}
+    }
+
+    @Test
+    public void testAmbiguousComponentExceptionMessageWithNoAccsesibleObjectOrNoComponent() {
+    	Generic<String> generic = Generic.get(String.class);
+    	Object[] keys = new Object[] {String.class, "fwibble", "fribbit"};
+    	AmbiguousComponentResolutionException acre = new AmbiguousComponentResolutionException(generic, keys);
+    	assertTrue("Got " + acre.getMessage(),
+    			acre.getMessage().contains("<not-specified> needs a 'java.lang.String' injected through : <unknown>'," +
+    					" but there are too many choices to inject. These:[class java.lang.String, fwibble, fribbit]"));
+    }
+    
+    @Test
+    public void testAmbiguousComponentExceptionMessageWithComponentButNoAccessibleObject() {
+    	Generic<String> generic = Generic.get(String.class);
+    	Object[] keys = new Object[] {String.class, "fwibble", "fribbit"};
+    	AmbiguousComponentResolutionException acre = new AmbiguousComponentResolutionException(generic, keys);
+    	acre.setComponent(TestObject.class);
+    	assertTrue("Got " + acre.getMessage(),
+    			acre.getMessage().contains("org.picocontainer.injectors.AbstractInjectorTestCase$TestObject needs a 'java.lang.String' injected through : <unknown>'," +
+    					" but there are too many choices to inject. These:[class java.lang.String, fwibble, fribbit]"));
+    }
+    
+    
+    @Test
+    public void testAmbiguousComponentExceptionMessageWithConstructorAccessibleObject() {
+    	Generic<String> generic = Generic.get(String.class);
+    	Object[] keys = new Object[] {String.class, "fwibble", "fribbit"};
+    	AccessibleObject accessibleObject = TestObject.class.getConstructors()[0];
+    	
+    	AmbiguousComponentResolutionException acre = new AmbiguousComponentResolutionException(generic, keys);
+    	acre.setComponent(TestObject.class);
+    	acre.setMember(accessibleObject);
+    	
+    	assertTrue("Got " + acre.getMessage(),
+    			acre.getMessage().contains("class org.picocontainer.injectors.AbstractInjectorTestCase$TestObject needs a 'java.lang.String' injected into constructor " +
+    					"'public org.picocontainer.injectors.AbstractInjectorTestCase$TestObject(java.lang.String)'"));
+    	
+    }
+    
+    @Test
+    public void testAmbiguousComponentExceptionMessageWithConstructorAccessibleObjectAndParameterNumber() {
+    	Generic<String> generic = Generic.get(String.class);
+    	Object[] keys = new Object[] {String.class, "fwibble", "fribbit"};
+    	AccessibleObject accessibleObject = TestObject.class.getConstructors()[0];
+    	
+    	AmbiguousComponentResolutionException acre = new AmbiguousComponentResolutionException(generic, keys);
+    	acre.setComponent(TestObject.class);
+    	acre.setMember(accessibleObject);
+    	acre.setParameterNumber(0);
+    	
+    	assertTrue("Got " + acre.getMessage(),
+    			acre.getMessage().contains("class org.picocontainer.injectors.AbstractInjectorTestCase$TestObject needs a 'java.lang.String'" +
+    					" injected into parameter #0 (zero based index) of constructor " +
+    					"'public org.picocontainer.injectors.AbstractInjectorTestCase$TestObject(java.lang.String)'"));
+    	
+    }
+    
+    @Test
+    public void testAmbiguousComponentExceptionMessageWithFieldAccessibleObject() throws NoSuchFieldException, SecurityException {
+    	Generic<String> generic = Generic.get(String.class);
+    	Object[] keys = new Object[] {String.class, "fwibble", "fribbit"};
+    	AccessibleObject accessibleObject = TestObject.class.getField("someField");
+    	
+    	AmbiguousComponentResolutionException acre = new AmbiguousComponentResolutionException(generic, keys);
+    	acre.setComponent(TestObject.class);
+    	acre.setMember(accessibleObject);
+    	
+    	
+    	assertTrue("Got " + acre.getMessage(),
+    			acre.getMessage().contains("needs a 'java.lang.String' injected into field " +
+    					"'public java.lang.String org.picocontainer.injectors.AbstractInjectorTestCase$TestObject.someField'"));
+    }
+    
+    @Test
+    public void testAmbiguousComponentExceptionMessageWithMethodAccessibleObject() throws SecurityException, NoSuchMethodException {
+    	Generic<String> generic = Generic.get(String.class);
+    	Object[] keys = new Object[] {String.class, "fwibble", "fribbit"};
+    	AccessibleObject accessibleObject = TestObject.class.getMethod("setSomething", String.class);
+    	
+    	AmbiguousComponentResolutionException acre = new AmbiguousComponentResolutionException(generic, keys);
+    	acre.setComponent(TestObject.class);
+    	acre.setMember(accessibleObject);
+    	
+    	
+    	assertTrue("Got " + acre.getMessage(),
+    			acre.getMessage().contains("needs a 'java.lang.String' injected into method " +
+    					"'public void org.picocontainer.injectors.AbstractInjectorTestCase$TestObject.setSomething(java.lang.String)'"));
+    }
+       
+
 }