Skip to content

test(dynamodb): add coverage for numeric ExpressionAttributeNames (#0, #1)#166

Closed
rpalcolea wants to merge 2 commits intofloci-io:mainfrom
rpalcolea:fix/dynamodb-numeric-expression-attribute-names
Closed

test(dynamodb): add coverage for numeric ExpressionAttributeNames (#0, #1)#166
rpalcolea wants to merge 2 commits intofloci-io:mainfrom
rpalcolea:fix/dynamodb-numeric-expression-attribute-names

Conversation

@rpalcolea
Copy link
Copy Markdown
Contributor

Adds unit and integration tests verifying that numeric placeholder aliases in ExpressionAttributeNames are resolved correctly across:

  • Query KeyConditionExpression: #0 = :0 AND begins_with(fix: docker build on native #1, :prefix)
  • TransactWriteItems ConditionExpression: attribute_not_exists(#0)

Covers the scenario reported in #127 where PynamoDB-style numeric aliases were not being resolved, causing zero query results and spurious TransactionCanceledExceptions.

Associated to #127

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

Checklist

  • ./mvnw test passes locally
  • New or updated integration test added
  • Commit messages follow Conventional Commits

…floci-io#1)

Adds unit and integration tests verifying that numeric placeholder aliases
in ExpressionAttributeNames are resolved correctly across:

- Query KeyConditionExpression: #0 = :0 AND begins_with(floci-io#1, :prefix)
- TransactWriteItems ConditionExpression: attribute_not_exists(#0)

Covers the scenario reported in floci-io#127 where PynamoDB-style numeric aliases
were not being resolved, causing zero query results and spurious
TransactionCanceledExceptions.

Fixes floci-io#127
Copilot AI review requested due to automatic review settings April 1, 2026 17:15
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds DynamoDB test coverage to ensure numeric placeholder aliases (e.g., #0, #1) in ExpressionAttributeNames are correctly resolved in expressions, matching common PynamoDB-generated request patterns and guarding against the regression reported in #127.

Changes:

  • Add a unit test covering numeric ExpressionAttributeNames resolution in Query KeyConditionExpression.
  • Add an integration test covering numeric aliases in Query with begins_with.
  • Add an integration test covering numeric aliases in TransactWriteItems ConditionExpression (attribute_not_exists(#0)).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/test/java/io/github/hectorvent/floci/services/dynamodb/DynamoDbServiceTest.java Adds unit coverage for numeric ExpressionAttributeNames in a Query key condition expression.
src/test/java/io/github/hectorvent/floci/services/dynamodb/DynamoDbIntegrationTest.java Adds integration coverage for numeric aliases in Query and TransactWriteItems requests over the JSON 1.0 wire protocol.

Copy link
Copy Markdown
Collaborator

@hectorvent hectorvent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rpalcolea,

Please solve copilot review. Test Order duplicate.

@hectorvent
Copy link
Copy Markdown
Collaborator

@rpalcolea is this just to validate the issue is already solved? Could we close #127?

@fredpena
Copy link
Copy Markdown
Collaborator

fredpena commented Apr 1, 2026

@rpalcolea Are you going to create the service(function) related to those tests?

Because the function that corrects the fix has not yet been created; it has not been implemented yet.

Updated test method order for DynamoDbIntegrationTest.

Signed-off-by: Hector Ventura <hectorvent@gmail.com>
@hectorvent
Copy link
Copy Markdown
Collaborator

Closing as it is not required

@hectorvent hectorvent closed this Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants