Merge branch 'feature-2.6/DuplicateBinForNodesWithCopies' into 'release/V2.6'

MNT-20145 Duplicate bin for nodes with copies

See merge request records-management/records-management!1123

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

@@ -143,7 +143,6 @@
         <property name="recordService" ref="RecordService" />
         <property name="dispositionService" ref="DispositionService" />
         <property name="quickShareService" ref="QuickShareService"/>
-        <property name="fileFolderService" ref="FileFolderService"/>
    </bean>
 
    <bean id="rma.recordComponentIdentifier" class="org.alfresco.module.org_alfresco_module_rm.model.rma.aspect.RecordComponentIdentifierAspect" parent="rm.baseBehaviour">

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

@@ -45,6 +45,7 @@
         <property name="authenticationUtil" ref="rm.authenticationUtil"/>
         <property name="transactionalResourceHelper" ref="rm.transactionalResourceHelper" />
         <property name="renditionService" ref="RenditionService" />
+        <property name="contentService" ref="ContentService" />
      </bean>
 
     <!-- Records Management Service Registry -->

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspect.java

@@ -51,13 +51,9 @@ import org.alfresco.repo.policy.annotation.BehaviourBean;
 import org.alfresco.repo.policy.annotation.BehaviourKind;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
-import org.alfresco.service.cmr.model.FileFolderService;
 import org.alfresco.service.cmr.quickshare.QuickShareService;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.ContentData;
-import org.alfresco.service.cmr.repository.ContentReader;
-import org.alfresco.service.cmr.repository.ContentService;
-import org.alfresco.service.cmr.repository.ContentWriter;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.ScriptService;
 import org.alfresco.service.namespace.QName;
@@ -98,9 +94,6 @@ public class RecordAspect extends    AbstractDisposableItem
     /** quickShare service */
     private QuickShareService quickShareService;
 
-    /** File folder service */
-    private FileFolderService fileFolderService;
-
     /** I18N */
     private static final String MSG_CANNOT_UPDATE_RECORD_CONTENT = "rm.service.update-record-content";
 
@@ -137,15 +130,6 @@ public class RecordAspect extends    AbstractDisposableItem
         this.quickShareService = quickShareService;
     }
 
-    /**
-     *
-     * @param fileFolderService file folder service
-     */
-    public void setFileFolderService(FileFolderService fileFolderService)
-    {
-        this.fileFolderService = fileFolderService;
-    }
-
     /**
      * Behaviour to ensure renditions have the appropriate extended security.
      *
@@ -351,11 +335,7 @@ public class RecordAspect extends    AbstractDisposableItem
     /**
      * On copy complete behaviour for record aspect.
      *
-     * @param classRef
+     * @see org.alfresco.repo.copy.CopyServicePolicies.OnCopyCompletePolicy#onCopyComplete(QName, NodeRef, NodeRef, boolean, Map)
-     * @param sourceNodeRef
-     * @param targetNodeRef
-     * @param copyToNewNode
-     * @param copyMap
      */
     @Override
     @Behaviour
@@ -376,12 +356,7 @@ public class RecordAspect extends    AbstractDisposableItem
             extendedSecurityService.remove(targetNodeRef);
 
             //create a new content URL for the copy
-            ContentReader reader = fileFolderService.getReader(targetNodeRef);
+            createNewContentURL(targetNodeRef);
-            if (reader != null)
-            {
-                ContentWriter writer = fileFolderService.getWriter(targetNodeRef);
-                writer.putContent(reader);
-            }
         }
     }
 
@@ -402,6 +377,7 @@ public class RecordAspect extends    AbstractDisposableItem
 
     /**
      * Behaviour to remove the shared link before declare a record
+     * and to create new bin if the node is a copy or has copies
      *
      * @see org.alfresco.repo.node.NodeServicePolicies.BeforeAddAspectPolicy#beforeAddAspect(org.alfresco.service.cmr.repository.NodeRef,
      *      org.alfresco.service.namespace.QName)
@@ -421,6 +397,26 @@ public class RecordAspect extends    AbstractDisposableItem
                     quickShareService.unshareContent(sharedId);
                 }
 
+                // if the node has a copy or is a copy of an existing node
+                if (!nodeService.getTargetAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty() ||
+                        !nodeService.getSourceAssocs(nodeRef, ContentModel.ASSOC_ORIGINAL).isEmpty())
+                {
+                    //disable versioning and auditing
+                    behaviourFilter.disableBehaviour(ContentModel.ASPECT_AUDITABLE);
+                    behaviourFilter.disableBehaviour(ContentModel.ASPECT_VERSIONABLE);
+                    try
+                    {
+                        //create a new content URL for the copy/original node
+                        createNewContentURL(nodeRef);
+                    }
+                    finally
+                    {
+                        //enable versioning and auditing
+                        behaviourFilter.enableBehaviour(ContentModel.ASPECT_AUDITABLE);
+                        behaviourFilter.enableBehaviour(ContentModel.ASPECT_VERSIONABLE);
+                    }
+                }
+
                 return null;
             }
         }, AuthenticationUtil.getSystemUserName());

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/VersionRecordAspect.java

@@ -34,12 +34,14 @@ import org.alfresco.module.org_alfresco_module_rm.relationship.Relationship;
 import org.alfresco.module.org_alfresco_module_rm.relationship.RelationshipService;
 import org.alfresco.module.org_alfresco_module_rm.version.RecordableVersionService;
 import org.alfresco.repo.node.NodeServicePolicies;
+import org.alfresco.repo.policy.Behaviour.NotificationFrequency;
 import org.alfresco.repo.policy.annotation.Behaviour;
 import org.alfresco.repo.policy.annotation.BehaviourBean;
 import org.alfresco.repo.policy.annotation.BehaviourKind;
 import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.version.Version;
+import org.alfresco.service.namespace.QName;
 
 /**
  * rmv:versionRecord behaviour bean
@@ -52,14 +54,15 @@ import org.alfresco.service.cmr.version.Version;
    defaultType = "rmv:versionRecord"
 )
 public class VersionRecordAspect extends    BaseBehaviourBean
-                                 implements NodeServicePolicies.BeforeDeleteNodePolicy
+                                 implements NodeServicePolicies.BeforeAddAspectPolicy,
+                                            NodeServicePolicies.BeforeDeleteNodePolicy
 {
     /** recordable version service */
     private RecordableVersionService recordableVersionService;
     
     /** relationship service */
     private RelationshipService relationshipService;
-    
+
     /**
      * @param recordableVersionService  recordable version service
      */
@@ -75,7 +78,7 @@ public class VersionRecordAspect extends    BaseBehaviourBean
     {
         this.relationshipService = relationshipService;
     }
-    
+
     /**
      * If the record is a version record then delete the associated version entry
      * 
@@ -129,4 +132,18 @@ public class VersionRecordAspect extends    BaseBehaviourBean
             });
         }         
     }
+
+    /**
+     * Behaviour to duplicate the bin before declaring a version record
+     *
+     * @see org.alfresco.repo.node.NodeServicePolicies.BeforeAddAspectPolicy#beforeAddAspect(org.alfresco.service.cmr.repository.NodeRef,
+     *      org.alfresco.service.namespace.QName)
+     */
+    @Override
+    @Behaviour(kind = BehaviourKind.CLASS, notificationFrequency = NotificationFrequency.FIRST_EVENT)
+    public void beforeAddAspect(NodeRef nodeRef, QName qName)
+    {
+        //create a new content URL for the version record
+        createNewContentURL(nodeRef);
+    }
 }

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/util/ServiceBaseImpl.java

@@ -31,6 +31,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.WeakHashMap;
 
+import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanComponentKind;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.hold.HoldService;
@@ -38,6 +39,9 @@ import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.rendition.RenditionService;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
+import org.alfresco.service.cmr.repository.ContentReader;
+import org.alfresco.service.cmr.repository.ContentService;
+import org.alfresco.service.cmr.repository.ContentWriter;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.namespace.QName;
@@ -71,9 +75,12 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte
 
     /** authentication helper */
     protected AuthenticationUtil authenticationUtil;
-    
+
     /** transactional resource helper */
-    protected TransactionalResourceHelper transactionalResourceHelper; 
+    protected TransactionalResourceHelper transactionalResourceHelper;
+
+    /** Content service */
+    protected ContentService contentService;
 
     /**
      * @see org.springframework.context.ApplicationContextAware#setApplicationContext(org.springframework.context.ApplicationContext)
@@ -124,6 +131,16 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte
         this.transactionalResourceHelper = transactionalResourceHelper;
     }
 
+    /**
+     * Set the content service
+     *
+     * @param contentService content service
+     */
+    public void setContentService(ContentService contentService)
+    {
+        this.contentService = contentService;
+    }
+
     /**
      * Helper to get internal node service.
      * <p>
@@ -537,4 +554,30 @@ public class ServiceBaseImpl implements RecordsManagementModel, ApplicationConte
         result.add(nodeService.getType(nodeRef));
         return result;
     }
+
+    /**
+     * Helper to update the given content property for the node
+     *
+     * @param nodeRef the node
+     * @param contentProperty the property to be updated
+     */
+    protected void updateContentProperty(NodeRef nodeRef, QName contentProperty)
+    {
+        ContentReader reader = contentService.getReader(nodeRef, contentProperty);
+        if (reader != null)
+        {
+            ContentWriter writer = contentService.getWriter(nodeRef, contentProperty, true);
+            writer.putContent(reader);
+        }
+    }
+
+    /**
+     * Helper to create a new content URL for the node
+     *
+     * @param nodeRef the node
+     */
+    protected void createNewContentURL(NodeRef nodeRef)
+    {
+        updateContentProperty(nodeRef, ContentModel.PROP_CONTENT);
+    }
 }

rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/version/RecordableVersionsBaseTest.java

@@ -238,7 +238,16 @@ public abstract class RecordableVersionsBaseTest extends BaseRMTestCase implemen
             if (frozenProperties.containsKey(beforePropertyName))
             {
                 Serializable frozenValue = frozenProperties.get(beforePropertyName);
-                assertEquals("Frozen property " + beforePropertyName.getLocalName() + " value is incorrect.", entry.getValue(), frozenValue);
+                if(beforePropertyName.equals(ContentModel.PROP_CONTENT))
+                {
+                    assertTrue("Content property value should be different.",
+                            entry.getValue() != frozenValue);
+                }
+                else
+                {
+                    assertEquals("Frozen property " + beforePropertyName.getLocalName() + " value is incorrect.",
+                            entry.getValue(), frozenValue);
+                }
                 cloneFrozenProperties.remove(beforePropertyName);               
             }
             else if (!PROP_FILE_PLAN.equals(beforePropertyName) &&

rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/model/rma/aspect/RecordAspectUnitTest.java

@@ -0,0 +1,176 @@
+/*
+ * #%L
+ * Alfresco Records Management Module
+ * %%
+ * Copyright (C) 2005 - 2019 Alfresco Software Limited
+ * %%
+ * This file is part of the Alfresco software.
+ * -
+ * If the software was purchased under a paid Alfresco license, the terms of
+ * the paid license agreement will prevail.  Otherwise, the software is
+ * provided under the following open source license terms:
+ * -
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ * -
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ * -
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * #L%
+ */
+package org.alfresco.module.org_alfresco_module_rm.model.rma.aspect;
+
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+
+import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASPECT_RECORD;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.mockito.MockitoAnnotations.initMocks;
+
+import org.alfresco.model.ContentModel;
+import org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService;
+import org.alfresco.repo.policy.BehaviourFilter;
+import org.alfresco.service.cmr.repository.AssociationRef;
+import org.alfresco.service.cmr.repository.ContentReader;
+import org.alfresco.service.cmr.repository.ContentService;
+import org.alfresco.service.cmr.repository.ContentWriter;
+import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.repository.NodeService;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+
+/**
+ * Unit tests for the {@link RecordAspect}.
+ *
+ * @author Claudia Agache
+ */
+public class RecordAspectUnitTest
+{
+    private static final NodeRef NODE_REF = new NodeRef("node://Ref/");
+    private static final NodeRef COPY_REF = new NodeRef("node://Copy/");
+    private static final AssociationRef SOURCE_ASSOC_REF = new AssociationRef(COPY_REF, ContentModel.ASSOC_ORIGINAL,
+            NODE_REF);
+    private static final AssociationRef TARGET_ASSOC_REF = new AssociationRef(NODE_REF, ContentModel.ASSOC_ORIGINAL,
+            COPY_REF);
+
+    @InjectMocks
+    private RecordAspect recordAspect;
+    @Mock
+    private NodeService mockNodeService;
+    @Mock
+    private BehaviourFilter mockBehaviorFilter;
+    @Mock
+    private ContentService mockContentService;
+    @Mock
+    private ContentReader mockContentReader;
+    @Mock
+    private ContentWriter mockContentWriter;
+    @Mock
+    private ExtendedSecurityService mockExtendedSecurityService;
+
+    @Before
+    public void setUp()
+    {
+        initMocks(this);
+    }
+
+    /** Check that the bin is duplicated before adding the aspect if the file has a copy. */
+    @Test
+    public void testDuplicateBinBeforeAddingAspectForFileWithCopy()
+    {
+        when(mockNodeService.getSourceAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(asList(SOURCE_ASSOC_REF));
+        when(mockContentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(mockContentReader);
+        when(mockContentService.getWriter(NODE_REF, ContentModel.PROP_CONTENT, true)).thenReturn(mockContentWriter);
+
+        recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD);
+
+         verifyBeforeAddAspectMethodsInvocations(1);
+    }
+
+    /** Check that the bin is duplicated before adding the aspect if the file is a copy. */
+    @Test
+    public void testDuplicateBinBeforeAddingAspectForCopy()
+    {
+        when(mockNodeService.getTargetAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(asList(TARGET_ASSOC_REF));
+        when(mockContentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(mockContentReader);
+        when(mockContentService.getWriter(NODE_REF, ContentModel.PROP_CONTENT, true)).thenReturn(mockContentWriter);
+
+        recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD);
+
+         verifyBeforeAddAspectMethodsInvocations(1);
+    }
+
+    /** Check that no content bin is created if the file does not have content. */
+    @Test
+    public void testBeforeAddAspectOnFileWithNoContent()
+    {
+        when(mockNodeService.getTargetAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(asList(TARGET_ASSOC_REF));
+        when(mockContentService.getReader(NODE_REF, ContentModel.PROP_CONTENT)).thenReturn(null);
+
+        recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD);
+
+        verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_AUDITABLE));
+        verify(mockBehaviorFilter, times(1)).disableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE));
+        verify(mockContentService, times(1)).getReader(NODE_REF, ContentModel.PROP_CONTENT);
+        verify(mockContentService, never()).getWriter(NODE_REF, ContentModel.PROP_CONTENT, true);
+        verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_AUDITABLE));
+        verify(mockBehaviorFilter, times(1)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE));
+    }
+
+    /** Check that the bin is not duplicated before adding the aspect if the node has no copies. */
+    @Test
+    public void testNotDuplicateBinForFileWithNoCopies()
+    {
+        when(mockNodeService.getSourceAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(emptyList());
+        when(mockNodeService.getTargetAssocs(NODE_REF, ContentModel.ASSOC_ORIGINAL)).thenReturn(emptyList());
+
+        recordAspect.beforeAddAspect(NODE_REF, ASPECT_RECORD);
+
+        verifyBeforeAddAspectMethodsInvocations(0);
+    }
+
+    /** Check that the bin is duplicated when copying a record. */
+    @Test
+    public void testDuplicateBinWhenCopyingRecord()
+    {
+        when(mockNodeService.exists(COPY_REF)).thenReturn(true);
+        when(mockNodeService.hasAspect(COPY_REF, ASPECT_RECORD)).thenReturn(true);
+        when(mockContentService.getReader(COPY_REF, ContentModel.PROP_CONTENT)).thenReturn(mockContentReader);
+        when(mockContentService.getWriter(COPY_REF, ContentModel.PROP_CONTENT, true)).thenReturn(mockContentWriter);
+
+        recordAspect.onCopyComplete(null, NODE_REF, COPY_REF, true, null);
+
+        verify(mockExtendedSecurityService, times(1)).remove(COPY_REF);
+        verify(mockContentService, times(1)).getReader(COPY_REF, ContentModel.PROP_CONTENT);
+        verify(mockContentService, times(1)).getWriter(COPY_REF, ContentModel.PROP_CONTENT, true);
+        verify(mockContentWriter, times(1)).putContent(mockContentReader);
+    }
+
+    /**
+     * Helper to verify beforeAddAspect methods invocations
+     *
+     * @param wantedNumberOfInvocations wanted number of invocations for each method
+     */
+    private void verifyBeforeAddAspectMethodsInvocations(int wantedNumberOfInvocations)
+    {
+        verify(mockBehaviorFilter, times(wantedNumberOfInvocations)).disableBehaviour(eq(ContentModel.ASPECT_AUDITABLE));
+        verify(mockBehaviorFilter, times(wantedNumberOfInvocations)).disableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE));
+        verify(mockContentService, times(wantedNumberOfInvocations)).getReader(NODE_REF, ContentModel.PROP_CONTENT);
+        verify(mockContentService, times(wantedNumberOfInvocations)).getWriter(NODE_REF, ContentModel.PROP_CONTENT, true);
+        verify(mockContentWriter, times(wantedNumberOfInvocations)).putContent(mockContentReader);
+        verify(mockBehaviorFilter, times(wantedNumberOfInvocations)).enableBehaviour(eq(ContentModel.ASPECT_AUDITABLE));
+        verify(mockBehaviorFilter, times(wantedNumberOfInvocations)).enableBehaviour(eq(ContentModel.ASPECT_VERSIONABLE));
+    }
+}