Skip validation on delete requests #318 (#330)

* Skip validation on delete requests #318

* Replace Set of to Immutable Set

Co-authored-by: Anshika Garg <anshikg@amazon.com>

Author: John Tompkins

src/main/java/software/amazon/cloudformation/AbstractWrapper.java

@@ -19,16 +19,17 @@
 import com.fasterxml.jackson.databind.exc.MismatchedInputException;
 import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
 import java.time.Instant;
-import java.util.Arrays;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
@@ -73,7 +74,8 @@ public abstract class AbstractWrapper<ResourceT, CallbackT> {
 
     public static final SdkHttpClient HTTP_CLIENT = ApacheHttpClient.builder().build();
 
-    private static final List<Action> MUTATING_ACTIONS = Arrays.asList(Action.CREATE, Action.DELETE, Action.UPDATE);
+    private static final Set<Action> MUTATING_ACTIONS = ImmutableSet.of(Action.CREATE, Action.DELETE, Action.UPDATE);
+    private static final Set<Action> VALIDATING_ACTIONS = ImmutableSet.of(Action.CREATE, Action.UPDATE);
 
     protected final Serializer serializer;
     protected LoggerProxy loggerProxy;
@@ -245,7 +247,8 @@ public void processRequest(final InputStream inputStream, final OutputStream out
             throw new TerminalException("Invalid request object received");
         }
 
-        if (MUTATING_ACTIONS.contains(request.getAction())) {
+        final boolean isMutatingAction = MUTATING_ACTIONS.contains(request.getAction());
+        if (isMutatingAction) {
             if (request.getRequestData().getResourceProperties() == null) {
                 throw new TerminalException("Invalid resource properties object received");
             }
@@ -271,13 +274,16 @@ public void processRequest(final InputStream inputStream, final OutputStream out
 
         this.metricsPublisherProxy.publishInvocationMetric(Instant.now(), request.getAction());
 
-        // for CUD actions, validate incoming model - any error is a terminal failure on
+        // for create and update actions, validate incoming model - any error is a
+        // terminal failure on
         // the invocation
         // NOTE: we validate the raw pre-deserialized payload to account for lenient
         // serialization.
+        // NOTE: Validation is not required on deletion as only the primary identifier
+        // is required to delete.
         // Here, we want to surface ALL input validation errors to the caller.
-        boolean isMutatingAction = MUTATING_ACTIONS.contains(request.getAction());
-        if (isMutatingAction) {
+        final boolean shouldValidate = VALIDATING_ACTIONS.contains(request.getAction());
+        if (shouldValidate) {
             // validate entire incoming payload, including extraneous fields which
             // are stripped by the Serializer (due to FAIL_ON_UNKNOWN_PROPERTIES setting)
             JSONObject rawModelObject = rawRequest.getJSONObject("requestData").getJSONObject("resourceProperties");

src/test/java/software/amazon/cloudformation/ExecutableWrapperTest.java

@@ -132,8 +132,8 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ
             // verify initialiseRuntime was called and initialised dependencies
             verifyInitialiseRuntime();
 
-            // verify that model validation occurred for CREATE/UPDATE/DELETE
-            if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
+            // verify that model validation occurred for CREATE/UPDATE
+            if (action == Action.CREATE || action == Action.UPDATE) {
                 verify(validator, times(1)).validateObject(any(JSONObject.class), any(JSONObject.class));
             }
 

src/test/java/software/amazon/cloudformation/LambdaWrapperTest.java

@@ -145,8 +145,8 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ
             // verify initialiseRuntime was called and initialised dependencies
             verifyInitialiseRuntime();
 
-            // verify that model validation occurred for CREATE/UPDATE/DELETE
-            if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
+            // verify that model validation occurred for CREATE/UPDATE
+            if (action == Action.CREATE || action == Action.UPDATE) {
                 verify(validator, times(1)).validateObject(any(JSONObject.class), any(JSONObject.class));
             }
 

src/test/java/software/amazon/cloudformation/WrapperTest.java

@@ -154,8 +154,8 @@ public void invokeHandler_nullResponse_returnsFailure(final String requestDataPa
             verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(action));
             verify(providerMetricsPublisher).publishDurationMetric(any(Instant.class), eq(action), anyLong());
 
-            // verify that model validation occurred for CREATE/UPDATE/DELETE
-            if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
+            // verify that model validation occurred for CREATE/UPDATE
+            if (action == Action.CREATE || action == Action.UPDATE) {
                 verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
             }
 
@@ -269,8 +269,8 @@ public void invokeHandler_handlerFailed_returnsFailure(final String requestDataP
             // verify initialiseRuntime was called and initialised dependencies
             verifyInitialiseRuntime();
 
-            // verify that model validation occurred for CREATE/UPDATE/DELETE
-            if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
+            // verify that model validation occurred for CREATE/UPDATE
+            if (action == Action.CREATE || action == Action.UPDATE) {
                 verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
             }
 
@@ -325,8 +325,8 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ
             // verify initialiseRuntime was called and initialised dependencies
             verifyInitialiseRuntime();
 
-            // verify that model validation occurred for CREATE/UPDATE/DELETE
-            if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
+            // verify that model validation occurred for CREATE/UPDATE
+            if (action == Action.CREATE || action == Action.UPDATE) {
                 verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
             }
 
@@ -400,11 +400,12 @@ public void invokeHandler_InProgress_returnsInProgress(final String requestDataP
             verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(action));
             verify(providerMetricsPublisher).publishDurationMetric(any(Instant.class), eq(action), anyLong());
 
-            // verify that model validation occurred for CREATE/UPDATE/DELETE
-            if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
+            // verify that model validation occurred for CREATE/UPDATE
+            if (action == Action.CREATE || action == Action.UPDATE) {
                 verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
+            }
 
-                // verify output response
+            if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
                 verifyHandlerResponse(out, ProgressEvent.<TestModel, TestContext>builder().status(OperationStatus.IN_PROGRESS)
                     .resourceModel(TestModel.builder().property1("abc").property2(123).build()).build());
             } else {
@@ -465,8 +466,8 @@ public void reInvokeHandler_InProgress_returnsInProgress(final String requestDat
             // validation failure metric should not be published
             verifyNoMoreInteractions(providerMetricsPublisher);
 
-            // verify that model validation occurred for CREATE/UPDATE/DELETE
-            if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
+            // verify that model validation occurred for CREATE/UPDATE
+            if (action == Action.CREATE || action == Action.UPDATE) {
                 verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
             }
 
@@ -477,7 +478,7 @@ public void reInvokeHandler_InProgress_returnsInProgress(final String requestDat
     }
 
     @ParameterizedTest
-    @CsvSource({ "create.request.json,CREATE", "update.request.json,UPDATE", "delete.request.json,DELETE" })
+    @CsvSource({ "create.request.json,CREATE", "update.request.json,UPDATE" })
     public void invokeHandler_SchemaValidationFailure(final String requestDataPath, final String actionAsString)
         throws IOException {
         final Action action = Action.valueOf(actionAsString);
@@ -499,8 +500,8 @@ public void invokeHandler_SchemaValidationFailure(final String requestDataPath,
             // all metrics should be published, even for a single invocation
             verify(providerMetricsPublisher, times(1)).publishInvocationMetric(any(Instant.class), eq(action));
 
-            // verify that model validation occurred for CREATE/UPDATE/DELETE
-            if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
+            // verify that model validation occurred for CREATE/UPDATE
+            if (action == Action.CREATE || action == Action.UPDATE) {
                 verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
             }