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

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> 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,10 +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();
-            nodeService.setChildAssociationIndex(childAssocRef, getAllVersions(versionHistoryRef).size());
-            
+
             // NOTE: special ML case - see also MultilingualContentServiceImpl.makeMLContainer
             if (sourceTypeRef.equals(ContentModel.TYPE_MULTILINGUAL_CONTAINER))
             {

repository/src/main/resources/alfresco/core-services-context.xml

@@ -488,6 +488,9 @@
         <property name="versionComparatorClass">
             <value>${version.store.versionComparatorClass}</value>
         </property>
+        <property name="useVersionAssocIndex">
+            <value>${version.store.useVersionAssocIndex}</value>
+        </property>
     </bean>
 
     <bean id="versionNodeService" class="org.alfresco.repo.version.Node2ServiceImpl">

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

repository/src/test/java/org/alfresco/repo/version/VersionServiceImplTest.java

@@ -583,6 +583,7 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
     @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<QName, Serializable>());
@@ -616,6 +617,45 @@ public class VersionServiceImplTest extends BaseVersionStoreTest
                 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<QName, Serializable>());
+        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<ChildAssociationRef> 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.
@@ -653,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.