Merge pull request #1349 from Alfresco/feature/APPS-659_PatchFor35

Feature/apps 659 patch for35

rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/patch/rm-patch-v35-context.xml

@@ -5,7 +5,7 @@
         http://www.springframework.org/schema/beans/spring-beans.xsd">
 
    <!-- RM v3.5 Patches -->
-
+   <!-- fixesFromSchema must be 3201 because we apply this patch over RMv32HoldChildAssocPatch  -->
    <bean id="rm.holdChildAssocPatch"
          parent="rm.parentModulePatch"
          class="org.alfresco.module.org_alfresco_module_rm.patch.v35.RMv35HoldNewChildAssocPatch">

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatch.java

@@ -26,6 +26,12 @@
  */
 package org.alfresco.module.org_alfresco_module_rm.patch.v35;
 
+import static org.alfresco.model.ContentModel.ASSOC_CONTAINS;
+import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementCustomModel.RM_CUSTOM_URI;
+import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASSOC_FROZEN_CONTENT;
+
+import java.util.List;
+
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.hold.HoldService;
@@ -34,17 +40,21 @@ import org.alfresco.repo.policy.BehaviourFilter;
 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.namespace.QName;
+import org.alfresco.service.namespace.RegexQNamePattern;
 
 /**
  * Patch to create new hold child association to link the record to the hold
- *
+ * <p>
  * See: https://alfresco.atlassian.net/browse/APPS-659
  *
- *
  * @since 3.5
  */
 public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch
 {
+    /** A name for the associations created by this patch. */
+    protected static final QName PATCH_ASSOC_NAME = QName.createQName(RM_CUSTOM_URI, RMv35HoldNewChildAssocPatch.class.getSimpleName());
+
     /**
      * File plan service interface
      */
@@ -64,6 +74,7 @@ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch
 
     /**
      * Setter for fileplanservice
+     *
      * @param filePlanService File plan service interface
      */
     public void setFilePlanService(FilePlanService filePlanService)
@@ -73,6 +84,7 @@ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch
 
     /**
      * Setter for hold service
+     *
      * @param holdService Hold service interface.
      */
     public void setHoldService(HoldService holdService)
@@ -82,6 +94,7 @@ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch
 
     /**
      * Setter for node service
+     *
      * @param nodeService Interface for public and internal node and store operations.
      */
     public void setNodeService(NodeService nodeService)
@@ -110,10 +123,19 @@ public class RMv35HoldNewChildAssocPatch extends AbstractModulePatch
             {
                 for (NodeRef hold : holdService.getHolds(filePlan))
                 {
-                    for (ChildAssociationRef ref : nodeService.getChildAssocs(hold))
+                    List<ChildAssociationRef> frozenAssoc = nodeService.getChildAssocs(hold, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL);
+                    for (ChildAssociationRef ref : frozenAssoc)
                     {
-                        holdService.removeFromHold(hold, ref.getChildRef());
+                        NodeRef childNodeRef = ref.getChildRef();
-                        holdService.addToHold(hold, ref.getChildRef());
+                        // In testing we found that this was returning more than just "contains" associations.
+                        // Possibly this is due to the code in Node2ServiceImpl.getParentAssocs not using the second parameter.
+                        List<ChildAssociationRef> parentAssocs = nodeService.getParentAssocs(childNodeRef, ASSOC_CONTAINS, RegexQNamePattern.MATCH_ALL);
+                        boolean childContainedByHold =
+                                parentAssocs.stream().anyMatch(entry -> entry.getParentRef().equals(hold) && entry.getTypeQName().equals(ASSOC_CONTAINS));
+                        if (!childContainedByHold)
+                        {
+                            nodeService.addChild(hold, childNodeRef, ASSOC_CONTAINS, PATCH_ASSOC_NAME);
+                        }
                     }
                 }
             }

rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/patch/v35/RMv35HoldNewChildAssocPatchUnitTest.java

@@ -27,9 +27,16 @@
 
 package org.alfresco.module.org_alfresco_module_rm.patch.v35;
 
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+
+import static org.alfresco.model.ContentModel.ASSOC_CONTAINS;
+import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASSOC_FROZEN_CONTENT;
+import static org.alfresco.module.org_alfresco_module_rm.patch.v35.RMv35HoldNewChildAssocPatch.PATCH_ASSOC_NAME;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyMap;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
@@ -48,6 +55,7 @@ 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.namespace.QName;
+import org.alfresco.service.namespace.RegexQNamePattern;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.InjectMocks;
@@ -77,6 +85,7 @@ public class RMv35HoldNewChildAssocPatchUnitTest
     private RMv35HoldNewChildAssocPatch patch;
 
     private NodeRef filePlanRef, holdRef, heldItemRef;
+
     private Set<NodeRef> fileplans;
     private List<NodeRef> holds;
 
@@ -101,45 +110,44 @@ public class RMv35HoldNewChildAssocPatchUnitTest
     }
 
     /**
-     * Test held items are removed from a hold and re-add to make sure the association is correct
+     * Test secondary associations are created for held items so that they are "contained" in the hold.
      */
     @Test
-    public void testAHoldIsRemovedAndReplacedDuringUpgrade()
+    public void testAddChildDuringUpgrade()
     {
         when(mockFilePlanService.getFilePlans()).thenReturn(fileplans);
         when(mockHoldService.getHolds(filePlanRef)).thenReturn(holds);
+        when(mockNodeService.getChildAssocs(holdRef, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(childAssocs);
         when(childAssociationRef.getChildRef()).thenReturn(heldItemRef);
-        when(mockNodeService.getChildAssocs(holdRef)).thenReturn(childAssocs);
+
         patch.applyInternal();
-        verify(mockHoldService, times(1)).removeFromHold(holdRef, heldItemRef);
+
-        verify(mockHoldService, times(1)).addToHold(holdRef, heldItemRef);
+        verify(mockNodeService, times(1)).addChild(holdRef, heldItemRef, ASSOC_CONTAINS, PATCH_ASSOC_NAME);
     }
 
     @Test
-    public void patchRunWithSuccessWhenNoHoldChilds()
+    public void patchRunWithSuccessWhenNoHeldChildren()
     {
-        List<NodeRef> holdList = new ArrayList<>();
+        when(mockFilePlanService.getFilePlans()).thenReturn(fileplans);
-        holdList.add(holdRef);
+        when(mockHoldService.getHolds(filePlanRef)).thenReturn(holds);
-        when(childAssociationRef.getChildRef()).thenReturn(heldItemRef);
+        when(mockNodeService.getChildAssocs(holdRef, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(emptyList());
-        when(mockNodeService.getChildAssocs(holdRef)).thenReturn(new ArrayList<>());
+
         patch.applyInternal();
 
-        verify(mockHoldService, times(0)).removeFromHold(holdRef, heldItemRef);
+        verify(mockNodeService, never()).addChild(any(NodeRef.class), any(NodeRef.class), any(QName.class), any(QName.class));
-        verify(mockHoldService, times(0)).addToHold(holdRef, heldItemRef);
-
     }
 
     @Test
     public void patchRunWithSuccessWhenNoHolds()
     {
         //no holds
-        List<NodeRef> holdList = new ArrayList<>();
+        List<NodeRef> holdList = emptyList();
         when(mockFilePlanService.getFilePlans()).thenReturn(fileplans);
         when(mockHoldService.getHolds(filePlanRef)).thenReturn(holdList);
+
         patch.applyInternal();
 
-        verify(mockHoldService, times(0)).removeFromHold(holdRef, heldItemRef);
+        verify(mockNodeService, never()).addChild(any(NodeRef.class), any(NodeRef.class), any(QName.class), any(QName.class));
-        verify(mockHoldService, times(0)).addToHold(holdRef, heldItemRef);
     }
 
     @Test
@@ -152,7 +160,6 @@ public class RMv35HoldNewChildAssocPatchUnitTest
         patch.applyInternal();
 
         // then
-        verifyZeroInteractions(mockHoldService);
+        verify(mockNodeService, never()).addChild(any(NodeRef.class), any(NodeRef.class), any(QName.class), any(QName.class));
-        verify(mockNodeService, times(0)).addAspect(any(NodeRef.class), any(QName.class), anyMap());
     }
 }