[PICO-364] Cannot assign Null Parameters

Details

Type: Bug
Status: Resolved
Priority: Major
Resolution: Fixed
Affects Version/s: 2.8
Fix Version/s: 2.9
Component/s: PicoContainer (Java)
Labels:
None

Testcase included:
yes
Number of attachments :
1

Description

I'm trying to explicitly assign a null in a constructor param:

===
public class ComponentClass

{ public ComponentClass(ArgClass arg) }

}

pico.addComponent("myComponent", ComponentClass.class, new
ConstantParameter(null))
===

produces an unsatisfied dependency error on ArgClass.

Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

pico-364-proposed-patch.diff

08/Sep/09 11:55 AM

3 kB

Michael Rimov

Activity

Hide

Permalink

Michael Rimov added a comment - 05/Sep/09 12:55 PM

Initial attempt at fixing:

@SuppressWarnings("serial")
public class NullParameter extends AbstractParameter implements Serializable {

	/**
	 * The one and only instance of null parameter.
	 */
	public static final NullParameter INSTANCE = new NullParameter();
	
	/**
	 * Only once instance of Null parameter needed.
	 */
	protected NullParameter() {
	}

	/**
	 * {@inheritDoc}
	 * @see org.picocontainer.Parameter#accept(org.picocontainer.PicoVisitor)
	 */
	public void accept(PicoVisitor visitor) {
		visitor.visitParameter(this);
	}

	/**
	 * {@inheritDoc}
	 * @see org.picocontainer.Parameter#resolve(org.picocontainer.PicoContainer, org.picocontainer.ComponentAdapter, org.picocontainer.ComponentAdapter, java.lang.reflect.Type, org.picocontainer.NameBinding, boolean, java.lang.annotation.Annotation)
	 */
	public Resolver resolve(PicoContainer container,
			ComponentAdapter<?> forAdapter,
			ComponentAdapter<?> injecteeAdapter, Type expectedType,
			NameBinding expectedNameBinding, boolean useNames,
			Annotation binding) {
		return new ValueResolver(isAssignable(expectedType), null, null);
	}

	/**
	 * {@inheritDoc}
	 * @see org.picocontainer.Parameter#verify(org.picocontainer.PicoContainer, org.picocontainer.ComponentAdapter, java.lang.reflect.Type, org.picocontainer.NameBinding, boolean, java.lang.annotation.Annotation)
	 */
	public void verify(PicoContainer container, ComponentAdapter<?> adapter,
			Type expectedType, NameBinding expectedNameBinding,
			boolean useNames, Annotation binding) {
		if (!isAssignable(expectedType)) {
			throw new PicoCompositionException(expectedType + " cannot be assigned a null value");
		}
	}
	
	/**
	 * Nulls cannot be assigned to primitives.
	 * @param expectedType
	 * @return
	 */
    protected boolean isAssignable(Type expectedType) {
        if (expectedType instanceof Class<?>) {
            Class<?> expectedClass = Class.class.cast(expectedType);
            if (expectedClass.isPrimitive()) {
                return false;
            }
        }
        return true;
    }
	
	
}

With test case:

    public void testNullConstantParameter() {
    	MutablePicoContainer pico = createPicoContainer(null);
    	pico.addComponent(ConstantParameterTestService.class, ConstantParameterTestService.class, NullParameter.INSTANCE);
    	ConstantParameterTestService service = (ConstantParameterTestService) pico.getComponent(ConstantParameterTestService.class);
    	assertNotNull(service);
    	assertNull(service.getArg());
    }

Failure for new case is:

org.picocontainer.injectors.SingleMemberInjector$ParameterCannotBeNullException: Parameter 0 of 'public org.picocontainer.tck.AbstractPicoContainerTest$ConstantParameterTestService(java.lang.String)' named 'arg' cannot be null
	at org.picocontainer.injectors.SingleMemberInjector.nullCheck(SingleMemberInjector.java:80)
	at org.picocontainer.injectors.SingleMemberInjector.getParameter(SingleMemberInjector.java:74)
	at org.picocontainer.injectors.ConstructorInjector$CtorAndAdapters.getParameterArguments(ConstructorInjector.java:283)
	at org.picocontainer.injectors.ConstructorInjector$1.run(ConstructorInjector.java:306)
	at org.picocontainer.injectors.AbstractInjector$ThreadLocalCyclicDependencyGuard.observe(AbstractInjector.java:289)
	at org.picocontainer.injectors.ConstructorInjector.getComponentInstance(ConstructorInjector.java:335)
	at org.picocontainer.DefaultPicoContainer.getInstance(DefaultPicoContainer.java:669)
	at org.picocontainer.DefaultPicoContainer.getComponent(DefaultPicoContainer.java:634)
	at org.picocontainer.DefaultPicoContainer.getComponent(DefaultPicoContainer.java:642)
	at org.picocontainer.tck.AbstractPicoContainerTest.testNullConstantParameter(AbstractPicoContainerTest.java:914)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:585)
	at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:59)
	at org.junit.internal.runners.MethodRoadie.runTestMethod(MethodRoadie.java:98)
	at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:79)
	at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:87)
	at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:77)
	at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:42)
	at org.junit.internal.runners.JUnit4ClassRunner.invokeTestMethod(JUnit4ClassRunner.java:88)
	at org.junit.internal.runners.JUnit4ClassRunner.runMethods(JUnit4ClassRunner.java:51)
	at org.junit.internal.runners.JUnit4ClassRunner$1.run(JUnit4ClassRunner.java:44)
	at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:27)
	at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:37)
	at org.junit.internal.runners.JUnit4ClassRunner.run(JUnit4ClassRunner.java:42)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:46)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197)

This is due to SingleMemberInjector.isNullParamAllowed();

Show

Michael Rimov added a comment - 05/Sep/09 12:55 PM Initial attempt at fixing: @SuppressWarnings( "serial" ) public class NullParameter extends AbstractParameter implements Serializable { /** * The one and only instance of null parameter. */ public static final NullParameter INSTANCE = new NullParameter(); /** * Only once instance of Null parameter needed. */ protected NullParameter() { } /** * {@inheritDoc} * @see org.picocontainer.Parameter#accept(org.picocontainer.PicoVisitor) */ public void accept(PicoVisitor visitor) { visitor.visitParameter( this ); } /** * {@inheritDoc} * @see org.picocontainer.Parameter#resolve(org.picocontainer.PicoContainer, org.picocontainer.ComponentAdapter, org.picocontainer.ComponentAdapter, java.lang.reflect.Type, org.picocontainer.NameBinding, boolean , java.lang.annotation.Annotation) */ public Resolver resolve(PicoContainer container, ComponentAdapter<?> forAdapter, ComponentAdapter<?> injecteeAdapter, Type expectedType, NameBinding expectedNameBinding, boolean useNames, Annotation binding) { return new ValueResolver(isAssignable(expectedType), null , null ); } /** * {@inheritDoc} * @see org.picocontainer.Parameter#verify(org.picocontainer.PicoContainer, org.picocontainer.ComponentAdapter, java.lang.reflect.Type, org.picocontainer.NameBinding, boolean , java.lang.annotation.Annotation) */ public void verify(PicoContainer container, ComponentAdapter<?> adapter, Type expectedType, NameBinding expectedNameBinding, boolean useNames, Annotation binding) { if (!isAssignable(expectedType)) { throw new PicoCompositionException(expectedType + " cannot be assigned a null value" ); } } /** * Nulls cannot be assigned to primitives. * @param expectedType * @ return */ protected boolean isAssignable(Type expectedType) { if (expectedType instanceof Class <?>) { Class <?> expectedClass = Class .class. cast (expectedType); if (expectedClass.isPrimitive()) { return false ; } } return true ; } } With test case: public void testNullConstantParameter() { MutablePicoContainer pico = createPicoContainer( null ); pico.addComponent(ConstantParameterTestService.class, ConstantParameterTestService.class, NullParameter.INSTANCE); ConstantParameterTestService service = (ConstantParameterTestService) pico.getComponent(ConstantParameterTestService.class); assertNotNull(service); assertNull(service.getArg()); } Failure for new case is: org.picocontainer.injectors.SingleMemberInjector$ParameterCannotBeNullException: Parameter 0 of ' public org.picocontainer.tck.AbstractPicoContainerTest$ConstantParameterTestService(java.lang. String )' named 'arg' cannot be null at org.picocontainer.injectors.SingleMemberInjector.nullCheck(SingleMemberInjector.java:80) at org.picocontainer.injectors.SingleMemberInjector.getParameter(SingleMemberInjector.java:74) at org.picocontainer.injectors.ConstructorInjector$CtorAndAdapters.getParameterArguments(ConstructorInjector.java:283) at org.picocontainer.injectors.ConstructorInjector$1.run(ConstructorInjector.java:306) at org.picocontainer.injectors.AbstractInjector$ThreadLocalCyclicDependencyGuard.observe(AbstractInjector.java:289) at org.picocontainer.injectors.ConstructorInjector.getComponentInstance(ConstructorInjector.java:335) at org.picocontainer.DefaultPicoContainer.getInstance(DefaultPicoContainer.java:669) at org.picocontainer.DefaultPicoContainer.getComponent(DefaultPicoContainer.java:634) at org.picocontainer.DefaultPicoContainer.getComponent(DefaultPicoContainer.java:642) at org.picocontainer.tck.AbstractPicoContainerTest.testNullConstantParameter(AbstractPicoContainerTest.java:914) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:585) at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:59) at org.junit.internal.runners.MethodRoadie.runTestMethod(MethodRoadie.java:98) at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:79) at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:87) at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:77) at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:42) at org.junit.internal.runners.JUnit4ClassRunner.invokeTestMethod(JUnit4ClassRunner.java:88) at org.junit.internal.runners.JUnit4ClassRunner.runMethods(JUnit4ClassRunner.java:51) at org.junit.internal.runners.JUnit4ClassRunner$1.run(JUnit4ClassRunner.java:44) at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:27) at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:37) at org.junit.internal.runners.JUnit4ClassRunner.run(JUnit4ClassRunner.java:42) at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:46) at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:467) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:683) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:390) at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:197) This is due to SingleMemberInjector.isNullParamAllowed();

Hide

Permalink

Jörg Schaible added a comment - 05/Sep/09 1:49 PM

The problem is that null needs a type to be assigned properly for a component. Maybe a placeholder can be used like the one in XStream: http://svn.codehaus.org/xstream/trunk/xstream/src/java/com/thoughtworks/xstream/core/util/TypedNull.java

Show

Jörg Schaible added a comment - 05/Sep/09 1:49 PM The problem is that null needs a type to be assigned properly for a component. Maybe a placeholder can be used like the one in XStream: http://svn.codehaus.org/xstream/trunk/xstream/src/java/com/thoughtworks/xstream/core/util/TypedNull.java

Hide

Permalink

Michael Rimov added a comment - 08/Sep/09 11:55 AM

Joerg,

I believe that Paul's refactoring to Parameters will actually protect against unintentioned null components. I've included a patch that demonstrates that SingleMemberInjector allow null default of 'true' not only still passes, but I've tried a few different ways of passing in a null value to pico other than through the special case NullParameter object.

It seems to me that if a component parameter says that the parameter value has been properly resolved that we should take it at its word, even if the parameter value is null?

Thanks for taking a look
-Mike (R)

Show

Michael Rimov added a comment - 08/Sep/09 11:55 AM Joerg, I believe that Paul's refactoring to Parameters will actually protect against unintentioned null components. I've included a patch that demonstrates that SingleMemberInjector allow null default of 'true' not only still passes, but I've tried a few different ways of passing in a null value to pico other than through the special case NullParameter object. It seems to me that if a component parameter says that the parameter value has been properly resolved that we should take it at its word, even if the parameter value is null? Thanks for taking a look -Mike (R)

Hide

Permalink

Michael Rimov added a comment - 16/Oct/09 5:06 PM

Final fix applied in SVN revision 5531.

Show

Michael Rimov added a comment - 16/Oct/09 5:06 PM Final fix applied in SVN revision 5531.

Cannot assign Null Parameters

Details

Description

Attachments

Activity

People

Dates