Details
-
Type:
Bug
-
Status:
Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 1.0-beta-4, 1.0
-
Fix Version/s: 1.0-beta-5
-
Component/s: PicoContainer (Java)
-
Labels:None
-
Number of attachments :
Description
The testcase:
public static class ContainerDependency implements Startable{
private final DefaultPicoContainer _container;
private boolean _started = false;
public ContainerDependency(DefaultPicoContainer container)
{ _container = container; }public void start()
{ _started = true; } public void stop() {
}
}
public void testSelfRegistry()
{ DefaultPicoContainer pico = new DefaultPicoContainer(); pico.registerComponentInstance(pico); pico.registerComponentImplementation(ContainerDependency.class); pico.start(); ContainerDependency dep = (ContainerDependency)pico.getComponentInstance(ContainerDependency.class); assertTrue(pico == dep._container); assertTrue(dep._started); }This will cause a stack overflow in the current codebase b/c the container will recursive try to start itself.
Activity
Michael Rettig
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Attachment | patch.txt [ 11311 ] |
Why would you want to register a container in itself?
Aslak Hellesøy
added a comment - Why would you want to register a container in itself? I mean, what your testcase does is an antipattern:
Aslak Hellesøy
added a comment - I mean, what your testcase does is an antipattern:
http://wiki.codehaus.org/picocontainer/ContainerDependency Yes, I would agree that components should not know anything about their container. However, in this case, I need the container b/c I need it to act as a dynamic factory for creating other objects.
http://jaxor.sourceforge.net/?q=pico
The article uses an older snapshot of pico, but it should show why I need the container injected into my constructor. I could also just use the constructor directly.
InstanceFactory factory = new PicoInstanceFactory(picoContainer);
.. which is what I do now in my testcase to work around the stack overflow in pico. However, I like the elegance of being able to self-register the container, allowing me to discover the creating container if needed. Also, this allows a very simple way to create a service locator using constructor based dependency injection.
Thanks,
Mike Rettig
Michael Rettig
added a comment - Yes, I would agree that components should not know anything about their container. However, in this case, I need the container b/c I need it to act as a dynamic factory for creating other objects.
http://jaxor.sourceforge.net/?q=pico
The article uses an older snapshot of pico, but it should show why I need the container injected into my constructor. I could also just use the constructor directly.
InstanceFactory factory = new PicoInstanceFactory(picoContainer);
.. which is what I do now in my testcase to work around the stack overflow in pico. However, I like the elegance of being able to self-register the container, allowing me to discover the creating container if needed. Also, this allows a very simple way to create a service locator using constructor based dependency injection.
Thanks,
Mike Rettig The container is actually already available as an "implicit" (but hidden) component adapter.
It is illustrated by testComponentsAreStartedBreadthFirstAndStoppedDepthFirst() method in http://cvs.codehaus.org/viewcvs.cgi/pico/src/test/org/picocontainer/defaults/DefaultPicoContainerLifecycleTestCase.java?rev=1.7&root=picocontainer&view=auto
The test registers of a DefaultPicoContainer.class in a the parent container. When the instances are asked for, the parent container will implicitly be used to resolve the dependency that a DefaultPicoContainer has on MutablePicoContainer in its constructor.
This will work for any component that declares a dependency on a container.
So there is no need to register a pico in itself. It is already available.
Aslak Hellesøy
added a comment - The container is actually already available as an "implicit" (but hidden) component adapter.
It is illustrated by testComponentsAreStartedBreadthFirstAndStoppedDepthFirst() method in http://cvs.codehaus.org/viewcvs.cgi/pico/src/test/org/picocontainer/defaults/DefaultPicoContainerLifecycleTestCase.java?rev=1.7&root=picocontainer&view=auto
The test registers of a DefaultPicoContainer.class in a the parent container. When the instances are asked for, the parent container will implicitly be used to resolve the dependency that a DefaultPicoContainer has on MutablePicoContainer in its constructor.
This will work for any component that declares a dependency on a container.
So there is no need to register a pico in itself. It is already available. Updated patch that includes a new test case for the implicit registry of the container to itself. Also added an exception to be thrown if anyone ever tries to register a container to itself.
Michael Rettig
added a comment - Updated patch that includes a new test case for the implicit registry of the container to itself. Also added an exception to be thrown if anyone ever tries to register a container to itself. Michael Rettig
made changes -
| Attachment | patch2.txt [ 11315 ] |
New exception for self registry exception.
Michael Rettig
added a comment - New exception for self registry exception. Michael Rettig
made changes -
| Attachment | PicoSelfRegistrationException.java [ 11316 ] |
Applied with minor modifications.
Aslak Hellesøy
added a comment - Applied with minor modifications. Aslak Hellesøy
made changes -
| Fix Version/s | 1.0-beta-5 [ 10145 ] | |
| Resolution | Fixed [ 1 ] | |
| Assignee | Aslak Hellesoy [ rinkrank ] | |
| Status | Open [ 1 ] | Closed [ 6 ] |
This patch includes the junit testcase and the fix.