diff --git a/spring-context/src/test/java/org/springframework/aop/framework/AbstractAopProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/AbstractAopProxyTests.java index e89d4727507..2e554c192be 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/AbstractAopProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/AbstractAopProxyTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -28,17 +28,13 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; -import junit.framework.TestCase; import org.aopalliance.aop.Advice; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; + import org.junit.After; import org.junit.Before; import org.junit.Test; -import test.mixin.LockMixin; -import test.mixin.LockMixinAdvisor; -import test.mixin.Lockable; -import test.mixin.LockedException; import org.springframework.aop.Advisor; import org.springframework.aop.AfterReturningAdvice; @@ -76,6 +72,11 @@ import org.springframework.tests.sample.beans.TestBean; import org.springframework.util.SerializationTestUtils; import org.springframework.util.StopWatch; +import test.mixin.LockMixin; +import test.mixin.LockMixinAdvisor; +import test.mixin.Lockable; +import test.mixin.LockedException; + import static org.junit.Assert.*; /** @@ -120,18 +121,12 @@ public abstract class AbstractAopProxyTests { } - @Test + @Test(expected = AopConfigException.class) public void testNoInterceptorsAndNoTarget() { - AdvisedSupport pc = new AdvisedSupport(new Class[] {ITestBean.class}); + AdvisedSupport pc = new AdvisedSupport(new Class[] { ITestBean.class }); // Add no interceptors - try { - AopProxy aop = createAopProxy(pc); - aop.getProxy(); - fail("Shouldn't allow no interceptors"); - } - catch (AopConfigException ex) { - // Ok - } + AopProxy aop = createAopProxy(pc); + aop.getProxy(); } /** @@ -171,7 +166,6 @@ public abstract class AbstractAopProxyTests { sw.start("Create " + howMany + " proxies"); testManyProxies(howMany); sw.stop(); - System.out.println(sw.getTotalTimeMillis()); assertTrue("Proxy creation was too slow", sw.getTotalTimeMillis() < 5000); } @@ -243,7 +237,7 @@ public abstract class AbstractAopProxyTests { p.echo(new IOException()); } catch (IOException ex) { - + /* expected */ } assertEquals(1, cta.getCalls()); @@ -281,7 +275,6 @@ public abstract class AbstractAopProxyTests { } assertEquals(2, cta.getCalls()); - } /** @@ -317,7 +310,6 @@ public abstract class AbstractAopProxyTests { pf2.addAdvice(2, new CheckMethodInvocationIsSameInAndOutInterceptor()); pf2.addAdvice(1, new CheckMethodInvocationViaThreadLocalIsSameInAndOutInterceptor()); pf2.addAdvice(0, ExposeInvocationInterceptor.INSTANCE); - //System.err.println(pf2.toProxyConfigString()); ITestBean advised2 = (ITestBean) createProxy(pf2); advised2.setAge(age2); advised1.setSpouse(advised2); // = 2 invocations @@ -379,20 +371,14 @@ public abstract class AbstractAopProxyTests { assertEquals("3 more invocations via AOP as the first call was reentrant through the proxy", 4, di.getCount()); } - - @Test + @Test(expected = IllegalStateException.class) + // Should fail to get proxy as exposeProxy wasn't set to true public void testTargetCantGetProxyByDefault() { NeedsToSeeProxy et = new NeedsToSeeProxy(); ProxyFactory pf1 = new ProxyFactory(et); assertFalse(pf1.isExposeProxy()); INeedsToSeeProxy proxied = (INeedsToSeeProxy) createProxy(pf1); - try { - proxied.incrementViaProxy(); - fail("Should have failed to get proxy as exposeProxy wasn't set to true"); - } - catch (IllegalStateException ex) { - // Ok - } + proxied.incrementViaProxy(); } @Test @@ -417,7 +403,7 @@ public abstract class AbstractAopProxyTests { if (!context) { assertNoInvocationContext(); } else { - assertTrue("have context", ExposeInvocationInterceptor.currentInvocation() != null); + assertNotNull("have context", ExposeInvocationInterceptor.currentInvocation()); } return s; } @@ -436,7 +422,7 @@ public abstract class AbstractAopProxyTests { assertNoInvocationContext(); ITestBean tb = (ITestBean) aop.getProxy(); assertNoInvocationContext(); - assertTrue("correct return value", tb.getName() == s); + assertSame("correct return value", s, tb.getName()); } /** @@ -453,7 +439,7 @@ public abstract class AbstractAopProxyTests { pc.setTarget(raw); ITestBean tb = (ITestBean) createProxy(pc); - assertTrue("this return is wrapped in proxy", tb.getSpouse() == tb); + assertSame("this return is wrapped in proxy", tb, tb.getSpouse()); } @Test @@ -593,15 +579,6 @@ public abstract class AbstractAopProxyTests { ITestBean tb = (ITestBean) aop.getProxy(); tb.getName(); - // Not safe to trap invocation - //assertTrue(tii.invocation == target.invocation); - - //assertTrue(target.invocation.getProxy() == tb); - - // ((IOther) tb).absquatulate(); - //MethodInvocation minv = tii.invocation; - //assertTrue("invoked on iother, not " + minv.getMethod().getDeclaringClass(), minv.getMethod().getDeclaringClass() == IOther.class); - //assertTrue(target.invocation == tii.invocation); } /** @@ -645,13 +622,13 @@ public abstract class AbstractAopProxyTests { int newAge = 65; ITestBean itb = (ITestBean) createProxy(pc); itb.setAge(newAge); - assertTrue(itb.getAge() == newAge); + assertEquals(newAge, itb.getAge()); Lockable lockable = (Lockable) itb; assertFalse(lockable.locked()); lockable.lock(); - assertTrue(itb.getAge() == newAge); + assertEquals(newAge, itb.getAge()); try { itb.setAge(1); fail("Setters should fail when locked"); @@ -659,16 +636,15 @@ public abstract class AbstractAopProxyTests { catch (LockedException ex) { // ok } - assertTrue(itb.getAge() == newAge); + assertEquals(newAge, itb.getAge()); // Unlock assertTrue(lockable.locked()); lockable.unlock(); itb.setAge(1); - assertTrue(itb.getAge() == 1); + assertEquals(1, itb.getAge()); } - @Test public void testReplaceArgument() throws Throwable { TestBean tb = new TestBean(); @@ -679,14 +655,14 @@ public abstract class AbstractAopProxyTests { ITestBean t = (ITestBean) pc.getProxy(); int newAge = 5; t.setAge(newAge); - assertTrue(t.getAge() == newAge); + assertEquals(newAge, t.getAge()); String newName = "greg"; t.setName(newName); assertEquals(newName, t.getName()); t.setName(null); // Null replacement magic should work - assertTrue(t.getName().equals("")); + assertEquals("", t.getName()); } @Test @@ -1310,9 +1286,9 @@ public abstract class AbstractAopProxyTests { IOther proxyB = (IOther) createProxy(pfb); assertEquals(pfa.getAdvisors().length, pfb.getAdvisors().length); - assertTrue(a.equals(b)); - assertTrue(i1.equals(i2)); - assertTrue(proxyA.equals(proxyB)); + assertEquals(a, b); + assertEquals(i1, i2); + assertEquals(proxyA, proxyB); assertEquals(proxyA.hashCode(), proxyB.hashCode()); assertFalse(proxyA.equals(a)); @@ -1682,9 +1658,7 @@ public abstract class AbstractAopProxyTests { public Object invoke(MethodInvocation mi) throws Throwable { Object proxy = AopContext.currentProxy(); Object ret = mi.proceed(); - // TODO why does this cause stack overflow? - //assertEquals(proxy, AopContext.currentProxy()); - assertTrue(proxy == AopContext.currentProxy()); + assertEquals(proxy, AopContext.currentProxy()); return ret; } } @@ -1785,7 +1759,7 @@ public abstract class AbstractAopProxyTests { * Note that trapping the Invocation as in previous version of this test * isn't safe, as invocations may be reused * and hence cleared at the end of each invocation. - * So we trap only the targe. + * So we trap only the target. */ protected static class TrapTargetInterceptor implements MethodInterceptor { @@ -2105,11 +2079,10 @@ public abstract class AbstractAopProxyTests { static class InvocationCheckExposedInvocationTestBean extends ExposedInvocationTestBean { @Override protected void assertions(MethodInvocation invocation) { - TestCase.assertTrue(invocation.getThis() == this); - TestCase.assertTrue("Invocation should be on ITestBean: " + invocation.getMethod(), - ITestBean.class.isAssignableFrom(invocation.getMethod().getDeclaringClass())); + assertSame(this, invocation.getThis()); + assertTrue("Invocation should be on ITestBean: " + invocation.getMethod(), + ITestBean.class.isAssignableFrom(invocation.getMethod().getDeclaringClass())); } } } - diff --git a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java index ac5bf92e4e3..e129bbc5231 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java @@ -21,6 +21,7 @@ import java.io.Serializable; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.junit.Test; + import test.mixin.LockMixinAdvisor; import org.springframework.aop.ClassFilter; @@ -74,30 +75,17 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab return true; } - - @Test + @Test(expected = IllegalArgumentException.class) public void testNullConfig() { - try { - new CglibAopProxy(null); - fail("Shouldn't allow null interceptors"); - } - catch (IllegalArgumentException ex) { - // Ok - } + new CglibAopProxy(null); } - @Test + @Test(expected = AopConfigException.class) public void testNoTarget() { - AdvisedSupport pc = new AdvisedSupport(new Class[]{ITestBean.class}); + AdvisedSupport pc = new AdvisedSupport(new Class[] { ITestBean.class }); pc.addAdvice(new NopInterceptor()); - try { - AopProxy aop = createAopProxy(pc); - aop.getProxy(); - fail("Shouldn't allow no target with CGLIB proxy"); - } - catch (AopConfigException ex) { - // Ok - } + AopProxy aop = createAopProxy(pc); + aop.getProxy(); } @Test @@ -210,7 +198,7 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab ITestBean proxy1 = getAdvisedProxy(target); ITestBean proxy2 = getAdvisedProxy(target2); - assertTrue(proxy1.getClass() == proxy2.getClass()); + assertSame(proxy1.getClass(), proxy2.getClass()); assertEquals(target.getAge(), proxy1.getAge()); assertEquals(target2.getAge(), proxy2.getAge()); } @@ -256,7 +244,7 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab ITestBean proxy1 = getIntroductionAdvisorProxy(target); ITestBean proxy2 = getIntroductionAdvisorProxy(target2); - assertTrue("Incorrect duplicate creation of proxy classes", proxy1.getClass() == proxy2.getClass()); + assertSame("Incorrect duplicate creation of proxy classes", proxy1.getClass(), proxy2.getClass()); } private ITestBean getIntroductionAdvisorProxy(TestBean target) { @@ -427,6 +415,7 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab return x + y; } + @SuppressWarnings("unchecked") public boolean doWithVarargs(V... args) { return true; } diff --git a/spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java index 47d3b776986..7a194d9a434 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java @@ -20,6 +20,7 @@ import java.io.Serializable; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; + import org.junit.Test; import org.springframework.aop.interceptor.ExposeInvocationInterceptor; @@ -53,15 +54,9 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria } - @Test + @Test(expected = IllegalArgumentException.class) public void testNullConfig() { - try { - new JdkDynamicAopProxy(null); - fail("Shouldn't allow null interceptors"); - } - catch (IllegalArgumentException ex) { - // Ok - } + new JdkDynamicAopProxy(null); } @Test @@ -74,13 +69,13 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria Object proxy = aop.getProxy(); assertTrue(proxy instanceof ITestBean); - assertTrue(!(proxy instanceof TestBean)); + assertFalse(proxy instanceof TestBean); } @Test public void testInterceptorIsInvokedWithNoTarget() throws Throwable { // Test return value - final Integer age = 25; + final int age = 25; MethodInterceptor mi = (invocation -> age); AdvisedSupport pc = new AdvisedSupport(new Class[] {ITestBean.class}); @@ -88,7 +83,7 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria AopProxy aop = createAopProxy(pc); ITestBean tb = (ITestBean) aop.getProxy(); - assertTrue("correct return value", tb.getAge() == age); + assertEquals("correct return value", age, tb.getAge()); } @Test @@ -96,9 +91,9 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria final ExposedInvocationTestBean expectedTarget = new ExposedInvocationTestBean() { @Override protected void assertions(MethodInvocation invocation) { - assertTrue(invocation.getThis() == this); - assertTrue("Invocation should be on ITestBean: " + invocation.getMethod(), - invocation.getMethod().getDeclaringClass() == ITestBean.class); + assertEquals(this, invocation.getThis()); + assertEquals("Invocation should be on ITestBean: " + invocation.getMethod(), + ITestBean.class, invocation.getMethod().getDeclaringClass()); } }; @@ -118,15 +113,6 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria ITestBean tb = (ITestBean) aop.getProxy(); tb.getName(); - // Not safe to trap invocation - //assertTrue(tii.invocation == target.invocation); - - //assertTrue(target.invocation.getProxy() == tb); - - // ((IOther) tb).absquatulate(); - //MethodInvocation minv = tii.invocation; - //assertTrue("invoked on iother, not " + minv.getMethod().getDeclaringClass(), minv.getMethod().getDeclaringClass() == IOther.class); - //assertTrue(target.invocation == tii.invocation); } @Test @@ -139,7 +125,6 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria Foo proxy = (Foo) createProxy(as); assertSame("Target should be returned when return types are incompatible", bean, proxy.getBarThis()); assertSame("Proxy should be returned when return types are compatible", proxy, proxy.getFooThis()); - } @Test @@ -149,8 +134,8 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria JdkDynamicAopProxy aopProxy = new JdkDynamicAopProxy(as); Named proxy = (Named) aopProxy.getProxy(); Named named = new Person(); - assertEquals("equals() returned false", proxy, named); - assertEquals("hashCode() not equal", proxy.hashCode(), named.hashCode()); + assertEquals("equals()", proxy, named); + assertEquals("hashCode()", proxy.hashCode(), named.hashCode()); } diff --git a/spring-context/src/test/java/org/springframework/aop/framework/ObjenesisProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/ObjenesisProxyTests.java index 2b5225efa90..8732124b802 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/ObjenesisProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/ObjenesisProxyTests.java @@ -34,6 +34,7 @@ public class ObjenesisProxyTests { @Test public void appliesAspectToClassWithComplexConstructor() { + @SuppressWarnings("resource") ApplicationContext context = new ClassPathXmlApplicationContext("ObjenesisProxyTests-context.xml", getClass()); ClassWithComplexConstructor bean = context.getBean(ClassWithComplexConstructor.class);