ClassAdapter vs MetaDataContext

[adrian.brock]

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

[adrian.brock]

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.

[adrian.brock]

The JIRA task for this is here:
http://jira.jboss.com/jira/browse/JBMICROCONT-14

[kabirkhan]

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.

[adrian.brock]

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.

[adrian.brock]

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()

[adrian.brock]

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.

[bill.burke]

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

[adrian.brock]

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)

[adrian.brock]

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)

[adrian.brock]

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);
 }
 }
...

[kabirkhan]

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);
 }
 }

[kabirkhan]

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

[adrian.brock]

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.

[adrian.brock]

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).