Merged HEAD-BUG-FIX (5.1/Cloud) to HEAD (5.1/Cloud)

98999: Merged 5.0.N (5.0.2) to HEAD-BUG-FIX (5.1/Cloud)
      98978: Merged V4.2-BUG-FIX (4.2.5) to 5.0.N (5.0.2)
         98931: MNT-13562: Merged V4.1-BUG-FIX (4.1.10) to V4.2-BUG-FIX (4.2.5)
            98757: Merged DEV to V4.1-BUG-FIX (4.1.10)
               94378 : MNT-6400: Issue with versioning and comments
                  - Ignore comments properties/aspects/children
               96791 : MNT-6400: Issue with versioning and comments
                  - Added a test


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

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

@@ -31,6 +31,7 @@ import java.util.Set;
 
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
+import org.alfresco.model.ForumModel;
 import org.alfresco.repo.policy.PolicyScope;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.version.VersionRevertCallback.RevertAspectAction;
@@ -52,6 +53,7 @@ import org.alfresco.service.cmr.version.Version;
 import org.alfresco.service.cmr.version.VersionHistory;
 import org.alfresco.service.cmr.version.VersionService;
 import org.alfresco.service.cmr.version.VersionServiceException;
+import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.namespace.RegexQNamePattern;
 import org.alfresco.util.Pair;
@@ -1012,6 +1014,26 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe
             // The frozen (which will become new) values
             // Get the node that represents the frozen state
             NodeRef versionNodeRef = version.getFrozenStateNodeRef();
+                boolean needToRestoreDiscussion = !this.nodeService.hasAspect(versionNodeRef, ForumModel.ASPECT_DISCUSSABLE) 
+                        && this.nodeService.hasAspect(nodeRef, ForumModel.ASPECT_DISCUSSABLE);
+    
+                Map<QName, Serializable> forumProps = null;
+                
+                // Collect forum properties
+                // only if previous version hasn't discussable aspect
+                if (needToRestoreDiscussion)
+                {
+                    Map<QName, Serializable> currentVersionProp = this.nodeService.getProperties(nodeRef);
+                    forumProps = new HashMap<QName, Serializable>();
+                    for (QName key : currentVersionProp.keySet())
+                    {
+                        if (key.getNamespaceURI().equals(NamespaceService.FORUMS_MODEL_1_0_URI))
+                        {
+                            forumProps.put(key, currentVersionProp.get(key));
+                        }
+                    }
+                }
+                    
             Map<QName, Serializable> newProps = this.nodeService.getProperties(versionNodeRef);
             VersionUtil.convertFrozenToOriginalProps(newProps);
             Set<QName> newAspectQNames = this.nodeService.getAspects(versionNodeRef);
@@ -1066,6 +1088,12 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe
             
             this.nodeService.setProperties(nodeRef, newProps);
                 
+                //Restore forum properties
+                if (needToRestoreDiscussion)
+                {
+                    this.nodeService.addProperties(nodeRef, forumProps);
+                }
+
             Set<QName> aspectsToRemove = new HashSet<QName>(oldAspectQNames);
         	aspectsToRemove.removeAll(newAspectQNames);
         	
@@ -1080,6 +1108,19 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe
         	        this.nodeService.addAspect(nodeRef, aspect, null);
         		}
                 }
+                // Don't remove forum aspects
+                if (needToRestoreDiscussion)
+                {
+                    Set<QName> ignoredAspects = new HashSet<QName>();
+                    for (QName aspectForCheck : aspectsToRemove)
+                    {
+                        if (aspectForCheck.getNamespaceURI().equals(NamespaceService.FORUMS_MODEL_1_0_URI))
+                        {
+                            ignoredAspects.add(aspectForCheck);
+                        }
+                    }
+                    aspectsToRemove.removeAll(ignoredAspects);
+        	}
         	
             // remove aspects that are not on the frozen node
             for (QName aspect : aspectsToRemove)
@@ -1175,6 +1216,19 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe
                     children.remove(versionedChild);
                 } 
             }
+                // Don't remove forum children
+                if (needToRestoreDiscussion)
+                {
+                    List<ChildAssociationRef> ignoredChildren = new ArrayList<ChildAssociationRef>();
+                    for (ChildAssociationRef childForCheck : children)
+                    {
+                        if (childForCheck.getTypeQName().getNamespaceURI().equals(NamespaceService.FORUMS_MODEL_1_0_URI))
+                        {
+                            ignoredChildren.add(childForCheck);
+                        }
+                    }
+                    children.removeAll(ignoredChildren);
+                }
             for (ChildAssociationRef ref : children)
             {
             	if (!assocsToLeaveAlone.contains(ref.getTypeQName()))

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

@@ -117,23 +117,66 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
         // NOOP
     }
     
-    // MNT-6400 Reverted node must not have fm:discussable aspect
+    /**
-    //  from next version
+     * MNT-6400 : Issue with versioning and comments
+     * 
+     * Test scenarios:
+     * 1) Create three versions with comments. Then revert to v1. All comments must be exist.
+     * 2) Create three versions. Add comment to the latest two versions (v2 and v3). Then revert to v1. Comments must be exist.
+     */
     public void testDiscussableAspect()
     {
+    	final String V1_COMMENT = "<p>Comment for version 1</p>";
+    	final String V2_COMMENT = "<p>Comment for version 2</p>";
+    	final String V3_COMMENT = "Comment for third version";
         NodeRef versionableNode = createNewVersionableNode();
+        
+        // Test scenario 1
         Version v1 = createVersion(versionableNode);
-        createVersion(versionableNode);
+        addComment(versionableNode, V1_COMMENT, false);
+        Version v2 = createVersion(versionableNode);
         VersionHistory vh = this.versionService.getVersionHistory(versionableNode);
         assertEquals(2, vh.getAllVersions().size());
-        addComment(versionableNode, "<p>Comm123</p>", false);
+        addComment(versionableNode, V2_COMMENT, false);
         
         Set<QName> aspects = nodeService.getAspects(versionableNode);
         assertTrue(aspects.contains(ForumModel.ASPECT_DISCUSSABLE));
+        assertTrue(isCommentExist(versionableNode, V2_COMMENT));
+        
+        Version v3 = createVersion(versionableNode);
+        vh = this.versionService.getVersionHistory(versionableNode);
+        assertEquals(3, vh.getAllVersions().size());
+        addComment(versionableNode, V3_COMMENT, false);
+        assertTrue(isCommentExist(versionableNode, V3_COMMENT));
         
         this.versionService.revert(versionableNode, v1);
-        aspects = nodeService.getAspects(versionableNode);
+        assertTrue(isCommentExist(versionableNode, V3_COMMENT));
-        assertFalse(aspects.contains(ForumModel.ASPECT_DISCUSSABLE));
+        assertTrue(isCommentExist(versionableNode, V2_COMMENT));
+        assertTrue(isCommentExist(versionableNode, V1_COMMENT));
+        
+        // Test scenario 2
+        versionableNode = createNewVersionableNode();
+        v1 = createVersion(versionableNode);
+        vh = this.versionService.getVersionHistory(versionableNode);
+        assertEquals(1, vh.getAllVersions().size());
+        
+        v2 = createVersion(versionableNode);
+        vh = this.versionService.getVersionHistory(versionableNode);
+        assertEquals(2, vh.getAllVersions().size());
+        addComment(versionableNode, V2_COMMENT, false);
+        assertTrue(isCommentExist(versionableNode, V2_COMMENT));
+        
+        v3 = createVersion(versionableNode);
+        vh = this.versionService.getVersionHistory(versionableNode);
+        assertEquals(3, vh.getAllVersions().size());
+        addComment(versionableNode, V3_COMMENT, false);
+        assertTrue(isCommentExist(versionableNode, V3_COMMENT));
+        
+        this.versionService.revert(versionableNode, v1);
+        assertTrue(isCommentExist(versionableNode, V3_COMMENT));
+        assertTrue(isCommentExist(versionableNode, V2_COMMENT));
+        
+        assertFalse(isCommentExist(versionableNode, V1_COMMENT));
     }
     
     private NodeRef addComment(NodeRef nr, String comment, boolean suppressRollups)
@@ -172,6 +215,39 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
         return postNode;
     }
     
+    private boolean isCommentExist(NodeRef nr, String commentForCheck)
+    {
+    	if (!nodeService.hasAspect(nr, ForumModel.ASPECT_DISCUSSABLE))
+        {
+            return false;
+        }
+    	
+    	NodeRef forumNode = nodeService.getChildAssocs(nr, ForumModel.ASSOC_DISCUSSION, QName.createQName(NamespaceService.FORUMS_MODEL_1_0_URI, "discussion")).get(0).getChildRef();
+    	final List<ChildAssociationRef> existingTopics = nodeService.getChildAssocs(forumNode, ContentModel.ASSOC_CONTAINS, QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "Comments"));
+    	if (existingTopics.isEmpty())
+        {
+    		return false;
+        }
+    	NodeRef topicNode = existingTopics.get(0).getChildRef();
+    	Collection<ChildAssociationRef> comments = nodeService.getChildAssocsWithoutParentAssocsOfType(topicNode, ContentModel.ASSOC_CONTAINS);
+    	for (ChildAssociationRef comment : comments)
+    	{
+    		NodeRef commentRef = comment.getChildRef();
+    		ContentReader reader = contentService.getReader(commentRef, ContentModel.PROP_CONTENT);
+    		if (reader == null)
+    		{
+    			continue;
+    		}
+    		String contentString = reader.getContentString();
+    		if (commentForCheck.equals(contentString))
+    		{
+    			return true;
+    		}
+    	}
+    	
+    	return false;
+    }
+    
     /**
      * Tests the creation of the initial version of a versionable node
      */
@@ -675,7 +751,7 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
         //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));
+        assertTrue(nodeService.hasAspect(versionableNode, ForumModel.ASPECT_DISCUSSABLE));
        
         //Revert to a version that has comments.
         this.versionService.revert(versionableNode, version3);
@@ -699,7 +775,7 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
        
         //Revert to version without comments
         this.versionService.revert(clearNode, clearVersion1);
-        assertFalse(nodeService.hasAspect(clearNode, ForumModel.ASPECT_DISCUSSABLE));
+        assertTrue(nodeService.hasAspect(clearNode, ForumModel.ASPECT_DISCUSSABLE));
        
         //Create new version with comments
         createComment(clearNode, "my comment", "Do great work", false);