Support update expressions in single request update #6471
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "type": "feature", | ||
| "category": "Amazon DynamoDB Enhanced Client", | ||
| "contributor": "", | ||
| "description": "Support update expressions in single request update" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| /* | ||
| * 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.internal.update; | ||
|
|
||
| import static java.util.Objects.requireNonNull; | ||
| import static software.amazon.awssdk.enhanced.dynamodb.internal.EnhancedClientUtils.isNullAttributeValue; | ||
| import static software.amazon.awssdk.enhanced.dynamodb.internal.update.UpdateExpressionUtils.removeActionsFor; | ||
| import static software.amazon.awssdk.enhanced.dynamodb.internal.update.UpdateExpressionUtils.setActionsFor; | ||
| import static software.amazon.awssdk.utils.CollectionUtils.filterMap; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Set; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.Stream; | ||
| import software.amazon.awssdk.annotations.SdkInternalApi; | ||
| import software.amazon.awssdk.enhanced.dynamodb.TableMetadata; | ||
| import software.amazon.awssdk.enhanced.dynamodb.update.UpdateExpression; | ||
| import software.amazon.awssdk.services.dynamodb.model.AttributeValue; | ||
|
|
||
| /** | ||
| * Resolves and merges UpdateExpressions from multiple sources (item attributes, extensions, requests) with priority-based | ||
| * conflict resolution and smart filtering to prevent attribute conflicts. | ||
| */ | ||
| @SdkInternalApi | ||
| public final class UpdateExpressionResolver { | ||
|
|
||
| private final TableMetadata tableMetadata; | ||
| private final Map<String, AttributeValue> nonKeyAttributes; | ||
| private final UpdateExpression extensionExpression; | ||
| private final UpdateExpression requestExpression; | ||
|
|
||
| private UpdateExpressionResolver(Builder builder) { | ||
| this.tableMetadata = builder.tableMetadata; | ||
| this.nonKeyAttributes = builder.nonKeyAttributes; | ||
| this.extensionExpression = builder.extensionExpression; | ||
| this.requestExpression = builder.requestExpression; | ||
| } | ||
|
|
||
| public static Builder builder() { | ||
| return new Builder(); | ||
| } | ||
|
|
||
| /** | ||
| * Merges UpdateExpressions from three sources with priority: item attributes (lowest), extension expressions (medium), | ||
| * request expressions (highest). | ||
| * | ||
| * <p><b>Steps:</b> Identify attributes used by extensions/requests to prevent REMOVE conflicts → | ||
| * create item SET/REMOVE actions → merge extensions (override item) → merge request (override all). | ||
|
There was a problem hiding this comment. This is not happening, this concatenates and DynamoDB will throw a conflict error, see
Contributor
Author
There was a problem hiding this comment. The referenced test is no longer present because the implementation was changed to add an opt-in flag (updateExpressionMergeStrategy) in order to preserve backward compatibility. Default behavior remains LEGACY: actions are concatenated and overlapping paths may still fail in DynamoDB (Two document paths overlap). Conflict resolution is applied only when users opt into PRIORITIZE_HIGHER_SOURCE (request > extension > POJO). The javadoc was updated and additional details are captured here. |
||
| * | ||
| * <p><b>Backward compatibility:</b> Without request expressions, behavior is identical to previous versions. | ||
| * <p><b>Exceptions:</b> DynamoDbException may be thrown when the same attribute is updated by multiple sources. | ||
| * | ||
| * @return merged UpdateExpression, or empty if no updates needed | ||
| */ | ||
| public UpdateExpression resolve() { | ||
| UpdateExpression itemExpression = null; | ||
|
|
||
| if (!nonKeyAttributes.isEmpty()) { | ||
| Set<String> attributesExcludedFromRemoval = attributesPresentInOtherExpressions( | ||
| Arrays.asList(extensionExpression, requestExpression)); | ||
|
|
||
| itemExpression = UpdateExpression.mergeExpressions( | ||
| generateItemSetExpression(nonKeyAttributes, tableMetadata), | ||
| generateItemRemoveExpression(nonKeyAttributes, attributesExcludedFromRemoval)); | ||
| } | ||
|
|
||
| return Stream.of(itemExpression, extensionExpression, requestExpression) | ||
| .filter(Objects::nonNull) | ||
| .reduce(UpdateExpression::mergeExpressions) | ||
| .orElse(null); | ||
| } | ||
|
|
||
| private static Set<String> attributesPresentInOtherExpressions(Collection<UpdateExpression> updateExpressions) { | ||
| return updateExpressions.stream() | ||
| .filter(Objects::nonNull) | ||
| .map(UpdateExpressionConverter::findAttributeNames) | ||
| .flatMap(List::stream) | ||
| .collect(Collectors.toSet()); | ||
| } | ||
|
|
||
| public static UpdateExpression generateItemSetExpression(Map<String, AttributeValue> itemMap, | ||
|
Contributor
Author
There was a problem hiding this comment. The method was moved to UpdateExpressionUtils to centralize shared update-expression construction logic and keep the resolver focused on merge behavior. This was also made package-private, since it's only used by UpdateExpressionResolver and tests in the same package. There was a problem hiding this comment. Should this be package private as it is used only by unit tests?
Contributor
Author
There was a problem hiding this comment. The two methods were moved to UpdateExpressionUtils to centralize shared update-expression construction logic and keep the resolver focused on merge behavior. Thay were also made package-private, since they are just used by UpdateExpressionResolver and tests in the same package. |
||
| TableMetadata tableMetadata) { | ||
|
|
||
| Map<String, AttributeValue> setAttributes = filterMap(itemMap, e -> !isNullAttributeValue(e.getValue())); | ||
| return UpdateExpression.builder() | ||
| .actions(setActionsFor(setAttributes, tableMetadata)) | ||
| .build(); | ||
| } | ||
|
|
||
| public static UpdateExpression generateItemRemoveExpression(Map<String, AttributeValue> itemMap, | ||
|
Contributor
Author
There was a problem hiding this comment. The method was moved to UpdateExpressionUtils to centralize shared update-expression construction logic and keep the resolver focused on merge behavior. This was also made package-private, since it's only used by UpdateExpressionResolver and tests in the same package. |
||
| Collection<String> nonRemoveAttributes) { | ||
| Map<String, AttributeValue> removeAttributes = | ||
| filterMap(itemMap, e -> isNullAttributeValue(e.getValue()) && !nonRemoveAttributes.contains(e.getKey())); | ||
|
|
||
| return UpdateExpression.builder() | ||
| .actions(removeActionsFor(removeAttributes)) | ||
| .build(); | ||
| } | ||
|
|
||
| public static final class Builder { | ||
|
|
||
| private TableMetadata tableMetadata; | ||
| private Map<String, AttributeValue> nonKeyAttributes; | ||
| private UpdateExpression extensionExpression; | ||
| private UpdateExpression requestExpression; | ||
|
|
||
| public Builder tableMetadata(TableMetadata tableMetadata) { | ||
| this.tableMetadata = requireNonNull( | ||
| tableMetadata, "A TableMetadata is required when generating an Update Expression"); | ||
| return this; | ||
| } | ||
|
|
||
| public Builder nonKeyAttributes(Map<String, AttributeValue> nonKeyAttributes) { | ||
| if (nonKeyAttributes == null) { | ||
| this.nonKeyAttributes = Collections.emptyMap(); | ||
| } else { | ||
| this.nonKeyAttributes = Collections.unmodifiableMap(new HashMap<>(nonKeyAttributes)); | ||
| } | ||
| return this; | ||
| } | ||
|
|
||
| public Builder extensionExpression(UpdateExpression extensionExpression) { | ||
| this.extensionExpression = extensionExpression; | ||
| return this; | ||
| } | ||
|
|
||
| public Builder requestExpression(UpdateExpression requestExpression) { | ||
| this.requestExpression = requestExpression; | ||
| return this; | ||
| } | ||
|
|
||
| public UpdateExpressionResolver build() { | ||
|
Contributor
Author
There was a problem hiding this comment. Javadoc was added. |
||
| return new UpdateExpressionResolver(this); | ||
|
There was a problem hiding this comment. If nonKeyAttributes() is never called on the builder, this.nonKeyAttributes remains null. The resolve() method calls nonKeyAttributes.isEmpty() which will throw NullPointerException.
Contributor
Author
There was a problem hiding this comment. Null handling was added in Builder.nonKeyAttributes(...) to set Collections.emptyMap() when null is passed, so resolve() wil not throw NullPointerException: public Builder nonKeyAttributes(Map<String, AttributeValue> nonKeyAttributes) {
if (nonKeyAttributes == null) {
this.nonKeyAttributes = Collections.emptyMap();
} else {
this.nonKeyAttributes = Collections.unmodifiableMap(new HashMap<>(nonKeyAttributes));
}
return this;
} |
||
| } | ||
|
There was a problem hiding this comment. tableMetadata has no default value, so if the caller never calls .tableMetadata(...), the field is null. resolve() passes tableMetadata to generateItemSetExpression, which passes it to UpdateBehaviorTag.resolveForAttribute, which will NPE. The requireNonNull guard is only on the setter, it doesn't protect against the setter never being called. So, it may cause a NPE at runtime with a confusing stack trace instead of a clear IllegalStateException at build time. You can add validation in build(): public UpdateExpressionResolver build() {
Validate.paramNotNull(tableMetadata, "tableMetadata");
return new UpdateExpressionResolver(this);
}
Contributor
Author
There was a problem hiding this comment. Added validation in UpdateExpressionResolver: public UpdateExpressionResolver build() {
if (!nonKeyAttributes.isEmpty()) {
Validate.paramNotNull(tableMetadata, "tableMetadata");
}
return new UpdateExpressionResolver(this);
}and tests in UpdateExpressionResolverTest that cover this scenario:
|
||
|
|
||
| } | ||
| } | ||
|
There was a problem hiding this comment. We don't have new line at the end
Contributor
Author
There was a problem hiding this comment. Added new line. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,11 +15,9 @@ | |
|
|
||
| package software.amazon.awssdk.enhanced.dynamodb.internal.update; | ||
|
|
||
| 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 java.util.Arrays; | ||
| import java.util.Collections; | ||
|
|
@@ -35,7 +33,6 @@ | |
| import software.amazon.awssdk.enhanced.dynamodb.mapper.UpdateBehavior; | ||
| import software.amazon.awssdk.enhanced.dynamodb.update.RemoveAction; | ||
| import software.amazon.awssdk.enhanced.dynamodb.update.SetAction; | ||
| import software.amazon.awssdk.services.dynamodb.model.AttributeValue; | ||
|
|
||
| @SdkInternalApi | ||
|
|
@@ -53,32 +50,10 @@ public static String ifNotExists(String key, String initValue) { | |
| return "if_not_exists(" + keyRef(key) + ", " + valueRef(initValue) + ")"; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a list of SET actions for all attributes supplied in the map. | ||
| */ | ||
| static List<SetAction> setActionsFor(Map<String, AttributeValue> attributesToSet, TableMetadata tableMetadata) { | ||
| return attributesToSet.entrySet() | ||
| .stream() | ||
| .map(entry -> setValue(entry.getKey(), | ||
|
|
@@ -90,7 +65,7 @@ private static List<SetAction> setActionsFor(Map<String, AttributeValue> attribu | |
| /** | ||
| * Creates a list of REMOVE actions for all attributes supplied in the map. | ||
| */ | ||
| static List<RemoveAction> removeActionsFor(Map<String, AttributeValue> attributesToSet) { | ||
| return attributesToSet.entrySet() | ||
| .stream() | ||
| .map(entry -> remove(entry.getKey())) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to clarify the javadoc here, If I understand this right,
Given a POJO with: item.setCounter(100L)
And a request UpdateExpression: SET counter = counter + :inc
The resolver produces BOTH actions (concat, not override):
SET #AMZN_MAPPED_counter = :AMZN_MAPPED_counter, counter = counter + :inc
DynamoDB rejects this with: "Two document paths overlap with each other"
The only conflict that IS auto-resolved: if item.setCounter(null), the POJO would generate REMOVE counter, but the resolver suppresses that REMOVE because counter appears in the request expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation and javadoc were updated and details related to the new approach are captured here.