ACS-9923 Removing an aspect needs to invoke onUpdateProperties (#3504) (#3505)

repository/src/test/java/org/alfresco/repo/event2/UpdateRepoEventIT.java

@@ -26,6 +26,12 @@
 
 package org.alfresco.repo.event2;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
 import static org.alfresco.model.ContentModel.PROP_DESCRIPTION;
 
 import java.io.Serializable;
@@ -35,6 +41,9 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+
+import org.junit.Test;
+
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.content.MimetypeMap;
 import org.alfresco.repo.dictionary.M2Model;
@@ -53,7 +62,6 @@ import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.util.GUID;
 import org.alfresco.util.Pair;
-import org.junit.Test;
 
 /**
  * @author Iulian Aftene
@@ -66,20 +74,20 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
     public void testUpdateNodeResourceContent()
     {
         ContentService contentService = (ContentService) applicationContext.getBean(
-            "contentService");
+                "contentService");
 
         final NodeRef nodeRef = createNode(ContentModel.TYPE_CONTENT);
 
         RepoEvent<EventData<NodeResource>> resultRepoEvent = getRepoEvent(1);
         assertEquals("Wrong repo event type.", EventType.NODE_CREATED.getType(),
-            resultRepoEvent.getType());
+                resultRepoEvent.getType());
 
         NodeResource resource = getNodeResource(resultRepoEvent);
         assertNull("Content should have been null.", resource.getContent());
 
         retryingTransactionHelper.doInTransaction(() -> {
             ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.TYPE_CONTENT,
-                true);
+                    true);
             writer.setMimetype(MimetypeMap.MIMETYPE_PDF);
             writer.setEncoding("UTF-8");
             writer.putContent("test content.");
@@ -90,7 +98,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
 
         resultRepoEvent = getRepoEvent(2);
         assertEquals("Wrong repo event type.", EventType.NODE_UPDATED.getType(),
-            resultRepoEvent.getType());
+                resultRepoEvent.getType());
 
         resource = getNodeResource(resultRepoEvent);
         ContentInfo content = resource.getContent();
@@ -105,7 +113,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         // Update the content again
         retryingTransactionHelper.doInTransaction(() -> {
             ContentWriter writer = contentService.getWriter(nodeRef, ContentModel.TYPE_CONTENT,
-                true);
+                    true);
             writer.setMimetype(MimetypeMap.MIMETYPE_PDF);
             writer.setEncoding("UTF-8");
             writer.putContent("A quick brown fox jumps over the lazy dog.");
@@ -370,7 +378,6 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         assertEquals("new test title", title);
         assertEquals("new test title", getLocalizedProperty(resource, "cm:title", defaultLocale));
 
-
         resourceBefore = getNodeResourceBefore(3);
         title = getProperty(resourceBefore, "cm:title");
         assertEquals("Wrong old property.", "test title", title);
@@ -490,14 +497,14 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         resource = getNodeResource(2);
         assertNotNull(resource.getAspectNames());
         assertTrue(resource.getAspectNames().contains("cm:versionable"));
-        //Check all aspects
+        // Check all aspects
         Set<String> expectedAspects = new HashSet<>(originalAspects);
         expectedAspects.add("cm:versionable");
         assertEquals(expectedAspects, resource.getAspectNames());
         // Check properties
         assertFalse(resource.getProperties().isEmpty());
 
-        //Check resourceBefore
+        // Check resourceBefore
         NodeResource resourceBefore = getNodeResourceBefore(2);
         assertNotNull(resourceBefore.getAspectNames());
         assertEquals(originalAspects, resourceBefore.getAspectNames());
@@ -544,21 +551,64 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         assertEquals(aspectsBeforeRemove, resourceBefore.getAspectNames());
     }
 
+    @Test
+    public void testRemoveAspectPropertiesTest()
+    {
+        final NodeRef nodeRef = createNode(ContentModel.TYPE_CONTENT);
+        NodeResource resource = getNodeResource(1);
+        final Set<String> originalAspects = resource.getAspectNames();
+        assertNotNull(originalAspects);
+
+        // Add cm:geographic aspect with properties
+        retryingTransactionHelper.doInTransaction(() -> {
+            Map<QName, Serializable> aspectProperties = new HashMap<>();
+            aspectProperties.put(ContentModel.PROP_LATITUDE, "12.345678");
+            aspectProperties.put(ContentModel.PROP_LONGITUDE, "12.345678");
+            nodeService.addAspect(nodeRef, ContentModel.ASPECT_GEOGRAPHIC, aspectProperties);
+            return null;
+        });
+        resource = getNodeResource(2);
+        Set<String> aspectsBeforeRemove = resource.getAspectNames();
+        assertNotNull(aspectsBeforeRemove);
+        assertTrue(aspectsBeforeRemove.contains("cm:geographic"));
+
+        // Remove cm:geographic aspect - this automatically removes the properties from the node
+        retryingTransactionHelper.doInTransaction(() -> {
+            nodeService.removeAspect(nodeRef, ContentModel.ASPECT_GEOGRAPHIC);
+            return null;
+        });
+
+        resource = getNodeResource(3);
+        assertEquals(originalAspects, resource.getAspectNames());
+
+        NodeResource resourceBefore = getNodeResourceBefore(3);
+        assertNotNull(resourceBefore.getAspectNames());
+        assertEquals(aspectsBeforeRemove, resourceBefore.getAspectNames());
+        // Resource before should contain cm:latitude and cm:longitude properties
+        assertNotNull(resourceBefore.getProperties());
+        assertTrue(resourceBefore.getProperties().containsKey("cm:latitude"));
+        assertTrue(resourceBefore.getProperties().containsKey("cm:longitude"));
+        // Resource after should NOT contain cm:latitude and cm:longitude properties
+        assertNotNull(resource.getProperties());
+        assertFalse(resource.getProperties().containsKey("cm:latitude"));
+        assertFalse(resource.getProperties().containsKey("cm:longitude"));
+    }
+
     @Test
     public void testCreateAndUpdateInTheSameTransaction()
     {
         retryingTransactionHelper.doInTransaction(() -> {
 
             NodeRef node1 = nodeService.createNode(
-                rootNodeRef,
-                ContentModel.ASSOC_CHILDREN,
-                QName.createQName(TEST_NAMESPACE, GUID.generate()),
-                ContentModel.TYPE_CONTENT).getChildRef();
+                    rootNodeRef,
+                    ContentModel.ASSOC_CHILDREN,
+                    QName.createQName(TEST_NAMESPACE, GUID.generate()),
+                    ContentModel.TYPE_CONTENT).getChildRef();
 
             nodeService.setProperty(node1, PROP_DESCRIPTION, "test description");
             return null;
         });
-        //Create and update node are done in the same transaction so one event is expected
+        // Create and update node are done in the same transaction so one event is expected
         // to be generated
         checkNumOfEvents(1);
     }
@@ -593,8 +643,8 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         assertEquals("Incorrect node type was found", "cm:folder", nodeResource.getNodeType());
 
         NodeResource resourceBefore = getNodeResourceBefore(2);
-        assertEquals("Incorrect node type was found","cm:content", resourceBefore.getNodeType());
-        // assertNotNull(resourceBefore.getModifiedAt());  uncomment this when the issue will be fixed
+        assertEquals("Incorrect node type was found", "cm:content", resourceBefore.getNodeType());
+        // assertNotNull(resourceBefore.getModifiedAt()); uncomment this when the issue will be fixed
         assertNull(resourceBefore.getId());
         assertNull(resourceBefore.getContent());
         assertNull(resourceBefore.isFile());
@@ -624,8 +674,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         m2Type.setTitle("Test type title");
 
         // Create active model
-        CustomModelDefinition modelDefinition =
-            retryingTransactionHelper.doInTransaction(() -> customModelService.createCustomModel(model, true));
+        CustomModelDefinition modelDefinition = retryingTransactionHelper.doInTransaction(() -> customModelService.createCustomModel(model, true));
 
         assertNotNull(modelDefinition);
         assertEquals(modelName, modelDefinition.getName().getLocalName());
@@ -655,7 +704,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         assertEquals("Wrong repo event type.", EventType.NODE_CREATED.getType(), resultRepoEvent.getType());
         assertEquals("cm:content node type was not found", "cm:content", nodeResource.getNodeType());
 
-        QName typeQName = QName.createQName("{" + namespacePair.getFirst()+ "}" + typeName);
+        QName typeQName = QName.createQName("{" + namespacePair.getFirst() + "}" + typeName);
         retryingTransactionHelper.doInTransaction(() -> {
             nodeService.setType(nodeRef, typeQName);
 
@@ -757,7 +806,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         // we should have only 1 event, node.Created
         checkNumOfEvents(1);
 
-        RepoEvent<EventData<NodeResource>>  resultRepoEvent = getRepoEvent(1);
+        RepoEvent<EventData<NodeResource>> resultRepoEvent = getRepoEvent(1);
         assertEquals("Wrong repo event type.", EventType.NODE_CREATED.getType(), resultRepoEvent.getType());
         NodeResource nodeResource = getNodeResource(resultRepoEvent);
         assertEquals("Incorrect node type was found", "cm:folder", nodeResource.getNodeType());
@@ -783,10 +832,10 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
 
         retryingTransactionHelper.doInTransaction(() -> {
             nodeService.moveNode(
-                moveFile,
-                folder2,
-                ContentModel.ASSOC_CONTAINS,
-                QName.createQName(TEST_NAMESPACE));
+                    moveFile,
+                    folder2,
+                    ContentModel.ASSOC_CONTAINS,
+                    QName.createQName(TEST_NAMESPACE));
             return null;
         });
 
@@ -801,7 +850,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         assertEquals("Wrong node parent.", folder1ID, moveFileParentBeforeMove);
         assertEquals("Wrong node parent.", folder2ID, moveFileParentAfterMove);
         assertEquals("Wrong repo event type.", EventType.NODE_UPDATED.getType(),
-            getRepoEvent(4).getType());
+                getRepoEvent(4).getType());
 
         assertNull(resourceBefore.getId());
         assertNull(resourceBefore.getName());
@@ -833,10 +882,10 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
 
         retryingTransactionHelper.doInTransaction(() -> {
             nodeService.moveNode(
-                moveFolder,
-                grandParent,
-                ContentModel.ASSOC_CONTAINS,
-                QName.createQName(TEST_NAMESPACE));
+                    moveFolder,
+                    grandParent,
+                    ContentModel.ASSOC_CONTAINS,
+                    QName.createQName(TEST_NAMESPACE));
             return null;
         });
 
@@ -845,15 +894,13 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         final String grandParentID = getNodeResource(1).getId();
         final String parentID = getNodeResource(2).getId();
 
-        final String moveFolderParentBeforeMove =
-            getNodeResourceBefore(4).getPrimaryHierarchy().get(0);
-        final String moveFolderParentAfterMove =
-            getNodeResource(4).getPrimaryHierarchy().get(0);
+        final String moveFolderParentBeforeMove = getNodeResourceBefore(4).getPrimaryHierarchy().get(0);
+        final String moveFolderParentAfterMove = getNodeResource(4).getPrimaryHierarchy().get(0);
 
         assertEquals("Wrong node parent.", parentID, moveFolderParentBeforeMove);
         assertEquals("Wrong node parent.", grandParentID, moveFolderParentAfterMove);
         assertEquals("Wrong repo event type.", EventType.NODE_UPDATED.getType(),
-            getRepoEventWithoutWait(4).getType());
+                getRepoEventWithoutWait(4).getType());
     }
 
     @Test
@@ -867,28 +914,25 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
 
         retryingTransactionHelper.doInTransaction(() -> {
             nodeService.moveNode(
-                grandParent,
-                root2,
-                ContentModel.ASSOC_CONTAINS,
-                QName.createQName(TEST_NAMESPACE));
+                    grandParent,
+                    root2,
+                    ContentModel.ASSOC_CONTAINS,
+                    QName.createQName(TEST_NAMESPACE));
             return null;
         });
 
         checkNumOfEvents(6);
 
         final String root2ID = getNodeResource(2).getId();
-        final String grandParentParentAfterMove =
-            getNodeResource(6).getPrimaryHierarchy().get(0);
+        final String grandParentParentAfterMove = getNodeResource(6).getPrimaryHierarchy().get(0);
         assertEquals("Wrong node parent.", root2ID, grandParentParentAfterMove);
 
         final String grandParentID = getNodeResource(3).getId();
-        final String parentIDOfTheParentFolder =
-            getNodeResource(4).getPrimaryHierarchy().get(0);
+        final String parentIDOfTheParentFolder = getNodeResource(4).getPrimaryHierarchy().get(0);
         assertEquals("Wrong node parent.", grandParentID, parentIDOfTheParentFolder);
 
         final String parentID = getNodeResource(4).getId();
-        final String contentParentID =
-            getNodeResource(5).getPrimaryHierarchy().get(0);
+        final String contentParentID = getNodeResource(5).getPrimaryHierarchy().get(0);
         assertEquals("Wrong node parent.", parentID, contentParentID);
     }
 
@@ -906,10 +950,10 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
 
         retryingTransactionHelper.doInTransaction(() -> {
             nodeService.moveNode(
-                moveFile,
-                folder2,
-                ContentModel.ASSOC_CONTAINS,
-                QName.createQName(TEST_NAMESPACE));
+                    moveFile,
+                    folder2,
+                    ContentModel.ASSOC_CONTAINS,
+                    QName.createQName(TEST_NAMESPACE));
             return null;
         });
 
@@ -918,8 +962,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         assertTrue("Wrong aspect.", resource.getAspectNames().contains("cm:versionable"));
 
         final String folder2ID = getNodeResource(2).getId();
-        final String moveFileParentAfterMove =
-            getNodeResource(5).getPrimaryHierarchy().get(0);
+        final String moveFileParentAfterMove = getNodeResource(5).getPrimaryHierarchy().get(0);
 
         assertEquals("Wrong node parent.", folder2ID, moveFileParentAfterMove);
     }
@@ -935,10 +978,10 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
             nodeService.setProperty(moveFile, ContentModel.PROP_NAME, "test_new_name");
 
             nodeService.moveNode(
-                moveFile,
-                folder2,
-                ContentModel.ASSOC_CONTAINS,
-                QName.createQName(TEST_NAMESPACE));
+                    moveFile,
+                    folder2,
+                    ContentModel.ASSOC_CONTAINS,
+                    QName.createQName(TEST_NAMESPACE));
             return null;
         });
 
@@ -946,8 +989,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         assertEquals("test_new_name", resource.getName());
 
         final String folder2ID = getNodeResource(2).getId();
-        final String moveFileParentAfterMove =
-            getNodeResource(4).getPrimaryHierarchy().get(0);
+        final String moveFileParentAfterMove = getNodeResource(4).getPrimaryHierarchy().get(0);
 
         assertEquals("Wrong node parent.", folder2ID, moveFileParentAfterMove);
     }
@@ -958,28 +1000,28 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         retryingTransactionHelper.doInTransaction(() -> {
 
             NodeRef folder1 = nodeService.createNode(
-                rootNodeRef,
-                ContentModel.ASSOC_CHILDREN,
-                QName.createQName(TEST_NAMESPACE),
-                ContentModel.TYPE_FOLDER).getChildRef();
+                    rootNodeRef,
+                    ContentModel.ASSOC_CHILDREN,
+                    QName.createQName(TEST_NAMESPACE),
+                    ContentModel.TYPE_FOLDER).getChildRef();
 
             NodeRef folder2 = nodeService.createNode(
-                rootNodeRef,
-                ContentModel.ASSOC_CHILDREN,
-                QName.createQName(TEST_NAMESPACE),
-                ContentModel.TYPE_FOLDER).getChildRef();
+                    rootNodeRef,
+                    ContentModel.ASSOC_CHILDREN,
+                    QName.createQName(TEST_NAMESPACE),
+                    ContentModel.TYPE_FOLDER).getChildRef();
 
             NodeRef fileToMove = nodeService.createNode(
-                folder1,
-                ContentModel.ASSOC_CONTAINS,
-                QName.createQName(TEST_NAMESPACE),
-                ContentModel.TYPE_CONTENT).getChildRef();
+                    folder1,
+                    ContentModel.ASSOC_CONTAINS,
+                    QName.createQName(TEST_NAMESPACE),
+                    ContentModel.TYPE_CONTENT).getChildRef();
 
             nodeService.moveNode(
-                fileToMove,
-                folder2,
-                ContentModel.ASSOC_CONTAINS,
-                QName.createQName(TEST_NAMESPACE));
+                    fileToMove,
+                    folder2,
+                    ContentModel.ASSOC_CONTAINS,
+                    QName.createQName(TEST_NAMESPACE));
 
             assertEquals(folder2, nodeService.getPrimaryParent(fileToMove).getParentRef());
 
@@ -989,8 +1031,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         checkNumOfEvents(3);
 
         final String folder2ID = getNodeResource(2).getId();
-        final String moveFileParentAfterMove =
-            getNodeResource(3).getPrimaryHierarchy().get(0);
+        final String moveFileParentAfterMove = getNodeResource(3).getPrimaryHierarchy().get(0);
 
         assertEquals("Wrong node parent.", folder2ID, moveFileParentAfterMove);
     }
@@ -1003,7 +1044,6 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         final Set<String> originalAspects = resource.getAspectNames();
         assertNotNull(originalAspects);
 
-
         retryingTransactionHelper.doInTransaction(() -> {
             // Add cm:geographic aspect with default value
             nodeService.addAspect(nodeRef, ContentModel.ASPECT_GEOGRAPHIC, null);