From 32a4ca28999cf5d1147f5dc10ef3ea5b1d3b16a5 Mon Sep 17 00:00:00 2001 From: Michael Brewer Date: Sat, 4 Dec 2021 14:32:26 -0800 Subject: [PATCH 1/3] fix(idempotency): include function name in hash closes #855 --- .../utilities/idempotency/base.py | 2 +- .../utilities/idempotency/persistence/base.py | 5 +- tests/functional/idempotency/conftest.py | 4 +- .../idempotency/test_idempotency.py | 51 +++++++++++++------ 4 files changed, 41 insertions(+), 21 deletions(-) diff --git a/aws_lambda_powertools/utilities/idempotency/base.py b/aws_lambda_powertools/utilities/idempotency/base.py index 4b82c923a70..7dee94fc356 100644 --- a/aws_lambda_powertools/utilities/idempotency/base.py +++ b/aws_lambda_powertools/utilities/idempotency/base.py @@ -56,7 +56,7 @@ def __init__( self.fn_args = function_args self.fn_kwargs = function_kwargs - persistence_store.configure(config) + persistence_store.configure(config, self.function.__name__) self.persistence_store = persistence_store def handle(self) -> Any: diff --git a/aws_lambda_powertools/utilities/idempotency/persistence/base.py b/aws_lambda_powertools/utilities/idempotency/persistence/base.py index 907af8edaa7..bcd7f0fb8dd 100644 --- a/aws_lambda_powertools/utilities/idempotency/persistence/base.py +++ b/aws_lambda_powertools/utilities/idempotency/persistence/base.py @@ -124,7 +124,7 @@ def __init__(self): self._cache: Optional[LRUDict] = None self.hash_function = None - def configure(self, config: IdempotencyConfig) -> None: + def configure(self, config: IdempotencyConfig, function_name: str) -> None: """ Initialize the base persistence layer from the configuration settings @@ -133,6 +133,7 @@ def configure(self, config: IdempotencyConfig) -> None: config: IdempotencyConfig Idempotency configuration settings """ + self.function_name = function_name if self.configured: # Prevent being reconfigured multiple times return @@ -178,7 +179,7 @@ def _get_hashed_idempotency_key(self, data: Dict[str, Any]) -> str: warnings.warn(f"No value found for idempotency_key. jmespath: {self.event_key_jmespath}") generated_hash = self._generate_hash(data=data) - function_name = os.getenv(constants.LAMBDA_FUNCTION_NAME_ENV, "test-func") + function_name = os.getenv(constants.LAMBDA_FUNCTION_NAME_ENV, "test-func") + "." + self.function_name return f"{function_name}#{generated_hash}" @staticmethod diff --git a/tests/functional/idempotency/conftest.py b/tests/functional/idempotency/conftest.py index 71b5978497c..0f74d503b88 100644 --- a/tests/functional/idempotency/conftest.py +++ b/tests/functional/idempotency/conftest.py @@ -150,7 +150,7 @@ def expected_params_put_item_with_validation(hashed_idempotency_key, hashed_vali def hashed_idempotency_key(lambda_apigw_event, default_jmespath, lambda_context): compiled_jmespath = jmespath.compile(default_jmespath) data = compiled_jmespath.search(lambda_apigw_event) - return "test-func#" + hashlib.md5(serialize(data).encode()).hexdigest() + return "test-func.lambda_handler#" + hashlib.md5(serialize(data).encode()).hexdigest() @pytest.fixture @@ -158,7 +158,7 @@ def hashed_idempotency_key_with_envelope(lambda_apigw_event): event = extract_data_from_envelope( data=lambda_apigw_event, envelope=envelopes.API_GATEWAY_HTTP, jmespath_options={} ) - return "test-func#" + hashlib.md5(serialize(event).encode()).hexdigest() + return "test-func.lambda_handler#" + hashlib.md5(serialize(event).encode()).hexdigest() @pytest.fixture diff --git a/tests/functional/idempotency/test_idempotency.py b/tests/functional/idempotency/test_idempotency.py index 043fb06a04a..806f38a1c75 100644 --- a/tests/functional/idempotency/test_idempotency.py +++ b/tests/functional/idempotency/test_idempotency.py @@ -648,7 +648,7 @@ def test_in_progress_never_saved_to_cache( ): # GIVEN a data record with status "INPROGRESS" # and persistence_store has use_local_cache = True - persistence_store.configure(idempotency_config) + persistence_store.configure(idempotency_config, "handler") data_record = DataRecord("key", status="INPROGRESS") # WHEN saving to local cache @@ -661,7 +661,7 @@ def test_in_progress_never_saved_to_cache( @pytest.mark.parametrize("idempotency_config", [{"use_local_cache": False}], indirect=True) def test_user_local_disabled(idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer): # GIVEN a persistence_store with use_local_cache = False - persistence_store.configure(idempotency_config) + persistence_store.configure(idempotency_config, "") # WHEN calling any local cache options data_record = DataRecord("key", status="COMPLETED") @@ -683,7 +683,7 @@ def test_delete_from_cache_when_empty( idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer ): # GIVEN use_local_cache is True AND the local cache is empty - persistence_store.configure(idempotency_config) + persistence_store.configure(idempotency_config, "handler") try: # WHEN we _delete_from_cache @@ -735,7 +735,8 @@ def test_default_no_raise_on_missing_idempotency_key( idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer, lambda_context ): # GIVEN a persistence_store with use_local_cache = False and event_key_jmespath = "body" - persistence_store.configure(idempotency_config) + function_name = "foo" + persistence_store.configure(idempotency_config, function_name) assert persistence_store.use_local_cache is False assert "body" in persistence_store.event_key_jmespath @@ -743,7 +744,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 = "test-func#" + md5(serialize(None).encode()).hexdigest() + expected_value = f"test-func.{function_name}#" + md5(serialize(None).encode()).hexdigest() assert expected_value == hashed_key @@ -754,7 +755,7 @@ def test_raise_on_no_idempotency_key( idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer, lambda_context ): # GIVEN a persistence_store with raise_on_no_idempotency_key and no idempotency key in the request - persistence_store.configure(idempotency_config) + persistence_store.configure(idempotency_config, "handler") persistence_store.raise_on_no_idempotency_key = True assert persistence_store.use_local_cache is False assert "body" in persistence_store.event_key_jmespath @@ -781,7 +782,7 @@ def test_jmespath_with_powertools_json( idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer, lambda_context ): # GIVEN an event_key_jmespath with powertools_json custom function - persistence_store.configure(idempotency_config) + persistence_store.configure(idempotency_config, "handler") sub_attr_value = "cognito_user" static_pk_value = "some_key" expected_value = [sub_attr_value, static_pk_value] @@ -794,7 +795,7 @@ def test_jmespath_with_powertools_json( result = persistence_store._get_hashed_idempotency_key(api_gateway_proxy_event) # THEN the hashed idempotency key should match the extracted values generated hash - assert result == "test-func#" + persistence_store._generate_hash(expected_value) + assert result == "test-func.handler#" + persistence_store._generate_hash(expected_value) @pytest.mark.parametrize("config_with_jmespath_options", ["powertools_json(data).payload"], indirect=True) @@ -803,7 +804,7 @@ def test_custom_jmespath_function_overrides_builtin_functions( ): # GIVEN an persistence store with a custom jmespath_options # AND use a builtin powertools custom function - persistence_store.configure(config_with_jmespath_options) + persistence_store.configure(config_with_jmespath_options, "handler") with pytest.raises(jmespath.exceptions.UnknownFunctionError, match="Unknown function: powertools_json()"): # WHEN calling _get_hashed_idempotency_key @@ -871,7 +872,9 @@ def _delete_record(self, data_record: DataRecord) -> None: def test_idempotent_lambda_event_source(lambda_context): # Scenario to validate that we can use the event_source decorator before or after the idempotent decorator mock_event = load_event("apiGatewayProxyV2Event.json") - persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()) + persistence_layer = MockPersistenceLayer( + "test-func.lambda_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest() + ) expected_result = {"message": "Foo"} # GIVEN an event_source decorator @@ -891,7 +894,9 @@ def lambda_handler(event, _): def test_idempotent_function(): # Scenario to validate we can use idempotent_function with any function mock_event = {"data": "value"} - persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()) + persistence_layer = MockPersistenceLayer( + "test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest() + ) expected_result = {"message": "Foo"} @idempotent_function(persistence_store=persistence_layer, data_keyword_argument="record") @@ -908,7 +913,9 @@ def test_idempotent_function_arbitrary_args_kwargs(): # Scenario to validate we can use idempotent_function with a function # with an arbitrary number of args and kwargs mock_event = {"data": "value"} - persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()) + persistence_layer = MockPersistenceLayer( + "test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest() + ) expected_result = {"message": "Foo"} @idempotent_function(persistence_store=persistence_layer, data_keyword_argument="record") @@ -923,7 +930,9 @@ def record_handler(arg_one, arg_two, record, is_record): def test_idempotent_function_invalid_data_kwarg(): mock_event = {"data": "value"} - persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()) + persistence_layer = MockPersistenceLayer( + "test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest() + ) expected_result = {"message": "Foo"} keyword_argument = "payload" @@ -940,7 +949,9 @@ def record_handler(record): def test_idempotent_function_arg_instead_of_kwarg(): mock_event = {"data": "value"} - persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()) + persistence_layer = MockPersistenceLayer( + "test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest() + ) expected_result = {"message": "Foo"} keyword_argument = "record" @@ -958,13 +969,19 @@ def record_handler(record): def test_idempotent_function_and_lambda_handler(lambda_context): # Scenario to validate we can use both idempotent_function and idempotent decorators mock_event = {"data": "value"} - persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(serialize(mock_event).encode()).hexdigest()) + persistence_layer = MockPersistenceLayer( + "test-func.record_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest() + ) expected_result = {"message": "Foo"} @idempotent_function(persistence_store=persistence_layer, data_keyword_argument="record") def record_handler(record): return expected_result + persistence_layer = MockPersistenceLayer( + "test-func.lambda_handler#" + hashlib.md5(serialize(mock_event).encode()).hexdigest() + ) + @idempotent(persistence_store=persistence_layer) def lambda_handler(event, _): return expected_result @@ -986,7 +1003,9 @@ def test_idempotent_data_sorting(): data_two = {"more_data": "more data 1", "data": "test message 1"} # Assertion will happen in MockPersistenceLayer - persistence_layer = MockPersistenceLayer("test-func#" + hashlib.md5(json.dumps(data_one).encode()).hexdigest()) + persistence_layer = MockPersistenceLayer( + "test-func.dummy#" + hashlib.md5(json.dumps(data_one).encode()).hexdigest() + ) # GIVEN @idempotent_function(data_keyword_argument="payload", persistence_store=persistence_layer) From ac34729981d9ec60c5421083c1cef8e7e7dd1642 Mon Sep 17 00:00:00 2001 From: Michael Brewer Date: Sat, 4 Dec 2021 14:55:12 -0800 Subject: [PATCH 2/3] tests(idempotency): add missing fail test --- .../utilities/idempotency/persistence/base.py | 1 + .../idempotency/test_idempotency.py | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/aws_lambda_powertools/utilities/idempotency/persistence/base.py b/aws_lambda_powertools/utilities/idempotency/persistence/base.py index bcd7f0fb8dd..ae5fa3d7368 100644 --- a/aws_lambda_powertools/utilities/idempotency/persistence/base.py +++ b/aws_lambda_powertools/utilities/idempotency/persistence/base.py @@ -112,6 +112,7 @@ class BasePersistenceLayer(ABC): def __init__(self): """Initialize the defaults""" + self.function_name = None self.configured = False self.event_key_jmespath: Optional[str] = None self.event_key_compiled_jmespath = None diff --git a/tests/functional/idempotency/test_idempotency.py b/tests/functional/idempotency/test_idempotency.py index 806f38a1c75..42985444b80 100644 --- a/tests/functional/idempotency/test_idempotency.py +++ b/tests/functional/idempotency/test_idempotency.py @@ -1036,3 +1036,24 @@ def dummy_handler(event, context): dummy_handler(mock_event, lambda_context) assert len(persistence_store.table.method_calls) == 0 + + +@pytest.mark.parametrize("idempotency_config", [{"use_local_cache": True}], indirect=True) +def test_idempotent_function_duplicates( + idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer +): + # Scenario to validate the both methods are called + mock_event = {"data": "value"} + persistence_store.table = MagicMock() + + @idempotent_function(data_keyword_argument="data", persistence_store=persistence_store, config=idempotency_config) + def one(data): + return "one" + + @idempotent_function(data_keyword_argument="data", persistence_store=persistence_store, config=idempotency_config) + def two(data): + return "two" + + assert one(data=mock_event) == "one" + assert two(data=mock_event) == "two" + assert len(persistence_store.table.method_calls) == 4 From 2cd2b3bb6b5f753a77b7289024603583429d6d6b Mon Sep 17 00:00:00 2001 From: Michael Brewer Date: Mon, 6 Dec 2021 15:02:04 -0800 Subject: [PATCH 3/3] refactor(idemptonency): slight performance tuning make configure backwards compatible resolve the full function name ahead of time --- .../utilities/idempotency/persistence/base.py | 12 +++++++----- tests/functional/idempotency/test_idempotency.py | 12 ++++++------ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/aws_lambda_powertools/utilities/idempotency/persistence/base.py b/aws_lambda_powertools/utilities/idempotency/persistence/base.py index ae5fa3d7368..8f2b30d289a 100644 --- a/aws_lambda_powertools/utilities/idempotency/persistence/base.py +++ b/aws_lambda_powertools/utilities/idempotency/persistence/base.py @@ -112,7 +112,7 @@ class BasePersistenceLayer(ABC): def __init__(self): """Initialize the defaults""" - self.function_name = None + self.function_name = "" self.configured = False self.event_key_jmespath: Optional[str] = None self.event_key_compiled_jmespath = None @@ -125,7 +125,7 @@ def __init__(self): self._cache: Optional[LRUDict] = None self.hash_function = None - def configure(self, config: IdempotencyConfig, function_name: str) -> None: + def configure(self, config: IdempotencyConfig, function_name: Optional[str] = None) -> None: """ Initialize the base persistence layer from the configuration settings @@ -133,8 +133,11 @@ def configure(self, config: IdempotencyConfig, function_name: str) -> None: ---------- config: IdempotencyConfig Idempotency configuration settings + function_name: str, Optional + The name of the function being decorated """ - self.function_name = function_name + self.function_name = f"{os.getenv(constants.LAMBDA_FUNCTION_NAME_ENV, 'test-func')}.{function_name or ''}" + if self.configured: # Prevent being reconfigured multiple times return @@ -180,8 +183,7 @@ def _get_hashed_idempotency_key(self, data: Dict[str, Any]) -> str: warnings.warn(f"No value found for idempotency_key. jmespath: {self.event_key_jmespath}") generated_hash = self._generate_hash(data=data) - function_name = os.getenv(constants.LAMBDA_FUNCTION_NAME_ENV, "test-func") + "." + self.function_name - return f"{function_name}#{generated_hash}" + return f"{self.function_name}#{generated_hash}" @staticmethod def is_missing_idempotency_key(data) -> bool: diff --git a/tests/functional/idempotency/test_idempotency.py b/tests/functional/idempotency/test_idempotency.py index 42985444b80..a8cf652d8a0 100644 --- a/tests/functional/idempotency/test_idempotency.py +++ b/tests/functional/idempotency/test_idempotency.py @@ -648,7 +648,7 @@ def test_in_progress_never_saved_to_cache( ): # GIVEN a data record with status "INPROGRESS" # and persistence_store has use_local_cache = True - persistence_store.configure(idempotency_config, "handler") + persistence_store.configure(idempotency_config) data_record = DataRecord("key", status="INPROGRESS") # WHEN saving to local cache @@ -661,7 +661,7 @@ def test_in_progress_never_saved_to_cache( @pytest.mark.parametrize("idempotency_config", [{"use_local_cache": False}], indirect=True) def test_user_local_disabled(idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer): # GIVEN a persistence_store with use_local_cache = False - persistence_store.configure(idempotency_config, "") + persistence_store.configure(idempotency_config) # WHEN calling any local cache options data_record = DataRecord("key", status="COMPLETED") @@ -683,7 +683,7 @@ def test_delete_from_cache_when_empty( idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer ): # GIVEN use_local_cache is True AND the local cache is empty - persistence_store.configure(idempotency_config, "handler") + persistence_store.configure(idempotency_config) try: # WHEN we _delete_from_cache @@ -755,7 +755,7 @@ def test_raise_on_no_idempotency_key( idempotency_config: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer, lambda_context ): # GIVEN a persistence_store with raise_on_no_idempotency_key and no idempotency key in the request - persistence_store.configure(idempotency_config, "handler") + persistence_store.configure(idempotency_config) persistence_store.raise_on_no_idempotency_key = True assert persistence_store.use_local_cache is False assert "body" in persistence_store.event_key_jmespath @@ -802,9 +802,9 @@ def test_jmespath_with_powertools_json( def test_custom_jmespath_function_overrides_builtin_functions( config_with_jmespath_options: IdempotencyConfig, persistence_store: DynamoDBPersistenceLayer, lambda_context ): - # GIVEN an persistence store with a custom jmespath_options + # GIVEN a persistence store with a custom jmespath_options # AND use a builtin powertools custom function - persistence_store.configure(config_with_jmespath_options, "handler") + persistence_store.configure(config_with_jmespath_options) with pytest.raises(jmespath.exceptions.UnknownFunctionError, match="Unknown function: powertools_json()"): # WHEN calling _get_hashed_idempotency_key