diff --git a/core/src/main/java/org/springframework/ldap/pool/DelegatingContext.java b/core/src/main/java/org/springframework/ldap/pool/DelegatingContext.java index 1bca7781..1f70a288 100644 --- a/core/src/main/java/org/springframework/ldap/pool/DelegatingContext.java +++ b/core/src/main/java/org/springframework/ldap/pool/DelegatingContext.java @@ -172,7 +172,20 @@ public class DelegatingContext implements Context { //Return the object to the Pool and then null the pool reference try { - this.keyedObjectPool.returnObject(this.dirContextType, context); + boolean valid = true; + + if (context instanceof FailureAwareContext) { + FailureAwareContext failureAwareContext = (FailureAwareContext) context; + if(failureAwareContext.hasFailed()) { + valid = false; + } + } + + if (valid) { + this.keyedObjectPool.returnObject(this.dirContextType, context); + } else { + this.keyedObjectPool.invalidateObject(this.dirContextType, context); + } } catch (Exception e) { final NamingException namingException = new NamingException("Failed to return delegate Context to pool."); diff --git a/core/src/main/java/org/springframework/ldap/pool/FailureAwareContext.java b/core/src/main/java/org/springframework/ldap/pool/FailureAwareContext.java new file mode 100644 index 00000000..34a2e82b --- /dev/null +++ b/core/src/main/java/org/springframework/ldap/pool/FailureAwareContext.java @@ -0,0 +1,24 @@ +/* + * Copyright 2005-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.ldap.pool; + +/** + * @author Mattias Hellborg Arthursson + */ +public interface FailureAwareContext { + boolean hasFailed(); +} diff --git a/core/src/main/java/org/springframework/ldap/pool/factory/DirContextPoolableObjectFactory.java b/core/src/main/java/org/springframework/ldap/pool/factory/DirContextPoolableObjectFactory.java index b113dc34..80e3671c 100644 --- a/core/src/main/java/org/springframework/ldap/pool/factory/DirContextPoolableObjectFactory.java +++ b/core/src/main/java/org/springframework/ldap/pool/factory/DirContextPoolableObjectFactory.java @@ -19,11 +19,22 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.commons.pool.BaseKeyedPoolableObjectFactory; import org.springframework.ldap.core.ContextSource; +import org.springframework.ldap.core.DirContextProxy; import org.springframework.ldap.pool.DirContextType; +import org.springframework.ldap.pool.FailureAwareContext; import org.springframework.ldap.pool.validation.DirContextValidator; +import org.springframework.ldap.support.LdapUtils; import org.springframework.util.Assert; +import javax.naming.CommunicationException; import javax.naming.directory.DirContext; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; /** * Factory that creates {@link DirContext} instances for pooling via a @@ -59,6 +70,7 @@ import javax.naming.directory.DirContext; * * @author Eric Dalquist eric.dalquist@doit.wisc.edu + * @author Mattias Hellborg Arthursson */ class DirContextPoolableObjectFactory extends BaseKeyedPoolableObjectFactory { /** @@ -66,10 +78,21 @@ class DirContextPoolableObjectFactory extends BaseKeyedPoolableObjectFactory { */ protected final Log logger = LogFactory.getLog(this.getClass()); + private final static Set> DEFAULT_NONTRANSIENT_EXCEPTIONS + = new HashSet>(){{ + add(CommunicationException.class); + }}; + private ContextSource contextSource; private DirContextValidator dirContextValidator; + private Set> nonTransientExceptions = DEFAULT_NONTRANSIENT_EXCEPTIONS; + + void setNonTransientExceptions(Collection> nonTransientExceptions) { + this.nonTransientExceptions = new HashSet>(nonTransientExceptions); + } + /** * @return the contextSource */ @@ -131,7 +154,7 @@ class DirContextPoolableObjectFactory extends BaseKeyedPoolableObjectFactory { + " DirContext='" + readWriteContext + "'"); } - return readWriteContext; + return makeFailureAwareProxy(readWriteContext); } else if (contextType == DirContextType.READ_ONLY) { final DirContext readOnlyContext = this.contextSource @@ -142,13 +165,23 @@ class DirContextPoolableObjectFactory extends BaseKeyedPoolableObjectFactory { + " DirContext='" + readOnlyContext + "'"); } - return readOnlyContext; + return makeFailureAwareProxy(readOnlyContext); } else { throw new IllegalArgumentException("Unrecognized ContextType: " + contextType); } } + private Object makeFailureAwareProxy(DirContext readOnlyContext) { + return Proxy.newProxyInstance(DirContextProxy.class + .getClassLoader(), + new Class[]{ + LdapUtils.getActualTargetClass(readOnlyContext), + DirContextProxy.class, + FailureAwareContext.class}, + new FailureAwareContextProxy(readOnlyContext)); + } + /** * @see org.apache.commons.pool.BaseKeyedPoolableObjectFactory#validateObject(java.lang.Object, * java.lang.Object) @@ -200,4 +233,61 @@ class DirContextPoolableObjectFactory extends BaseKeyedPoolableObjectFactory { "An exception occured while closing '" + obj + "'", e); } } + + /** + * Invocation handler that checks thrown exceptions against the configured {@link #nonTransientExceptions}, + * marking the Context as invalid on match. + * + * @author Mattias Hellborg Arthursson + * @since 2.0 + */ + private class FailureAwareContextProxy implements + InvocationHandler { + + private DirContext target; + + private boolean hasFailed = false; + + public FailureAwareContextProxy(DirContext target) { + Assert.notNull(target, "Target must not be null"); + this.target = target; + } + + /* + * @see java.lang.reflect.InvocationHandler#invoke(java.lang.Object, + * java.lang.reflect.Method, java.lang.Object[]) + */ + public Object invoke(Object proxy, Method method, Object[] args) + throws Throwable { + + String methodName = method.getName(); + if (methodName.equals("getTargetContext")) { + return target; + } else if (methodName.equals("hasFailed")) { + return hasFailed; + } + + try { + return method.invoke(target, args); + } + catch (InvocationTargetException e) { + Throwable targetException = e.getTargetException(); + Class targetExceptionClass = targetException.getClass(); + if(nonTransientExceptions.contains(targetExceptionClass)) { + logger.info( + String.format("An %s - explicitly configured to be a non-transient exception - encountered; eagerly invalidating the target context.", + targetExceptionClass)); + hasFailed = true; + } else { + if (logger.isDebugEnabled()) { + logger.debug(String.format("An %s - not explicitly configured to be a non-transient exception - encountered; ignoring.", + targetExceptionClass)); + } + } + + throw targetException; + } + } + } + } diff --git a/core/src/main/java/org/springframework/ldap/pool/factory/PoolingContextSource.java b/core/src/main/java/org/springframework/ldap/pool/factory/PoolingContextSource.java index ca0fc614..283d106b 100644 --- a/core/src/main/java/org/springframework/ldap/pool/factory/PoolingContextSource.java +++ b/core/src/main/java/org/springframework/ldap/pool/factory/PoolingContextSource.java @@ -16,9 +16,6 @@ package org.springframework.ldap.pool.factory; -import javax.naming.directory.DirContext; -import javax.naming.ldap.LdapContext; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.commons.pool.impl.GenericKeyedObjectPool; @@ -31,6 +28,10 @@ import org.springframework.ldap.pool.DelegatingLdapContext; import org.springframework.ldap.pool.DirContextType; import org.springframework.ldap.pool.validation.DirContextValidator; +import javax.naming.directory.DirContext; +import javax.naming.ldap.LdapContext; +import java.util.Collection; + /** * A {@link ContextSource} implementation that wraps an object pool and another * {@link ContextSource}. {@link DirContext}s are retrieved from the pool which @@ -376,7 +377,23 @@ public class PoolingContextSource implements ContextSource, DisposableBean { this.dirContextPoolableObjectFactory.setDirContextValidator(dirContextValidator); } - // ***** DisposableBean interface methods *****// + /** + * Configure the exception classes that are to be interpreted as no-transient with regards to eager + * context invalidation. If one of the configured exceptions is thrown by any method on a pooled + * DirContext, that instance will immediately be marked as invalid without any additional testing + * (i.e. testOnReturn). This allows for more efficient management of dead connections. Default is + * {@link javax.naming.CommunicationException}. + * + * @param nonTransientExceptions the exception classes that should be interpreted as non-transient + * with regards to eager invalidation. + * @since 2.0 + */ + public void setNonTransientExceptions(Collection> nonTransientExceptions) { + this.dirContextPoolableObjectFactory.setNonTransientExceptions(nonTransientExceptions); + } + + + // ***** DisposableBean interface methods *****// /* * (non-Javadoc) diff --git a/core/src/test/java/org/springframework/ldap/pool/factory/DirContextPoolableObjectFactoryTest.java b/core/src/test/java/org/springframework/ldap/pool/factory/DirContextPoolableObjectFactoryTest.java index d11ea9b0..2b04b8f9 100644 --- a/core/src/test/java/org/springframework/ldap/pool/factory/DirContextPoolableObjectFactoryTest.java +++ b/core/src/test/java/org/springframework/ldap/pool/factory/DirContextPoolableObjectFactoryTest.java @@ -17,12 +17,15 @@ package org.springframework.ldap.pool.factory; import org.junit.Test; import org.mockito.Mockito; +import org.mockito.internal.util.reflection.Whitebox; import org.springframework.ldap.core.ContextSource; import org.springframework.ldap.pool.AbstractPoolTestCase; import org.springframework.ldap.pool.DirContextType; import org.springframework.ldap.pool.validation.DirContextValidator; import javax.naming.directory.DirContext; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Proxy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -99,7 +102,8 @@ public class DirContextPoolableObjectFactoryTest extends AbstractPoolTestCase { objectFactory.setContextSource(contextSourceMock); final Object createdDirContext = objectFactory.makeObject(DirContextType.READ_ONLY); - assertEquals(readOnlyContextMock, createdDirContext); + InvocationHandler invocationHandler = Proxy.getInvocationHandler(createdDirContext); + assertEquals(readOnlyContextMock, Whitebox.getInternalState(invocationHandler, "target")); } @Test @@ -111,9 +115,10 @@ public class DirContextPoolableObjectFactoryTest extends AbstractPoolTestCase { when(contextSourceMock.getReadWriteContext()).thenReturn(readWriteContextMock); objectFactory.setContextSource(contextSourceMock); - final Object createdDirContext = objectFactory.makeObject(DirContextType.READ_WRITE); - assertEquals(readWriteContextMock, createdDirContext); + + InvocationHandler invocationHandler = Proxy.getInvocationHandler(createdDirContext); + assertEquals(readWriteContextMock, Whitebox.getInternalState(invocationHandler, "target")); } @Test diff --git a/src/docbkx/pooling.xml b/src/docbkx/pooling.xml index 2df9a509..5eca8f1e 100644 --- a/src/docbkx/pooling.xml +++ b/src/docbkx/pooling.xml @@ -82,6 +82,15 @@ DirContext . + + Connections will be automatically invalidated if they throw an exception that is considered + non-transient. E.g. if a DirContext instance throws a + javax.naming.CommunicationException, this will be interpreted + as a non-transient error and the instance will be automatically invalidated, without the overhead + of an additional testOnReturn operation. The exceptions that are interpreted as non-transient are + configured using the nonTransientExceptions property of the + PoolingContextSource. + @@ -408,6 +417,25 @@ any). + + + + nonTransientExceptions + + + + + javax.naming.CommunicationException + + + + The Exceptions that should be considered non-transient with + regards to eager invalidation. Should any of the listed exceptions be + thrown by a call to a pooled DirContext instance, + that object will be automatically invalidated without any additional + testOnReturn operation. + + diff --git a/test-support/src/main/java/org/springframework/ldap/test/LdapTestUtils.java b/test-support/src/main/java/org/springframework/ldap/test/LdapTestUtils.java index 7fc3b952..5e39673b 100644 --- a/test-support/src/main/java/org/springframework/ldap/test/LdapTestUtils.java +++ b/test-support/src/main/java/org/springframework/ldap/test/LdapTestUtils.java @@ -297,5 +297,4 @@ public class LdapTestUtils { return attributes; } - } diff --git a/test/integration-tests/src/test/java/org/springframework/ldap/itest/LdapTemplatePooledITest.java b/test/integration-tests/src/test/java/org/springframework/ldap/itest/LdapTemplatePooledITest.java new file mode 100644 index 00000000..ee263116 --- /dev/null +++ b/test/integration-tests/src/test/java/org/springframework/ldap/itest/LdapTemplatePooledITest.java @@ -0,0 +1,87 @@ +/* + * Copyright 2005-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.ldap.itest; + +import org.junit.After; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.core.io.ClassPathResource; +import org.springframework.ldap.core.ContextSource; +import org.springframework.ldap.core.DirContextOperations; +import org.springframework.ldap.core.LdapTemplate; +import org.springframework.ldap.support.LdapUtils; +import org.springframework.ldap.test.LdapTestUtils; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.AbstractJUnit4SpringContextTests; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertTrue; +import static junit.framework.Assert.fail; + +/** + * Tests the lookup methods of LdapTemplate. + * + * @author Mattias Hellborg Arthursson + * @author Ulrik Sandberg + */ +@ContextConfiguration(locations = {"/conf/ldapTemplatePooledTestContext.xml"}) +public class LdapTemplatePooledITest extends AbstractJUnit4SpringContextTests { + + @Autowired + private LdapTemplate tested; + + @Autowired + @Qualifier("contextSourceTarget") + protected ContextSource contextSource; + + @After + public void cleanup() throws Exception { + LdapTestUtils.shutdownEmbeddedServer(); + } + + /** + * This method depends on a DirObjectFactory ( + * {@link org.springframework.ldap.core.support.DefaultDirObjectFactory}) + * being set in the ContextSource. + */ + @Test + public void verifyThatInvalidConnectionIsAutomaticallyPurged() throws Exception { + LdapTestUtils.startEmbeddedServer(1888, "dc=jayway,dc=se", "jayway"); + LdapTestUtils.cleanAndSetup(contextSource, LdapUtils.emptyLdapName(), new ClassPathResource("/setup_data.ldif")); + + DirContextOperations result = tested.lookupContext("cn=Some Person2, ou=company1,c=Sweden"); + assertEquals("Some Person2", result.getStringAttribute("cn")); + assertEquals("Person2", result.getStringAttribute("sn")); + assertEquals("Sweden, Company1, Some Person2", result.getStringAttribute("description")); + + // Shutdown server and kill all existing connections + LdapTestUtils.shutdownEmbeddedServer(); + LdapTestUtils.startEmbeddedServer(1888, "dc=jayway,dc=se", "jayway"); + + try { + tested.lookup("cn=Some Person2, ou=company1,c=Sweden"); + fail("Exception expected"); + } catch (Exception expected) { + // This should fail because the target connection was closed + assertTrue(true); + } + + // But this should be OK, because the dirty connection should have been automatically purged. + tested.lookup("cn=Some Person2, ou=company1,c=Sweden"); + } +} diff --git a/test/integration-tests/src/test/resources/conf/ldapTemplatePooledTestContext.xml b/test/integration-tests/src/test/resources/conf/ldapTemplatePooledTestContext.xml new file mode 100644 index 00000000..80d94570 --- /dev/null +++ b/test/integration-tests/src/test/resources/conf/ldapTemplatePooledTestContext.xml @@ -0,0 +1,50 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +