1 2 Previous Next 26 Replies Latest reply on Mar 28, 2006 10:26 AM by Kabir Khan

    ClassAdapter vs MetaDataContext

    Adrian Brock Master

      I've done some work on refactoring the ClassAdapter and configuration
      to remove some of the stupid depedencies and the need to override
      about 4 classes just to use your own ConstructorJoinpoint method.

      In fact, I've removed the AOPClassAdaptor altogether for now
      since after these changes it was just delegating to the BasicClassAdapter.

      I've added a test that shows why we might want to reintroduce it.
      Which is the InterceptorWithDependencyTestCase.
      This fails (but not for the reason I was trying to demonstrate :-)

      Basically what I need is for the AOP layer to return the list of
      Advice factories (which should have the same name as the bean deployment).
      Bill envisioned this being done via the ClassAdapter.getDependencies().

      There is this code in the attic where he had it working in a previous
      prototype:
      http://fisheye.jboss.org/viewrep/JBoss/jboss-aspects/src/main/org/jboss/aspects/kernel/AspectClassAdapter.java?r=1.12

        • 1. Re: ClassAdapter vs MetaDataContext
          Adrian Brock Master

          I'm not convinced by the ClassAdapter api.

          It does serve as a good point to hold "instance level" metadata
          before the instance exists, but it is not a good way to override
          implementation because it is dealing with too many concerns.

          The refactoring I did was to introduce a BasicClassAdaptor that
          just delegates to a Configuration object. The idea being that
          this configuration object is where the implementation is defined.

          It could even do something similar to what it does now
          which is to automagically discover which one to use based upon
          what is in the classpath. But obviously this needs improving
          to make it more configurable and better handle errors.

          • 3. Re: ClassAdapter vs MetaDataContext
            Kabir Khan Master

            It's been a while since I last looked at Bill's prototype, but I will do tomorrow to refresh my memory of how he got the dependencies set up.

            I had a play with prototyping the dependencies in the current prototype earlier this week, and there is a problem stemming from the AspectBindings etc. all being defined as beans. My understanding is that the AdviceBindings do not get added until the AdviceBinding bean is started, which means they are nowhere in aop when the pojos we want to instantiate are created. The exception to this is if the aop stuff has all been configured and started before we get to describing the pojos we want to advise.

            • 4. Re: ClassAdapter vs MetaDataContext
              Adrian Brock Master

              I think you are correct. AOP obviously cannot tell the MC
              to wait for aspects that haven't been deployed yet.

              This looks like a failing of my prototype aspect binding beans.

              The point is supposed to be that AOP knows that an aspect
              should apply to a bean.

              So we obviously need to be able to deploy the pointcut definition,
              even if the aspect itself cannot be fully deployed because it is waiting for
              a dependency.

              Transitively - via the getDependencies() - this means the beans that match
              the pointcut don't get deployed.

              • 5. Re: ClassAdapter vs MetaDataContext
                Adrian Brock Master

                The way Bill had it working was a bit of a kludge,
                but it is probably good enough.

                His aspect factory doesn't contain a direct reference to the
                GenericBeanFactory (the real advice factory)
                that has dependencies waiting to be deployed.

                Instead it just gets the name of it - the one that will
                be returned by getDependencies()

                He then looks it up using the KernelController when somebody
                wants to create an advice.
                http://fisheye.jboss.org/viewrep/~raw,r=1.6/JBoss/jboss-aspects/src/main/org/jboss/aspects/kernel/GenericKernelAspectFactory.java

                This looks like a fairly good assumption.
                * The bean that matches the pointcut will depend upon the
                real advice.
                * The AspectFactory and AspectDefinition can be deployed straight away
                * Nobody will actually use the AspectFactory/Definition until the
                real advice factory is deployed because they have a depends on it
                from the getDependencies()

                • 6. Re: ClassAdapter vs MetaDataContext
                  Adrian Brock Master

                  So it should just be a case of changing this:

                  
                   <bean name="InterceptedAspect" class="org.jboss.aop.microcontainer.prototype.Aspect">
                  - <property name="advice"><inject bean="InterceptedAdvice"/></property>
                  + <property name="advice">InterceptedAdvice</property>
                   <property name="manager"><inject bean="AspectManager"/></property>
                   </bean>
                  


                  And then doing the kernel controller lookup at advice construction time
                  like Bill does.

                  I'll look at this tomorrow.

                  • 7. Re: ClassAdapter vs MetaDataContext
                    Bill Burke Master

                    yes...I told you I had this shit working...

                    • 8. Re: ClassAdapter vs MetaDataContext
                      Adrian Brock Master

                      I made the changes, but it appears to want to create the advice
                      when I add the aspect binding?

                      9144 ERROR [AbstractKernelController] Error installing to Start: name=InterceptedBinding state=Create
                      java.lang.RuntimeException: java.lang.IllegalStateException: Advice InterceptedAdvice is not installed
                       at org.jboss.aop.advice.AdviceFactory.create(AdviceFactory.java:72)
                       at org.jboss.aop.Advisor.createInterceptorChain(Advisor.java:639)
                       at org.jboss.aop.Advisor.pointcutResolved(Advisor.java:907)
                       at org.jboss.aop.Advisor.resolveMethodPointcut(Advisor.java:671)
                       at org.jboss.aop.ClassContainer.createInterceptorChains(ClassContainer.java:247)
                       at org.jboss.aop.ClassContainer.rebuildInterceptors(ClassContainer.java:113)
                       at org.jboss.aop.Advisor.newBindingAdded(Advisor.java:509)
                       at org.jboss.aop.AspectManager.updateAdvisorsForAddedBinding(AspectManager.java:1342)
                       at org.jboss.aop.AspectManager.updateAdvisorsForAddedBinding(AspectManager.java:1365)
                       at org.jboss.aop.AspectManager.addBinding(AspectManager.java:1314)
                       at org.jboss.aop.microcontainer.prototype.AspectBinding.start(AspectBinding.java:138)
                      


                      • 9. Re: ClassAdapter vs MetaDataContext
                        Adrian Brock Master

                        This is the root cause

                        Caused by: java.lang.IllegalStateException: Advice InterceptedAdvice is not installed
                         at org.jboss.aop.microcontainer.prototype.LazyGenericBeanAspectFactory.doCreate(LazyGenericBeanAspectFactory.java:86)
                         at org.jboss.aop.microcontainer.prototype.LazyGenericBeanAspectFactory.createPerVM(LazyGenericBeanAspectFactory.java:57)
                         at org.jboss.aop.AspectManager.getPerVMAspect(AspectManager.java:1657)
                         at org.jboss.aop.AspectManager.getPerVMAspect(AspectManager.java:1646)
                         at org.jboss.aop.Domain.getPerVMAspect(Domain.java:223)
                         at org.jboss.aop.advice.PerVmAdvice.generateOptimized(PerVmAdvice.java:51)
                         at org.jboss.aop.advice.AdviceFactory.create(AdviceFactory.java:68)
                        


                        • 10. Re: ClassAdapter vs MetaDataContext
                          Adrian Brock Master

                          This looks to be a sideaffect of the proxy stuff?


                           public void updateAdvisorsForAddedBinding(AdviceBinding binding)
                          ...
                           Collection keys = subscribedSubDomains.keySet();
                           if (keys.size() > 0)
                           {
                           //When interceptors are installed as beans in the microcontainer, creating the interceptor instances
                           for (Iterator it = keys.iterator() ; it.hasNext() ; )
                           {
                           Domain domain = (Domain)it.next();
                           domain.updateAdvisorsForAddedBinding(binding);
                           }
                           }
                          ...
                          


                          • 11. Re: ClassAdapter vs MetaDataContext
                            Kabir Khan Master

                             

                            "adrian@jboss.org" wrote:
                            This looks to be a sideaffect of the proxy stuff?


                            What you have pasted is there for the main aop 2 weaving model. Every advised class gets its own domain, which is a child of the aspect manager. Then each instance advisor gets its own domain which is a child of the classadvisor's domain

                            However, the stuff surrounding this is to avoid ConcurrentModificationExceptions if updating the chains needs to create a new child domain (which the proxy does)
                             synchronized (subscribedSubDomains)
                             {
                             copySubDomainsFromQueue(true);
                             boolean newSubscribers = true;
                             while (newSubscribers)
                             {
                             Collection keys = subscribedSubDomains.keySet();
                             if (keys.size() > 0)
                             {
                             //When interceptors are installed as beans in the microcontainer, creating the interceptor instances
                             for (Iterator it = keys.iterator() ; it.hasNext() ; )
                             {
                             Domain domain = (Domain)it.next();
                             domain.updateAdvisorsForAddedBinding(binding);
                             }
                             }
                             newSubscribers = copySubDomainsFromQueue(false);
                             }
                             }
                            
                            


                            • 12. Re: ClassAdapter vs MetaDataContext
                              Kabir Khan Master

                              I mean the interceptors that are beans are instantiated via the mc, which in turn goes to the proxy factory which needs to create a subdomain to see if it is advised and if so create a proxy

                              • 13. Re: ClassAdapter vs MetaDataContext
                                Adrian Brock Master

                                So the problem should fix itself if this bean:

                                 <bean name="Intercepted" class="org.jboss.test.microcontainer.support.SimpleBeanImpl"/>
                                


                                waits for the advice factory to be installed - via the getDependencies().
                                There won't be a subdomain that wants to create the advice.

                                • 14. Re: ClassAdapter vs MetaDataContext
                                  Adrian Brock Master

                                  Kabir, this last commit is wrong:

                                  "
                                  Modified: src/main/org/jboss/beans/info/plugins AbstractBeanInfo.java
                                  Log:
                                  MetaDataContext should be stored in BeanInfo (per instance) rather than in ClassAdapter (per class)

                                  Make a start at defining dependencies
                                  "

                                  There should be a one-one correspondance between BeanInfo
                                  and ClassAdapter.

                                  The ClassAdapter is per class by default, but you should clone it
                                  using the getInstanceAdapter() if you want to override the class defaults
                                  (like add annotations).

                                  1 2 Previous Next