diff --git a/source/java/org/alfresco/repo/audit/AuditableAspectTest.java b/source/java/org/alfresco/repo/audit/AuditableAspectTest.java index fa04a984ac..82479054aa 100644 --- a/source/java/org/alfresco/repo/audit/AuditableAspectTest.java +++ b/source/java/org/alfresco/repo/audit/AuditableAspectTest.java @@ -24,14 +24,20 @@ import java.util.HashMap; import java.util.Map; import java.util.Set; +import junit.framework.TestCase; + import org.alfresco.model.ContentModel; +import org.alfresco.repo.policy.BehaviourFilter; +import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; +import org.alfresco.service.ServiceRegistry; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.namespace.QName; +import org.alfresco.service.transaction.TransactionService; import org.alfresco.util.ApplicationContextHelper; -import org.alfresco.util.BaseSpringTest; import org.alfresco.util.debug.NodeStoreInspector; import org.springframework.context.ApplicationContext; @@ -39,37 +45,41 @@ import org.springframework.context.ApplicationContext; * Checks that the behaviour of the {@link org.alfresco.repo.audit.AuditableAspect auditable aspect} * is correct. * - * @author Roy Wetherall + * @author Derek Hulley */ -public class AuditableAspectTest extends BaseSpringTest +public class AuditableAspectTest extends TestCase { private static final ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(); - /* - * Services used by the tests - */ + private TransactionService transactionService; private NodeService nodeService; + private BehaviourFilter behaviourFilter; - /** - * Data used by the tests - */ private StoreRef storeRef; private NodeRef rootNodeRef; - /** - * On setup in transaction implementation - */ @Override - protected void onSetUpInTransaction() throws Exception + public void setUp() throws Exception { + ServiceRegistry serviceRegistry = (ServiceRegistry) ctx.getBean(ServiceRegistry.SERVICE_REGISTRY); // Set the services - this.nodeService = (NodeService)ctx.getBean("dbNodeService"); + this.transactionService = serviceRegistry.getTransactionService(); + this.nodeService = serviceRegistry.getNodeService(); + this.behaviourFilter = (BehaviourFilter) ctx.getBean("policyBehaviourFilter"); + + AuthenticationUtil.setRunAsUserSystem(); // Create the store and get the root node reference this.storeRef = this.nodeService.createStore(StoreRef.PROTOCOL_WORKSPACE, "Test_" + System.currentTimeMillis()); this.rootNodeRef = this.nodeService.getRootNode(storeRef); } + @Override + protected void tearDown() throws Exception + { + AuthenticationUtil.clearCurrentSecurityContext(); + } + public void testAudit() { // Create a folder @@ -93,6 +103,7 @@ public class AuditableAspectTest extends BaseSpringTest personProps.put(ContentModel.PROP_HOMEFOLDER, rootNodeRef); personProps.put(ContentModel.PROP_FIRSTNAME, "test first name"); personProps.put(ContentModel.PROP_LASTNAME, "test last name"); + personProps.put(ContentModel.PROP_SIZE_CURRENT, 0); ChildAssociationRef childAssocRef = nodeService.createNode( rootNodeRef, @@ -124,6 +135,7 @@ public class AuditableAspectTest extends BaseSpringTest personProps.put(ContentModel.PROP_HOMEFOLDER, rootNodeRef); personProps.put(ContentModel.PROP_FIRSTNAME, "test first name"); personProps.put(ContentModel.PROP_LASTNAME, "test last name"); + personProps.put(ContentModel.PROP_SIZE_CURRENT, 0); ChildAssociationRef childAssocRef = nodeService.createNode( rootNodeRef, @@ -160,6 +172,7 @@ public class AuditableAspectTest extends BaseSpringTest personProps.put(ContentModel.PROP_HOMEFOLDER, rootNodeRef); personProps.put(ContentModel.PROP_FIRSTNAME, "test first name "); personProps.put(ContentModel.PROP_LASTNAME, "test last name"); + personProps.put(ContentModel.PROP_SIZE_CURRENT, 0); long t1 = System.currentTimeMillis(); this.wait(100); // Needed for system clock inaccuracies @@ -224,8 +237,9 @@ public class AuditableAspectTest extends BaseSpringTest personProps.put(ContentModel.PROP_HOMEFOLDER, rootNodeRef); personProps.put(ContentModel.PROP_FIRSTNAME, "test first name "); personProps.put(ContentModel.PROP_LASTNAME, "test last name"); + personProps.put(ContentModel.PROP_SIZE_CURRENT, 0); // Add some auditable properties - Map auditableProps = new HashMap(); + final Map auditableProps = new HashMap(); auditableProps.put(ContentModel.PROP_CREATED, new Date(0L)); auditableProps.put(ContentModel.PROP_CREATOR, "ZeroPerson"); auditableProps.put(ContentModel.PROP_MODIFIED, new Date(1L)); @@ -239,8 +253,27 @@ public class AuditableAspectTest extends BaseSpringTest QName.createQName("{test}testperson"), ContentModel.TYPE_PERSON, personProps); - NodeRef nodeRef = childAssocRef.getChildRef(); + final NodeRef nodeRef = childAssocRef.getChildRef(); + // Check + assertAuditableProperties(nodeRef, auditableProps); + // Now modify the node so that the auditable values advance + nodeService.setProperty(nodeRef, ContentModel.PROP_FIRSTNAME, "TEST-FIRST-NAME-" + System.currentTimeMillis()); + + RetryingTransactionCallback setAuditableCallback = new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + behaviourFilter.disableBehaviour(ContentModel.ASPECT_AUDITABLE); // Lasts for txn + // Set the auditable properties explicitly + auditableProps.put(ContentModel.PROP_MODIFIER, "ThisUser"); + nodeService.addProperties(nodeRef, auditableProps); + // Done + return null; + } + }; + transactionService.getRetryingTransactionHelper().doInTransaction(setAuditableCallback); + // Check assertAuditableProperties(nodeRef, auditableProps); } @@ -251,6 +284,8 @@ public class AuditableAspectTest extends BaseSpringTest private void assertAuditableProperties(NodeRef nodeRef, Map checkProps) { + assertTrue("Auditable aspect not present", nodeService.hasAspect(nodeRef, ContentModel.ASPECT_AUDITABLE)); + Map props = nodeService.getProperties(nodeRef); assertNotNull(props.get(ContentModel.PROP_CREATED)); assertNotNull(props.get(ContentModel.PROP_MODIFIED)); diff --git a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java index 487b7ebf5c..45e6ca5178 100644 --- a/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/AbstractNodeDAOImpl.java @@ -1154,9 +1154,20 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO /** * Updates the node's transaction and cm:auditable properties only. * - * @see #updateNodeImpl(NodeEntity, NodeUpdateEntity) + * @see #touchNodeImpl(Long, AuditablePropertiesEntity) */ private void touchNodeImpl(Long nodeId) + { + touchNodeImpl(nodeId, null); + } + /** + * Updates the node's transaction and cm:auditable properties only. + * + * @param auditableProps optionally override the cm:auditable values + * + * @see #updateNodeImpl(NodeEntity, NodeUpdateEntity) + */ + private void touchNodeImpl(Long nodeId, AuditablePropertiesEntity auditableProps) { Node node = null; try @@ -1171,6 +1182,10 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } NodeUpdateEntity nodeUpdate = new NodeUpdateEntity(); nodeUpdate.setId(nodeId); + if (auditableProps != null) + { + nodeUpdate.setAuditableProperties(auditableProps); + } updateNodeImpl(node, nodeUpdate); } @@ -1247,11 +1262,8 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO nodeUpdate.setAuditableProperties(auditableProps); nodeUpdate.setUpdateAuditableProperties(updateAuditableProperties); } - else + else if (nodeUpdate.getAuditableProperties() == null) { - // else: The auditable aspect is manual, so we expect the client code to have done - // the necessary updates on the 'nodeUpdate' - // cache the explicit setting of auditable properties when creating node (note: auditable aspect is not yet present) AuditablePropertiesEntity auditableProps = oldNode.getAuditableProperties(); if (auditableProps != null) @@ -1259,6 +1271,11 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO nodeUpdate.setAuditableProperties(auditableProps); } } + // else + // { + // // SAIL-390: NodeDAO: Allow cm:auditable to be set + // // The nodeUpdate had auditable properties set, so we just use that directly + // } } else { @@ -1577,6 +1594,15 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO Node node = getNodeNotNull(nodeId); // Copy inbound values newProps = new HashMap(newProps); + + // Copy cm:auditable + AuditablePropertiesEntity auditableProps = null; + if (!policyBehaviourFilter.isEnabled(node.getNodeRef(), ContentModel.ASPECT_AUDITABLE)) + { + auditableProps = new AuditablePropertiesEntity(); + auditableProps.setAuditValues(null, null, newProps); + } + // Remove cm:auditable newProps.keySet().removeAll(AuditablePropertiesEntity.getAuditablePropertyQNames()); // Remove sys:referenceable @@ -1694,12 +1720,6 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } } - // Shortcut if there are no diffs - if (propsToDelete.isEmpty() && propsToAdd.isEmpty()) - { - return false; - } - // Remove by key List propKeysToDeleteList = new ArrayList(propsToDelete.keySet()); deleteNodeProperties(nodeId, propKeysToDeleteList); @@ -1738,8 +1758,11 @@ public abstract class AbstractNodeDAOImpl implements NodeDAO, BatchingDAO } // Update cache setNodePropertiesCached(nodeId, propsToCache); - // Touch to bring into current txn - touchNodeImpl(nodeId); + } + // Touch to bring into current transaction + if (updated || auditableProps != null) + { + touchNodeImpl(nodeId, auditableProps); } // Done diff --git a/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java b/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java index a8375370c0..a4cf50d187 100644 --- a/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java +++ b/source/java/org/alfresco/repo/domain/node/ibatis/NodeDAOImpl.java @@ -502,6 +502,11 @@ public class NodeDAOImpl extends AbstractNodeDAOImpl @Override protected void insertNodeProperties(Long nodeId, Map persistableProps) { + if (persistableProps.isEmpty()) + { + return; + } + List rows = makePersistentRows(nodeId, persistableProps); startBatch();