AOP/Metadata integration config

[bill.burke]

I was just thinking about metadata and per bean AOP configuration. I was thinking of a generic ability to define metadata on a bean. It would be a special XML element and BeanMetaData would have an additional property called "Metadata". Metadata would be a map that you defined in the XML. This would allow AOP to receive any metadata it needed in any format it wanted.

for example:

<bean name="foo" class="org.jboss.Test">
 <metadata>
 <map>
 <entry>
 <key>AOP</key>
 <value>
 <list>
 <bean class="org.jboss.aop.advice.AdviceBinding">
 <property name="pointcut">execution(...)</property>
 <property name="advices">
 <list>
 <bean class="org.jboss.aop.advice.Advice>etc...</bean
 </list>
 </property
 </bean>
 <bean class="org.jboss.aop.annotation.AnnotationOverride"/>

 </value>
 <metadata>
</bean>

So, the ClassAdapter could query the BeanMetaData for AOP metadata and apply things like annotation overrides and such. This allows us to reuse the annotation override facility built into JBoss AOP yet gives us flexibility to provide any metadata for any subsystem that plugs into the ClassAdapter.

[bill.burke]

Ok, i'm almost done with this. I need to make a few changes though.

ClassAdapter.getDependencies() and JoinPointFactory.getConstructorJoinpoint to take a Map (metadata) so that they can receive AOP based metadata.

I've also had to modify BeanMetaData to add the additional metadata map.

I'm really starting to think that the ClassAdapter API should move to the kernel. I'm finding that I think the ClassAdapter will need access to the Kernel, or be able to be configured as any other bean in the system so that it can be initialized with various AOP policies/config.

[adrian.brock]

I'm confused again. We always seem to be on completely different wavelengths :-)

1) I already had a MAP in the original implementation, but changed it to Annotation
since we agreed that all config will come from annotations (as opposed to ad hoc
invocation payload) and this would make it easier for construct/configure from xml.
In fact, there is an open issue on whether the MC should just use AOP's annotation
metadata model.
There is even a test in the jboss-aspects module using AbstractAnnotationMetaData:
http://cvs.sourceforge.net/viewcvs.py/jboss/jboss-aspects/src/tests/org/jboss/test/aspects/microcontainer/test/InterceptorTestCase.java?rev=1.1&view=markup

Did you know that about FeatureMetaData or even the tasks I created after our Boston
meetings?
http://cvs.sourceforge.net/viewcvs.py/jboss/microkernel/src/main/org/jboss/beans/metadata/plugins/AbstractFeatureMetaData.java?r1=1.4&r2=1.5
http://jira.jboss.com/jira/browse/JBMICROCONT-18

2) It is too late to pass this in the join point constructor isn't it? I need to tell JBoss/AOP
about the annotations to find out what they mean in terms of dependencies????
This cannot be done at the construction stage, it must be done during DESCRIBE.
Otherwise, the MC/AOP integration serves no purpose.

 /**
 * Get a constructor join point
 *
 * @param constructorInfo the constructor info
 * @param metadata // THIS IS TOO LATE
 * @return the constructor join point
 * @throws JoinpointException when no such constructor
 */
 ConstructorJoinpoint getConstructorJoinpoint(ConstructorInfo constructorInfo, Map metadata) throws JoinpointException;

Maybe you have simplied this or found a way around the problem? But it
seems pretty fundamental to me?

3) The ClassAdapter should just be a bridge to JBoss/AOP or reflection
model. That is its only purpose. i.e. which implementation should the MC use.

To repeat the only reasons for the reflection model are:
a) I need to be convinced that JBoss/AOP (really javassist) can be used in all environments
b) AOP's ClassAdapter wasn't finished but I needed to work on other things

If (a) can be satisfied, (i.e. in all circumstances no matter what the security/memory
restrictions are JBoss/AOP/Javasssit can be used) then I don't have any problem
with predecating JBoss/MC on JBoss/AOP and the need for an abstract
integration "ClassAdapter" disappears.

[bill.burke]

I'm confused again. We always seem to be on completely different wavelengths :-)

1) I already had a MAP in the original implementation, but changed it to Annotation
since we agreed that all config will come from annotations (as opposed to ad hoc
invocation payload)

THis metadata I added has nothing to do with ad hoc invocation payloads. The metadata is needed by the ClassAdapter to resolve aspect dependencies and for the ConstructorJoinpoint so that it can either create a proxy or modify the object instance's InstanceAdvisor.

and this would make it easier for construct/configure from xml.

JBoss AOP already has an annotation override facility. What I added allows MC to take advantage of this facility without also tying MC to this facility.

In fact, there is an open issue on whether the MC should just use AOP's annotation
metadata model.

This sure would make things easier.

There is even a test in the jboss-aspects module using AbstractAnnotationMetaData:
http://cvs.sourceforge.net/viewcvs.py/jboss/jboss-aspects/src/tests/org/jboss/test/aspects/microcontainer/test/InterceptorTestCase.java?rev=1.1&view=markup

If you want to fully redo what JBoss AOP already does, be my guest, but count me out...

Did you know that about FeatureMetaData or even the tasks I created after our Boston
meetings?
http://cvs.sourceforge.net/viewcvs.py/jboss/microkernel/src/main/org/jboss/beans/metadata/plugins/AbstractFeatureMetaData.java?r1=1.4&r2=1.5
http://jira.jboss.com/jira/browse/JBMICROCONT-18

Sure saw it, but again, didn't want to rewrite the entire JBoss AOP annotation facility.

2) It is too late to pass this in the join point constructor isn't it? I need to tell JBoss/AOP
about the annotations to find out what they mean in terms of dependencies????

You mean for dependencies like the SecurityDomain example? I don't see why you need to tell JBoss AOP these dependencies. JBoss AOP doesn't care about dependencies. It really depends on who is analyzing the annotations for the @DependsAttribute annotation (or whatever it will be). If it is the microcontainer, then no, it is not too late as the microcontainer can just add these to the dependency list itself.

This cannot be done at the construction stage, it must be done during DESCRIBE.
Otherwise, the MC/AOP integration serves no purpose.

 /**
 * Get a constructor join point
 *
 * @param constructorInfo the constructor info
 * @param metadata // THIS IS TOO LATE
 * @return the constructor join point
 * @throws JoinpointException when no such constructor
 */
 ConstructorJoinpoint getConstructorJoinpoint(ConstructorInfo constructorInfo, Map metadata) throws JoinpointException;

Maybe you have simplied this or found a way around the problem? But it
seems pretty fundamental to me?

Did you see that I added passing metadata to ClassAdapter.getDependencies()? getConstructorJoinpoint needs to know about annotation overrides, mixin definitions, etc... because the ConstructorJoinpoint itself needs to know about this information so that it knows wheter or not to create a proxy (if the class is not weaved) or to interact with the per-instance InstanceAdvised interface. If you do not pass in these annotation overrides into the ConstructorJoinpoint, then the ConstructorJoinpoint does not have the exact information it needs to make its decisions.

To repeat the only reasons for the reflection model are:
a) I need to be convinced that JBoss/AOP (really javassist) can be used in all environments

THis idea that everything has to work in all environments with the 1st iteration or even the first release of MC, MC+AOP is just stupid. Iterate!

I've taken out the Javassist requirement for the MC /AOP integration. I've posted how/why on architecture council email list, I'll repost here.

Load-time AOP probably just will not work on all environments. AOPC precompilation should work in all environments because Javassist is take out of the equation. If you have look at my MC/AOP integration code and tests you'll see that it supports a proxy based model. Currently proxies are created by Javassist for performance and flexibility reasons, but this could be changed to be fully java.lang.reflect.Proxy based if you so desire.

b) AOP's ClassAdapter wasn't finished but I needed to work on other things

AOP's ClassAdapter is not fully finished. I'll talk about this in another post.

If (a) can be satisfied, (i.e. in all circumstances no matter what the security/memory
restrictions are JBoss/AOP/Javasssit can be used) then I don't have any problem
with predecating JBoss/MC on JBoss/AOP and the need for an abstract
integration "ClassAdapter" disappears.

(a) cannot uniformily be solved by JBoss AOP unless AOPC precompilation is a requirement. If you dislike AOPC precompilation as a requirement, then you just have to deal with less AOP funtionality and use proxies. JBoss AOP can support this and does partially support it now.

[adrian.brock]

Ok, let's deal with a fundamental problem first.

The MC does not necessarily use a ConstructorJoinPoint.
It could use a factory to create the object. See the ConstructorMetaData.

The idea of the ClassAdapter was that it would use this as a context for overrides
on the object being constructed.
The only other mechanism would be to pass the override context under the wire in a thread local.

Of course we could disallow factories as a mechanism to construct objects,
it certainly is a part of the javabean spec.
But this limits the number of object heirarchies that can be deployed using the MC.

[adrian.brock]

The reason for knowing about the SecurityDomain is a fundamental difference
in the deployment strategies of AOP vs MC

In AOP if the advice is not deployed it is not included in the stack.
If a depdendency of the advice is not deployed you find out after requests are
being processed with a runtime exception.

In the MC if the advice and its dependencies are not installed the object is not created
and the user is told to deploy the missing dependency.

[adrian.brock]

I don't mind the MC using the AOP annotation metadata format. My only concern would be
the potential in future for an annotation to include data that also requires DI.

[adrian.brock]

I don't mind the MC using the AOP annotation metadata format. My only concern would be
the potential in future for an annotation to include data that also requires DI.

And that the AOP version has a notion of instance???
I only want the annotation to apply to that object not all future objects of the same class.

[adrian.brock]

THis idea that everything has to work in all environments with the 1st iteration or even the first release of MC, MC+AOP is just stupid. Iterate!

+ others

At the time this was an important requirement because of the need for jmx/mc
interopability/backwards compatibilty.
This is less important now that JBoss5 will not be release of this software.

[adrian.brock]

The idea of the ClassAdapter was that it would use this as a context for overrides
on the object being constructed.

Anticipating you saying this contradicts my description of the ClassAdapter as
just a bridge.
Yes the ClassAdapter is a bridge, but that was why I had
ClassAdapter.getInstanceAdapter()
that created a context from which I retrieved joinpoints that knew about the context,
i.e. annotation overrides.

[bill.burke]

Ok, let's deal with a fundamental problem first.

The MC does not necessarily use a ConstructorJoinPoint.
It could use a factory to create the object. See the ConstructorMetaData.

The AOP container needs to intercept the allocation of the bean by the MC. Whether this is done by a constructor or factory method, it doesn't matter. AOP needs to intercept allocation so that it can put a proxy in place or initialize the InstanceAdvisor (depending whether the class is aspectized or not).

What I think we should do is ditch the ClassAdapter stuff and just have an AspectAdapter. An AspectAdapter should be created per bean. This would require a rewrite of your MC because allocation information and the ClassAdapter stuff is embedded within the BeanInfo class which is cached on a per-class basis.

I think the AspectAdapter interface should be as follows in live within the kernel/ module:

public interface AspectAdapterFactory {
 AspectAdapter createAspectAdapter(Class clazz, BeanMetaData metadata);
}

Yes, AOP Requires the java.lang.Class. Again, I refuse to use ClassInfos because it would require too much refactoring to JBoss AOP and creating ClassInfos with Javassist, is just way too slow. I have no desire to refactor Javassist as well to make it faster. I think you are going way overboard with dependency management here. Requiring too much work for too little reward.

The AspectAdapter interface might look something like this:

public interface AspectAdapter {
 public List getDependencies();
 public ConstructorJoinpoint getConstructorJoinpoint(ConstructorInfo);
 public MethodJoinpoint getFactoryJoinpoint();
 public FieldGetJoinpoint getFieldGetJoinpoint(FieldInfo);
 public FieldSetJoinpoint getFieldSetJoinpoint(FieldInfo);
 public MethodJonipoint getMethodJoinpoint(MethodInfo);
}

For JBoss AOP at least, the getMethodJoinpoint would not be needed. FieldGet/FieldSet are needed because JBoss AOP wraps field access in a static method call. A separate getFactoryJoinpoint and getConstructorJoinpoint is needed, because again, JBoss AOP needs to be able to intercept allocation to create a proxy or to interact with the aspectized class's object instance's InstanceAdvisor.

This is going to require a bunch of refactoring to MC because currently the classadapter is hidden by BeanInfos.

Bill

[adrian.brock]

It will take me a while to digest your proposal.
I don't see the meat of it (how it could possibly work) at the moment.

At first blush, this looks like my original proposal, which was that the MC would
push all the metadata to the AspectManager, but we decided against that solution
because
1) it would require the MC understanding all the point cut expressions, etc.
i.e. understanding the cross cutting.
2) users can still deploy using jboss-aop.xml which the MC wouldn't know about.

Instead, we decided on the query approach where the MC asks the AOP
layer "what would it mean if I created an instance of this object" during the
describe stage.

Correct me if I have misunderstood.

This is going to require a bunch of refactoring to MC because currently the classadapter is hidden by BeanInfos.

I don't mind refactoring, especially if it simplifies the problem.

I don't see why the ClassAdapter inside the BeanInfo creates a problem.
The BeanInfo is just a description of the class after AOP has played with it
and it is per ClassAdapter (which could be instance level) not per Class.

[adrian.brock]

On the joinpoint model:

This is serving more than just a dialogue between the MC and AOP.
It is acting as a cache for the reflection style objects.

Resolving reflection objects is much more expensive than using them
which is why the current EJB/JMX containers preresolve these objects.

They are also used by the Bus/Detyped invocations:

public interface KernelBus extends KernelObject
{
 /**
 * Invoke an operation
 *
 * @param name the name of the object
 * @param joinPoint the join point
 * @return the result
 * @throws Throwable for any error
 */
 Object invoke(Object name, TargettedJoinpoint joinPoint) throws Throwable;
}

This goes back to the "do we need to support a reflection implementation" question.

[adrian.brock]

I think you are going way overboard with dependency management here.

I think this goes to heart of the disagreement.

My vote is for using picocontainer with limited ability to support hot-deployment
or correctly startup a disparate group of loosy couple modules.

Leaving me to get on with doing something more useful than arguing about DI.

[adrian.brock]

I think you are going way overboard with dependency management here.

I think this goes to heart of the disagreement.

My vote is for using picocontainer with limited ability to support hot-deployment
or correctly startup a disparate group of loosy couple modules.

Leaving me to get on with doing something more useful than arguing about DI.

A joke, but like most good jokes it is not far from the truth. :-)