Bug: Idempotency DynamoDB _put_record method overwrites static_pk_value

[Tankanow]

Expected Behaviour

When I pass a static_pk_value value into the DynamoDBPersistenceLayer, I expect that value to be used in the Primary Key.

Current Behaviour

The PK value is overwritten with data_record.idempotency_key.

Code snippet

[This test](https://github.com/Tankanow/aws-lambda-powertools-python/pull/1/files) shows the problem. Running this test results in this failure:


E           aws_lambda_powertools.utilities.idempotency.exceptions.IdempotencyPersistenceLayerError: Failed to save in progress record to idempotency store - (Error getting response stub for operation PutItem: Expected parameters:
E           {'ConditionExpression': 'attribute_not_exists(#id) OR #expiry < :now OR '
E                                   '(#status = :inprogress AND '
E                                   'attribute_exists(#in_progress_expiry) AND '
E                                   '#in_progress_expiry < :now_in_millis)',
E            'ExpressionAttributeNames': {'#expiry': 'expiration',
E                                         '#id': 'id',
E                                         '#in_progress_expiry': 'in_progress_expiration',
E                                         '#status': 'status'},
E            'ExpressionAttributeValues': {':inprogress': {'S': 'INPROGRESS'},
E                                          ':now': {'N': <ANY>},
E                                          ':now_in_millis': {'N': <ANY>}},
E            'Item': {'expiration': {'N': <ANY>},
E                     'id': {'S': 'static-value'},
E                     'in_progress_expiration': {'N': <ANY>},
E                     'sk': {'S': 'test-func.functional.idempotency.test_idempotency.test_idempotent_lambda_compound_static_pk_value_has_correct_pk.<locals>.lambda_handler#be4755f20c4e6f94f43bc3f5849a26fb'},
E                     'status': {'S': 'INPROGRESS'}},
E            'TableName': 'TEST_TABLE'},
E           but received:
E           {'ConditionExpression': 'attribute_not_exists(#id) OR #expiry < :now OR '
E                                   '(#status = :inprogress AND '
E                                   'attribute_exists(#in_progress_expiry) AND '
E                                   '#in_progress_expiry < :now_in_millis)',
E            'ExpressionAttributeNames': {'#expiry': 'expiration',
E                                         '#id': 'id',
E                                         '#in_progress_expiry': 'in_progress_expiration',
E                                         '#status': 'status'},
E            'ExpressionAttributeValues': {':inprogress': {'S': 'INPROGRESS'},
E                                          ':now': {'N': '1677689099'},
E                                          ':now_in_millis': {'N': '1677689099925'}},
E            'Item': {'expiration': {'N': '1677692699'},
E                     'id': {'S': 'test-func.functional.idempotency.test_idempotency.test_idempotent_lambda_compound_static_pk_value_has_correct_pk.<locals>.lambda_handler#be4755f20c4e6f94f43bc3f5849a26fb'},
E                     'in_progress_expiration': {'N': '1677689100925'},
E                     'sk': {'S': 'test-func.functional.idempotency.test_idempotency.test_idempotent_lambda_compound_static_pk_value_has_correct_pk.<locals>.lambda_handler#be4755f20c4e6f94f43bc3f5849a26fb'},
E                     'status': {'S': 'INPROGRESS'}},
E            'TableName': 'TEST_TABLE'})

Possible Solution

Simply remove the offending line, self.key_attr: {"S": data_record.idempotency_key}, in https://github.com/Tankanow/aws-lambda-powertools-python/blob/5f9addda12d3c4066fe7f480f99184126b154d57/aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py#L149.

Steps to Reproduce

Run the tests in this PR

I will happily fix if this issue is accepted.

AWS Lambda Powertools for Python version

latest

AWS Lambda function runtime

3.9

Packaging format used

PyPi

Debugging logs

No response

[heitorlessa]

Hey @Tankanow - feel free to send a PR for this, as you're a highly regarded contributor ;-)

I do wonder if we missed this earlier during implementation, or if it's a regression.

Since you last contributed we now have E2E tests ;-)

Thank you as always

PS: dinner time now but can look into it first thing tomorrow morning, and cut a patch release

[Tankanow]

Hi @heitorlessa, I believe it is a regression. We found it in our unit tests that consume this code after upgrading to 2.8.0.

[heitorlessa]

Hmmm no bueno and surprises me since we had tests for that.

Last change on Idempotency persistence later was this that made it thread-safe.

Barely keeping my eyes open but can investigate it tomorrow morning (CET) and cut a patch release ASAP.

#1899

[heitorlessa]

sadly it was added in the PR that addressed thread-safety (larger structural change) - got my laptop now, eye dropper and looking into it what test was changed that shouldn't have, and cutting a patch release if confirmed.

[heitorlessa]

looking at git blame and history, there wasn't an explicit test for this feature static_pk_value, otherwise that dict mutation line would've raised an error.

@Tankanow You opened a PR in your base branch instead of the origin ;) Feel free to correct it, and I can make minor modifications, merge, and release

[heitorlessa]

PR up, copied tests over to make this quicker, will credit as usual, and will double check one more time to be safe.

[heitorlessa]

@Tankanow - good to go. I'll wait a few minutes to hear it's okay to merge from you, otherwise I'll go ahead.

[heitorlessa]

Release in process - might take ~2m for PyPi and 30m for Lambda Layer (v23) to be available across all regions.

https://github.com/awslabs/aws-lambda-powertools-python/actions/runs/4307851934