From c7c8290ac50858eb24de8222e93a20095ffe37ef Mon Sep 17 00:00:00 2001 From: Sameer Zuberi Date: Tue, 30 Jan 2024 12:14:53 -0800 Subject: [PATCH] fix: retain null field in sync node merger --- .../sync/model/FullShadowSyncRequest.java | 4 ++-- .../shadowmanager/util/JsonMerger.java | 18 +----------------- .../shadowmanager/util/SyncNodeMerger.java | 7 ++++--- 3 files changed, 7 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/aws/greengrass/shadowmanager/sync/model/FullShadowSyncRequest.java b/src/main/java/com/aws/greengrass/shadowmanager/sync/model/FullShadowSyncRequest.java index 9a62bc7b..aa4ce856 100644 --- a/src/main/java/com/aws/greengrass/shadowmanager/sync/model/FullShadowSyncRequest.java +++ b/src/main/java/com/aws/greengrass/shadowmanager/sync/model/FullShadowSyncRequest.java @@ -172,13 +172,13 @@ && isDocVersionSame(localShadowDocument.get(), syncInformation, DataOwner.LOCAL) long cloudDocumentVersion = cloudShadowDocument.get().getVersion(); // If the cloud document version is different from the last sync, that means the local document needed - // some updates. So we go ahead an update the local shadow document. + // some updates. So we go ahead and update the local shadow document. if (!isDocVersionSame(cloudShadowDocument.get(), syncInformation, DataOwner.CLOUD)) { localDocumentVersion = updateLocalDocumentAndGetUpdatedVersion(updateDocument, Optional.of(localDocumentVersion)); } // If the local document version is different from the last sync, that means the cloud document needed - // some updates. So we go ahead an update the cloud shadow document. + // some updates. So we go ahead and update the cloud shadow document. if (!isDocVersionSame(localShadowDocument.get(), syncInformation, DataOwner.LOCAL)) { cloudDocumentVersion = updateCloudDocumentAndGetUpdatedVersion(updateDocument, Optional.of(cloudDocumentVersion)); diff --git a/src/main/java/com/aws/greengrass/shadowmanager/util/JsonMerger.java b/src/main/java/com/aws/greengrass/shadowmanager/util/JsonMerger.java index e9418100..6cc651ed 100644 --- a/src/main/java/com/aws/greengrass/shadowmanager/util/JsonMerger.java +++ b/src/main/java/com/aws/greengrass/shadowmanager/util/JsonMerger.java @@ -26,7 +26,7 @@ private JsonMerger() { /** * Merges the patch JSON node to the existing source JSON node. If the node already exists in the source, then - * it replaces it. If the node is an array, then the the source array's contents are overwritten with the contents + * it replaces it. If the node is an array, then the source array's contents are overwritten with the contents * from the patch. * * @param source The source JSON to merge. @@ -44,9 +44,6 @@ public static void merge(JsonNode source, final JsonNode patch) throws InvalidRe return; } - // If the source node contains keys not present in the patch node, those keys have been deleted - removeDeletedFields((ObjectNode) source, (ObjectNode) patch); - // If both nodes are objects then do a recursive patch if (source.isObject() && patch.isObject()) { merge((ObjectNode) source, (ObjectNode) patch); @@ -107,19 +104,6 @@ private static void merge(final ObjectNode source, final ObjectNode patch) { } } - private static void removeDeletedFields(final ObjectNode source, final ObjectNode patch) { - final Iterator fieldNames = source.fieldNames(); - while (fieldNames.hasNext()) { - final String field = fieldNames.next(); - final JsonNode patchValue = patch.get(field); - - // If the patch value is null then the field has been deleted, remove from source - if (isNullOrMissing(patchValue)) { - source.remove(field); - } - } - } - /** * Creates a tree that can be merged into a JsonNode. Does the following: * 1. removes nulls diff --git a/src/main/java/com/aws/greengrass/shadowmanager/util/SyncNodeMerger.java b/src/main/java/com/aws/greengrass/shadowmanager/util/SyncNodeMerger.java index 0b8c4a98..0f52b70f 100644 --- a/src/main/java/com/aws/greengrass/shadowmanager/util/SyncNodeMerger.java +++ b/src/main/java/com/aws/greengrass/shadowmanager/util/SyncNodeMerger.java @@ -142,10 +142,11 @@ private static void iterateOverAllUnvisitedFields(JsonNode local, JsonNode cloud final JsonNode cloudValue = cloud.get(field); final JsonNode baseValue = base.get(field); JsonNode mergedResult = getMergedNode(localValue, cloudValue, baseValue, owner); - if (mergedResult != null) { - ((ObjectNode) result).set(field, mergedResult); + ((ObjectNode) result).set(field, mergedResult); + if (mergedResult == null) { + visited.add(field); } - visited.add(field); + //visited.add(field); } }