Details
Bug
Closed
Major
Description
Here's the test case that fails:
(Specific test run is: DefaultNanoPicoContainerTestCase)
org.picocontainer.tck.AbstractPicoContainerTestCase:
public void testMakeRemoveChildContainer()
{ final NanoPicoContainer parent = (NanoPicoContainer) createPicoContainer(null); parent.registerComponentInstance("java.lang.String", "This is a test"); MutablePicoContainer pico = parent.makeChildContainer(); //Verify they are indeed wired together. assertNotNull(pico.getComponentInstance("java.lang.String")); boolean result = parent.removeChildContainer(pico); assertTrue(result); }I'm posting the concept here because after wracking my brains out, the BEST solution I could come up with was modify a few methods in AbstractNanoPicoContainer to have if/thens like this:
public boolean removeChildContainer(PicoContainer child) {
boolean result;
if (child instanceof AbstractNanoPicoContainer)
else
{ result = getDelegate().removeChildContainer(child); }
//.....
}
(Full details in the patch).
To me, this fix is u g l y. But I couldn't think of any other way to pull it off without changing the API.
Anybody have any thoughts?
Thanks!
-Mike (R)
Activity
Jörg,
It does pass the test cases, but I haven't replied up until now because so far I haven't figured out WHY your patch doesn't also see the original test case fail for anything deriving from AbstractNanoPicoContainer. (ex: ImplementationHidingNanoPicoContainer).
I haven't stepped through it enough to understand, but I really appreciate your feedback. I'll continue to chew on it this week.
-Mike (R)
Michael Rimov
added a comment - Jörg,
It does pass the test cases, but I haven't replied up until now because so far I haven't figured out WHY your patch doesn't also see the original test case fail for anything deriving from AbstractNanoPicoContainer. (ex: ImplementationHidingNanoPicoContainer).
I haven't stepped through it enough to understand, but I really appreciate your feedback. I'll continue to chew on it this week.
-Mike (R) You're right, the IHNPC has the same problem. Try this patch, I've refactored the necessary functionality ...
Jörg Schaible
added a comment - You're right, the IHNPC has the same problem. Try this patch, I've refactored the necessary functionality ... Jörg Schaible
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Attachment | removeChildContainer.patch [ 20949 ] |
Thanks Jörg,
Very clever and clean. Huge improvement over my first patch! 
Michael Rimov
added a comment - Thanks Jörg,
Very clever and clean. Huge improvement over my first patch!
Michael Rimov
made changes -
| Resolution | Fixed [ 1 ] | |
| Fix Version/s | 1.1 [ 12307 ] | |
| Status | Open [ 1 ] | Closed [ 6 ] |
| Assignee | Michael Rimov [ rimovm ] |
Can you try the following patch? Basic idea is, that the registered child container in the MPC is replaced with the DNPC itself.
Index: C:/Work/Apps/Codehaus/pico/java/nanocontainer/container/src/java/org/nanocontainer/reflection/DefaultNanoPicoContainer.java =================================================================== --- C:/Work/Apps/Codehaus/pico/java/nanocontainer/container/src/java/org/nanocontainer/reflection/DefaultNanoPicoContainer.java (revision 2961) +++ C:/Work/Apps/Codehaus/pico/java/nanocontainer/container/src/java/org/nanocontainer/reflection/DefaultNanoPicoContainer.java (working copy) @@ -94,6 +94,9 @@ */ protected DefaultNanoPicoContainer(final DefaultNanoPicoContainer parent) { super(parent.getDelegate().makeChildContainer(), parent.getComponentClassLoader()); + MutablePicoContainer parentDelegate = parent.getDelegate(); + parentDelegate.removeChildContainer(getDelegate()); + parentDelegate.addChildContainer(this); } /**