diff --git a/spring-ldap/src/main/java/org/springframework/ldap/support/LdapUtils.java b/spring-ldap/src/main/java/org/springframework/ldap/support/LdapUtils.java index c82382a8..8e8bc539 100644 --- a/spring-ldap/src/main/java/org/springframework/ldap/support/LdapUtils.java +++ b/spring-ldap/src/main/java/org/springframework/ldap/support/LdapUtils.java @@ -21,6 +21,7 @@ import java.lang.reflect.Method; import javax.naming.Name; import javax.naming.directory.DirContext; +import javax.naming.ldap.LdapContext; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; @@ -335,4 +336,22 @@ public abstract class LdapUtils { } } } + + /** + * Get the actual class of the supplied DirContext instance; LdapContext or + * DirContext. + * + * @param context + * the DirContext instance to check. + * @return LdapContext.class if context is an LdapContext, DirContext.class + * otherwise. + */ + public static Class getActualTargetClass(DirContext context) { + if (context instanceof LdapContext) { + return LdapContext.class; + } + + return DirContext.class; + } + } diff --git a/spring-ldap/src/main/java/org/springframework/ldap/transaction/DefaultCompensatingTransactionOperationManager.java b/spring-ldap/src/main/java/org/springframework/ldap/transaction/DefaultCompensatingTransactionOperationManager.java index 16af2734..6bb097ab 100644 --- a/spring-ldap/src/main/java/org/springframework/ldap/transaction/DefaultCompensatingTransactionOperationManager.java +++ b/spring-ldap/src/main/java/org/springframework/ldap/transaction/DefaultCompensatingTransactionOperationManager.java @@ -15,10 +15,12 @@ */ package org.springframework.ldap.transaction; +import java.util.Iterator; import java.util.Stack; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.transaction.TransactionSystemException; /** * Default implementation of {@link CompensatingTransactionOperationManager}. @@ -33,7 +35,7 @@ public class DefaultCompensatingTransactionOperationManager implements private static Log log = LogFactory .getLog(DefaultCompensatingTransactionOperationManager.class); - private Stack rollbackOperations = new Stack(); + private Stack operationExecutors = new Stack(); private CompensatingTransactionOperationFactory operationFactory; @@ -63,7 +65,7 @@ public class DefaultCompensatingTransactionOperationManager implements executor.performOperation(); // Don't push the executor until the actual operation passed. - rollbackOperations.push(executor); + operationExecutors.push(executor); } /* @@ -73,10 +75,15 @@ public class DefaultCompensatingTransactionOperationManager implements */ public void rollback() { log.debug("Performing rollback"); - while (!rollbackOperations.isEmpty()) { - CompensatingTransactionOperationExecutor rollbackOperation = (CompensatingTransactionOperationExecutor) rollbackOperations + while (!operationExecutors.isEmpty()) { + CompensatingTransactionOperationExecutor rollbackOperation = (CompensatingTransactionOperationExecutor) operationExecutors .pop(); - rollbackOperation.rollback(); + try { + rollbackOperation.rollback(); + } catch (Exception e) { + throw new TransactionSystemException( + "Error occurred during rollback", e); + } } } @@ -85,19 +92,19 @@ public class DefaultCompensatingTransactionOperationManager implements * * @return the rollback operations. */ - protected Stack getRollbackOperations() { - return rollbackOperations; + protected Stack getOperationExecutors() { + return operationExecutors; } /** * Set the rollback operations. Package protected - for testing purposes * only. * - * @param rollbackOperations + * @param operationExecutors * the rollback operations. */ - void setRollbackOperations(Stack rollbackOperations) { - this.rollbackOperations = rollbackOperations; + void setOperationExecutors(Stack operationExecutors) { + this.operationExecutors = operationExecutors; } /* @@ -107,11 +114,15 @@ public class DefaultCompensatingTransactionOperationManager implements */ public void commit() { log.debug("Performing rollback"); - // TODO: Should this really be done in reverse order? - while (!rollbackOperations.isEmpty()) { - CompensatingTransactionOperationExecutor rollbackOperation = (CompensatingTransactionOperationExecutor) rollbackOperations - .pop(); - rollbackOperation.commit(); + for (Iterator iter = operationExecutors.iterator(); iter.hasNext();) { + CompensatingTransactionOperationExecutor operationExecutor = (CompensatingTransactionOperationExecutor) iter + .next(); + try { + operationExecutor.commit(); + } catch (Exception e) { + throw new TransactionSystemException( + "Error occurred during commit", e); + } } } diff --git a/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/DirContextProxy.java b/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/DirContextProxy.java index 0ff0d744..75e69a8b 100644 --- a/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/DirContextProxy.java +++ b/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/DirContextProxy.java @@ -23,7 +23,7 @@ import javax.naming.directory.DirContext; * * @author Mattias Arthursson */ -public interface DirContextProxy extends DirContext { +public interface DirContextProxy { /** * Get the target DirContext of the proxy. * diff --git a/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/LdapCompensatingTransactionOperationFactory.java b/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/LdapCompensatingTransactionOperationFactory.java index 1079c70d..e932122a 100644 --- a/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/LdapCompensatingTransactionOperationFactory.java +++ b/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/LdapCompensatingTransactionOperationFactory.java @@ -124,7 +124,9 @@ public class LdapCompensatingTransactionOperationFactory implements private DirContext getNonClosingDirContextProxy(DirContext context) { return (DirContext) Proxy.newProxyInstance(DirContextProxy.class - .getClassLoader(), new Class[] { DirContextProxy.class }, + .getClassLoader(), new Class[] { + LdapUtils.getActualTargetClass(context), + DirContextProxy.class }, new NonClosingDirContextInvocationHandler(context)); } diff --git a/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/TransactionAwareContextSourceProxy.java b/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/TransactionAwareContextSourceProxy.java index 283810ff..17d73513 100644 --- a/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/TransactionAwareContextSourceProxy.java +++ b/spring-ldap/src/main/java/org/springframework/ldap/transaction/core/TransactionAwareContextSourceProxy.java @@ -21,6 +21,7 @@ import javax.naming.directory.DirContext; import org.springframework.dao.DataAccessException; import org.springframework.ldap.core.ContextSource; +import org.springframework.ldap.support.LdapUtils; import org.springframework.transaction.support.TransactionSynchronizationManager; /** @@ -68,7 +69,8 @@ public class TransactionAwareContextSourceProxy implements ContextSource { ContextSource target) { return (DirContext) Proxy .newProxyInstance(DirContextProxy.class.getClassLoader(), - new Class[] { DirContextProxy.class }, + new Class[] { LdapUtils.getActualTargetClass(context), + DirContextProxy.class }, new TransactionAwareDirContextInvocationHandler( context, target)); diff --git a/spring-ldap/src/test/java/org/springframework/ldap/core/DistinguishedNameTest.java b/spring-ldap/src/test/java/org/springframework/ldap/core/DistinguishedNameTest.java index 40f557b4..6b96c490 100644 --- a/spring-ldap/src/test/java/org/springframework/ldap/core/DistinguishedNameTest.java +++ b/spring-ldap/src/test/java/org/springframework/ldap/core/DistinguishedNameTest.java @@ -447,4 +447,10 @@ public class DistinguishedNameTest extends TestCase { assertTrue(result < 0); } + public void test_longDN() throws InvalidNameException { + DistinguishedName name = new DistinguishedName( + ""); + assertNotNull(name); + } + } \ No newline at end of file diff --git a/spring-ldap/src/test/java/org/springframework/ldap/transaction/DefaultCompensatingTransactionOperationManagerTest.java b/spring-ldap/src/test/java/org/springframework/ldap/transaction/DefaultCompensatingTransactionOperationManagerTest.java index f22ca6f3..ee2a02cb 100644 --- a/spring-ldap/src/test/java/org/springframework/ldap/transaction/DefaultCompensatingTransactionOperationManagerTest.java +++ b/spring-ldap/src/test/java/org/springframework/ldap/transaction/DefaultCompensatingTransactionOperationManagerTest.java @@ -5,10 +5,7 @@ import java.util.Stack; import junit.framework.TestCase; import org.easymock.MockControl; -import org.springframework.ldap.transaction.CompensatingTransactionOperationExecutor; -import org.springframework.ldap.transaction.CompensatingTransactionOperationFactory; -import org.springframework.ldap.transaction.CompensatingTransactionOperationRecorder; -import org.springframework.ldap.transaction.DefaultCompensatingTransactionOperationManager; +import org.springframework.transaction.TransactionSystemException; public class DefaultCompensatingTransactionOperationManagerTest extends TestCase { @@ -85,7 +82,7 @@ public class DefaultCompensatingTransactionOperationManagerTest extends tested.performOperation("some method", expectedArgs); verify(); - Stack result = tested.getRollbackOperations(); + Stack result = tested.getOperationExecutors(); assertFalse(result.isEmpty()); assertSame(operationExecutorMock, result.peek()); } @@ -93,7 +90,7 @@ public class DefaultCompensatingTransactionOperationManagerTest extends public void testRollback() { DefaultCompensatingTransactionOperationManager tested = new DefaultCompensatingTransactionOperationManager( operationFactoryMock); - tested.getRollbackOperations().push(operationExecutorMock); + tested.getOperationExecutors().push(operationExecutorMock); operationExecutorMock.rollback(); @@ -102,10 +99,28 @@ public class DefaultCompensatingTransactionOperationManagerTest extends verify(); } + public void testRollback_Exception() { + DefaultCompensatingTransactionOperationManager tested = new DefaultCompensatingTransactionOperationManager( + operationFactoryMock); + tested.getOperationExecutors().push(operationExecutorMock); + + operationExecutorMock.rollback(); + operationExecutorControl.setThrowable(new RuntimeException()); + + replay(); + try { + tested.rollback(); + fail("TransactionSystemException expected"); + } catch (TransactionSystemException expected) { + assertTrue(true); + } + verify(); + } + public void testCommit() { DefaultCompensatingTransactionOperationManager tested = new DefaultCompensatingTransactionOperationManager( operationFactoryMock); - tested.getRollbackOperations().push(operationExecutorMock); + tested.getOperationExecutors().push(operationExecutorMock); operationExecutorMock.commit(); @@ -113,4 +128,22 @@ public class DefaultCompensatingTransactionOperationManagerTest extends tested.commit(); verify(); } + + public void testCommit_Exception() { + DefaultCompensatingTransactionOperationManager tested = new DefaultCompensatingTransactionOperationManager( + operationFactoryMock); + tested.getOperationExecutors().push(operationExecutorMock); + + operationExecutorMock.commit(); + operationExecutorControl.setThrowable(new RuntimeException()); + + replay(); + try { + tested.commit(); + fail("TransactionSystemException expected"); + } catch (TransactionSystemException expected) { + assertTrue(true); + } + verify(); + } } diff --git a/spring-ldap/src/test/java/org/springframework/ldap/transaction/core/TransactionAwareContextSourceProxyTest.java b/spring-ldap/src/test/java/org/springframework/ldap/transaction/core/TransactionAwareContextSourceProxyTest.java new file mode 100644 index 00000000..d7fa93d1 --- /dev/null +++ b/spring-ldap/src/test/java/org/springframework/ldap/transaction/core/TransactionAwareContextSourceProxyTest.java @@ -0,0 +1,125 @@ +/* + * 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.ldap.transaction.core; + +import javax.naming.directory.DirContext; +import javax.naming.ldap.LdapContext; + +import junit.framework.TestCase; + +import org.easymock.MockControl; +import org.springframework.ldap.core.ContextSource; + +/** + * Tests for {@link TransactionAwareContextSourceProxy}. + * + * @author Mattias Arthursson + */ +public class TransactionAwareContextSourceProxyTest extends TestCase { + private MockControl contextSourceControl; + + private ContextSource contextSourceMock; + + private TransactionAwareContextSourceProxy tested; + + private MockControl ldapContextControl; + + private LdapContext ldapContextMock; + + private MockControl dirContextControl; + + private DirContext dirContextMock; + + protected void setUp() throws Exception { + + contextSourceControl = MockControl.createControl(ContextSource.class); + contextSourceMock = (ContextSource) contextSourceControl.getMock(); + + ldapContextControl = MockControl.createControl(LdapContext.class); + ldapContextMock = (LdapContext) ldapContextControl.getMock(); + + dirContextControl = MockControl.createControl(DirContext.class); + dirContextMock = (DirContext) dirContextControl.getMock(); + + tested = new TransactionAwareContextSourceProxy(contextSourceMock); + } + + protected void tearDown() throws Exception { + contextSourceControl = null; + contextSourceMock = null; + + ldapContextControl = null; + ldapContextMock = null; + + dirContextControl = null; + dirContextMock = null; + + tested = null; + } + + public void testGetReadWriteContext_LdapContext() { + contextSourceControl.expectAndReturn(contextSourceMock + .getReadWriteContext(), ldapContextMock); + + contextSourceControl.replay(); + + DirContext result = tested.getReadWriteContext(); + + contextSourceControl.verify(); + + assertNotNull("Result should not be null", result); + assertTrue("Should be an LdapContext instance", + result instanceof LdapContext); + assertTrue("Should be a DirContextProxy instance", + result instanceof DirContextProxy); + } + + public void testGetReadWriteContext_DirContext() { + contextSourceControl.expectAndReturn(contextSourceMock + .getReadWriteContext(), dirContextMock); + + contextSourceControl.replay(); + + DirContext result = tested.getReadWriteContext(); + + contextSourceControl.verify(); + + assertNotNull("Result should not be null", result); + assertTrue("Should be a DirContext instance", + result instanceof DirContext); + assertFalse("Should not be an LdapContext instance", + result instanceof LdapContext); + assertTrue("Should be a DirContextProxy instance", + result instanceof DirContextProxy); + } + + public void testGetReadOnlyContext_LdapContext() { + contextSourceControl.expectAndReturn(contextSourceMock + .getReadWriteContext(), ldapContextMock); + + contextSourceControl.replay(); + + DirContext result = tested.getReadOnlyContext(); + + contextSourceControl.verify(); + + assertNotNull("Result should not be null", result); + assertTrue("Should be an LdapContext instance", + result instanceof LdapContext); + assertTrue("Should be a DirContextProxy instance", + result instanceof DirContextProxy); + } +}