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.

[LeeroyHannigan]

When a flattened object is null, this puts NULL for each of its attributes individually. But what if the flattened object has nested flattened objects? Does this correctly handle multi-level nesting, or will it only create nulls for the immediate level?

[iulianbudau]

It does handle multi-level flattening correctly. FlattenedObjectMappers is keyed by final flattened attribute names, and itemToMap is invoked recursively for each nested level via the corresponding FlattenedMapper. As a result, when item == null and ignoreNulls == false, NULL values are created for all flattened attributes across all levels, as validated by the unit tests added in this PR.

[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.

[LeeroyHannigan]

Why are we checking if item (the top-level object being mapped) is null here?

In the context of itemToMap(T item, boolean ignoreNulls), shouldn't item never be null? This seems different from the flattened object case where checking for null makes sense.

[iulianbudau]

Item can be null in this context because itemToMap is invoked recursively when processing FlattenedMappers. If a nested flattened object is null but itself contains another @DynamoDbFlatten member, we still need to traverse it to emit NULL values for its flattened attributes when ignoreNulls == false. This mirrors the flattened-object behavior and ensures consistent null propagation across all nesting levels.

[alextwoods]

Looks like some of the CI is failing, looking at logs it appears to be because of checkstyle:

[INFO] ------------------------------------------------------------------------
--
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.2:check (checkstyle) on project dynamodb-enhanced: Failed during checkstyle execution: There are 2 errors reported by Checkstyle 8.42 with software/amazon/awssdk/checkstyle.xml ruleset. -> [Help 1]

You should be able to check locally with:

mvn clean install -pl :dynamodb-enhanced

[alextwoods]

Nice - looks good and agree this addresses the issue from #6037

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
95.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud