Ayende @ Rahien

It's a girl

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:

image

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.

image

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:

image

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:

image

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

Andrey Shchekin
12/19/2007 07:24 PM by
Andrey Shchekin

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?

Andrey Shchekin
12/19/2007 07:24 PM by
Andrey Shchekin

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?

Jacques Philip
12/19/2007 07:48 PM by
Jacques Philip

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.

Ayende Rahien
12/19/2007 07:59 PM by
Ayende Rahien

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.

Ayende Rahien
12/19/2007 08:01 PM by
Ayende Rahien

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

Francois Tanguay
12/19/2007 08:56 PM by
Francois Tanguay

So the solution was?

Is there a way to have hybrid interception where both virtual and interface based strategies are used?

Ayende Rahien
12/19/2007 08:59 PM by
Ayende Rahien

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

Matt
12/20/2007 03:23 AM by
Matt

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?

Fabian Schmied
12/20/2007 09:26 AM by
Fabian Schmied

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.

Mats Helander
12/20/2007 10:28 AM by
Mats Helander

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

Alex Simkin
12/20/2007 02:14 PM by
Alex Simkin

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?

Ayende Rahien
12/20/2007 03:48 PM by
Ayende Rahien

Alex,

Yes. But this is something that was caught.

Alex Simkin
12/20/2007 04:41 PM by
Alex Simkin

Thank you! I am making presentation on TDD, not -against- TDD :)

Comments have been closed on this topic.