Dynamic AOP API

[flavia.rainone]

As discussed in JBW, we are going to define a new, state of the art Dynamic AOP API for the next JBoss AOP version (after we go GA).

I decided to give the initial kick and start this thread.

Mainly, I see two options here: improving the existing API or coming up with a brand new API (an adaptation layer to our current API)

Improving the existent API would consist mainly of creating more intuitive constructors for AspectDefinition and AdviceBinding (such as removing cflow, and allowing definition of cflow using varargs), and adding more methods to AdviceBinding (addAdvice(Class<?> aspectClass, String adviceName)).

The advantage of doing so is that we stick with our already existing classes.

One of the problems I see with extending the existent API is that we would still force the user to register secondary stuff in the AspectManager, such as cflow and aspect definitions.

Besides, with a new API, we would be free to make dynamic AOP more straighforward, and it could be simpler for the end user to use it if he doesn't have to use AspectManager (I mean, there are so many methods in AspectManager, everything being mixed up in a single class can make users confused).

So, Kabir, what do you have in mind for this?

[kabirkhan]

I think at present the API is way to complicated, and gives the user access to all sorts of internal things. I think a greatly simplified API is the way to go.

For example, the -aop.xml format is straightforward, and gets resolved by AspectXmlLoader into the underlying dynamic API, which is the API currently being exposed to the users. I think in a similar way, the new dynamic API should be greatly simplified, but internally map onto the existing dynamic stuff.

[flavia.rainone]

I agree that we should design a new API. IMHO the design of this API poses quite a challenge to the JBoss AOP team. Being dynamic is one of the strongest points of JBoss AOP and yet we need to provide something easy and intuititve to use, without taking out the power of flexibility (or, at least, try to balance simplicity with flexibililty).

I think we should be able of providing most of the options a user has when defining an xml. And I also think it would be a great step to provide overloaded methods as a way of avoiding null parameters. I mean, if you need to use cflow, choose the method with cflow parameter. If you don't need, passing a null cflow is not accepted, use the method without cflow instead (in the current API, you must use null in AdviceBinding constructor to indicate that a binding won't use cflow).

What do you say about annotation overrides? Maybe it would be an interesting feature to add it to our dynamic API? Or you think it would just make things too complicated?

[kabirkhan]

I think we should be able to add everything that has any bearing on dynamic aop, which would include annotation overrides.

Just a half-baked thought, instead of creating something with loads of overloaded methods like:

addBinding(String pointcut, String cflow, AdviceInfo[] advices);
addBinding(String pointcut, AdviceInfo[] advices);
//And other variations

it might be better to encapsulate these in a parameter object

interface DynamicBindingOperation{
 void setPointcut(String pointcut);
 void setAdvices(AdviceInfo[] advices);
 void setCFlow(String cflow);
}

And then on the facade have methods like

DynamicBindingOperation createDynamicBindingOperation();
void addDynamicBindingOperation(DynamicBindingOperation dbo);

The user would then call the create method, add whatever info via the DBO setters, and pass this in to the add method, which in turn will validate that everything that is required is part of the DBO, create the relevant artifacts and install them in the AspectManager.

[kabirkhan]

The DBO could also contain a "handle" populated by the add method. We talked about needing an easy way to be able to roll back things that had been added

[flavia.rainone]

Yes, the DBO is a nice idea. The handle is not only needed for removal, but also for binding updates.

I think we should also provide some sort of dynamic AOP transaction, useful for two reasons:

performance: it is faster to perform a single dynamic AOP operation than two or three (think about caller pointcuts, and about hot swap)
consistency: if two aspects work only together, the user won't want to risk having one of them enabled for a few ms without the other one. Other scenario that would need this is when a set of aspects is to be removed, so they can be replaced by a second set of aspects. The user won't want to risk running the joinpoints without aspects, nor with the two sets of aspects at the same time.

This way, the user has an option of opening a transaction, indicating that all operations inside it will be performed in an atomic way, assuring consistency:

DynamicAOP.start();
DynamicAOP.add(dbo1, dbo2, dbo3);
DynamicAOP.remove(oldDbo1.getHandle(), oldDbo2.getHandle(), oldDbo3.getHandle());
DynamicAOP.commit();

If, on the other hand, the user does not start a transaction, each operation will be an atomic unit:

DynamicAOP.add(dbo1, dbo2, dbo3);
DynamicAOP.remove(oldDbo1.getHandle(), oldDbo2.getHandle(), oldDbo3.getHandle());

Naturally, the start and commit methods could have better names, I just haven't come up with a nice idea for them.

[flavia.rainone]

I've been thinking about this API, and I realized that, if I was going to use it, I would rather do this:

String pointcut = "execution(*->new(..))";
Object handle = DynamicAOP.bind(pointcut, MyAspect.class, "adviceName");

Than this:

String pointcut = "execution(*->new(..))";
DynamicBindingOperation dbo = new DynamicBindingOperation();
dbo.setPointcut(pontcut);
AdviceInfo advice = new AdviceInfo(MyAspect.class, "adviceName");
dbo.setAdviceInfo(advice);
DynamicAOP.execute(dbo);

I mean, for simple operations we should try to keep it simple, making it possible to execute the whole thing in a couple of lines?
In this case, I think we could maintain the dbo, using it with more complex bindings.

[kabirkhan]

I mean, for simple operations we should try to keep it simple, making it possible to execute the whole thing in a couple of lines?
In this case, I think we could maintain the dbo, using it with more complex bindings.

Agreed