diff --git a/spring-aop/src/main/java/org/springframework/aop/target/CommonsPool2TargetSource.java b/spring-aop/src/main/java/org/springframework/aop/target/CommonsPool2TargetSource.java index 31b57a3da67..c75392c30be 100644 --- a/spring-aop/src/main/java/org/springframework/aop/target/CommonsPool2TargetSource.java +++ b/spring-aop/src/main/java/org/springframework/aop/target/CommonsPool2TargetSource.java @@ -43,14 +43,7 @@ import org.apache.commons.pool2.impl.GenericObjectPoolConfig; * meaningful validation. All exposed Commons Pool properties use the * corresponding Commons Pool defaults. * - *

Commons Pool 2.x uses object equality while Commons Pool 1.x used identity - * equality. This clearly means that Commons Pool 2 behaves differently if several - * instances having the same identity according to their {@link Object#equals(Object)} - * method are managed in the same pool. To provide a smooth upgrade, a - * backward-compatible pool is created by default; use {@link #setUseObjectEquality(boolean)} - * if you need the standard Commons Pool 2.x behavior. - * - *

Compatible with Apache Commons Pool 2.2 + *

Compatible with Apache Commons Pool 2.4 * * @author Rod Johnson * @author Rob Harrop @@ -81,8 +74,6 @@ public class CommonsPool2TargetSource extends AbstractPoolingTargetSource implem private boolean blockWhenExhausted = GenericObjectPoolConfig.DEFAULT_BLOCK_WHEN_EXHAUSTED; - private boolean useObjectEquality; - /** * The Apache Commons {@code ObjectPool} used to pool target objects */ @@ -197,24 +188,6 @@ public class CommonsPool2TargetSource extends AbstractPoolingTargetSource implem return blockWhenExhausted; } - /** - * Set if the pool should use object equality. Commons Pool 1.x has no specific requirement in - * that regard and allows two distinct instances being equal to be put in the same pool. However, - * this behavior has changed with commons pool 2. To preserve backward compatibility, the pool - * is configured to use reference equality ({@code false}. - */ - public void setUseObjectEquality(boolean useObjectEquality) { - this.useObjectEquality = useObjectEquality; - } - - /** - * Specify if the pool should use object equality. Return {@code false} if it should use - * reference equality (as it was the case for Commons Pool 1.x) - */ - public boolean isUseObjectEquality() { - return useObjectEquality; - } - /** * Creates and holds an ObjectPool instance. * @see #createObjectPool() @@ -251,8 +224,7 @@ public class CommonsPool2TargetSource extends AbstractPoolingTargetSource implem */ @Override public Object getTarget() throws Exception { - Object o = this.pool.borrowObject(); - return (isUseObjectEquality() ? o : ((IdentityWrapper)o).target); + return this.pool.borrowObject(); } /** @@ -260,8 +232,7 @@ public class CommonsPool2TargetSource extends AbstractPoolingTargetSource implem */ @Override public void releaseTarget(Object target) throws Exception { - Object value = (isUseObjectEquality() ? target : new IdentityWrapper(target)); - this.pool.returnObject(value); + this.pool.returnObject(target); } @Override @@ -291,9 +262,7 @@ public class CommonsPool2TargetSource extends AbstractPoolingTargetSource implem @Override public PooledObject makeObject() throws Exception { - Object target = newPrototypeInstance(); - Object poolValue = (isUseObjectEquality() ? target : new IdentityWrapper(target)); - return new DefaultPooledObject(poolValue); + return new DefaultPooledObject(newPrototypeInstance()); } @Override @@ -314,32 +283,4 @@ public class CommonsPool2TargetSource extends AbstractPoolingTargetSource implem public void passivateObject(PooledObject p) throws Exception { } - - /** - * Wraps the target type in the pool to restore the behavior of commons-pool 1.x. - */ - private static class IdentityWrapper { - private final Object target; - - public IdentityWrapper(Object target) { - this.target = target; - } - - @Override - public int hashCode() { - return System.identityHashCode(this.target); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - IdentityWrapper that = (IdentityWrapper) o; - - return !(target != null ? !(target == that.target) : that.target != null); - } - - } - } diff --git a/spring-context/src/test/java/org/springframework/aop/target/CommonsPool2TargetSourceTests.java b/spring-context/src/test/java/org/springframework/aop/target/CommonsPool2TargetSourceTests.java index 6dcf240e641..0302e536c7a 100644 --- a/spring-context/src/test/java/org/springframework/aop/target/CommonsPool2TargetSourceTests.java +++ b/spring-context/src/test/java/org/springframework/aop/target/CommonsPool2TargetSourceTests.java @@ -228,25 +228,6 @@ public class CommonsPool2TargetSourceTests { targetSource.releaseTarget(second); } - @Test - public void objectIdentityReleaseWithSeveralEqualInstances() throws Exception{ - CommonsPool2TargetSource targetSource = new CommonsPool2TargetSource(); - targetSource.setUseObjectEquality(true); - targetSource.setMaxWait(1); - prepareTargetSource(targetSource); - - Object first = targetSource.getTarget(); - Object second = targetSource.getTarget(); - assertTrue(first instanceof SerializablePerson); - assertTrue(second instanceof SerializablePerson); - assertEquals(first, second); - - targetSource.releaseTarget(first); - thrown.expect(IllegalStateException.class); // Only one object in pool - targetSource.releaseTarget(second); - - } - private void prepareTargetSource(CommonsPool2TargetSource targetSource) { String beanName = "target"; diff --git a/spring-context/src/test/java/org/springframework/aop/target/CommonsPoolTargetSourceTests.java b/spring-context/src/test/java/org/springframework/aop/target/CommonsPoolTargetSourceTests.java new file mode 100644 index 00000000000..976428be142 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/aop/target/CommonsPoolTargetSourceTests.java @@ -0,0 +1,219 @@ +/* + * 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. + * 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.target; + +import java.util.NoSuchElementException; + +import org.apache.commons.pool.impl.GenericObjectPool; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import org.springframework.aop.framework.Advised; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; +import org.springframework.context.support.StaticApplicationContext; +import org.springframework.core.io.ClassPathResource; +import org.springframework.tests.sample.beans.Person; +import org.springframework.tests.sample.beans.SerializablePerson; +import org.springframework.tests.sample.beans.SideEffectBean; +import org.springframework.util.SerializationTestUtils; + +import static org.junit.Assert.*; + +/** + * Tests for pooling invoker interceptor. + * TODO: need to make these tests stronger: it's hard to + * make too many assumptions about a pool. + * + * @author Rod Johnson + * @author Rob Harrop + * @author Chris Beams + */ +public class CommonsPoolTargetSourceTests { + + /** + * Initial count value set in bean factory XML + */ + private static final int INITIAL_COUNT = 10; + + private DefaultListableBeanFactory beanFactory; + + @Before + public void setUp() throws Exception { + this.beanFactory = new DefaultListableBeanFactory(); + new XmlBeanDefinitionReader(this.beanFactory).loadBeanDefinitions( + new ClassPathResource(getClass().getSimpleName() + "-context.xml", getClass())); + } + + /** + * We must simulate container shutdown, which should clear threads. + */ + @After + public void tearDown() { + // Will call pool.close() + this.beanFactory.destroySingletons(); + } + + private void testFunctionality(String name) { + SideEffectBean pooled = (SideEffectBean) beanFactory.getBean(name); + assertEquals(INITIAL_COUNT, pooled.getCount()); + pooled.doWork(); + assertEquals(INITIAL_COUNT + 1, pooled.getCount()); + + pooled = (SideEffectBean) beanFactory.getBean(name); + // Just check that it works--we can't make assumptions + // about the count + pooled.doWork(); + //assertEquals(INITIAL_COUNT + 1, apartment.getCount() ); + } + + @Test + public void testFunctionality() { + testFunctionality("pooled"); + } + + @Test + public void testFunctionalityWithNoInterceptors() { + testFunctionality("pooledNoInterceptors"); + } + + @Test + public void testConfigMixin() { + SideEffectBean pooled = (SideEffectBean) beanFactory.getBean("pooledWithMixin"); + assertEquals(INITIAL_COUNT, pooled.getCount()); + PoolingConfig conf = (PoolingConfig) beanFactory.getBean("pooledWithMixin"); + // TODO one invocation from setup + //assertEquals(1, conf.getInvocations()); + pooled.doWork(); + // assertEquals("No objects active", 0, conf.getActive()); + assertEquals("Correct target source", 25, conf.getMaxSize()); + // assertTrue("Some free", conf.getFree() > 0); + //assertEquals(2, conf.getInvocations()); + assertEquals(25, conf.getMaxSize()); + } + + @Test + public void testTargetSourceSerializableWithoutConfigMixin() throws Exception { + CommonsPoolTargetSource cpts = (CommonsPoolTargetSource) beanFactory.getBean("personPoolTargetSource"); + + SingletonTargetSource serialized = (SingletonTargetSource) SerializationTestUtils.serializeAndDeserialize(cpts); + assertTrue(serialized.getTarget() instanceof Person); + } + + + @Test + public void testProxySerializableWithoutConfigMixin() throws Exception { + Person pooled = (Person) beanFactory.getBean("pooledPerson"); + + //System.out.println(((Advised) pooled).toProxyConfigString()); + assertTrue(((Advised) pooled).getTargetSource() instanceof CommonsPoolTargetSource); + + //((Advised) pooled).setTargetSource(new SingletonTargetSource(new SerializablePerson())); + Person serialized = (Person) SerializationTestUtils.serializeAndDeserialize(pooled); + assertTrue(((Advised) serialized).getTargetSource() instanceof SingletonTargetSource); + serialized.setAge(25); + assertEquals(25, serialized.getAge()); + } + + @Test + public void testHitMaxSize() throws Exception { + int maxSize = 10; + + CommonsPoolTargetSource targetSource = new CommonsPoolTargetSource(); + targetSource.setMaxSize(maxSize); + targetSource.setMaxWait(1); + prepareTargetSource(targetSource); + + Object[] pooledInstances = new Object[maxSize]; + + for (int x = 0; x < maxSize; x++) { + Object instance = targetSource.getTarget(); + assertNotNull(instance); + pooledInstances[x] = instance; + } + + // should be at maximum now + try { + targetSource.getTarget(); + fail("Should throw NoSuchElementException"); + } + catch (NoSuchElementException ex) { + // desired + } + + // lets now release an object and try to accquire a new one + targetSource.releaseTarget(pooledInstances[9]); + pooledInstances[9] = targetSource.getTarget(); + + // release all objects + for (int i = 0; i < pooledInstances.length; i++) { + targetSource.releaseTarget(pooledInstances[i]); + } + } + + @Test + public void testHitMaxSizeLoadedFromContext() throws Exception { + Advised person = (Advised) beanFactory.getBean("maxSizePooledPerson"); + CommonsPoolTargetSource targetSource = (CommonsPoolTargetSource) person.getTargetSource(); + + int maxSize = targetSource.getMaxSize(); + Object[] pooledInstances = new Object[maxSize]; + + for (int x = 0; x < maxSize; x++) { + Object instance = targetSource.getTarget(); + assertNotNull(instance); + pooledInstances[x] = instance; + } + + // should be at maximum now + try { + targetSource.getTarget(); + fail("Should throw NoSuchElementException"); + } + catch (NoSuchElementException ex) { + // desired + } + + // lets now release an object and try to accquire a new one + targetSource.releaseTarget(pooledInstances[9]); + pooledInstances[9] = targetSource.getTarget(); + + // release all objects + for (int i = 0; i < pooledInstances.length; i++) { + targetSource.releaseTarget(pooledInstances[i]); + } + } + + @Test + public void testSetWhenExhaustedAction() { + CommonsPoolTargetSource targetSource = new CommonsPoolTargetSource(); + targetSource.setWhenExhaustedActionName("WHEN_EXHAUSTED_BLOCK"); + assertEquals(GenericObjectPool.WHEN_EXHAUSTED_BLOCK, targetSource.getWhenExhaustedAction()); + } + + private void prepareTargetSource(CommonsPoolTargetSource targetSource) { + String beanName = "target"; + + StaticApplicationContext applicationContext = new StaticApplicationContext(); + applicationContext.registerPrototype(beanName, SerializablePerson.class); + + targetSource.setTargetBeanName(beanName); + targetSource.setBeanFactory(applicationContext); + } + +} \ No newline at end of file diff --git a/spring-context/src/test/resources/org/springframework/aop/target/CommonsPoolTargetSourceTests-context.xml b/spring-context/src/test/resources/org/springframework/aop/target/CommonsPoolTargetSourceTests-context.xml new file mode 100644 index 00000000000..2dd0157922d --- /dev/null +++ b/spring-context/src/test/resources/org/springframework/aop/target/CommonsPoolTargetSourceTests-context.xml @@ -0,0 +1,69 @@ + + + + + + + 10 + + + + prototypeTest + 25 + + + + + getPoolingConfigMixin + + + + + + + + nop + + + + + + + + + poolConfigAdvisor + + true + + + + + + + + + prototypePerson + 10 + + + + + serializableNop + + + + + + + + + + + + serializableNop + + + \ No newline at end of file