diff --git a/source/java/org/alfresco/repo/batch/BatchProcessor.java b/source/java/org/alfresco/repo/batch/BatchProcessor.java index 1b722225c2..9cca3fc266 100644 --- a/source/java/org/alfresco/repo/batch/BatchProcessor.java +++ b/source/java/org/alfresco/repo/batch/BatchProcessor.java @@ -33,7 +33,9 @@ import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.repo.transaction.AlfrescoTransactionSupport; import org.alfresco.repo.transaction.RetryingTransactionHelper; +import org.alfresco.repo.transaction.TransactionListenerAdapter; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.service.cmr.rule.RuleService; import org.apache.commons.logging.Log; @@ -79,6 +81,9 @@ public class BatchProcessor implements BatchMonitor /** The current entry id. */ private String currentEntryId; + + /** The number of batches currently executing. */ + private int executingCount; /** The last error. */ private Throwable lastError; @@ -420,7 +425,7 @@ public class BatchProcessor implements BatchMonitor /** * A callback that invokes a worker on a batch, optionally in a new transaction. */ - class TxnCallback implements RetryingTransactionCallback, Runnable + class TxnCallback extends TransactionListenerAdapter implements RetryingTransactionCallback, Runnable { /** @@ -463,6 +468,9 @@ public class BatchProcessor implements BatchMonitor /** The last error entry id. */ private String txnLastErrorEntryId; + + /** Has a retryable failure occurred ? */ + private boolean hadRetryFailure; /* * (non-Javadoc) @@ -471,13 +479,41 @@ public class BatchProcessor implements BatchMonitor public Object execute() throws Throwable { reset(); + if (this.batch.isEmpty()) + { + return null; + } + + // Bind this instance to the transaction + AlfrescoTransactionSupport.bindListener(this); + + synchronized (BatchProcessor.this) + { + // If we are retrying after failure, assume there are cross-dependencies and wait for other + // executing batches to complete + if (this.hadRetryFailure) + { + while (BatchProcessor.this.executingCount > 0) + { + if (BatchProcessor.this.logger.isDebugEnabled()) + { + BatchProcessor.this.logger.debug(Thread.currentThread().getName() + + " Recoverable failure: waiting for other batches to complete"); + } + BatchProcessor.this.wait(); + } + if (BatchProcessor.this.logger.isDebugEnabled()) + { + BatchProcessor.this.logger.debug(Thread.currentThread().getName() + " ready to execute"); + } + } + BatchProcessor.this.currentEntryId = this.worker.getIdentifier(this.batch.get(0)); + BatchProcessor.this.executingCount++; + } + for (T entry : this.batch) { - this.txnEntryId = this.worker.getIdentifier(entry); - synchronized (BatchProcessor.this) - { - BatchProcessor.this.currentEntryId = this.txnEntryId; - } + this.txnEntryId = this.worker.getIdentifier(entry); try { this.worker.process(entry); @@ -498,6 +534,8 @@ public class BatchProcessor implements BatchMonitor } else { + // Next time we retry, we will wait for other executing batches to complete + this.hadRetryFailure = true; throw t; } } @@ -621,6 +659,28 @@ public class BatchProcessor implements BatchMonitor reset(); } } + + @Override + public void afterCommit() + { + // Wake up any waiting batches + synchronized (BatchProcessor.this) + { + BatchProcessor.this.executingCount--; + BatchProcessor.this.notifyAll(); + } + } + + @Override + public void afterRollback() + { + // Wake up any waiting batches + synchronized (BatchProcessor.this) + { + BatchProcessor.this.executingCount--; + BatchProcessor.this.notifyAll(); + } + } } } diff --git a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java index 2581618da3..3b2a69d92d 100644 --- a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java +++ b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java @@ -38,8 +38,6 @@ import org.alfresco.repo.node.AbstractNodeServiceImpl; import org.alfresco.repo.node.StoreArchiveMap; import org.alfresco.repo.node.index.NodeIndexer; import org.alfresco.repo.security.authentication.AuthenticationUtil; -import org.alfresco.repo.transaction.AlfrescoTransactionSupport; -import org.alfresco.repo.transaction.TransactionListenerAdapter; import org.alfresco.repo.transaction.TransactionalResourceHelper; import org.alfresco.service.cmr.dictionary.AspectDefinition; import org.alfresco.service.cmr.dictionary.AssociationDefinition; @@ -336,13 +334,6 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl addMissingAspects(childNodePair, propertiesBefore, propertiesAfter); addMissingAspects(parentNodePair, assocTypeQName); - /** - * track new node ref so we can validate its path. - * - * it may be valid now, but who knows what will happen between - * now and commit! - */ - trackNewNodeRef(childAssocRef.getChildRef()); untrackDeletedNodeRef(childAssocRef.getChildRef()); // Index @@ -352,27 +343,6 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl return childAssocRef; } - /** - * Track a new node ref so we can validate its path at commit time. - * - * It may have a valid path now, but who knows what will happen between - * now and commit! - * - * @param newNodeRef the node to track - */ - private void trackNewNodeRef(NodeRef newNodeRef) - { - // bind a pre-commit listener to validate any new node associations - Set newNodes = TransactionalResourceHelper.getSet(KEY_PRE_COMMIT_ADD_NODE); - if (newNodes.size() == 0) - { - PreCommitNewNodeListener listener = new PreCommitNewNodeListener(); - AlfrescoTransactionSupport.bindListener(listener); - } - newNodes.add(newNodeRef); - } - - /** * Track a deleted node @@ -427,59 +397,6 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl } } - private class PreCommitNewNodeListener extends TransactionListenerAdapter - { - public void afterCommit() - { - // NO-OP - } - - public void afterRollback() - { - // NO-OP - - } - - public void beforeCommit(boolean readOnly) - { - if (readOnly) - { - return; - } - Set nodeRefs = TransactionalResourceHelper.getSet(KEY_PRE_COMMIT_ADD_NODE); - for (NodeRef nodeRef : nodeRefs) - { - // Need to check for exists since the node may be created - // and deleted within the same transaction - if(exists(nodeRef)) - { - try - { - // Check that the primary path is valid for this node - getPaths(nodeRef, false); - } - catch (AlfrescoRuntimeException are) - { - throw new AlfrescoRuntimeException("Error while validating path:" + are.toString(), are); - } - } - } - nodeRefs.clear(); - } - - public void beforeCompletion() - { - // NO-OP - - } - - public void flush() - { - // NO-OP - } - } - - /** * Adds all the default aspects and properties required for the given type. * Existing values will not be overridden. @@ -1042,6 +959,10 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl List> parentNodePairs = new ArrayList>(parentRefs.size()); for (NodeRef parentRef : parentRefs) { + if (isDeletedNodeRef(parentRef)) + { + throw new InvalidNodeRefException("The parent node has been deleted", parentRef); + } Pair parentNodePair = getNodePairNotNull(parentRef); Long parentNodeId = parentNodePair.getFirst(); parentNodePairs.add(parentNodePair); @@ -2231,6 +2152,11 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl QName assocTypeQName, QName assocQName) { + if (isDeletedNodeRef(newParentRef)) + { + throw new InvalidNodeRefException("The parent node has been deleted", newParentRef); + } + Pair nodeToMovePair = getNodePairNotNull(nodeToMoveRef); Pair parentNodePair = getNodePairNotNull(newParentRef); @@ -2331,7 +2257,6 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl // Check that there is not a cyclic relationship getPaths(newNodeToMoveRef, false); - trackNewNodeRef(newNodeToMoveRef); // Call behaviours if (movingStore) diff --git a/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerImpl.java b/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerImpl.java index bb8daa9229..6ccf718dbf 100644 --- a/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerImpl.java +++ b/source/java/org/alfresco/repo/search/impl/lucene/ADMLuceneIndexerImpl.java @@ -32,6 +32,7 @@ import java.util.Collection; import java.util.Date; import java.util.HashMap; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Locale; @@ -595,7 +596,7 @@ public class ADMLuceneIndexerImpl extends AbstractLuceneIndexerImpl imp Map properties = nodeService.getProperties(nodeRef); NodeRef.Status nodeStatus = nodeService.getNodeStatus(nodeRef); - Collection directPaths = nodeService.getPaths(nodeRef, false); + Collection directPaths = new LinkedHashSet(nodeService.getPaths(nodeRef, false)); Collection> categoryPaths = getCategoryPaths(nodeRef, properties); Collection> paths = new ArrayList>(directPaths.size() + categoryPaths.size()); for (Path path : directPaths) @@ -625,6 +626,8 @@ public class ADMLuceneIndexerImpl extends AbstractLuceneIndexerImpl imp } boolean isRoot = nodeRef.equals(tenantService.getName(nodeService.getRootNode(nodeRef.getStoreRef()))); + boolean mayHaveChildren = includeDirectoryDocuments && mayHaveChildren(nodeRef); + boolean isCategory = isCategory(getDictionaryService().getType(nodeService.getType(nodeRef))); StringBuilder qNameBuffer = new StringBuilder(64); StringBuilder assocTypeQNameBuffer = new StringBuilder(64); @@ -683,28 +686,25 @@ public class ADMLuceneIndexerImpl extends AbstractLuceneIndexerImpl imp // check for child associations - if (includeDirectoryDocuments) + if (mayHaveChildren) { - if (mayHaveChildren(nodeRef)) + if (directPaths.contains(pair.getFirst())) { - if (directPaths.contains(pair.getFirst())) + Document directoryEntry = new Document(); + directoryEntry.add(new Field("ID", nodeRef.toString(), Field.Store.YES, Field.Index.NO_NORMS, Field.TermVector.NO)); + directoryEntry.add(new Field("PATH", pathString, Field.Store.YES, Field.Index.TOKENIZED, Field.TermVector.NO)); + for (NodeRef parent : getParents(pair.getFirst())) { - Document directoryEntry = new Document(); - directoryEntry.add(new Field("ID", nodeRef.toString(), Field.Store.YES, Field.Index.NO_NORMS, Field.TermVector.NO)); - directoryEntry.add(new Field("PATH", pathString, Field.Store.YES, Field.Index.TOKENIZED, Field.TermVector.NO)); - for (NodeRef parent : getParents(pair.getFirst())) - { - directoryEntry.add(new Field("ANCESTOR", tenantService.getName(parent).toString(), Field.Store.NO, Field.Index.NO_NORMS, Field.TermVector.NO)); - } - directoryEntry.add(new Field("ISCONTAINER", "T", Field.Store.YES, Field.Index.NO_NORMS, Field.TermVector.NO)); - - if (isCategory(getDictionaryService().getType(nodeService.getType(nodeRef)))) - { - directoryEntry.add(new Field("ISCATEGORY", "T", Field.Store.YES, Field.Index.NO_NORMS, Field.TermVector.NO)); - } - - docs.add(directoryEntry); + directoryEntry.add(new Field("ANCESTOR", tenantService.getName(parent).toString(), Field.Store.NO, Field.Index.NO_NORMS, Field.TermVector.NO)); } + directoryEntry.add(new Field("ISCONTAINER", "T", Field.Store.YES, Field.Index.NO_NORMS, Field.TermVector.NO)); + + if (isCategory) + { + directoryEntry.add(new Field("ISCATEGORY", "T", Field.Store.YES, Field.Index.NO_NORMS, Field.TermVector.NO)); + } + + docs.add(directoryEntry); } } } diff --git a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java b/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java index af9a6eebe3..59bb492bf8 100644 --- a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java +++ b/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java @@ -25,6 +25,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -1152,10 +1153,23 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl // Filter out associations to unknown parent authorities associations.retainAll(allAuthorities); int insertIndex = authorityPath.size(); - for (String parentAuthority : associations) + Iterator i = associations.iterator(); + while (i.hasNext()) { + String parentAuthority = i.next(); + // Prevent cyclic paths - if (!authorityPath.contains(parentAuthority)) + if (authorityPath.contains(parentAuthority)) + { + if (ChainingUserRegistrySynchronizer.logger.isWarnEnabled()) + { + ChainingUserRegistrySynchronizer.logger.warn("Detected cyclic dependencies in group '" + + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(parentAuthority) + + "'"); + } + i.remove(); + } + else { authorityPath.add(parentAuthority); visitGroupAssociations(authorityPath, allAuthorities, associationsOld, associationsNew); diff --git a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java b/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java index 02be23f343..82c48a7d85 100644 --- a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java +++ b/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java @@ -29,6 +29,7 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Random; import java.util.Set; import junit.framework.TestCase; @@ -363,7 +364,7 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase public void testVolume() throws Exception { List persons = new ArrayList(new RandomPersonCollection(100)); - List groups = new ArrayList(new RandomGroupCollection(100, persons)); + List groups = new ArrayList(new RandomGroupCollection(50, persons)); this.applicationContextManager.setUserRegistries(new MockUserRegistry("Z0", persons, groups)); this.synchronizer.synchronize(true, true, true); tearDownTestUsersAndGroups(); @@ -777,6 +778,8 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase */ public class RandomGroupCollection extends AbstractCollection { + /** Use a fixed seed to give this class deterministic behaviour */ + private Random generator = new Random(1628876500L); /** The collection size. */ private final int size; @@ -839,12 +842,16 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase String[] authorityNames = new String[17]; for (int i = 0; i < authorityNames.length; i++) { + // Choose an authority at random from the list of known authorities + int index = RandomGroupCollection.this.generator.nextInt(RandomGroupCollection.this.authorities + .size()); authorityNames[i] = ChainingUserRegistrySynchronizerTest.this.authorityService - .getShortName((String) RandomGroupCollection.this.authorities - .get((int) (Math.random() * (double) (RandomGroupCollection.this.authorities - .size() - 1)))); + .getShortName((String) RandomGroupCollection.this.authorities.get(index)); } - return newGroup("G" + GUID.generate(), authorityNames); + NodeDescription group = newGroup("G" + GUID.generate(), authorityNames); + // Make this group a candidate for adding to other groups + RandomGroupCollection.this.authorities.add((String) group.getProperties().get(ContentModel.PROP_AUTHORITY_NAME)); + return group; } public void remove()