7 Replies Latest reply on Jun 5, 2007 12:26 PM by Kabir Khan

    New Lifecycle aspects is wrong

    Adrian Brock Master

      This relates to this long discussion thread:
      http://www.jboss.com/index.html?module=bb&op=viewtopic&t=101253

      I finally got chance to look at what has been done for this and it is wrong.

      1) Type safety

      I quite like the idea of the BeanInfo.getDependencies()
      returning a builder rather than just names so it can be called back
      with additional context on what it needs to work on.

      But this needs to be type safe.
      e.g. This code shows the problem (the cast - which isn't even checked properly!)

       // add custom dependencies (e.g. AOP layer).
       List<Object> dependencies = info.getDependencies(md);
       log.trace("Extra dependencies for " + context.getName() + " " + dependencies);
       if (dependencies != null)
       {
       for (Object dependencyItem : dependencies)
       {
       ((DependencyBuilderListItem)dependencyItem).addDependency(context);
       }
       }
      


      2) BeanMetaData is not a place to store runtime information
      that is what the ControllerContext is for.

      You should not be modifying the BeanMetaData. This is information
      that the user/client is requesting. Any additional information runtime information
      should be stored in the ControllerContext, i.e. linked in via DependencyInfo/Item

      This is how the initial runtime/additional depedencies were coded and that rule
      must be maintained.

      Ales also made the same mistake, so I guess we need a big warning
      message on BeanMetaData to stop people adding things to the model
      that are not declartive (intended to be declared by the user)? :-)

      3) Too much of this implementation detail is being leaked into spi.

      e.g. the KernelControllerContext interface
       /**
       * Get the lifecycle callbacks for a particular state.
       *
       * @param state the state callbacks refer to
       * @return List<LifecycleCallbackMetaData>
       */
       List<LifecycleCallbackMetaData> getLifecycleCallbacks(ControllerState state);
      


      Besides what I said on the other thread that this is too POJO specifc,
      it is also an implementation detail that should not need to be exposed on the interface.

      This is also something else that I've tried to maintain in the Microcontainer api.

      If the processing is generic state that is going to be useful for all implementations
      of an interface then it belongs on thatinterface, Otherwise it should be
      a method specific to an implementation and the integration should use
      "private api". e.g. I wouldn't have a problem if this was on the
      Abstract[Kernel]ControllerContext.