diff --git a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/extensions/VersionedRecordExtension.java b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/extensions/VersionedRecordExtension.java index 5f5cdb02c354..e9e4b85eab54 100644 --- a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/extensions/VersionedRecordExtension.java +++ b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/extensions/VersionedRecordExtension.java @@ -158,7 +158,7 @@ public WriteModification beforeWrite(DynamoDbExtensionContext.BeforeWrite contex .orElse(this.incrementBy); - if (isInitialVersion(existingVersionValue, versionStartAtFromAnnotation)) { + if (existingVersionValue == null || isNullAttributeValue(existingVersionValue)) { newVersionValue = AttributeValue.builder() .n(Long.toString(versionStartAtFromAnnotation + versionIncrementByFromAnnotation)) .build(); @@ -175,7 +175,6 @@ public WriteModification beforeWrite(DynamoDbExtensionContext.BeforeWrite contex long existingVersion = Long.parseLong(existingVersionValue.n()); String existingVersionValueKey = VERSIONED_RECORD_EXPRESSION_VALUE_KEY_MAPPER.apply(versionAttributeKey.get()); - long increment = versionIncrementByFromAnnotation; /* @@ -190,12 +189,25 @@ public WriteModification beforeWrite(DynamoDbExtensionContext.BeforeWrite contex newVersionValue = AttributeValue.builder().n(Long.toString(existingVersion + increment)).build(); - condition = Expression.builder() - .expression(String.format("%s = %s", attributeKeyRef, existingVersionValueKey)) - .expressionNames(Collections.singletonMap(attributeKeyRef, versionAttributeKey.get())) - .expressionValues(Collections.singletonMap(existingVersionValueKey, - existingVersionValue)) - .build(); + // When version equals startAt, we can't distinguish between new and existing records + // Use OR condition to handle both cases + if (existingVersion == versionStartAtFromAnnotation) { + condition = Expression.builder() + .expression(String.format("attribute_not_exists(%s) OR %s = %s", + attributeKeyRef, attributeKeyRef, existingVersionValueKey)) + .expressionNames(Collections.singletonMap(attributeKeyRef, versionAttributeKey.get())) + .expressionValues(Collections.singletonMap(existingVersionValueKey, + existingVersionValue)) + .build(); + } else { + // Normal case - version doesn't equal startAt, must be existing record + condition = Expression.builder() + .expression(String.format("%s = %s", attributeKeyRef, existingVersionValueKey)) + .expressionNames(Collections.singletonMap(attributeKeyRef, versionAttributeKey.get())) + .expressionValues(Collections.singletonMap(existingVersionValueKey, + existingVersionValue)) + .build(); + } } itemToTransform.put(versionAttributeKey.get(), newVersionValue); @@ -206,21 +218,6 @@ public WriteModification beforeWrite(DynamoDbExtensionContext.BeforeWrite contex .build(); } - private boolean isInitialVersion(AttributeValue existingVersionValue, Long versionStartAtFromAnnotation) { - if (existingVersionValue == null || isNullAttributeValue(existingVersionValue)) { - return true; - } - - if (existingVersionValue.n() != null) { - long currentVersion = Long.parseLong(existingVersionValue.n()); - // If annotation value is present, use it, otherwise fall back to the extension's value - Long effectiveStartAt = versionStartAtFromAnnotation != null ? versionStartAtFromAnnotation : this.startAt; - return currentVersion == effectiveStartAt; - } - - return false; - } - @NotThreadSafe public static final class Builder { private Long startAt; diff --git a/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/extensions/VersionedRecordExtensionTest.java b/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/extensions/VersionedRecordExtensionTest.java index 4feb23a43943..30d3dd796693 100644 --- a/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/extensions/VersionedRecordExtensionTest.java +++ b/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/extensions/VersionedRecordExtensionTest.java @@ -212,7 +212,7 @@ public void beforeWrite_versionEqualsStartAt_treatedAsInitialVersion() { .operationContext(PRIMARY_CONTEXT).build()); assertThat(result.additionalConditionalExpression().expression(), - is("attribute_not_exists(#AMZN_MAPPED_version)")); + is("attribute_not_exists(#AMZN_MAPPED_version) OR #AMZN_MAPPED_version = :old_version_value")); } @ParameterizedTest @@ -321,7 +321,7 @@ public void beforeWrite_versionEqualsAnnotationStartAt_isTreatedAsInitialVersion .operationContext(PRIMARY_CONTEXT).build()); assertThat(result.additionalConditionalExpression().expression(), - is("attribute_not_exists(#AMZN_MAPPED_version)")); + is("attribute_not_exists(#AMZN_MAPPED_version) OR #AMZN_MAPPED_version = :old_version_value")); } @@ -634,6 +634,26 @@ public void isInitialVersion_shouldPrioritizeAnnotationValueOverBuilderValue() { is("#AMZN_MAPPED_version = :old_version_value")); } + @Test + public void updateItem_existingRecordWithVersionEqualToStartAt_shouldSucceed() { + VersionedRecordExtension recordExtension = VersionedRecordExtension.builder().build(); + FakeItem item = createUniqueFakeItem(); + item.setVersion(0); + + Map inputMap = new HashMap<>(FakeItem.getTableSchema().itemToMap(item, true)); + + WriteModification result = + recordExtension.beforeWrite(DefaultDynamoDbExtensionContext + .builder() + .items(inputMap) + .tableMetadata(FakeItem.getTableMetadata()) + .operationContext(PRIMARY_CONTEXT).build()); + + assertThat(result.additionalConditionalExpression().expression(), + is("attribute_not_exists(#AMZN_MAPPED_version) OR #AMZN_MAPPED_version = :old_version_value")); + } + + public static Stream customIncrementForExistingVersionValues() { return Stream.of( Arguments.of(0L, 1L, 5L, "6"), diff --git a/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/VersionedRecordTest.java b/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/VersionedRecordTest.java index 32cbfd5332a4..6d3d0abcd670 100644 --- a/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/VersionedRecordTest.java +++ b/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/VersionedRecordTest.java @@ -17,10 +17,13 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static software.amazon.awssdk.enhanced.dynamodb.extensions.VersionedRecordExtension.AttributeTags.versionAttribute; import static software.amazon.awssdk.enhanced.dynamodb.internal.AttributeValues.stringValue; import static software.amazon.awssdk.enhanced.dynamodb.mapper.StaticAttributeTags.primaryPartitionKey; +import java.util.HashMap; +import java.util.Map; import java.util.Objects; import org.junit.After; import org.junit.Before; @@ -38,6 +41,7 @@ import software.amazon.awssdk.enhanced.dynamodb.mapper.annotations.DynamoDbPartitionKey; import software.amazon.awssdk.enhanced.dynamodb.model.PutItemEnhancedRequest; import software.amazon.awssdk.enhanced.dynamodb.model.UpdateItemEnhancedRequest; +import software.amazon.awssdk.services.dynamodb.model.AttributeValue; import software.amazon.awssdk.services.dynamodb.model.ConditionalCheckFailedException; import software.amazon.awssdk.services.dynamodb.model.DeleteTableRequest; @@ -407,4 +411,173 @@ public void annotationBasedCustomVersioningWorks() { assertThat(updated.getVersion(), is(11L)); } + + @Test + public void updateItem_existingRecordWithVersionZero_defaultStartAt_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("version-zero")); + item.put("version", AttributeValue.builder().n("0").build()); + + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("table-name")).item(item)); + + Record retrieved = mappedTable.getItem(r -> r.key(k -> k.partitionValue("version-zero"))); + assertThat(retrieved.getVersion(), is(0)); + + Record updated = mappedTable.updateItem(retrieved); + assertThat(updated.getVersion(), is(1)); + } + + @Test + public void updateItem_existingRecordWithVersionEqualToBuilderStartAt_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("version-ten")); + item.put("version", AttributeValue.builder().n("10").build()); + + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("table-name2")).item(item)); + + Record retrieved = mappedCustomVersionedTable.getItem(r -> r.key(k -> k.partitionValue("version-ten"))); + assertThat(retrieved.getVersion(), is(10)); + + Record updated = mappedCustomVersionedTable.updateItem(retrieved); + assertThat(updated.getVersion(), is(12)); + } + + @Test + public void updateItem_existingRecordWithVersionEqualToAnnotationStartAt_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("version-five")); + item.put("version", AttributeValue.builder().n("5").build()); + + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("annotated-table")).item(item)); + + AnnotatedRecord retrieved = annotatedTable.getItem(r -> r.key(k -> k.partitionValue("version-five"))); + assertThat(retrieved.getVersion(), is(5L)); + + AnnotatedRecord updated = annotatedTable.updateItem(retrieved); + assertThat(updated.getVersion(), is(8L)); + } + + @Test + public void putItem_existingRecordWithVersionEqualToStartAt_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("put-version-ten")); + item.put("version", AttributeValue.builder().n("10").build()); + + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("table-name2")).item(item)); + + Record overwrite = new Record().setId("put-version-ten").setVersion(10); + mappedCustomVersionedTable.putItem(overwrite); + + Record retrieved = mappedCustomVersionedTable.getItem(r -> r.key(k -> k.partitionValue("put-version-ten"))); + assertThat(retrieved.getVersion(), is(12)); + } + + @Test + public void putItem_newRecordWithVersionEqualToStartAt_shouldSucceed() { + Record newRecord = new Record().setId("explicit-zero").setAttribute("test").setVersion(0); + mappedTable.putItem(newRecord); + + Record retrieved = mappedTable.getItem(r -> r.key(k -> k.partitionValue("explicit-zero"))); + assertThat(retrieved.getVersion(), is(1)); + } + + @Test + public void sdkV1MigrationFlow_createRetrieveUpdate_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("v1-record")); + item.put("attribute", stringValue("initial")); + item.put("version", AttributeValue.builder().n("0").build()); + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("table-name")).item(item)); + + Record retrieved = mappedTable.getItem(r -> r.key(k -> k.partitionValue("v1-record"))); + assertThat(retrieved.getVersion(), is(0)); + + retrieved.setAttribute("updated"); + Record updated = mappedTable.updateItem(retrieved); + assertThat(updated.getVersion(), is(1)); + assertThat(updated.getAttribute(), is("updated")); + } + + @Test + public void deleteItem_existingRecordWithVersionZero_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("delete-test")); + item.put("attribute", stringValue("test")); + item.put("version", AttributeValue.builder().n("0").build()); + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("table-name")).item(item)); + + Record toDelete = mappedTable.getItem(r -> r.key(k -> k.partitionValue("delete-test"))); + assertThat(toDelete.getVersion(), is(0)); + + mappedTable.deleteItem(toDelete); + + Record shouldBeNull = mappedTable.getItem(r -> r.key(k -> k.partitionValue("delete-test"))); + assertThat(shouldBeNull, is(nullValue())); + } + + @Test + public void updateItem_bothBuilderAndAnnotationWithVersionEqualToStartAt_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("both-config")); + item.put("attribute", stringValue("initial")); + item.put("version", AttributeValue.builder().n("5").build()); + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("annotated-table")).item(item)); + + AnnotatedRecord retrieved = annotatedTable.getItem(r -> r.key(k -> k.partitionValue("both-config"))); + assertThat(retrieved.getVersion(), is(5L)); + + retrieved.setAttribute("updated"); + AnnotatedRecord updated = annotatedTable.updateItem(retrieved); + assertThat(updated.getVersion(), is(8L)); + } + + @Test + public void putItem_annotationConfigWithVersionEqualToStartAt_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("annotation-put")); + item.put("attribute", stringValue("initial")); + item.put("version", AttributeValue.builder().n("5").build()); + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("annotated-table")).item(item)); + + AnnotatedRecord overwrite = new AnnotatedRecord().setId("annotation-put").setAttribute("overwritten").setVersion(5L); + annotatedTable.putItem(overwrite); + + AnnotatedRecord retrieved = annotatedTable.getItem(r -> r.key(k -> k.partitionValue("annotation-put"))); + assertThat(retrieved.getAttribute(), is("overwritten")); + assertThat(retrieved.getVersion(), is(8L)); + } + + @Test + public void deleteItem_builderConfigWithVersionEqualToStartAt_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("delete-builder")); + item.put("attribute", stringValue("test")); + item.put("version", AttributeValue.builder().n("10").build()); + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("table-name2")).item(item)); + + Record toDelete = mappedCustomVersionedTable.getItem(r -> r.key(k -> k.partitionValue("delete-builder"))); + assertThat(toDelete.getVersion(), is(10)); + + mappedCustomVersionedTable.deleteItem(toDelete); + + Record shouldBeNull = mappedCustomVersionedTable.getItem(r -> r.key(k -> k.partitionValue("delete-builder"))); + assertThat(shouldBeNull, is(nullValue())); + } + + @Test + public void deleteItem_annotationConfigWithVersionEqualToStartAt_shouldSucceed() { + Map item = new HashMap<>(); + item.put("id", stringValue("delete-annotation")); + item.put("attribute", stringValue("test")); + item.put("version", AttributeValue.builder().n("5").build()); + getDynamoDbClient().putItem(r -> r.tableName(getConcreteTableName("annotated-table")).item(item)); + + AnnotatedRecord toDelete = annotatedTable.getItem(r -> r.key(k -> k.partitionValue("delete-annotation"))); + assertThat(toDelete.getVersion(), is(5L)); + + annotatedTable.deleteItem(toDelete); + + AnnotatedRecord shouldBeNull = annotatedTable.getItem(r -> r.key(k -> k.partitionValue("delete-annotation"))); + assertThat(shouldBeNull, is(nullValue())); + } }