Fixed EnhancedClient UpdateItem operation to make it work on nested attributes as well #5593

anirudh9391 · 2024-09-12T22:26:05Z

Modified Enhanced Client to facilitate nested update operations.

Motivation and Context

Customer reported an issue where UpdateItem does not work for nested attributes.

Modifications

Modified UpdateItem query path to form the right updateQuery for nested update attributes.
Previously nested update queries looked like

SET #AMZN_MAPPED_mainAddress = :AMZN_MAPPED_mainAddress
:AMZN_MAPPED_mainAddress -> "AttributeValue(M={state=AttributeValue(S=WA)})"

This operates like a putItem and updates the map with a new state.

Intead modified the update query to

aws dynamodb update-item \
    --table-name TEST_TABLE3 \
    --key '{"id":{"N":"111"}}' \
    --region us-west-2 \
    --update-expression "SET #AMZN_MAPPED_mainAddress.#AMZN_MAPPED_city = :AMZN_MAPPED_mainAddress_city" \
    --expression-attribute-values '{":AMZN_MAPPED_mainAddress_city": {"S": "Salem"}}' \  
    --expression-attribute-names '{"#AMZN_MAPPED_mainAddress": "mainAddress","#AMZN_MAPPED_city":"city"}'

Public APIs

ignoreNullsMode(IgnoreNullsMode ignoreNullsMode) - Sets the ignoreNullsMode, defining the configuration in which the update is to be carried out.

Usage :

dynamoDBTable.updateItem(r -> r.item(update_record).ignoreNullsMode(IgnoreNullsMode.SCALAR_ONLY));

public IgnoreNullsMode ignoreNullsMode() - Retrieves the mode of usage

Usage :

updateItemEnhancedRequest.ignoreNullsMode() == IgnoreNullsMode.SCALAR_ONLY ? : clause1 : clause2

Testing

Unit Tests pass

Screenshots (if appropriate)

Types of changes

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

…ttributes as well

…e operations on nested DynamoDB object attributes.

… configuration

anirudh9391 · 2024-09-12T22:26:34Z

...n/java/software/amazon/awssdk/enhanced/dynamodb/internal/operations/UpdateItemOperation.java

+        // If ignoreNulls is set to true, check for nested params to be updated
+        // If needed, Transform itemMap for it to be able to handle them.
+
+        Map<String, AttributeValue> itemMap = ignoreNullsMode == IgnoreNullsMode.SCALAR_ONLY ?


This line is different from the approved PR

So just to confirm, MAPS_ONLY mode is what we originally supported with ignoreNulls == true, and SCALAR_ONLY is new support we added?

Yes that is correct. with MAPS_ONLY, essentially a new map is created as part of a every item update regardless of whether its scalar or non-scalar. This does not work for scalar updates, and hence the SCALAR_ONLY

anirudh9391 · 2024-09-12T22:26:53Z

...b-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/IgnoreNullsMode.java

@@ -0,0 +1,32 @@
+/*


This enum is new

anirudh9391 · 2024-09-12T22:27:21Z

...c/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/UpdateBehaviorTest.java

@@ -145,6 +162,355 @@ public void updateBehaviors_transactWriteItems_secondUpdate() {
        assertThat(persistedRecord.getCreatedAutoUpdateOn()).isEqualTo(firstUpdatedRecord.getCreatedAutoUpdateOn());
    }

+    @Test


I have modified and added more tests

joviegas · 2024-09-12T23:43:23Z

Since we are adding Public APIS for this could you please mention the Public APIS in PR description and user experience in PR description

joviegas · 2024-09-13T18:03:20Z

...c/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/UpdateBehaviorTest.java

+        verifySingleLevelNestingDefaultUpdateBehavior(persistedRecord.getNestedRecord(), updatedNestedCounter);
+    }
+
+    @Test


Can we have parameterized test so that,its easy to find expected behaviors

public static List<Arguments> getDifferentCombinationOfignoreNullsModeAndExpectedResult() { return Arrays.asList( Arguments.of(simpleNestedAttrib, enum1, expectedMap1), Arguments.of(simpleNestedAttrib, enum2, expectedMap1), Arguments.of(nestedAttribWithEmpty, enum2, expectedMap1), etc etc @ParameterizedTest @MethodSource("getDifferentCombinationOfignoreNullsModeAndExpectedResult") void testUpdateWithDifferentModes(NestedAttibute attrib, IgnoreNullsMode ignoreNullsMode, Map<String, AttributeVale> expectedUpdate) { //existing common code mappedTable.updateItem(r -> r.item(attrib).ignoreNullsMode(ignoreNullsMode)); Map<String, AttributeVale> valueInDDB = nativeClient.get(item); assertThat(expectedUpdate).isEqualTo(valueInDDB) }

This is not going to make it neater. For each case, i.e. simple nested attribute, empty attribute or a multi nested attribute, the validation functions are different. If at all we group some methods together, it will not make it neater.

For eg. updating non-scalar object updates in scalar_only mode throws exception, whereas for maps_only mode has an expected output. Much more like this.

To make it more readable, I have grouped some of the validation methods together in the next revision into :

verifyMultipleLevelNestingTargetedUpdateBehavior

verifySingleLevelNestingTargetedUpdateBehavior

dagnir · 2024-09-16T16:35:26Z

...b-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/IgnoreNullsMode.java

+/**
+ * This enum offers users different modes of performing DDB item updates
+ * <p>
+ * In the SCALAR_ONLY mode, updates to nested scalar attributes are supported
+ * <p>
+ * In the MAPS_ONLY mode, updates to nested map structures are supported
+ * <p>
+ * The DEFAULT mode operates by setting ignoreNulls to false, and requires the user to
+ * fetch existing DDB item, make modifications to it and then update the item
+ */


This probably needs more detail, especially regarding tradeoffs

dagnir · 2024-09-16T16:39:57Z

...b-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/IgnoreNullsMode.java

+import software.amazon.awssdk.annotations.SdkPublicApi;
+
+/**
+ * This enum offers users different modes of performing DDB item updates


different modes of performing DDB item updates

This statement seems too generic

dagnir · 2024-09-16T16:48:08Z

...b-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/IgnoreNullsMode.java

+    MAPS_ONLY,
+    DEFAULT


What is the difference between MAPS_ONLY and DEFAULT? In the code, seems like we behave the same unless its SCALAR_ONLY

Good point. I have changed this and added a unit test.

MAPS_ONLY - allows users to set non-null values to null valued / nonexistent maps in ignoreNulls = true mode.

DEFAULT - ignoreNulls = false mode, expecting users to retrieve and modify the item

dagnir · 2024-09-16T22:55:09Z

...b-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/model/IgnoreNullsMode.java

+ * <p>
+ * In the SCALAR_ONLY mode, updates to nested scalar attributes are supported in the ignoreNulls = true mode, i.e. when the user
+ * wants to update nested scalar attributes by providing only the delta of changes to be updated. This mode does not support
+ * updates to maps and is expected to throw a 400x DynamoDB exception if done so.


nit: 400x looks like "400 times" to me. Should this be 4xx?

Yeah i will change it

dagnir · 2024-09-16T22:59:34Z

...n/java/software/amazon/awssdk/enhanced/dynamodb/internal/operations/UpdateItemOperation.java

+        // If ignoreNulls is set to true, check for nested params to be updated
+        // If needed, Transform itemMap for it to be able to handle them.
+
+        Map<String, AttributeValue> itemMap = ignoreNullsMode == IgnoreNullsMode.SCALAR_ONLY ?


So just to confirm, MAPS_ONLY mode is what we originally supported with ignoreNulls == true, and SCALAR_ONLY is new support we added?

sonarcloud · 2024-09-19T08:52:37Z

Quality Gate passed

Issues
36 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.9% Coverage on New Code
1.7% Duplication on New Code

See analysis details on SonarCloud

anirudh9391 added 30 commits May 30, 2024 04:57

Fixed EnhancedClient UpdateItem operation to make it work on nested a…

df37a55

…ttributes as well

Merge branch 'master' into anirudkr/ddb-update

1a2c32a

Add Tests for multi-level nesting updates to work correctly

8c39b17

Merge branch 'master' into anirudkr/ddb-update

f431d6f

Merge branch 'master' into anirudkr/ddb-update

c6c2feb

Merge branch 'master' into anirudkr/ddb-update

58d223a

Merge branch 'master' into anirudkr/ddb-update

01584fe

Fixed PR feedback

559b7b4

Merge branch 'master' into anirudkr/ddb-update

b0b86be

Updated Javadocs

f677cfd

Addressed Pr feedback

cf96d13

Removed indendation changes

c06c200

Merge branch 'master' into anirudkr/ddb-update

3a65dee

Merge branch 'master' into anirudkr/ddb-update

4080638

Added tests for FlattenedMapper

5f6232d

Merge branch 'master' into anirudkr/ddb-update

f2beece

fixed indendation

ca54c25

Merge branch 'master' into anirudkr/ddb-updateitem-bug

5e1fff1

Fix indendation and remove unintentional changes

ac7033a

Configured MappingConfiguration object

4793709

Added methods to AttributeMapping interface

4909472

Fixed unintentional indendation changes

c599881

Fixed unintentional indendation changes

8a222ce

Merge branch 'master' into anirudkr/ddb-updateitem-bug

6e5e39a

Add changelogs

7cedb09

Merge branch 'master' into anirudkr/ddb-updateitem-bug

950d308

Merge branch 'master' into anirudkr/ddb-updateitem-bug

7ea245d

Introduce a new method to transform input to be able to perform updat…

459d040

…e operations on nested DynamoDB object attributes.

Merge branch 'master' into anirudkr/ddb-updateitem-bug

f8500b3

Remove unwanted changes

1f56f0b

anirudh9391 added 4 commits August 29, 2024 18:48

Fixed checkstyle

9ba36aa

Addressed pr feedback

227d00f

Merge branch 'master' into anirudkr/ddb-updateitem-bug

abaa8de

Created modes of update operation and deprecated existing ignoreNulls…

435906c

… configuration

anirudh9391 requested a review from a team as a code owner September 12, 2024 22:26

anirudh9391 commented Sep 12, 2024

View reviewed changes

Added more comments around deprecation

405b65d

joviegas reviewed Sep 13, 2024

View reviewed changes

anirudh9391 added 2 commits September 16, 2024 02:09

Grouped validation methods

1fde90c

Merge branch 'master' into anirudkr/ddb-updateitem-bug

9c4d4e1

dagnir reviewed Sep 16, 2024

View reviewed changes

anirudh9391 added 4 commits September 16, 2024 11:43

Added more documentation to define the modes of update operations

4ca8427

Removed MAPS_ONLY mode and added more documentation

018bf42

Added unit test

d53c705

Improved documentation to establish tradeoffs between update modes

56da8c0

dagnir reviewed Sep 16, 2024

View reviewed changes

anirudh9391 added 2 commits September 17, 2024 08:13

Modified error code in documentation

7cde360

Updated Documentation around update modes

74a6177

dagnir approved these changes Sep 17, 2024

View reviewed changes

Merge branch 'master' into anirudkr/ddb-updateitem-bug

6fd1bf9

anirudh9391 added this pull request to the merge queue Sep 18, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 18, 2024

Merge branch 'master' into anirudkr/ddb-updateitem-bug

cee6627

anirudh9391 added this pull request to the merge queue Sep 19, 2024

Merged via the queue into master with commit 7ae6ed3 Sep 19, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed EnhancedClient UpdateItem operation to make it work on nested attributes as well #5593

Fixed EnhancedClient UpdateItem operation to make it work on nested attributes as well #5593

anirudh9391 commented Sep 12, 2024 •

edited

Loading

anirudh9391 Sep 12, 2024

dagnir Sep 16, 2024

anirudh9391 Sep 17, 2024

anirudh9391 Sep 12, 2024 •

edited

Loading

anirudh9391 Sep 12, 2024

joviegas commented Sep 12, 2024

joviegas Sep 13, 2024 •

edited

Loading

anirudh9391 Sep 16, 2024 •

edited

Loading

anirudh9391 Sep 16, 2024

dagnir Sep 16, 2024

dagnir Sep 16, 2024

dagnir Sep 16, 2024

anirudh9391 Sep 16, 2024 •

edited

Loading

dagnir Sep 16, 2024

anirudh9391 Sep 17, 2024

dagnir Sep 16, 2024

sonarcloud bot commented Sep 19, 2024

Fixed EnhancedClient UpdateItem operation to make it work on nested attributes as well #5593

Fixed EnhancedClient UpdateItem operation to make it work on nested attributes as well #5593

Conversation

anirudh9391 commented Sep 12, 2024 • edited Loading

Motivation and Context

Modifications

Public APIs

Testing

Screenshots (if appropriate)

Types of changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anirudh9391 Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joviegas commented Sep 12, 2024

joviegas Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

anirudh9391 Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anirudh9391 Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 19, 2024

Quality Gate passed

anirudh9391 commented Sep 12, 2024 •

edited

Loading

anirudh9391 Sep 12, 2024 •

edited

Loading

joviegas Sep 13, 2024 •

edited

Loading

anirudh9391 Sep 16, 2024 •

edited

Loading

anirudh9391 Sep 16, 2024 •

edited

Loading