From bb2ebb78a1e50e215652471a95f653a4f89084d7 Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Wed, 1 Mar 2023 11:47:30 -0500 Subject: [PATCH 1/3] fix(idempotency): static pk is overwritten by hashed_idempotency_key --- tests/functional/idempotency/conftest.py | 64 ++++++++++++++++++- .../idempotency/test_idempotency.py | 31 +++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/tests/functional/idempotency/conftest.py b/tests/functional/idempotency/conftest.py index 7e5fa0e7c61..b2242e950b7 100644 --- a/tests/functional/idempotency/conftest.py +++ b/tests/functional/idempotency/conftest.py @@ -77,7 +77,7 @@ def default_jmespath(): @pytest.fixture -def expected_params_update_item(serialized_lambda_response, hashed_idempotency_key): +def expected_params_update_item(serialized_lambda_response, hashed_idempotency_keyx): return { "ExpressionAttributeNames": { "#expiry": "expiration", @@ -95,6 +95,27 @@ def expected_params_update_item(serialized_lambda_response, hashed_idempotency_k } +@pytest.fixture +def expected_params_update_item_compound_key_static_pk_value( + serialized_lambda_response, hashed_idempotency_key, static_pk_value +): + return { + "ExpressionAttributeNames": { + "#expiry": "expiration", + "#response_data": "data", + "#status": "status", + }, + "ExpressionAttributeValues": { + ":expiry": {"N": stub.ANY}, + ":response_data": {"S": serialized_lambda_response}, + ":status": {"S": "COMPLETED"}, + }, + "Key": {"id": {"S": static_pk_value}, "sk": {"S": hashed_idempotency_key}}, + "TableName": "TEST_TABLE", + "UpdateExpression": "SET #response_data = :response_data, " "#expiry = :expiry, #status = :status", + } + + @pytest.fixture def expected_params_update_item_with_validation( serialized_lambda_response, hashed_idempotency_key, hashed_validation_key @@ -150,6 +171,35 @@ def expected_params_put_item(hashed_idempotency_key): } +@pytest.fixture +def expected_params_put_item_compound_key_static_pk_value(hashed_idempotency_key, static_pk_value): + return { + "ConditionExpression": ( + "attribute_not_exists(#id) OR #expiry < :now OR " + "(#status = :inprogress AND attribute_exists(#in_progress_expiry) AND #in_progress_expiry < :now_in_millis)" + ), + "ExpressionAttributeNames": { + "#id": "id", + "#expiry": "expiration", + "#status": "status", + "#in_progress_expiry": "in_progress_expiration", + }, + "ExpressionAttributeValues": { + ":now": {"N": stub.ANY}, + ":now_in_millis": {"N": stub.ANY}, + ":inprogress": {"S": "INPROGRESS"}, + }, + "Item": { + "expiration": {"N": stub.ANY}, + "in_progress_expiration": {"N": stub.ANY}, + "id": {"S": static_pk_value}, + "sk": {"S": hashed_idempotency_key}, + "status": {"S": "INPROGRESS"}, + }, + "TableName": "TEST_TABLE", + } + + @pytest.fixture def expected_params_put_item_with_validation(hashed_idempotency_key, hashed_validation_key): return { @@ -200,6 +250,11 @@ def hashed_idempotency_key_with_envelope(request, lambda_apigw_event): ) +@pytest.fixture +def static_pk_value(): + return "static-value" + + @pytest.fixture def hashed_validation_key(lambda_apigw_event): return hash_idempotency_key(lambda_apigw_event["requestContext"]) @@ -215,6 +270,13 @@ def persistence_store_compound(config): return DynamoDBPersistenceLayer(table_name=TABLE_NAME, boto_config=config, key_attr="id", sort_key_attr="sk") +@pytest.fixture +def persistence_store_compound_static_pk_value(config, static_pk_value): + return DynamoDBPersistenceLayer( + table_name=TABLE_NAME, boto_config=config, key_attr="id", sort_key_attr="sk", static_pk_value=static_pk_value + ) + + @pytest.fixture def idempotency_config(config, request, default_jmespath): return IdempotencyConfig( diff --git a/tests/functional/idempotency/test_idempotency.py b/tests/functional/idempotency/test_idempotency.py index dfc6b03b60c..68aeabeb50a 100644 --- a/tests/functional/idempotency/test_idempotency.py +++ b/tests/functional/idempotency/test_idempotency.py @@ -1504,3 +1504,34 @@ def lambda_handler(event, context): stubber.assert_no_pending_responses() stubber.deactivate() + + +@pytest.mark.parametrize("idempotency_config", [{"use_local_cache": False}], indirect=True) +def test_idempotent_lambda_compound_static_pk_value_has_correct_pk( + idempotency_config: IdempotencyConfig, + persistence_store_compound_static_pk_value: DynamoDBPersistenceLayer, + lambda_apigw_event, + expected_params_put_item_compound_key_static_pk_value, + expected_params_update_item_compound_key_static_pk_value, + lambda_response, + lambda_context, +): + """ + Test idempotent decorator having a DynamoDBPersistenceLayer with a compound key and a static PK value + """ + + stubber = stub.Stubber(persistence_store_compound_static_pk_value._client) + ddb_response = {} + + stubber.add_response("put_item", ddb_response, expected_params_put_item_compound_key_static_pk_value) + stubber.add_response("update_item", ddb_response, expected_params_update_item_compound_key_static_pk_value) + stubber.activate() + + @idempotent(config=idempotency_config, persistence_store=persistence_store_compound_static_pk_value) + def lambda_handler(event, context): + return lambda_response + + lambda_handler(lambda_apigw_event, lambda_context) + + stubber.assert_no_pending_responses() + stubber.deactivate() From 95784b5feaffd94576f57a5aee633756d2dede42 Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Wed, 1 Mar 2023 15:05:55 -0500 Subject: [PATCH 2/3] fix: dynamodb persisentece code and tests --- .../utilities/idempotency/persistence/dynamodb.py | 1 - tests/functional/idempotency/conftest.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py b/aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py index b05d8216b50..df356dd6ebb 100644 --- a/aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py +++ b/aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py @@ -146,7 +146,6 @@ def _get_record(self, idempotency_key) -> DataRecord: def _put_record(self, data_record: DataRecord) -> None: item = { **self._get_key(data_record.idempotency_key), - self.key_attr: {"S": data_record.idempotency_key}, self.expiry_attr: {"N": str(data_record.expiry_timestamp)}, self.status_attr: {"S": data_record.status}, } diff --git a/tests/functional/idempotency/conftest.py b/tests/functional/idempotency/conftest.py index b2242e950b7..bbcc25b8ab6 100644 --- a/tests/functional/idempotency/conftest.py +++ b/tests/functional/idempotency/conftest.py @@ -77,7 +77,7 @@ def default_jmespath(): @pytest.fixture -def expected_params_update_item(serialized_lambda_response, hashed_idempotency_keyx): +def expected_params_update_item(serialized_lambda_response, hashed_idempotency_key): return { "ExpressionAttributeNames": { "#expiry": "expiration", From be578fa4040526ee3bf17fb72e4565e3af4b0f8e Mon Sep 17 00:00:00 2001 From: Adam Tankanow Date: Wed, 1 Mar 2023 15:19:30 -0500 Subject: [PATCH 3/3] refactor: reduce copy/pasta in fixtures --- tests/functional/idempotency/conftest.py | 36 ++++-------------------- 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/tests/functional/idempotency/conftest.py b/tests/functional/idempotency/conftest.py index bbcc25b8ab6..53813a8b49d 100644 --- a/tests/functional/idempotency/conftest.py +++ b/tests/functional/idempotency/conftest.py @@ -97,22 +97,11 @@ def expected_params_update_item(serialized_lambda_response, hashed_idempotency_k @pytest.fixture def expected_params_update_item_compound_key_static_pk_value( - serialized_lambda_response, hashed_idempotency_key, static_pk_value + expected_params_update_item, hashed_idempotency_key, static_pk_value ): return { - "ExpressionAttributeNames": { - "#expiry": "expiration", - "#response_data": "data", - "#status": "status", - }, - "ExpressionAttributeValues": { - ":expiry": {"N": stub.ANY}, - ":response_data": {"S": serialized_lambda_response}, - ":status": {"S": "COMPLETED"}, - }, + **expected_params_update_item, "Key": {"id": {"S": static_pk_value}, "sk": {"S": hashed_idempotency_key}}, - "TableName": "TEST_TABLE", - "UpdateExpression": "SET #response_data = :response_data, " "#expiry = :expiry, #status = :status", } @@ -172,23 +161,11 @@ def expected_params_put_item(hashed_idempotency_key): @pytest.fixture -def expected_params_put_item_compound_key_static_pk_value(hashed_idempotency_key, static_pk_value): +def expected_params_put_item_compound_key_static_pk_value( + expected_params_put_item, hashed_idempotency_key, static_pk_value +): return { - "ConditionExpression": ( - "attribute_not_exists(#id) OR #expiry < :now OR " - "(#status = :inprogress AND attribute_exists(#in_progress_expiry) AND #in_progress_expiry < :now_in_millis)" - ), - "ExpressionAttributeNames": { - "#id": "id", - "#expiry": "expiration", - "#status": "status", - "#in_progress_expiry": "in_progress_expiration", - }, - "ExpressionAttributeValues": { - ":now": {"N": stub.ANY}, - ":now_in_millis": {"N": stub.ANY}, - ":inprogress": {"S": "INPROGRESS"}, - }, + **expected_params_put_item, "Item": { "expiration": {"N": stub.ANY}, "in_progress_expiration": {"N": stub.ANY}, @@ -196,7 +173,6 @@ def expected_params_put_item_compound_key_static_pk_value(hashed_idempotency_key "sk": {"S": hashed_idempotency_key}, "status": {"S": "INPROGRESS"}, }, - "TableName": "TEST_TABLE", }