Polishing

(cherry picked from commit de4ff4b)

spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJAdviceParameterNameDiscoverAnnotationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2018 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,9 +16,6 @@
 
 package org.springframework.aop.aspectj;
 
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.junit.Test;
 
@@ -30,13 +27,7 @@ import org.junit.Test;
  * @author Adrian Colyer
  * @author Chris Beams
  */
-public class AspectJAdviceParameterNameDiscoverAnnotationTests
+public class AspectJAdviceParameterNameDiscoverAnnotationTests extends AspectJAdviceParameterNameDiscovererTests {
-		extends AspectJAdviceParameterNameDiscovererTests {
-
-	@Retention(RetentionPolicy.RUNTIME)
-	@interface MyAnnotation {}
-
-	public void pjpAndAnAnnotation(ProceedingJoinPoint pjp, MyAnnotation ann) {}
 
 	@Test
 	public void testAnnotationBinding() {
@@ -45,4 +36,9 @@ public class AspectJAdviceParameterNameDiscoverAnnotationTests
 				new String[] {"thisJoinPoint","ann"});
 	}
 
+
+	public void pjpAndAnAnnotation(ProceedingJoinPoint pjp, MyAnnotation ann) {}
+
+	@interface MyAnnotation {}
+
 }

spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJAdviceParameterNameDiscovererTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.aop.aspectj;
 
 import java.lang.reflect.Method;
@@ -34,47 +35,6 @@ import static org.junit.Assert.*;
  */
 public class AspectJAdviceParameterNameDiscovererTests {
 
-	// methods to discover parameter names for
-	public void noArgs() {
-	}
-
-	public void tjp(JoinPoint jp) {
-	}
-
-	public void tjpsp(JoinPoint.StaticPart tjpsp) {
-	}
-
-	public void twoJoinPoints(JoinPoint jp1, JoinPoint jp2) {
-	}
-
-	public void oneThrowable(Exception ex) {
-	}
-
-	public void jpAndOneThrowable(JoinPoint jp, Exception ex) {
-	}
-
-	public void jpAndTwoThrowables(JoinPoint jp, Exception ex, Error err) {
-	}
-
-	public void oneObject(Object x) {
-	}
-
-	public void twoObjects(Object x, Object y) {
-	}
-
-	public void onePrimitive(int x) {
-	}
-
-	public void oneObjectOnePrimitive(Object x, int y) {
-	}
-
-	public void oneThrowableOnePrimitive(Throwable x, int y) {
-	}
-
-	public void theBigOne(JoinPoint jp, Throwable x, int y, Object foo) {
-	}
-
-
 	@Test
 	public void testNoArgs() {
 		assertParameterNames(getMethod("noArgs"), "execution(* *(..))", new String[0]);
@@ -221,22 +181,26 @@ public class AspectJAdviceParameterNameDiscovererTests {
 
 	@Test
 	public void testThisAndPrimitive() {
-		assertParameterNames(getMethod("oneObjectOnePrimitive"), "args(count) && this(obj)", new String[] {"obj", "count"});
+		assertParameterNames(getMethod("oneObjectOnePrimitive"), "args(count) && this(obj)",
+				new String[] {"obj", "count"});
 	}
 
 	@Test
 	public void testTargetAndPrimitive() {
-		assertParameterNames(getMethod("oneObjectOnePrimitive"), "args(count) && target(obj)", new String[] {"obj", "count"});
+		assertParameterNames(getMethod("oneObjectOnePrimitive"), "args(count) && target(obj)",
+				new String[] {"obj", "count"});
 	}
 
 	@Test
 	public void testThrowingAndPrimitive() {
-		assertParameterNames(getMethod("oneThrowableOnePrimitive"), "args(count)", null, "ex", new String[] {"ex", "count"});
+		assertParameterNames(getMethod("oneThrowableOnePrimitive"), "args(count)", null, "ex",
+				new String[] {"ex", "count"});
 	}
 
 	@Test
 	public void testAllTogetherNow() {
-		assertParameterNames(getMethod("theBigOne"), "this(foo) && args(x)", null, "ex", new String[] {"thisJoinPoint", "ex", "x", "foo"});
+		assertParameterNames(getMethod("theBigOne"), "this(foo) && args(x)", null, "ex",
+				new String[] {"thisJoinPoint", "ex", "x", "foo"});
 	}
 
 	@Test
@@ -253,8 +217,8 @@ public class AspectJAdviceParameterNameDiscovererTests {
 
 
 	protected Method getMethod(String name) {
-		// assumes no overloading of test methods...
+		// Assumes no overloading of test methods...
-		Method[] candidates = this.getClass().getMethods();
+		Method[] candidates = getClass().getMethods();
 		for (Method candidate : candidates) {
 			if (candidate.getName().equals(name)) {
 				return candidate;
@@ -268,8 +232,8 @@ public class AspectJAdviceParameterNameDiscovererTests {
 		assertParameterNames(method, pointcut, null, null, parameterNames);
 	}
 
-	protected void assertParameterNames(Method method, String pointcut, String returning, String throwing,
+	protected void assertParameterNames(
-			String[] parameterNames) {
+			Method method, String pointcut, String returning, String throwing, String[] parameterNames) {
 
 		assertEquals("bad test specification, must have same number of parameter names as method arguments",
 				method.getParameterCount(), parameterNames.length);
@@ -300,8 +264,8 @@ public class AspectJAdviceParameterNameDiscovererTests {
 		assertException(method, pointcut, null, null, exceptionType, message);
 	}
 
-	protected void assertException(Method method, String pointcut, String returning, String throwing,
+	protected void assertException(
-			Class<?> exceptionType, String message) {
+			Method method, String pointcut, String returning, String throwing, Class<?> exceptionType, String message) {
 
 		AspectJAdviceParameterNameDiscoverer discoverer = new AspectJAdviceParameterNameDiscoverer(pointcut);
 		discoverer.setRaiseExceptions(true);
@@ -333,4 +297,46 @@ public class AspectJAdviceParameterNameDiscovererTests {
 		return sb.toString();
 	}
 
+
+	// Methods to discover parameter names for
+
+	public void noArgs() {
+	}
+
+	public void tjp(JoinPoint jp) {
+	}
+
+	public void tjpsp(JoinPoint.StaticPart tjpsp) {
+	}
+
+	public void twoJoinPoints(JoinPoint jp1, JoinPoint jp2) {
+	}
+
+	public void oneThrowable(Exception ex) {
+	}
+
+	public void jpAndOneThrowable(JoinPoint jp, Exception ex) {
+	}
+
+	public void jpAndTwoThrowables(JoinPoint jp, Exception ex, Error err) {
+	}
+
+	public void oneObject(Object x) {
+	}
+
+	public void twoObjects(Object x, Object y) {
+	}
+
+	public void onePrimitive(int x) {
+	}
+
+	public void oneObjectOnePrimitive(Object x, int y) {
+	}
+
+	public void oneThrowableOnePrimitive(Throwable x, int y) {
+	}
+
+	public void theBigOne(JoinPoint jp, Throwable x, int y, Object foo) {
+	}
+
 }

spring-aop/src/test/java/org/springframework/aop/aspectj/AspectJExpressionPointcutTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -218,38 +218,27 @@ public class AspectJExpressionPointcutTests {
 	@Test
 	public void testSimpleAdvice() {
 		String expression = "execution(int org.springframework.tests.sample.beans.TestBean.getAge())";
-
 		CallCountingInterceptor interceptor = new CallCountingInterceptor();
-
 		TestBean testBean = getAdvisedProxy(expression, interceptor);
 
 		assertEquals("Calls should be 0", 0, interceptor.getCount());
-
 		testBean.getAge();
-
 		assertEquals("Calls should be 1", 1, interceptor.getCount());
-
 		testBean.setAge(90);
-
 		assertEquals("Calls should still be 1", 1, interceptor.getCount());
 	}
 
 	@Test
 	public void testDynamicMatchingProxy() {
 		String expression = "execution(void org.springframework.tests.sample.beans.TestBean.setSomeNumber(Number)) && args(Double)";
-
 		CallCountingInterceptor interceptor = new CallCountingInterceptor();
-
 		TestBean testBean = getAdvisedProxy(expression, interceptor);
 
 		assertEquals("Calls should be 0", 0, interceptor.getCount());
-
 		testBean.setSomeNumber(new Double(30));
-
 		assertEquals("Calls should be 1", 1, interceptor.getCount());
 
 		testBean.setSomeNumber(new Integer(90));
-
 		assertEquals("Calls should be 1", 1, interceptor.getCount());
 	}
 
@@ -291,7 +280,7 @@ public class AspectJExpressionPointcutTests {
 	}
 
 	@Test
-	public void testWithUnsupportedPointcutPrimitive() throws Exception {
+	public void testWithUnsupportedPointcutPrimitive() {
 		String expression = "call(int org.springframework.tests.sample.beans.TestBean.getAge())";
 
 		try {
@@ -301,7 +290,6 @@ public class AspectJExpressionPointcutTests {
 		catch (UnsupportedPointcutPrimitiveException ex) {
 			assertEquals("Should not support call pointcut", PointcutPrimitive.CALL, ex.getUnsupportedPrimitive());
 		}
-
 	}
 
 	@Test
@@ -332,6 +320,7 @@ public class AspectJExpressionPointcutTests {
 			// Empty
 		}
 	}
+
 }
 
 

spring-aop/src/test/java/org/springframework/aop/aspectj/BeanNamePointcutMatchingTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -77,6 +77,7 @@ public class BeanNamePointcutMatchingTests {
 		assertMisMatch("someName", "!bean(someName) || bean(someOtherName)");
 	}
 
+
 	private void assertMatch(String beanName, String pcExpression) {
 		assertTrue("Unexpected mismatch for bean \"" + beanName + "\" for pcExpression \"" + pcExpression + "\"",
 				matches(beanName, pcExpression));
@@ -98,4 +99,5 @@ public class BeanNamePointcutMatchingTests {
 		pointcut.setExpression(pcExpression);
 		return pointcut.matches(TestBean.class);
 	}
+
 }

spring-aop/src/test/java/org/springframework/aop/aspectj/TigerAspectJExpressionPointcutTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -31,25 +31,24 @@ import org.springframework.tests.sample.beans.TestBean;
 import static org.junit.Assert.*;
 
 /**
- * Java5-specific {@link AspectJExpressionPointcutTests}.
+ * Java 5 specific {@link AspectJExpressionPointcutTests}.
  *
  * @author Rod Johnson
  * @author Chris Beams
  */
 public class TigerAspectJExpressionPointcutTests {
 
-	// TODO factor into static in AspectJExpressionPointcut
 	private Method getAge;
 
-	private Map<String,Method> methodsOnHasGeneric = new HashMap<>();
+	private final Map<String, Method> methodsOnHasGeneric = new HashMap<>();
 
 
 	@Before
-	public void setUp() throws NoSuchMethodException {
+	public void setup() throws NoSuchMethodException {
 		getAge = TestBean.class.getMethod("getAge");
 		// Assumes no overloading
-		for (Method m : HasGeneric.class.getMethods()) {
+		for (Method method : HasGeneric.class.getMethods()) {
-			methodsOnHasGeneric.put(m.getName(), m);
+			methodsOnHasGeneric.put(method.getName(), method);
 		}
 	}
 
@@ -87,11 +86,6 @@ public class TigerAspectJExpressionPointcutTests {
 		AspectJExpressionPointcut jdbcVarArgs = new AspectJExpressionPointcut();
 		jdbcVarArgs.setExpression(expression);
 
-		// TODO: the expression above no longer matches Object[]
-		// assertFalse(jdbcVarArgs.matches(
-		//	JdbcTemplate.class.getMethod("queryForInt", String.class, Object[].class),
-		//	JdbcTemplate.class));
-
 		assertTrue(jdbcVarArgs.matches(
 				MyTemplate.class.getMethod("queryForInt", String.class, Object[].class),
 				MyTemplate.class));
@@ -167,8 +161,10 @@ public class TigerAspectJExpressionPointcutTests {
 		anySpringMethodAnnotation.setExpression(expression);
 
 		assertFalse(anySpringMethodAnnotation.matches(getAge, TestBean.class));
-		assertFalse(anySpringMethodAnnotation.matches(HasTransactionalAnnotation.class.getMethod("foo"), HasTransactionalAnnotation.class));
+		assertFalse(anySpringMethodAnnotation.matches(
-		assertFalse(anySpringMethodAnnotation.matches(HasTransactionalAnnotation.class.getMethod("bar", String.class), HasTransactionalAnnotation.class));
+				HasTransactionalAnnotation.class.getMethod("foo"), HasTransactionalAnnotation.class));
+		assertFalse(anySpringMethodAnnotation.matches(
+				HasTransactionalAnnotation.class.getMethod("bar", String.class), HasTransactionalAnnotation.class));
 		assertFalse(anySpringMethodAnnotation.matches(BeanA.class.getMethod("setName", String.class), BeanA.class));
 		assertTrue(anySpringMethodAnnotation.matches(BeanA.class.getMethod("getAge"), BeanA.class));
 		assertFalse(anySpringMethodAnnotation.matches(BeanA.class.getMethod("setName", String.class), BeanA.class));
@@ -181,8 +177,10 @@ public class TigerAspectJExpressionPointcutTests {
 		takesSpringAnnotatedArgument2.setExpression(expression);
 
 		assertFalse(takesSpringAnnotatedArgument2.matches(getAge, TestBean.class));
-		assertFalse(takesSpringAnnotatedArgument2.matches(HasTransactionalAnnotation.class.getMethod("foo"), HasTransactionalAnnotation.class));
+		assertFalse(takesSpringAnnotatedArgument2.matches(
-		assertFalse(takesSpringAnnotatedArgument2.matches(HasTransactionalAnnotation.class.getMethod("bar", String.class), HasTransactionalAnnotation.class));
+				HasTransactionalAnnotation.class.getMethod("foo"), HasTransactionalAnnotation.class));
+		assertFalse(takesSpringAnnotatedArgument2.matches(
+				HasTransactionalAnnotation.class.getMethod("bar", String.class), HasTransactionalAnnotation.class));
 		assertFalse(takesSpringAnnotatedArgument2.matches(BeanA.class.getMethod("setName", String.class), BeanA.class));
 		assertFalse(takesSpringAnnotatedArgument2.matches(BeanA.class.getMethod("getAge"), BeanA.class));
 		assertFalse(takesSpringAnnotatedArgument2.matches(BeanA.class.getMethod("setName", String.class), BeanA.class));
@@ -209,8 +207,10 @@ public class TigerAspectJExpressionPointcutTests {
 		takesSpringAnnotatedArgument2.setExpression(expression);
 
 		assertFalse(takesSpringAnnotatedArgument2.matches(getAge, TestBean.class));
-		assertFalse(takesSpringAnnotatedArgument2.matches(HasTransactionalAnnotation.class.getMethod("foo"), HasTransactionalAnnotation.class));
+		assertFalse(takesSpringAnnotatedArgument2.matches(
-		assertFalse(takesSpringAnnotatedArgument2.matches(HasTransactionalAnnotation.class.getMethod("bar", String.class), HasTransactionalAnnotation.class));
+				HasTransactionalAnnotation.class.getMethod("foo"), HasTransactionalAnnotation.class));
+		assertFalse(takesSpringAnnotatedArgument2.matches(
+				HasTransactionalAnnotation.class.getMethod("bar", String.class), HasTransactionalAnnotation.class));
 		assertFalse(takesSpringAnnotatedArgument2.matches(BeanA.class.getMethod("setName", String.class), BeanA.class));
 		assertFalse(takesSpringAnnotatedArgument2.matches(BeanA.class.getMethod("getAge"), BeanA.class));
 		assertFalse(takesSpringAnnotatedArgument2.matches(BeanA.class.getMethod("setName", String.class), BeanA.class));
@@ -260,12 +260,14 @@ public class TigerAspectJExpressionPointcutTests {
 
 	@EmptySpringAnnotation
 	public static class SpringAnnotated {
+
 		public void foo() {
 		}
 	}
 
 
 	static class BeanA {
+
 		private String name;
 
 		private int age;
@@ -283,6 +285,7 @@ public class TigerAspectJExpressionPointcutTests {
 
 	@Tx
 	static class BeanB {
+
 		private String name;
 
 		public void setName(String name) {