PR aws-powertools#717: changes post review

Author: jeromevdl

powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/IdempotencyConfig.java

@@ -13,6 +13,8 @@
  */
 package software.amazon.lambda.powertools.idempotency;
 
+import software.amazon.lambda.powertools.utilities.cache.LRUCache;
+
 /**
  * Configuration of the idempotency feature. Use the {@link Builder} to create an instance.
  */
@@ -76,7 +78,7 @@ public static Builder builder() {
     public static class Builder {
 
         private int localCacheMaxItems = 256;
-        private boolean useLocalCache = false;
+        private boolean useLocalCache = true;
         private int expirationInSeconds = 60 * 60; // 1 hour
         private String eventKeyJMESPath;
         private String payloadValidationJMESPath;
@@ -140,27 +142,17 @@ public Builder withLocalCacheMaxItems(int localCacheMaxItems) {
         }
 
         /**
-         * Whether to locally cache idempotency results, by default false
+         * Whether to locally cache idempotency results, by default true
          *
          * @param useLocalCache boolean that indicate if a local cache must be used in addition to the persistence store.
-         *                      If set to true, will use the {@link software.amazon.lambda.powertools.idempotency.persistence.cache.LRUCache}
+         *                      If set to true, will use the {@link LRUCache}
          * @return the instance of the builder (to chain operations)
          */
         public Builder withUseLocalCache(boolean useLocalCache) {
             this.useLocalCache = useLocalCache;
             return this;
         }
 
-        /**
-         * Locally cache idempotency results.
-         * Same as {@link #withUseLocalCache(boolean)} forced as true
-         *
-         * @return the instance of the builder (to chain operations)
-         */
-        public Builder withUseLocalCache() {
-            return withUseLocalCache(true);
-        }
-
         /**
          * The number of seconds to wait before a record is expired
          *

powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyHandler.java

@@ -84,7 +84,7 @@ private Object processIdempotency() throws Throwable {
         } catch (IdempotencyKeyException ike) {
             throw ike;
         } catch (Exception e) {
-            throw new IdempotencyPersistenceLayerException("Failed to save in progress record to idempotency store", e);
+            throw new IdempotencyPersistenceLayerException("Failed to save in progress record to idempotency store. If you believe this is a powertools bug, please open an issue.", e);
         }
         return getFunctionResponse();
     }
@@ -101,10 +101,10 @@ private DataRecord getIdempotencyRecord() {
             // This code path will only be triggered if the record is removed between saveInProgress and getRecord
             LOG.debug("An existing idempotency record was deleted before we could fetch it");
             throw new IdempotencyInconsistentStateException("saveInProgress and getRecord return inconsistent results", e);
-        } catch (IdempotencyValidationException ve) {
-            throw ve;
+        } catch (IdempotencyValidationException | IdempotencyKeyException vke) {
+            throw vke;
         } catch (Exception e) {
-            throw new IdempotencyPersistenceLayerException("Failed to get record from idempotency store", e);
+            throw new IdempotencyPersistenceLayerException("Failed to get record from idempotency store. If you believe this is a powertools bug, please open an issue.", e);
         }
     }
 
@@ -142,16 +142,18 @@ private Object getFunctionResponse() throws Throwable {
             // also raises an exception
             try {
                 persistenceStore.deleteRecord(data, handlerException);
+            } catch (IdempotencyKeyException ke) {
+                throw ke;
             } catch (Exception e) {
-                throw new IdempotencyPersistenceLayerException("Failed to delete record from idempotency store", e);
+                throw new IdempotencyPersistenceLayerException("Failed to delete record from idempotency store. If you believe this is a powertools bug, please open an issue.", e);
             }
             throw handlerException;
         }
 
         try {
             persistenceStore.saveSuccess(data, response, Instant.now());
         } catch (Exception e) {
-            throw new IdempotencyPersistenceLayerException("Failed to update record state to success in idempotency store", e);
+            throw new IdempotencyPersistenceLayerException("Failed to update record state to success in idempotency store. If you believe this is a powertools bug, please open an issue.", e);
         }
         return response;
     }

powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/persistence/BasePersistenceStore.java

@@ -26,8 +26,8 @@
 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.persistence.cache.LRUCache;
 import software.amazon.lambda.powertools.utilities.JsonConfig;
+import software.amazon.lambda.powertools.utilities.cache.LRUCache;
 
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
@@ -70,7 +70,7 @@ public abstract class BasePersistenceStore implements PersistenceStore {
      */
     public void configure(IdempotencyConfig config, String functionName) {
         String funcEnv = System.getenv(Constants.LAMBDA_FUNCTION_NAME_ENV);
-        this.functionName = funcEnv != null ? funcEnv : "test-func";
+        this.functionName = funcEnv != null ? funcEnv : "testFunction";
         if (!StringUtils.isEmpty(functionName)) {
             this.functionName += "." + functionName;
         }

powertools-idempotency/src/main/java/software/amazon/lambda/powertools/idempotency/persistence/DynamoDBPersistenceStore.java

@@ -131,7 +131,7 @@ public void putRecord(DataRecord record, Instant now) throws IdempotencyItemAlre
             );
         } catch (ConditionalCheckFailedException e) {
             LOG.debug("Failed to put record for already existing idempotency key: {}", record.getIdempotencyKey());
-            throw new IdempotencyItemAlreadyExistsException("Failed to put record for already existing idempotency key: {}", e);
+            throw new IdempotencyItemAlreadyExistsException("Failed to put record for already existing idempotency key: " + record.getIdempotencyKey(), e);
         }
     }
 

powertools-idempotency/src/test/java/software/amazon/lambda/powertools/idempotency/internal/IdempotencyAspectTest.java

@@ -205,8 +205,7 @@ public void idempotencyOnSubMethodAnnotated_firstCall_shouldPutInStore() {
 
         ArgumentCaptor<Basket> resultCaptor = ArgumentCaptor.forClass(Basket.class);
         verify(store).saveSuccess(any(), resultCaptor.capture(), any());
-        assertThat(resultCaptor.getValue().getProducts()).contains(basket.getProducts().get(0));
-        assertThat(resultCaptor.getValue().getProducts()).contains(new Product(0, "fake", 0));
+        assertThat(resultCaptor.getValue().getProducts()).contains(basket.getProducts().get(0), new Product(0, "fake", 0));
     }
 
     @Test