diff --git a/src/integrationtests/java/com/aws/greengrass/integrationtests/SyncTest.java b/src/integrationtests/java/com/aws/greengrass/integrationtests/SyncTest.java index 49eefeae..1e230a93 100644 --- a/src/integrationtests/java/com/aws/greengrass/integrationtests/SyncTest.java +++ b/src/integrationtests/java/com/aws/greengrass/integrationtests/SyncTest.java @@ -104,7 +104,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -@SuppressWarnings("PMD.ExcessiveClassLength") +@SuppressWarnings({"PMD.ExcessiveClassLength", "PMD.CouplingBetweenObjects"}) @ExtendWith({MockitoExtension.class, GGExtension.class}) class SyncTest extends NucleusLaunchUtils { public static final String MOCK_THING_NAME_1 = "Thing1"; @@ -1480,13 +1480,75 @@ void GIVEN_local_shadow_state_empty_WHEN_shadow_manager_syncs_THEN_cloud_shadow_ assertLocalShadowEquals(initialLocalState); resp = updateHandler.handleRequest(updateRequest2, "DoAll"); - assertUpdateThingShadowHandlerResponseStateEquals(localUpdate1, resp); // null is not a valid document, so {} is returned + assertUpdateThingShadowHandlerResponseStateEquals(localUpdate2, resp); assertEmptySyncQueue(clazz); assertThat("sync info exists", () -> syncInfo.get().isPresent(), eventuallyEval(is(true))); assertThat("cloud version", () -> syncInfo.get().get().getCloudVersion(), eventuallyEval(is(2L))); assertThat("local version", () -> syncInfo.get().get().getLocalVersion(), eventuallyEval(is(3L))); assertLocalShadowEquals(finalLocalState); - assertCloudUpdateEquals(finalLocalState); + assertCloudUpdateEquals("{\"state\":null}"); // verify clear request forwarded to cloud + } + + @ParameterizedTest + @ValueSource(classes = {RealTimeSyncStrategy.class, PeriodicSyncStrategy.class}) + void GIVEN_synced_shadow_WHEN_cloud_cleared_THEN_local_cleared(Class clazz, ExtensionContext context) throws IOException, InterruptedException, IoTDataPlaneClientCreationException { + ignoreExceptionOfType(context, InterruptedException.class); + ignoreExceptionOfType(context, ResourceNotFoundException.class); + + mockCloudUpdateResponsesWithIncreasingVersions(); + setCloudThingShadow("{\"version\":1,\"state\":{\"desired\":{\"SomeKey\":\"foo\"}}}"); + + startNucleusWithConfig(NucleusLaunchUtilsConfig.builder() + .configFile(getSyncConfigFile(clazz)) + .syncClazz(clazz) + .mockCloud(true) + .build()); + waitForInitialSync(clazz, 1L, 1L); + + // empty state update does not affect shadow state + sendCloudUpdate(MOCK_THING_NAME_1, CLASSIC_SHADOW, "{\"version\":2,\"state\":{}}"); + assertSyncInfo(2L, 1L); + assertLocalShadowEquals("{\"state\":{\"desired\":{\"SomeKey\":\"foo\"}}}"); + + // null state update clears the shadow + sendCloudUpdate(MOCK_THING_NAME_1, CLASSIC_SHADOW, "{\"version\":3,\"state\":null}"); + assertSyncInfo(3L, 2L); + assertLocalShadowEquals("{\"state\":{}}"); + } + + private void mockCloudUpdateResponsesWithIncreasingVersions() throws IoTDataPlaneClientCreationException { + when(iotDataPlaneClientFactory.getIotDataPlaneClient() + .updateThingShadow(cloudUpdateThingShadowRequestCaptor.capture())) + .thenAnswer(invocation -> { + UpdateThingShadowResponse response = mock(UpdateThingShadowResponse.class); + String responseDocument = String.format("{\"version\": %d}", syncInfo.get().get().getCloudVersion() + 1); + when(response.payload()).thenReturn(SdkBytes.fromString(responseDocument, UTF_8)); + return response; + }); + } + + private void sendCloudUpdate(String thingName, String shadowName, String document) { + SyncHandler syncHandler = kernel.getContext().get(SyncHandler.class); + syncHandler.pushLocalUpdateSyncRequest(thingName, shadowName, document.getBytes(UTF_8)); + } + + private void waitForInitialSync(Class strategy, + long expectedCloudVersion, long expectedLocalVersion) throws InterruptedException { + verify(syncQueue, after(7000).atMost(4)).put(any(FullShadowSyncRequest.class)); + assertEmptySyncQueue(strategy); + assertSyncInfo(expectedCloudVersion, expectedLocalVersion); + } + + private void assertSyncInfo(long expectedCloudVersion, long expectedLocalVersion) { + assertThat("sync info exists", () -> syncInfo.get().isPresent(), eventuallyEval(is(true))); + assertThat("cloud version", () -> syncInfo.get().get().getCloudVersion(), eventuallyEval(is(expectedCloudVersion))); + assertThat("local version", syncInfo.get().get().getLocalVersion(), is(expectedLocalVersion)); + } + + private void setCloudThingShadow(String document) throws IoTDataPlaneClientCreationException { + GetThingShadowResponse initialCloudStateShadowResponse = mock(GetThingShadowResponse.class, Answers.RETURNS_DEEP_STUBS); + lenient().when(initialCloudStateShadowResponse.payload().asByteArray()).thenReturn(document.getBytes(UTF_8)); + when(iotDataPlaneClientFactory.getIotDataPlaneClient().getThingShadow(any(GetThingShadowRequest.class))).thenReturn(initialCloudStateShadowResponse); } private void assertUpdateThingShadowHandlerResponseStateEquals(String expectedDocument, UpdateThingShadowHandlerResponse resp) throws IOException { diff --git a/src/main/java/com/aws/greengrass/shadowmanager/ipc/GetThingShadowRequestHandler.java b/src/main/java/com/aws/greengrass/shadowmanager/ipc/GetThingShadowRequestHandler.java index 4d57f7a3..b30a12d3 100644 --- a/src/main/java/com/aws/greengrass/shadowmanager/ipc/GetThingShadowRequestHandler.java +++ b/src/main/java/com/aws/greengrass/shadowmanager/ipc/GetThingShadowRequestHandler.java @@ -103,7 +103,8 @@ public GetThingShadowResponse handleRequest(GetThingShadowRequest request, Strin } ObjectNode responseNode = ResponseMessageBuilder.builder() - .withState(currentShadowDocument.get().getState().toJsonWithDelta()) + .withState(currentShadowDocument.get().getState() == null ? JsonUtil.createEmptyObject() + : currentShadowDocument.get().getState().toJsonWithDelta()) .withMetadata(currentShadowDocument.get().getMetadata().toJson()) .withVersion(currentShadowDocument.get().getVersion()) .withTimestamp(Instant.now()).build(); diff --git a/src/main/java/com/aws/greengrass/shadowmanager/ipc/UpdateThingShadowRequestHandler.java b/src/main/java/com/aws/greengrass/shadowmanager/ipc/UpdateThingShadowRequestHandler.java index 5a407388..01fdc118 100644 --- a/src/main/java/com/aws/greengrass/shadowmanager/ipc/UpdateThingShadowRequestHandler.java +++ b/src/main/java/com/aws/greengrass/shadowmanager/ipc/UpdateThingShadowRequestHandler.java @@ -184,11 +184,13 @@ public UpdateThingShadowHandlerResponse handleRequest(UpdateThingShadowRequest r // to avoid double serialization, but DB stores the single document int desiredLength = 0; int reportedLength = 0; - if (!isNullOrMissing(updatedDocument.getState().getDesired())) { - desiredLength = JsonUtil.getPayloadBytes(updatedDocument.getState().getDesired()).length; - } - if (!isNullOrMissing(updatedDocument.getState().getReported())) { - reportedLength = JsonUtil.getPayloadBytes(updatedDocument.getState().getReported()).length; + if (updatedDocument.getState() != null) { + if (!isNullOrMissing(updatedDocument.getState().getDesired())) { + desiredLength = JsonUtil.getPayloadBytes(updatedDocument.getState().getDesired()).length; + } + if (!isNullOrMissing(updatedDocument.getState().getReported())) { + reportedLength = JsonUtil.getPayloadBytes(updatedDocument.getState().getReported()).length; + } } // Make sure new document is not too big @@ -242,9 +244,7 @@ public UpdateThingShadowHandlerResponse handleRequest(UpdateThingShadowRequest r .withVersion(updatedDocument.getVersion()) .withClientToken(clientToken) .withTimestamp(Instant.now()) - // explicitly convert to shadow document to return valid state. - // this is to prevent edge cases like returning null - .withState(new ShadowDocument(updateDocumentRequest, false).getState().toJson()) + .withState(updateDocumentRequest.get("state")) .withMetadata(metadata) .build(); byte[] responseNodeBytes = JsonUtil.getPayloadBytes(responseNode); diff --git a/src/main/java/com/aws/greengrass/shadowmanager/model/ShadowDocument.java b/src/main/java/com/aws/greengrass/shadowmanager/model/ShadowDocument.java index 6e382a89..5fe3b4e3 100644 --- a/src/main/java/com/aws/greengrass/shadowmanager/model/ShadowDocument.java +++ b/src/main/java/com/aws/greengrass/shadowmanager/model/ShadowDocument.java @@ -17,6 +17,7 @@ import java.io.IOException; import java.util.Optional; +import javax.annotation.Nullable; import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_METADATA; import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_STATE; @@ -29,7 +30,8 @@ */ @Getter public class ShadowDocument { - @JsonProperty(value = SHADOW_DOCUMENT_STATE, required = true) + @Nullable + @JsonProperty(SHADOW_DOCUMENT_STATE) private ShadowState state; @JsonProperty(SHADOW_DOCUMENT_METADATA) @@ -134,7 +136,7 @@ private void setFields(ShadowDocument shadowDocument, Long versionOverride) { } private void setFields(ShadowState state, ShadowStateMetadata metadata, Long version) { - this.state = state == null ? new ShadowState() : state; + this.state = state; this.metadata = metadata == null ? new ShadowStateMetadata() : metadata; this.version = version; } @@ -156,7 +158,9 @@ public boolean isNewDocument() { */ public JsonNode update(JsonNode updateDocumentRequest) { JsonNode updatedStateNode = updateDocumentRequest.get(SHADOW_DOCUMENT_STATE); - + if (this.state == null) { + this.state = new ShadowState(); + } this.state.update(updatedStateNode); JsonNode patchMetadata = this.metadata.update(updatedStateNode, this.state); // Incrementing the version here since we are creating a new version of the shadow document. @@ -172,8 +176,8 @@ public JsonNode update(JsonNode updateDocumentRequest) { * @return a JSON node containing the shadow document. */ public JsonNode toJson(boolean withVersion) { - final ObjectNode result = JsonUtil.OBJECT_MAPPER.createObjectNode(); - result.set(SHADOW_DOCUMENT_STATE, this.state.toJson()); + final ObjectNode result = JsonUtil.createEmptyObject(); + result.set(SHADOW_DOCUMENT_STATE, this.state == null ? JsonUtil.createNull() : this.state.toJson()); if (this.metadata != null) { result.set(SHADOW_DOCUMENT_METADATA, this.metadata.toJson()); } diff --git a/src/main/java/com/aws/greengrass/shadowmanager/model/ShadowState.java b/src/main/java/com/aws/greengrass/shadowmanager/model/ShadowState.java index 36c9f75f..e3b231c0 100644 --- a/src/main/java/com/aws/greengrass/shadowmanager/model/ShadowState.java +++ b/src/main/java/com/aws/greengrass/shadowmanager/model/ShadowState.java @@ -103,7 +103,7 @@ public void update(JsonNode updatedStateNode) { * @return a JSON node containing the shadow state. */ public JsonNode toJson() { - final ObjectNode result = JsonUtil.OBJECT_MAPPER.createObjectNode(); + final ObjectNode result = JsonUtil.createEmptyObject(); if (this.desired != null) { result.set(SHADOW_DOCUMENT_STATE_DESIRED, this.desired); } diff --git a/src/main/java/com/aws/greengrass/shadowmanager/sync/model/BaseSyncRequest.java b/src/main/java/com/aws/greengrass/shadowmanager/sync/model/BaseSyncRequest.java index e698393d..7bad65e3 100644 --- a/src/main/java/com/aws/greengrass/shadowmanager/sync/model/BaseSyncRequest.java +++ b/src/main/java/com/aws/greengrass/shadowmanager/sync/model/BaseSyncRequest.java @@ -98,6 +98,11 @@ abstract boolean isUpdateNecessary(SyncContext context) throws RetryableExceptio boolean isUpdateNecessary(JsonNode baseDocument, JsonNode update) { JsonNode merged = baseDocument.deepCopy(); JsonMerger.merge(merged.get(SHADOW_DOCUMENT_STATE), update.get(SHADOW_DOCUMENT_STATE)); + // explicitly handle case where state is cleared with null, + // since resulting merge will have equal documents + if (!JsonUtil.isNullStateDocument(baseDocument) && JsonUtil.isNullStateDocument(update)) { + return true; + } return !baseDocument.equals(merged); } diff --git a/src/main/java/com/aws/greengrass/shadowmanager/util/JsonUtil.java b/src/main/java/com/aws/greengrass/shadowmanager/util/JsonUtil.java index 1d525047..342c2d06 100644 --- a/src/main/java/com/aws/greengrass/shadowmanager/util/JsonUtil.java +++ b/src/main/java/com/aws/greengrass/shadowmanager/util/JsonUtil.java @@ -305,4 +305,12 @@ public static boolean hasVersion(JsonNode document) { public static long getVersion(JsonNode document) { return document.get(SHADOW_DOCUMENT_VERSION).asLong(); } + + public static ObjectNode createEmptyObject() { + return OBJECT_MAPPER.createObjectNode(); + } + + public static JsonNode createNull() { + return OBJECT_MAPPER.nullNode(); + } }