Merge pull request #1234 from Alfresco/feature/APPS-361

APPS-361: [AGS/S3]Moving records between buckets in S3 (also possible moving records between stores-filecontenstore) fails

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

@@ -39,7 +39,6 @@ import org.alfresco.module.org_alfresco_module_rm.model.behaviour.AbstractDispos
 import org.alfresco.module.org_alfresco_module_rm.record.RecordService;
 import org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService;
 import org.alfresco.module.org_alfresco_module_rm.util.ContentBinDuplicationUtility;
-import org.alfresco.repo.content.ContentServicePolicies;
 import org.alfresco.repo.copy.CopyBehaviourCallback;
 import org.alfresco.repo.copy.CopyDetails;
 import org.alfresco.repo.copy.CopyServicePolicies;
@@ -58,6 +57,7 @@ import org.alfresco.service.cmr.repository.ContentData;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.ScriptService;
 import org.alfresco.service.namespace.QName;
+import org.alfresco.util.EqualsHelper;
 import org.springframework.extensions.surf.util.I18NUtil;
 
 /**
@@ -77,7 +77,7 @@ public class RecordAspect extends    AbstractDisposableItem
                                      RecordsManagementPolicies.OnRemoveReference,
                                      NodeServicePolicies.OnMoveNodePolicy,
                                      CopyServicePolicies.OnCopyCompletePolicy,
-                                     ContentServicePolicies.OnContentPropertyUpdatePolicy
+                                     NodeServicePolicies.OnUpdatePropertiesPolicy
 {
     /** Well-known location of the scripts folder. */
     // TODO make configurable
@@ -373,21 +373,6 @@ public class RecordAspect extends    AbstractDisposableItem
         }
     }
 
-    @Override
-    @Behaviour
-    (
-       kind = BehaviourKind.CLASS,
-       notificationFrequency = NotificationFrequency.FIRST_EVENT
-    )
-    public void onContentPropertyUpdate(NodeRef nodeRef, QName propertyQName, ContentData beforeValue, ContentData afterValue)
-    {
-        // Allow creation of content but not update
-        if (beforeValue != null)
-        {
-            throw new IntegrityException(I18NUtil.getMessage(MSG_CANNOT_UPDATE_RECORD_CONTENT), null);
-        }
-    }
-
     /**
      * Behaviour to remove the shared link before declare a record
      * and to create new bin if the node is a copy or has copies
@@ -421,4 +406,35 @@ public class RecordAspect extends    AbstractDisposableItem
             }
         }, AuthenticationUtil.getSystemUserName());
     }
+
+    /**
+     * Behaviour to prevent content update for records
+     *
+     * @see org.alfresco.repo.node.NodeServicePolicies.OnUpdatePropertiesPolicy#onUpdateProperties(NodeRef, Map, Map)
+     */
+    @Override
+    @Behaviour
+            (
+                    kind = BehaviourKind.CLASS,
+                    notificationFrequency = NotificationFrequency.FIRST_EVENT
+            )
+    public void onUpdateProperties(NodeRef nodeRef, Map<QName, Serializable> before, Map<QName, Serializable> after)
+    {
+        String storeNameAfter = (String) after.get(ContentModel.PROP_STORE_NAME);
+
+        ContentData contentBefore = (ContentData) before.get(ContentModel.PROP_CONTENT);
+        ContentData contentAfter = (ContentData) after.get(ContentModel.PROP_CONTENT);
+
+        // Check only storeNameAfter since the store name is updated before this method is triggered
+        // Does not allow setting content to null when moving content between stores (case not covered by
+        // ContentPropertyRestrictionInterceptor)
+        if (storeNameAfter != null && contentAfter != null)
+        {
+            return;
+        }
+        else if (contentBefore != null && !contentBefore.equals(contentAfter))
+        {
+                throw new IntegrityException(I18NUtil.getMessage(MSG_CANNOT_UPDATE_RECORD_CONTENT), null);
+        }
+    }
 }

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

@@ -29,19 +29,29 @@ 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_ARCHIVED;
+import static org.alfresco.model.ContentModel.PROP_CONTENT;
+import static org.alfresco.model.ContentModel.PROP_STORE_NAME;
 import static org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel.ASPECT_RECORD;
 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 java.io.Serializable;
+import java.util.Locale;
+import java.util.Map;
+
+import com.google.common.collect.ImmutableMap;
+
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService;
 import org.alfresco.module.org_alfresco_module_rm.util.ContentBinDuplicationUtility;
+import org.alfresco.repo.node.integrity.IntegrityException;
 import org.alfresco.service.cmr.repository.AssociationRef;
+import org.alfresco.service.cmr.repository.ContentData;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
+import org.alfresco.service.namespace.QName;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.InjectMocks;
@@ -123,5 +133,29 @@ public class RecordAspectUnitTest
         verify(mockContentBinDuplicationUtility, times(1)).duplicate(COPY_REF);
     }
 
+    /**
+     * Check that an IntegrityException is thrown when content is changed
+     */
+    @Test (expected = IntegrityException.class)
+    public void testOnUpdatePropertiesContentChanged()
+    {
+        Map<QName, Serializable> before = ImmutableMap.of(PROP_CONTENT, new ContentData("dummyContentUrl", "text/plain",
+                0L, "UTF-8", Locale.UK));
+        Map<QName, Serializable> after = ImmutableMap.of(PROP_CONTENT, new ContentData("dummyContentUrl2", "text" +
+                "/plain", 0L, "UTF-8", Locale.UK));
+        recordAspect.onUpdateProperties(NODE_REF, before, after);
+    }
 
+    /**
+     * Check that no exception is thrown when moving record between stores
+     */
+    @Test
+    public void testOnUpdatePropertiesContentMovedToAnotherStore()
+    {
+        Map<QName, Serializable> before = ImmutableMap.of(PROP_CONTENT, new ContentData("dummyContentUrl1", "text" +
+                "/plain", 0L, "UTF-8", Locale.UK));
+        Map<QName, Serializable> after = ImmutableMap.of(PROP_CONTENT, new ContentData("dummyContentUrl2", "text" +
+                "/plain", 0L, "UTF-8", Locale.UK), PROP_STORE_NAME, "store2");
+        recordAspect.onUpdateProperties(NODE_REF, before, after);
+    }
 }