Skip to content

Pass DynamoDb mapper config when using Table Mapper #1015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

vvondra
Copy link

@vvondra vvondra commented Feb 7, 2017

This is to fix the following scenario

     new DynamoDBMapper(
            dynamoDB,
            DynamoDBMapperConfig.builder()
                .withTableNameOverride(withTableNameReplacement(tableName))
                .build()
     ).newTableMapper(DomainObject.class);

In the scenario above, the TableMapper does not respect the mapper config, e.g. in this case the table name override. When loading the object, it uses the @DynamoDbTable annotation to find the table name instead of using the override table value.

Some methods already use config, but these PR makes the whole class consistent.

This is to fix the following scenario

```java
     new DynamoDBMapper(
            dynamoDB,
            DynamoDBMapperConfig.builder()
                .withTableNameOverride(withTableNameReplacement(tableName))
                .build()
     ).newTableMapper(DomainObject.class);
```

In the scenario aboce, the TableMapper does not respect the mapper config, e.g. in this case the table name override.
@millems
Copy link
Contributor

millems commented Nov 10, 2017

Sorry for the delay on this one.

Did we still want to integrate this change into 1.11.x? We're mostly heads-down on the 2.0 SDK, and we'll make sure this issue is fixed when we redesign the dynamo DB mapper as part of this issue: aws/aws-sdk-java-v2#35

@millems
Copy link
Contributor

millems commented Jun 8, 2018

Closing due to no reply. Feel free to reopen if this is still something you want merged. Sorry again for the original delay.

@millems millems closed this Jun 8, 2018
@vvondra
Copy link
Author

vvondra commented Jun 11, 2018

@millems the PR is still valid and I have a workaround in place. there's no BC break and the PR still cleanly merges, anything you'd need to get added to have it considered?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants