Skip to content
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

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

Merged
merged 61 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
df37a55
Fixed EnhancedClient UpdateItem operation to make it work on nested a…
anirudh9391 May 30, 2024
1a2c32a
Merge branch 'master' into anirudkr/ddb-update
anirudh9391 May 30, 2024
8c39b17
Add Tests for multi-level nesting updates to work correctly
anirudh9391 May 30, 2024
f431d6f
Merge branch 'master' into anirudkr/ddb-update
anirudh9391 May 30, 2024
c6c2feb
Merge branch 'master' into anirudkr/ddb-update
anirudh9391 May 31, 2024
58d223a
Merge branch 'master' into anirudkr/ddb-update
anirudh9391 Jun 24, 2024
01584fe
Merge branch 'master' into anirudkr/ddb-update
anirudh9391 Jun 28, 2024
559b7b4
Fixed PR feedback
anirudh9391 Jul 1, 2024
b0b86be
Merge branch 'master' into anirudkr/ddb-update
anirudh9391 Jul 1, 2024
f677cfd
Updated Javadocs
anirudh9391 Jul 1, 2024
cf96d13
Addressed Pr feedback
anirudh9391 Jul 2, 2024
c06c200
Removed indendation changes
anirudh9391 Jul 2, 2024
3a65dee
Merge branch 'master' into anirudkr/ddb-update
anirudh9391 Jul 2, 2024
4080638
Merge branch 'master' into anirudkr/ddb-update
anirudh9391 Jul 2, 2024
5f6232d
Added tests for FlattenedMapper
anirudh9391 Jul 8, 2024
f2beece
Merge branch 'master' into anirudkr/ddb-update
anirudh9391 Jul 8, 2024
ca54c25
fixed indendation
anirudh9391 Jul 8, 2024
5e1fff1
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Jul 9, 2024
ac7033a
Fix indendation and remove unintentional changes
anirudh9391 Jul 9, 2024
4793709
Configured MappingConfiguration object
anirudh9391 Jul 11, 2024
4909472
Added methods to AttributeMapping interface
anirudh9391 Jul 11, 2024
c599881
Fixed unintentional indendation changes
anirudh9391 Jul 11, 2024
8a222ce
Fixed unintentional indendation changes
anirudh9391 Jul 11, 2024
6e5e39a
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Jul 11, 2024
7cedb09
Add changelogs
anirudh9391 Jul 11, 2024
950d308
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Jul 15, 2024
7ea245d
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Jul 23, 2024
459d040
Introduce a new method to transform input to be able to perform updat…
anirudh9391 Jul 24, 2024
f8500b3
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Jul 24, 2024
1f56f0b
Remove unwanted changes
anirudh9391 Jul 24, 2024
dcc3edc
Indent
anirudh9391 Jul 24, 2024
374a917
Remove unwanted changes
anirudh9391 Jul 24, 2024
3c9b67c
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Jul 25, 2024
b144d62
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Jul 25, 2024
5c7cbc7
Added testing
anirudh9391 Jul 28, 2024
d361c3b
Added test to validate updating string to null
anirudh9391 Jul 28, 2024
f5e694c
Remove unintentional indentation changes
anirudh9391 Jul 28, 2024
149e90d
Fix checkstyle
anirudh9391 Jul 28, 2024
f35e858
Update test case to add empty nested attribute
anirudh9391 Jul 30, 2024
ae8c56b
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Aug 13, 2024
84e82db
Added a test to verify that updates to non-scalar nested attributes w…
anirudh9391 Aug 19, 2024
7b5a488
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Aug 19, 2024
7ce686c
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Aug 27, 2024
46fd7c3
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Aug 28, 2024
db3f2fc
Uncomment test assertions
anirudh9391 Aug 28, 2024
711b020
Ensure correct workings of updating to an emoty map
anirudh9391 Aug 30, 2024
9ba36aa
Fixed checkstyle
anirudh9391 Aug 30, 2024
227d00f
Addressed pr feedback
anirudh9391 Sep 3, 2024
abaa8de
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Sep 3, 2024
435906c
Created modes of update operation and deprecated existing ignoreNulls…
anirudh9391 Sep 12, 2024
405b65d
Added more comments around deprecation
anirudh9391 Sep 13, 2024
1fde90c
Grouped validation methods
anirudh9391 Sep 16, 2024
9c4d4e1
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Sep 16, 2024
4ca8427
Added more documentation to define the modes of update operations
anirudh9391 Sep 16, 2024
018bf42
Removed MAPS_ONLY mode and added more documentation
anirudh9391 Sep 16, 2024
d53c705
Added unit test
anirudh9391 Sep 16, 2024
56da8c0
Improved documentation to establish tradeoffs between update modes
anirudh9391 Sep 16, 2024
7cde360
Modified error code in documentation
anirudh9391 Sep 17, 2024
74a6177
Updated Documentation around update modes
anirudh9391 Sep 17, 2024
6fd1bf9
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Sep 17, 2024
cee6627
Merge branch 'master' into anirudkr/ddb-updateitem-bug
anirudh9391 Sep 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-2a80679.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS SDK for Java v2",
"contributor": "anirudh9391",
"description": "Introduce a new method to transform input to be able to perform update operations on nested DynamoDB object attributes."
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@

package software.amazon.awssdk.enhanced.dynamodb.internal;

import static software.amazon.awssdk.enhanced.dynamodb.internal.operations.UpdateItemOperation.NESTED_OBJECT_UPDATE;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import software.amazon.awssdk.annotations.SdkInternalApi;
Expand All @@ -40,6 +43,7 @@ public final class EnhancedClientUtils {
private static final Set<Character> SPECIAL_CHARACTERS = Stream.of(
'*', '.', '-', '#', '+', ':', '/', '(', ')', ' ',
'&', '<', '>', '?', '=', '!', '@', '%', '$', '|').collect(Collectors.toSet());
private static final Pattern NESTED_OBJECT_PATTERN = Pattern.compile(NESTED_OBJECT_UPDATE);

private EnhancedClientUtils() {

Expand Down Expand Up @@ -67,18 +71,30 @@ public static String cleanAttributeName(String key) {
return somethingChanged ? new String(chars) : key;
}

private static boolean isNestedAttribute(String key) {
return key.contains(NESTED_OBJECT_UPDATE);
}

/**
* Creates a key token to be used with an ExpressionNames map.
*/
public static String keyRef(String key) {
return "#AMZN_MAPPED_" + cleanAttributeName(key);
String cleanAttributeName = cleanAttributeName(key);
cleanAttributeName = isNestedAttribute(cleanAttributeName) ?
NESTED_OBJECT_PATTERN.matcher(cleanAttributeName).replaceAll(".#AMZN_MAPPED_")
: cleanAttributeName;
return "#AMZN_MAPPED_" + cleanAttributeName;
}

/**
* Creates a value token to be used with an ExpressionValues map.
*/
public static String valueRef(String value) {
return ":AMZN_MAPPED_" + cleanAttributeName(value);
String cleanAttributeName = cleanAttributeName(value);
cleanAttributeName = isNestedAttribute(cleanAttributeName) ?
NESTED_OBJECT_PATTERN.matcher(cleanAttributeName).replaceAll("_")
: cleanAttributeName;
return ":AMZN_MAPPED_" + cleanAttributeName;
}

public static <T> T readAndTransformSingleItem(Map<String, AttributeValue> itemMap,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@

package software.amazon.awssdk.enhanced.dynamodb.internal.operations;

import static software.amazon.awssdk.enhanced.dynamodb.internal.EnhancedClientUtils.isNullAttributeValue;
import static software.amazon.awssdk.enhanced.dynamodb.internal.EnhancedClientUtils.readAndTransformSingleItem;
import static software.amazon.awssdk.enhanced.dynamodb.internal.update.UpdateExpressionUtils.operationExpression;
import static software.amazon.awssdk.utils.CollectionUtils.filterMap;

import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand All @@ -34,6 +36,7 @@
import software.amazon.awssdk.enhanced.dynamodb.extensions.WriteModification;
import software.amazon.awssdk.enhanced.dynamodb.internal.extensions.DefaultDynamoDbExtensionContext;
import software.amazon.awssdk.enhanced.dynamodb.internal.update.UpdateExpressionConverter;
import software.amazon.awssdk.enhanced.dynamodb.model.IgnoreNullsMode;
import software.amazon.awssdk.enhanced.dynamodb.model.TransactUpdateItemEnhancedRequest;
import software.amazon.awssdk.enhanced.dynamodb.model.UpdateItemEnhancedRequest;
import software.amazon.awssdk.enhanced.dynamodb.model.UpdateItemEnhancedResponse;
Expand All @@ -53,7 +56,8 @@
public class UpdateItemOperation<T>
implements TableOperation<T, UpdateItemRequest, UpdateItemResponse, UpdateItemEnhancedResponse<T>>,
TransactableWriteOperation<T> {


public static final String NESTED_OBJECT_UPDATE = "_NESTED_ATTR_UPDATE_";
private final Either<UpdateItemEnhancedRequest<T>, TransactUpdateItemEnhancedRequest<T>> request;

private UpdateItemOperation(UpdateItemEnhancedRequest<T> request) {
Expand Down Expand Up @@ -90,7 +94,22 @@ public UpdateItemRequest generateRequest(TableSchema<T> tableSchema,
r -> Optional.ofNullable(r.ignoreNulls()))
.orElse(null);

Map<String, AttributeValue> itemMap = tableSchema.itemToMap(item, Boolean.TRUE.equals(ignoreNulls));
IgnoreNullsMode ignoreNullsMode = request.map(r -> Optional.ofNullable(r.ignoreNullsMode()),
r -> Optional.ofNullable(r.ignoreNullsMode()))
.orElse(IgnoreNullsMode.DEFAULT);

if (ignoreNullsMode == IgnoreNullsMode.SCALAR_ONLY
|| ignoreNullsMode == IgnoreNullsMode.MAPS_ONLY) {
ignoreNulls = true;
}
Map<String, AttributeValue> itemMapImmutable = tableSchema.itemToMap(item, Boolean.TRUE.equals(ignoreNulls));

// 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 ?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line is different from the approved PR

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

transformItemToMapForUpdateExpression(itemMapImmutable) : itemMapImmutable;

TableMetadata tableMetadata = tableSchema.tableMetadata();

WriteModification transformation =
Expand Down Expand Up @@ -141,6 +160,58 @@ public UpdateItemRequest generateRequest(TableSchema<T> tableSchema,

return requestBuilder.build();
}

/**
* Method checks if a nested object parameter requires an update
* If so flattens out nested params separated by "_NESTED_ATTR_UPDATE_"
* this is consumed by @link EnhancedClientUtils to form the appropriate UpdateExpression
*/
public Map<String, AttributeValue> transformItemToMapForUpdateExpression(Map<String, AttributeValue> itemToMap) {

Map<String, AttributeValue> nestedAttributes = new HashMap<>();

itemToMap.forEach((key, value) -> {
if (value.hasM() && isNotEmptyMap(value.m())) {
nestedAttributes.put(key, value);
}
});

if (!nestedAttributes.isEmpty()) {
Map<String, AttributeValue> itemToMapMutable = new HashMap<>(itemToMap);
nestedAttributes.forEach((key, value) -> {
itemToMapMutable.remove(key);
nestedItemToMap(itemToMapMutable, key, value);
});
return itemToMapMutable;
}

return itemToMap;
}

private Map<String, AttributeValue> nestedItemToMap(Map<String, AttributeValue> itemToMap,
String key,
AttributeValue attributeValue) {
attributeValue.m().forEach((mapKey, mapValue) -> {
String nestedAttributeKey = key + NESTED_OBJECT_UPDATE + mapKey;
if (attributeValueNonNullOrShouldWriteNull(mapValue)) {
if (mapValue.hasM()) {
nestedItemToMap(itemToMap, nestedAttributeKey, mapValue);
} else {
itemToMap.put(nestedAttributeKey, mapValue);
}
}
});
return itemToMap;
}

private boolean isNotEmptyMap(Map<String, AttributeValue> map) {
return !map.isEmpty() && map.values().stream()
.anyMatch(this::attributeValueNonNullOrShouldWriteNull);
}

private boolean attributeValueNonNullOrShouldWriteNull(AttributeValue attributeValue) {
return !isNullAttributeValue(attributeValue);
}

@Override
public UpdateItemEnhancedResponse<T> transformResponse(UpdateItemResponse response,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
import static software.amazon.awssdk.enhanced.dynamodb.internal.EnhancedClientUtils.isNullAttributeValue;
import static software.amazon.awssdk.enhanced.dynamodb.internal.EnhancedClientUtils.keyRef;
import static software.amazon.awssdk.enhanced.dynamodb.internal.EnhancedClientUtils.valueRef;
import static software.amazon.awssdk.enhanced.dynamodb.internal.operations.UpdateItemOperation.NESTED_OBJECT_UPDATE;
import static software.amazon.awssdk.utils.CollectionUtils.filterMap;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.enhanced.dynamodb.TableMetadata;
Expand All @@ -39,6 +41,8 @@
@SdkInternalApi
public final class UpdateExpressionUtils {

private static final Pattern PATTERN = Pattern.compile(NESTED_OBJECT_UPDATE);

private UpdateExpressionUtils() {
}

Expand Down Expand Up @@ -136,9 +140,12 @@ private static Function<String, String> behaviorBasedValue(UpdateBehavior update
/**
* Simple utility method that can create an ExpressionNames map based on a list of attribute names.
*/
private static Map<String, String> expressionNamesFor(String... attributeNames) {
return Arrays.stream(attributeNames)
.collect(Collectors.toMap(EnhancedClientUtils::keyRef, Function.identity()));
}
private static Map<String, String> expressionNamesFor(String attributeNames) {
if (attributeNames.contains(NESTED_OBJECT_UPDATE)) {
return Arrays.stream(PATTERN.split(attributeNames)).distinct()
.collect(Collectors.toMap(EnhancedClientUtils::keyRef, Function.identity()));
}

return Collections.singletonMap(keyRef(attributeNames), attributeNames);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
Copy link
Contributor Author

@anirudh9391 anirudh9391 Sep 12, 2024

Choose a reason for hiding this comment

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

This enum is new

* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.awssdk.enhanced.dynamodb.model;

import software.amazon.awssdk.annotations.SdkPublicApi;

/**
* <p>
* The SCALAR_ONLY mode supports updates to scalar attributes to any level (top level, first nested level, second nested level,
* etc.) when the user wants to update 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 4xx DynamoDB exception if done so.
* <p>
* In the MAPS_ONLY mode, creation of new map/bean structures through update statements are supported, i.e. setting
* null/non-existent maps to non-null values. If users try to update scalar attributes in this mode, it will overwrite
* existing values in the table.
* <p>
* The DEFAULT mode disables any special handling around null values in the update query expression
*/
@SdkPublicApi
public enum IgnoreNullsMode {
SCALAR_ONLY,
MAPS_ONLY,
DEFAULT
Comment on lines +35 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@anirudh9391 anirudh9391 Sep 16, 2024

Choose a reason for hiding this comment

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

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

  1. MAPS_ONLY - allows users to set non-null values to null valued / nonexistent maps in ignoreNulls = true mode.
  2. DEFAULT - ignoreNulls = false mode, expecting users to retrieve and modify the item

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ public class TransactUpdateItemEnhancedRequest<T> {

private final T item;
private final Boolean ignoreNulls;
private final IgnoreNullsMode ignoreNullsMode;
private final Expression conditionExpression;
private final String returnValuesOnConditionCheckFailure;

private TransactUpdateItemEnhancedRequest(Builder<T> builder) {
this.item = builder.item;
this.ignoreNulls = builder.ignoreNulls;
this.ignoreNullsMode = builder.ignoreNullsMode;
this.conditionExpression = builder.conditionExpression;
this.returnValuesOnConditionCheckFailure = builder.returnValuesOnConditionCheckFailure;
}
Expand All @@ -67,6 +69,7 @@ public static <T> Builder<T> builder(Class<? extends T> itemClass) {
public Builder<T> toBuilder() {
return new Builder<T>().item(item)
.ignoreNulls(ignoreNulls)
.ignoreNullsMode(ignoreNullsMode)
.conditionExpression(conditionExpression)
.returnValuesOnConditionCheckFailure(returnValuesOnConditionCheckFailure);
}
Expand All @@ -80,11 +83,20 @@ public T item() {

/**
* Returns if the update operation should ignore attributes with null values, or false if it has not been set.
* This is deprecated in favour of ignoreNullsMode()
*/
@Deprecated
public Boolean ignoreNulls() {
return ignoreNulls;
}

/**
* Returns the mode of update to be performed
*/
public IgnoreNullsMode ignoreNullsMode() {
return ignoreNullsMode;
}

/**
* Returns the condition {@link Expression} set on this request object, or null if it doesn't exist.
*/
Expand Down Expand Up @@ -161,6 +173,7 @@ public int hashCode() {
public static final class Builder<T> {
private T item;
private Boolean ignoreNulls;
private IgnoreNullsMode ignoreNullsMode;
private Expression conditionExpression;
private String returnValuesOnConditionCheckFailure;

Expand All @@ -178,11 +191,17 @@ private Builder() {
* @param ignoreNulls the boolean value
* @return a builder of this type
*/
@Deprecated
public Builder<T> ignoreNulls(Boolean ignoreNulls) {
this.ignoreNulls = ignoreNulls;
return this;
}

public Builder<T> ignoreNullsMode(IgnoreNullsMode ignoreNullsMode) {
this.ignoreNullsMode = ignoreNullsMode;
return this;
}

/**
* Defines a logical expression on an item's attribute values which, if evaluating to true,
* will allow the update operation to succeed. If evaluating to false, the operation will not succeed.
Expand Down
Loading
Loading