The default Aspect Order should not be Ordered.LOWEST_PRECEDENCE [SPR-5453]

[spring-projects-issues]

Kek opened SPR-5453 and commented

I have a situation, when I need to guarantie that my Aspect is the last one in the invocation chain. So I annotated the Aspect class with @Order(Ordered.LOWEST_PRECEDENCE). But this does not work, after some hours of debuging I found, that the problem is in BeanFactoryAspectInstanceFactory.getOrder() method, because all other aspects, without @Order and without "implements Ordered", have the default order value Ordered.LOWEST_PRECEDENCE.

public int getOrder() {
	Class type = this.beanFactory.getType(this.name);
	if (type != null) {
		if (Ordered.class.isAssignableFrom(type) && this.beanFactory.isSingleton(this.name)) {
			return ((Ordered) this.beanFactory.getBean(this.name)).getOrder();
		}
		Order order = (Order) type.getAnnotation(Order.class);
		if (order != null) {
			return order.value();
		}
	}
	return Ordered.LOWEST_PRECEDENCE;
}

So it is not possible to create some Aspect with lower precedence than the "not ordered" aspect. In my opinion, the default order value should be something between Ordered.LOWEST_PRECEDENCE and Ordered.HIGHEST_PRECEDENCE like Ordered.DEFAULT_PRECEDENCE=0. It is the same idea as in java.lang.Thred.priority, there is MIN_PRIORITY, NORM_PRIORITY, MAX_PRIORITY. But I know, there will be problem with backward compatibility.

I didn't find any way how to change the BeanFactoryAspectInstanceFactory implementation for my aspectj-autoproxy configuration, so my current solution is to define @Order on all Aspects.

Thank's for any other better solution.

---

Affects: 2.5.6

2 votes, 4 watchers

[spring-projects-issues]

Stéphane Nicoll commented

We have a similar issue (without AOP) where we order the application events in such a way that the one tagged with Ordered.LOWEST_PRECEDENCE is the last one. Unfortunately, other app events without this annotation can be invoked after it.

Changing this contract now will obviously break a lot of applications but changing the logic in such a way that the event without the annotation comes just before the one tagged with Ordered.LOWEST_PRECEDENCE might do the trick.

Any thoughts? While relying on this information is sometimes bad practices, there is a need to make sure something will come last (just the same way as the original intention of this feature, e.g. to make sure something will come first).

[spring-projects-issues]

Stéphane Nicoll commented

We've discussed this suggestion and I am afraid we can't afford to change the default at this point.