Skip to content

Commit

Permalink
fix: drop reported delta in update shadow (#178)
Browse files Browse the repository at this point in the history
  • Loading branch information
MikeDombo authored Mar 29, 2023
1 parent a8d71cc commit 2e8d744
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import static com.aws.greengrass.shadowmanager.model.Constants.LOG_SHADOW_NAME_KEY;
import static com.aws.greengrass.shadowmanager.model.Constants.LOG_THING_NAME_KEY;
import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_METADATA;
import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_STATE;
import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_STATE_DELTA;
import static com.aws.greengrass.shadowmanager.util.JsonUtil.isNullOrMissing;
import static software.amazon.awssdk.aws.greengrass.GreengrassCoreIPCService.UPDATE_THING_SHADOW;

Expand Down Expand Up @@ -129,6 +131,14 @@ public UpdateThingShadowHandlerResponse handleRequest(UpdateThingShadowRequest r
.orElseThrow(() ->
new InvalidRequestParametersException(ErrorMessage
.createInvalidPayloadJsonMessage("")));

// drop "delta" from the state (if we have a state).
// delta isn't valid for users to set.
if (updateDocumentRequest.has(SHADOW_DOCUMENT_STATE)
&& updateDocumentRequest.get(SHADOW_DOCUMENT_STATE).isObject()) {
((ObjectNode) updateDocumentRequest.get(SHADOW_DOCUMENT_STATE))
.remove(SHADOW_DOCUMENT_STATE_DELTA);
}
// Validate the payload schema
JsonUtil.validatePayloadSchema(updateDocumentRequest);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import static com.aws.greengrass.shadowmanager.model.Constants.ERROR_MESSAGE_FIELD_NAME;
import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_METADATA;
import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_STATE;
import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_STATE_DELTA;
import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_TIMESTAMP;
import static com.aws.greengrass.shadowmanager.model.Constants.SHADOW_DOCUMENT_VERSION;
import static com.aws.greengrass.testcommons.testutilities.ExceptionLogProtector.ignoreExceptionOfType;
Expand All @@ -70,10 +71,12 @@
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.times;
Expand All @@ -97,6 +100,7 @@ class UpdateThingShadowRequestHandlerTest {
private final static String GOOD_DOCUMENTS_PAYLOAD_WITH_NO_PREVIOUS_FILE_NAME = "good_documents_payload_with_no_previous.json";
private final static String GOOD_DELTA_FILE_NAME = "good_delta_node.json";
private final static String BAD_UPDATE_DOCUMENT_WITHOUT_STATE_NODE_FILE_NAME = "bad_update_document_without_state_node.json";
private final static String BAD_UPDATE_DOCUMENT_WITH_DELTA_FILE_NAME = "bad_update_document_with_delta_node.json";

@Mock
AuthorizationHandlerWrapper mockAuthorizationHandlerWrapper;
Expand Down Expand Up @@ -735,6 +739,26 @@ void GIVEN_bad_update_document_with_no_state_node_WHEN_handle_request_THEN_throw
assertInvalidArgumentsErrorFromPayloadUpdate(null, badUpdateRequest, expectedErrorString, expectedErrorCode, context);
}

@Test
void GIVEN_update_document_with_delta_WHEN_handle_request_THEN_delta_is_dropped_from_data() throws IOException, URISyntaxException {
byte[] badUpdateRequest =
getJsonFromResource(RESOURCE_DIRECTORY_NAME + BAD_UPDATE_DOCUMENT_WITH_DELTA_FILE_NAME);
UpdateThingShadowRequestHandler updateThingShadowIPCHandler = new UpdateThingShadowRequestHandler(mockDao, mockAuthorizationHandlerWrapper, mockPubSubClientWrapper, mockSynchronizeHelper, mockSyncHandler);
ArgumentCaptor<JsonNode> documentCaptor = ArgumentCaptor.forClass(JsonNode.class);
doNothing().when(mockSyncHandler).pushCloudUpdateSyncRequest(any(), any(), documentCaptor.capture());
when(mockDao.updateShadowThing(any(), any(), any(), anyLong()))
.thenReturn(Optional.of(new byte[]{}));

UpdateThingShadowHandlerResponse actualResponse =
updateThingShadowIPCHandler.handleRequest(new UpdateThingShadowRequest()
.withPayload(badUpdateRequest).withThingName("thing"), TEST_SERVICE);
Optional<JsonNode> responseJson = JsonUtil.getPayloadJson(actualResponse.getUpdateThingShadowResponse().getPayload());
assertTrue(responseJson.isPresent());
// Validate that the state doesn't contain the delta
assertThat(documentCaptor.getValue().toString(), not(containsString("delta")));
assertFalse(responseJson.get().get(SHADOW_DOCUMENT_STATE).has(SHADOW_DOCUMENT_STATE_DELTA));
}

@Test
void GIVEN_bad_update_with_non_int_version_WHEN_handle_request_THEN_throw_invalid_argument_error_and_send_message_on_rejected_topic(ExtensionContext context)
throws IOException, URISyntaxException {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"version": 1,
"clientToken": "device",
"state": {
"desired": {
"a": 1
},
"delta": {
"something": {}
}
}
}

0 comments on commit 2e8d744

Please sign in to comment.