LDAP-179: DataSource connections not being released when LDAP connection fails in JDBC integrated transaction.

Now cleaning up in DataSource and Hibernate transaction managers in doBegin if failing to create LDAP transaction.

core/src/main/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndDataSourceTransactionManager.java

@@ -77,8 +77,14 @@ public class ContextSourceAndDataSourceTransactionManager extends
 
         super.doBegin(actualTransactionObject.getDataSourceTransactionObject(),
                 definition);
-        ldapManagerDelegate.doBegin(actualTransactionObject
-                .getLdapTransactionObject(), definition);
+        try {
+            ldapManagerDelegate.doBegin(actualTransactionObject
+                    .getLdapTransactionObject(), definition);
+        } catch (TransactionException e) {
+            // Failed to start LDAP transaction - make sure we clean up properly
+            super.doCleanupAfterCompletion(actualTransactionObject.getDataSourceTransactionObject());
+            throw e;
+        }
     }
 
     /*

core/src/main/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndHibernateTransactionManager.java

@@ -76,8 +76,14 @@ public class ContextSourceAndHibernateTransactionManager extends HibernateTransa
 
         super.doBegin(actualTransactionObject.getHibernateTransactionObject(),
                 definition);
-        ldapManagerDelegate.doBegin(actualTransactionObject
-                .getLdapTransactionObject(), definition);
+        try {
+            ldapManagerDelegate.doBegin(actualTransactionObject
+                    .getLdapTransactionObject(), definition);
+        } catch (TransactionException e) {
+            // Failed to start LDAP transaction - make sure we clean up properly
+            super.doCleanupAfterCompletion(actualTransactionObject.getHibernateTransactionObject());
+            throw e;
+        }
     }
 
     /*

test/integration-tests/src/test/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndDataSourceTransactionManagerLdap179IntegrationTest.java

@@ -0,0 +1,90 @@
+/*
+ * Copyright 2005-2010 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.compensating.manager;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.ldap.CommunicationException;
+import org.springframework.ldap.core.LdapTemplate;
+import org.springframework.ldap.itest.transaction.compensating.manager.DummyDao;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.AbstractJUnit4SpringContextTests;
+import org.springframework.transaction.CannotCreateTransactionException;
+import org.springframework.transaction.support.TransactionSynchronizationManager;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
+ * Integration tests for {@link org.springframework.ldap.transaction.compensating.manager.ContextSourceAndDataSourceTransactionManager}.
+ *
+ * @author Mattias Hellborg Arthursson
+ */
+@ContextConfiguration(locations = {"/conf/missingLdapAndJdbcTransactionTestContext.xml"})
+public class ContextSourceAndDataSourceTransactionManagerLdap179IntegrationTest extends AbstractJUnit4SpringContextTests {
+
+    private static Log log = LogFactory.getLog(ContextSourceAndDataSourceTransactionManagerLdap179IntegrationTest.class);
+
+    @Autowired
+    @Qualifier("dummyDao")
+    private DummyDao dummyDao;
+
+    @Autowired
+    private LdapTemplate ldapTemplate;
+
+    @Autowired
+    private JdbcTemplate jdbcTemplate;
+
+    @Before
+    public void prepareTestedInstance() throws Exception {
+        if (TransactionSynchronizationManager.isSynchronizationActive()) {
+            TransactionSynchronizationManager.clearSynchronization();
+        }
+    }
+
+    @After
+    public void cleanup() throws Exception {
+        jdbcTemplate.execute("drop table PERSON if exists");
+    }
+
+
+    @Test
+    public void verifyThatJdbcTransactionIsClosedIfLdapServerUnavailable_ldap179() {
+        try {
+            dummyDao.create("Sweden", "company1", "some testperson", "testperson", "some description");
+            fail("CannotCreateTransactionException expected");
+        } catch (CannotCreateTransactionException expected) {
+            assertTrue(expected.getCause() instanceof CommunicationException);
+        }
+
+        // Make sure there is no transaction synchronization
+        assertFalse(TransactionSynchronizationManager.isSynchronizationActive());
+
+        try {
+            dummyDao.create("Sweden", "company1", "some testperson", "testperson", "some description");
+            fail("CannotCreateTransactionException expected");
+        } catch (CannotCreateTransactionException expected) {
+            assertTrue(expected.getCause() instanceof CommunicationException);
+        }
+    }
+}

test/integration-tests/src/test/java/org/springframework/ldap/transaction/compensating/manager/hibernate/ContextSourceAndHibernateTransactionManagerLdap179IntegrationTest.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright 2005-2010 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.compensating.manager.hibernate;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.ldap.CommunicationException;
+import org.springframework.ldap.itest.transaction.compensating.manager.hibernate.OrgPerson;
+import org.springframework.ldap.itest.transaction.compensating.manager.hibernate.OrgPersonDao;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.AbstractJUnit4SpringContextTests;
+import org.springframework.transaction.CannotCreateTransactionException;
+import org.springframework.transaction.support.TransactionSynchronizationManager;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Integration tests for {@link org.springframework.ldap.transaction.compensating.manager.ContextSourceAndHibernateTransactionManager}.
+ *
+ * @author Hans Westerbeek
+ */
+@ContextConfiguration(locations = { "/conf/missingLdapAndHibernateTransactionTestContext.xml" })
+public class ContextSourceAndHibernateTransactionManagerLdap179IntegrationTest extends AbstractJUnit4SpringContextTests {
+
+	private static Log log = LogFactory.getLog(ContextSourceAndHibernateTransactionManagerLdap179IntegrationTest.class);
+
+	@Autowired
+	@Qualifier("dummyDao")
+	private OrgPersonDao dummyDao;
+
+	@Before
+	public void prepareTest() throws Exception {
+		if (TransactionSynchronizationManager.isSynchronizationActive()) {
+			TransactionSynchronizationManager.clearSynchronization();
+		}
+	}
+
+	@Test
+	public void testCreate() {
+		OrgPerson person = new OrgPerson();
+
+		person.setId(new Integer(2));
+		person.setDescription("some description");
+		person.setFullname("Some testperson");
+		person.setLastname("testperson");
+		person.setCountry("Sweden");
+		person.setCompany("company1");
+
+        try {
+            this.dummyDao.create(person);
+        } catch (CannotCreateTransactionException expected) {
+            assertTrue(expected.getCause() instanceof CommunicationException);
+        }
+
+        // Make sure there is no transaction synchronization
+        assertFalse(TransactionSynchronizationManager.isSynchronizationActive());
+
+        try {
+            this.dummyDao.create(person);
+        } catch (CannotCreateTransactionException expected) {
+            assertTrue(expected.getCause() instanceof CommunicationException);
+        }
+    }
+}

test/integration-tests/src/test/resources/conf/missingLdapAndHibernateTransactionTestContext.xml

@@ -0,0 +1,77 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xsi:schemaLocation="http://www.springframework.org/schema/beans
+                           http://www.springframework.org/schema/beans/spring-beans-2.0.xsd">
+
+    <import resource="classpath:/conf/commonTestContext.xml"/>
+
+    <!-- This LDAP server shouldn't exist, and that's what we're testing -->
+    <bean id="contextSource"
+          class="org.springframework.ldap.core.support.LdapContextSource">
+        <property name="userDn" value="${userDn}" />
+        <property name="password" value="${password}" />
+        <property name="url" value="ldap://127.0.0.1:1337" />
+    </bean>
+
+	<bean id="dataSource"
+		class="org.springframework.jdbc.datasource.DriverManagerDataSource">
+		<property name="driverClassName" value="org.hsqldb.jdbcDriver" />
+		<property name="url" value="jdbc:hsqldb:mem:aname" />
+		<property name="username" value="sa" />
+		<property name="password" value="" />
+	</bean>
+
+	<bean id="transactedContextSource"
+		class="org.springframework.ldap.transaction.compensating.manager.TransactionAwareContextSourceProxy">
+		<constructor-arg ref="contextSource" />
+	</bean>
+
+	<bean id="ldapTemplate"
+		class="org.springframework.ldap.core.LdapTemplate">
+		<constructor-arg ref="transactedContextSource" />
+	</bean>
+	
+	<bean id="hibernateTemplate" class="org.springframework.orm.hibernate3.HibernateTemplate">
+		<property name="sessionFactory" ref="sessionFactory" />
+	</bean>
+
+	 <bean id="sessionFactory" class="org.springframework.orm.hibernate3.LocalSessionFactoryBean">
+	    <property name="dataSource" ref="dataSource"/>
+	    <property name="mappingResources">
+	      <list>
+	        <value>conf/OrgPerson.hbm.xml</value>
+	      </list>
+	    </property>
+	    <property name="hibernateProperties">
+	      <value>
+	        hibernate.dialect=org.hibernate.dialect.HSQLDialect
+	        hibernate.hbm2ddl.auto=create
+	      </value>
+	    </property>
+	  </bean>
+	<bean id="transactionManager"
+		class="org.springframework.ldap.transaction.compensating.manager.ContextSourceAndHibernateTransactionManager">
+		<property name="sessionFactory" ref="sessionFactory" />
+		<property name="contextSource" ref="transactedContextSource" />
+	</bean>
+
+
+	<bean name="dummyDaoTarget"
+		class="org.springframework.ldap.itest.transaction.compensating.manager.hibernate.DummyDaoLdapAndHibernateImpl">
+		<property name="ldapTemplate" ref="ldapTemplate" />
+		<property name="sessionFactory" ref="sessionFactory" />
+	</bean>
+
+	<bean name="dummyDao"
+		class="org.springframework.transaction.interceptor.TransactionProxyFactoryBean">
+		<property name="transactionManager"
+			ref="transactionManager" />
+		<property name="target" ref="dummyDaoTarget" />
+		<property name="transactionAttributes">
+			<props>
+				<prop key="*">PROPAGATION_REQUIRES_NEW</prop>
+			</props>
+		</property>
+	</bean>
+</beans>

test/integration-tests/src/test/resources/conf/missingLdapAndJdbcTransactionTestContext.xml

@@ -0,0 +1,63 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xsi:schemaLocation="http://www.springframework.org/schema/beans
+                           http://www.springframework.org/schema/beans/spring-beans-2.0.xsd">
+    <import resource="classpath:/conf/commonTestContext.xml"/>
+
+    <!-- This LDAP server shouldn't exist, and that's what we're testing -->
+	<bean id="contextSource"
+		class="org.springframework.ldap.core.support.LdapContextSource">
+		<property name="userDn" value="${userDn}" />
+		<property name="password" value="${password}" />
+        <property name="url" value="ldap://127.0.0.1:1337" />
+	</bean>
+
+	<bean id="dataSource"
+		class="org.springframework.jdbc.datasource.DriverManagerDataSource">
+		<property name="driverClassName" value="org.hsqldb.jdbcDriver" />
+		<property name="url" value="jdbc:hsqldb:mem:aname" />
+		<property name="username" value="sa" />
+		<property name="password" value="" />
+	</bean>
+
+	<bean id="transactedContextSource"
+		class="org.springframework.ldap.transaction.compensating.manager.TransactionAwareContextSourceProxy">
+		<constructor-arg ref="contextSource" />
+	</bean>
+
+	<bean id="ldapTemplate"
+		class="org.springframework.ldap.core.LdapTemplate">
+		<constructor-arg ref="transactedContextSource" />
+	</bean>
+
+	<bean id="jdbcTemplate"
+		class="org.springframework.jdbc.core.JdbcTemplate">
+		<property name="dataSource" ref="dataSource" />
+	</bean>
+
+	<bean id="transactionManager"
+		class="org.springframework.ldap.transaction.compensating.manager.ContextSourceAndDataSourceTransactionManager">
+		<property name="dataSource" ref="dataSource" />
+		<property name="contextSource" ref="transactedContextSource" />
+	</bean>
+
+
+	<bean name="dummyDaoTarget"
+		class="org.springframework.ldap.itest.transaction.compensating.manager.LdapAndJdbcDummyDaoImpl">
+		<property name="ldapTemplate" ref="ldapTemplate" />
+		<property name="jdbcTemplate" ref="jdbcTemplate" />
+	</bean>
+
+	<bean name="dummyDao"
+		class="org.springframework.transaction.interceptor.TransactionProxyFactoryBean">
+		<property name="transactionManager"
+			ref="transactionManager" />
+		<property name="target" ref="dummyDaoTarget" />
+		<property name="transactionAttributes">
+			<props>
+				<prop key="*">PROPAGATION_REQUIRES_NEW</prop>
+			</props>
+		</property>
+	</bean>
+</beans>