From d3969de101e018515b715f32e1e189a8fabda97c Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 7 Feb 2013 14:11:24 +0100 Subject: [PATCH] Allow for ordering of mixed AspectJ before/after advices Issue: SPR-9438 --- .../AspectJPrecedenceComparator.java | 21 +++++-------------- .../AspectJPrecedenceComparatorTests.java | 11 +++++----- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java index 0dd176354e..31fdb83f7d 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparator.java +++ b/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 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) { diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparatorTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparatorTests.java index 08d95808ed..96313821a3 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJPrecedenceComparatorTests.java +++ b/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