From 5763abbf6de765e07335598777d7813aafb1448f Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Fri, 16 May 2014 20:09:47 +0000 Subject: [PATCH] Merged HEAD-BUG-FIX (4.3/Cloud) to HEAD (4.3/Cloud) 69949: Merged V4.2-BUG-FIX (4.2.3) to HEAD-BUG-FIX (4.3/Cloud) 69867: Fix handling of non-propagating transactions embedded in write transactions that have been forced writable (MNT-11310) - Test to demonstrate the failure - Regardless of whether a RetryingTransactionHelper was going to create a new transaction (non-propagating) or not, it did a sanity check of its read-write state against the desired state. This was kicking things out for cases where the RetryingTransactionInterceptor should just have been going along with the current transaction. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@70465 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../RetryingTransactionHelper.java | 14 ++-- .../RetryingTransactionHelperTest.java | 73 +++++++++++++++++++ 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java index cd79b4eb6b..9384e06b31 100644 --- a/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java +++ b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java @@ -360,11 +360,6 @@ public class RetryingTransactionHelper */ public R doInTransaction(RetryingTransactionCallback cb, boolean readOnly, boolean requiresNew) { - if (this.readOnly && !readOnly) - { - throw new AccessDeniedException(MSG_READ_ONLY); - } - // First validate the requiresNew setting if (!requiresNew) { @@ -390,6 +385,15 @@ public class RetryingTransactionHelper throw new RuntimeException("Unknown transaction state: " + readState); } } + + // If we need a new transaction, then we have to check that the read-write request can be served + if (requiresNew) + { + if (this.readOnly && !readOnly) + { + throw new AccessDeniedException(MSG_READ_ONLY); + } + } // If we are time limiting, set ourselves a time limit and maintain the count of concurrent transactions long startTime = 0; diff --git a/source/test-java/org/alfresco/repo/transaction/RetryingTransactionHelperTest.java b/source/test-java/org/alfresco/repo/transaction/RetryingTransactionHelperTest.java index 87d841f607..096f5fc6cd 100644 --- a/source/test-java/org/alfresco/repo/transaction/RetryingTransactionHelperTest.java +++ b/source/test-java/org/alfresco/repo/transaction/RetryingTransactionHelperTest.java @@ -38,6 +38,7 @@ import org.alfresco.error.ExceptionStackUtil; import org.alfresco.model.ContentModel; import org.alfresco.repo.security.authentication.AuthenticationComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.repo.transaction.AlfrescoTransactionSupport.TxnReadState; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.service.ServiceRegistry; @@ -620,6 +621,78 @@ public class RetryingTransactionHelperTest extends TestCase } + public void testForceWritable() throws Exception + { + authenticationComponent.setCurrentUser(AuthenticationUtil.getAdminUserName()); + + final RetryingTransactionCallback doNothingCallback = new RetryingTransactionCallback() + { + @Override + public Void execute() throws Throwable + { + return null; + } + }; + + TransactionServiceImpl txnService = (TransactionServiceImpl) transactionService; + txnService.setAllowWrite(false, QName.createQName("{test}testForceWritable")); + try + { + final RetryingTransactionHelper vetoedTxnHelper = txnService.getRetryingTransactionHelper(); + // We can execute read-only + vetoedTxnHelper.doInTransaction(doNothingCallback, true, false); + // We fail on write + try + { + vetoedTxnHelper.doInTransaction(doNothingCallback, false, false); + fail("Failed to prevent read-write txn in vetoed txn helper."); + } + catch (RuntimeException e) + { + // Expected + } + + // Now call the vetoed callback in one that has been forced writable + // A failure would be one of the causes of MNT-11310. + RetryingTransactionHelper forcedTxnHelper = txnService.getRetryingTransactionHelper(); + forcedTxnHelper.setForceWritable(true); + forcedTxnHelper.doInTransaction(new RetryingTransactionCallback() + { + @Override + public Void execute() throws Throwable + { + // Participate in the outer transaction + vetoedTxnHelper.doInTransaction(doNothingCallback, false, false); + return null; + } + }, false); + + // However, if we attempt to force a new transaction, then the forcing should have no effect + try + { + forcedTxnHelper.doInTransaction(new RetryingTransactionCallback() + { + @Override + public Void execute() throws Throwable + { + // Start a new transaction + vetoedTxnHelper.doInTransaction(doNothingCallback, false, true); + return null; + } + }, false); + fail("Inner, non-propagating transactions should still fall foul of the write veto."); + } + catch (AccessDeniedException e) + { + // Expected + } + } + finally + { + txnService.setAllowWrite(true, QName.createQName("{test}testForceWritable")); + } + } + public void testStartNewTransaction() throws Exception { // MNT-10096