Allow for ordering of mixed AspectJ before/after advices

Issue: SPR-9438

spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -53,7 +53,6 @@ class AspectJPrecedenceComparator implements Comparator {
 	private static final int HIGHER_PRECEDENCE = -1;
 	private static final int SAME_PRECEDENCE = 0;
 	private static final int LOWER_PRECEDENCE = 1;
-	private static final int NOT_COMPARABLE = 0;
 
 	private final Comparator<? super Advisor> advisorComparator;
 
@@ -85,21 +84,11 @@ class AspectJPrecedenceComparator implements Comparator {
 
 		Advisor advisor1 = (Advisor) o1;
 		Advisor advisor2 = (Advisor) o2;
-
-		boolean oneOrOtherIsAfterAdvice =
-				(AspectJAopUtils.isAfterAdvice(advisor1) || AspectJAopUtils.isAfterAdvice(advisor2));
-		boolean oneOrOtherIsBeforeAdvice =
-				(AspectJAopUtils.isBeforeAdvice(advisor1) || AspectJAopUtils.isBeforeAdvice(advisor2));
-		if (oneOrOtherIsAfterAdvice && oneOrOtherIsBeforeAdvice) {
-			return NOT_COMPARABLE;
-		}
-		else {
-			int advisorPrecedence = this.advisorComparator.compare(advisor1, advisor2);
-			if (advisorPrecedence == SAME_PRECEDENCE && declaredInSameAspect(advisor1, advisor2)) {
-				advisorPrecedence = comparePrecedenceWithinAspect(advisor1, advisor2);
-			}
-			return advisorPrecedence;
+		int advisorPrecedence = this.advisorComparator.compare(advisor1, advisor2);
+		if (advisorPrecedence == SAME_PRECEDENCE && declaredInSameAspect(advisor1, advisor2)) {
+			advisorPrecedence = comparePrecedenceWithinAspect(advisor1, advisor2);
 		}
+		return advisorPrecedence;
 	}
 
 	private int comparePrecedenceWithinAspect(Advisor advisor1, Advisor advisor2) {

spring-aop/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparatorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,12 +16,11 @@
 
 package org.springframework.aop.aspectj.autoproxy;
 
-import static org.junit.Assert.*;
-
 import java.lang.reflect.Method;
 
 import org.junit.Before;
 import org.junit.Test;
+
 import org.springframework.aop.Advisor;
 import org.springframework.aop.AfterReturningAdvice;
 import org.springframework.aop.BeforeAdvice;
@@ -35,11 +34,13 @@ import org.springframework.aop.aspectj.AspectJMethodBeforeAdvice;
 import org.springframework.aop.aspectj.AspectJPointcutAdvisor;
 import org.springframework.aop.support.DefaultPointcutAdvisor;
 
+import static org.junit.Assert.*;
+
 /**
  * @author Adrian Colyer
  * @author Chris Beams
  */
-public final class AspectJPrecedenceComparatorTests {
+public class AspectJPrecedenceComparatorTests {
 
 	private static final int HIGH_PRECEDENCE_ADVISOR_ORDER = 100;
 	private static final int LOW_PRECEDENCE_ADVISOR_ORDER = 200;
@@ -89,7 +90,7 @@ public final class AspectJPrecedenceComparatorTests {
 	public void testSameAspectOneOfEach() {
 		Advisor advisor1 = createAspectJAfterAdvice(HIGH_PRECEDENCE_ADVISOR_ORDER, EARLY_ADVICE_DECLARATION_ORDER, "someAspect");
 		Advisor advisor2 = createAspectJBeforeAdvice(HIGH_PRECEDENCE_ADVISOR_ORDER, LATE_ADVICE_DECLARATION_ORDER, "someAspect");
-		assertEquals("advisor1 and advisor2 not comparable", 0, this.comparator.compare(advisor1, advisor2));
+		assertEquals("advisor1 and advisor2 not comparable", 1, this.comparator.compare(advisor1, advisor2));
 	}
 
 	@Test