Details
-
Type:
Improvement
-
Status:
Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: 1.2
-
Component/s: None
-
Labels:None
-
Number of attachments :
Description
The dependency resolution should never take into account a (cyclic) self dependency. Parameter already suppots this, but not the Pico interface itself. The xxxOfType methods have an additional parameter to exclude self.
public void testUnambiguousSelfDependency()
{ MutablePicoContainer pico = createPicoContainer(null); pico.registerComponentImplementation(SimpleTouchable.class); pico.registerComponentImplementation(DecoratedTouchable.class); Touchable t = (Touchable) pico.getComponentInstance(DecoratedTouchable.class); assertNotNull(t); }Activity
Konstantin Pribluda
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Assignee | Konstantin Pribluda [ ko5tik ] |
It seems that most elegant way to solve this is to implement some kind of guard, which will exclude component(key) in question from getComponentAdaptersOfType()
Less elegnat way would be fixing BasicComponentAdapter.resolveAdapter() - but this will duplicate code...
See also PICO-115
Konstantin Pribluda
added a comment - It seems that most elegant way to solve this is to implement some kind of guard, which will exclude component(key) in question from getComponentAdaptersOfType()
Less elegnat way would be fixing BasicComponentAdapter.resolveAdapter() - but this will duplicate code...
See also PICO-115 Hi Konstantin,
hope you recognized that both tests are already in the sources, but "commented out" by a TODO prefix for the function name.
Last time I tried to resolve this, I finally stopped, because I found no way to implement it, without change of the API. Therefore this is more a Pico 2.0 issue also in regard of Thomas' latest approach.
We have currently some obscure cases in the code base resulting from the fact that the dependecny resoulition is splitted between container and component adapter. I doubt that we can fix these without some kind of redesign.
- Jörg
Jörg Schaible
added a comment - Hi Konstantin,
hope you recognized that both tests are already in the sources, but "commented out" by a TODO prefix for the function name.
Last time I tried to resolve this, I finally stopped, because I found no way to implement it, without change of the API. Therefore this is more a Pico 2.0 issue also in regard of Thomas' latest approach.
We have currently some obscure cases in the code base resulting from the fact that the dependecny resoulition is splitted between container and component adapter. I doubt that we can fix these without some kind of redesign.
Jörg
did not found them... I added one of your tests though.
... gone hunting 
Konstantin Pribluda
added a comment - did not found them... I added one of your tests though.
... gone hunting Well, your second test case ( with linked list ) produces now CircularDependencyException - and this is correct...
Konstantin Pribluda
added a comment - Well, your second test case ( with linked list ) produces now CircularDependencyException - and this is correct... Konstantin Pribluda
made changes -
| Fix Version/s | 1.2 [ 11330 ] | |
| Status | Open [ 1 ] | Closed [ 6 ] |
| Resolution | Fixed [ 1 ] |
Similar case for Aslak:
When pico tries to instantiate ArrayList, it will attempt to use the constructor that takes a java.util.Collection. Since both the ArrayList and LinkedList are Collection, it will fail with AmbiguousComponentResolutionException.
Pico should be smart enough to figure out that it shouldn't consider a component as a candidate parameter for its own instantiation.
public void TODOtestComponentsWithCommonSupertypeWhichIsAConstructorArgumentCanBeLookedUpByConcreteType()
{ MutablePicoContainer pico = createPicoContainer(null); pico.registerComponentImplementation(LinkedList.class); pico.registerComponentImplementation(ArrayList.class); assertEquals(ArrayList.class, pico.getComponentInstanceOfType(ArrayList.class).getClass()); }