From 669a177a021fd979c960a379e30883ac01497ab5 Mon Sep 17 00:00:00 2001 From: Chris Shields Date: Fri, 16 Oct 2020 17:34:06 +0100 Subject: [PATCH] ACS-771: Fix update event to handle content and property update from null (#35) * ACS-771: Fix update event to handle content and property update from null. --- .../repo/event2/EventConsolidator.java | 18 +++ .../repo/event2/NodeResourceHelper.java | 9 +- .../repo/event2/UpdateRepoEventIT.java | 111 +++++++++++++++++- 3 files changed, 128 insertions(+), 10 deletions(-) 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 1887c6eae9..01027f93f0 100644 --- a/repository/src/main/java/org/alfresco/repo/event2/EventConsolidator.java +++ b/repository/src/main/java/org/alfresco/repo/event2/EventConsolidator.java @@ -330,6 +330,14 @@ public class EventConsolidator implements EventSupportedPolicies } } + // Handle case where the content does not exist on the propertiesBefore + if (propertiesBefore != null && !propertiesBefore.containsKey(ContentModel.PROP_CONTENT) && + propertiesAfter != null && propertiesAfter.containsKey(ContentModel.PROP_CONTENT)) + { + builder.setContent(new ContentInfo()); + resourceBeforeAllFieldsNull = false; + } + Set aspectsBefore = getMappedAspectsBefore(after.getAspectNames()); if (!aspectsBefore.isEmpty()) { @@ -407,6 +415,16 @@ public class EventConsolidator implements EventSupportedPolicies Map beforeDelta = new HashMap<>(before); beforeDelta.entrySet().removeAll(after.entrySet()); + // Add nulls for before properties + Set beforeKeys = before.keySet(); + Set newKeys = after.keySet(); + newKeys.removeAll(beforeKeys); + + for (K key : newKeys) + { + beforeDelta.put(key, null); + } + return beforeDelta; } diff --git a/repository/src/main/java/org/alfresco/repo/event2/NodeResourceHelper.java b/repository/src/main/java/org/alfresco/repo/event2/NodeResourceHelper.java index e9c1f38119..8177541523 100644 --- a/repository/src/main/java/org/alfresco/repo/event2/NodeResourceHelper.java +++ b/repository/src/main/java/org/alfresco/repo/event2/NodeResourceHelper.java @@ -171,18 +171,15 @@ public class NodeResourceHelper implements InitializingBean Map filteredProps = new HashMap<>(props.size()); props.forEach((k, v) -> { - if (!nodePropertyFilter.isExcluded(k) && v != null) + if (!nodePropertyFilter.isExcluded(k)) { - if (v instanceof MLText) + if (v != null && v instanceof MLText) { //TODO - should we send all of the values if multiple locales exist? v = ((MLText) v).getDefaultValue(); } - if (isNotEmptyString(v)) - { - filteredProps.put(getQNamePrefixString(k), v); - } + filteredProps.put(getQNamePrefixString(k), v); } }); 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 64ccb2ebbd..a9ab2e5b50 100644 --- a/repository/src/test/java/org/alfresco/repo/event2/UpdateRepoEventIT.java +++ b/repository/src/test/java/org/alfresco/repo/event2/UpdateRepoEventIT.java @@ -26,8 +26,11 @@ package org.alfresco.repo.event2; +import java.io.Serializable; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.alfresco.model.ContentModel; @@ -92,7 +95,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent assertTrue(content.getSizeInBytes() > 0); NodeResource resourceBefore = getNodeResourceBefore(resultRepoEvent); - assertNull("Content should have been null.", resourceBefore.getContent()); + assertNotNull("Content should not have been null.", resourceBefore.getContent()); // Update the content again retryingTransactionHelper.doInTransaction(() -> { @@ -135,6 +138,66 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent assertNull(resourceBefore.getPrimaryHierarchy()); } + @Test + public void testUpdateNodeResourceContent_NullBefore() + { + ContentService contentService = (ContentService) applicationContext.getBean( + "contentService"); + + final NodeRef nodeRef = createNode(ContentModel.TYPE_CONTENT); + + RepoEvent> resultRepoEvent = getRepoEvent(1); + assertEquals("Wrong repo event type.", EventType.NODE_CREATED.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); + writer.setMimetype(MimetypeMap.MIMETYPE_PDF); + writer.setEncoding("UTF-8"); + writer.putContent("test content."); + return null; + }); + + checkNumOfEvents(2); + + resultRepoEvent = getRepoEvent(2); + assertEquals("Wrong repo event type.", EventType.NODE_UPDATED.getType(), + resultRepoEvent.getType()); + + resource = getNodeResource(resultRepoEvent); + ContentInfo content = resource.getContent(); + assertNotNull(content); + assertEquals(MimetypeMap.MIMETYPE_PDF, content.getMimeType()); + assertEquals("UTF-8", content.getEncoding()); + assertTrue(content.getSizeInBytes() > 0); + + NodeResource resourceBefore = getNodeResourceBefore(resultRepoEvent); + assertNotNull("Content should not have been null.", resourceBefore.getContent()); + content = resourceBefore.getContent(); + assertNull(content.getMimeType()); + assertNull(content.getEncoding()); + assertNull(content.getSizeInBytes()); + assertNotNull(resourceBefore.getModifiedAt()); + + // Apart from the 'content' and 'modifiedAt' properties the rest should not be set + // for the resourceBefore object + assertNull(resourceBefore.getId()); + assertNull(resourceBefore.getName()); + assertNull(resourceBefore.getNodeType()); + assertNull(resourceBefore.isFile()); + assertNull(resourceBefore.isFolder()); + assertNull(resourceBefore.getModifiedByUser()); + assertNull(resourceBefore.getCreatedAt()); + assertNull(resourceBefore.getCreatedByUser()); + assertNull(resourceBefore.getProperties()); + assertNull(resourceBefore.getAspectNames()); + assertNull(resourceBefore.getPrimaryHierarchy()); + } + @Test public void testUpdateNodeResourceContentSameContentSize() { @@ -169,7 +232,7 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent assertEquals(14, (long) content.getSizeInBytes()); NodeResource resourceBefore = getNodeResourceBefore(resultRepoEvent); - assertNull("Content should have been null.", resourceBefore.getContent()); + assertNotNull("Content should not have been null.", resourceBefore.getContent()); // Update the content again - different content but same size retryingTransactionHelper.doInTransaction(() -> { @@ -247,6 +310,34 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent assertNotNull(resourceBefore.getModifiedAt()); } + @Test + public void testUpdateContentTitleFromNull() + { + final NodeRef nodeRef = createNode(ContentModel.TYPE_CONTENT); + NodeResource resource = getNodeResource(1); + + assertNotNull(resource.getProperties()); + String title = getProperty(resource, "cm:title"); + assertNull("Title should have been null.", title); + + // update content cm:title property with "test title" value + retryingTransactionHelper.doInTransaction(() -> { + nodeService.setProperty(nodeRef, ContentModel.PROP_TITLE, "test title"); + return null; + }); + + resource = getNodeResource(2); + title = getProperty(resource, "cm:title"); + assertEquals("test title", title); + + NodeResource resourceBefore = getNodeResourceBefore(2); + Map expectedResourceBeforeProperties = new HashMap<>(); + expectedResourceBeforeProperties.put("cm:title", null); + assertEquals(expectedResourceBeforeProperties, resourceBefore.getProperties()); + + assertNotNull(resourceBefore.getModifiedAt()); + } + @Test public void testUpdateContentDescription() { @@ -267,7 +358,11 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent assertEquals("test description", desc); NodeResource resourceBefore = getNodeResourceBefore(2); - assertNull(resourceBefore.getProperties()); + assertNotNull(resourceBefore.getProperties()); + + Map expectedResourceBeforeProperties = new HashMap<>(); + expectedResourceBeforeProperties.put("cm:description", null); + assertEquals(expectedResourceBeforeProperties, resourceBefore.getProperties()); } @Test @@ -338,7 +433,15 @@ public class UpdateRepoEventIT extends AbstractContextAwareRepoEvent NodeResource resourceBefore = getNodeResourceBefore(2); assertNotNull(resourceBefore.getAspectNames()); assertEquals(originalAspects, resourceBefore.getAspectNames()); - assertNull(resourceBefore.getProperties()); + assertNotNull(resourceBefore.getProperties()); + + Map expectedResourceBeforeProperties = new HashMap<>(); + expectedResourceBeforeProperties.put("cm:autoVersion", null); + expectedResourceBeforeProperties.put("cm:initialVersion", null); + expectedResourceBeforeProperties.put("cm:versionType", null); + expectedResourceBeforeProperties.put("cm:autoVersionOnUpdateProps", null); + expectedResourceBeforeProperties.put("cm:versionLabel", null); + assertEquals(expectedResourceBeforeProperties, resourceBefore.getProperties()); } @Test