diff --git a/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupport.java b/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupport.java index 74a3a85d31..66021f3d44 100644 --- a/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupport.java +++ b/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupport.java @@ -16,8 +16,10 @@ */ package org.alfresco.repo.transaction; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -485,6 +487,14 @@ public abstract class AlfrescoTransactionSupport { return listeners; } + + /** + * @return Returns the listeners in a list disconnected from the original set + */ + private List getListenersIterable() + { + return new ArrayList(listeners); + } public String toString() { @@ -511,7 +521,7 @@ public abstract class AlfrescoTransactionSupport integrityChecker.checkIntegrity(); } // flush listeners - for (TransactionListener listener : listeners) + for (TransactionListener listener : getListenersIterable()) { listener.flush(); } @@ -568,7 +578,7 @@ public abstract class AlfrescoTransactionSupport } // These are still considered part of the transaction so are executed here - for (TransactionListener listener : listeners) + for (TransactionListener listener : getListenersIterable()) { listener.beforeCommit(readOnly); } @@ -590,7 +600,7 @@ public abstract class AlfrescoTransactionSupport logger.debug("Before completion: " + this); } // notify listeners - for (TransactionListener listener : listeners) + for (TransactionListener listener : getListenersIterable()) { listener.beforeCompletion(); } @@ -636,10 +646,11 @@ public abstract class AlfrescoTransactionSupport } } + List iterableListeners = getListenersIterable(); // notify listeners if (status == TransactionSynchronization.STATUS_COMMITTED) { - for (TransactionListener listener : listeners) + for (TransactionListener listener : iterableListeners) { try { @@ -655,7 +666,7 @@ public abstract class AlfrescoTransactionSupport } else { - for (TransactionListener listener : listeners) + for (TransactionListener listener : iterableListeners) { try { diff --git a/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupportTest.java b/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupportTest.java index a4fd99ffcc..4a124556c9 100644 --- a/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupportTest.java +++ b/source/java/org/alfresco/repo/transaction/AlfrescoTransactionSupportTest.java @@ -23,6 +23,7 @@ import javax.transaction.UserTransaction; import junit.framework.TestCase; +import org.alfresco.repo.transaction.TransactionUtil.TransactionWork; import org.alfresco.service.ServiceRegistry; import org.alfresco.service.transaction.TransactionService; import org.alfresco.util.ApplicationContextHelper; @@ -42,10 +43,12 @@ public class AlfrescoTransactionSupportTest extends TestCase private static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(); private ServiceRegistry serviceRegistry; + TransactionService transactionService; public void setUp() throws Exception { serviceRegistry = (ServiceRegistry) ctx.getBean(ServiceRegistry.SERVICE_REGISTRY); + transactionService = serviceRegistry.getTransactionService(); } public void testTransactionId() throws Exception @@ -60,16 +63,6 @@ public class AlfrescoTransactionSupportTest extends TestCase String txnId = AlfrescoTransactionSupport.getTransactionId(); assertNotNull("Expected thread to have a txn id", txnId); - // check that it is threadlocal - Thread thread = new Thread(new Runnable() - { - public void run() - { - String txnId = AlfrescoTransactionSupport.getTransactionId(); - assertNull("New thread seeing txn id"); - } - }); - // check that the txn id doesn't change String txnIdCheck = AlfrescoTransactionSupport.getTransactionId(); assertEquals("Transaction ID changed on same thread", txnId, txnIdCheck); @@ -138,7 +131,6 @@ public class AlfrescoTransactionSupportTest extends TestCase }; // begin a transaction - TransactionService transactionService = serviceRegistry.getTransactionService(); UserTransaction txn = transactionService.getUserTransaction(); txn.begin(); @@ -155,4 +147,77 @@ public class AlfrescoTransactionSupportTest extends TestCase assertTrue("beforeCompletion not called on listener", strings.contains("beforeCompletion")); assertTrue("afterCommit not called on listener", strings.contains("afterCommit")); } + + /** + * Tests the condition whereby a listener can cause failure by attempting to bind itself to + * the transaction in the pre-commit callback. This is caused by the listener set being + * modified during calls to the listeners. + */ + public void testPreCommitListenerBinding() throws Exception + { + final String beforeCommit = "beforeCommit"; + final String afterCommitInner = "afterCommit - inner"; + final String afterCommitOuter = "afterCommit = outer"; + + // the listeners will play with this + final List testList = new ArrayList(1); + testList.add(beforeCommit); + testList.add(afterCommitInner); + testList.add(afterCommitOuter); + + final TransactionListener listener = new TransactionListenerAdapter() + { + @Override + public int hashCode() + { + // force this listener to be first in the bound set + return 100; + } + @Override + public void beforeCommit(boolean readOnly) + { + testList.remove(beforeCommit); + TransactionListener postCommitListener = new TransactionListenerAdapter() + { + @Override + public void afterCommit() + { + testList.remove(afterCommitInner); + } + }; + // register bogus on the transaction + AlfrescoTransactionSupport.bindListener(postCommitListener); + } + @Override + public void afterCommit() + { + testList.remove(afterCommitOuter); + } + }; + final TransactionListener dummyListener = new TransactionListenerAdapter() + { + @Override + public int hashCode() + { + // force the dummy listener to be AFTER the binding listener + return 200; + } + }; + // start a transaction + TransactionWork bindWork = new TransactionWork() + { + public Object doWork() throws Exception + { + // just bind the listener to the transaction + AlfrescoTransactionSupport.bindListener(dummyListener); + AlfrescoTransactionSupport.bindListener(listener); + return null; + } + }; + // kick it all off + TransactionUtil.executeInNonPropagatingUserTransaction(transactionService, bindWork); + + // make sure that the binding all worked + assertTrue("Expected callbacks not all processed: " + testList, testList.size() == 0); + } } diff --git a/source/java/org/alfresco/repo/transaction/TransactionListenerAdapter.java b/source/java/org/alfresco/repo/transaction/TransactionListenerAdapter.java new file mode 100644 index 0000000000..db972b7779 --- /dev/null +++ b/source/java/org/alfresco/repo/transaction/TransactionListenerAdapter.java @@ -0,0 +1,60 @@ +/* + * Copyright (C) 2005 Alfresco, Inc. + * + * Licensed under the Mozilla Public License version 1.1 + * with a permitted attribution clause. You may obtain a + * copy of the License at + * + * http://www.alfresco.org/legal/license.txt + * + * 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.alfresco.repo.transaction; + +/** + * NO-OP listener. + * + * @author Derek Hulley + */ +public abstract class TransactionListenerAdapter implements TransactionListener +{ + /** + * @inheritDoc + */ + public void flush() + { + } + + /** + * @inheritDoc + */ + public void beforeCommit(boolean readOnly) + { + } + + /** + * @inheritDoc + */ + public void beforeCompletion() + { + } + + /** + * @inheritDoc + */ + public void afterCommit() + { + } + + /** + * @inheritDoc + */ + public void afterRollback() + { + } +}