MNT-9318: CLONE - It's impossible to update comment for the content after version revert

Merged V4.1-BUG-FIX (4.1.7) to HEAD (4.2)
    55265: Merged DEV to V4.1-BUG-FIX (4.1.7)
       55184: MNT-6334: It's impossible to update comment for the content after version revert
        - Restore association if it was removed in older document version.
        - Extend unit test.
       55221: MNT-6334: It's impossible to update comment for the content after version revert
        - Make corrections to the code.
        - Extend unit test. 


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

config/alfresco/comment-services-context.xml

@@ -89,6 +89,9 @@
         <property name="nodeService">
             <ref bean="NodeService" />
         </property>
+        <property name="dbNodeService">
+            <ref bean="dbNodeService" />
+        </property>		
         <property name="fileFolderService">
             <ref bean="fileFolderService" />
         </property>

source/java/org/alfresco/repo/forum/DiscussableAspect.java

@@ -39,12 +39,17 @@ import org.alfresco.repo.node.NodeServicePolicies;
 import org.alfresco.repo.policy.JavaBehaviour;
 import org.alfresco.repo.policy.PolicyComponent;
 import org.alfresco.repo.transaction.TransactionalResourceHelper;
+import org.alfresco.repo.version.VersionServicePolicies;
+import org.alfresco.repo.version.VersionServicePolicies.AfterVersionRevertPolicy;
+import org.alfresco.repo.version.common.VersionUtil;
 import org.alfresco.service.cmr.model.FileExistsException;
 import org.alfresco.service.cmr.model.FileFolderService;
 import org.alfresco.service.cmr.model.FileNotFoundException;
 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.cmr.version.Version;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.namespace.RegexQNamePattern;
@@ -61,7 +66,8 @@ import org.springframework.dao.ConcurrencyFailureException;
 public class DiscussableAspect implements
             NodeServicePolicies.OnAddAspectPolicy,
             CopyServicePolicies.OnCopyNodePolicy,
-            CopyServicePolicies.OnCopyCompletePolicy
+            CopyServicePolicies.OnCopyCompletePolicy,
+            VersionServicePolicies.AfterVersionRevertPolicy
 {
     private static final String KEY_WORKING_COPIES = DiscussableAspect.class.getName() + ".WorkingCopies";
     
@@ -69,6 +75,7 @@ public class DiscussableAspect implements
     
     private PolicyComponent policyComponent;
     private NodeService nodeService;
+    private NodeService dbNodeService;
     private FileFolderService fileFolderService;
     
     public void setPolicyComponent(PolicyComponent policyComponent) 
@@ -81,6 +88,11 @@ public class DiscussableAspect implements
         this.nodeService = nodeService;
     }
     
+    public void setDbNodeService(NodeService dbNodeService)
+    {
+        this.dbNodeService = dbNodeService;
+    }
+
     public final void setFileFolderService(FileFolderService fileFolderService)
     {
         this.fileFolderService = fileFolderService;
@@ -104,6 +116,10 @@ public class DiscussableAspect implements
                 QName.createQName(NamespaceService.ALFRESCO_URI, "onCopyComplete"),
                 ForumModel.ASPECT_DISCUSSABLE,
                 new JavaBehaviour(this, "onCopyComplete"));
+        this.policyComponent.bindClassBehaviour(
+                AfterVersionRevertPolicy.QNAME,
+                ContentModel.ASPECT_VERSIONABLE,
+                new JavaBehaviour(this, "afterVersionRevert"));           
     }
     
     /**
@@ -370,4 +386,52 @@ public class DiscussableAspect implements
             }
         }
     }
+    
+    @Override
+    public void afterVersionRevert(NodeRef nodeRef, Version version)
+    {
+        NodeRef versionNodeRef = version.getFrozenStateNodeRef();
+        if (!this.nodeService.hasAspect(versionNodeRef, ForumModel.ASPECT_DISCUSSABLE))
+        {
+            return;
+        }
+        
+        // Get the discussion assoc references from the version store
+        List<ChildAssociationRef> childAssocRefs = this.nodeService.getChildAssocs(VersionUtil.convertNodeRef(versionNodeRef), ForumModel.ASSOC_DISCUSSION,
+                RegexQNamePattern.MATCH_ALL);
+        for (ChildAssociationRef childAssocRef : childAssocRefs)
+        {
+            // Get the child reference
+            NodeRef childRef = childAssocRef.getChildRef();
+            NodeRef referencedNode = (NodeRef) this.dbNodeService.getProperty(childRef, ContentModel.PROP_REFERENCE);
+
+            if (referencedNode != null && this.nodeService.exists(referencedNode) == false)
+            {
+                StoreRef orginalStoreRef = referencedNode.getStoreRef();
+                NodeRef archiveRootNodeRef = this.nodeService.getStoreArchiveNode(orginalStoreRef);
+                if (archiveRootNodeRef == null)
+                {
+                    // Store doesn't support archiving
+                    continue;
+                }
+                NodeRef archivedNodeRef = new NodeRef(archiveRootNodeRef.getStoreRef(), referencedNode.getId());
+
+                if (!this.nodeService.exists(archivedNodeRef) || !nodeService.hasAspect(archivedNodeRef, ContentModel.ASPECT_ARCHIVED))
+                {
+                    // Node doesn't support archiving or it was deleted within parent node.
+                    continue;
+                }
+
+                NodeRef existingChild = this.nodeService.getChildByName(nodeRef, childAssocRef.getTypeQName(), this.nodeService
+                        .getProperty(archivedNodeRef, ContentModel.PROP_NAME).toString());
+                if (existingChild != null)
+                {
+                    this.nodeService.deleteNode(existingChild);
+                }
+
+                this.nodeService.restoreNode(archivedNodeRef, null, null, null);
+            }
+        }
+    }
+    
 }

source/java/org/alfresco/repo/version/Version2ServiceImpl.java

@@ -1310,6 +1310,8 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe
                 // Turn auto-version policies back on
                 this.policyBehaviourFilter.enableBehaviour(nodeRef, ContentModel.ASPECT_VERSIONABLE);
             }
+            
+            invokeAfterVersionRevert(nodeRef, version);
         }
     }
 

source/java/org/alfresco/repo/version/VersionServicePolicies.java

@@ -125,4 +125,21 @@ public interface VersionServicePolicies
 				int versionNumber,
 				Map<String, Serializable>verisonProperties);
 	}
+    
+    
+    /**
+     * After create version policy interface
+     *
+     */
+    public interface AfterVersionRevertPolicy extends ClassPolicy
+    {
+        public static final QName QNAME = QName.createQName(NamespaceService.ALFRESCO_URI, "afterVersionRevert");
+        /**
+         * Called after the version has been reverted 
+         * 
+         * @param nodeRef   the node that has been reverted
+         * @param version           the reverted version
+         */
+        public void afterVersionRevert(NodeRef nodeRef, Version version);
+    }    
 }

source/java/org/alfresco/repo/version/common/AbstractVersionServiceImpl.java

@@ -33,6 +33,7 @@ import org.alfresco.repo.version.VersionRevertCallback.RevertAssocAction;
 import org.alfresco.repo.version.VersionRevertDetails;
 import org.alfresco.repo.version.VersionServicePolicies;
 import org.alfresco.repo.version.VersionServicePolicies.AfterCreateVersionPolicy;
+import org.alfresco.repo.version.VersionServicePolicies.AfterVersionRevertPolicy;
 import org.alfresco.repo.version.VersionServicePolicies.BeforeCreateVersionPolicy;
 import org.alfresco.repo.version.VersionServicePolicies.CalculateVersionLabelPolicy;
 import org.alfresco.repo.version.VersionServicePolicies.OnCreateVersionPolicy;
@@ -76,6 +77,7 @@ public abstract class AbstractVersionServiceImpl
 	private ClassPolicyDelegate<OnCreateVersionPolicy> onCreateVersionDelegate;
 	private ClassPolicyDelegate<CalculateVersionLabelPolicy> calculateVersionLabelDelegate;
 	private ClassPolicyDelegate<OnRevertVersionPolicy> onRevertVersionDelegate;
+    private ClassPolicyDelegate<AfterVersionRevertPolicy> afterVersionRevertDelegate;
     
 	/**
      * Sets the general node service
@@ -118,6 +120,24 @@ public abstract class AbstractVersionServiceImpl
 		this.onCreateVersionDelegate = this.policyComponent.registerClassPolicy(VersionServicePolicies.OnCreateVersionPolicy.class);
 		this.calculateVersionLabelDelegate = this.policyComponent.registerClassPolicy(VersionServicePolicies.CalculateVersionLabelPolicy.class);	
 		this.onRevertVersionDelegate = this.policyComponent.registerClassPolicy(VersionServicePolicies.OnRevertVersionPolicy.class);	
+        this.afterVersionRevertDelegate = this.policyComponent.registerClassPolicy(VersionServicePolicies.AfterVersionRevertPolicy.class);
+    }   
+    
+    
+    /**
+     * Invokes after version has been reverted
+     * 
+     * @param nodeRef   the node that has been reverted
+     * @param version           the reverted version
+     */
+    protected void invokeAfterVersionRevert(NodeRef nodeRef,Version version)
+    {
+        // invoke for node type
+        QName nodeTypeQName = nodeService.getType(nodeRef);
+        this.afterVersionRevertDelegate.get(nodeTypeQName).afterVersionRevert(nodeRef, version);
+        // invoke for node aspects
+        Set<QName> nodeAspectQNames = nodeService.getAspects(nodeRef);
+        this.afterVersionRevertDelegate.get(nodeAspectQNames).afterVersionRevert(nodeRef, version);
     }	
 	
 	/**

source/test-java/org/alfresco/repo/version/BaseVersionStoreTest.java

@@ -30,6 +30,7 @@ import java.util.Map;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.dictionary.DictionaryBootstrap;
 import org.alfresco.repo.dictionary.DictionaryDAO;
+import org.alfresco.repo.node.StoreArchiveMap;
 import org.alfresco.repo.node.archive.NodeArchiveService;
 import org.alfresco.repo.policy.BehaviourFilter;
 import org.alfresco.repo.policy.PolicyComponent;
@@ -202,6 +203,12 @@ public abstract class BaseVersionStoreTest extends BaseSpringTest
         // Create a workspace that contains the 'live' nodes
         this.testStoreRef = this.dbNodeService.createStore(StoreRef.PROTOCOL_WORKSPACE, "Test_" + System.currentTimeMillis());
         
+        StoreRef archiveStoreRef = nodeService.createStore(StoreRef.PROTOCOL_WORKSPACE, "archive" + getName() + System.currentTimeMillis());
+        
+        // Map the work store to the archive store.  This will already be wired into the NodeService.
+        StoreArchiveMap archiveMap = (StoreArchiveMap) applicationContext.getBean("storeArchiveMap");
+        archiveMap.put(testStoreRef, archiveStoreRef);        
+        
         // Get a reference to the root node
         this.rootNodeRef = this.dbNodeService.getRootNode(this.testStoreRef);
         

source/test-java/org/alfresco/repo/version/VersionServiceImplTest.java

@@ -61,6 +61,7 @@ import org.alfresco.service.cmr.version.VersionServiceException;
 import org.alfresco.service.cmr.version.VersionType;
 import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
+import org.alfresco.service.namespace.RegexQNamePattern;
 import org.alfresco.util.ApplicationContextHelper;
 import org.alfresco.util.GUID;
 import org.apache.commons.logging.Log;
@@ -593,7 +594,7 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
     	
         createComment(versionableNode, "my comment", "Do great work", false);
         assertTrue(nodeService.hasAspect(versionableNode, ForumModel.ASPECT_DISCUSSABLE));
-        assertTrue("fm:discussion association must exist", nodeService.getChildAssocs(versionableNode).size() > 0);
+        assertTrue("fm:discussion association must exist", nodeService.getChildAssocs(versionableNode, ForumModel.ASSOC_DISCUSSION, RegexQNamePattern.MATCH_ALL).size() > 0);
         assertEquals(1, this.dbNodeService.getProperty(versionableNode,  ForumModel.PROP_COMMENT_COUNT));
     	
     	// Create a new version
@@ -607,8 +608,48 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
     	//Revert to a version that has comments.
     	this.versionService.revert(versionableNode, version3);
         assertTrue(nodeService.hasAspect(versionableNode, ForumModel.ASPECT_DISCUSSABLE));
-        assertTrue("fm:discussion association must exist", nodeService.getChildAssocs(versionableNode).size() > 0);
+        assertTrue("fm:discussion association must exist", nodeService.getChildAssocs(versionableNode, ForumModel.ASSOC_DISCUSSION, RegexQNamePattern.MATCH_ALL).size() > 0);
         assertEquals("I am version 3", this.dbNodeService.getProperty(versionableNode, PROP_1));
+        
+        //Test reverting from version without comments to version that has comments
+        
+        //Revert to a version that has no comments.
+        this.versionService.revert(versionableNode, version1);
+        assertEquals("I am before version", this.dbNodeService.getProperty(versionableNode, PROP_1));  
+        assertFalse(nodeService.hasAspect(versionableNode, ForumModel.ASPECT_DISCUSSABLE));
+       
+        //Revert to a version that has comments.
+        this.versionService.revert(versionableNode, version3);
+        assertTrue(nodeService.hasAspect(versionableNode, ForumModel.ASPECT_DISCUSSABLE));
+        assertTrue("fm:discussion association must exist", nodeService.getChildAssocs(versionableNode, ForumModel.ASSOC_DISCUSSION, RegexQNamePattern.MATCH_ALL).size() > 0);
+        assertEquals("I am version 3", this.dbNodeService.getProperty(versionableNode, PROP_1));
+
+       
+       
+        //Test reverting from version with comments to another version with comments, but with another 'forum' node
+       
+        NodeRef clearNode = createNewVersionableNode();
+       
+        //Create version without comments
+        Version clearVersion1 = createVersion(clearNode);
+       
+        //Create version with comments       
+        createComment(clearNode, "my comment", "Do great work", false);
+        assertTrue(nodeService.hasAspect(clearNode, ForumModel.ASPECT_DISCUSSABLE));
+        Version clearVersion2 = createVersion(clearNode);
+       
+        //Revert to version without comments
+        this.versionService.revert(clearNode, clearVersion1);
+        assertFalse(nodeService.hasAspect(clearNode, ForumModel.ASPECT_DISCUSSABLE));
+       
+        //Create new version with comments
+        createComment(clearNode, "my comment", "Do great work", false);
+        Version clearVersion3 = createVersion(clearNode);
+
+        //Revert from version with comments, to version with another comments
+        this.versionService.revert(clearNode, clearVersion2);
+        assertTrue(nodeService.hasAspect(versionableNode, ForumModel.ASPECT_DISCUSSABLE));
+        assertTrue("fm:discussion association must exist", nodeService.getChildAssocs(clearNode, ForumModel.ASSOC_DISCUSSION, RegexQNamePattern.MATCH_ALL).size() > 0);  
 
     }
     
@@ -788,6 +829,7 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
         
         // Create a versionable node
         NodeRef versionableNode = createNewVersionableNode();
+        createComment(versionableNode, "my comment", "Do great work", false);
         
         // It isn't currently versionable
         assertEquals(null, versionService.getVersionHistory(versionableNode));