Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,10 @@ default CompletableFuture<T> deleteItem(T keyItem) {
throw new UnsupportedOperationException();
}

default CompletableFuture<T> deleteItem(T keyItem, boolean useOptimisticLocking) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

missing Javadoc ?

Copy link
Copy Markdown
Contributor Author

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.

throw new UnsupportedOperationException();
}

/**
* Deletes a single item from the mapped table using a supplied primary {@link Key}.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@ default T deleteItem(T keyItem) {
throw new UnsupportedOperationException();
}

default T deleteItem(T keyItem, boolean useOptimisticLocking) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we want to add JavaDoc for this ?

Copy link
Copy Markdown
Contributor Author

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.

throw new UnsupportedOperationException();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 ?

Copy link
Copy Markdown
Contributor Author

@anasatirbasa anasatirbasa Feb 25, 2026

Choose a reason for hiding this comment

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

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:
https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/BestPractices_ConditionalBatchUpdate.html

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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package software.amazon.awssdk.enhanced.dynamodb.internal.client;

import static software.amazon.awssdk.enhanced.dynamodb.internal.EnhancedClientUtils.createKeyFromItem;
import static software.amazon.awssdk.enhanced.dynamodb.model.OptimisticLockingHelper.conditionallyApplyOptimisticLocking;

import java.util.ArrayList;
import java.util.concurrent.CompletableFuture;
Expand Down Expand Up @@ -124,28 +125,55 @@ public CompletableFuture<Void> createTable() {
.build());
}

/**
* Supports optimistic locking via {@link software.amazon.awssdk.enhanced.dynamodb.model.OptimisticLockingHelper}.
*/
@Override
public CompletableFuture<T> deleteItem(DeleteItemEnhancedRequest request) {
TableOperation<T, ?, ?, DeleteItemEnhancedResponse<T>> operation = DeleteItemOperation.create(request);
return operation.executeOnPrimaryIndexAsync(tableSchema, tableName, extension, dynamoDbClient)
.thenApply(DeleteItemEnhancedResponse::attributes);
}

/**
* Supports optimistic locking via {@link software.amazon.awssdk.enhanced.dynamodb.model.OptimisticLockingHelper}.
*/
@Override
public CompletableFuture<T> deleteItem(Consumer<DeleteItemEnhancedRequest.Builder> requestConsumer) {
DeleteItemEnhancedRequest.Builder builder = DeleteItemEnhancedRequest.builder();
requestConsumer.accept(builder);
return deleteItem(builder.build());
}

/**
* Does not support optimistic locking. Use {@link #deleteItem(Object, boolean)} for optimistic locking support.
*/
@Override
public CompletableFuture<T> deleteItem(Key key) {
return deleteItem(r -> r.key(key));
}

/**
* @deprecated Use {@link #deleteItem(Object, boolean)} instead to explicitly control optimistic locking behavior.
*/
@Override
@Deprecated
public CompletableFuture<T> deleteItem(T keyItem) {
return deleteItem(keyFrom(keyItem));
return deleteItem(keyItem, false);
}

/**
* 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 a CompletableFuture containing the deleted item, or null if the item was not found
*/
@Override
public CompletableFuture<T> deleteItem(T keyItem, boolean useOptimisticLocking) {
DeleteItemEnhancedRequest request = DeleteItemEnhancedRequest.builder().key(keyFrom(keyItem)).build();
request = conditionallyApplyOptimisticLocking(request, keyItem, tableSchema, useOptimisticLocking);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we should pass the builder here as the conditionallyApplyOptimisticLocking may need to add (overwrite actually) the condition expression with the OptLocking condition. Builder objects work well in these cases where the building logic is complex and scattered.

Here the withOptimisticLocking methods in OptimisticLockingHelper won't need to rebuild the request.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have updated the method in order to pass the builder instead of the actual request.

return deleteItem(request);
}

@Override
Expand Down Expand Up @@ -311,7 +339,7 @@ public CompletableFuture<T> updateItem(T item) {
public Key keyFrom(T item) {
return createKeyFromItem(item, tableSchema, TableMetadata.primaryIndexName());
}


@Override
public CompletableFuture<Void> deleteTable() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package software.amazon.awssdk.enhanced.dynamodb.internal.client;

import static software.amazon.awssdk.enhanced.dynamodb.internal.EnhancedClientUtils.createKeyFromItem;
import static software.amazon.awssdk.enhanced.dynamodb.model.OptimisticLockingHelper.conditionallyApplyOptimisticLocking;

import java.util.ArrayList;
import java.util.function.Consumer;
Expand Down Expand Up @@ -126,27 +127,54 @@ public void createTable() {
.build());
}

/**
* Supports optimistic locking via {@link software.amazon.awssdk.enhanced.dynamodb.model.OptimisticLockingHelper}.
*/
@Override
public T deleteItem(DeleteItemEnhancedRequest request) {
TableOperation<T, ?, ?, DeleteItemEnhancedResponse<T>> operation = DeleteItemOperation.create(request);
return operation.executeOnPrimaryIndex(tableSchema, tableName, extension, dynamoDbClient).attributes();
}

/**
* Supports optimistic locking via {@link software.amazon.awssdk.enhanced.dynamodb.model.OptimisticLockingHelper}.
*/
@Override
public T deleteItem(Consumer<DeleteItemEnhancedRequest.Builder> requestConsumer) {
DeleteItemEnhancedRequest.Builder builder = DeleteItemEnhancedRequest.builder();
requestConsumer.accept(builder);
return deleteItem(builder.build());
}

/**
* Does not support optimistic locking. Use {@link #deleteItem(Object, boolean)} for optimistic locking support.
*/
@Override
public T deleteItem(Key key) {
return deleteItem(r -> r.key(key));
}

/**
* @deprecated Use {@link #deleteItem(Object, boolean)} instead to explicitly control optimistic locking behavior.
*/
@Override
@Deprecated
public T deleteItem(T keyItem) {
return deleteItem(keyFrom(keyItem));
return deleteItem(keyItem, false);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is more of a question, the old deleteItem(T keyItem) used to call deleteItem(keyFrom(keyItem)) which goes through deleteItem(Key) → deleteItem(Consumer) → deleteItem(DeleteItemEnhancedRequest). The new code delegates to deleteItem(keyItem, false) which builds a DeleteItemEnhancedRequest directly and calls deleteItem(DeleteItemEnhancedRequest). It is called with the false flag means no optimistic locking is applied, the code path is different. The old path went through the Consumer overload, the new path constructs the request directly. If any subclass overrides the Consumer-based overload (which is a default method on the interface, so unlikely but possible), this changes behavior.

Is this a concern ? how do we think about backward compatibility ? (same applies to async table)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right — this could introduce a potential backward compatibility issue.

Previously, deleteItem(T keyItem) delegated through the Consumer-based overload.
The new implementation calls deleteItem(keyItem, false) directly, which bypasses that path.

Although it is unlikely that users override the Consumer-based method (since it is a default interface method), it is technically possible. In such a case, behavior could change.

To avoid breaking existing behavior, I kept the old method unchanged and marked it as deprecated:

/**
 * @deprecated Use {@link #deleteItem(Object, boolean)} instead to explicitly
 * control optimistic locking behavior.
 */

This preserves backward compatibility while guiding users toward the new API.
The same approach was applied to the async table implementation.

}

/**
* 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
*/
@Override
public T deleteItem(T keyItem, boolean useOptimisticLocking) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should optimistic locking not follow the same experience as UpdateItem/PutItem, where the user passes the entire item? I believe its useful to keep this version, but also align the dev experience with existing API's

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since this is for backwards compatability, perhaps its cleaner to do this at the annotation level, rather than changing the API signature:

@DynamoDbVersionAttribute(useVersionOnDelete = true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would also prevent the "boolean trap".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have changed the implementation in order to use a boolean flag via @DynamoDbVersionAttribute instead of overloading the existing delete APIs.

DeleteItemEnhancedRequest request = DeleteItemEnhancedRequest.builder().key(keyFrom(keyItem)).build();
request = conditionallyApplyOptimisticLocking(request, keyItem, tableSchema, useOptimisticLocking);
return deleteItem(request);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

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

import static software.amazon.awssdk.enhanced.dynamodb.model.OptimisticLockingHelper.createVersionCondition;

import java.util.Objects;
import java.util.function.Consumer;
import software.amazon.awssdk.annotations.NotThreadSafe;
Expand All @@ -24,6 +26,7 @@
import software.amazon.awssdk.enhanced.dynamodb.DynamoDbTable;
import software.amazon.awssdk.enhanced.dynamodb.Expression;
import software.amazon.awssdk.enhanced.dynamodb.Key;
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
import software.amazon.awssdk.services.dynamodb.model.DeleteItemRequest;
import software.amazon.awssdk.services.dynamodb.model.PutItemRequest;
import software.amazon.awssdk.services.dynamodb.model.ReturnConsumedCapacity;
Expand Down Expand Up @@ -289,6 +292,22 @@ public Builder returnValuesOnConditionCheckFailure(String returnValuesOnConditio
return this;
}

/**
* Adds optimistic locking to this delete request.
* <p>
* This method applies a condition expression that ensures the delete operation only succeeds
* if the version attribute of the item matches the provided expected value.
*
* @param versionValue the expected version value that must match for the deletion to succeed
* @param versionAttributeName the name of the version attribute in the DynamoDB table
* @return a builder of this type with optimistic locking condition applied
* @throws IllegalArgumentException if any parameter is null
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

where does this thorw IllegalArgumentException ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The method createVersionCondition() throws an IllegalArgumentException if versionAttributeName is null or empty. This validation is mandatory in order to build a valid conditional expression.

/**
 * 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()) {
        throw new IllegalArgumentException("Version attribute name must not be null or empty.");
    }
    ...
}

I have removed the @throws IllegalArgumentException declaration from the caller method.

*/
public Builder withOptimisticLocking(AttributeValue versionValue, String versionAttributeName) {
Expression optimisticLockingCondition = createVersionCondition(versionValue, versionAttributeName);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it possible that withOptimisticLocking() silently overwrites any existing conditionExpression ?

should this use Use Expression.join() to merge with any existing condition instead ?

Copy link
Copy Markdown
Contributor Author

@anasatirbasa anasatirbasa Feb 25, 2026

Choose a reason for hiding this comment

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

I have updated the code in order to merge the optimistic locking condition with the other existing conditions, if available:

 /**
         * Adds optimistic locking to this delete request.
         * <p>
         * If a {@link #conditionExpression(Expression)} was already set, this will combine it with the optimistic locking
         * condition using {@code AND}. If either expression has conflicting name/value tokens, {@link Expression#join} will throw
         * {@link IllegalArgumentException}.
         *
         * @param versionValue         the expected version value that must match for the deletion to succeed
         * @param versionAttributeName the name of the version attribute in the DynamoDB table
         * @return a builder of this type with optimistic locking condition applied (and merged if needed)
         */
        public Builder withOptimisticLocking(AttributeValue versionValue, String versionAttributeName) {
            Expression optimisticLockingCondition = createVersionCondition(versionValue, versionAttributeName);
            this.conditionExpression = Expression.join(this.conditionExpression, optimisticLockingCondition, " AND ");
            return this;
        }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have updated the code in order to merge the optimistic locking condition with the existing ones:

public static DeleteItemEnhancedRequest optimisticLocking(
DeleteItemEnhancedRequest.Builder requestBuilder, 
AttributeValue versionValue, String versionAttributeName) {

        Expression mergedCondition = mergeConditions(
            requestBuilder.build().conditionExpression(),
            createVersionCondition(versionValue, versionAttributeName));

        return requestBuilder
            .conditionExpression(mergedCondition)
            .build();
}

public static Expression mergeConditions(Expression initialCondition, Expression optimisticLockingCondition) {
        return Expression.join(initialCondition, optimisticLockingCondition, " AND ");
}

Added tests in OptimisticLockingCrudTest / OptimisticLockingAsyncCrudTest:

    // 30. deleteItem(DeleteItemEnhancedRequest) with Optimistic Locking true + custom condition respected
    // -> deletes the record
    @Test
    public void deleteItemWithRequest_whenOptimisticLockingHelperMergesConditionAndCustomConditionRespected_deletesTheRecord() {
        VersionedRecordWithDeleteOptimisticLocking item =
            new VersionedRecordWithDeleteOptimisticLocking().setId("123").setSort(10).setStringAttribute("test");
        Key recordKey = Key.builder().partitionValue(item.getId()).sortValue(item.getSort()).build();

        versionedRecordWithDeleteLockingTable.putItem(item);
        VersionedRecordWithDeleteOptimisticLocking savedItem =
            versionedRecordWithDeleteLockingTable.getItem(r -> r.key(recordKey));

        Map<String, String> expressionNames = new HashMap<>();
        expressionNames.put("#stringAttribute", "stringAttribute");

        Map<String, AttributeValue> expressionValues = new HashMap<>();
        expressionValues.put(":value", AttributeValue.fromS("test"));

        Expression conditionExpression =
            Expression.builder()
                      .expression("#stringAttribute = :value")
                      .expressionNames(Collections.unmodifiableMap(expressionNames))
                      .expressionValues(Collections.unmodifiableMap(expressionValues))
                      .build();

        DeleteItemEnhancedRequest.Builder requestBuilder =
            DeleteItemEnhancedRequest.builder()
                                     .key(recordKey)
                                     .conditionExpression(conditionExpression);

        DeleteItemEnhancedRequest requestWithMergedConditions =
            optimisticLocking(requestBuilder,
                              AttributeValue.builder().n(savedItem.getVersion().toString()).build(),
                              "version");

        versionedRecordWithDeleteLockingTable.deleteItem(requestWithMergedConditions);

        VersionedRecordWithDeleteOptimisticLocking deletedItem =
            versionedRecordWithDeleteLockingTable.getItem(r -> r.key(recordKey));
        assertThat(deletedItem).isNull();
    }

    // 31. deleteItem(DeleteItemEnhancedRequest) with Optimistic Locking true + custom condition fails
    // -> does NOT delete the record
    @Test
    public void deleteItemWithRequest_whenOptimisticLockingHelperMergesConditionAndCustomConditionFails_doesNotDeleteTheRecord() {
        VersionedRecordWithDeleteOptimisticLocking item =
            new VersionedRecordWithDeleteOptimisticLocking().setId("123").setSort(10).setStringAttribute("test");
        Key recordKey = Key.builder().partitionValue(item.getId()).sortValue(item.getSort()).build();

        versionedRecordWithDeleteLockingTable.putItem(item);
        VersionedRecordWithDeleteOptimisticLocking savedItem =
            versionedRecordWithDeleteLockingTable.getItem(r -> r.key(recordKey));

        Map<String, String> expressionNames = new HashMap<>();
        expressionNames.put("#stringAttribute", "stringAttribute");

        Map<String, AttributeValue> expressionValues = new HashMap<>();
        expressionValues.put(":value", AttributeValue.fromS("nonMatchingValue"));

        Expression conditionExpression =
            Expression.builder()
                      .expression("#stringAttribute = :value")
                      .expressionNames(Collections.unmodifiableMap(expressionNames))
                      .expressionValues(Collections.unmodifiableMap(expressionValues))
                      .build();

        DeleteItemEnhancedRequest.Builder requestBuilder =
            DeleteItemEnhancedRequest.builder()
                                     .key(recordKey)
                                     .conditionExpression(conditionExpression);

        DeleteItemEnhancedRequest requestWithMergedConditions =
            optimisticLocking(requestBuilder,
                              AttributeValue.builder().n(savedItem.getVersion().toString()).build(),
                              "version");

        assertThatThrownBy(() -> versionedRecordWithDeleteLockingTable.deleteItem(requestWithMergedConditions))
            .isInstanceOf(ConditionalCheckFailedException.class)
            .satisfies(e -> assertThat(e.getMessage()).contains("The conditional request failed"));
    }

return conditionExpression(optimisticLockingCondition);
}

public DeleteItemEnhancedRequest build() {
return new DeleteItemEnhancedRequest(this);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
/*
* 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 java.util.Optional;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.enhanced.dynamodb.Expression;
import software.amazon.awssdk.enhanced.dynamodb.TableSchema;
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}.
*/
@SdkPublicApi
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is also more of a question. This class is annotated @SdkPublicApi but it lives in the model package and exposes internal implementation details like the VersionedRecordExtension:VersionAttribute metadata key string. So it brings a nessecity to have a strong backward-compatibility commitment. The conditionallyApplyOptimisticLocking methods are only called from DefaultDynamoDbTable and DefaultDynamoDbAsyncTable (internal classes). Is it possible to use @SdkInternalApi instead and move to the internal package ?

Copy link
Copy Markdown
Contributor Author

@anasatirbasa anasatirbasa Feb 25, 2026

Choose a reason for hiding this comment

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

To clarify its intended scope, I’ve moved the class to the internal package and annotated it with @SdkInternalApi.

This helps avoid accidental external dependencies on it and protects against backward compatibility issues.

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
*/
public static Expression createVersionCondition(AttributeValue versionValue, String versionAttributeName) {
return Expression.builder()
.expression(versionAttributeName + " = :version_value")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This uses the raw attribute name directly in the expression string. The existing VersionedRecordExtension uses keyRef() and expressionNames maps to safely handle DynamoDB reserved words. Should we do the same thing here ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have updated the code in order to use keyRef(), in the same way it is used in VersionedRecordExtension.

.putExpressionValue(":version_value", versionValue)
.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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

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

import static software.amazon.awssdk.enhanced.dynamodb.model.OptimisticLockingHelper.createVersionCondition;

import java.util.Objects;
import java.util.function.Consumer;
import software.amazon.awssdk.annotations.NotThreadSafe;
Expand All @@ -24,6 +26,7 @@
import software.amazon.awssdk.enhanced.dynamodb.DynamoDbEnhancedClient;
import software.amazon.awssdk.enhanced.dynamodb.Expression;
import software.amazon.awssdk.enhanced.dynamodb.Key;
import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
import software.amazon.awssdk.services.dynamodb.model.ReturnValuesOnConditionCheckFailure;

/**
Expand Down Expand Up @@ -215,6 +218,21 @@ public Builder returnValuesOnConditionCheckFailure(String returnValuesOnConditio
return this;
}

/**
* Adds optimistic locking to this transactional delete request.
* <p>
* This method applies a condition expression that ensures the delete operation only succeeds if the version attribute of
* the item matches the provided expected value. If the condition fails, the entire transaction will be cancelled.
*
* @param versionValue the expected version value that must match for the deletion to succeed
* @param versionAttributeName the name of the version attribute in the DynamoDB table
* @return a builder of this type with optimistic locking condition applied
* @throws IllegalArgumentException if any parameter is null
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

where does this throw IllegalArgumentException ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The method createVersionCondition() throws an IllegalArgumentException if versionAttributeName is null or empty. This validation is mandatory in order to build a valid conditional expression.

/**
 * 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()) {
        throw new IllegalArgumentException("Version attribute name must not be null or empty.");
    }
    ...
}

I have removed the @throws IllegalArgumentException declaration from the caller method.

*/
public Builder withOptimisticLocking(AttributeValue versionValue, String versionAttributeName) {
Expression optimisticLockingCondition = createVersionCondition(versionValue, versionAttributeName);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it possible that withOptimisticLocking() silently overwrites any existing conditionExpression ?

should this use Use Expression.join() to merge with any existing condition instead ?

Copy link
Copy Markdown
Contributor Author

@anasatirbasa anasatirbasa Feb 25, 2026

Choose a reason for hiding this comment

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

I have updated the code in order to merge the optimistic locking condition with the other existing conditions, if available:

 /**
         * Adds optimistic locking to this delete request.
         * <p>
         * If a {@link #conditionExpression(Expression)} was already set, this will combine it with the optimistic locking
         * condition using {@code AND}. If either expression has conflicting name/value tokens, {@link Expression#join} will throw
         * {@link IllegalArgumentException}.
         *
         * @param versionValue         the expected version value that must match for the deletion to succeed
         * @param versionAttributeName the name of the version attribute in the DynamoDB table
         * @return a builder of this type with optimistic locking condition applied (and merged if needed)
         */
        public Builder withOptimisticLocking(AttributeValue versionValue, String versionAttributeName) {
            Expression optimisticLockingCondition = createVersionCondition(versionValue, versionAttributeName);
            this.conditionExpression = Expression.join(this.conditionExpression, optimisticLockingCondition, " AND ");
            return this;
        }

return conditionExpression(optimisticLockingCondition);
}

public TransactDeleteItemEnhancedRequest build() {
return new TransactDeleteItemEnhancedRequest(this);
Expand Down
Loading