Optimistic Locking for Delete Operations #6747
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -241,10 +241,22 @@ default T deleteItem(Key key) { | |
| * delete from the database table. | ||
| * @return The item that was persisted in the database before it was deleted. | ||
| */ | ||
| @Deprecated | ||
| default T deleteItem(T keyItem) { | ||
|
There was a problem hiding this comment. The deleteItem(T keyItem) method is marked @deprecated in the implementation classes (DefaultDynamoDbTable, DefaultDynamoDbAsyncTable) but not here (same for async version)
Contributor
Author
There was a problem hiding this comment. Marked the method as @deprecated for both Sync and Async versions. |
||
| throw new UnsupportedOperationException(); | ||
| } | ||
|
|
||
| /** | ||
| * Deletes an item from the table with optional optimistic locking. | ||
| * | ||
| * @param keyItem the item containing the key to delete | ||
| * @param useOptimisticLocking if true, applies optimistic locking if the item has version information | ||
| * @return the deleted item, or null if the item was not found | ||
| */ | ||
| default T deleteItem(T keyItem, boolean useOptimisticLocking) { | ||
|
There was a problem hiding this comment. Do we want to add JavaDoc for this ?
Contributor
Author
There was a problem hiding this comment. Added JavaDoc for this method. |
||
| throw new UnsupportedOperationException(); | ||
|
There was a problem hiding this comment. Also, the pr description and test checklist show batchWriteItem as covered, but there's no code change to WriteBatch.Builder.addDeleteItem() or any batch-related class ? do we support it or not ?
Contributor
Author
There was a problem hiding this comment. Batch operations do not support condition expressions, so batchWriteItem is not applicable here. I’ve also removed it from the test coverage checklist in the PR. The DynamoDB documentation confirms this: It states that you cannot specify conditions on individual put and delete requests within a batch - “you cannot specify conditions on individual put and delete requests” in a BatchWriteItem request. |
||
| } | ||
|
|
||
| /** | ||
| * Deletes a single item from the mapped table using a supplied primary {@link Key}. | ||
| * <p> | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| /* | ||
| * 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; | ||
|
|
||
| import static software.amazon.awssdk.enhanced.dynamodb.internal.EnhancedClientUtils.keyRef; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.Optional; | ||
| import software.amazon.awssdk.annotations.SdkInternalApi; | ||
| import software.amazon.awssdk.enhanced.dynamodb.Expression; | ||
| import software.amazon.awssdk.enhanced.dynamodb.TableSchema; | ||
| import software.amazon.awssdk.enhanced.dynamodb.model.DeleteItemEnhancedRequest; | ||
| import software.amazon.awssdk.enhanced.dynamodb.model.TransactDeleteItemEnhancedRequest; | ||
| import software.amazon.awssdk.services.dynamodb.model.AttributeValue; | ||
|
|
||
| /** | ||
| * Utility class for adding optimistic locking to DynamoDB delete operations. | ||
| * <p> | ||
| * Optimistic locking prevents concurrent modifications by checking that an item's version hasn't changed since it was last read. | ||
| * If the version has changed, the delete operation fails with a {@code ConditionalCheckFailedException}. | ||
| */ | ||
| @SdkInternalApi | ||
| public final class OptimisticLockingHelper { | ||
|
|
||
| private OptimisticLockingHelper() { | ||
| } | ||
|
|
||
| /** | ||
| * Adds optimistic locking to a delete request. | ||
| * | ||
| * @param request the original delete request | ||
| * @param versionValue the expected version value | ||
| * @param versionAttributeName the version attribute name | ||
| * @return delete request with optimistic locking condition | ||
| */ | ||
| public static DeleteItemEnhancedRequest withOptimisticLocking( | ||
| DeleteItemEnhancedRequest request, AttributeValue versionValue, String versionAttributeName) { | ||
|
|
||
| Expression conditionExpression = createVersionCondition(versionValue, versionAttributeName); | ||
| return request.toBuilder() | ||
| .conditionExpression(conditionExpression) | ||
| .build(); | ||
| } | ||
|
|
||
| /** | ||
| * Adds optimistic locking to a transactional delete request. | ||
| * | ||
| * @param request the original transactional delete request | ||
| * @param versionValue the expected version value | ||
| * @param versionAttributeName the version attribute name | ||
| * @return transactional delete request with optimistic locking condition | ||
| */ | ||
| public static TransactDeleteItemEnhancedRequest withOptimisticLocking( | ||
| TransactDeleteItemEnhancedRequest request, AttributeValue versionValue, String versionAttributeName) { | ||
|
|
||
| Expression conditionExpression = createVersionCondition(versionValue, versionAttributeName); | ||
| return request.toBuilder() | ||
| .conditionExpression(conditionExpression) | ||
| .build(); | ||
| } | ||
|
|
||
| /** | ||
| * Conditionally applies optimistic locking if enabled and version information exists. | ||
| * | ||
| * @param <T> the type of the item | ||
| * @param request the original delete request | ||
| * @param keyItem the item containing version information | ||
| * @param tableSchema the table schema | ||
| * @param useOptimisticLocking if true, applies optimistic locking | ||
| * @return delete request with optimistic locking if enabled and version exists, otherwise original request | ||
| */ | ||
| public static <T> DeleteItemEnhancedRequest conditionallyApplyOptimisticLocking( | ||
| DeleteItemEnhancedRequest request, T keyItem, TableSchema<T> tableSchema, boolean useOptimisticLocking) { | ||
|
|
||
| if (!useOptimisticLocking) { | ||
| return request; | ||
| } | ||
|
|
||
| return getVersionAttributeName(tableSchema) | ||
| .map(versionAttributeName -> { | ||
| AttributeValue version = tableSchema.attributeValue(keyItem, versionAttributeName); | ||
| return version != null ? withOptimisticLocking(request, version, versionAttributeName) : request; | ||
| }) | ||
| .orElse(request); | ||
| } | ||
|
|
||
| /** | ||
| * Conditionally applies optimistic locking if enabled and version information exists. | ||
| * | ||
| * @param <T> the type of the item | ||
| * @param request the original transactional delete request | ||
| * @param keyItem the item containing version information | ||
| * @param tableSchema the table schema | ||
| * @param useOptimisticLocking if true, applies optimistic locking | ||
| * @return delete request with optimistic locking if enabled and version exists, otherwise original request | ||
| */ | ||
| public static <T> TransactDeleteItemEnhancedRequest conditionallyApplyOptimisticLocking( | ||
| TransactDeleteItemEnhancedRequest request, T keyItem, TableSchema<T> tableSchema, boolean useOptimisticLocking) { | ||
|
|
||
| if (!useOptimisticLocking) { | ||
| return request; | ||
| } | ||
|
|
||
| return getVersionAttributeName(tableSchema) | ||
| .map(versionAttributeName -> { | ||
| AttributeValue version = tableSchema.attributeValue(keyItem, versionAttributeName); | ||
| return version != null ? withOptimisticLocking(request, version, versionAttributeName) : request; | ||
| }) | ||
| .orElse(request); | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Creates a version condition expression. | ||
| * | ||
| * @param versionValue the expected version value | ||
| * @param versionAttributeName the version attribute name | ||
| * @return version check condition expression | ||
| * @throws IllegalArgumentException if {@code versionAttributeName} is null or empty | ||
| */ | ||
| public static Expression createVersionCondition(AttributeValue versionValue, String versionAttributeName) { | ||
| if (versionAttributeName == null || versionAttributeName.trim().isEmpty()) { | ||
|
There was a problem hiding this comment. Why are we checking the
Contributor
Author
There was a problem hiding this comment. Added a check for "versionValue" also: if (versionAttributeName == null || versionAttributeName.trim().isEmpty()) {
throw new IllegalArgumentException("Version attribute name must not be null or empty.");
}
if (versionValue == null || versionValue.n() == null || versionValue.n().trim().isEmpty()) {
throw new IllegalArgumentException("Version value must not be null or empty.");
} |
||
| throw new IllegalArgumentException("Version attribute name must not be null or empty."); | ||
| } | ||
|
|
||
| String attributeKeyRef = keyRef(versionAttributeName); | ||
|
|
||
| return Expression.builder() | ||
| .expression(String.format("%s = :version_value", attributeKeyRef)) | ||
| .putExpressionValue(":version_value", versionValue) | ||
|
There was a problem hiding this comment. I think we should use
Contributor
Author
There was a problem hiding this comment. I have done the changes in order to use "keyRef" and "valueRef": String attributeKeyRef = keyRef(versionAttributeName);
String attributeValueRef = valueRef(versionAttributeName);
return Expression.builder()
.expression(String.format("%s = %s", attributeKeyRef, attributeValueRef))
.expressionNames(Collections.singletonMap(attributeKeyRef, versionAttributeName))
.expressionValues(Collections.singletonMap(attributeValueRef, versionValue))
.build(); |
||
| .expressionNames(Collections.singletonMap(attributeKeyRef, versionAttributeName)) | ||
| .build(); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the version attribute name from table schema. | ||
| * | ||
| * @param <T> the type of the item | ||
| * @param tableSchema the table schema | ||
| * @return version attribute name if present, empty otherwise | ||
| */ | ||
| public static <T> Optional<String> getVersionAttributeName(TableSchema<T> tableSchema) { | ||
| return tableSchema.tableMetadata().customMetadataObject("VersionedRecordExtension:VersionAttribute", String.class); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
missing Javadoc ?
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.
Added JavaDoc for this method.