Fix ALF-1006 - problems with tag scope on share Wiki pages

The tagging service and checkout/checkin weren't quite playing nicely
 together due to the way the copy policy was being triggered. A new
 "before copy" policy was added, to complement the "on copy complete"
 policy, and the tagging service updated to use this.
A tagging test which calls checkout/checkin was added that verifies
 that the tag scopes now behave correctly under this action.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@19692 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/copy/CopyServiceImpl.java

@@ -106,6 +106,7 @@ public class CopyServiceImpl implements CopyService
     /** Policy delegates */
     private ClassPolicyDelegate<CopyServicePolicies.OnCopyNodePolicy> onCopyNodeDelegate;
     private ClassPolicyDelegate<CopyServicePolicies.OnCopyCompletePolicy> onCopyCompleteDelegate;
+    private ClassPolicyDelegate<CopyServicePolicies.BeforeCopyPolicy> beforeCopyDelegate;
     
     /**
      * Set the node service
@@ -195,6 +196,7 @@ public class CopyServiceImpl implements CopyService
         // Register the policies
         onCopyNodeDelegate = policyComponent.registerClassPolicy(CopyServicePolicies.OnCopyNodePolicy.class);
         onCopyCompleteDelegate = policyComponent.registerClassPolicy(CopyServicePolicies.OnCopyCompletePolicy.class);
+        beforeCopyDelegate = policyComponent.registerClassPolicy(CopyServicePolicies.BeforeCopyPolicy.class);
         
         // Register policy behaviours
         this.policyComponent.bindClassBehaviour(
@@ -346,6 +348,9 @@ public class CopyServiceImpl implements CopyService
         // Remove the name property from the source properties to avoid having it copied
         sourceNodeProperties.remove(ContentModel.PROP_NAME);
         
+        // invoke the before copy policy
+        invokeBeforeCopy(sourceNodeRef, targetNodeRef);
+        
         // Copy 
         copyProperties(copyDetails, targetNodeRef, sourceNodeTypeQName, callbacks);
         copyAspects(copyDetails, targetNodeRef, Collections.<QName>emptySet(), callbacks);
@@ -524,6 +529,9 @@ public class CopyServiceImpl implements CopyService
         // Save the mapping for later
         copiesByOriginal.put(sourceNodeRef, targetNodeRef);
         copies.add(targetNodeRef);
+        
+        // We now have a node, so fire the BeforeCopyPolicy
+        invokeBeforeCopy(sourceNodeRef, targetNodeRef);
 
         // Work out which aspects still need copying.  The source aspects less the default aspects
         // will give this set.
@@ -637,6 +645,40 @@ public class CopyServiceImpl implements CopyService
         return copyProperties;
     }
     
+    /**
+     * Invokes the before copy policy for the node reference provided
+     * 
+     * @param sourceNodeRef         the source node reference
+     * @param targetNodeRef         the destination node reference
+     */
+    private void invokeBeforeCopy(
+            NodeRef sourceNodeRef, 
+            NodeRef targetNodeRef) 
+    {
+        // By Type
+        QName targetClassRef = internalNodeService.getType(targetNodeRef);     
+        invokeBeforeCopy(targetClassRef, sourceNodeRef, targetNodeRef);
+        
+        // And by Aspect
+        Set<QName> targetAspects = this.nodeService.getAspects(targetNodeRef);
+        for (QName targetAspect : targetAspects) 
+        {
+            invokeBeforeCopy(targetAspect, sourceNodeRef, targetNodeRef);
+        }
+    }
+    
+    private void invokeBeforeCopy(
+          QName typeQName, 
+          NodeRef sourceNodeRef, 
+          NodeRef targetNodeRef)
+    {
+        Collection<CopyServicePolicies.BeforeCopyPolicy> policies = beforeCopyDelegate.getList(typeQName);
+        for (CopyServicePolicies.BeforeCopyPolicy policy : policies) 
+        {
+            policy.beforeCopy(typeQName, sourceNodeRef, targetNodeRef);
+        }
+    }
+    
     /**
      * Invokes the copy complete policy for the node reference provided
      * 
@@ -650,6 +692,7 @@ public class CopyServiceImpl implements CopyService
             boolean copyToNewNode,
             Map<NodeRef, NodeRef> copiedNodeRefs)
     {
+        // By Type
         QName sourceClassRef = internalNodeService.getType(sourceNodeRef);     
         invokeCopyComplete(sourceClassRef, sourceNodeRef, targetNodeRef, copyToNewNode, copiedNodeRefs);
         

source/java/org/alfresco/repo/copy/CopyServicePolicies.java

@@ -119,6 +119,28 @@ public interface CopyServicePolicies
         static Arg ARG_1 = Arg.KEY;
     }
     
+    /**
+     * The intermediate copy callback, which occurs once it has been decided which properties
+     *  and aspects will be copied, but before the copy occurs.
+     * This allows you to remove cached data based on the destination node, before it is
+     *  overwritten. You are unable to make changes to what gets copied though, that must
+     *  be done earlier via a {@link OnCopyNodePolicy}.
+     */
+    public interface BeforeCopyPolicy extends ClassPolicy
+    {
+        public static final QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "beforeCopy");
+        
+        /**
+         * @param classRef          the type of the node that will be copied
+         * @param sourceNodeRef     the original node
+         * @param targetNodeRef     the destination node
+         */
+        public void beforeCopy(
+                QName classRef,
+                NodeRef sourceNodeRef,
+                NodeRef targetNodeRef);
+    }
+    
     /**
      * Final callback after the copy (including any cascading) has been completed.  This should
      * be used where post-copy manipulation of nodes is required in order to enforce adherence
@@ -133,7 +155,7 @@ public interface CopyServicePolicies
         
         /**
          * @param classRef          the type of the node that was copied
-         * @param sourceNodeRef     the origional node
+         * @param sourceNodeRef     the original node
          * @param targetNodeRef     the destination node
          * @param copyMap           a map containing all the nodes that have been created during the copy
          */
@@ -143,8 +165,5 @@ public interface CopyServicePolicies
                 NodeRef targetNodeRef,
                 boolean copyToNewNode,
                 Map<NodeRef, NodeRef> copyMap);
-        
-        static Arg ARG_0 = Arg.KEY;
-        static Arg ARG_1 = Arg.KEY; 
     }
 }

source/java/org/alfresco/repo/tagging/TaggingServiceImpl.java

@@ -33,6 +33,7 @@ import java.util.Map;
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.copy.CopyServicePolicies;
+import org.alfresco.repo.copy.CopyServicePolicies.BeforeCopyPolicy;
 import org.alfresco.repo.copy.CopyServicePolicies.OnCopyCompletePolicy;
 import org.alfresco.repo.node.NodeServicePolicies;
 import org.alfresco.repo.node.NodeServicePolicies.OnCreateNodePolicy;
@@ -71,7 +72,8 @@ public class TaggingServiceImpl implements TaggingService,
                                            TransactionListener,
                                            NodeServicePolicies.BeforeDeleteNodePolicy,
                                            NodeServicePolicies.OnMoveNodePolicy,
-                                           CopyServicePolicies.OnCopyCompletePolicy
+                                           CopyServicePolicies.OnCopyCompletePolicy,
+                                           CopyServicePolicies.BeforeCopyPolicy
 {
     /** Node service */
     private NodeService nodeService;
@@ -188,14 +190,18 @@ public class TaggingServiceImpl implements TaggingService,
                 updateTagBehaviour);
         
         // We need to know when you move or copy nodes
-        this.policyComponent.bindClassBehaviour(
-              OnCopyCompletePolicy.QNAME,
-              ContentModel.ASPECT_TAGGABLE, 
-              new JavaBehaviour(this, "onCopyComplete", NotificationFrequency.EVERY_EVENT));
         this.policyComponent.bindClassBehaviour(
               OnMoveNodePolicy.QNAME,
               ContentModel.ASPECT_TAGGABLE, 
               new JavaBehaviour(this, "onMoveNode", NotificationFrequency.EVERY_EVENT));
+        this.policyComponent.bindClassBehaviour(
+              BeforeCopyPolicy.QNAME,
+              ContentModel.ASPECT_TAGGABLE, 
+              new JavaBehaviour(this, "beforeCopy", NotificationFrequency.EVERY_EVENT));
+        this.policyComponent.bindClassBehaviour(
+              OnCopyCompletePolicy.QNAME,
+              ContentModel.ASPECT_TAGGABLE, 
+              new JavaBehaviour(this, "onCopyComplete", NotificationFrequency.EVERY_EVENT));
     }
     
     /**
@@ -230,22 +236,40 @@ public class TaggingServiceImpl implements TaggingService,
     public void beforeDeleteNode(NodeRef nodeRef)
     {
        if (this.nodeService.exists(nodeRef) == true &&          
-           this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_TAGGABLE) == true &&
+           this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_TAGGABLE) == true)
-           this.nodeService.hasAspect(nodeRef, ContentModel.ASPECT_WORKING_COPY) == false)
        {
            updateAllScopeTags(nodeRef, Boolean.FALSE);
        }
     }
     
-   public void onCopyComplete(QName classRef, NodeRef sourceNodeRef,
+    /**
+     * Fired once per node, before a copy overrides one node
+     *  (which is possibly newly created) with the contents
+     *  of another one.
+     * We should remove any tags from the scope, as they'll
+     *  shortly be overwritten.
+     */
+    public void beforeCopy(QName classRef, NodeRef sourceNodeRef,
+          NodeRef targetNodeRef) {
+       if(this.nodeService.hasAspect(targetNodeRef, ContentModel.ASPECT_TAGGABLE)) {
+          updateAllScopeTags(targetNodeRef, Boolean.FALSE);
+       }
+    }
+
+    /**
+     * Fired once per node that was copied, after the copy
+     *  has completed. 
+     * We need to add in all the tags to the scope.
+     */
+    public void onCopyComplete(QName classRef, NodeRef sourceNodeRef,
          NodeRef targetNodeRef, boolean copyToNewNode,
          Map<NodeRef, NodeRef> copyMap) {
       if(this.nodeService.hasAspect(targetNodeRef, ContentModel.ASPECT_TAGGABLE)) {
          updateAllScopeTags(targetNodeRef, Boolean.TRUE);
       }
-   }
+    }
 
-   public void onMoveNode(ChildAssociationRef oldChildAssocRef,
+    public void onMoveNode(ChildAssociationRef oldChildAssocRef,
          ChildAssociationRef newChildAssocRef) {
       NodeRef oldRef = oldChildAssocRef.getChildRef();
       NodeRef oldParent = oldChildAssocRef.getParentRef();

source/java/org/alfresco/repo/tagging/TaggingServiceImplTest.java

@@ -26,6 +26,7 @@ import java.util.Map;
 
 import javax.transaction.UserTransaction;
 
+import org.alfresco.cmis.mapping.CheckinCommentProperty;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.action.AsynchronousActionExecutionQueuePolicies;
 import org.alfresco.repo.jscript.ClasspathScriptLocation;
@@ -35,6 +36,7 @@ import org.alfresco.repo.policy.PolicyComponent;
 import org.alfresco.repo.security.authentication.AuthenticationComponent;
 import org.alfresco.service.cmr.action.Action;
 import org.alfresco.service.cmr.action.ActionService;
+import org.alfresco.service.cmr.coci.CheckOutCheckInService;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.ContentService;
 import org.alfresco.service.cmr.repository.CopyService;
@@ -64,6 +66,7 @@ public class TaggingServiceImplTest extends BaseAlfrescoSpringTest
     /** Services */
     private TaggingService taggingService;
     private CopyService copyService;
+    private CheckOutCheckInService checkOutCheckInService;
     private ScriptService scriptService;
     private PolicyComponent policyComponent;
     
@@ -96,6 +99,7 @@ public class TaggingServiceImplTest extends BaseAlfrescoSpringTest
         this.nodeService = (NodeService) this.applicationContext.getBean("NodeService");
         this.copyService = (CopyService) this.applicationContext.getBean("CopyService");
         this.contentService = (ContentService) this.applicationContext.getBean("ContentService");
+        this.checkOutCheckInService = (CheckOutCheckInService) this.applicationContext.getBean("checkOutCheckInService");
         this.authenticationService = (MutableAuthenticationService) this.applicationContext.getBean("authenticationService");
         this.actionService = (ActionService)this.applicationContext.getBean("ActionService");
         this.transactionService = (TransactionService)this.applicationContext.getBean("transactionComponent");
@@ -635,6 +639,45 @@ public class TaggingServiceImplTest extends BaseAlfrescoSpringTest
        //  otherwise later checks will fail for really odd reasons
        assertEquals(1, nodeService.getChildAssocs(container1).size());
        assertEquals(1, nodeService.getChildAssocs(taggedFolder).size());
+       
+       
+       // Check out the node
+       // Tags should be doubled up. (We don't care about ContentModel.ASPECT_WORKING_COPY
+       //  because it isn't applied at suitable times to take not of)
+       UserTransaction tx = this.transactionService.getUserTransaction();
+       tx.begin();
+       NodeRef checkedOutDoc = checkOutCheckInService.checkout(taggedDoc);
+       tx.commit();
+       
+       waitForActionExecution();
+       assertEquals(3, taggingService.findTagScope(container1).getTags().size());
+       assertEquals(0, taggingService.findTagScope(container2).getTags().size());
+       
+       assertEquals(3, taggingService.findTagScope(container1).getTag("foo1").getCount());
+       assertEquals(2, taggingService.findTagScope(container1).getTag("foo2").getCount());
+       assertEquals(1, taggingService.findTagScope(container1).getTag("foo3").getCount());
+       
+       assertEquals(1, nodeService.getChildAssocs(container1).size());
+       assertEquals(2, nodeService.getChildAssocs(taggedFolder).size());
+       
+       
+       // And check it back in again
+       // Tags should go back to how they were
+       tx = this.transactionService.getUserTransaction();
+       tx.begin();
+       checkOutCheckInService.checkin(checkedOutDoc, null);
+       tx.commit();
+       
+       waitForActionExecution();
+       assertEquals(3, taggingService.findTagScope(container1).getTags().size());
+       assertEquals(0, taggingService.findTagScope(container2).getTags().size());
+       
+       assertEquals(2, taggingService.findTagScope(container1).getTag("foo1").getCount());
+       assertEquals(1, taggingService.findTagScope(container1).getTag("foo2").getCount());
+       assertEquals(1, taggingService.findTagScope(container1).getTag("foo3").getCount());
+       
+       assertEquals(1, nodeService.getChildAssocs(container1).size());
+       assertEquals(1, nodeService.getChildAssocs(taggedFolder).size());
 
        
        // Do a node->node copy of the document onto the other container