polishing .aop tests

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@495 50f2f4bb-b051-0410-bef5-90022cba6387

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2007 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -20,6 +20,7 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 
 import org.aspectj.lang.ProceedingJoinPoint;
+import org.junit.Test;
 
 
 /**
@@ -28,14 +29,17 @@ import org.aspectj.lang.ProceedingJoinPoint;
  * doesn't matter in the grand scheme of things...
  *
  * @author Adrian Colyer
+ * @author Chris Beams
  */
-public class AspectJAdviceParameterNameDiscoverAnnotationTests extends AspectJAdviceParameterNameDiscovererTests {
+public final class AspectJAdviceParameterNameDiscoverAnnotationTests
+		extends AspectJAdviceParameterNameDiscovererTests {
 
 	@Retention(RetentionPolicy.RUNTIME)
 	@interface MyAnnotation {}
 	
 	public void pjpAndAnAnnotation(ProceedingJoinPoint pjp, MyAnnotation ann) {}
 
+	@Test
 	public void testAnnotationBinding() {
 		assertParameterNames(getMethod("pjpAndAnAnnotation"),
 				"execution(* *(..)) && @annotation(ann)",

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

@@ -25,11 +25,12 @@ import java.lang.reflect.Method;
 
 /**
  * Unit tests for the {@link AspectJAdviceParameterNameDiscoverer} class.
- * <p/>
+ *
- * <p>See also <code>TigerAspectJAdviceParameterNameDiscovererTests</code> in
+ * <p>See also {@link TigerAspectJAdviceParameterNameDiscovererTests} in
  * the 'tiger' tree for tests relating to annotations.
  *
  * @author Adrian Colyer
+ * @author Chris Beams
  */
 public class AspectJAdviceParameterNameDiscovererTests {
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2005 the original author or authors.
+ * Copyright 2002-2008 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,17 +16,20 @@
 
 package org.springframework.aop.aspectj;
 
+import static org.junit.Assert.*;
+
 import java.lang.reflect.Method;
 
-import junit.framework.TestCase;
+import org.aopalliance.intercept.MethodInterceptor;
-
+import org.aopalliance.intercept.MethodInvocation;
 import org.aspectj.weaver.tools.PointcutExpression;
 import org.aspectj.weaver.tools.PointcutPrimitive;
 import org.aspectj.weaver.tools.UnsupportedPointcutPrimitiveException;
+import org.junit.Before;
+import org.junit.Test;
 import org.springframework.aop.ClassFilter;
 import org.springframework.aop.MethodMatcher;
 import org.springframework.aop.Pointcut;
-import org.springframework.aop.aspectj.AspectJExpressionPointcut;
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.aop.support.DefaultPointcutAdvisor;
 import org.springframework.beans.IOther;
@@ -38,8 +41,9 @@ import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 /**
  * @author Rob Harrop
  * @author Rod Johnson
+ * @author Chris Beams
  */
-public class AspectJExpressionPointcutTests extends TestCase {
+public final class AspectJExpressionPointcutTests {
 	
 	public static final String MATCH_ALL_METHODS = "execution(* *(..))";
 
@@ -51,6 +55,16 @@ public class AspectJExpressionPointcutTests extends TestCase {
 
 	private Method isPostProcessed;
 	
+	
+	@Before
+	public void setUp() throws NoSuchMethodException {
+		getAge = TestBean.class.getMethod("getAge", (Class<?>[])null);
+		setAge = TestBean.class.getMethod("setAge", new Class[]{int.class});
+		setSomeNumber = TestBean.class.getMethod("setSomeNumber", new Class[]{Number.class});
+		isPostProcessed = TestBean.class.getMethod("isPostProcessed", (Class[]) null);
+	}
+	
+	@Test
 	public void testMatchExplicit() {
 		String expression = "execution(int org.springframework.beans.TestBean.getAge())";
 
@@ -68,16 +82,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		assertFalse("Expression should match setAge() method", methodMatcher.matches(setAge, TestBean.class));
 	}
 
-
+	@Test
-	public void setUp() throws NoSuchMethodException {
-		getAge = TestBean.class.getMethod("getAge", null);
-		setAge = TestBean.class.getMethod("setAge", new Class[]{int.class});
-		setSomeNumber = TestBean.class.getMethod("setSomeNumber", new Class[]{Number.class});
-		isPostProcessed = TestBean.class.getMethod("isPostProcessed", (Class[]) null);
-	}
-	
-	
-
 	public void testMatchWithTypePattern() throws Exception {
 		String expression = "execution(* *..TestBean.*Age(..))";
 
@@ -96,10 +101,12 @@ public class AspectJExpressionPointcutTests extends TestCase {
 	}
 
 	
+	@Test
 	public void testThis() throws SecurityException, NoSuchMethodException{
 		testThisOrTarget("this");
 	}
 	
+	@Test
 	public void testTarget() throws SecurityException, NoSuchMethodException {
 		testThisOrTarget("target");
 	}
@@ -130,17 +137,19 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		
 		assertTrue(testBeanPc.matches(TestBean.class));
 		assertTrue(testBeanPc.matches(getAge, TestBean.class));
-		assertTrue(iOtherPc.matches(OtherIOther.class.getMethod("absquatulate", null),
+		assertTrue(iOtherPc.matches(OtherIOther.class.getMethod("absquatulate", (Class<?>[])null),
 				OtherIOther.class));
 	
-		assertFalse(testBeanPc.matches(OtherIOther.class.getMethod("absquatulate", null),
+		assertFalse(testBeanPc.matches(OtherIOther.class.getMethod("absquatulate", (Class<?>[])null),
 				OtherIOther.class));
 	}
 	
+	@Test
 	public void testWithinRootPackage() throws SecurityException, NoSuchMethodException {
 		testWithinPackage(false);
 	}
 	
+	@Test
 	public void testWithinRootAndSubpackages() throws SecurityException, NoSuchMethodException {
 		testWithinPackage(true);
 	}
@@ -159,13 +168,14 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		assertTrue(withinBeansPc.matches(getAge, TestBean.class));
 		assertEquals(matchSubpackages, withinBeansPc.matches(BeanFactory.class));
 		assertEquals(matchSubpackages, withinBeansPc.matches(
-				DefaultListableBeanFactory.class.getMethod("getBeanDefinitionCount", null),
+				DefaultListableBeanFactory.class.getMethod("getBeanDefinitionCount", (Class<?>[])null),
 				DefaultListableBeanFactory.class));
 		assertFalse(withinBeansPc.matches(String.class));
-		assertFalse(withinBeansPc.matches(OtherIOther.class.getMethod("absquatulate", null),
+		assertFalse(withinBeansPc.matches(OtherIOther.class.getMethod("absquatulate", (Class<?>[])null),
 				OtherIOther.class));
 	}
 	
+	@Test
 	public void testFriendlyErrorOnNoLocationClassMatching() {
 		AspectJExpressionPointcut pc = new AspectJExpressionPointcut();
 		try {
@@ -177,6 +187,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		}
 	}
 	
+	@Test
 	public void testFriendlyErrorOnNoLocation2ArgMatching() {
 		AspectJExpressionPointcut pc = new AspectJExpressionPointcut();
 		try {
@@ -188,6 +199,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		}
 	}
 	
+	@Test
 	public void testFriendlyErrorOnNoLocation3ArgMatching() {
 		AspectJExpressionPointcut pc = new AspectJExpressionPointcut();
 		try {
@@ -200,6 +212,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 	}
 
 	
+	@Test
 	public void testMatchWithArgs() throws Exception {
 		String expression = "execution(void org.springframework.beans.TestBean.setSomeNumber(Number)) && args(Double)";
 
@@ -220,6 +233,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		assertTrue("Should be a runtime match", methodMatcher.isRuntime());
 	}
 
+	@Test
 	public void testSimpleAdvice() {
 		String expression = "execution(int org.springframework.beans.TestBean.getAge())";
 
@@ -238,6 +252,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		assertEquals("Calls should still be 1", 1, interceptor.getCount());
 	}
 
+	@Test
 	public void testDynamicMatchingProxy() {
 		String expression = "execution(void org.springframework.beans.TestBean.setSomeNumber(Number)) && args(Double)";
 
@@ -256,6 +271,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		assertEquals("Calls should be 1", 1, interceptor.getCount());
 	}
 
+	@Test
 	public void testInvalidExpression() {
 		String expression = "execution(void org.springframework.beans.TestBean.setSomeNumber(Number) && args(Double)";
 
@@ -297,6 +313,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		assertFalse("Expression should not match String class", classFilter.matches(String.class));
 	}
 
+	@Test
 	public void testWithUnsupportedPointcutPrimitive() throws Exception {
 		String expression = "call(int org.springframework.beans.TestBean.getAge())";
 
@@ -310,6 +327,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 
 	}
 
+	@Test
 	public void testAndSubstitution() {
 		Pointcut pc = getPointcut("execution(* *(..)) and args(String)");
 		PointcutExpression expr = 
@@ -317,6 +335,7 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		assertEquals("execution(* *(..)) && args(String)",expr.getPointcutExpression());
 	}
 	
+	@Test
 	public void testMultipleAndSubstitutions() {
 		Pointcut pc = getPointcut("execution(* *(..)) and args(String) and this(Object)");
 		PointcutExpression expr = 
@@ -330,3 +349,23 @@ public class AspectJExpressionPointcutTests extends TestCase {
 		return pointcut;
 	}
 }
+
+
+class CallCountingInterceptor implements MethodInterceptor {
+
+	private int count;
+
+	public Object invoke(MethodInvocation methodInvocation) throws Throwable {
+		count++;
+		return methodInvocation.proceed();
+	}
+
+	public int getCount() {
+		return count;
+	}
+
+	public void reset() {
+		this.count = 0;
+	}
+
+}

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

@@ -27,7 +27,7 @@ import org.springframework.beans.TestBean;
  * @author Ramnivas Laddad
  * @author Chris Beams
  */
-public class BeanNamePointcutMatchingTests {
+public final class BeanNamePointcutMatchingTests {
 
 	@Test
 	public void testMatchingPointcuts() {

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/CallCountingInterceptor.java

@@ -1,26 +0,0 @@
-
-package org.springframework.aop.aspectj;
-
-import org.aopalliance.intercept.MethodInterceptor;
-import org.aopalliance.intercept.MethodInvocation;
-
-/**
- * @author robh
- */
-class CallCountingInterceptor implements MethodInterceptor {
-
-	private int count;
-
-	public Object invoke(MethodInvocation methodInvocation) throws Throwable {
-		count++;
-		return methodInvocation.proceed();
-	}
-
-	public int getCount() {
-		return count;
-	}
-
-	public void reset() {
-		this.count = 0;
-	}
-}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/MethodInvocationProceedingJoinPointTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2006 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -41,7 +41,7 @@ import org.springframework.beans.TestBean;
  * @author Chris Beams
  * @since 2.0
  */
-public class MethodInvocationProceedingJoinPointTests {
+public final class MethodInvocationProceedingJoinPointTests {
 
 	@Test
 	public void testingBindingWithJoinPoint() {

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/TigerAspectJAdviceParameterNameDiscovererTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2006 the original author or authors.
+ * Copyright 2002-2008 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,50 +16,62 @@
 
 package org.springframework.aop.aspectj;
 
+import org.junit.Test;
 import org.springframework.aop.aspectj.AspectJAdviceParameterNameDiscoverer.AmbiguousBindingException;
 
 /**
- * Tests just the annotation binding part of AspectJAdviceParameterNameDiscoverer;
+ * Tests just the annotation binding part of {@link AspectJAdviceParameterNameDiscoverer};
  * see supertype for remaining tests.
  *
  * @author Adrian Colyer
+ * @author Chris Beams
  */
-public class TigerAspectJAdviceParameterNameDiscovererTests extends AspectJAdviceParameterNameDiscovererTests {
+public final class TigerAspectJAdviceParameterNameDiscovererTests
+		extends AspectJAdviceParameterNameDiscovererTests {
 
+	@Test
 	public void testAtThis() {
 		assertParameterNames(getMethod("oneAnnotation"),"@this(a)",new String[]{"a"});
 	}
 
+	@Test
 	public void testAtTarget() {
 		assertParameterNames(getMethod("oneAnnotation"),"@target(a)",new String[]{"a"});
 	}
 	
+	@Test
 	public void testAtArgs() {
 		assertParameterNames(getMethod("oneAnnotation"),"@args(a)",new String[]{"a"});
 	}
 	
+	@Test
 	public void testAtWithin() {
 		assertParameterNames(getMethod("oneAnnotation"),"@within(a)",new String[]{"a"});
 	}
 	
+	@Test
 	public void testAtWithincode() {
 		assertParameterNames(getMethod("oneAnnotation"),"@withincode(a)",new String[]{"a"});
 	}
 	
+	@Test
 	public void testAtAnnotation() {
 		assertParameterNames(getMethod("oneAnnotation"),"@annotation(a)",new String[]{"a"});
 	}
 	
+	@Test
 	public void testAmbiguousAnnotationTwoVars() {
 		assertException(getMethod("twoAnnotations"),"@annotation(a) && @this(x)",AmbiguousBindingException.class,
 				"Found 2 potential annotation variable(s), and 2 potential argument slots");
 	}
 	
+	@Test
 	public void testAmbiguousAnnotationOneVar() {
 		assertException(getMethod("oneAnnotation"),"@annotation(a) && @this(x)",IllegalArgumentException.class,
 		"Found 2 candidate annotation binding variables but only one potential argument binding slot");		
 	}
 
+	@Test
 	public void testAnnotationMedley() {
 		assertParameterNames(getMethod("annotationMedley"),"@annotation(a) && args(count) && this(foo)",null,"ex",
 				new String[] {"ex","foo","count","a"});
@@ -72,7 +84,6 @@ public class TigerAspectJAdviceParameterNameDiscovererTests extends AspectJAdvic
 
 	public void annotationMedley(Throwable t, Object foo, int x, MyAnnotation ma) {}
 
-
 	@interface MyAnnotation {}
 
 }

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

@@ -33,12 +33,12 @@ import test.annotation.transaction.Tx;
 
 
 /** 
- * Java5-specific AspectJExpressionPointcutTests.
+ * Java5-specific {@link AspectJExpressionPointcutTests}.
  *
  * @author Rod Johnson
  * @author Chris Beams
  */
-public class TigerAspectJExpressionPointcutTests {
+public final class TigerAspectJExpressionPointcutTests {
 
 	// TODO factor into static in AspectJExpressionPointcut
 	private Method getAge;

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/TypePatternClassFilterTests.java

@@ -35,14 +35,10 @@ import org.springframework.beans.factory.support.DefaultListableBeanFactory;
  */
 public final class TypePatternClassFilterTests {
 
-	@Test
+	@Test(expected=IllegalArgumentException.class)
 	public void testInvalidPattern() {
-		try {
+		// should throw - pattern must be recognized as invalid
-			new TypePatternClassFilter("-");
+		new TypePatternClassFilter("-");
-			fail("Pattern must be recognized as invalid.");
-		}
-		catch (IllegalArgumentException expected) {
-		}
 	}
 
 	@Test

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2007 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -18,12 +18,14 @@ package org.springframework.aop.aspectj.annotation;
 import static org.junit.Assert.*;
 
 import java.io.FileNotFoundException;
+import java.lang.reflect.Method;
 import java.lang.reflect.UndeclaredThrowableException;
 import java.rmi.RemoteException;
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 
+import org.aspectj.lang.JoinPoint;
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.annotation.After;
 import org.aspectj.lang.annotation.AfterReturning;
@@ -31,14 +33,16 @@ import org.aspectj.lang.annotation.AfterThrowing;
 import org.aspectj.lang.annotation.Around;
 import org.aspectj.lang.annotation.Aspect;
 import org.aspectj.lang.annotation.Before;
+import org.aspectj.lang.annotation.DeclareParents;
 import org.aspectj.lang.annotation.DeclarePrecedence;
 import org.aspectj.lang.annotation.Pointcut;
+import org.aspectj.lang.reflect.MethodSignature;
 import org.junit.Test;
-
 import org.springframework.aop.Advisor;
 import org.springframework.aop.aspectj.annotation.ReflectiveAspectJAdvisorFactory.SyntheticInstantiationAdvisor;
 import org.springframework.aop.framework.Advised;
 import org.springframework.aop.framework.AopConfigException;
+import org.springframework.aop.framework.DefaultLockable;
 import org.springframework.aop.framework.Lockable;
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.aop.interceptor.ExposeInvocationInterceptor;
@@ -48,16 +52,17 @@ import org.springframework.beans.TestBean;
 import org.springframework.core.OrderComparator;
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.Order;
+import org.springframework.util.ObjectUtils;
 
 import test.aspect.PerTargetAspect;
 import test.aspect.TwoAdviceAspect;
 
-
 /**
  * Abstract tests for AspectJAdvisorFactory.
  * See subclasses for tests of concrete factories.
  *
  * @author Rod Johnson
+ * @author Chris Beams
  */
 public abstract class AbstractAspectJAdvisorFactoryTests {
 	
@@ -861,3 +866,198 @@ public abstract class AbstractAspectJAdvisorFactoryTests {
 	}
 
 }
+
+
+/**
+ * Add a DeclareParents field in concrete subclasses, to identify
+ * the type pattern to apply the introduction to.
+ *
+ * @author Rod Johnson
+ * @since 2.0
+ */
+@Aspect
+abstract class AbstractMakeModifiable {
+	
+	public interface MutableModifable extends Modifiable {
+		void markDirty();
+	}
+	
+	public static class ModifiableImpl implements MutableModifable {
+		private boolean modified;
+		
+		public void acceptChanges() {
+			modified = false;
+		}
+		
+		public boolean isModified() {
+			return modified;
+		}
+		
+		public void markDirty() {
+			this.modified = true;
+		}
+	}
+	
+	@Before(value="execution(void set*(*)) && this(modifiable) && args(newValue)", 
+			argNames="modifiable,newValue")
+	public void recordModificationIfSetterArgumentDiffersFromOldValue(JoinPoint jp, 
+		MutableModifable mixin, Object newValue) {
+		
+		/*
+		 * We use the mixin to check and, if necessary, change,
+		 * modification status. We need the JoinPoint to get the 
+		 * setter method. We use newValue for comparison. 
+		 * We try to invoke the getter if possible.
+		 */
+		
+		if (mixin.isModified()) {
+			// Already changed, don't need to change again
+			//System.out.println("changed");
+			return;
+		}
+		
+		// Find the current raw value, by invoking the corresponding setter
+		Method correspondingGetter =  getGetterFromSetter(((MethodSignature) jp.getSignature()).getMethod());
+		boolean modified = true;
+		if (correspondingGetter != null) {
+			try {
+				Object oldValue = correspondingGetter.invoke(jp.getTarget());
+				//System.out.println("Old value=" + oldValue + "; new=" + newValue);
+				modified = !ObjectUtils.nullSafeEquals(oldValue, newValue);
+			}
+			catch (Exception ex) {
+				ex.printStackTrace();
+				// Don't sweat on exceptions; assume value was modified
+			}
+		}
+		else {
+			//System.out.println("cannot get getter for " + jp);
+		}
+		if (modified) {
+			mixin.markDirty();
+		}
+	}
+	
+	private Method getGetterFromSetter(Method setter) {
+		String getterName = setter.getName().replaceFirst("set", "get");
+		try {
+			return setter.getDeclaringClass().getMethod(getterName, (Class[]) null);
+		} 
+		catch (NoSuchMethodException ex) {
+			// must be write only
+			return null;
+		}
+	}
+
+}
+
+
+/**
+ * Adds a declare parents pointcut.
+ * @author Rod Johnson
+ * @since 2.0
+ */
+@Aspect
+class MakeITestBeanModifiable extends AbstractMakeModifiable {
+	
+	@DeclareParents(value = "org.springframework.beans.ITestBean+",
+			defaultImpl=ModifiableImpl.class)
+	public static MutableModifable mixin;
+
+}
+
+
+/**
+ * Demonstrates introductions, AspectJ annotation style.
+ */
+@Aspect
+class MakeLockable {
+	
+	@DeclareParents(value = "org.springframework..*",
+			defaultImpl=DefaultLockable.class)
+	public static Lockable mixin;
+	
+	@Before(value="execution(void set*(*)) && this(mixin)", argNames="mixin")
+	public void checkNotLocked(
+		Lockable mixin)  // Bind to arg
+	{
+		// Can also obtain the mixin (this) this way
+		//Lockable mixin = (Lockable) jp.getThis();
+		if (mixin.locked()) {
+			throw new IllegalStateException();
+		}
+	}
+
+}
+
+
+class CannotBeUnlocked implements Lockable, Comparable<Object> {
+
+	public void lock() {
+	}
+
+	public void unlock() {
+		throw new UnsupportedOperationException();
+	}
+
+	public boolean locked() {
+		return true;
+	}
+
+	public int compareTo(Object arg0) {
+		throw new UnsupportedOperationException();
+	}
+
+}
+
+
+/**
+ * Used as a mixin.
+ *
+ * @author Rod Johnson
+ */
+interface Modifiable {
+
+	boolean isModified();
+	
+	void acceptChanges();
+	
+}
+
+
+class NotLockable {
+	
+	private int intValue;
+
+	public int getIntValue() {
+		return intValue;
+	}
+
+	public void setIntValue(int intValue) {
+		this.intValue = intValue;
+	}
+
+}
+
+
+@Aspect("perthis(execution(* *.getSpouse()))")
+class PerThisAspect {
+
+	public int count;
+
+	/**
+	 * Just to check that this doesn't cause problems with introduction processing
+	 */
+	private ITestBean fieldThatShouldBeIgnoredBySpringAtAspectJProcessing = new TestBean();
+
+	@Around("execution(int *.getAge())")
+	public int returnCountAsAge() {
+		return count++;
+	}
+
+	@Before("execution(void *.set*(int))")
+	public void countSetter() {
+		++count;
+	}
+	
+}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractMakeModifiable.java

@@ -1,109 +0,0 @@
-/*
- * Copyright 2002-2006 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.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
- 
-package org.springframework.aop.aspectj.annotation;
-
-import java.lang.reflect.Method;
-
-import org.aspectj.lang.JoinPoint;
-import org.aspectj.lang.annotation.Aspect;
-import org.aspectj.lang.annotation.Before;
-import org.aspectj.lang.reflect.MethodSignature;
-
-import org.springframework.util.ObjectUtils;
-
-/**
- * Add a DeclareParents field in concrete subclasses, to identify
- * the type pattern to apply the introduction to.
- *
- * @author Rod Johnson
- * @since 2.0
- */
-@Aspect
-public abstract class AbstractMakeModifiable {
-	
-	public interface MutableModifable extends Modifiable {
-		void markDirty();
-	}
-	
-	public static class ModifiableImpl implements MutableModifable {
-		private boolean modified;
-		
-		public void acceptChanges() {
-			modified = false;
-		}
-		
-		public boolean isModified() {
-			return modified;
-		}
-		
-		public void markDirty() {
-			this.modified = true;
-		}
-	}
-	
-	@Before(value="execution(void set*(*)) && this(modifiable) && args(newValue)", 
-			argNames="modifiable,newValue")
-	public void recordModificationIfSetterArgumentDiffersFromOldValue(JoinPoint jp, 
-		MutableModifable mixin, Object newValue) {
-		
-		/*
-		 * We use the mixin to check and, if necessary, change,
-		 * modification status. We need the JoinPoint to get the 
-		 * setter method. We use newValue for comparison. 
-		 * We try to invoke the getter if possible.
-		 */
-		
-		if (mixin.isModified()) {
-			// Already changed, don't need to change again
-			//System.out.println("changed");
-			return;
-		}
-		
-		// Find the current raw value, by invoking the corresponding setter
-		Method correspondingGetter =  getGetterFromSetter(((MethodSignature) jp.getSignature()).getMethod());
-		boolean modified = true;
-		if (correspondingGetter != null) {
-			try {
-				Object oldValue = correspondingGetter.invoke(jp.getTarget());
-				//System.out.println("Old value=" + oldValue + "; new=" + newValue);
-				modified = !ObjectUtils.nullSafeEquals(oldValue, newValue);
-			}
-			catch (Exception ex) {
-				ex.printStackTrace();
-				// Don't sweat on exceptions; assume value was modified
-			}
-		}
-		else {
-			//System.out.println("cannot get getter for " + jp);
-		}
-		if (modified) {
-			mixin.markDirty();
-		}
-	}
-	
-	private Method getGetterFromSetter(Method setter) {
-		String getterName = setter.getName().replaceFirst("set", "get");
-		try {
-			return setter.getDeclaringClass().getMethod(getterName, (Class[]) null);
-		} 
-		catch (NoSuchMethodException ex) {
-			// must be write only
-			return null;
-		}
-	}
-
-}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/ArgumentBindingTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2007 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -25,6 +25,7 @@ import java.lang.reflect.Method;
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.annotation.Around;
 import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Pointcut;
 import org.junit.Test;
 import org.springframework.aop.aspectj.AspectJAdviceParameterNameDiscoverer;
 import org.springframework.beans.ITestBean;
@@ -35,7 +36,7 @@ import org.springframework.beans.TestBean;
  * @author Juergen Hoeller
  * @author Chris Beams
  */
-public class ArgumentBindingTests {
+public final class ArgumentBindingTests {
 
 	@Test(expected=IllegalArgumentException.class)
 	public void testBindingInPointcutUsedByAdvice() {
@@ -96,6 +97,7 @@ public class ArgumentBindingTests {
 @interface Transactional {
 }
 
+
 /**
  * @author Juergen Hoeller
  */
@@ -109,3 +111,21 @@ class PointcutWithAnnotationArgument {
 	}
 
 }
+
+
+/**
+ * @author Adrian Colyer
+ */
+@Aspect
+class NamedPointcutWithArgs {
+
+	@Pointcut("execution(* *(..)) && args(s,..)")
+	public void pointcutWithArgs(String s) {}
+
+	@Around("pointcutWithArgs(aString)")
+	public Object doAround(ProceedingJoinPoint pjp, String aString) throws Throwable {
+		System.out.println("got '" + aString + "' at '" + pjp + "'");
+		throw new IllegalArgumentException(aString);
+	}
+
+}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectJPointcutAdvisorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2006 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -32,7 +32,7 @@ import test.aspect.PerTargetAspect;
  * @author Rod Johnson 
  * @author Chris Beams
  */
-public class AspectJPointcutAdvisorTests {
+public final class AspectJPointcutAdvisorTests {
 	
 	private AspectJAdvisorFactory af = new ReflectiveAspectJAdvisorFactory();
 

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectMetadataTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2005 the original author or authors.
+ * Copyright 2002-2008 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,7 +31,7 @@ import test.aspect.PerTargetAspect;
  * @author Rod Johnson
  * @author Chris Beams
  */
-public class AspectMetadataTests {
+public final class AspectMetadataTests {
 
 	@Test(expected=IllegalArgumentException.class)
 	public void testNotAnAspect() {

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2007 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -18,8 +18,10 @@ package org.springframework.aop.aspectj.annotation;
 
 import static org.junit.Assert.*;
 
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.lang.annotation.Aspect;
 import org.junit.Test;
-import org.springframework.aop.aspectj.autoproxy.MultiplyReturnValue;
 
 import test.aspect.PerThisAspect;
 
@@ -29,7 +31,7 @@ import test.aspect.PerThisAspect;
  * @author Juergen Hoeller
  * @author Chris Beams
  */
-public class AspectProxyFactoryTests {
+public final class AspectProxyFactoryTests {
 
 	@Test(expected=IllegalArgumentException.class)
 	public void testWithNonAspect() {
@@ -117,3 +119,31 @@ public class AspectProxyFactoryTests {
 	}
 
 }
+
+
+/**
+ * @author Rod Johnson
+ */
+@Aspect
+class MultiplyReturnValue {
+
+	private int multiple = 2;
+
+	public int invocations;
+
+	public void setMultiple(int multiple) {
+		this.multiple = multiple;
+	}
+
+	public int getMultiple() {
+		return this.multiple;
+	}
+
+	@Around("execution(int *.getAge())")
+	public Object doubleReturnValue(ProceedingJoinPoint pjp) throws Throwable {
+		++this.invocations;
+		int result = (Integer) pjp.proceed();
+		return result * this.multiple;
+	}
+
+}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/CannotBeUnlocked.java

@@ -1,41 +0,0 @@
-/*
- * Copyright 2002-2005 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.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.aop.aspectj.annotation;
-
-import org.springframework.aop.framework.Lockable;
-
-/**
- * @author Rod Johnson
- */
-public class CannotBeUnlocked implements Lockable, Comparable<Object> {
-
-	public void lock() {
-	}
-
-	public void unlock() {
-		throw new UnsupportedOperationException();
-	}
-
-	public boolean locked() {
-		return true;
-	}
-
-	public int compareTo(Object arg0) {
-		throw new UnsupportedOperationException();
-	}
-
-}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/FooAspect.java

@@ -1,28 +0,0 @@
-/*
- * Copyright 2002-2006 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.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.aop.aspectj.annotation;
-
-import org.aspectj.lang.annotation.Aspect;
-
-/**
- * @author Rob Harrop
- * @since 2.o
- */
-@Aspect
-public class FooAspect {
-
-}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/MakeITestBeanModifiable.java

@@ -1,34 +0,0 @@
-/*
- * Copyright 2002-2006 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.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
- 
-package org.springframework.aop.aspectj.annotation;
-
-import org.aspectj.lang.annotation.Aspect;
-import org.aspectj.lang.annotation.DeclareParents;
-
-/**
- * Adds a declare parents pointcut.
- * @author Rod Johnson
- * @since 2.0
- */
-@Aspect
-public class MakeITestBeanModifiable extends AbstractMakeModifiable {
-	
-	@DeclareParents(value = "org.springframework.beans.ITestBean+",
-			defaultImpl=ModifiableImpl.class)
-	public static MutableModifable mixin;
-
-}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/MakeLockable.java

@@ -1,73 +0,0 @@
-/*
- * Copyright 2002-2006 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.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
- 
-package org.springframework.aop.aspectj.annotation;
-
-import org.aspectj.lang.annotation.Aspect;
-import org.aspectj.lang.annotation.Before;
-import org.aspectj.lang.annotation.DeclareParents;
-import org.springframework.aop.framework.DefaultLockable;
-import org.springframework.aop.framework.Lockable;
-
-/**
- * Demonstrates introductions, AspectJ annotation style.
- * <p>
- * @author Rod Johnson
- * @since 2.0
- */
-@Aspect
-public class MakeLockable {
-	
-	@DeclareParents(value = "org.springframework..*",
-			defaultImpl=DefaultLockable.class)
-	public static Lockable mixin;
-	
-	@Before(value="execution(void set*(*)) && this(mixin)", argNames="mixin")
-	public void checkNotLocked(
-		Lockable mixin)  // Bind to arg
-	{
-		// Can also obtain the mixin (this) this way
-		//Lockable mixin = (Lockable) jp.getThis();
-		if (mixin.locked()) {
-			throw new IllegalStateException();
-		}
-	}
-
-}
-
-/*
- * 
- * public aspect MakeLockable {
- *   
- *  declare parents org....* implements Lockable;
- *   
- *  private boolean Lockable.locked;
-	
- *	public void Lockable.lock() {
-		this.locked = true;
-	}
-	
- *	public void Lockable.unlock() {
-		this.locked = false;
-	}
-	
- *	public boolean Lockable.locked() {
-		return this.locked;
-	}
- * 
- * 
- * }
- */

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/Modifiable.java

@@ -1,30 +0,0 @@
-/*
- * Copyright 2002-2006 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.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.aop.aspectj.annotation;
-
-/**
- * Used as a mixin.
- *
- * @author Rod Johnson
- */
-public interface Modifiable {
-
-	boolean isModified();
-	
-	void acceptChanges();
-	
-}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/NamedPointcutWithArgs.java

@@ -1,39 +0,0 @@
-/*
- * Copyright 2002-2007 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.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.aop.aspectj.annotation;
-
-import org.aspectj.lang.ProceedingJoinPoint;
-import org.aspectj.lang.annotation.Around;
-import org.aspectj.lang.annotation.Aspect;
-import org.aspectj.lang.annotation.Pointcut;
-
-/**
- * @author Adrian Colyer
- */
-@Aspect
-public class NamedPointcutWithArgs {
-
-	@Pointcut("execution(* *(..)) && args(s,..)")
-	public void pointcutWithArgs(String s) {}
-
-	@Around("pointcutWithArgs(aString)")
-	public Object doAround(ProceedingJoinPoint pjp, String aString) throws Throwable {
-		System.out.println("got '" + aString + "' at '" + pjp + "'");
-		throw new IllegalArgumentException(aString);
-	}
-
-}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/NotLockable.java

@@ -1,15 +0,0 @@
-package org.springframework.aop.aspectj.annotation;
-
-public class NotLockable {
-	
-	private int intValue;
-
-	public int getIntValue() {
-		return intValue;
-	}
-
-	public void setIntValue(int intValue) {
-		this.intValue = intValue;
-	}
-
-}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/PerThisAspect.java

@@ -1,31 +0,0 @@
-/**
- * 
- */
-package org.springframework.aop.aspectj.annotation;
-
-import org.aspectj.lang.annotation.Around;
-import org.aspectj.lang.annotation.Aspect;
-import org.aspectj.lang.annotation.Before;
-import org.springframework.beans.ITestBean;
-import org.springframework.beans.TestBean;
-
-@Aspect("perthis(execution(* *.getSpouse()))")
-public class PerThisAspect {
-
-	public int count;
-
-	/**
-	 * Just to check that this doesn't cause problems with introduction processing
-	 */
-	private ITestBean fieldThatShouldBeIgnoredBySpringAtAspectJProcessing = new TestBean();
-
-	@Around("execution(int *.getAge())")
-	public int returnCountAsAge() {
-		return count++;
-	}
-
-	@Before("execution(void *.set*(int))")
-	public void countSetter() {
-		++count;
-	}
-}

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactoryTests.java

@@ -23,7 +23,7 @@ package org.springframework.aop.aspectj.annotation;
  * @author Rod Johnson
  * @since 2.0
  */
-public class ReflectiveAspectJAdvisorFactoryTests extends AbstractAspectJAdvisorFactoryTests {
+public final class ReflectiveAspectJAdvisorFactoryTests extends AbstractAspectJAdvisorFactoryTests {
 
 	@Override
 	protected AspectJAdvisorFactory getFixture() {

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJNamespaceHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2006 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -36,7 +36,7 @@ import org.springframework.beans.factory.xml.XmlReaderContext;
  * @author Rob Harrop
  * @author Chris Beams
  */
-public class AspectJNamespaceHandlerTests {
+public final class AspectJNamespaceHandlerTests {
 
 	private ParserContext parserContext;
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2007 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -39,7 +39,7 @@ import org.springframework.aop.support.DefaultPointcutAdvisor;
  * @author Adrian Colyer
  * @author Chris Beams
  */
-public class AspectJPrecedenceComparatorTests {
+public final class AspectJPrecedenceComparatorTests {
 
 	/*
 	 * Specification for the comparator (as defined in the 

org.springframework.aop/src/test/java/org/springframework/aop/aspectj/autoproxy/MultiplyReturnValue.java

@@ -1,48 +0,0 @@
-/*
- * Copyright 2002-2006 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.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.aop.aspectj.autoproxy;
-
-import org.aspectj.lang.ProceedingJoinPoint;
-import org.aspectj.lang.annotation.Around;
-import org.aspectj.lang.annotation.Aspect;
-
-/**
- * @author Rod Johnson
- */
-@Aspect
-public class MultiplyReturnValue {
-
-	private int multiple = 2;
-
-	public int invocations;
-
-	public void setMultiple(int multiple) {
-		this.multiple = multiple;
-	}
-
-	public int getMultiple() {
-		return this.multiple;
-	}
-
-	@Around("execution(int *.getAge())")
-	public Object doubleReturnValue(ProceedingJoinPoint pjp) throws Throwable {
-		++this.invocations;
-		int result = (Integer) pjp.proceed();
-		return result * this.multiple;
-	}
-
-}

org.springframework.aop/src/test/java/org/springframework/aop/config/AopNamespaceHandlerEventTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2007 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -36,8 +36,9 @@ import org.springframework.core.io.ClassPathResource;
 /**
  * @author Rob Harrop
  * @author Juergen Hoeller
+ * @author Chris Beams
  */
-public class AopNamespaceHandlerEventTests {
+public final class AopNamespaceHandlerEventTests {
 
 	private CollectingReaderEventListener eventListener = new CollectingReaderEventListener();
 

org.springframework.aop/src/test/java/org/springframework/aop/config/AopNamespaceHandlerPointcutErrorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2006 the original author or authors.
+ * Copyright 2002-2008 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.
@@ -26,8 +26,9 @@ import org.springframework.core.io.ClassPathResource;
 
 /**
  * @author Mark Fisher
+ * @author Chris Beams
  */
-public class AopNamespaceHandlerPointcutErrorTests {
+public final class AopNamespaceHandlerPointcutErrorTests {
 
 	@Test
 	public void testDuplicatePointcutConfig() {

org.springframework.aop/src/test/java/org/springframework/aop/config/TopLevelAopTagTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2006 the original author or authors.
+ * Copyright 2002-2008 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.