Skip to content

Commit 7725100

Browse files
John Tompkinsanshikg
John Tompkins
and
anshikg
authored
Skip validation on delete requests #318 (#330)
* Skip validation on delete requests #318 * Replace Set of to Immutable Set Co-authored-by: Anshika Garg <[email protected]>
1 parent 8d02fd1 commit 7725100

File tree

4 files changed

+31
-24
lines changed

4 files changed

+31
-24
lines changed

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,17 @@
1919
import com.fasterxml.jackson.databind.exc.MismatchedInputException;
2020
import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
2121
import com.google.common.annotations.VisibleForTesting;
22+
import com.google.common.collect.ImmutableSet;
2223
import java.io.IOException;
2324
import java.io.InputStream;
2425
import java.io.OutputStream;
2526
import java.nio.charset.StandardCharsets;
2627
import java.time.Instant;
27-
import java.util.Arrays;
2828
import java.util.Date;
2929
import java.util.HashMap;
3030
import java.util.List;
3131
import java.util.Map;
32+
import java.util.Set;
3233
import org.apache.commons.collections.CollectionUtils;
3334
import org.apache.commons.io.FileUtils;
3435
import org.apache.commons.io.IOUtils;
@@ -73,7 +74,8 @@ public abstract class AbstractWrapper<ResourceT, CallbackT> {
7374

7475
public static final SdkHttpClient HTTP_CLIENT = ApacheHttpClient.builder().build();
7576

76-
private static final List<Action> MUTATING_ACTIONS = Arrays.asList(Action.CREATE, Action.DELETE, Action.UPDATE);
77+
private static final Set<Action> MUTATING_ACTIONS = ImmutableSet.of(Action.CREATE, Action.DELETE, Action.UPDATE);
78+
private static final Set<Action> VALIDATING_ACTIONS = ImmutableSet.of(Action.CREATE, Action.UPDATE);
7779

7880
protected final Serializer serializer;
7981
protected LoggerProxy loggerProxy;
@@ -245,7 +247,8 @@ public void processRequest(final InputStream inputStream, final OutputStream out
245247
throw new TerminalException("Invalid request object received");
246248
}
247249

248-
if (MUTATING_ACTIONS.contains(request.getAction())) {
250+
final boolean isMutatingAction = MUTATING_ACTIONS.contains(request.getAction());
251+
if (isMutatingAction) {
249252
if (request.getRequestData().getResourceProperties() == null) {
250253
throw new TerminalException("Invalid resource properties object received");
251254
}
@@ -271,13 +274,16 @@ public void processRequest(final InputStream inputStream, final OutputStream out
271274

272275
this.metricsPublisherProxy.publishInvocationMetric(Instant.now(), request.getAction());
273276

274-
// for CUD actions, validate incoming model - any error is a terminal failure on
277+
// for create and update actions, validate incoming model - any error is a
278+
// terminal failure on
275279
// the invocation
276280
// NOTE: we validate the raw pre-deserialized payload to account for lenient
277281
// serialization.
282+
// NOTE: Validation is not required on deletion as only the primary identifier
283+
// is required to delete.
278284
// Here, we want to surface ALL input validation errors to the caller.
279-
boolean isMutatingAction = MUTATING_ACTIONS.contains(request.getAction());
280-
if (isMutatingAction) {
285+
final boolean shouldValidate = VALIDATING_ACTIONS.contains(request.getAction());
286+
if (shouldValidate) {
281287
// validate entire incoming payload, including extraneous fields which
282288
// are stripped by the Serializer (due to FAIL_ON_UNKNOWN_PROPERTIES setting)
283289
JSONObject rawModelObject = rawRequest.getJSONObject("requestData").getJSONObject("resourceProperties");

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ
132132
// verify initialiseRuntime was called and initialised dependencies
133133
verifyInitialiseRuntime();
134134

135-
// verify that model validation occurred for CREATE/UPDATE/DELETE
136-
if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
135+
// verify that model validation occurred for CREATE/UPDATE
136+
if (action == Action.CREATE || action == Action.UPDATE) {
137137
verify(validator, times(1)).validateObject(any(JSONObject.class), any(JSONObject.class));
138138
}
139139

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ
145145
// verify initialiseRuntime was called and initialised dependencies
146146
verifyInitialiseRuntime();
147147

148-
// verify that model validation occurred for CREATE/UPDATE/DELETE
149-
if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
148+
// verify that model validation occurred for CREATE/UPDATE
149+
if (action == Action.CREATE || action == Action.UPDATE) {
150150
verify(validator, times(1)).validateObject(any(JSONObject.class), any(JSONObject.class));
151151
}
152152

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

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ public void invokeHandler_nullResponse_returnsFailure(final String requestDataPa
154154
verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(action));
155155
verify(providerMetricsPublisher).publishDurationMetric(any(Instant.class), eq(action), anyLong());
156156

157-
// verify that model validation occurred for CREATE/UPDATE/DELETE
158-
if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
157+
// verify that model validation occurred for CREATE/UPDATE
158+
if (action == Action.CREATE || action == Action.UPDATE) {
159159
verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
160160
}
161161

@@ -269,8 +269,8 @@ public void invokeHandler_handlerFailed_returnsFailure(final String requestDataP
269269
// verify initialiseRuntime was called and initialised dependencies
270270
verifyInitialiseRuntime();
271271

272-
// verify that model validation occurred for CREATE/UPDATE/DELETE
273-
if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
272+
// verify that model validation occurred for CREATE/UPDATE
273+
if (action == Action.CREATE || action == Action.UPDATE) {
274274
verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
275275
}
276276

@@ -325,8 +325,8 @@ public void invokeHandler_CompleteSynchronously_returnsSuccess(final String requ
325325
// verify initialiseRuntime was called and initialised dependencies
326326
verifyInitialiseRuntime();
327327

328-
// verify that model validation occurred for CREATE/UPDATE/DELETE
329-
if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
328+
// verify that model validation occurred for CREATE/UPDATE
329+
if (action == Action.CREATE || action == Action.UPDATE) {
330330
verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
331331
}
332332

@@ -400,11 +400,12 @@ public void invokeHandler_InProgress_returnsInProgress(final String requestDataP
400400
verify(providerMetricsPublisher).publishInvocationMetric(any(Instant.class), eq(action));
401401
verify(providerMetricsPublisher).publishDurationMetric(any(Instant.class), eq(action), anyLong());
402402

403-
// verify that model validation occurred for CREATE/UPDATE/DELETE
404-
if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
403+
// verify that model validation occurred for CREATE/UPDATE
404+
if (action == Action.CREATE || action == Action.UPDATE) {
405405
verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
406+
}
406407

407-
// verify output response
408+
if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
408409
verifyHandlerResponse(out, ProgressEvent.<TestModel, TestContext>builder().status(OperationStatus.IN_PROGRESS)
409410
.resourceModel(TestModel.builder().property1("abc").property2(123).build()).build());
410411
} else {
@@ -465,8 +466,8 @@ public void reInvokeHandler_InProgress_returnsInProgress(final String requestDat
465466
// validation failure metric should not be published
466467
verifyNoMoreInteractions(providerMetricsPublisher);
467468

468-
// verify that model validation occurred for CREATE/UPDATE/DELETE
469-
if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
469+
// verify that model validation occurred for CREATE/UPDATE
470+
if (action == Action.CREATE || action == Action.UPDATE) {
470471
verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
471472
}
472473

@@ -477,7 +478,7 @@ public void reInvokeHandler_InProgress_returnsInProgress(final String requestDat
477478
}
478479

479480
@ParameterizedTest
480-
@CsvSource({ "create.request.json,CREATE", "update.request.json,UPDATE", "delete.request.json,DELETE" })
481+
@CsvSource({ "create.request.json,CREATE", "update.request.json,UPDATE" })
481482
public void invokeHandler_SchemaValidationFailure(final String requestDataPath, final String actionAsString)
482483
throws IOException {
483484
final Action action = Action.valueOf(actionAsString);
@@ -499,8 +500,8 @@ public void invokeHandler_SchemaValidationFailure(final String requestDataPath,
499500
// all metrics should be published, even for a single invocation
500501
verify(providerMetricsPublisher, times(1)).publishInvocationMetric(any(Instant.class), eq(action));
501502

502-
// verify that model validation occurred for CREATE/UPDATE/DELETE
503-
if (action == Action.CREATE || action == Action.UPDATE || action == Action.DELETE) {
503+
// verify that model validation occurred for CREATE/UPDATE
504+
if (action == Action.CREATE || action == Action.UPDATE) {
504505
verify(validator).validateObject(any(JSONObject.class), any(JSONObject.class));
505506
}
506507

0 commit comments

Comments
 (0)