From f2bd6ca466bafa65c69cf5335e3ca7e151b00b30 Mon Sep 17 00:00:00 2001 From: evasques Date: Fri, 10 Dec 2021 09:34:28 +0000 Subject: [PATCH] MNT-22715 - Document Version Issue (#838) * MNT-22715 - Document Version Issue (#831) * MNT-22715 - Document Version Issue - Unexpected: current version does not appear to be 1st version in the list * Set association index on new version creation * Unit test to verify the child assoc index is set on versions * Set association index on new version creation on AGS create record from version (cherry picked from commit c9e98b483391b41d3e2fbf7b69e3f883cc38a798) * MNT-22715 - Setting Version Child Assoc Index Configurable (#836) * Added configuration to use child association index on version creation - disabled by default * Added unit test to verify both cenarios * Included configuration in AGS (cherry picked from commit d729443b71a5de7aaab38147857763a30d3ffb7b) --- .../version/RecordableVersionServiceImpl.java | 10 ++ .../repo/version/Version2ServiceImpl.java | 26 ++++- .../alfresco/core-services-context.xml | 3 + .../resources/alfresco/repository.properties | 8 ++ .../repo/version/VersionServiceImplTest.java | 102 +++++++++++++++++- 5 files changed, 145 insertions(+), 4 deletions(-) diff --git a/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/version/RecordableVersionServiceImpl.java b/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/version/RecordableVersionServiceImpl.java index a22c06cd4d..b2a22a7a86 100644 --- a/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/version/RecordableVersionServiceImpl.java +++ b/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/version/RecordableVersionServiceImpl.java @@ -434,6 +434,11 @@ public class RecordableVersionServiceImpl extends Version2ServiceImpl QName.createQName(Version2Model.NAMESPACE_URI, Version2Model.CHILD_VERSIONS + "-" + versionNumber), sourceTypeRef, null); + + if (isUseVersionAssocIndex()) + { + nodeService.setChildAssociationIndex(childAssocRef, getAllVersions(versionHistoryRef).size()); + } versionNodeRef = childAssocRef.getChildRef(); // add aspect with the standard version properties to the 'version' node @@ -808,6 +813,11 @@ public class RecordableVersionServiceImpl extends Version2ServiceImpl QName.createQName(Version2Model.NAMESPACE_URI, Version2Model.CHILD_VERSIONS + "-" + versionNumber), sourceTypeRef, null); + + if (isUseVersionAssocIndex()) + { + nodeService.setChildAssociationIndex(childAssocRef, getAllVersions(versionHistoryRef).size()); + } NodeRef versionNodeRef = childAssocRef.getChildRef(); // add aspect with the standard version properties to the 'version' node diff --git a/repository/src/main/java/org/alfresco/repo/version/Version2ServiceImpl.java b/repository/src/main/java/org/alfresco/repo/version/Version2ServiceImpl.java index 7bce9cbc61..7eac519f2a 100644 --- a/repository/src/main/java/org/alfresco/repo/version/Version2ServiceImpl.java +++ b/repository/src/main/java/org/alfresco/repo/version/Version2ServiceImpl.java @@ -84,6 +84,7 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe private static Log logger = LogFactory.getLog(Version2ServiceImpl.class); private PermissionService permissionService; + private boolean useVersionAssocIndex = false; private ExtendedTrait versionServiceTrait; @@ -96,7 +97,23 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe { this.permissionService = permissionService; } - + + /** + * Set to use child association index on versions. This helps ordering versions when sequential IDs are not + * guaranteed by the DBMS. + * + * @param useVersionAssocIndex + */ + public void setUseVersionAssocIndex(boolean useVersionAssocIndex) + { + this.useVersionAssocIndex = useVersionAssocIndex; + } + + public boolean isUseVersionAssocIndex() + { + return useVersionAssocIndex; + } + /** * Initialise method */ @@ -506,9 +523,12 @@ public class Version2ServiceImpl extends VersionServiceImpl implements VersionSe QName.createQName(Version2Model.NAMESPACE_URI, Version2Model.CHILD_VERSIONS+"-"+versionNumber), // TODO - testing - note: all children (of a versioned node) will have the same version number, maybe replace with a version sequence of some sort 001-...00n sourceTypeRef, nodeDetails.getProperties()); - + if (isUseVersionAssocIndex()) + { + nodeService.setChildAssociationIndex(childAssocRef, getAllVersions(versionHistoryRef).size()); + } versionNodeRef = childAssocRef.getChildRef(); - + // NOTE: special ML case - see also MultilingualContentServiceImpl.makeMLContainer if (sourceTypeRef.equals(ContentModel.TYPE_MULTILINGUAL_CONTAINER)) { diff --git a/repository/src/main/resources/alfresco/core-services-context.xml b/repository/src/main/resources/alfresco/core-services-context.xml index 13bf823d49..8fb1e46483 100644 --- a/repository/src/main/resources/alfresco/core-services-context.xml +++ b/repository/src/main/resources/alfresco/core-services-context.xml @@ -488,6 +488,9 @@ ${version.store.versionComparatorClass} + + ${version.store.useVersionAssocIndex} + diff --git a/repository/src/main/resources/alfresco/repository.properties b/repository/src/main/resources/alfresco/repository.properties index debd17d6f4..6e423919ae 100644 --- a/repository/src/main/resources/alfresco/repository.properties +++ b/repository/src/main/resources/alfresco/repository.properties @@ -382,6 +382,14 @@ version.store.version2Store=workspace://version2Store # if upgrading from a version that used unordered sequences in a cluster. version.store.versionComparatorClass= +# Optional to set the child association index when creating a new version. +# This helps ordering versions when sequential IDs are not guaranteed by the DBMS. +# Not compatible with AGS < 7.1.1 +# Once enabled, it should not be disabled again or new versions will go back +# to have index -1 and you will get the wrong order in version history. +# Please, see MNT-22715 for details. +version.store.useVersionAssocIndex=false + # Folders for storing people system.system_container.childname=sys:system system.people_container.childname=sys:people diff --git a/repository/src/test/java/org/alfresco/repo/version/VersionServiceImplTest.java b/repository/src/test/java/org/alfresco/repo/version/VersionServiceImplTest.java index 69b1e368be..e9c54bca19 100644 --- a/repository/src/test/java/org/alfresco/repo/version/VersionServiceImplTest.java +++ b/repository/src/test/java/org/alfresco/repo/version/VersionServiceImplTest.java @@ -39,7 +39,6 @@ import java.util.Properties; import java.util.Set; import javax.transaction.UserTransaction; -import junit.framework.AssertionFailedError; import org.alfresco.model.ApplicationModel; import org.alfresco.model.ContentModel; @@ -94,6 +93,8 @@ import org.springframework.test.annotation.Commit; import org.springframework.test.context.transaction.TestTransaction; import org.springframework.transaction.annotation.Transactional; +import junit.framework.AssertionFailedError; + /** * versionService test class. * @@ -575,6 +576,86 @@ public class VersionServiceImplTest extends BaseVersionStoreTest } } + /** + * When IDs are out of order the comparator only fixes the order we retrieve versions. Any operation fails due to + * the head version not being the latest. (MNT-22715) + */ + @Test + public void testVersionIndex() + { + setUseVersionAssocIndex(true); + NodeRef versionableNode = nodeService.createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, + QName.createQName("{test}MyVersionableNodeTestIndex"), ContentModel.TYPE_CONTENT, null).getChildRef(); + nodeService.addAspect(versionableNode, ContentModel.ASPECT_VERSIONABLE, new HashMap()); + Version version1 = createVersion(versionableNode); + Version version2 = createVersion(versionableNode); + Version version3 = createVersion(versionableNode); + + VersionHistory vh = versionService.getVersionHistory(versionableNode); + assertEquals("Version History does not contain 3 versions", 3, vh.getAllVersions().size()); + + NodeRef root = nodeService.getPrimaryParent(vh.getRootVersion().getFrozenStateNodeRef()).getParentRef(); + NodeRef versionHistoryNode = dbNodeService.getChildByName(root, Version2Model.CHILD_QNAME_VERSION_HISTORIES, + versionableNode.getId()); + + // getChildAssocs orders by assoc_index first and then by ID. Version History relies on this. + List vhChildAssocs = nodeService.getChildAssocs(versionHistoryNode); + int index = 0; + for (ChildAssociationRef vhChildAssoc : vhChildAssocs) + { + // Unset indexes are -1 + assertFalse("Index is not set", vhChildAssoc.getNthSibling() < 0); + assertTrue("Index is not increasing as expected", vhChildAssoc.getNthSibling() > index); + index = vhChildAssoc.getNthSibling(); + } + + assertEquals("1st version is not 1st assoc", version1.getFrozenStateNodeRef().getId(), + vhChildAssocs.get(0).getChildRef().getId()); + assertEquals("2nd version is not 2nd assoc", version2.getFrozenStateNodeRef().getId(), + vhChildAssocs.get(1).getChildRef().getId()); + assertEquals("3rd version is not 3rd assoc", version3.getFrozenStateNodeRef().getId(), + vhChildAssocs.get(2).getChildRef().getId()); + } + + /** + * Test version assoc index use disabled + */ + @Test + public void testVersionIndexDisabled() + { + setUseVersionAssocIndex(false); + NodeRef versionableNode = nodeService + .createNode(rootNodeRef, ContentModel.ASSOC_CHILDREN, + QName.createQName("{test}MyVersionableNodeTestWithoutIndex"), ContentModel.TYPE_CONTENT, null) + .getChildRef(); + nodeService.addAspect(versionableNode, ContentModel.ASPECT_VERSIONABLE, new HashMap()); + Version version1 = createVersion(versionableNode); + Version version2 = createVersion(versionableNode); + Version version3 = createVersion(versionableNode); + + VersionHistory vh = versionService.getVersionHistory(versionableNode); + assertEquals("Version History does not contain 3 versions", 3, vh.getAllVersions().size()); + + NodeRef root = nodeService.getPrimaryParent(vh.getRootVersion().getFrozenStateNodeRef()).getParentRef(); + NodeRef versionHistoryNode = dbNodeService.getChildByName(root, Version2Model.CHILD_QNAME_VERSION_HISTORIES, + versionableNode.getId()); + + // getChildAssocs orders by assoc_index first and then by ID. Version History relies on this. + List vhChildAssocs = nodeService.getChildAssocs(versionHistoryNode); + for (ChildAssociationRef vhChildAssoc : vhChildAssocs) + { + // Unset indexes are -1 + assertTrue("Index is not set", vhChildAssoc.getNthSibling() < 0); + } + + assertEquals("1st version is not 1st assoc", version1.getFrozenStateNodeRef().getId(), + vhChildAssocs.get(0).getChildRef().getId()); + assertEquals("2nd version is not 2nd assoc", version2.getFrozenStateNodeRef().getId(), + vhChildAssocs.get(1).getChildRef().getId()); + assertEquals("3rd version is not 3rd assoc", version3.getFrozenStateNodeRef().getId(), + vhChildAssocs.get(2).getChildRef().getId()); + } + /** * Sets the versionService to be one that has is db ids out of order * so would normally have versions displayed in the wrong order. @@ -612,6 +693,25 @@ public class VersionServiceImplTest extends BaseVersionStoreTest setVersionService(versionService); } + /** + * Sets the versionService to use the version assoc Index + * @param useVersionAssocIndex + */ + private void setUseVersionAssocIndex(boolean useVersionAssocIndex) + { + Version2ServiceImpl versionService = new Version2ServiceImpl(); + versionService.setNodeService(nodeService); + versionService.setDbNodeService(dbNodeService); // mtAwareNodeService + versionService.setSearcher(versionSearchService); + versionService.setDictionaryService(dictionaryService); + versionService.setPolicyComponent(policyComponent); + versionService.setPolicyBehaviourFilter(policyBehaviourFilter); + versionService.setPermissionService(permissionService); + versionService.setUseVersionAssocIndex(useVersionAssocIndex); + versionService.initialise(); + setVersionService(versionService); + } + /** * Adds another version to the version history then checks that getVersionHistory is returning * the correct data.