PicoContainer

Cannot assign Null Parameters

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major 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.

Activity

Hide
Michael Rimov added a comment -

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 - 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
Joerg Schaible added a comment -

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
Joerg Schaible added a comment - 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
Michael Rimov added a comment -

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 - 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
Michael Rimov added a comment -

Final fix applied in SVN revision 5531.

Show
Michael Rimov added a comment - Final fix applied in SVN revision 5531.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: