fix: missing idempotency key should not persist any data (#1201)

Author: jeromevdl

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"

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 {

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<String> 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<String> 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<String> 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<String> 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<String> 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) {

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