From af76d879cb7d6fd86a08199daef74ef322edc343 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Sat, 7 Mar 2015 16:35:57 +0000 Subject: [PATCH] Merged HEAD-BUG-FIX (5.1/Cloud) to HEAD (5.1/Cloud) 98997: Merged 5.0.N (5.0.2) to HEAD-BUG-FIX (5.1/Cloud) 98975: Merged V4.2-BUG-FIX (4.2.5) to 5.0.N (5.0.2) 98859: 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@99000 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../repo/version/Version2ServiceImpl.java | 54 +++++++++++ .../repo/version/VersionServiceImplTest.java | 92 +++++++++++++++++-- 2 files changed, 138 insertions(+), 8 deletions(-) diff --git a/source/java/org/alfresco/repo/version/Version2ServiceImpl.java b/source/java/org/alfresco/repo/version/Version2ServiceImpl.java index d4e749b9cf..ba4be54b18 100644 --- a/source/java/org/alfresco/repo/version/Version2ServiceImpl.java +++ b/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 forumProps = null; + + // Collect forum properties + // only if previous version hasn't discussable aspect + if (needToRestoreDiscussion) + { + Map currentVersionProp = this.nodeService.getProperties(nodeRef); + forumProps = new HashMap(); + for (QName key : currentVersionProp.keySet()) + { + if (key.getNamespaceURI().equals(NamespaceService.FORUMS_MODEL_1_0_URI)) + { + forumProps.put(key, currentVersionProp.get(key)); + } + } + } + Map newProps = this.nodeService.getProperties(versionNodeRef); VersionUtil.convertFrozenToOriginalProps(newProps); Set newAspectQNames = this.nodeService.getAspects(versionNodeRef); @@ -1065,6 +1087,12 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe } this.nodeService.setProperties(nodeRef, newProps); + + //Restore forum properties + if (needToRestoreDiscussion) + { + this.nodeService.addProperties(nodeRef, forumProps); + } Set aspectsToRemove = new HashSet(oldAspectQNames); aspectsToRemove.removeAll(newAspectQNames); @@ -1079,6 +1107,19 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe { this.nodeService.addAspect(nodeRef, aspect, null); } + } + // Don't remove forum aspects + if (needToRestoreDiscussion) + { + Set ignoredAspects = new HashSet(); + for (QName aspectForCheck : aspects) + { + if (aspectForCheck.getNamespaceURI().equals(NamespaceService.FORUMS_MODEL_1_0_URI)) + { + ignoredAspects.add(aspectForCheck); + } + } + aspects.removeAll(ignoredAspects); } // remove aspects that are not on the frozen node @@ -1175,6 +1216,19 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe children.remove(versionedChild); } } + // Don't remove forum children + if (needToRestoreDiscussion) + { + List ignoredChildren = new ArrayList(); + 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())) diff --git a/source/test-java/org/alfresco/repo/version/VersionServiceImplTest.java b/source/test-java/org/alfresco/repo/version/VersionServiceImplTest.java index 433667cd5f..f98f4494ee 100644 --- a/source/test-java/org/alfresco/repo/version/VersionServiceImplTest.java +++ b/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 = "

Comment for version 1

"; + final String V2_COMMENT = "

Comment for version 2

"; + 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, "

Comm123

", false); + addComment(versionableNode, V2_COMMENT, false); Set 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); - assertFalse(aspects.contains(ForumModel.ASPECT_DISCUSSABLE)); + assertTrue(isCommentExist(versionableNode, V3_COMMENT)); + 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 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 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);