Skip to content

[Fix] Leave empty objects in metadata #213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: release_2.3.x
Choose a base branch
from
Open

[Fix] Leave empty objects in metadata #213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c3e293
[Fix] Leave empty objects in metadata
kstribrnAmzn Oct 3, 2025
ff39ae7
Account for fields being added and removed at the same time
kstribrnAmzn Oct 10, 2025
c62b7d2
Update variable creation order
kstribrnAmzn Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 32 additions & 14 deletions src/main/java/com/aws/greengrass/shadowmanager/model/ShadowStateMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,32 +70,40 @@ public ShadowStateMetadata deepCopy() {
*/
@SuppressWarnings("PMD.NullAssignment")
public JsonNode update(JsonNode patch, ShadowState state) {
// Create the patch metadata tree. This will transform nulls to metadata nodes.
final JsonNode metadataPatch = createMetadataPatch(patch);
// The persisted metadata should have all removed fields (aka value null) actually removed
final JsonNode metadataPatchWithoutRemovedFields = createMetadataPatch(patch, true);

// If the thing now has null state after the update then the metadata should also be null
if (state.isEmpty()) {
desired = null;
reported = null;
return metadataPatch;
return metadataPatchWithoutRemovedFields;
}

// Removed field null values need to be kept when merging so that the field is removed from metadata
final JsonNode metadataPatchWithRemovedFields = createMetadataPatch(patch, false);

// Merge in the desired metadata
final JsonNode patchDesired = metadataPatch.get(SHADOW_DOCUMENT_STATE_DESIRED);
final JsonNode patchDesired = metadataPatchWithRemovedFields.get(SHADOW_DOCUMENT_STATE_DESIRED);
if (!isNullOrMissing(patchDesired)) {
desired = nullIfEmpty(merge(state.getDesired(), desired, patchDesired));
}

// Merge in the reported metadata
final JsonNode patchReported = metadataPatch.get(SHADOW_DOCUMENT_STATE_REPORTED);
final JsonNode patchReported = metadataPatchWithRemovedFields.get(SHADOW_DOCUMENT_STATE_REPORTED);
if (!isNullOrMissing(patchReported)) {
reported = nullIfEmpty(merge(state.getReported(), reported, patchReported));
}

return metadataPatch;
return metadataPatchWithoutRemovedFields;
}

private JsonNode createMetadataPatch(final JsonNode source) {
private JsonNode createMetadataPatch(final JsonNode source, boolean removeFields) {
// If the JsonNode is a NullNode then this field should be removed from the metadata
if (source.isNull()) {
return null;
}

if (source.isValueNode()) {
ObjectNode node = JsonUtil.OBJECT_MAPPER.createObjectNode();
node.set(SHADOW_DOCUMENT_TIMESTAMP, new LongNode(this.clock.instant().getEpochSecond()));
Expand All @@ -105,7 +113,7 @@ private JsonNode createMetadataPatch(final JsonNode source) {
if (source.isArray()) {
final ArrayNode result = JsonUtil.OBJECT_MAPPER.createArrayNode();
for (final JsonNode node : source) {
result.add(createMetadataPatch(node));
result.add(createMetadataPatch(node, removeFields));
}
return result;
}
Expand All @@ -117,7 +125,12 @@ private JsonNode createMetadataPatch(final JsonNode source) {
while (fieldIter.hasNext()) {
final String fieldName = fieldIter.next();
final JsonNode node = sourceObject.get(fieldName);
result.set(fieldName, createMetadataPatch(node));
JsonNode nodeMetadataPatch = createMetadataPatch(node, !removeFields);

// If the field isn't being removed then recurse
if (!removeFields || nodeMetadataPatch != null) {
result.set(fieldName, nodeMetadataPatch);
}
}
return result;
}
Expand Down Expand Up @@ -145,9 +158,9 @@ private void merge(final ObjectNode state, final ObjectNode metadata, final Obje
JsonNode metadataFieldNode = metadata.get(patchFieldName);
final JsonNode stateFieldNode = state.get(patchFieldName);

// If the state doesn't have the node then it was remove from state and should be
// removed from metadata if present.
if (isNullOrMissing(stateFieldNode)) {
// If the state doesn't have the node then it was removed from state and should be
// removed from metadata if present. If it's an object then leave it as empty.
if (patchFieldNode == null || patchFieldNode.isNull()) {
metadata.remove(patchFieldName);
continue;
}
Expand Down Expand Up @@ -196,9 +209,14 @@ private void merge(final ObjectNode state, final ObjectNode metadata, final Obje
}

// Now we have gotten to the case where the original and patch nodes are the same type and are not
// metadata nodes, recurse.
// metadata nodes, recurse if not an empty object.
if (patchFieldNode.isObject()) {
merge((ObjectNode) stateFieldNode, (ObjectNode) metadataFieldNode, (ObjectNode) patchFieldNode);
// If the patch is an empty object, then the metadata should be set to an empty object.
if (patchFieldNode.isEmpty()) {
metadata.set(patchFieldName, patchFieldNode);
} else {
merge((ObjectNode) stateFieldNode, (ObjectNode) metadataFieldNode, (ObjectNode) patchFieldNode);
}
}
}
}
Expand Down
125 changes: 124 additions & 1 deletion src/test/java/com/aws/greengrass/shadowmanager/model/ShadowStateMetadataTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ void GIVEN_non_empty_state_with_null_field_and_non_empty_patch_with_array_WHEN_u

JsonNode patchMetadata = shadowStateMetadata.update(patchJson.get(), state);

assertTrue(JsonUtil.isNullOrMissing(shadowStateMetadata.getDesired()));
assertFalse(JsonUtil.isNullOrMissing(shadowStateMetadata.getDesired()));
assertTrue(JsonUtil.isNullOrMissing(shadowStateMetadata.getReported()));

assertFalse(JsonUtil.isNullOrMissing(patchMetadata));
Expand All @@ -237,6 +237,129 @@ void GIVEN_non_empty_state_with_null_field_and_non_empty_patch_with_array_WHEN_u
assertThat(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).get("SomObject").get(SHADOW_DOCUMENT_TIMESTAMP).asLong(), is(timestamp));
}

@Test
void GIVEN_non_empty_state_with_nested_empty_object_and_non_empty_patch_WHEN_update_THEN_metadata_is_correctly_updated() throws IOException {
String stateString = "{\"SomeObject\": {\"SomeVal\": \"123\"}}";
JsonNode stateJson = JsonUtil.getPayloadJson(stateString.getBytes()).get();
String patchString = "{\"desired\": {\"SomeObject\": {\"SomeVal\": null}}}";
Optional<JsonNode> patchJson = JsonUtil.getPayloadJson(patchString.getBytes());
String patchMetadataString = "{\"SomeObject\": {\"SomeVal\": \"123\"}}";
Optional<JsonNode> patchMetadataJson = JsonUtil.getPayloadJson(patchMetadataString.getBytes());

ShadowStateMetadata shadowStateMetadata = new ShadowStateMetadata(patchMetadataJson.get(), null, mockClock);
ShadowState state = new ShadowState(stateJson, null);

JsonNode patchMetadata = shadowStateMetadata.update(patchJson.get(), state);

// The desired field will be present because of the nested object but the object will be empty
assertFalse(JsonUtil.isNullOrMissing(shadowStateMetadata.getDesired()));
String expectedDesired = "{\"SomeObject\": {}}";
assertThat(shadowStateMetadata.getDesired(), is(JsonUtil.getPayloadJson(expectedDesired.getBytes()).get()));

assertTrue(JsonUtil.isNullOrMissing(shadowStateMetadata.getReported()));

assertFalse(JsonUtil.isNullOrMissing(patchMetadata));
assertTrue(patchMetadata.has(SHADOW_DOCUMENT_STATE_DESIRED));
assertTrue(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).has("SomeObject"));
assertThat(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).get("SomeObject"), is(JsonUtil.OBJECT_MAPPER.createObjectNode()));
}

@Test
void GIVEN_non_empty_state_with_nested_object_and_nested_field_removed_WHEN_update_THEN_metadata_is_correctly_updated() throws IOException {
String stateString = "{\"SomeObject\": {\"SomeVal\": \"123\", \"AnotherVal\": \"4567\"}}";
JsonNode stateJson = JsonUtil.getPayloadJson(stateString.getBytes()).get();
String patchString = "{\"desired\": {\"SomeObject\": {\"SomeVal\": null, \"AnotherVal\": \"4567\"}}}";
Optional<JsonNode> patchJson = JsonUtil.getPayloadJson(patchString.getBytes());
String patchMetadataString = "{\"SomeObject\": {\"SomeVal\": \"123\", \"AnotherVal\": \"4567\"}}";
Optional<JsonNode> patchMetadataJson = JsonUtil.getPayloadJson(patchMetadataString.getBytes());

ShadowStateMetadata shadowStateMetadata = new ShadowStateMetadata(patchMetadataJson.get(), null, mockClock);
ShadowState state = new ShadowState(stateJson, null);

JsonNode patchMetadata = shadowStateMetadata.update(patchJson.get(), state);

// The desired field will be present because of the nested object but the object will be empty
assertFalse(JsonUtil.isNullOrMissing(shadowStateMetadata.getDesired()));
assertTrue(shadowStateMetadata.getDesired().has("SomeObject"));
assertTrue(shadowStateMetadata.getDesired().get("SomeObject").has("AnotherVal"));
assertTrue(shadowStateMetadata.getDesired().get("SomeObject").get("AnotherVal").has(SHADOW_DOCUMENT_TIMESTAMP));
assertThat(shadowStateMetadata.getDesired().get("SomeObject").get("AnotherVal").get(SHADOW_DOCUMENT_TIMESTAMP).asLong(),
is(timestamp));

assertTrue(JsonUtil.isNullOrMissing(shadowStateMetadata.getReported()));

assertFalse(JsonUtil.isNullOrMissing(patchMetadata));
assertTrue(patchMetadata.has(SHADOW_DOCUMENT_STATE_DESIRED));
assertTrue(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).has("SomeObject"));
assertThat(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).get("SomeObject").get("AnotherVal").get(SHADOW_DOCUMENT_TIMESTAMP).asLong(), is(timestamp));
assertFalse(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).get("SomeObject").has("SomeVal"));
}

@Test
void GIVEN_non_empty_state_with_nested_object_and_nested_field_added_WHEN_update_THEN_metadata_is_correctly_updated() throws IOException {
String stateString = "{\"SomeObject\": {\"SomeVal\": \"123\"}}";
JsonNode stateJson = JsonUtil.getPayloadJson(stateString.getBytes()).get();
String patchString = "{\"desired\": {\"SomeObject\": {\"SomeVal\": \"456\", \"AnotherVal\": " +
"\"testingValue\"}}}";
Optional<JsonNode> patchJson = JsonUtil.getPayloadJson(patchString.getBytes());
String patchMetadataString = "{\"SomeObject\": {\"SomeVal\": \"123\", \"AnotherVal\": \"testingValue\"}}";
Optional<JsonNode> patchMetadataJson = JsonUtil.getPayloadJson(patchMetadataString.getBytes());

ShadowStateMetadata shadowStateMetadata = new ShadowStateMetadata(patchMetadataJson.get(), null, mockClock);
ShadowState state = new ShadowState(stateJson, null);

JsonNode patchMetadata = shadowStateMetadata.update(patchJson.get(), state);

// The desired field will be present because of the nested object but the object will be empty
assertFalse(JsonUtil.isNullOrMissing(shadowStateMetadata.getDesired()));
assertTrue(shadowStateMetadata.getDesired().has("SomeObject"));
assertTrue(shadowStateMetadata.getDesired().get("SomeObject").has("AnotherVal"));
assertTrue(shadowStateMetadata.getDesired().get("SomeObject").get("AnotherVal").has(SHADOW_DOCUMENT_TIMESTAMP));
assertThat(shadowStateMetadata.getDesired().get("SomeObject").get("AnotherVal").get(SHADOW_DOCUMENT_TIMESTAMP).asLong(),
is(timestamp));

assertTrue(JsonUtil.isNullOrMissing(shadowStateMetadata.getReported()));

assertFalse(JsonUtil.isNullOrMissing(patchMetadata));
assertTrue(patchMetadata.has(SHADOW_DOCUMENT_STATE_DESIRED));
assertTrue(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).has("SomeObject"));
assertThat(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).get("SomeObject").get("SomeVal").get(SHADOW_DOCUMENT_TIMESTAMP).asLong(), is(timestamp));
assertThat(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).get("SomeObject").get("AnotherVal").get(SHADOW_DOCUMENT_TIMESTAMP).asLong(), is(timestamp));
}

@Test
void GIVEN_non_empty_state_with_nested_object_and_nested_field_exchanged_WHEN_update_THEN_metadata_is_correctly_updated() throws IOException {
String stateString = "{\"SomeObject\": {\"SomeVal\": \"123\"}}";
JsonNode stateJson = JsonUtil.getPayloadJson(stateString.getBytes()).get();
String patchString = "{\"desired\": {\"SomeObject\": {\"SomeVal\": null, \"AnotherVal\": " +
"\"testingValue\"}}}";
Optional<JsonNode> patchJson = JsonUtil.getPayloadJson(patchString.getBytes());
String patchMetadataString = "{\"SomeObject\": {\"SomeVal\": \"123\", \"AnotherVal\": \"testingValue\"}}";
Optional<JsonNode> patchMetadataJson = JsonUtil.getPayloadJson(patchMetadataString.getBytes());

ShadowStateMetadata shadowStateMetadata = new ShadowStateMetadata(patchMetadataJson.get(), null, mockClock);
ShadowState state = new ShadowState(stateJson, null);

JsonNode patchMetadata = shadowStateMetadata.update(patchJson.get(), state);

// The desired field will be present because of the nested object but the object will be empty
assertFalse(JsonUtil.isNullOrMissing(shadowStateMetadata.getDesired()));
assertTrue(shadowStateMetadata.getDesired().has("SomeObject"));
assertFalse(shadowStateMetadata.getDesired().get("SomeObject").has("SomeVal"));
assertTrue(shadowStateMetadata.getDesired().get("SomeObject").has("AnotherVal"));
assertTrue(shadowStateMetadata.getDesired().get("SomeObject").get("AnotherVal").has(SHADOW_DOCUMENT_TIMESTAMP));
assertThat(shadowStateMetadata.getDesired().get("SomeObject").get("AnotherVal").get(SHADOW_DOCUMENT_TIMESTAMP).asLong(),
is(timestamp));

assertTrue(JsonUtil.isNullOrMissing(shadowStateMetadata.getReported()));

assertFalse(JsonUtil.isNullOrMissing(patchMetadata));
assertTrue(patchMetadata.has(SHADOW_DOCUMENT_STATE_DESIRED));
assertTrue(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).has("SomeObject"));
assertFalse(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).get("SomeObject").has("SomeVal"));
assertThat(patchMetadata.get(SHADOW_DOCUMENT_STATE_DESIRED).get("SomeObject").get("AnotherVal").get(SHADOW_DOCUMENT_TIMESTAMP).asLong(), is(timestamp));
}

@Test
void GIVEN_reported_and_desired_metadata_WHEN_toJson_THEN_gets_the_correct_json() throws IOException {
Optional<JsonNode> patchMetadataJson = JsonUtil.getPayloadJson(patchMetadataString.getBytes());
Expand Down
Loading