+972 52-548-6969
AOP: Be aware where your point cuts are
So, this issue cause some head scratching today. We are using WIndsor's Automatic Transaction Management with NHibernate's flush-on-commit option, so if a transaction doesn't commit, nothing is written to the database.
Anyway, this is a story about refactoring, and what it showed us. We performed the following refactoring:
Some things that is important to understand, the LoginController is decorated with [Transactional], and there is a [Transaction] attribute on CreateUserLoggedInAuditRecord.
When it was on the controller, it just worked. When we moved it to its own class, it didn't work. To be rather more exact, it worked, it just never committed the transaction. That was weird. After some head scratching I found out that I forgot to put [Transactional] on the UsageRegistrationImpl. With a small smile of geeky triumph, I run the code again. It didn't save.
That was really worrying, and I had no idea what was going on. Since this is rarely popular, I repeatedly run the code, hoping that something would turn up and that no one would pull the old quote about insanity.
After a few repetitions, I suddenly saw the light.
It had to do where I placed the pointcut. A pointcut, in AOP terms, is where the AOP can interfere with the running code. Let us take a look at how it worked when we used the LoginController directly. Because we (well, the transaction facility) asked the container to create an interceptor for it, we got the following classes at runtime:
The login controller is the original class, the login controller proxy was generated at runtime, and any invocation of any of its methods would fire the transaction interceptor, so it would get a chance to create/rollback/commit a transaction if needed. Since those methods are virtual, this means that even if I am calling methods on the same class, they will be intercepted correctly.
Now, when I moved to the interface + implementing class, we have a different behavior. Now, we use the interface pointcuts in order to inject behavior, it looks like this:
Windsor will create a proxy interface implementation that would call the AOP interceptors and will forward to the UsageRegistrationImpl.
The problem was with the RegisterUserLoggedIn method. It was similar to this:
public virtual void RegisterUserLoggedIn(string username) { // do other things CreateUserLoggedInAuditRecord(username); } [Transaction] public virtual void CreateUserLoggedInAuditRecord(string username) { //do database stuff }
Given the story so far, you can obviously see the problem. When we call the CreateUserLoggedInAuditRecord() method, we call it from the UsageRegistrationImpl class, so we never pass through any of the pointcuts.
When we used the method from the controller directly, we made a virtual method call, which was intercepted, but since in this case, we were using the interface as our pointcut, this simply by passed the whole thing.
That was an interesting lesson, and one that I'll need to remember for the future.
Comments
I do not know AOP well enough, so, am I right when I see it as a container/proxy problem that does not exist when AOP is in language/platform itself?
And a nitpicking question: do you think that *Impl is a good name?
I do not know AOP well enough, so, am I right when I see it as a container/proxy problem that does not exist when AOP is in language/platform itself?
And a nitpicking question: do you think that *Impl is a good name?
I had a problem maybe related:
public virtual bool Save()
{
return Save(false);
}
[Transaction]
public virtual bool Save(bool skipValidation)
{
Code for saving...
}
When I called Save(), the transaction did not happen and the record was not saved even though the Saving code executed in the second overload that had a Transaction attribute.
I needed to have a Transaction attribute on the first one too...
I figured it out, but without having the full understanding of why.
Andrey,
No, this is generic AOP problem, it means that you need to be aware of where your pointcuts are when you rely on their implementation.
Yes, *Impl is a good idea, at times.
Jacques,
This is really something that depend on what you are doing. Yet, it is possible that calling save this way will cause just this issue, if you are using interface proxies for pointcuts
So the solution was?
Is there a way to have hybrid interception where both virtual and interface based strategies are used?
Fancious,
No, I don't think there are hybrids, it is certainly possible, but not something that the ATM supports at the moment.
There are several solutions for this, manual transaction management, put [Transaction] on the register, use With.Transaction, etc
Being completely off topic.... but what program do you use to make your diagrams? I know the first one looks like it was made in the Visual Studio class browser, but what about the last two?
Not to be nitpicking, but you're actually talking about "join points", not "pointcuts". A join point is where your aspects (potentially) interfere with the application. A pointcut, on the other hand, is a (usually declarative) selection of a subset of the possible join points.
In your example, all virtual methods on LoginController are potential join points (due to the way Windsor intercepts the methods on LoginController via the subclass proxy), whereas the methods on UsageRegistrationImpl do not constitute join points (because there is no interception going on for this class).
The [Transaction] attribute would probably constitute a pointcut specification. In your first implementation, the pointcut selected the LoginController.CreateUserLoggedInAuditRecord method. In the second implementation, it selected nothing.
So - be aware of where your join points are when formulating pointcuts.
BTW, that's an issue that would have probably been detected by an AOP-aware compiler - after all, a pointcut that doesn't select any join points is almost always a mistake.
Agreeing with Fabian ^
Also,
"BTW, that's an issue that would have probably been detected by an AOP-aware compiler - after all, a pointcut that doesn't select any join points is almost always a mistake."
Indeed, but even a Visualisation tool of any kind, showing how aspects are applied to targets according to the defined pointcuts, would have saved the day.
In my opinion, subclass proxies are preferrable to interface based ones for just this reason (calls from a target instance to its own methods will not go via the proxy and so go unintercepted). To me, that means interface based proxy generation is very nearly broken.
/Mats
What a beautiful example of how refactoring can break your code!
It doesn't matter how tight is your TDD "safety net" is, if you swim in murky waters of AOP and other "auto-magical" technologies you finaly revert to the ye olde debugger...
Could I please have your permission to use your post in my presentation on TDD?
Alex,
Yes. But this is something that was caught.
Thank you! I am making presentation on TDD, not -against- TDD :)
Comment preview