PicoContainer
  1. PicoContainer
  2. PICO-227

Patch to allow CAF to be specified for children

    Details

    • Type: New Feature New Feature
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 2.5
    • Component/s: PicoContainer (Java)
    • Labels:
      None
    • Number of attachments :
      1

      Activity

      Hide
      Aslak Hellesøy added a comment -

      patch by mark hobson

      Show
      Aslak Hellesøy added a comment - patch by mark hobson
      Aslak Hellesøy made changes -
      Field Original Value New Value
      Attachment patch.zip [ 13622 ]
      Hide
      Aslak Hellesøy added a comment -

      Some initial comments:

      • The deprecated method that delegate to the new one should never pass null, but its own CAF or the default CAF
      • The XMLContainerBuilderTestCase.testComponentAdapterClassCanBeSpecifiedInChildContainerElement
        doesn't seem to be actually testing that the new functionality is working. The test should be improved to specify the name of a test-CAF, which could be examined afterwards (for example look at some static field)
      Show
      Aslak Hellesøy added a comment - Some initial comments: The deprecated method that delegate to the new one should never pass null, but its own CAF or the default CAF The XMLContainerBuilderTestCase.testComponentAdapterClassCanBeSpecifiedInChildContainerElement doesn't seem to be actually testing that the new functionality is working. The test should be improved to specify the name of a test-CAF, which could be examined afterwards (for example look at some static field)
      Hide
      Mauro Talevi added a comment -

      In current design, CAF is not part of the API. Exposing CAF would imply adding to the API.

      Is that something we want to do?

      Show
      Mauro Talevi added a comment - In current design, CAF is not part of the API. Exposing CAF would imply adding to the API. Is that something we want to do?
      Hide
      Jörg Schaible added a comment -

      We certainly don't want a reference to .defaults in core. So do we want to move it (with keeping a deprecated and derived CAF in .default)?

      The CA/CAF relationship is something we will address for certain in Pico 2.0 (see PICO-91 and PICO-220) and the more we have in core, the more API changes will be.

      Paul, comments?

      Show
      Jörg Schaible added a comment - We certainly don't want a reference to .defaults in core. So do we want to move it (with keeping a deprecated and derived CAF in .default)? The CA/CAF relationship is something we will address for certain in Pico 2.0 (see PICO-91 and PICO-220 ) and the more we have in core, the more API changes will be. Paul, comments?
      Hide
      Mauro Talevi added a comment -

      I have no issues with moving CAF interface to core (particularly as it is so much interwoven with CA in current pico 1 design).
      But I don't know if we need to.

      If the use case is to enable to add a custom child container with a new CAF (and possibly a new lifecycle manager and/or monitor),
      IMO we could use the existing

      public boolean addChildContainer(PicoContainer child)

      { return children.add(child); }

      where a completely customisable instance of PC can be added (we should add the check on the parent of the child container)

      We would though have to expose the getLifecycleManager() method in PC (which is entirely justified IMO).

      Then the makeChildContainer would simply make the child container with the default dependencies of the parent.

      public MutablePicoContainer makeChildContainer()

      { DefaultPicoContainer pc = new DefaultPicoContainer(componentAdapterFactory, this, lifecycleManager); addChildContainer(pc); return pc; }
      Show
      Mauro Talevi added a comment - I have no issues with moving CAF interface to core (particularly as it is so much interwoven with CA in current pico 1 design). But I don't know if we need to. If the use case is to enable to add a custom child container with a new CAF (and possibly a new lifecycle manager and/or monitor), IMO we could use the existing public boolean addChildContainer(PicoContainer child) { return children.add(child); } where a completely customisable instance of PC can be added (we should add the check on the parent of the child container) We would though have to expose the getLifecycleManager() method in PC (which is entirely justified IMO). Then the makeChildContainer would simply make the child container with the default dependencies of the parent. public MutablePicoContainer makeChildContainer() { DefaultPicoContainer pc = new DefaultPicoContainer(componentAdapterFactory, this, lifecycleManager); addChildContainer(pc); return pc; }
      Mauro Talevi made changes -
      Assignee Mauro Talevi [ maurotalevi ]
      Hide
      Paul Hammant added a comment -

      I've just spiked a solution:

      Add this to PicoContainer and DefaultPicoContainer (etc)

      public MutablePicoContainer makeChildContainer(Object[] constituentParts) {
      DefaultPicoContainer transient1 = new DefaultPicoContainer();
      MutablePicoContainer transient2 = transient1.makeChildContainer();
      for (int i = 0; i < constituentParts.length; i++) {
      Object constituentPart = constituentParts[i];
      if (constituentPart instanceof Class) {
      Class clazz = (Class) constituentPart;
      if (MutablePicoContainer.class.isAssignableFrom(clazz))

      { transient2.registerComponentImplementation(MutablePicoContainer.class, clazz); }

      else if (ComponentAdapterFactory.class.isAssignableFrom(clazz))

      { transient2.registerComponentImplementation(ComponentAdapterFactory.class, clazz); }

      else if(LifecycleManager.class.isAssignableFrom(clazz))

      { transient2.registerComponentImplementation(LifecycleManager.class, clazz); }

      else

      { transient2.registerComponentImplementation(clazz); }

      } else {
      if (constituentPart instanceof ComponentAdapterFactory)

      { transient2.registerComponentInstance(ComponentAdapterFactory.class, constituentPart); }

      else if(constituentPart instanceof LifecycleManager)

      { transient2.registerComponentInstance(LifecycleManager.class, constituentPart); }

      else

      { transient2.registerComponentInstance(constituentPart); }

      }
      }
      transient1.registerComponentInstance(PicoContainer.class, this); // parent
      // fallbacks
      transient1.registerComponentInstance(ComponentAdapterFactory.class, componentAdapterFactory);
      transient1.registerComponentInstance(LifecycleManager.class, lifecycleManager);
      return (MutablePicoContainer) transient2.getComponentInstance(MutablePicoContainer.class);
      }

      and this method to DPCTestCase :-

      public void testComplexMakeChildContainer() {

      DefaultPicoContainer pc1 = new DefaultPicoContainer();
      ComponentAdapterFactory caf = new ComponentAdapterFactory() {
      public ComponentAdapter createComponentAdapter(Object componentKey,
      Class componentImplementation,
      Parameter[] parameters) throws PicoIntrospectionException, AssignabilityRegistrationException, NotConcreteRegistrationException {
      return new ComponentAdapter() {

      public Object getComponentKey()

      { return String.class; }

      public Class getComponentImplementation() { return String.class; }

      public Object getComponentInstance(PicoContainer container) throws PicoInitializationException, PicoIntrospectionException

      { return "foo"; }

      public void verify(PicoContainer container) throws PicoIntrospectionException {
      }

      public void accept(PicoVisitor visitor) {
      }

      };
      }
      };

      MutablePicoContainer pc2 = pc1.makeChildContainer(new Object[]

      {caf, ImplementationHidingPicoContainer.class}

      );
      pc2.registerComponentImplementation(String.class);
      String foo = (String) pc2.getComponentInstanceOfType(String.class);

      assertTrue(pc2 instanceof ImplementationHidingPicoContainer);
      assertEquals("foo", foo);
      }

      Show
      Paul Hammant added a comment - I've just spiked a solution: Add this to PicoContainer and DefaultPicoContainer (etc) public MutablePicoContainer makeChildContainer(Object[] constituentParts) { DefaultPicoContainer transient1 = new DefaultPicoContainer(); MutablePicoContainer transient2 = transient1.makeChildContainer(); for (int i = 0; i < constituentParts.length; i++) { Object constituentPart = constituentParts [i] ; if (constituentPart instanceof Class) { Class clazz = (Class) constituentPart; if (MutablePicoContainer.class.isAssignableFrom(clazz)) { transient2.registerComponentImplementation(MutablePicoContainer.class, clazz); } else if (ComponentAdapterFactory.class.isAssignableFrom(clazz)) { transient2.registerComponentImplementation(ComponentAdapterFactory.class, clazz); } else if(LifecycleManager.class.isAssignableFrom(clazz)) { transient2.registerComponentImplementation(LifecycleManager.class, clazz); } else { transient2.registerComponentImplementation(clazz); } } else { if (constituentPart instanceof ComponentAdapterFactory) { transient2.registerComponentInstance(ComponentAdapterFactory.class, constituentPart); } else if(constituentPart instanceof LifecycleManager) { transient2.registerComponentInstance(LifecycleManager.class, constituentPart); } else { transient2.registerComponentInstance(constituentPart); } } } transient1.registerComponentInstance(PicoContainer.class, this); // parent // fallbacks transient1.registerComponentInstance(ComponentAdapterFactory.class, componentAdapterFactory); transient1.registerComponentInstance(LifecycleManager.class, lifecycleManager); return (MutablePicoContainer) transient2.getComponentInstance(MutablePicoContainer.class); } and this method to DPCTestCase :- public void testComplexMakeChildContainer() { DefaultPicoContainer pc1 = new DefaultPicoContainer(); ComponentAdapterFactory caf = new ComponentAdapterFactory() { public ComponentAdapter createComponentAdapter(Object componentKey, Class componentImplementation, Parameter[] parameters) throws PicoIntrospectionException, AssignabilityRegistrationException, NotConcreteRegistrationException { return new ComponentAdapter() { public Object getComponentKey() { return String.class; } public Class getComponentImplementation() { return String.class; } public Object getComponentInstance(PicoContainer container) throws PicoInitializationException, PicoIntrospectionException { return "foo"; } public void verify(PicoContainer container) throws PicoIntrospectionException { } public void accept(PicoVisitor visitor) { } }; } }; MutablePicoContainer pc2 = pc1.makeChildContainer(new Object[] {caf, ImplementationHidingPicoContainer.class} ); pc2.registerComponentImplementation(String.class); String foo = (String) pc2.getComponentInstanceOfType(String.class); assertTrue(pc2 instanceof ImplementationHidingPicoContainer); assertEquals("foo", foo); }
      Hide
      Jörg Schaible added a comment -

      Nice idea. But why do you all this "instaneof" and "isAssignable" stuff ? Just register impl or instance and it should be enough.

      Show
      Jörg Schaible added a comment - Nice idea. But why do you all this "instaneof" and "isAssignable" stuff ? Just register impl or instance and it should be enough.
      Hide
      Paul Hammant added a comment -

      It did not work during experimentation. I half remember that unkeyed things could satisfy other comp's needs.

      Also, the ...

      return (MutablePicoContainer) transient2.getComponentInstance(MutablePicoContainer.class);

      .... needs to also add the child to children list.

      Show
      Paul Hammant added a comment - It did not work during experimentation. I half remember that unkeyed things could satisfy other comp's needs. Also, the ... return (MutablePicoContainer) transient2.getComponentInstance(MutablePicoContainer.class); .... needs to also add the child to children list.
      Hide
      Mauro Talevi added a comment -

      Should an instance of DPC be allowed to make children of another impl?

      This solution seems to allow that.
      I would be more inclined to have each impl of MPC implement something like:

      public MutablePicoContainer makeChildContainer(Object[] dependendies) {
      DefaultPicoContainer transient1 = new DefaultPicoContainer();
      MutablePicoContainer transient2 = transient1.makeChildContainer();
      for (int i = 0; i < dependendies.length; i++) {
      Object dependency = dependendies[i];
      if (dependency instanceof Class) {
      Class clazz = (Class) dependency;
      if (MutablePicoContainer.class.isAssignableFrom(clazz))

      { transient2.registerComponentImplementation(MutablePicoContainer.class, clazz); }

      else if (ComponentAdapterFactory.class.isAssignableFrom(clazz))

      { transient2.registerComponentImplementation(ComponentAdapterFactory.class, clazz); }

      else if(LifecycleManager.class.isAssignableFrom(clazz))

      { transient2.registerComponentImplementation(LifecycleManager.class, clazz); }

      else

      { throw PicoInstantiationException("Invalid dependency for DefaultPicoContainer "+clazz); }
      } else {
      if (dependency instanceof ComponentAdapterFactory) { transient2.registerComponentInstance(ComponentAdapterFactory.class, constituentPart); } else if(dependency instanceof LifecycleManager) { transient2.registerComponentInstance(LifecycleManager.class, constituentPart); } else { throw PicoInstantiationException("Invalid dependency for DefaultPicoContainer "+clazz); }

      }
      }
      transient1.registerComponentInstance(PicoContainer.class, this); // parent
      // fallbacks
      transient1.registerComponentInstance(ComponentAdapterFactory.class, componentAdapterFactory);
      transient1.registerComponentInstance(LifecycleManager.class, lifecycleManager);
      return (MutablePicoContainer) transient2.getComponentInstance(MutablePicoContainer.class);
      }

      Show
      Mauro Talevi added a comment - Should an instance of DPC be allowed to make children of another impl? This solution seems to allow that. I would be more inclined to have each impl of MPC implement something like: public MutablePicoContainer makeChildContainer(Object[] dependendies) { DefaultPicoContainer transient1 = new DefaultPicoContainer(); MutablePicoContainer transient2 = transient1.makeChildContainer(); for (int i = 0; i < dependendies.length; i++) { Object dependency = dependendies [i] ; if (dependency instanceof Class) { Class clazz = (Class) dependency; if (MutablePicoContainer.class.isAssignableFrom(clazz)) { transient2.registerComponentImplementation(MutablePicoContainer.class, clazz); } else if (ComponentAdapterFactory.class.isAssignableFrom(clazz)) { transient2.registerComponentImplementation(ComponentAdapterFactory.class, clazz); } else if(LifecycleManager.class.isAssignableFrom(clazz)) { transient2.registerComponentImplementation(LifecycleManager.class, clazz); } else { throw PicoInstantiationException("Invalid dependency for DefaultPicoContainer "+clazz); } } else { if (dependency instanceof ComponentAdapterFactory) { transient2.registerComponentInstance(ComponentAdapterFactory.class, constituentPart); } else if(dependency instanceof LifecycleManager) { transient2.registerComponentInstance(LifecycleManager.class, constituentPart); } else { throw PicoInstantiationException("Invalid dependency for DefaultPicoContainer "+clazz); } } } transient1.registerComponentInstance(PicoContainer.class, this); // parent // fallbacks transient1.registerComponentInstance(ComponentAdapterFactory.class, componentAdapterFactory); transient1.registerComponentInstance(LifecycleManager.class, lifecycleManager); return (MutablePicoContainer) transient2.getComponentInstance(MutablePicoContainer.class); }
      Michael Rimov made changes -
      Fix Version/s 2.0 [ 10411 ]
      Fix Version/s 2.2 [ 14251 ]
      Michael Rimov made changes -
      Fix Version/s 2.3 [ 14303 ]
      Fix Version/s 2.2 [ 14251 ]
      Michael Rimov made changes -
      Fix Version/s 2.3 [ 14303 ]
      Fix Version/s 2.4 [ 14362 ]
      Michael Rimov made changes -
      Fix Version/s 2.4 [ 14362 ]
      Fix Version/s 2.5 [ 14424 ]
      Hide
      Paul Hammant added a comment -

      change number 4829 added a addChildToParent() method

      This is where such flexibility should be done, not in an increasingly elaborate makeChildContainer(..) method.

      Show
      Paul Hammant added a comment - change number 4829 added a addChildToParent() method This is where such flexibility should be done, not in an increasingly elaborate makeChildContainer(..) method.
      Paul Hammant made changes -
      Resolution Fixed [ 1 ]
      Assignee Mauro Talevi [ maurotalevi ] Paul Hammant [ paul ]
      Status Open [ 1 ] Closed [ 6 ]

        People

        • Assignee:
          Paul Hammant
          Reporter:
          Aslak Hellesøy
        • Votes:
          0 Vote for this issue
          Watchers:
          1 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved: