diff --git a/config/alfresco/core-services-context.xml b/config/alfresco/core-services-context.xml index bb19c2e542..77da27184e 100644 --- a/config/alfresco/core-services-context.xml +++ b/config/alfresco/core-services-context.xml @@ -299,48 +299,32 @@ - - - PROPAGATION_REQUIRED + + + false - - - - - PROPAGATION_REQUIRED - - - true - - - - - - - - - - - - 20 + + false - - + + - - + + true - - 20 + + false + + @@ -1022,6 +1006,9 @@ ${dir.indexes.backup} + + true + diff --git a/config/alfresco/dbscripts/upgrade/2.2/org.hibernate.dialect.MySQLInnoDBDialect/AlfrescoSchemaUpdate-2.2-ACL.sql b/config/alfresco/dbscripts/upgrade/2.2/org.hibernate.dialect.MySQLInnoDBDialect/AlfrescoSchemaUpdate-2.2-ACL.sql index 5063db87bf..4fea339df4 100644 --- a/config/alfresco/dbscripts/upgrade/2.2/org.hibernate.dialect.MySQLInnoDBDialect/AlfrescoSchemaUpdate-2.2-ACL.sql +++ b/config/alfresco/dbscripts/upgrade/2.2/org.hibernate.dialect.MySQLInnoDBDialect/AlfrescoSchemaUpdate-2.2-ACL.sql @@ -82,7 +82,7 @@ UPDATE alf_access_control_entry ace -- migrate data - build equivalent ACL entries INSERT INTO alf_acl_member (version, acl_id, ace_id, pos) - select 1, acl_id, id, 0 from alf_access_control_entry; + select 1, ace.acl_id, ace.id, 0 from alf_access_control_entry ace join alf_access_control_list acl on acl.id = ace.acl_id; -- Create ACE context CREATE TABLE alf_ace_context ( @@ -125,16 +125,30 @@ CREATE INDEX fk_alf_ace_ctx ON alf_access_control_entry (context_id); ALTER TABLE alf_access_control_entry ADD CONSTRAINT fk_alf_ace_ctx FOREIGN KEY (context_id) REFERENCES alf_ace_context (id); +CREATE TABLE alf_tmp_min_ace ( + min BIGINT NOT NULL, + permission_id BIGINT NOT NULL, + authority_id BIGINT NOT NULL, + allowed BIT(1) NOT NULL, + applies INT NOT NULL, + UNIQUE (permission_id, authority_id, allowed, applies) +) ENGINE=InnoDB; + +INSERT INTO alf_tmp_min_ace (min, permission_id, authority_id, allowed, applies) + SELECT min(ace1.id), ace1.permission_id, ace1.authority_id, ace1.allowed, ace1.applies FROM alf_access_control_entry ace1 group by ace1.permission_id, ace1.authority_id, ace1.allowed, ace1.applies; + -- Update members to point to the first use of an access control entry UPDATE alf_acl_member mem - SET ace_id = (SELECT min(ace2.id) FROM alf_access_control_entry ace1 - JOIN alf_access_control_entry ace2 - ON ace1.permission_id = ace2.permission_id AND - ace1.authority_id = ace2.authority_id AND - ace1.allowed = ace2.allowed AND - ace1.applies = ace2.applies - WHERE ace1.id = mem.ace_id ); + SET ace_id = (SELECT help.min FROM alf_access_control_entry ace + JOIN alf_tmp_min_ace help + ON help.permission_id = ace.permission_id AND + help.authority_id = ace.authority_id AND + help.allowed = ace.allowed AND + help.applies = ace.applies + WHERE ace.id = mem.ace_id ); + +DROP TABLE alf_tmp_min_ace; -- Remove duplicate aces the mysql way (as you can not use the deleted table in the where clause ...) diff --git a/config/alfresco/hibernate-context.xml b/config/alfresco/hibernate-context.xml index 3e953ad854..588a1bac54 100644 --- a/config/alfresco/hibernate-context.xml +++ b/config/alfresco/hibernate-context.xml @@ -356,9 +356,6 @@ - - - diff --git a/config/alfresco/messages/content-service.properties b/config/alfresco/messages/content-service.properties index 5104a7a94b..d70c90bcad 100644 --- a/config/alfresco/messages/content-service.properties +++ b/config/alfresco/messages/content-service.properties @@ -17,4 +17,6 @@ content.http_reader.err.no_authentication=The HTTP reader was unable to authenti content.http_reader.err.check_cluster=Please ensure that 'replicateUpdates' and 'replicateUpdatesViaCopy' is enabled for the cache 'org.alfresco.cache.ticketsCache'. Check that the general cluster configuration is correct and working. content.http_reader.err.unrecognized=An unrecognized error occured when attempting to download content from remote server:\n Server: {0} \n Content: {1} \n HTTP Response: {2} -metadata.extraction.err.type_conversion=Metadata extraction failed because an extracted value failed to convert to the required type: \n Extractor: {0} \n Target Property QName: {1} \n Required Type: {2} \n Extracted Value: {3} \ No newline at end of file +metadata.extraction.err.type_conversion=Metadata extraction failed because an extracted value failed to convert to the required type: \n Extractor: {0} \n Target Property QName: {1} \n Required Type: {2} \n Extracted Value: {3} + +transform.err.format_or_password=Failed to convert content, possibly due to an incorrectly formatted or password protected file. \ No newline at end of file diff --git a/config/alfresco/public-services-context.xml b/config/alfresco/public-services-context.xml index 2eb027bd9d..b286310338 100644 --- a/config/alfresco/public-services-context.xml +++ b/config/alfresco/public-services-context.xml @@ -774,6 +774,7 @@ crossRepositoryCopyServiceWriteTxnAdvisor + checkTxnAdvisor @@ -798,6 +799,7 @@ avmServiceWriteTxnAdvisor avmServiceReadTxnAdvisor + checkTxnAdvisor avmSnapShotTriggeredIndexingMethodInterceptor @@ -841,6 +843,17 @@ + + + + + + + .* + + + + @@ -863,6 +876,7 @@ getPaths getHeadPaths getPathsInStoreHead + getPathsInStoreVersion getIndirectionPath getHistory getCommonAncestor @@ -944,6 +958,7 @@ avmServiceWriteTxnAdvisor avmServiceReadTxnAdvisor + checkTxnAdvisor avmSnapShotTriggeredIndexingMethodInterceptor @@ -1043,6 +1058,7 @@ avmSyncServiceWriteTxnAdvisor avmSyncServiceReadTxnAdvisor + checkTxnAdvisor @@ -1056,6 +1072,7 @@ + getAttribute getAttributes query getKeys @@ -1095,6 +1112,7 @@ attributeServiceWriteTxnAdvisor attributeServiceReadTxnAdvisor + checkTxnAdvisor @@ -1107,18 +1125,17 @@ - @@ -1144,6 +1161,8 @@ deploymentServiceWriteTxnAdvisor + deploymentServiceReadTxnAdvisor + checkTxnAdvisor @@ -1158,7 +1177,7 @@ getLock - getUserLocks + getUsersLocks getWebProjectLocks getWebProjects getStoreLocks @@ -1179,6 +1198,7 @@ removeWebProject modifyLock removeStoreLocks + removeLocksInDirectory @@ -1196,6 +1216,7 @@ avmLockingServiceWriteTxnAdvisor avmLockingServiceReadTxnAdvisor + checkTxnAdvisor @@ -1420,6 +1441,8 @@ getHrefDifference getHrefManifestBrokenByNewOrMod getHrefsDependentUponFile + getPollInterval + isLinkValidationDisabled @@ -1432,6 +1455,9 @@ + onBootstrap + onShutdown + setLinkValidationDisabled updateHrefInfo @@ -1452,6 +1478,7 @@ linkValidationServiceWriteTxnAdvisor linkValidationServiceReadTxnAdvisor + checkTxnAdvisor diff --git a/source/java/org/alfresco/repo/avm/AVMRepository.java b/source/java/org/alfresco/repo/avm/AVMRepository.java index aaeebba3d1..69f21648ca 100644 --- a/source/java/org/alfresco/repo/avm/AVMRepository.java +++ b/source/java/org/alfresco/repo/avm/AVMRepository.java @@ -563,32 +563,30 @@ public class AVMRepository // branching from. I'd be considerably happier if we disallowed // certain scenarios, but Jon won't let me :P (bhp). - Long parentAcl = dirNode.getAcl() == null ? null : dirNode.getAcl().getId(); + Long inheritAcl = srcNode.getAcl() == null ? null : srcNode.getAcl().getId(); if (srcNode.getType() == AVMNodeType.PLAIN_DIRECTORY) { - dstNode = new PlainDirectoryNodeImpl((PlainDirectoryNode)srcNode, dstRepo, parentAcl, ACLCopyMode.COPY); + dstNode = new PlainDirectoryNodeImpl((PlainDirectoryNode)srcNode, dstRepo, inheritAcl, ACLCopyMode.INHERIT); } else if (srcNode.getType() == AVMNodeType.LAYERED_DIRECTORY) { dstNode = - new LayeredDirectoryNodeImpl((LayeredDirectoryNode)srcNode, dstRepo, sPath, false, parentAcl, ACLCopyMode.COPY); + new LayeredDirectoryNodeImpl((LayeredDirectoryNode)srcNode, dstRepo, sPath, false, inheritAcl, ACLCopyMode.INHERIT); ((LayeredDirectoryNode)dstNode).setLayerID(issueLayerID()); } else if (srcNode.getType() == AVMNodeType.LAYERED_FILE) { - dstNode = new LayeredFileNodeImpl((LayeredFileNode)srcNode, dstRepo, parentAcl, ACLCopyMode.COPY); + dstNode = new LayeredFileNodeImpl((LayeredFileNode)srcNode, dstRepo, inheritAcl, ACLCopyMode.INHERIT); } else // This is a plain file. { - dstNode = new PlainFileNodeImpl((PlainFileNode)srcNode, dstRepo, parentAcl, ACLCopyMode.COPY); + dstNode = new PlainFileNodeImpl((PlainFileNode)srcNode, dstRepo, inheritAcl, ACLCopyMode.INHERIT); } // dstNode.setVersionID(dstRepo.getNextVersionID()); dstNode.setAncestor(srcNode); dirNode.putChild(name, dstNode); dirNode.updateModTime(); - DbAccessControlList acl = srcNode.getAcl(); - dstNode.setAcl(acl != null ? acl.getCopy(parentAcl, ACLCopyMode.COPY) : null); String beginingPath = AVMNodeConverter.NormalizePath(srcPath); String finalPath = AVMNodeConverter.ExtendAVMPath(dstPath, name); finalPath = AVMNodeConverter.NormalizePath(finalPath); diff --git a/source/java/org/alfresco/repo/avm/AVMServicePermissionsTest.java b/source/java/org/alfresco/repo/avm/AVMServicePermissionsTest.java index a259f7eedb..7cfc169b21 100644 --- a/source/java/org/alfresco/repo/avm/AVMServicePermissionsTest.java +++ b/source/java/org/alfresco/repo/avm/AVMServicePermissionsTest.java @@ -528,7 +528,7 @@ public class AVMServicePermissionsTest extends TestCase List diffs = avmSyncService.compare(-1, storeName + "-layer-base:/layer-to-base", -1, storeName + ":/base", null); assertEquals(1, diffs.size()); - assertEquals("["+storeName+"-layer-base:/layer-to-base/update-dir[-1] > "+storeName+":/base/update-dir[-1]]", diffs.toString()); + assertEquals("[" + storeName + "-layer-base:/layer-to-base/update-dir[-1] > " + storeName + ":/base/update-dir[-1]]", diffs.toString()); avmSyncService.update(diffs, null, false, false, false, false, "A", "A"); desc = avmService.lookup(-1, storeName + ":/base/update-dir"); @@ -611,7 +611,7 @@ public class AVMServicePermissionsTest extends TestCase List diffs = avmSyncService.compare(-1, storeName + "-layer-base:/layer-to-base", -1, storeName + ":/base", null); assertEquals(1, diffs.size()); - assertEquals("["+storeName+"-layer-base:/layer-to-base/update-dir[-1] > "+storeName+":/base/update-dir[-1]]", diffs.toString()); + assertEquals("[" + storeName + "-layer-base:/layer-to-base/update-dir[-1] > " + storeName + ":/base/update-dir[-1]]", diffs.toString()); avmSyncService.update(diffs, null, false, false, false, false, "A", "A"); desc = avmService.lookup(-1, storeName + ":/base/update-dir"); @@ -2775,6 +2775,98 @@ public class AVMServicePermissionsTest extends TestCase } } + public void testWCMStyleTemplateAsBranch() + { + runAs("admin"); + String storeName = "PermissionsTest-" + getName() + "-" + (new Date().getTime()); + String branchName = storeName + "-Branch"; + try + { + avmService.createStore(storeName); + avmService.createDirectory(storeName + ":/", "www"); + avmService.createDirectory(storeName + ":/www", "avm-web-apps"); + avmService.createDirectory(storeName + ":/www/avm-web-apps", "ROOT"); + AVMNodeDescriptor desc = avmService.lookup(-1, storeName + ":/www"); + NodeRef nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + Map s1 = avmService.createSnapshot(storeName, null, null); + desc = avmService.lookup(-1, storeName + ":/www"); + permissionService.setPermission(nodeRef, PermissionService.ALL_AUTHORITIES, PermissionService.ALL_PERMISSIONS, true); + Map s2 = avmService.createSnapshot(storeName, null, null); + desc = avmService.lookup(-1, storeName + ":/www"); + permissionService.setPermission(nodeRef, "manager", "ContentManager", true); + Map s3 = avmService.createSnapshot(storeName, null, null); + desc = avmService.lookup(-1, storeName + ":/www"); + permissionService.setPermission(nodeRef, "publisher", "ContentPublisher", true); + Map s4 = avmService.createSnapshot(storeName, null, null); + desc = avmService.lookup(-1, storeName + ":/www"); + permissionService.setPermission(nodeRef, "contributor", "ContentContributor", true); + Map s5 = avmService.createSnapshot(storeName, null, null); + desc = avmService.lookup(-1, storeName + ":/www"); + permissionService.setPermission(nodeRef, "reviewer", "ContentReviewer", true); + Map s6 = avmService.createSnapshot(storeName, null, null); + desc = avmService.lookup(-1, storeName + ":/www"); + + desc = avmService.lookup(-1, storeName + ":/www"); + nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + assertEquals(permissionService.getSetPermissions(nodeRef).getPermissionEntries().size(), 5); + + avmService.createStore(branchName); + avmService.createBranch(-1, storeName + ":/www", branchName + ":/", "www"); + avmService.createSnapshot(branchName, null, null); + + + desc = avmService.lookup(-1, branchName + ":/www"); + nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + assertEquals(permissionService.getSetPermissions(nodeRef).getPermissionEntries().size(), 5); + desc = avmService.lookup(-1, branchName + ":/www/avm-web-apps"); + nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + assertEquals(permissionService.getSetPermissions(nodeRef).getPermissionEntries().size(), 5); + + + // Check the branch remains unchanged when the template is changed + + debugPermissions(storeName + ":/www"); + debugPermissions(branchName + ":/www"); + debugPermissions(branchName + ":/www/avm-web-apps"); + + desc = avmService.lookup(-1, storeName + ":/www"); + nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + permissionService.setPermission(nodeRef, "template", "ContentReviewer", true); + + desc = avmService.lookup(-1, branchName + ":/www"); + nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + assertEquals(permissionService.getSetPermissions(nodeRef).getPermissionEntries().size(), 5); + desc = avmService.lookup(-1, branchName + ":/www/avm-web-apps"); + nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + assertEquals(permissionService.getSetPermissions(nodeRef).getPermissionEntries().size(), 5); + + debugPermissions(storeName + ":/www"); + debugPermissions(branchName + ":/www"); + debugPermissions(branchName + ":/www/avm-web-apps"); + + desc = avmService.lookup(-1, branchName + ":/www"); + nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + permissionService.setPermission(nodeRef, "new", "ContentReviewer", true); + + desc = avmService.lookup(-1, branchName + ":/www"); + nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + assertEquals(permissionService.getSetPermissions(nodeRef).getPermissionEntries().size(), 6); + desc = avmService.lookup(-1, branchName + ":/www/avm-web-apps"); + nodeRef = AVMNodeConverter.ToNodeRef(-1, desc.getPath()); + debugPermissions(storeName + ":/www"); + debugPermissions(branchName + ":/www"); + debugPermissions(branchName + ":/www/avm-web-apps"); + assertEquals(permissionService.getSetPermissions(nodeRef).getPermissionEntries().size(), 6); + + + } + finally + { + avmService.purgeStore(storeName); + avmService.purgeStore(branchName); + } + } + /* * Test the basic permission model where */ diff --git a/source/java/org/alfresco/repo/avm/AVMServiceTest.java b/source/java/org/alfresco/repo/avm/AVMServiceTest.java index e7208eb76f..96cd586e79 100644 --- a/source/java/org/alfresco/repo/avm/AVMServiceTest.java +++ b/source/java/org/alfresco/repo/avm/AVMServiceTest.java @@ -89,6 +89,20 @@ import org.alfresco.util.Pair; */ public class AVMServiceTest extends AVMServiceTestBase { + public void testETWOTWO570() throws Exception + { + // Check that read-write methods are properly intercepted + RetryingTransactionCallback readOnlyCallback = new RetryingTransactionCallback() + { + public Object execute() throws Throwable + { + fService.createStore("StagingArea" + "-" + getName() + "-" + System.currentTimeMillis()); + return null; + } + }; + fTransactionService.getRetryingTransactionHelper().doInTransaction(readOnlyCallback, false, true); + } + public void test_WCM_949() throws Exception { try diff --git a/source/java/org/alfresco/repo/avm/AVMServiceTestBase.java b/source/java/org/alfresco/repo/avm/AVMServiceTestBase.java index 4957fd41d5..8aa8837636 100644 --- a/source/java/org/alfresco/repo/avm/AVMServiceTestBase.java +++ b/source/java/org/alfresco/repo/avm/AVMServiceTestBase.java @@ -45,6 +45,8 @@ import org.alfresco.service.cmr.search.ResultSetRow; import org.alfresco.service.cmr.search.SearchService; import org.alfresco.service.cmr.security.AuthenticationService; import org.alfresco.service.transaction.TransactionService; +import org.alfresco.util.ApplicationContextHelper; +import org.springframework.context.ApplicationContext; import org.springframework.context.support.FileSystemXmlApplicationContext; /** @@ -70,7 +72,7 @@ public class AVMServiceTestBase extends TestCase /** * The application context. */ - protected static FileSystemXmlApplicationContext fContext; + protected static ApplicationContext fContext; /** * The start time of actual work for a test. @@ -95,7 +97,7 @@ public class AVMServiceTestBase extends TestCase { if (fContext == null) { - fContext = new FileSystemXmlApplicationContext("config/alfresco/application-context.xml"); + fContext = ApplicationContextHelper.getApplicationContext(); fService = (AVMService)fContext.getBean("AVMService"); fReaper = (OrphanReaper)fContext.getBean("orphanReaper"); fSyncService = (AVMSyncService)fContext.getBean("AVMSyncService"); diff --git a/source/java/org/alfresco/repo/content/transform/PoiHssfContentTransformer.java b/source/java/org/alfresco/repo/content/transform/PoiHssfContentTransformer.java index 77e818e6b5..532a8a3488 100644 --- a/source/java/org/alfresco/repo/content/transform/PoiHssfContentTransformer.java +++ b/source/java/org/alfresco/repo/content/transform/PoiHssfContentTransformer.java @@ -31,6 +31,9 @@ import org.alfresco.repo.content.MimetypeMap; import org.alfresco.service.cmr.repository.ContentReader; import org.alfresco.service.cmr.repository.ContentWriter; import org.alfresco.service.cmr.repository.TransformationOptions; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.poi.hssf.record.RecordFormatException; import org.apache.poi.hssf.usermodel.HSSFCell; import org.apache.poi.hssf.usermodel.HSSFRow; import org.apache.poi.hssf.usermodel.HSSFSheet; @@ -53,10 +56,16 @@ import org.apache.poi.hssf.usermodel.HSSFWorkbook; */ public class PoiHssfContentTransformer extends AbstractContentTransformer2 { + /** + * Error message to delegate to NodeInfoBean + */ + public static final String WRONG_FORMAT_MESSAGE_ID = "transform.err.format_or_password"; + /** * Windows carriage return line feed pair. */ private static final String LINE_BREAK = "\r\n"; + private static Log logger = LogFactory.getLog(PoiHssfContentTransformer.class); /** * Currently the only transformation performed is that of text extraction from XLS documents. @@ -100,6 +109,14 @@ public class PoiHssfContentTransformer extends AbstractContentTransformer2 PoiHssfContentTransformer.writeString(os, encoding, LINE_BREAK, false); } } + catch (RecordFormatException ex) + { + // Catching specific exception to propagate it to NodeInfoBean + // to fix issue https://issues.alfresco.com/jira/browse/ETWOTWO-440 + + logger.error(ex); + throw new TransformerInfoException(WRONG_FORMAT_MESSAGE_ID, ex); + } finally { if (is != null) diff --git a/source/java/org/alfresco/repo/content/transform/TransformerInfoException.java b/source/java/org/alfresco/repo/content/transform/TransformerInfoException.java new file mode 100644 index 0000000000..f545a8bdc1 --- /dev/null +++ b/source/java/org/alfresco/repo/content/transform/TransformerInfoException.java @@ -0,0 +1,30 @@ +package org.alfresco.repo.content.transform; + +import org.alfresco.error.AlfrescoRuntimeException; + +/** + * + * Wraps an exception that could be thrown in any transformer to + * propagate it up to NodeInfoBean.sendNodeInfo method. + * NodeInfoBean can handle this exception to display it in NodeInfo frame + * to avoid error message box with "Exception in Transaction" message. + * + * See {@link org.alfresco.repo.content.transform.PoiHssfContentTransformer} for pattern. + * + * @author Arseny Kovalchuk + * + */ +public class TransformerInfoException extends AlfrescoRuntimeException +{ + private static final long serialVersionUID = -4343331677825559617L; + + public TransformerInfoException(String msg) + { + super(msg); + } + + public TransformerInfoException(String msg, Throwable err) + { + super(msg, err); + } +} diff --git a/source/java/org/alfresco/repo/domain/QNameDAOTest.java b/source/java/org/alfresco/repo/domain/QNameDAOTest.java index a7e40d0e7c..275b6c305e 100644 --- a/source/java/org/alfresco/repo/domain/QNameDAOTest.java +++ b/source/java/org/alfresco/repo/domain/QNameDAOTest.java @@ -200,10 +200,9 @@ public class QNameDAOTest extends TestCase // Let the threads go startLatch.countDown(); // Wait for them all to be done (within limit of 10 seconds per thread) - doneLatch.await(threadCount, TimeUnit.SECONDS); - if (doneLatch.getCount() > 0) + if (doneLatch.await(threadCount * 10, TimeUnit.SECONDS)) { - fail("Still waiting for threads to finish"); + logger.warn("Still waiting for threads to finish after " + threadCount + " seconds."); } // Check if there are errors if (errors.size() > 0) diff --git a/source/java/org/alfresco/repo/domain/hibernate/DirtySessionAnnotation.java b/source/java/org/alfresco/repo/domain/hibernate/DirtySessionAnnotation.java index b53bbcd2b5..0189583c24 100644 --- a/source/java/org/alfresco/repo/domain/hibernate/DirtySessionAnnotation.java +++ b/source/java/org/alfresco/repo/domain/hibernate/DirtySessionAnnotation.java @@ -39,18 +39,6 @@ import java.lang.annotation.Target; @Retention(RetentionPolicy.RUNTIME) public @interface DirtySessionAnnotation { - /** - * Method must flush before execution.
- * Default: false - */ - boolean flushBefore() default false; - - /** - * Method must flush after execution.
- * Default: false - */ - boolean flushAfter() default false; - /** * The session must be flagged as dirty after execution.
* Default: false diff --git a/source/java/org/alfresco/repo/domain/hibernate/DirtySessionMethodInterceptor.java b/source/java/org/alfresco/repo/domain/hibernate/DirtySessionMethodInterceptor.java index 51fc7d01c9..aecb4c0036 100644 --- a/source/java/org/alfresco/repo/domain/hibernate/DirtySessionMethodInterceptor.java +++ b/source/java/org/alfresco/repo/domain/hibernate/DirtySessionMethodInterceptor.java @@ -25,11 +25,14 @@ package org.alfresco.repo.domain.hibernate; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.Stack; +import org.alfresco.error.StackTraceUtil; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; import org.alfresco.util.Pair; import org.aopalliance.intercept.MethodInterceptor; @@ -39,7 +42,6 @@ import org.apache.commons.logging.LogFactory; import org.hibernate.FlushMode; import org.hibernate.Query; import org.hibernate.Session; -import org.springframework.orm.hibernate3.support.HibernateDaoSupport; /** * This method interceptor determines if a Hibernate flush is required and performs the @@ -60,6 +62,13 @@ import org.springframework.orm.hibernate3.support.HibernateDaoSupport; *

* It is also possible to {@link #flushSession(Session) flush the session} manually. Using this method * allows the dirty count to be updated properly, thus avoiding unecessary flushing. + *

+ * To trace failures caused by data flushes, it is necessary to track methods called and values passed + * that lead to the session being marked as dirty. If the flush fails or if a method call fails then + * the stacks and method values will be dumped. Turn on trace debugging for this:
+ * 

+ *    log4j.logger.org.alfresco.repo.domain.hibernate.DirtySessionMethodInterceptor.trace=DEBUG
+ *
* * @see #setQueryFlushMode(Session, Query) * @see #flushSession(Session) @@ -67,11 +76,23 @@ import org.springframework.orm.hibernate3.support.HibernateDaoSupport; * @author Derek Hulley * @since 2.1.5 */ -public class DirtySessionMethodInterceptor extends HibernateDaoSupport implements MethodInterceptor +public class DirtySessionMethodInterceptor implements MethodInterceptor { private static final String KEY_FLUSH_DATA = "FlushIfRequiredMethodInterceptor.FlushData"; private static Log logger = LogFactory.getLog(DirtySessionMethodInterceptor.class); + private static final boolean loggerDebugEnabled; + private static Log traceLogger = LogFactory.getLog(DirtySessionMethodInterceptor.class.getName() + ".trace"); + private static final boolean traceLoggerDebugEnabled; + static + { + loggerDebugEnabled = logger.isDebugEnabled(); + traceLoggerDebugEnabled = traceLogger.isDebugEnabled(); + if (traceLoggerDebugEnabled) + { + traceLogger.warn("Trace logging is enabled and will affect performance"); + } + } /** * Keep track of methods that have been warned about, i.e. methods that are not annotated. @@ -91,6 +112,7 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement { private int dirtyCount; private Stack> methodStack; + private List traceStacks; private FlushData() { dirtyCount = 0; @@ -117,6 +139,7 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement public void resetDirtyCount() { dirtyCount = 0; + traceStacks = null; } public void pushMethod(String methodName, boolean isAnnotated) { @@ -147,6 +170,39 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement // All were annotated return true; } + public void addTraceStack(MethodInvocation method) + { + Exception e = new Exception(); + e.fillInStackTrace(); + StringBuilder sb = new StringBuilder(2048); + sb.append(" Method: ").append(method.getMethod().getName()); + Object[] arguments = method.getArguments(); + for (int i = 0; i < arguments.length; i++) + { + String argumentStr; + try + { + argumentStr = arguments[i] == null ? "NULL" : arguments[i].toString(); + } + catch (Throwable ee) + { + argumentStr = "(10); + } + traceStacks.add(sb.toString()); + } + public List getTraceStacks() + { + return (traceStacks == null) ? Collections.emptyList() : traceStacks; + } } /** @@ -177,7 +233,7 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement // play with the session if (!flushData.isStackAnnotated()) { - if (logger.isDebugEnabled()) + if (loggerDebugEnabled) { logger.debug( "Method stack is not annotated. Not setting query flush mode: \n" + @@ -187,7 +243,7 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement } // The stack is fully annotated, so flush if required and set the flush mode on the query - if (logger.isDebugEnabled()) + if (loggerDebugEnabled) { logger.debug( "Setting query flush mode: \n" + @@ -230,7 +286,7 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement FlushData flushData = DirtySessionMethodInterceptor.getFlushData(); if (force) { - if (logger.isDebugEnabled()) + if (loggerDebugEnabled) { logger.debug( "Flushing session forcefully: \n" + @@ -243,7 +299,7 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement { if (flushData.isDirty()) { - if (logger.isDebugEnabled()) + if (loggerDebugEnabled) { logger.debug( "Flushing dirty session: \n" + @@ -254,7 +310,7 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement } else { - if (logger.isDebugEnabled()) + if (loggerDebugEnabled) { logger.debug( "Session is not dirty - no flush: \n" + @@ -276,13 +332,9 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement // Get the flush and dirty mark requirements for the call DirtySessionAnnotation annotation = method.getAnnotation(DirtySessionAnnotation.class); - boolean flushBefore = false; - boolean flushAfter = false; boolean markDirty = false; if (annotation != null) { - flushBefore = annotation.flushBefore(); - flushAfter = annotation.flushAfter(); markDirty = annotation.markDirty(); } else if (unannotatedMethodNames.add(methodName)) @@ -292,17 +344,12 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement FlushData flushData = DirtySessionMethodInterceptor.getFlushData(); - Session session = null; - if (flushBefore || flushAfter) + // If we are to mark it dirty and we are tracing, then record the stacks + if (markDirty && traceLoggerDebugEnabled) { - session = getSession(false); + flushData.addTraceStack(invocation); } - if (flushBefore) - { - DirtySessionMethodInterceptor.flushSession(session); - } - boolean isAnnotated = (annotation != null); Object ret = null; try @@ -310,23 +357,36 @@ public class DirtySessionMethodInterceptor extends HibernateDaoSupport implement // Push the method onto the stack flushData.pushMethod(methodName, isAnnotated); - if (logger.isDebugEnabled()) + if (loggerDebugEnabled) { logger.debug( "Flush state and parameters for DirtySessionInterceptor: \n" + " Method: " + methodName + "\n" + - " Annotated: BEFORE=" + flushBefore + ", AFTER=" + flushAfter + ", MARK-DIRTY=" + markDirty + "\n" + + " Annotated: MARK-DIRTY=" + markDirty + "\n" + " Session State: " + flushData); } // Do the call - ret = invocation.proceed(); - - if (flushAfter) + try { - DirtySessionMethodInterceptor.flushSession(session); + ret = invocation.proceed(); } - else if (markDirty) + catch (Throwable e) + { + // If we are tracing, then dump the current dirty stack + if (traceLoggerDebugEnabled) + { + traceLogger.debug("Dumping stack traces after exception: " + e.getMessage()); + for (String stackTrace : flushData.getTraceStacks()) + { + traceLogger.debug("\n" + stackTrace); + } + } + // Rethrow + throw e; + } + + if (markDirty) { flushData.incrementDirtyCount(); } diff --git a/source/java/org/alfresco/repo/domain/hibernate/HibernateQNameDAOImpl.java b/source/java/org/alfresco/repo/domain/hibernate/HibernateQNameDAOImpl.java index 9a24c48965..b183fe9bd6 100644 --- a/source/java/org/alfresco/repo/domain/hibernate/HibernateQNameDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/hibernate/HibernateQNameDAOImpl.java @@ -181,7 +181,8 @@ public class HibernateQNameDAOImpl extends HibernateDaoSupport implements QNameD else { // Found in the cache. Load using the ID. - result = getQNameEntity(id); + // Don't use the method that throws an exception as the cache might be invalid. + result = (QNameEntity) getSession().get(QNameEntityImpl.class, id); if (result == null) { // It is not available, so we need to go the query route. diff --git a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java index 81227b6f05..d6a5498028 100644 --- a/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java +++ b/source/java/org/alfresco/repo/node/BaseNodeServiceTest.java @@ -1176,6 +1176,43 @@ public abstract class BaseNodeServiceTest extends BaseSpringTest } } + public void testRemoveProperty() throws Exception + { + Map properties = nodeService.getProperties(rootNodeRef); + // Add an aspect with a default value + nodeService.addAspect(rootNodeRef, ASPECT_QNAME_WITH_DEFAULT_VALUE, null); + // Get the default value + Serializable defaultValue = nodeService.getProperty(rootNodeRef, PROP_QNAME_PROP2); + assertNotNull("No default property value assigned", defaultValue); + // Now apply the original node properties which didn't contain the value + nodeService.setProperties(rootNodeRef, properties); + // Ensure that it is now null + Serializable nullValue = nodeService.getProperty(rootNodeRef, PROP_QNAME_PROP2); + assertNull("Property was not removed", nullValue); + + // Remove the property by removing the aspect + nodeService.removeAspect(rootNodeRef, ASPECT_QNAME_WITH_DEFAULT_VALUE); + nullValue = nodeService.getProperty(rootNodeRef, PROP_QNAME_PROP2); + assertNull("Property was not removed", nullValue); + + // Do the same, but explicitly set the value to null + nodeService.addAspect(rootNodeRef, ASPECT_QNAME_WITH_DEFAULT_VALUE, null); + defaultValue = nodeService.getProperty(rootNodeRef, PROP_QNAME_PROP2); + assertNotNull("No default property value assigned", defaultValue); + nodeService.setProperty(rootNodeRef, PROP_QNAME_PROP2, null); + nullValue = nodeService.getProperty(rootNodeRef, PROP_QNAME_PROP2); + assertNull("Property was not removed", nullValue); + + // Now remove the property directly + nodeService.removeAspect(rootNodeRef, ASPECT_QNAME_WITH_DEFAULT_VALUE); + nodeService.addAspect(rootNodeRef, ASPECT_QNAME_WITH_DEFAULT_VALUE, null); + defaultValue = nodeService.getProperty(rootNodeRef, PROP_QNAME_PROP2); + assertNotNull("No default property value assigned", defaultValue); + nodeService.removeProperty(rootNodeRef, PROP_QNAME_PROP2); + nullValue = nodeService.getProperty(rootNodeRef, PROP_QNAME_PROP2); + assertNull("Property was not removed", nullValue); + } + /** * Makes a read-only transaction and then looks for a property using a non-existent QName. * The QName persistence must not lazily create QNameEntity instances for queries. diff --git a/source/java/org/alfresco/repo/node/BaseNodeServiceTest_model.xml b/source/java/org/alfresco/repo/node/BaseNodeServiceTest_model.xml index 210fd831fc..a5a6c057fe 100644 --- a/source/java/org/alfresco/repo/node/BaseNodeServiceTest_model.xml +++ b/source/java/org/alfresco/repo/node/BaseNodeServiceTest_model.xml @@ -295,7 +295,7 @@ d:text true - + false false true diff --git a/source/java/org/alfresco/repo/node/index/IndexTransactionTrackerTest.java b/source/java/org/alfresco/repo/node/index/IndexTransactionTrackerTest.java index 5126566412..fca9aad041 100644 --- a/source/java/org/alfresco/repo/node/index/IndexTransactionTrackerTest.java +++ b/source/java/org/alfresco/repo/node/index/IndexTransactionTrackerTest.java @@ -66,8 +66,8 @@ public class IndexTransactionTrackerTest extends TestCase public void setUp() throws Exception { ServiceRegistry serviceRegistry = (ServiceRegistry) ctx.getBean(ServiceRegistry.SERVICE_REGISTRY); - searchService = serviceRegistry.getSearchService(); - nodeService = serviceRegistry.getNodeService(); + searchService = (SearchService) ctx.getBean("searchService"); + nodeService = (NodeService) ctx.getBean("nodeService"); threadPoolExecutor = (ThreadPoolExecutor) ctx.getBean("indexTrackerThreadPoolExecutor"); fileFolderService = serviceRegistry.getFileFolderService(); authenticationComponent = (AuthenticationComponent) ctx.getBean("authenticationComponent"); diff --git a/source/java/org/alfresco/repo/search/impl/lucene/AbstractLuceneIndexerAndSearcherFactory.java b/source/java/org/alfresco/repo/search/impl/lucene/AbstractLuceneIndexerAndSearcherFactory.java index d081bb1ae9..28d600a7aa 100644 --- a/source/java/org/alfresco/repo/search/impl/lucene/AbstractLuceneIndexerAndSearcherFactory.java +++ b/source/java/org/alfresco/repo/search/impl/lucene/AbstractLuceneIndexerAndSearcherFactory.java @@ -32,6 +32,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -45,6 +46,7 @@ import javax.transaction.xa.XAResource; import javax.transaction.xa.Xid; import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.repo.avm.AVMNodeService; import org.alfresco.repo.search.IndexerException; import org.alfresco.repo.search.MLAnalysisMode; import org.alfresco.repo.search.QueryRegisterComponent; @@ -67,6 +69,7 @@ import org.quartz.Job; import org.quartz.JobDataMap; import org.quartz.JobExecutionContext; import org.quartz.JobExecutionException; +import org.springframework.beans.factory.InitializingBean; /** * This class is resource manager LuceneIndexers and LuceneSearchers. It supports two phase commit inside XA @@ -932,7 +935,7 @@ public abstract class AbstractLuceneIndexerAndSearcherFactory implements LuceneI * * @author Derek Hulley */ - public static class LuceneIndexBackupComponent + public static class LuceneIndexBackupComponent implements InitializingBean { private static String BACKUP_TEMP_NAME = ".indexbackup_temp"; @@ -946,6 +949,8 @@ public abstract class AbstractLuceneIndexerAndSearcherFactory implements LuceneI private String targetLocation; + private boolean checkConfiguration = true; + /** * Default constructor */ @@ -953,6 +958,16 @@ public abstract class AbstractLuceneIndexerAndSearcherFactory implements LuceneI { } + /** + * If false do not check the index configuration. + * + * @param checkConfiguration + */ + public void setCheckConfiguration(boolean checkConfiguration) + { + this.checkConfiguration = checkConfiguration; + } + /** * Provides transactions in which to perform the work * @@ -1294,6 +1309,86 @@ public abstract class AbstractLuceneIndexerAndSearcherFactory implements LuceneI } } + + public void afterPropertiesSet() throws Exception + { + RetryingTransactionCallback backupWork = new RetryingTransactionCallback() + { + public Object execute() throws Exception + { + File targetDir = new File(targetLocation).getCanonicalFile(); + + List stores; + try + { + stores = nodeService.getStores(); + } + catch (Exception e) + { + return null; + } + Set protocols = new HashSet(); + protocols.add(StoreRef.PROTOCOL_AVM); + protocols.add(StoreRef.PROTOCOL_ARCHIVE); + protocols.add(StoreRef.PROTOCOL_WORKSPACE); + protocols.add("locks"); + for (StoreRef store : stores) + { + protocols.add(store.getProtocol()); + } + + for (LuceneIndexerAndSearcher factory : factories) + { + File indexRootDir = new File(factory.getIndexRootLocation()).getCanonicalFile(); + + if (indexRootDir.getCanonicalPath().startsWith(targetDir.getCanonicalPath())) + { + throw new IllegalArgumentException("Backup directory can not contain or be an index directory"); + } + if (targetDir.getCanonicalPath().startsWith(indexRootDir.getCanonicalPath())) + { + for (String name : protocols) + { + File test = new File(indexRootDir, name); + if (targetDir.getCanonicalPath().startsWith(test.getCanonicalPath())) + { + throw new IllegalArgumentException("Backup directory can not be in index directory and match a store protocol name " + targetDir); + } + } + } + // if the back up directory exists make sure it only contains directories that are store + // protocols + + if (targetDir.exists()) + { + for (File file : targetDir.listFiles()) + { + if (file.isFile()) + { + throw new IllegalArgumentException("Existing index backup does not look like the expected structure. It constains a file " + + file.getCanonicalPath()); + } + if (!protocols.contains(file.getName())) + { + throw new IllegalArgumentException( + "Existing index backup does not look like the expected structure. It constains a directory with a name that does not match a store protocol " + + file.getCanonicalPath()); + + } + } + } + + } + return null; + } + }; + + if (checkConfiguration) + { + transactionService.getRetryingTransactionHelper().doInTransaction(backupWork); + } + + } } /** diff --git a/source/java/org/alfresco/repo/transaction/CheckTransactionAdvice.java b/source/java/org/alfresco/repo/transaction/CheckTransactionAdvice.java new file mode 100644 index 0000000000..327f4bf8b0 --- /dev/null +++ b/source/java/org/alfresco/repo/transaction/CheckTransactionAdvice.java @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2005-2008 Alfresco Software Limited. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + * As a special exception to the terms and conditions of version 2.0 of + * the GPL, you may redistribute this Program in connection with Free/Libre + * and Open Source Software ("FLOSS") applications as described in Alfresco's + * FLOSS exception. You should have recieved a copy of the text describing + * the FLOSS exception, and it is also available here: + * http://www.alfresco.com/legal/licensing" + */ +package org.alfresco.repo.transaction; + +import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.repo.transaction.AlfrescoTransactionSupport.TxnReadState; +import org.aopalliance.intercept.MethodInterceptor; +import org.aopalliance.intercept.MethodInvocation; + +/** + * A wrapper that checks that a transaction is present. + * + * @author Derek Hulley + * @since 2.2.1 + */ +public class CheckTransactionAdvice implements MethodInterceptor +{ + public CheckTransactionAdvice() + { + } + public Object invoke(final MethodInvocation methodInvocation) throws Throwable + { + // Just check for any transaction + if (AlfrescoTransactionSupport.getTransactionReadState() == TxnReadState.TXN_NONE) + { + String methodName = methodInvocation.getMethod().getName(); + String className = methodInvocation.getMethod().getDeclaringClass().getName(); + throw new AlfrescoRuntimeException("A transaction has not be started for method " + methodName + " on " + className); + } + return methodInvocation.proceed(); + } +} diff --git a/source/java/org/alfresco/repo/transaction/RetryingTransactionAdvice.java b/source/java/org/alfresco/repo/transaction/RetryingTransactionAdvice.java index 2e68be05a8..312174b163 100644 --- a/source/java/org/alfresco/repo/transaction/RetryingTransactionAdvice.java +++ b/source/java/org/alfresco/repo/transaction/RetryingTransactionAdvice.java @@ -1,160 +1,75 @@ -/** +/* + * Copyright (C) 2005-2008 Alfresco Software Limited. * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + * As a special exception to the terms and conditions of version 2.0 of + * the GPL, you may redistribute this Program in connection with Free/Libre + * and Open Source Software ("FLOSS") applications as described in Alfresco's + * FLOSS exception. You should have recieved a copy of the text describing + * the FLOSS exception, and it is also available here: + * http://www.alfresco.com/legal/licensing" */ package org.alfresco.repo.transaction; -import java.util.Random; - +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.hibernate.StaleObjectStateException; -import org.hibernate.StaleStateException; -import org.hibernate.exception.ConstraintViolationException; -import org.hibernate.exception.LockAcquisitionException; -import org.springframework.aop.framework.ReflectiveMethodInvocation; -import org.springframework.dao.ConcurrencyFailureException; -import org.springframework.dao.DeadlockLoserDataAccessException; -import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.TransactionDefinition; -import org.springframework.transaction.TransactionStatus; /** + * A this advice wrapper around the {@link RetryingTransactionHelper}. * - * @author britt + * @author Derek Hulley */ public class RetryingTransactionAdvice implements MethodInterceptor { - private static Log fgLogger = LogFactory.getLog(RetryingTransactionAdvice.class); - - /** - * The transaction manager instance. - */ - private PlatformTransactionManager fTxnManager; - - /** - * The TransactionDefinition. - */ - private TransactionDefinition fDefinition; - - /** - * The maximum number of retries. - */ - private int fMaxRetries; - - /** - * A Random number generator for getting retry intervals. - */ - private Random fRandom; + private RetryingTransactionHelper txnHelper; + private boolean readOnly; + private boolean requiresNew; public RetryingTransactionAdvice() { - fRandom = new Random(System.currentTimeMillis()); + readOnly = false; + requiresNew = false; } - /** - * Setter. - */ - public void setTransactionManager(PlatformTransactionManager manager) + public void setTxnHelper(RetryingTransactionHelper txnHelper) { - fTxnManager = manager; + this.txnHelper = txnHelper; } - /** - * Setter. - */ - public void setTransactionDefinition(TransactionDefinition def) + public void setReadOnly(boolean readOnly) { - fDefinition = def; + this.readOnly = readOnly; } - /** - * Setter. - */ - public void setMaxRetries(int maxRetries) + public void setRequiresNew(boolean requiresNew) { - fMaxRetries = maxRetries; + this.requiresNew = requiresNew; } - /* (non-Javadoc) - * @see org.aopalliance.intercept.MethodInterceptor#invoke(org.aopalliance.intercept.MethodInvocation) - */ - public Object invoke(MethodInvocation methodInvocation) throws Throwable + public Object invoke(final MethodInvocation methodInvocation) throws Throwable { - RuntimeException lastException = null; - for (int count = 0; fMaxRetries < -1 || count < fMaxRetries; count++) + // Just call the helper + RetryingTransactionCallback txnCallback = new RetryingTransactionCallback() { - TransactionStatus txn = null; - boolean isNewTxn = false; - try + public Object execute() throws Throwable { - txn = fTxnManager.getTransaction(fDefinition); - isNewTxn = txn.isNewTransaction(); - MethodInvocation clone = ((ReflectiveMethodInvocation)methodInvocation).invocableClone(); - Object result = clone.proceed(); - if (isNewTxn) - { - fTxnManager.commit(txn); - } - if (fgLogger.isDebugEnabled()) - { - if (count != 0) - { - fgLogger.debug("Transaction succeeded after " + count + " retries."); - } - } - return result; + return methodInvocation.proceed(); } - catch (RuntimeException e) - { - if (txn != null && isNewTxn && !txn.isCompleted()) - { - fTxnManager.rollback(txn); - } - if (!isNewTxn) - { - throw e; - } - lastException = e; - Throwable t = e; - boolean shouldRetry = false; - while (t != null) - { - if (t instanceof ConcurrencyFailureException || - t instanceof DeadlockLoserDataAccessException || - t instanceof StaleObjectStateException || - t instanceof LockAcquisitionException || - t instanceof StaleStateException || - t instanceof ConstraintViolationException) - { - shouldRetry = true; - try - { - Thread.sleep(fRandom.nextInt(500 * count + 500)); - } - catch (InterruptedException ie) - { - // Do nothing. - } - break; - } - // Apparently java.lang.Throwable default the cause as 'this'. - if (t == t.getCause()) - { - break; - } - t = t.getCause(); - } - if (shouldRetry) - { - fgLogger.warn("Retry #" + (count + 1)); - continue; - } - throw e; - } - } - fgLogger.error("Txn Failed after " + fMaxRetries + " retries:", lastException); - throw lastException; + }; + return txnHelper.doInTransaction(txnCallback, readOnly, requiresNew); } } diff --git a/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java index a61e667157..825d14e7c9 100644 --- a/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java +++ b/source/java/org/alfresco/repo/transaction/RetryingTransactionHelper.java @@ -37,6 +37,7 @@ import net.sf.ehcache.distribution.RemoteCacheException; import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.error.ExceptionStackUtil; import org.alfresco.repo.security.permissions.AccessDeniedException; +import org.alfresco.repo.transaction.AlfrescoTransactionSupport.TxnReadState; import org.alfresco.service.transaction.TransactionService; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -255,7 +256,6 @@ public class RetryingTransactionHelper for (int count = 0; maxRetries < 0 || count < maxRetries; ++count) { UserTransaction txn = null; - boolean isNew = false; try { if (requiresNew) @@ -264,20 +264,36 @@ public class RetryingTransactionHelper } else { - txn = txnService.getUserTransaction(readOnly); + TxnReadState readState = AlfrescoTransactionSupport.getTransactionReadState(); + switch (readState) + { + case TXN_READ_ONLY: + if (!readOnly) + { + // The current transaction is read-only, but a writable transaction is requested + throw new AlfrescoRuntimeException("Read-Write transaction started within read-only transaction"); + } + // We are in a read-only transaction and this is what we require so continue with it. + break; + case TXN_READ_WRITE: + // We are in a read-write transaction. It cannot be downgraded so just continue with it. + break; + case TXN_NONE: + // There is no current transaction so we need a new one. + txn = txnService.getUserTransaction(readOnly); + break; + default: + throw new RuntimeException("Unknown transaction state: " + readState); + } } - // Only start a transaction if required. This check isn't necessary as the transactional - // behaviour ensures that the appropriate propogation is performed. It is a useful and - // simple optimization. - isNew = requiresNew || txn.getStatus() == Status.STATUS_NO_TRANSACTION; - if (isNew) + if (txn != null) { txn.begin(); } // Do the work. R result = cb.execute(); // Only commit if we 'own' the transaction. - if (isNew) + if (txn != null) { if (txn.getStatus() == Status.STATUS_MARKED_ROLLBACK) { @@ -308,7 +324,7 @@ public class RetryingTransactionHelper catch (Throwable e) { // Somebody else 'owns' the transaction, so just rethrow. - if (!isNew) + if (txn == null) { if (e instanceof RuntimeException) { diff --git a/source/java/org/alfresco/repo/transaction/TransactionServiceImplTest.java b/source/java/org/alfresco/repo/transaction/TransactionServiceImplTest.java index d93ef0c809..e9592e9c93 100644 --- a/source/java/org/alfresco/repo/transaction/TransactionServiceImplTest.java +++ b/source/java/org/alfresco/repo/transaction/TransactionServiceImplTest.java @@ -153,6 +153,14 @@ public class TransactionServiceImplTest extends TestCase int i = 0; // expected } + finally + { + try + { + txn.rollback(); + } + catch (Throwable e) {} + } } public void testGetRetryingTransactionHelper()