From cc14bf33196ee70e461d87899b7398941fcdb1a2 Mon Sep 17 00:00:00 2001 From: Mattias Hellborg Arthursson Date: Wed, 14 Aug 2013 12:17:38 +0200 Subject: [PATCH] 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. --- ...SourceAndDataSourceTransactionManager.java | 10 ++- ...tSourceAndHibernateTransactionManager.java | 10 ++- ...nsactionManagerLdap179IntegrationTest.java | 90 +++++++++++++++++++ ...nsactionManagerLdap179IntegrationTest.java | 82 +++++++++++++++++ ...LdapAndHibernateTransactionTestContext.xml | 77 ++++++++++++++++ ...ssingLdapAndJdbcTransactionTestContext.xml | 63 +++++++++++++ 6 files changed, 328 insertions(+), 4 deletions(-) create mode 100644 test/integration-tests/src/test/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndDataSourceTransactionManagerLdap179IntegrationTest.java create mode 100755 test/integration-tests/src/test/java/org/springframework/ldap/transaction/compensating/manager/hibernate/ContextSourceAndHibernateTransactionManagerLdap179IntegrationTest.java create mode 100755 test/integration-tests/src/test/resources/conf/missingLdapAndHibernateTransactionTestContext.xml create mode 100644 test/integration-tests/src/test/resources/conf/missingLdapAndJdbcTransactionTestContext.xml diff --git a/core/src/main/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndDataSourceTransactionManager.java b/core/src/main/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndDataSourceTransactionManager.java index e09e3a45..e58be223 100644 --- a/core/src/main/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndDataSourceTransactionManager.java +++ b/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; + } } /* diff --git a/core/src/main/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndHibernateTransactionManager.java b/core/src/main/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndHibernateTransactionManager.java index 45e9565b..ccf7543a 100755 --- a/core/src/main/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndHibernateTransactionManager.java +++ b/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; + } } /* diff --git a/test/integration-tests/src/test/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndDataSourceTransactionManagerLdap179IntegrationTest.java b/test/integration-tests/src/test/java/org/springframework/ldap/transaction/compensating/manager/ContextSourceAndDataSourceTransactionManagerLdap179IntegrationTest.java new file mode 100644 index 00000000..fe7e8ab8 --- /dev/null +++ b/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); + } + } +} diff --git a/test/integration-tests/src/test/java/org/springframework/ldap/transaction/compensating/manager/hibernate/ContextSourceAndHibernateTransactionManagerLdap179IntegrationTest.java b/test/integration-tests/src/test/java/org/springframework/ldap/transaction/compensating/manager/hibernate/ContextSourceAndHibernateTransactionManagerLdap179IntegrationTest.java new file mode 100755 index 00000000..82bc4a3c --- /dev/null +++ b/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); + } + } +} diff --git a/test/integration-tests/src/test/resources/conf/missingLdapAndHibernateTransactionTestContext.xml b/test/integration-tests/src/test/resources/conf/missingLdapAndHibernateTransactionTestContext.xml new file mode 100755 index 00000000..fcef7e06 --- /dev/null +++ b/test/integration-tests/src/test/resources/conf/missingLdapAndHibernateTransactionTestContext.xml @@ -0,0 +1,77 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + conf/OrgPerson.hbm.xml + + + + + hibernate.dialect=org.hibernate.dialect.HSQLDialect + hibernate.hbm2ddl.auto=create + + + + + + + + + + + + + + + + + + + + PROPAGATION_REQUIRES_NEW + + + + diff --git a/test/integration-tests/src/test/resources/conf/missingLdapAndJdbcTransactionTestContext.xml b/test/integration-tests/src/test/resources/conf/missingLdapAndJdbcTransactionTestContext.xml new file mode 100644 index 00000000..5ece78e9 --- /dev/null +++ b/test/integration-tests/src/test/resources/conf/missingLdapAndJdbcTransactionTestContext.xml @@ -0,0 +1,63 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + PROPAGATION_REQUIRES_NEW + + + +