fix(idempotency): treat missing idempotency key as non-idempotent transaction (no-op) when raise_on_no_idempotency_key is False (#2477)

* bug: fix missing idempotency key

* fix return

* chore: add e2e test for optional idempotency key

* chore: fix e2e third payload

* docs: clarify operations

Signed-off-by: Heitor Lessa <[email protected]>

* chore: clarify warning

Signed-off-by: Heitor Lessa <[email protected]>

* chore: strip extra space

Signed-off-by: Heitor Lessa <[email protected]>

* chore: ignore long warning line

---------

Signed-off-by: Heitor Lessa <[email protected]>
Co-authored-by: heitorlessa <[email protected]>
Co-authored-by: Heitor Lessa <[email protected]>

Author: leandrodamascena

Diff for: aws_lambda_powertools/utilities/idempotency/base.py

@@ -111,7 +111,8 @@ def _process_idempotency(self):
         except IdempotencyItemAlreadyExistsError:
             # Now we know the item already exists, we can retrieve it
             record = self._get_idempotency_record()
-            return self._handle_for_status(record)
+            if record is not None:
+                return self._handle_for_status(record)
         except Exception as exc:
             raise IdempotencyPersistenceLayerError(
                 "Failed to save in progress record to idempotency store", exc
@@ -138,7 +139,7 @@ def _get_remaining_time_in_millis(self) -> Optional[int]:
 
         return None
 
-    def _get_idempotency_record(self) -> DataRecord:
+    def _get_idempotency_record(self) -> Optional[DataRecord]:
         """
         Retrieve the idempotency record from the persistence layer.
 

Diff for: aws_lambda_powertools/utilities/idempotency/persistence/base.py

@@ -161,7 +161,7 @@ def configure(self, config: IdempotencyConfig, function_name: Optional[str] = No
             self._cache = LRUDict(max_items=config.local_cache_max_items)
         self.hash_function = getattr(hashlib, config.hash_function)
 
-    def _get_hashed_idempotency_key(self, data: Dict[str, Any]) -> str:
+    def _get_hashed_idempotency_key(self, data: Dict[str, Any]) -> Optional[str]:
         """
         Extract idempotency key and return a hashed representation
 
@@ -182,7 +182,12 @@ def _get_hashed_idempotency_key(self, data: Dict[str, Any]) -> str:
         if self.is_missing_idempotency_key(data=data):
             if self.raise_on_no_idempotency_key:
                 raise IdempotencyKeyError("No data found to create a hashed idempotency_key")
-            warnings.warn(f"No value found for idempotency_key. jmespath: {self.event_key_jmespath}", stacklevel=2)
+
+            warnings.warn(
+                f"No idempotency key value found. Skipping persistence layer and validation operations. jmespath: {self.event_key_jmespath}",  # noqa: E501
+                stacklevel=2,
+            )
+            return None
 
         generated_hash = self._generate_hash(data=data)
         return f"{self.function_name}#{generated_hash}"
@@ -315,10 +320,16 @@ def save_success(self, data: Dict[str, Any], result: dict) -> None:
         result: dict
             The response from function
         """
+        idempotency_key = self._get_hashed_idempotency_key(data=data)
+        if idempotency_key is None:
+            # If the idempotency key is None, no data will be saved in the Persistence Layer.
+            # See: https://github.com/awslabs/aws-lambda-powertools-python/issues/2465
+            return None
+
         response_data = json.dumps(result, cls=Encoder, sort_keys=True)
 
         data_record = DataRecord(
-            idempotency_key=self._get_hashed_idempotency_key(data=data),
+            idempotency_key=idempotency_key,
             status=STATUS_CONSTANTS["COMPLETED"],
             expiry_timestamp=self._get_expiry_timestamp(),
             response_data=response_data,
@@ -343,8 +354,15 @@ def save_inprogress(self, data: Dict[str, Any], remaining_time_in_millis: Option
         remaining_time_in_millis: Optional[int]
             If expiry of in-progress invocations is enabled, this will contain the remaining time available in millis
         """
+
+        idempotency_key = self._get_hashed_idempotency_key(data=data)
+        if idempotency_key is None:
+            # If the idempotency key is None, no data will be saved in the Persistence Layer.
+            # See: https://github.com/awslabs/aws-lambda-powertools-python/issues/2465
+            return None
+
         data_record = DataRecord(
-            idempotency_key=self._get_hashed_idempotency_key(data=data),
+            idempotency_key=idempotency_key,
             status=STATUS_CONSTANTS["INPROGRESS"],
             expiry_timestamp=self._get_expiry_timestamp(),
             payload_hash=self._get_hashed_payload(data=data),
@@ -381,7 +399,14 @@ def delete_record(self, data: Dict[str, Any], exception: Exception):
         exception
             The exception raised by the function
         """
-        data_record = DataRecord(idempotency_key=self._get_hashed_idempotency_key(data=data))
+
+        idempotency_key = self._get_hashed_idempotency_key(data=data)
+        if idempotency_key is None:
+            # If the idempotency key is None, no data will be saved in the Persistence Layer.
+            # See: https://github.com/awslabs/aws-lambda-powertools-python/issues/2465
+            return None
+
+        data_record = DataRecord(idempotency_key=idempotency_key)
 
         logger.debug(
             f"Function raised an exception ({type(exception).__name__}). Clearing in progress record in persistence "
@@ -391,7 +416,7 @@ def delete_record(self, data: Dict[str, Any], exception: Exception):
 
         self._delete_from_cache(idempotency_key=data_record.idempotency_key)
 
-    def get_record(self, data: Dict[str, Any]) -> DataRecord:
+    def get_record(self, data: Dict[str, Any]) -> Optional[DataRecord]:
         """
         Retrieve idempotency key for data provided, fetch from persistence store, and convert to DataRecord.
 
@@ -414,6 +439,10 @@ def get_record(self, data: Dict[str, Any]) -> DataRecord:
         """
 
         idempotency_key = self._get_hashed_idempotency_key(data=data)
+        if idempotency_key is None:
+            # If the idempotency key is None, no data will be saved in the Persistence Layer.
+            # See: https://github.com/awslabs/aws-lambda-powertools-python/issues/2465
+            return None
 
         cached_record = self._retrieve_from_cache(idempotency_key=idempotency_key)
         if cached_record:

Diff for: docs/utilities/idempotency.md

@@ -3,6 +3,8 @@ title: Idempotency
 description: Utility
 ---
 
+<!-- markdownlint-disable MD051 -->
+
 The idempotency utility provides a simple solution to convert your Lambda functions into idempotent operations which
 are safe to retry.
 
@@ -853,6 +855,9 @@ If you want to enforce that an idempotency key is required, you can set **`raise
 
 This means that we will raise **`IdempotencyKeyError`** if the evaluation of **`event_key_jmespath`** is `None`.
 
+???+ warning
+    To prevent errors, transactions will not be treated as idempotent if **`raise_on_no_idempotency_key`** is set to `False` and the evaluation of **`event_key_jmespath`** is `None`. Therefore, no data will be fetched, stored, or deleted in the idempotency storage layer.
+
 === "app.py"
 
     ```python hl_lines="9-10 13"

Diff for: tests/e2e/idempotency/handlers/optional_idempotency_key_handler.py

@@ -0,0 +1,17 @@
+import os
+import uuid
+
+from aws_lambda_powertools.utilities.idempotency import (
+    DynamoDBPersistenceLayer,
+    IdempotencyConfig,
+    idempotent,
+)
+
+TABLE_NAME = os.getenv("IdempotencyTable", "")
+persistence_layer = DynamoDBPersistenceLayer(table_name=TABLE_NAME)
+config = IdempotencyConfig(event_key_jmespath='headers."X-Idempotency-Key"', use_local_cache=True)
+
+
+@idempotent(config=config, persistence_store=persistence_layer)
+def lambda_handler(event, context):
+    return {"request": str(uuid.uuid4())}

Diff for: tests/e2e/idempotency/infrastructure.py

@@ -16,6 +16,7 @@ def create_resources(self):
         table.grant_read_write_data(functions["TtlCacheTimeoutHandler"])
         table.grant_read_write_data(functions["ParallelExecutionHandler"])
         table.grant_read_write_data(functions["FunctionThreadSafetyHandler"])
+        table.grant_read_write_data(functions["OptionalIdempotencyKeyHandler"])
 
     def _create_dynamodb_table(self) -> Table:
         table = dynamodb.Table(

Diff for: tests/e2e/idempotency/test_idempotency_dynamodb.py

@@ -27,6 +27,11 @@ def function_thread_safety_handler_fn_arn(infrastructure: dict) -> str:
     return infrastructure.get("FunctionThreadSafetyHandlerArn", "")
 
 
+@pytest.fixture
+def optional_idempotency_key_fn_arn(infrastructure: dict) -> str:
+    return infrastructure.get("OptionalIdempotencyKeyHandlerArn", "")
+
+
 @pytest.fixture
 def idempotency_table_name(infrastructure: dict) -> str:
     return infrastructure.get("DynamoDBTable", "")
@@ -132,3 +137,33 @@ def test_idempotent_function_thread_safety(function_thread_safety_handler_fn_arn
 
     # we use set() here because we want to compare the elements regardless of their order in the array
     assert set(first_execution_response) == set(second_execution_response)
+
+
+@pytest.mark.xdist_group(name="idempotency")
+def test_optional_idempotency_key(optional_idempotency_key_fn_arn: str):
+    # GIVEN two payloads where only one has the expected idempotency key
+    payload = json.dumps({"headers": {"X-Idempotency-Key": "here"}})
+    payload_without = json.dumps({"headers": {}})
+
+    # WHEN
+    # we make one request with an idempotency key
+    first_execution, _ = data_fetcher.get_lambda_response(lambda_arn=optional_idempotency_key_fn_arn, payload=payload)
+    first_execution_response = first_execution["Payload"].read().decode("utf-8")
+
+    # and two others without the idempotency key
+    second_execution, _ = data_fetcher.get_lambda_response(
+        lambda_arn=optional_idempotency_key_fn_arn, payload=payload_without
+    )
+    second_execution_response = second_execution["Payload"].read().decode("utf-8")
+
+    third_execution, _ = data_fetcher.get_lambda_response(
+        lambda_arn=optional_idempotency_key_fn_arn, payload=payload_without
+    )
+    third_execution_response = third_execution["Payload"].read().decode("utf-8")
+
+    # THEN
+    # we should treat 2nd and 3rd requests with NULL idempotency key as non-idempotent transactions
+    # that is, no cache, no calls to persistent store, etc.
+    assert first_execution_response != second_execution_response
+    assert first_execution_response != third_execution_response
+    assert second_execution_response != third_execution_response

Diff for: tests/functional/idempotency/test_idempotency.py

@@ -2,7 +2,6 @@
 import datetime
 import sys
 import warnings
-from hashlib import md5
 from unittest.mock import MagicMock
 
 import jmespath
@@ -995,8 +994,7 @@ def test_default_no_raise_on_missing_idempotency_key(
     hashed_key = persistence_store._get_hashed_idempotency_key({})
 
     # THEN return the hash of None
-    expected_value = f"test-func.{function_name}#" + md5(json_serialize(None).encode()).hexdigest()
-    assert expected_value == hashed_key
+    assert hashed_key is None
 
 
 @pytest.mark.parametrize(
@@ -1093,7 +1091,7 @@ def lambda_handler(event, context):
     # WHEN handling the idempotent call
     # AND save_inprogress raises a ClientError
     with pytest.raises(IdempotencyPersistenceLayerError) as e:
-        lambda_handler({}, lambda_context)
+        lambda_handler({"data": "some"}, lambda_context)
 
     # THEN idempotent should raise an IdempotencyPersistenceLayerError
     # AND append downstream exception details
@@ -1363,7 +1361,7 @@ def two(data):
 
     assert one(data=mock_event) == "one"
     assert two(data=mock_event) == "two"
-    assert len(persistence_store.client.method_calls) == 4
+    assert len(persistence_store.client.method_calls) == 0
 
 
 def test_invalid_dynamodb_persistence_layer():