From 30c2b72ce823a23418d34d753f57dd6d32e8b3ed Mon Sep 17 00:00:00 2001 From: Jerome Van Der Linden Date: Thu, 15 Jun 2023 16:29:16 +0200 Subject: [PATCH 1/4] fix #1200 --- .../NullIdempotencyKeyException.java | 12 +++++ .../internal/IdempotencyHandler.java | 6 ++- .../persistence/BasePersistenceStore.java | 44 +++++++++++++------ .../persistence/BasePersistenceStoreTest.java | 6 +-- 4 files changed, 49 insertions(+), 19 deletions(-) create mode 100644 powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/exceptions/NullIdempotencyKeyException.java diff --git a/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/exceptions/NullIdempotencyKeyException.java b/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/exceptions/NullIdempotencyKeyException.java new file mode 100644 index 000000000..105af3898 --- /dev/null +++ b/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/exceptions/NullIdempotencyKeyException.java @@ -0,0 +1,12 @@ +package software.amazon.lambda.powertools.idempotency.exceptions; + +/** + * Exception that occurs when the Idepmpotency Key is null or could not be found with the provided JMESPath + */ +public class NullIdempotencyKeyException extends Exception { + private static final long serialVersionUID = 5115004524004542891L; + + public NullIdempotencyKeyException(String message) { + super(message); + } +} 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..8d58f6e62 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 @@ -21,10 +21,7 @@ import org.slf4j.LoggerFactory; import software.amazon.awssdk.utils.StringUtils; import software.amazon.lambda.powertools.idempotency.IdempotencyConfig; -import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyItemAlreadyExistsException; -import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyItemNotFoundException; -import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyKeyException; -import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyValidationException; +import software.amazon.lambda.powertools.idempotency.exceptions.*; import software.amazon.lambda.powertools.idempotency.internal.cache.LRUCache; import software.amazon.lambda.powertools.utilities.JsonConfig; @@ -34,12 +31,8 @@ import java.security.NoSuchAlgorithmException; import java.time.Instant; import java.time.temporal.ChronoUnit; -import java.util.Map; -import java.util.OptionalLong; -import java.util.OptionalInt; +import java.util.*; import java.util.stream.Stream; -import java.util.Spliterators; -import java.util.Spliterator; import java.util.stream.StreamSupport; import static software.amazon.lambda.powertools.core.internal.LambdaConstants.LAMBDA_FUNCTION_NAME_ENV; @@ -130,6 +123,9 @@ public void saveSuccess(JsonNode data, Object result, Instant now) { } catch (JsonProcessingException e) { // TODO : throw ? throw new RuntimeException("Error while serializing the response", e); + } catch (NullIdempotencyKeyException e) { + // missing idempotency key => non-idempotent transaction, we do not store the data, simply return + return; } } @@ -140,7 +136,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); + String idempotencyKey; + try { + idempotencyKey = getHashedIdempotencyKey(data); + } catch (NullIdempotencyKeyException e) { + // missing idempotency key => non-idempotent transaction, we do not store the data, simply return + return; + } if (retrieveFromCache(idempotencyKey, now) != null) { throw new IdempotencyItemAlreadyExistsException(); @@ -170,7 +172,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); + String idemPotencyKey; + try { + idemPotencyKey = getHashedIdempotencyKey(data); + } catch (NullIdempotencyKeyException e) { + // missing idempotency key => non-idempotent transaction, we do not delete the data, simply return + return; + } LOG.debug("Function raised an exception {}. " + "Clearing in progress record in persistence store for idempotency key: {}", @@ -190,7 +198,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); + String idemPotencyKey; + try { + idemPotencyKey = getHashedIdempotencyKey(data); + } catch (NullIdempotencyKeyException e) { + // missing idempotency key => non-idempotent transaction, we do not get the data, simply return nothing + return null; + } DataRecord cachedRecord = retrieveFromCache(idemPotencyKey, now); if (cachedRecord != null) { @@ -211,7 +225,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 String getHashedIdempotencyKey(JsonNode data) throws NullIdempotencyKeyException { JsonNode node = data; if (eventKeyJMESPath != null) { @@ -221,8 +235,10 @@ 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); + throw new NullIdempotencyKeyException("No data found to create a hashed idempotency key"); } - LOG.warn("No data found to create a hashed idempotency key. JMESPath: {}", eventKeyJMESPath); } String hash = generateHash(node); 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 From 18ec42b451ee77bb85f1699ae057b8075b4d4184 Mon Sep 17 00:00:00 2001 From: Jerome Van Der Linden Date: Thu, 15 Jun 2023 17:23:54 +0200 Subject: [PATCH 2/4] fix #1200: update doc --- docs/utilities/idempotency.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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" From b4bdc91cbe343c3aa9070d2b1475b5738b9978e9 Mon Sep 17 00:00:00 2001 From: Jerome Van Der Linden Date: Fri, 16 Jun 2023 08:42:13 +0200 Subject: [PATCH 3/4] replace exception with Optional --- .../NullIdempotencyKeyException.java | 12 ------ .../persistence/BasePersistenceStore.java | 42 ++++++++++--------- 2 files changed, 22 insertions(+), 32 deletions(-) delete mode 100644 powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/exceptions/NullIdempotencyKeyException.java diff --git a/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/exceptions/NullIdempotencyKeyException.java b/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/exceptions/NullIdempotencyKeyException.java deleted file mode 100644 index 105af3898..000000000 --- a/powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/exceptions/NullIdempotencyKeyException.java +++ /dev/null @@ -1,12 +0,0 @@ -package software.amazon.lambda.powertools.idempotency.exceptions; - -/** - * Exception that occurs when the Idepmpotency Key is null or could not be found with the provided JMESPath - */ -public class NullIdempotencyKeyException extends Exception { - private static final long serialVersionUID = 5115004524004542891L; - - public NullIdempotencyKeyException(String message) { - super(message); - } -} 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 8d58f6e62..10c53ce4f 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 @@ -21,7 +21,10 @@ import org.slf4j.LoggerFactory; import software.amazon.awssdk.utils.StringUtils; import software.amazon.lambda.powertools.idempotency.IdempotencyConfig; -import software.amazon.lambda.powertools.idempotency.exceptions.*; +import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyItemAlreadyExistsException; +import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyItemNotFoundException; +import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyKeyException; +import software.amazon.lambda.powertools.idempotency.exceptions.IdempotencyValidationException; import software.amazon.lambda.powertools.idempotency.internal.cache.LRUCache; import software.amazon.lambda.powertools.utilities.JsonConfig; @@ -110,8 +113,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, @@ -123,9 +131,6 @@ public void saveSuccess(JsonNode data, Object result, Instant now) { } catch (JsonProcessingException e) { // TODO : throw ? throw new RuntimeException("Error while serializing the response", e); - } catch (NullIdempotencyKeyException e) { - // missing idempotency key => non-idempotent transaction, we do not store the data, simply return - return; } } @@ -136,14 +141,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; - try { - idempotencyKey = getHashedIdempotencyKey(data); - } catch (NullIdempotencyKeyException e) { + 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(); } @@ -172,14 +176,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; - try { - idemPotencyKey = getHashedIdempotencyKey(data); - } catch (NullIdempotencyKeyException e) { + 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(), @@ -198,14 +201,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; - try { - idemPotencyKey = getHashedIdempotencyKey(data); - } catch (NullIdempotencyKeyException e) { + 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); @@ -225,7 +227,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) throws NullIdempotencyKeyException { + private Optional getHashedIdempotencyKey(JsonNode data) { JsonNode node = data; if (eventKeyJMESPath != null) { @@ -237,13 +239,13 @@ private String getHashedIdempotencyKey(JsonNode data) throws NullIdempotencyKeyE 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); - throw new NullIdempotencyKeyException("No data found to create a hashed idempotency key"); + return Optional.empty(); } } String hash = generateHash(node); hash = functionName + "#" + hash; - return hash; + return Optional.of(hash); } private boolean isMissingIdemPotencyKey(JsonNode data) { From 5b97faca4c23eef05cb0a2b8b2696fda0a62e6a8 Mon Sep 17 00:00:00 2001 From: Jerome Van Der Linden Date: Fri, 16 Jun 2023 08:48:04 +0200 Subject: [PATCH 4/4] fix imports --- .../idempotency/persistence/BasePersistenceStore.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 10c53ce4f..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 @@ -34,7 +34,12 @@ import java.security.NoSuchAlgorithmException; import java.time.Instant; import java.time.temporal.ChronoUnit; -import java.util.*; +import java.util.Map; +import java.util.Optional; +import java.util.OptionalInt; +import java.util.OptionalLong; +import java.util.Spliterator; +import java.util.Spliterators; import java.util.stream.Stream; import java.util.stream.StreamSupport;