diff --git a/config/alfresco/dao/dao-context.xml b/config/alfresco/dao/dao-context.xml index 9612d5ec20..fd587769be 100644 --- a/config/alfresco/dao/dao-context.xml +++ b/config/alfresco/dao/dao-context.xml @@ -104,7 +104,6 @@ - diff --git a/config/alfresco/node-services-context.xml b/config/alfresco/node-services-context.xml index c311d4d8e6..cfd254c95a 100644 --- a/config/alfresco/node-services-context.xml +++ b/config/alfresco/node-services-context.xml @@ -227,6 +227,7 @@ ${version.store.version2Store} + diff --git a/source/java/org/alfresco/repo/domain/AuditableProperties.java b/source/java/org/alfresco/repo/domain/AuditableProperties.java deleted file mode 100644 index e462dd9715..0000000000 --- a/source/java/org/alfresco/repo/domain/AuditableProperties.java +++ /dev/null @@ -1,353 +0,0 @@ -/* - * Copyright (C) 2005-2010 Alfresco Software Limited. - * - * This file is part of Alfresco - * - * Alfresco is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Alfresco 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 Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - */ -package org.alfresco.repo.domain; - -import java.io.Serializable; -import java.util.Date; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; - -import org.alfresco.model.ContentModel; -import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter; -import org.alfresco.service.namespace.QName; - -/** - * Class holding properties associated with the cm:auditable aspect. - * This aspect is common enough to warrant direct inclusion on the Node entity. - * - * @author Derek Hulley - * @since 2.2 SP2 - */ -public class AuditableProperties -{ - private static Set auditablePropertyQNames; - static - { - auditablePropertyQNames = new HashSet(13); - auditablePropertyQNames.add(ContentModel.PROP_CREATOR); - auditablePropertyQNames.add(ContentModel.PROP_CREATED); - auditablePropertyQNames.add(ContentModel.PROP_MODIFIER); - auditablePropertyQNames.add(ContentModel.PROP_MODIFIED); - auditablePropertyQNames.add(ContentModel.PROP_ACCESSED); - } - - /** - * @return Returns true if the property belongs to the cm:auditable aspect - */ - public static boolean isAuditableProperty(QName qname) - { - return auditablePropertyQNames.contains(qname); - } - - private String auditCreator; - private String auditCreated; - private String auditModifier; - private String auditModified; - private String auditAccessed; - - /** - * Default constructor with all null values. - */ - public AuditableProperties() - { - } - - /** - * @param qname the property name - * @return Returns the value of the cm:auditable property or null - */ - public Serializable getAuditableProperty(QName qname) - { - if (qname.equals(ContentModel.PROP_CREATOR)) - { - return auditCreator; - } - else if (qname.equals(ContentModel.PROP_CREATED)) - { - return DefaultTypeConverter.INSTANCE.convert(Date.class, auditCreated); - } - else if (qname.equals(ContentModel.PROP_MODIFIER)) - { - return auditModifier == null ? auditCreator : auditModifier; - } - else if (qname.equals(ContentModel.PROP_MODIFIED)) - { - String dateStr = auditModified == null ? auditCreated : auditModified; - return DefaultTypeConverter.INSTANCE.convert(Date.class, dateStr); - } - else if (qname.equals(ContentModel.PROP_ACCESSED)) - { - return DefaultTypeConverter.INSTANCE.convert(Date.class, auditAccessed); - } - else - { - return null; - } - } - - /** - * @param qname the property name - * @param value the property value - * @return Returns true if the property was used - * @deprecated Deprecated from the start, but possibly useful code - */ - @SuppressWarnings("unused") - private boolean setAuditableProperty(QName qname, Serializable value) - { - if (qname.equals(ContentModel.PROP_CREATOR)) - { - auditCreator = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - if (qname.equals(ContentModel.PROP_MODIFIER)) - { - auditModifier = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - if (qname.equals(ContentModel.PROP_CREATED)) - { - auditCreated = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - if (qname.equals(ContentModel.PROP_MODIFIED)) - { - auditModified = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - if (qname.equals(ContentModel.PROP_ACCESSED)) - { - auditAccessed = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - else - { - return false; - } - } - - /** - * @return Returns a Map of auditable properties - */ - public Map getAuditableProperties() - { - Map properties = new HashMap(7); - properties.put(ContentModel.PROP_CREATOR, auditCreator); - properties.put(ContentModel.PROP_CREATED, DefaultTypeConverter.INSTANCE.convert(Date.class, auditCreated)); - // cm:modifier - use cm:creator if not set - if (auditModifier != null) - { - properties.put(ContentModel.PROP_MODIFIER, auditModifier); - } - else - { - properties.put(ContentModel.PROP_MODIFIER, auditCreator); - } - // cm:modified - use cm:created if not set - if (auditModified != null) - { - properties.put(ContentModel.PROP_MODIFIED, DefaultTypeConverter.INSTANCE.convert(Date.class, auditModified)); - } - else - { - properties.put(ContentModel.PROP_MODIFIED, DefaultTypeConverter.INSTANCE.convert(Date.class, auditCreated)); - } - // Usually null - if (auditAccessed != null) - { - properties.put(ContentModel.PROP_ACCESSED, DefaultTypeConverter.INSTANCE.convert(Date.class, auditAccessed)); - } - return properties; - } - - /** - * Set all cm:auditable parameters as required. Where possible, the creation and modification data - * will be shared so as to reduce data duplication. - * - * @param user the username - * @param date the creation or modification date - * @param force true to force the values to overwrite any pre-existing values - */ - public void setAuditValues(String user, Date date, boolean force) - { - String dateStr = DefaultTypeConverter.INSTANCE.convert(String.class, date); - - // Always set cm:creator and cm:created - if (force || auditCreator == null) - { - auditCreator = user; - } - if (force || auditCreated == null) - { - auditCreated = dateStr; - } - auditModifier = user; - auditModified = dateStr; - } - - /** - * Set all cm:auditable parameters as required, giving precedence to the supplied - * property map. - * - * @param user the username - * @param date the creation or modification date - * @param properties the properties to override the user and date - */ - public void setAuditValues(String user, Date date, Map properties) - { - String dateStr = DefaultTypeConverter.INSTANCE.convert(String.class, date); - if (properties.containsKey(ContentModel.PROP_CREATOR)) - { - auditCreator = DefaultTypeConverter.INSTANCE.convert( - String.class, - properties.get(ContentModel.PROP_CREATOR)); - } - else if (auditCreator == null) - { - auditCreator = user; - } - if (properties.containsKey(ContentModel.PROP_MODIFIER)) - { - auditModifier = DefaultTypeConverter.INSTANCE.convert( - String.class, - properties.get(ContentModel.PROP_MODIFIER)); - } - else if (auditModifier == null) - { - auditModifier = user; - } - if (properties.containsKey(ContentModel.PROP_CREATED)) - { - auditCreated = DefaultTypeConverter.INSTANCE.convert( - String.class, - properties.get(ContentModel.PROP_CREATED)); - } - else if (auditCreated == null) - { - auditCreated = dateStr; - } - if (properties.containsKey(ContentModel.PROP_MODIFIED)) - { - auditModified = DefaultTypeConverter.INSTANCE.convert( - String.class, - properties.get(ContentModel.PROP_MODIFIED)); - } - else if (auditModified == null) - { - auditModified = dateStr; - } - if (properties.containsKey(ContentModel.PROP_ACCESSED)) - { - auditAccessed = DefaultTypeConverter.INSTANCE.convert( - String.class, - properties.get(ContentModel.PROP_ACCESSED)); - } - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private String getAuditCreator() - { - return auditCreator; - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private void setAuditCreator(String auditCreator) - { - this.auditCreator = auditCreator; - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private String getAuditCreated() - { - return auditCreated; - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private void setAuditCreated(String auditCreated) - { - this.auditCreated = auditCreated; - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private String getAuditModifier() - { - return auditModifier; - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private void setAuditModifier(String auditModifier) - { - this.auditModifier = auditModifier; - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private String getAuditModified() - { - return auditModified; - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private void setAuditModified(String auditModified) - { - this.auditModified = auditModified; - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private String getAuditAccessed() - { - return auditAccessed; - } - - /** - * For persistance use - */ - @SuppressWarnings("unused") - private void setAuditAccessed(String auditAccessed) - { - this.auditAccessed = auditAccessed; - } -} diff --git a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java index c980d3b82f..a7c2ba6e37 100644 --- a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java @@ -25,6 +25,7 @@ import java.sql.Savepoint; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; @@ -55,13 +56,8 @@ import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.repo.security.permissions.AccessControlListProperties; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; import org.alfresco.repo.transaction.AlfrescoTransactionSupport.TxnReadState; -import org.alfresco.repo.transaction.RetryingTransactionHelper; -import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.repo.transaction.TransactionAwareSingleton; import org.alfresco.repo.transaction.TransactionListenerAdapter; -import org.alfresco.repo.transaction.TransactionalResourceHelper; -import org.alfresco.service.cmr.dictionary.AssociationDefinition; -import org.alfresco.service.cmr.dictionary.ChildAssociationDefinition; import org.alfresco.service.cmr.dictionary.DataTypeDefinition; import org.alfresco.service.cmr.dictionary.DictionaryService; import org.alfresco.service.cmr.dictionary.InvalidTypeException; @@ -119,10 +115,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO private NodePropertyHelper nodePropertyHelper; private ServerIdCallback serverIdCallback = new ServerIdCallback(); private UpdateTransactionListener updateTransactionListener = new UpdateTransactionListener(); - private AuditableTransactionListener auditableTransactionListener = new AuditableTransactionListener(); private RetryingCallbackHelper childAssocRetryingHelper; - - private boolean enableTimestampPropagation; private TransactionService transactionService; private DictionaryService dictionaryService; @@ -187,18 +180,6 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO parentAssocsCache = new EntityLookupCache(new ParentAssocsCallbackDAO()); } - /** - * Set whether cm:auditable timestamps should be propagated to parent nodes - * where the parent-child relationship has been marked using propagateTimestamps. - * - * @param enableTimestampPropagation true to propagate timestamps to the parent - * node where appropriate - */ - public void setEnableTimestampPropagation(boolean enableTimestampPropagation) - { - this.enableTimestampPropagation = enableTimestampPropagation; - } - /** * @param transactionService the service to start post-txn processes */ @@ -602,8 +583,8 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO public Long getCurrentTransactionId() { - TransactionEntity txn = getCurrentTransaction(); - return txn.getId(); + TransactionEntity txn = AlfrescoTransactionSupport.getResource(KEY_TRANSACTION); + return txn == null ? null : txn.getId(); } /* @@ -767,7 +748,16 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO public Pair findByKey(Long nodeId) { NodeEntity node = selectNodeById(nodeId, Boolean.FALSE); - return node == null ? null : new Pair(nodeId, node); + if (node != null) + { + // Lock it to prevent 'accidental' modification + node.lock(); + return new Pair(nodeId, node); + } + else + { + return null; + } } /** @@ -787,7 +777,16 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO { NodeRef nodeRef = node.getNodeRef(); node = selectNodeByNodeRef(nodeRef, Boolean.FALSE); - return node == null ? null : new Pair(node.getId(), node); + if (node != null) + { + // Lock it to prevent 'accidental' modification + node.lock(); + return new Pair(node.getId(), node); + } + else + { + return null; + } } } @@ -804,6 +803,21 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO return pair != null && !pair.getSecond().getDeleted(); } + @Override + public boolean isInCurrentTxn(Long nodeId) + { + Long currentTxnId = getCurrentTransactionId(); + if (currentTxnId == null) + { + // No transactional changes have been made to any nodes, therefore the node cannot + // be part of the current transaction + return false; + } + Node node = getNodeNotNull(nodeId); + Long nodeTxnId = node.getTransaction().getId(); + return nodeTxnId.equals(currentTxnId); + } + public Status getNodeRefStatus(NodeRef nodeRef) { Node node = null; @@ -978,12 +992,6 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO assoc); parentAssocsCache.setValue(nodeId, parentAssocsInfo); - // Ensure that cm:auditable values are propagated, if required - if (enableTimestampPropagation) - { - propagateTimestamps(nodeId); - } - if (isDebugEnabled) { logger.debug( @@ -1189,12 +1197,12 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO childNodeUpdate.setUpdateDeleted(true); // Update the entity. // Note: We don't use delete here because that will attempt to clean everything up again. - updateNodeImpl(childNode, childNodeUpdate, false); + updateNodeImpl(childNode, childNodeUpdate); } else { // Ensure that the child node reflects the current txn and auditable data - touchNodeImpl(childNodeId, true); + touchNodeImpl(childNodeId); // The moved node's reference has not changed, so just remove the cache entry to // it's immediate parent. All children of the moved node will still point to the @@ -1260,7 +1268,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } @Override - public void updateNode(Long nodeId, QName nodeTypeQName, Locale nodeLocale, boolean propagate) + public boolean updateNode(Long nodeId, QName nodeTypeQName, Locale nodeLocale) { // Get the existing node; we need to check for a change in store or UUID Node oldNode = getNodeNotNull(nodeId); @@ -1301,13 +1309,26 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO nodeUpdate.setUpdateLocaleId(true); } - updateNodeImpl(oldNode, nodeUpdate, propagate); + return updateNodeImpl(oldNode, nodeUpdate); } /** * Updates the node's transaction and cm:auditable properties only. */ - private void touchNodeImpl(Long nodeId, boolean propagate) + private boolean touchNodeImpl(Long nodeId) + { + return touchNodeImpl(nodeId, null); + } + /** + * Updates the node's transaction and cm:auditable properties only. + * + * @param auditableProps optionally override the cm:auditable values + * @param propagate should this update be propagated to parent audit properties? + * + * + * @see #updateNodeImpl(NodeEntity, NodeUpdateEntity) + */ + private boolean touchNodeImpl(Long nodeId, AuditablePropertiesEntity auditableProps) { Node node = null; try @@ -1318,12 +1339,12 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO { // The ID doesn't reference a live node. // We do nothing w.r.t. touching - return; + return false; } NodeUpdateEntity nodeUpdate = new NodeUpdateEntity(); nodeUpdate.setId(nodeId); // Update it - updateNodeImpl(node, nodeUpdate, propagate); + return updateNodeImpl(node, nodeUpdate); } /** @@ -1335,9 +1356,9 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO * * @param oldNode the existing node, fully populated * @param nodeUpdate the node update with all update elements populated - * @param propagate should this update be propagated to parent audit properties? + * @return true if any updates were made */ - private void updateNodeImpl(Node oldNode, NodeUpdateEntity nodeUpdate, boolean propagate) + private boolean updateNodeImpl(Node oldNode, NodeUpdateEntity nodeUpdate) { Long nodeId = oldNode.getId(); @@ -1391,6 +1412,10 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO { auditableProps = new AuditablePropertiesEntity(); } + else + { + auditableProps = new AuditablePropertiesEntity(auditableProps); + } boolean updateAuditableProperties = auditableProps.setAuditValues(null, null, false, 1000L); nodeUpdate.setAuditableProperties(auditableProps); nodeUpdate.setUpdateAuditableProperties(updateAuditableProperties); @@ -1401,7 +1426,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO AuditablePropertiesEntity auditableProps = oldNode.getAuditableProperties(); if (auditableProps != null) { - nodeUpdate.setAuditableProperties(auditableProps); + nodeUpdate.setAuditableProperties(auditableProps); // Can reuse the locked instance nodeUpdate.setUpdateAuditableProperties(true); } } @@ -1426,7 +1451,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO // Just bug out if nothing has changed if (!nodeUpdate.isUpdateAnything()) { - return; + return false; } // The node is remaining in the current store @@ -1452,14 +1477,6 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } } - // Ensure that cm:auditable values are propagated, if required - if (propagate && enableTimestampPropagation && - nodeUpdate.isUpdateAuditableProperties() && - nodeUpdate.getAuditableProperties() != null) - { - propagateTimestamps(nodeId); - } - // Done if (isDebugEnabled) { @@ -1468,132 +1485,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO " OLD: " + oldNode + "\n" + " NEW: " + nodeUpdate); } - } - - private static final String KEY_AUDITABLE_PROPAGATION = "node.auditable.propagation"; - private static final String KEY_AUDITABLE_PROPAGATION_DISABLE = "node.auditable.propagation.disable"; - /** - * Schedule auditable property propagation for the post-commit phase - * - * @param childNodeId the ID of the node that has auditable properties changed - */ - private void propagateTimestamps(Long childNodeId) - { - if (!enableTimestampPropagation) - { - return; // Don't propagate - } - // Get the current timestamp - Node childNode = getNodeNotNull(childNodeId); - if (childNode.getAuditableProperties() == null) - { - return; // Not auditable - } - String modified = childNode.getAuditableProperties().getAuditModified(); - // Check the parent association - ChildAssocEntity primaryParentAssoc = getPrimaryParentAssocImpl(childNodeId); - if (primaryParentAssoc == null) - { - return; // This is a root - } - // Check the association type - Long assocTypeQNameId = primaryParentAssoc.getTypeQNameId(); - Pair assocTypeQNamePair = qnameDAO.getQName(assocTypeQNameId); - if (assocTypeQNamePair == null) - { - return; // Unknown association type - } - AssociationDefinition assocDef = dictionaryService.getAssociation(assocTypeQNamePair.getSecond()); - if (!assocDef.isChild() || !((ChildAssociationDefinition)assocDef).getPropagateTimestamps()) - { - return; // Don't propagate - } - - // Record the parent node ID for update - Long parentNodeId = primaryParentAssoc.getParentNode().getId(); - Map modifiedDatesById = TransactionalResourceHelper.getMap(KEY_AUDITABLE_PROPAGATION); - String existingModified = modifiedDatesById.get(parentNodeId); - if (existingModified != null && existingModified.compareTo(modified) > 0) - { - return; // Already have a later date ready to go - } - modifiedDatesById.put(parentNodeId, modified); - - // Bind a listener for post-transaction manipulation - AlfrescoTransactionSupport.bindListener(auditableTransactionListener); - } - - /** - * Wrapper to update the current transaction to get the change time correct - * - * @author Derek Hulley - * @since 3.4.2 - */ - private class AuditableTransactionListener extends TransactionListenerAdapter - { - @Override - public void afterCommit() - { - // Check if we are already propagating - if (AlfrescoTransactionSupport.getResource(KEY_AUDITABLE_PROPAGATION_DISABLE) != null) - { - // This is a propagating transaction, so do nothing - return; - } - - Map modifiedDatesById = TransactionalResourceHelper.getMap(KEY_AUDITABLE_PROPAGATION); - if (modifiedDatesById.size() == 0) - { - return; - } - // Walk through the IDs, processing groups - for (Map.Entry entry: modifiedDatesById.entrySet()) - { - Long parentNodeId = entry.getKey(); - String modified = entry.getValue(); - processBatch(parentNodeId, modified); - } - } - - private void processBatch(final Long parentNodeId, final String modified) - { - RetryingTransactionHelper txnHelper = transactionService.getRetryingTransactionHelper(); - txnHelper.setMaxRetries(1); - RetryingTransactionCallback callback = new RetryingTransactionCallback() - { - @Override - public Void execute() throws Throwable - { - // Disable all behaviour. - // This only affects cm:auditable and is discarded at the end of this txn - policyBehaviourFilter.disableAllBehaviours(); - // Tag the transaction to prevent further propagation - AlfrescoTransactionSupport.bindResource(KEY_AUDITABLE_PROPAGATION_DISABLE, Boolean.TRUE); - - Pair parentNodePair = getNodePair(parentNodeId); - if (parentNodePair == null) - { - return null; // Parent has gone away - } - // Modify the parent with the new date (if it needs) - // Disable cm:auditable for the parent so that we can set it manually - addNodeProperty(parentNodeId, ContentModel.PROP_MODIFIED, modified); - return null; - } - }; - try - { - txnHelper.doInTransaction(callback, false, true); - if (isDebugEnabled) - { - logger.debug("Propagated timestamps from node: " + parentNodeId); - } - } - catch (Throwable e) - { - logger.info("Failed to update auditable properties for nodes: " + parentNodeId); - } - } + return true; } public void setNodeAclId(Long nodeId, Long aclId) @@ -1603,7 +1495,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO nodeUpdateEntity.setId(nodeId); nodeUpdateEntity.setAclId(aclId); nodeUpdateEntity.setUpdateAclId(true); - updateNodeImpl(oldNode, nodeUpdateEntity, true); + updateNodeImpl(oldNode, nodeUpdateEntity); } public void setPrimaryChildrenSharedAclId( @@ -1628,12 +1520,6 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO public void deleteNode(Long nodeId) { - // Ensure that cm:auditable values are propagated, if required - if (enableTimestampPropagation) - { - propagateTimestamps(nodeId); - } - Node node = getNodeNotNull(nodeId); Long aclId = node.getAclId(); // Need this later @@ -1674,6 +1560,10 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO { auditableProps = new AuditablePropertiesEntity(); } + else + { + auditableProps = new AuditablePropertiesEntity(auditableProps); // Create unlocked instance + } auditableProps.setAuditValues(null, null, false, 1000L); nodeUpdate.setAuditableProperties(auditableProps); nodeUpdate.setUpdateAuditableProperties(true); @@ -1839,6 +1729,10 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO { auditableProps = new AuditablePropertiesEntity(); } + else + { + auditableProps = new AuditablePropertiesEntity(auditableProps); // Unlocked instance + } boolean containedAuditProperties = auditableProps.setAuditValues(null, null, newProps); if (!containedAuditProperties) { @@ -2016,7 +1910,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO // Touch to bring into current transaction if (updated) { - updateNodeImpl(node, nodeUpdate, propsToAdd.containsKey(ContentModel.PROP_NAME)); + updateNodeImpl(node, nodeUpdate); } // Done @@ -2084,12 +1978,58 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO cachedProps.keySet().removeAll(propertyQNames); setNodePropertiesCached(nodeId, cachedProps); // Touch to bring into current txn - touchNodeImpl(nodeId, propertyQNames.contains(ContentModel.PROP_NAME)); + touchNodeImpl(nodeId); } // Done return deleteCount > 0; } + @Override + public boolean setModifiedDate(Long nodeId, Date modifiedDate) + { + // Do nothing if the node is not cm:auditable + if (!hasNodeAspect(nodeId, ContentModel.ASPECT_AUDITABLE)) + { + return false; + } + // Get the node + Node node = getNodeNotNull(nodeId); + NodeRef nodeRef = node.getNodeRef(); + // Get the existing auditable values + AuditablePropertiesEntity auditableProps = node.getAuditableProperties(); + boolean dateChanged = false; + if (auditableProps == null) + { + // The properties should be present + auditableProps = new AuditablePropertiesEntity(); + auditableProps.setAuditValues(null, modifiedDate, true, 1000L); + dateChanged = true; + } + else + { + auditableProps = new AuditablePropertiesEntity(auditableProps); + dateChanged = auditableProps.setAuditModified(modifiedDate, 1000L); + } + if (dateChanged) + { + try + { + policyBehaviourFilter.disableBehaviour(nodeRef, ContentModel.ASPECT_AUDITABLE); + // Send this to the node update + return touchNodeImpl(nodeId, auditableProps); + } + finally + { + policyBehaviourFilter.enableBehaviour(nodeRef, ContentModel.ASPECT_AUDITABLE); + } + } + else + { + // Date did not advance + return false; + } + } + /** * @return Returns a writable copy of the cached property map */ @@ -2242,7 +2182,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } // Touch to bring into current txn - touchNodeImpl(nodeId, false); + touchNodeImpl(nodeId); // Done return true; @@ -2264,7 +2204,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO aspectsCache.setValue(nodeId, Collections.emptySet()); // Touch to bring into current txn - touchNodeImpl(nodeId, false); + touchNodeImpl(nodeId); // Done return deleteCount > 0; @@ -2290,7 +2230,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } // Touch to bring into current txn - touchNodeImpl(nodeId, false); + touchNodeImpl(nodeId); // Done return deleteCount > 0; @@ -2368,7 +2308,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } // Touch to bring into current txn and ensure concurrency is maintained on the nodes - touchNodeImpl(sourceNodeId, false); + touchNodeImpl(sourceNodeId); // Resolve type QName Long assocTypeQNameId = qnameDAO.getOrCreateQName(assocTypeQName).getFirst(); @@ -2414,7 +2354,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO return 0; } // Touch to bring into current txn - touchNodeImpl(sourceNodeId, false); + touchNodeImpl(sourceNodeId); Long assocTypeQNameId = assocTypeQNamePair.getFirst(); return deleteNodeAssoc(sourceNodeId, targetNodeId, assocTypeQNameId); @@ -2423,7 +2363,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO public int removeNodeAssocsToAndFrom(Long nodeId) { // Touch to bring into current txn - touchNodeImpl(nodeId, false); + touchNodeImpl(nodeId); return deleteNodeAssocsToAndFrom(nodeId); } @@ -2437,7 +2377,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO return 0; } // Touch to bring into current txn - touchNodeImpl(nodeId, false); + touchNodeImpl(nodeId); return deleteNodeAssocsToAndFrom(nodeId, assocTypeQNameIds); } @@ -2605,7 +2545,7 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO // node into the current transaction for secondary associations if (!isPrimary) { - updateNode(childNodeId, null, null, true); + updateNode(childNodeId, null, null); } // Done @@ -3246,11 +3186,11 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO private ParentAssocsInfo getParentAssocsCached(Long nodeId) { // We try to protect here against 'skew' between a cached node and its parent associations - // Unfortunately due to overlapping DB transactions and consistent read behaviour a thread - // can end up loading old associations and succeed in committing them to the shared cache - // without any conflicts - - // Allow for a single retry after cache validation + // Unfortunately due to overlapping DB transactions and consistent read behaviour a thread + // can end up loading old associations and succeed in committing them to the shared cache + // without any conflicts + + // Allow for a single retry after cache validation for (int i = 0; i < 2; i++) { Pair cacheEntry = parentAssocsCache.getByKey(nodeId); diff --git a/source/java/org/alfresco/repo/domain/node/AuditablePropertiesEntity.java b/source/java/org/alfresco/repo/domain/node/AuditablePropertiesEntity.java index dc4cb09a10..201e05f75c 100644 --- a/source/java/org/alfresco/repo/domain/node/AuditablePropertiesEntity.java +++ b/source/java/org/alfresco/repo/domain/node/AuditablePropertiesEntity.java @@ -100,6 +100,8 @@ public class AuditablePropertiesEntity return typeDef.getDefaultAspectNames().contains(ContentModel.ASPECT_AUDITABLE); } + private boolean locked; + private String auditCreator; private String auditCreated; private String auditModifier; @@ -114,6 +116,21 @@ public class AuditablePropertiesEntity */ public AuditablePropertiesEntity() { + locked = false; + } + + /** + * Copy constructor to create an unlocked instance + */ + public AuditablePropertiesEntity(AuditablePropertiesEntity that) + { + locked = false; + this.auditCreator = that.auditCreator; + this.auditCreated = that.auditCreated; + this.auditModifier = that.auditModifier; + this.auditModified = that.auditModified; + this.auditAccessed = that.auditAccessed; + this.auditModifiedTime = that.auditModifiedTime; } @Override @@ -129,6 +146,22 @@ public class AuditablePropertiesEntity return sb.toString(); } + /** + * Lock the entity against further updates to prevent accidental modification + */ + public synchronized void lock() + { + locked = true; + } + + private synchronized final void checkLock() + { + if (locked) + { + throw new IllegalStateException("The entity is locked against updates: " + this); + } + } + /** * @param qname the property name * @return Returns the value of the cm:auditable property or null @@ -162,46 +195,6 @@ public class AuditablePropertiesEntity } } - /** - * @param qname the property name - * @param value the property value - * @return Returns true if the property was used - * @deprecated Deprecated from the start, but possibly useful code - */ - @SuppressWarnings("unused") - private boolean setAuditableProperty(QName qname, Serializable value) - { - if (qname.equals(ContentModel.PROP_CREATOR)) - { - auditCreator = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - if (qname.equals(ContentModel.PROP_MODIFIER)) - { - auditModifier = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - if (qname.equals(ContentModel.PROP_CREATED)) - { - auditCreated = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - if (qname.equals(ContentModel.PROP_MODIFIED)) - { - auditModified = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - if (qname.equals(ContentModel.PROP_ACCESSED)) - { - auditAccessed = DefaultTypeConverter.INSTANCE.convert(String.class, value); - return true; - } - else - { - return false; - } - } - /** * @return Returns a Map of auditable properties */ @@ -252,6 +245,8 @@ public class AuditablePropertiesEntity */ public boolean setAuditValues(String user, Date date, boolean force, long modifiedDateToleranceMs) { + checkLock(); + // Get a user if we need if (user == null) { @@ -312,6 +307,8 @@ public class AuditablePropertiesEntity */ public boolean setAuditValues(String user, Date date, Map properties) { + checkLock(); + // Need to know if anything changed boolean changed = false; @@ -431,6 +428,7 @@ public class AuditablePropertiesEntity */ public void setAuditCreator(String auditCreator) { + checkLock(); this.auditCreator = auditCreator; } @@ -447,6 +445,7 @@ public class AuditablePropertiesEntity */ public void setAuditCreated(String auditCreated) { + checkLock(); this.auditCreated = auditCreated; } @@ -463,6 +462,7 @@ public class AuditablePropertiesEntity */ public void setAuditModifier(String auditModifier) { + checkLock(); this.auditModifier = auditModifier; } @@ -492,9 +492,34 @@ public class AuditablePropertiesEntity */ public void setAuditModified(String auditModified) { + checkLock(); this.auditModified = auditModified; } + /** + * @param modifiedDateToleranceMs the number of milliseconds' to tolerate before updating the + * modification date. + * Setting this to 1000L (say) will mean that the modification time will not be + * changed if the existing value is withing 1000 ms of the new time. + * @return Returns true if there were any changes made, otherwise false + */ + public boolean setAuditModified(Date date, long modifiedDateToleranceMs) + { + checkLock(); + + long dateTime = date.getTime(); + long lastModTime = getAuditModifiedTime(); + boolean changed = false; + if (lastModTime < 0 || (lastModTime + modifiedDateToleranceMs) < dateTime) + { + // The time has moved on enough + auditModifiedTime = dateTime; + auditModified = DefaultTypeConverter.INSTANCE.convert(String.class, date); + changed = true; + } + return changed; + } + /** * For persistance use */ @@ -508,6 +533,7 @@ public class AuditablePropertiesEntity */ public void setAuditAccessed(String auditAccessed) { + checkLock(); this.auditAccessed = auditAccessed; } } diff --git a/source/java/org/alfresco/repo/domain/node/Node.java b/source/java/org/alfresco/repo/domain/node/Node.java index 2440f3006c..9c262ac02a 100644 --- a/source/java/org/alfresco/repo/domain/node/Node.java +++ b/source/java/org/alfresco/repo/domain/node/Node.java @@ -28,6 +28,11 @@ import org.alfresco.util.Pair; */ public interface Node extends NodeIdAndAclId { + /** + * Helper method to force the instance to be read-only + */ + public void lock(); + public abstract NodeRef getNodeRef(); public NodeRef.Status getNodeStatus(); diff --git a/source/java/org/alfresco/repo/domain/node/NodeDAO.java b/source/java/org/alfresco/repo/domain/node/NodeDAO.java index 05b33933fd..05e39403e1 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeDAO.java +++ b/source/java/org/alfresco/repo/domain/node/NodeDAO.java @@ -20,6 +20,7 @@ package org.alfresco.repo.domain.node; import java.io.Serializable; import java.util.Collection; +import java.util.Date; import java.util.List; import java.util.Locale; import java.util.Map; @@ -68,7 +69,9 @@ public interface NodeDAO extends NodeBulkLoader /** * - * @return Returns the ID of the current transaction entry + * @return Returns the ID of the current transaction entry or null if + * there have not been any modifications to nodes registered in the + * transaction */ public Long getCurrentTransactionId(); @@ -123,6 +126,13 @@ public interface NodeDAO extends NodeBulkLoader public boolean exists(NodeRef nodeRef); public boolean exists(Long nodeId); + /** + * @return Returns true if the node was last modified in the current + * transaction, otherwise false. + * @throws InvalidNodeRefException if there is no record of the node, past or present + */ + public boolean isInCurrentTxn(Long nodeId); + /** * Get the current status of the node, including deleted nodes. * @@ -192,9 +202,9 @@ public interface NodeDAO extends NodeBulkLoader /** * @param nodeTypeQName the new type QName for the node or null to keep the existing one * @param nodeLocale the new locale for the node or null to keep the existing one - * @param propagate should this update be propagated to parent audit properties? + * @return true if any changes were made */ - public void updateNode(Long nodeId, QName nodeTypeQName, Locale nodeLocale, boolean propagate); + public boolean updateNode(Long nodeId, QName nodeTypeQName, Locale nodeLocale); public void setNodeAclId(Long nodeId, Long aclId); @@ -233,6 +243,17 @@ public interface NodeDAO extends NodeBulkLoader public boolean removeNodeProperties(Long nodeId, Set propertyQNames); + /** + * Pull the cm:modified up to the current time without changing any other + * cm:auditable properties. The change may be done in the current transaction + * or in a later transaction. + * + * @param nodeId the node to change + * @param modifiedDate the date to set for cm:modified + * @return Returns true if the cm:modified property was actually set + */ + public boolean setModifiedDate(Long nodeId, Date date); + /* * Aspects */ diff --git a/source/java/org/alfresco/repo/domain/node/NodeDAOTest.java b/source/java/org/alfresco/repo/domain/node/NodeDAOTest.java index 816a73a886..e56ac893c4 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeDAOTest.java +++ b/source/java/org/alfresco/repo/domain/node/NodeDAOTest.java @@ -29,10 +29,8 @@ import org.alfresco.repo.transaction.RetryingTransactionHelper; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; import org.alfresco.service.ServiceRegistry; import org.alfresco.service.cmr.repository.NodeRef; -import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.transaction.TransactionService; import org.alfresco.util.ApplicationContextHelper; -import org.alfresco.util.GUID; import org.alfresco.util.Pair; import org.springframework.context.ApplicationContext; @@ -85,49 +83,10 @@ public class NodeDAOTest extends TestCase // Expected } // Read-only - try - { - txnHelper.doInTransaction(getTxnIdCallback, true); - fail("Should have failed when running in read-only transaction"); - } - catch (Throwable e) - { - // Expected - } + assertNull("No Txn ID should be present in read-only txn", txnHelper.doInTransaction(getTxnIdCallback, true)); // First success Long txnId1 = txnHelper.doInTransaction(getTxnIdCallback); - assertNotNull("No txn ID 1", txnId1); - Long txnId2 = txnHelper.doInTransaction(getTxnIdCallback); - assertNotNull("No txn ID 2", txnId2); - assertTrue("2nd ID should be larger than first", txnId2.compareTo(txnId1) > 0); - } - - /** - * TODO: Clean up after test or force indexing on the stores - */ - public void xtestNewStore() throws Exception - { - final StoreRef storeRef = new StoreRef(StoreRef.PROTOCOL_TEST, getName() + "-" + GUID.generate()); - RetryingTransactionCallback> callback = new RetryingTransactionCallback>() - { - public Pair execute() throws Throwable - { - return nodeDAO.newStore(storeRef); - } - }; - Pair rootNodePair = txnHelper.doInTransaction(callback); - assertNotNull(rootNodePair); - assertEquals("Root node has incorrect store", storeRef, rootNodePair.getSecond().getStoreRef()); - // Should fail second time - try - { - txnHelper.doInTransaction(callback); - fail("Should not be able to create same store."); - } - catch (Throwable e) - { - // Expected - } + assertNull("No Txn ID should be present in untouched txn", txnId1); } public void testGetNodesWithAspects() throws Throwable diff --git a/source/java/org/alfresco/repo/domain/node/NodeEntity.java b/source/java/org/alfresco/repo/domain/node/NodeEntity.java index ee9ef82499..c5e57ca391 100644 --- a/source/java/org/alfresco/repo/domain/node/NodeEntity.java +++ b/source/java/org/alfresco/repo/domain/node/NodeEntity.java @@ -108,12 +108,16 @@ public class NodeEntity implements Node, PermissionCheckValue /** * Lock the entity against further updates to prevent accidental modification */ - public void lock() + public synchronized void lock() { locked = true; + if (auditableProperties != null) + { + auditableProperties.lock(); + } } - private final void checkLock() + private synchronized final void checkLock() { if (locked) { diff --git a/source/java/org/alfresco/repo/model/filefolder/FileFolderServiceImplTest.java b/source/java/org/alfresco/repo/model/filefolder/FileFolderServiceImplTest.java index 1a9bbd32a6..d0857bc0f3 100644 --- a/source/java/org/alfresco/repo/model/filefolder/FileFolderServiceImplTest.java +++ b/source/java/org/alfresco/repo/model/filefolder/FileFolderServiceImplTest.java @@ -41,7 +41,6 @@ import org.alfresco.repo.content.MimetypeMap; import org.alfresco.repo.dictionary.DictionaryDAO; import org.alfresco.repo.dictionary.M2Model; import org.alfresco.repo.dictionary.M2Type; -import org.alfresco.repo.domain.node.AbstractNodeDAOImpl; import org.alfresco.repo.node.integrity.IntegrityChecker; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; @@ -1024,7 +1023,7 @@ public class FileFolderServiceImplTest extends TestCase txn.commit(); nodeService.addAspect(workingRootNodeRef, ContentModel.ASPECT_AUDITABLE, null); - + FileInfo folderInfo = fileFolderService.create(workingRootNodeRef, "SomeFolder", ContentModel.TYPE_FOLDER); NodeRef folderNodeRef = folderInfo.getNodeRef(); // Get the dates for the folder we are using @@ -1040,6 +1039,15 @@ public class FileFolderServiceImplTest extends TestCase Date modifiedTooHigh = (Date) nodeService.getProperty(workingRootNodeRef, ContentModel.PROP_MODIFIED); // Create a new file and check the parent (expect changes) + Date beforeSleep = new Date(); + try + { + Thread.sleep(3000L); + } + catch (InterruptedException e) + { + //Respect but ignore + } FileInfo fileInfo = fileFolderService.create(folderNodeRef, "Something.html", ContentModel.TYPE_CONTENT); NodeRef fileNodeRef = fileInfo.getNodeRef(); nodeService.addAspect(fileNodeRef, ContentModel.ASPECT_AUDITABLE, null); @@ -1053,11 +1061,21 @@ public class FileFolderServiceImplTest extends TestCase assertEquals("cm:modifier should have changed", nodeService.getProperty(fileNodeRef, ContentModel.PROP_MODIFIER), nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIER)); - assertEquals("cm:modified should have changed", - nodeService.getProperty(fileNodeRef, ContentModel.PROP_MODIFIED), - nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIED)); - // Rename the child and check parent (expect changes) - fileFolderService.rename(fileNodeRef, "something.html"); + assertTrue("cm:modified should have changed", + beforeSleep.compareTo((Date)nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIED)) < 0); + + // Update the child and check parent (expect NO changes) + modifiedExpected = (Date) nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIED); + beforeSleep = new Date(); + try + { + Thread.sleep(3000L); + } + catch (InterruptedException e) + { + //Respect but ignore + } + nodeService.setProperty(fileNodeRef, ContentModel.PROP_TITLE, "Hippo"); assertEquals("cm:creator should not have changed", creatorExpected, nodeService.getProperty(folderNodeRef, ContentModel.PROP_CREATOR)); @@ -1067,12 +1085,20 @@ public class FileFolderServiceImplTest extends TestCase assertEquals("cm:modifier should not have changed", modifierExpected, nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIER)); - assertNotSame("cm:modified should have changed", - modifiedExpected, - nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIED)); - // Update the child and check parent (expect NO changes) + assertTrue("cm:modified should not have changed", + modifiedExpected.equals(nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIED))); + + // Rename the child and check parent (expect NO changes) modifiedExpected = (Date) nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIED); - nodeService.setProperty(fileNodeRef, ContentModel.PROP_TITLE, "Hippo"); + try + { + Thread.sleep(3000L); + } + catch (InterruptedException e) + { + //Respect but ignore + } + nodeService.setProperty(fileNodeRef, ContentModel.PROP_TITLE, "Something-new.html"); assertEquals("cm:creator should not have changed", creatorExpected, nodeService.getProperty(folderNodeRef, ContentModel.PROP_CREATOR)); @@ -1085,7 +1111,17 @@ public class FileFolderServiceImplTest extends TestCase assertEquals("cm:modified should not have changed", modifiedExpected, nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIED)); + // Delete node and check parent (expect modifier changes) + beforeSleep = new Date(); + try + { + Thread.sleep(3000L); + } + catch (InterruptedException e) + { + //Respect but ignore + } fileFolderService.delete(fileNodeRef); assertEquals("cm:creator should not have changed", creatorExpected, @@ -1093,12 +1129,12 @@ public class FileFolderServiceImplTest extends TestCase assertEquals("cm:created should not have changed", createdExpected, nodeService.getProperty(folderNodeRef, ContentModel.PROP_CREATED)); - assertSame("cm:modifier should have changed", - modifierExpected, - nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIER)); - assertNotSame("cm:modified should have changed", - modifiedExpected, - nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIED)); + assertEquals("cm:modifier should have changed", + modifierExpected, + nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIER)); + assertTrue("cm:modified should have changed", + beforeSleep.compareTo((Date)nodeService.getProperty(folderNodeRef, ContentModel.PROP_MODIFIED)) < 0); + // Finally check that the second level up was NOT modified assertEquals("cm:creator should not have changed (level too high)", creatorTooHigh, @@ -1112,6 +1148,70 @@ public class FileFolderServiceImplTest extends TestCase assertEquals("cm:modified should not have changed (level too high)", modifiedTooHigh, nodeService.getProperty(workingRootNodeRef, ContentModel.PROP_MODIFIED)); + + // Now let's test file moving: + // Create source folder + FileInfo sourceFolderInfo = fileFolderService.create(workingRootNodeRef, "SourceFolder", ContentModel.TYPE_FOLDER); + NodeRef sourceFolderNodeRef = sourceFolderInfo.getNodeRef(); + + //Create destination folder + FileInfo destinationFolderInfo = fileFolderService.create(workingRootNodeRef, "DestinationFolder", ContentModel.TYPE_FOLDER); + NodeRef destinationFolderNodeRef = destinationFolderInfo.getNodeRef(); + + String sourceFolderCreatorExpected = (String) nodeService.getProperty(sourceFolderNodeRef, ContentModel.PROP_CREATOR); + Date sourceFolderCreatedExpected = (Date) nodeService.getProperty(sourceFolderNodeRef, ContentModel.PROP_CREATED); + + String destinationFolderCreatorExpected = (String) nodeService.getProperty(destinationFolderNodeRef, ContentModel.PROP_CREATOR); + Date destinationFolderCreatedExpected = (Date) nodeService.getProperty(destinationFolderNodeRef, ContentModel.PROP_CREATED); + + FileInfo relocatableFileInfo = fileFolderService.create(sourceFolderNodeRef, "MoveMePlease.html", ContentModel.TYPE_CONTENT); + NodeRef relocatableFileNodeRef = relocatableFileInfo.getNodeRef(); + + nodeService.addAspect(relocatableFileNodeRef, ContentModel.ASPECT_AUDITABLE, null); + + // Get the dates for the source folder after file creation + String sourceFolderModifierExpected = (String) nodeService.getProperty(sourceFolderNodeRef, ContentModel.PROP_MODIFIER); + + // Get the dates for the destination folder + String destinationFolderModifierExpected = (String) nodeService.getProperty(destinationFolderNodeRef, ContentModel.PROP_MODIFIER); + + // Move the file from source folder to destination folder (both should change) + beforeSleep = new Date(); + try + { + Thread.sleep(3000L); + } + catch (InterruptedException e) + { + //Respect but ignore + } + fileFolderService.moveFrom(relocatableFileNodeRef, sourceFolderNodeRef, destinationFolderNodeRef, "MoveMePlease.html"); + + // Check the source folder + assertEquals("cm:creator for source folder should not have changed", + sourceFolderCreatorExpected, + nodeService.getProperty(sourceFolderNodeRef, ContentModel.PROP_CREATOR)); + assertEquals("cm:created for source folder should not have changed", + sourceFolderCreatedExpected, + nodeService.getProperty(sourceFolderNodeRef, ContentModel.PROP_CREATED)); + assertEquals("cm:modifier for source folder should not have changed", + sourceFolderModifierExpected, + nodeService.getProperty(sourceFolderNodeRef, ContentModel.PROP_MODIFIER)); + assertTrue("cm:modified for source folder should have changed", + beforeSleep.compareTo((Date)nodeService.getProperty(sourceFolderNodeRef, ContentModel.PROP_MODIFIED)) < 0); + + // Check the destination folder + assertEquals("cm:creator for destination folder should not have changed", + destinationFolderCreatorExpected, + nodeService.getProperty(destinationFolderNodeRef, ContentModel.PROP_CREATOR)); + assertEquals("cm:created for destination folder should not have changed", + destinationFolderCreatedExpected, + nodeService.getProperty(destinationFolderNodeRef, ContentModel.PROP_CREATED)); + assertEquals("cm:modifier for destination folder should not have changed", + destinationFolderModifierExpected, + nodeService.getProperty(destinationFolderNodeRef, ContentModel.PROP_MODIFIER)); + assertTrue("cm:modified for destination folder should have changed", + beforeSleep.compareTo((Date)nodeService.getProperty(destinationFolderNodeRef, ContentModel.PROP_MODIFIED)) < 0); } public void testPatterns() diff --git a/source/java/org/alfresco/repo/node/NodeServiceTest.java b/source/java/org/alfresco/repo/node/NodeServiceTest.java index c863635103..da67c07a04 100644 --- a/source/java/org/alfresco/repo/node/NodeServiceTest.java +++ b/source/java/org/alfresco/repo/node/NodeServiceTest.java @@ -273,7 +273,7 @@ public class NodeServiceTest extends TestCase * Tests archive and restore of simple hierarchy, checking that references and IDs are * used correctly. */ - public void DISABLED_testArchiveAndRestore() + public void testArchiveAndRestore() { // First create a node structure (a very simple one) and record the references and IDs final NodeRef[] liveNodeRefs = new NodeRef[10]; diff --git a/source/java/org/alfresco/repo/node/archive/ArchiveAndRestoreTest.java b/source/java/org/alfresco/repo/node/archive/ArchiveAndRestoreTest.java index 4860b114b6..8741409edd 100644 --- a/source/java/org/alfresco/repo/node/archive/ArchiveAndRestoreTest.java +++ b/source/java/org/alfresco/repo/node/archive/ArchiveAndRestoreTest.java @@ -761,7 +761,7 @@ public class ArchiveAndRestoreTest extends TestCase /** * ALF-7889 */ - public synchronized void DISABLED_testAR7889ArchiveAndRestoreMustNotModifyAuditable() throws Exception + public synchronized void testAR7889ArchiveAndRestoreMustNotModifyAuditable() throws Exception { AuthenticationUtil.setFullyAuthenticatedUser(USER_A); nodeService.addAspect(b, ContentModel.ASPECT_AUDITABLE, null); diff --git a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java index 7ede3b1ce4..eb78c39e33 100644 --- a/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java +++ b/source/java/org/alfresco/repo/node/db/DbNodeServiceImpl.java @@ -46,9 +46,14 @@ import org.alfresco.repo.node.archive.NodeArchiveService; import org.alfresco.repo.node.index.NodeIndexer; import org.alfresco.repo.policy.BehaviourFilter; import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.transaction.AlfrescoTransactionSupport; +import org.alfresco.repo.transaction.RetryingTransactionHelper; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +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; +import org.alfresco.service.cmr.dictionary.ChildAssociationDefinition; import org.alfresco.service.cmr.dictionary.ClassDefinition; import org.alfresco.service.cmr.dictionary.InvalidAspectException; import org.alfresco.service.cmr.dictionary.InvalidTypeException; @@ -85,6 +90,9 @@ import org.springframework.extensions.surf.util.I18NUtil; */ public class DbNodeServiceImpl extends AbstractNodeServiceImpl { + private final static String KEY_PRE_COMMIT_ADD_NODE = "DbNodeServiceImpl.PreCommitAddNode"; + private final static String KEY_DELETED_NODES = "DbNodeServiceImpl.DeletedNodes"; + private static Log logger = LogFactory.getLog(DbNodeServiceImpl.class); private QNameDAO qnameDAO; @@ -93,8 +101,7 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl private NodeService avmNodeService; private NodeIndexer nodeIndexer; private BehaviourFilter policyBehaviourFilter; - private final static String KEY_PRE_COMMIT_ADD_NODE = "DbNodeServiceImpl.PreCommitAddNode"; - private final static String KEY_DELETED_NODES = "DbNodeServiceImpl.DeletedNodes"; + private boolean enableTimestampPropagation; public DbNodeServiceImpl() { @@ -131,6 +138,7 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl } /** +<<<<<<< .working * * @param policyBehaviourFilter component used to enable and disable behaviours */ @@ -140,6 +148,20 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl } /** +======= + * Set whether cm:auditable timestamps should be propagated to parent nodes + * where the parent-child relationship has been marked using propagateTimestamps. + * + * @param enableTimestampPropagation true to propagate timestamps to the parent + * node where appropriate + */ + public void setEnableTimestampPropagation(boolean enableTimestampPropagation) + { + this.enableTimestampPropagation = enableTimestampPropagation; + } + + /** +>>>>>>> .merge-right.r30520 * Performs a null-safe get of the node * * @param nodeRef the node to retrieve @@ -358,6 +380,9 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl Map propertiesAfter = nodeDAO.getNodeProperties(childNodePair.getFirst()); + // Propagate timestamps + propagateTimeStamps(childAssocRef); + // Invoke policy behaviour invokeOnCreateNode(childAssocRef); invokeOnCreateChildAssociation(childAssocRef, true); @@ -742,16 +767,19 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl invokeBeforeUpdateNode(nodeRef); // Set the type - nodeDAO.updateNode(nodePair.getFirst(), typeQName, null, false); + boolean updatedNode = nodeDAO.updateNode(nodePair.getFirst(), typeQName, null); // Add the default aspects and properties required for the given type. Existing values will not be overridden. - addAspectsAndProperties(nodePair, typeQName, null, null, null, null, false); - - // Index - nodeIndexer.indexUpdateNode(nodeRef); + boolean updatedProps = addAspectsAndProperties(nodePair, typeQName, null, null, null, null, false); // Invoke policies - invokeOnUpdateNode(nodeRef); + if (updatedNode || updatedProps) + { + // Invoke policies + invokeOnUpdateNode(nodeRef); + // Index + nodeIndexer.indexUpdateNode(nodeRef); + } } /** @@ -1011,9 +1039,11 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl deletePrimaryChildrenNotArchived(nodePair); // perform a normal deletion nodeDAO.deleteNode(nodeId); + + // Propagate timestamps + propagateTimeStamps(childAssocRef); // Invoke policy behaviours invokeOnDeleteNode(childAssocRef, nodeTypeQName, nodeAspectQNames, false); - // Index nodeIndexer.indexDeleteNode(childAssocRef); } @@ -1092,6 +1122,9 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl deletePrimaryChildrenNotArchived(childNodePair); // Delete the child nodeDAO.deleteNode(childNodeId); + + // Propagate timestamps + propagateTimeStamps(childParentAssocRef); invokeOnDeleteNode(childParentAssocRef, childNodeType, childNodeQNames, false); // lose interest in tracking this node ref @@ -2257,7 +2290,6 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl Long nodeToMoveId = nodeToMovePair.getFirst(); QName nodeToMoveTypeQName = nodeDAO.getNodeType(nodeToMoveId); - Set nodeToMoveAspectQNames = nodeDAO.getNodeAspects(nodeToMoveId); NodeRef oldNodeToMoveRef = nodeToMovePair.getSecond(); Long parentNodeId = parentNodePair.getFirst(); NodeRef parentNodeRef = parentNodePair.getSecond(); @@ -2318,6 +2350,11 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl // Call behaviours if (movingStore) { + // Propagate timestamps + propagateTimeStamps(oldParentAssocRef); + propagateTimeStamps(newParentAssocRef); + + Set nodeToMoveAspectQNames = nodeDAO.getNodeAspects(nodeToMoveId); // The Node changes NodeRefs, so this is really the deletion of the old node and creation // of a node in a new store as far as the clients are concerned. invokeOnDeleteNode(oldParentAssocRef, nodeToMoveTypeQName, nodeToMoveAspectQNames, true); @@ -2328,6 +2365,13 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl } else { + // Propagate timestamps (watch out for moves within the same folder) + if (!oldParentAssocRef.getParentRef().equals(newParentAssocRef.getParentRef())) + { + propagateTimeStamps(oldParentAssocRef); + propagateTimeStamps(newParentAssocRef); + } + invokeOnCreateChildAssociation(newParentAssocRef, false); invokeOnDeleteChildAssociation(oldParentAssocRef); invokeOnMoveNode(oldParentAssocRef, newParentAssocRef); @@ -2386,8 +2430,7 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl QName childNodeTypeQName = nodeDAO.getNodeType(childNodeId); Set childNodeAspectQNames = nodeDAO.getNodeAspects(childNodeId); Pair oldParentAssocPair = nodeDAO.getPrimaryParentAssoc(childNodeId); - Pair newParentAssocPair = oldParentAssocPair; - ChildAssociationRef newParentAssocRef = newParentAssocPair.getSecond(); + ChildAssociationRef oldParentAssocRef = oldParentAssocPair.getSecond(); // remove the deleted node from the list of new nodes untrackNewNodeRef(childNodeRef); @@ -2398,9 +2441,9 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl // Fire node policies. This ensures that each node in the hierarchy gets a notification fired. invokeBeforeDeleteNode(childNodeRef); invokeBeforeCreateNode( - newParentAssocRef.getParentRef(), - newParentAssocRef.getTypeQName(), - newParentAssocRef.getQName(), + oldParentAssocPair.getSecond().getParentRef(), + oldParentAssocPair.getSecond().getTypeQName(), + oldParentAssocPair.getSecond().getQName(), childNodeTypeQName); // Move the node as this gives back the primary parent association Pair, Pair> moveResult; @@ -2413,13 +2456,18 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl deleteNode(e.getNodePair().getSecond()); moveResult = nodeDAO.moveNode(childNodeId, nodeId, null,null); } - newParentAssocPair = moveResult.getFirst(); + // Move the node as this gives back the primary parent association + Pair newParentAssocPair = moveResult.getFirst(); Pair newChildNodePair = moveResult.getSecond(); + ChildAssociationRef newParentAssocRef = newParentAssocPair.getSecond(); // Index nodeIndexer.indexCreateNode(newParentAssocPair.getSecond()); + // Propagate timestamps + propagateTimeStamps(oldParentAssocRef); + propagateTimeStamps(newParentAssocRef); // Fire node policies. This ensures that each node in the hierarchy gets a notification fired. - invokeOnDeleteNode(oldParentAssocPair.getSecond(), childNodeTypeQName, childNodeAspectQNames, true); - invokeOnCreateNode(newParentAssocPair.getSecond()); + invokeOnDeleteNode(oldParentAssocRef, childNodeTypeQName, childNodeAspectQNames, true); + invokeOnCreateNode(newParentAssocRef); // Cascade pullNodeChildrenToSameStore(newChildNodePair); } @@ -2471,4 +2519,177 @@ public class DbNodeServiceImpl extends AbstractNodeServiceImpl return true; } } + + /** + * Propagate, if necessary, a cm:modified timestamp change to the parent of the + * given association. The parent node has to be cm:auditable and the association + * has to be marked for propagation as well. + * + * @param assocRef the association to propagate along + */ + private void propagateTimeStamps(ChildAssociationRef assocRef) + { + if (!enableTimestampPropagation) + { + return; // Bypassed on a system-wide basis + } + // First check if the association type warrants propagation in the first place + AssociationDefinition assocDef = dictionaryService.getAssociation(assocRef.getTypeQName()); + if (assocDef == null || !assocDef.isChild()) + { + return; + } + ChildAssociationDefinition childAssocDef = (ChildAssociationDefinition) assocDef; + if (!childAssocDef.getPropagateTimestamps()) + { + return; + } + // The dictionary says propagate. Now get the parent node and prompt the touch. + NodeRef parentNodeRef = assocRef.getParentRef(); + Pair parentNodePair = getNodePairNotNull(parentNodeRef); + Long parentNodeId = parentNodePair.getFirst(); + // If we have already modified a particular parent node in the current txn, + // it is not necessary to start a new transaction to tweak the cm:modified date. + // But if the parent node was NOT touched, then doing so in this transaction would + // create excessive concurrency and retries; in latter case we defer to a small, + // post-commit isolated transaction. + if (TransactionalResourceHelper.getSet(KEY_AUDITABLE_PROPAGATION_PRE).contains(parentNodeId)) + { + // It is already registered in the current transaction. + return; + } + if (nodeDAO.isInCurrentTxn(parentNodeId)) + { + // The parent and child are in the same transaction + TransactionalResourceHelper.getSet(KEY_AUDITABLE_PROPAGATION_PRE).add(parentNodeId); + // Make sure that it is not processed after the transaction + TransactionalResourceHelper.getSet(KEY_AUDITABLE_PROPAGATION_POST).remove(parentNodeId); + } + else + { + TransactionalResourceHelper.getSet(KEY_AUDITABLE_PROPAGATION_POST).add(parentNodeId); + } + + // Bind a listener for post-transaction manipulation + AlfrescoTransactionSupport.bindListener(auditableTransactionListener); + } + + private static final String KEY_AUDITABLE_PROPAGATION_PRE = "node.auditable.propagation.pre"; + private static final String KEY_AUDITABLE_PROPAGATION_POST = "node.auditable.propagation.post"; + private AuditableTransactionListener auditableTransactionListener = new AuditableTransactionListener(); + /** + * Wrapper to set the cm:modified time on individual nodes. + * + * @author Derek Hulley + * @since 3.4.6 + */ + private class AuditableTransactionListener extends TransactionListenerAdapter + { + @Override + public void beforeCommit(boolean readOnly) + { + // An error in prior code if it's read only + if (readOnly) + { + throw new IllegalStateException("Attempting to modify parent cm:modified in read-only txn."); + } + + Set parentNodeIds = TransactionalResourceHelper.getSet(KEY_AUDITABLE_PROPAGATION_PRE); + if (parentNodeIds.size() == 0) + { + return; + } + // Process parents, but use the current txn + Date modifiedDate = new Date(); + process(parentNodeIds, modifiedDate, true); + } + + @Override + public void afterCommit() + { + Set parentNodeIds = TransactionalResourceHelper.getSet(KEY_AUDITABLE_PROPAGATION_POST); + if (parentNodeIds.size() == 0) + { + return; + } + Date modifiedDate = new Date(); + process(parentNodeIds, modifiedDate, false); + } + + /** + * @param parentNodeIds the parent node IDs that need to be touched for cm:modified + * @param modifiedDate the date to set + * @param useCurrentTxn true to use the current transaction + */ + private void process(final Set parentNodeIds, Date modifiedDate, boolean useCurrentTxn) + { + // Walk through the IDs + for (Long parentNodeId: parentNodeIds) + { + processSingle(parentNodeId, modifiedDate, useCurrentTxn); + } + } + + /** + * Touch a single node in a new, writable txn + * + * @param parentNodeId the parent node to touch + * @param modifiedDate the date to set + * @param useCurrentTxn true to use the current transaction + */ + private void processSingle(final Long parentNodeId, final Date modifiedDate, boolean useCurrentTxn) + { + RetryingTransactionHelper txnHelper = transactionService.getRetryingTransactionHelper(); + txnHelper.setMaxRetries(1); + RetryingTransactionCallback callback = new RetryingTransactionCallback() + { + @Override + public Void execute() throws Throwable + { + Pair parentNodePair = nodeDAO.getNodePair(parentNodeId); + if (parentNodePair == null) + { + return null; // Parent has gone away + } + else if (!nodeDAO.hasNodeAspect(parentNodeId, ContentModel.ASPECT_AUDITABLE)) + { + return null; // Not auditable + } + NodeRef parentNodeRef = parentNodePair.getSecond(); + + // Invoke policy behaviour + invokeBeforeUpdateNode(parentNodeRef); + + // Touch the node; it is cm:auditable + boolean changed = nodeDAO.setModifiedDate(parentNodeId, modifiedDate); + + if (changed) + { + // Invoke policy behaviour + invokeOnUpdateNode(parentNodeRef); + // Index + nodeIndexer.indexUpdateNode(parentNodeRef); + } + + return null; + } + }; + try + { + txnHelper.doInTransaction(callback, false, !useCurrentTxn); + if (logger.isDebugEnabled()) + { + logger.debug( + "Touched cm:modified date for node " + parentNodeId + + " (" + modifiedDate + ")" + + (useCurrentTxn ? " in txn " : " in new txn ") + + nodeDAO.getCurrentTransactionId()); + } + } + catch (Throwable e) + { + logger.info("Failed to update cm:modified date for node: " + parentNodeId); + } + } + } } diff --git a/source/java/org/alfresco/repo/solr/SOLRTrackingComponentTest.java b/source/java/org/alfresco/repo/solr/SOLRTrackingComponentTest.java index bfe0ba59d6..f7623032a6 100644 --- a/source/java/org/alfresco/repo/solr/SOLRTrackingComponentTest.java +++ b/source/java/org/alfresco/repo/solr/SOLRTrackingComponentTest.java @@ -135,7 +135,7 @@ public class SOLRTrackingComponentTest extends TestCase assertEquals("ACL count should have matched", totalAcls, totalAclsCheck); } - public void DISABLED_testGetNodeMetaData() + public void testGetNodeMetaData() { long startTime = System.currentTimeMillis(); @@ -157,7 +157,7 @@ public class SOLRTrackingComponentTest extends TestCase getNodeMetaData(nodeMetaDataParams, null, st); } - public void DISABLED_testGetNodeMetaData100Nodes() + public void testGetNodeMetaData100Nodes() { long startTime = System.currentTimeMillis(); @@ -186,7 +186,7 @@ public class SOLRTrackingComponentTest extends TestCase // assertEquals("Unxpected number of nodes", 3, bt.getSuccessCount()); } - public void DISABLED_testNodeMetaDataManyNodes() throws Exception + public void testNodeMetaDataManyNodes() throws Exception { long fromCommitTime = System.currentTimeMillis(); @@ -243,7 +243,7 @@ public class SOLRTrackingComponentTest extends TestCase getNodeMetaData(nodeMetaDataParams, null, st); } - public void DISABLED_testNodeMetaDataCache() throws Exception + public void testNodeMetaDataCache() throws Exception { long fromCommitTime = System.currentTimeMillis(); @@ -273,7 +273,7 @@ public class SOLRTrackingComponentTest extends TestCase getNodeMetaData(nodeMetaDataParams, filter, st); } - public void DISABLED_testNodeMetaDataNullPropertyValue() throws Exception + public void testNodeMetaDataNullPropertyValue() throws Exception { long fromCommitTime = System.currentTimeMillis(); @@ -295,7 +295,7 @@ public class SOLRTrackingComponentTest extends TestCase getNodeMetaData(nodeMetaDataParams, null, st); } - public void DISABLED_testFilters() + public void testFilters() { long startTime = System.currentTimeMillis();