Fixed DynamoDbEnhancedClient TableSchema::itemToMap to handle null fl…

[iulianbudau]

Fixed [DynamoDbEnhancedClient] TableSchema::itemToMap doesn't respect ignoreNulls with null @DynamoDbFlattened members #2540.

Description

Current behavior: TableSchema.fromBean(Bean.class).itemToMap(bean, false) returns a map without members of any @DynamoDbFlatten'd members if the flattened member is null even though ignoreNulls is set to false.

Expected behavior: TableSchema::itemToMap should return a map that contains a consistent representation of null top-level (non-flattened) attributes and flattened attributes when their "parent" member is null and ignoreNulls is set to false.

Motivation and Context

#2540

Modifications

Updated the StaticImmutableTableSchema -> Map<String, AttributeValue> itemToMap(T item, boolean ignoreNulls) logic to properly account for the ignoreNulls check and generate AttributeValue.fromNul(true) values for attributes of @DynamoDbFlattened members that are null.

Testing

The changes have already been tested by running the existing tests and also added new unit tests for the fixed behavior.

Test Coverage Checklist

Scenario	Done	Comments if Not Done
1. Different TableSchema Creation Methods
a. TableSchema.fromBean(Customer.class)	[ ]
b. TableSchema.fromImmutableClass(Customer.class) for immutable classes	[ ]
c. TableSchema.documentSchemaBuilder().build()	[ ]
d. StaticTableSchema.builder(Customer.class)	[ ]
2. Nesting of Different TableSchema Types
a. @DynamoDbBean with nested @DynamoDbBean as NonNull	[x]
b. @DynamoDbBean with nested @DynamoDbImmutable as NonNull	[x]
c. @DynamoDbImmutable with nested @DynamoDbBean as NonNull	[x]
d. @DynamoDbBean with nested @DynamoDbBean as Null	[x]
e. @DynamoDbBean with nested @DynamoDbImmutable as Null	[x]
f. @DynamoDbImmutable with nested @DynamoDbBean as Null	[x]
3. CRUD Operations
a. scan()	[ ]
b. query()	[ ]
c. updateItem()	[ ]
d. putItem()	[ ]
e. getItem()	[ ]
f. deleteItem()	[ ]
g. batchGetItem()	[ ]
h. batchWriteItem()	[ ]
i. transactGetItems()	[ ]
j. transactWriteItems()	[ ]
4. Data Types and Null Handling
a. top-level null attributes	[ ]
b. collections with null elements	[ ]
c. maps with null values	[ ]
d. conversion between null Java values and AttributeValue	[ ]
e. full serialization/deserialization cycle with null values	[ ]
5. AsyncTable and SyncTable
a. DynamoDbAsyncTable Testing	[ ]
b. DynamoDbTable Testing	[ ]
6. New/Modification in Extensions
a. Tables with Scenario in ScenarioSl No.1 (All table schemas are Must)	[ ]
b. Test with Default Values in Annotations	[ ]
c. Combination of Annotation and Builder passes extension	[ ]
7. New/Modification in Converters
a. Tables with Scenario in ScenarioSl No.1 (All table schemas are Must)	[ ]
b. Test with Default Values in Annotations	[ ]
c. Test All Scenarios from 1 to 5	[ ]

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist

• I have read the CONTRIBUTING document
• Local run of mvn install succeeds
• My code follows the code style of this project
• My change requires a change to the Javadoc documentation
• I have updated the Javadoc documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed
• I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
• My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

• I confirm that this pull request can be released under the Apache 2 license

[shetsa-amzn]

Is this the right behavior ? Here name is an attribute name from the flattened object, not the flattened field name itself.

[iulianbudau]

Yes the behavior is correct. The whole idea with the flattened is to bring the attributes from all the flattened members to root in the returned map, as like they belong to the parent root level. The flattened fields themself containing those attributes will actually not be visible in the DynamoDb table.

[shetsa-amzn]

Also when item != null, the same flattenedMapper.itemToMap() is called multiple times for the same flattened object. Is my understanding correct?

Can we add this so it loops just once for every flattenMapper?

if (processedMappers.add(flattenedMapper)) {
            attributeValueMap.putAll(flattenedMapper.itemToMap(item, ignoreNulls));
        }

[iulianbudau]

Good catch. I updated the PR with this improvement.