From 27d5e144a922929b68c215d298aa6209dd33c7fd Mon Sep 17 00:00:00 2001
From: Chris Shields <christopher.shields@alfresco.com>
Date: Mon, 9 Nov 2020 11:34:53 +0000
Subject: [PATCH] REPO-5209: Prevent creation of node updated event when
 property added and removed in same transaction (#113)

---
 README.md                                     |  2 +-
 .../repo/event2/EventConsolidator.java        | 14 +++++----
 .../repo/event2/UpdateRepoEventIT.java        | 29 +++++++++++++++++++
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/README.md b/README.md
index 948bd85c85..f8fc5a83a7 100644
--- a/README.md
+++ b/README.md
@@ -128,7 +128,7 @@ cd acs-community-packaging
 mvn clean install -PcommunityDocker -Dmaven.javadoc.skip=true
 cd ..
 ~~~
-In Itellij IDEA, create a new project using the `work` directory as the source.
+In Intellij IDEA, create a new project using the `work` directory as the source.
 * File > New Project from Existing Sources > .../work > Maven
 
 ## Docker
diff --git a/repository/src/main/java/org/alfresco/repo/event2/EventConsolidator.java b/repository/src/main/java/org/alfresco/repo/event2/EventConsolidator.java
index 01027f93f0..ec21920f22 100644
--- a/repository/src/main/java/org/alfresco/repo/event2/EventConsolidator.java
+++ b/repository/src/main/java/org/alfresco/repo/event2/EventConsolidator.java
@@ -292,6 +292,7 @@ public class EventConsolidator implements EventSupportedPolicies
 
         Builder builder = NodeResource.builder();
 
+        ZonedDateTime modifiedAt = null;
         Map<QName, Serializable> changedPropsBefore = getBeforeMapChanges(propertiesBefore, propertiesAfter);
         if (!changedPropsBefore.isEmpty())
         {
@@ -321,13 +322,8 @@ public class EventConsolidator implements EventSupportedPolicies
                 builder.setModifiedByUser(modifier);
                 resourceBeforeAllFieldsNull = false;
             }
-            ZonedDateTime modifiedAt =
+            modifiedAt =
                         helper.getZonedDateTime((Date) changedPropsBefore.get(ContentModel.PROP_MODIFIED));
-            if (modifiedAt != null)
-            {
-                builder.setModifiedAt(modifiedAt);
-                resourceBeforeAllFieldsNull = false;
-            }
         }
 
         // Handle case where the content does not exist on the propertiesBefore
@@ -357,6 +353,12 @@ public class EventConsolidator implements EventSupportedPolicies
             resourceBeforeAllFieldsNull = false;
         }
 
+        // Only set modifiedAt if one of the other fields is also not null
+        if (modifiedAt != null && !resourceBeforeAllFieldsNull)
+        {
+            builder.setModifiedAt(modifiedAt);
+        }
+
         return builder.build();
     }
 
diff --git a/repository/src/test/java/org/alfresco/repo/event2/UpdateRepoEventIT.java b/repository/src/test/java/org/alfresco/repo/event2/UpdateRepoEventIT.java
index a9ab2e5b50..f1802e165b 100644
--- a/repository/src/test/java/org/alfresco/repo/event2/UpdateRepoEventIT.java
+++ b/repository/src/test/java/org/alfresco/repo/event2/UpdateRepoEventIT.java
@@ -30,6 +30,7 @@ import java.io.Serializable;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -978,4 +979,32 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent
         assertEquals(1, aspectsBefore.size());
         assertTrue(aspectsBefore.contains("cm:auditable"));
     }
+
+    @Test
+    public void testAddAndRemovePropertyInTheSameTransaction()
+    {
+        final NodeRef nodeRef = createNode(ContentModel.TYPE_CONTENT);
+
+        checkNumOfEvents(1);
+
+        NodeResource resource = getNodeResource(1);
+        // Check properties
+        assertTrue(resource.getProperties().isEmpty());
+
+        // Add and remove cm:userName property
+        retryingTransactionHelper.doInTransaction(() -> {
+            Map<QName, Serializable> properties = Map.of(ContentModel.PROP_USERNAME, "user1");
+            nodeService.addProperties(nodeRef, properties);
+            nodeService.removeProperty(nodeRef, ContentModel.PROP_USERNAME);
+            return null;
+        });
+
+        // There should only be a create event
+        resource = getNodeResource(1);
+        assertTrue(resource.getProperties().isEmpty());
+
+        // Check there isn't a node update event
+        List<RepoEvent<EventData<NodeResource>>> nodeUpdatedEvents = getFilteredEvents(EventType.NODE_UPDATED);
+        assertEquals(0, nodeUpdatedEvents.size());
+    }
 }