diff --git a/docs/utilities/idempotency.md b/docs/utilities/idempotency.md index d8ec13e9d..c8dd59000 100644 --- a/docs/utilities/idempotency.md +++ b/docs/utilities/idempotency.md @@ -708,10 +708,12 @@ By using **`withPayloadValidationJMESPath("amount")`**, we prevent this potentia ### Making idempotency key required -If you want to enforce that an idempotency key is required, you can set **`ThrowOnNoIdempotencyKey`** to `True`. +If you want to enforce that an idempotency key is required, you can set **`ThrowOnNoIdempotencyKey`** to `true`. This means that we will throw **`IdempotencyKeyException`** if the evaluation of **`EventKeyJMESPath`** is `null`. +When set to `false` (the default), if the idempotency key is null, then the data is not persisted in the store. + === "App.java" ```java hl_lines="9-10 13" diff --git a/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyHandler.java b/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyHandler.java index 205ef41d4..5ce723f04 100644 --- a/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyHandler.java +++ b/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyHandler.java @@ -84,7 +84,9 @@ private Object processIdempotency() throws Throwable { persistenceStore.saveInProgress(data, Instant.now(), getRemainingTimeInMillis()); } catch (IdempotencyItemAlreadyExistsException iaee) { DataRecord record = getIdempotencyRecord(); - return handleForStatus(record); + if (record != null) { + return handleForStatus(record); + } } catch (IdempotencyKeyException ike) { throw ike; } catch (Exception e) { @@ -111,7 +113,7 @@ private OptionalInt getRemainingTimeInMillis() { /** * Retrieve the idempotency record from the persistence layer. * - * @return the record if available + * @return the record if available, potentially null */ private DataRecord getIdempotencyRecord() { try { diff --git a/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStore.java b/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStore.java index 727405257..c79068d1a 100644 --- a/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStore.java +++ b/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStore.java @@ -35,11 +35,12 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Map; -import java.util.OptionalLong; +import java.util.Optional; import java.util.OptionalInt; -import java.util.stream.Stream; -import java.util.Spliterators; +import java.util.OptionalLong; import java.util.Spliterator; +import java.util.Spliterators; +import java.util.stream.Stream; import java.util.stream.StreamSupport; import static software.amazon.lambda.powertools.core.internal.LambdaConstants.LAMBDA_FUNCTION_NAME_ENV; @@ -117,8 +118,13 @@ public void saveSuccess(JsonNode data, Object result, Instant now) { } else { responseJson = writer.writeValueAsString(result); } + Optional hashedIdempotencyKey = getHashedIdempotencyKey(data); + if (!hashedIdempotencyKey.isPresent()) { + // missing idempotency key => non-idempotent transaction, we do not store the data, simply return + return; + } DataRecord record = new DataRecord( - getHashedIdempotencyKey(data), + hashedIdempotencyKey.get(), DataRecord.Status.COMPLETED, getExpiryEpochSecond(now), responseJson, @@ -140,8 +146,13 @@ public void saveSuccess(JsonNode data, Object result, Instant now) { * @param now */ public void saveInProgress(JsonNode data, Instant now, OptionalInt remainingTimeInMs) throws IdempotencyItemAlreadyExistsException { - String idempotencyKey = getHashedIdempotencyKey(data); + Optional hashedIdempotencyKey = getHashedIdempotencyKey(data); + if (!hashedIdempotencyKey.isPresent()) { + // missing idempotency key => non-idempotent transaction, we do not store the data, simply return + return; + } + String idempotencyKey = hashedIdempotencyKey.get(); if (retrieveFromCache(idempotencyKey, now) != null) { throw new IdempotencyItemAlreadyExistsException(); } @@ -170,8 +181,13 @@ public void saveInProgress(JsonNode data, Instant now, OptionalInt remainingTime * @param throwable The throwable thrown by the function */ public void deleteRecord(JsonNode data, Throwable throwable) { - String idemPotencyKey = getHashedIdempotencyKey(data); + Optional hashedIdempotencyKey = getHashedIdempotencyKey(data); + if (!hashedIdempotencyKey.isPresent()) { + // missing idempotency key => non-idempotent transaction, we do not delete the data, simply return + return; + } + String idemPotencyKey = hashedIdempotencyKey.get(); LOG.debug("Function raised an exception {}. " + "Clearing in progress record in persistence store for idempotency key: {}", throwable.getClass(), @@ -190,8 +206,13 @@ public void deleteRecord(JsonNode data, Throwable throwable) { * @throws IdempotencyItemNotFoundException Exception thrown if no record exists in persistence store with the idempotency key */ public DataRecord getRecord(JsonNode data, Instant now) throws IdempotencyValidationException, IdempotencyItemNotFoundException { - String idemPotencyKey = getHashedIdempotencyKey(data); + Optional hashedIdempotencyKey = getHashedIdempotencyKey(data); + if (!hashedIdempotencyKey.isPresent()) { + // missing idempotency key => non-idempotent transaction, we do not get the data, simply return nothing + return null; + } + String idemPotencyKey = hashedIdempotencyKey.get(); DataRecord cachedRecord = retrieveFromCache(idemPotencyKey, now); if (cachedRecord != null) { LOG.debug("Idempotency record found in cache with idempotency key: {}", idemPotencyKey); @@ -211,7 +232,7 @@ public DataRecord getRecord(JsonNode data, Instant now) throws IdempotencyValida * @param data incoming data * @return Hashed representation of the data extracted by the jmespath expression */ - private String getHashedIdempotencyKey(JsonNode data) { + private Optional getHashedIdempotencyKey(JsonNode data) { JsonNode node = data; if (eventKeyJMESPath != null) { @@ -221,13 +242,15 @@ private String getHashedIdempotencyKey(JsonNode data) { if (isMissingIdemPotencyKey(node)) { if (throwOnNoIdempotencyKey) { throw new IdempotencyKeyException("No data found to create a hashed idempotency key"); + } else { + LOG.warn("No data found to create a hashed idempotency key. JMESPath: {}", eventKeyJMESPath); + return Optional.empty(); } - LOG.warn("No data found to create a hashed idempotency key. JMESPath: {}", eventKeyJMESPath); } String hash = generateHash(node); hash = functionName + "#" + hash; - return hash; + return Optional.of(hash); } private boolean isMissingIdemPotencyKey(JsonNode data) { diff --git a/powertools-idempotency/src/test/java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStoreTest.java b/powertools-idempotency/src/test/java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStoreTest.java index e77a995a4..47e5df1ab 100644 --- a/powertools-idempotency/src/test/java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStoreTest.java +++ b/powertools-idempotency/src/test/java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStoreTest.java @@ -143,15 +143,15 @@ public void saveInProgress_jmespath_NotFound_shouldThrowException() { } @Test - public void saveInProgress_jmespath_NotFound_shouldNotThrowException() { + public void saveInProgress_jmespath_NotFound_shouldNotPersist() { APIGatewayProxyRequestEvent event = EventLoader.loadApiGatewayRestEvent("apigw_event.json"); persistenceStore.configure(IdempotencyConfig.builder() .withEventKeyJMESPath("unavailable") .build(), ""); Instant now = Instant.now(); persistenceStore.saveInProgress(JsonConfig.get().getObjectMapper().valueToTree(event), now, OptionalInt.empty()); - assertThat(dr.getStatus()).isEqualTo(DataRecord.Status.INPROGRESS); - assertThat(status).isEqualTo(1); + assertThat(dr).isNull(); + assertThat(status).isEqualTo(-1); } @Test