porting version from Integer to Long, removing refactoring

RanVaknin · RanVaknin · commit 3d505a6f14aa · 2025-04-21T16:42:09.000-07:00
diff --git a/pom.xml b/pom.xml
@@ -679,6 +679,8 @@
                             <includeModule>polly</includeModule>
                         </includeModules>
                         <excludes>
+                            <exclude>software.amazon.awssdk.enhanced.dynamodb.extensions.annotations.DynamoDbVersionAttribute#incrementBy()</exclude>
+                            <exclude>software.amazon.awssdk.enhanced.dynamodb.extensions.annotations.DynamoDbVersionAttribute#startAt()</exclude>
                             <exclude>*.internal.*</exclude>
                             <exclude>software.amazon.awssdk.thirdparty.*</exclude>
                             <exclude>software.amazon.awssdk.regions.*</exclude>
diff --git a/services-custom/dynamodb-enhanced/pom.xml b/services-custom/dynamodb-enhanced/pom.xml
@@ -76,20 +76,6 @@
                     </archive>
                 </configuration>
             </plugin>
-            <plugin>
-                <groupId>com.github.siom79.japicmp</groupId>
-                <artifactId>japicmp-maven-plugin</artifactId>
-                <version>${japicmp-maven-plugin.version}</version>
-                <configuration>
-                    <parameter>
-                        <excludes>
-                            <!-- japicmp flagging these incorrectly as a breaking change -->
-                            <exclude>software.amazon.awssdk.enhanced.dynamodb.extensions.annotations.DynamoDbVersionAttribute#incrementBy()</exclude>
-                            <exclude>software.amazon.awssdk.enhanced.dynamodb.extensions.annotations.DynamoDbVersionAttribute#startAt()</exclude>
-                        </excludes>
-                    </parameter>
-                </configuration>
-            </plugin>
         </plugins>
         <pluginManagement>
             <plugins>
diff --git a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/Expression.java b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/Expression.java
@@ -321,7 +321,7 @@ public String toString() {
     }
 
     /**
-     * A builder for {@link Expression}v
+     * A builder for {@link Expression}
      */
     @NotThreadSafe
     public static final class Builder {
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
@@ -34,7 +34,7 @@
 import software.amazon.awssdk.enhanced.dynamodb.mapper.StaticAttributeTag;
 import software.amazon.awssdk.enhanced.dynamodb.mapper.StaticTableMetadata;
 import software.amazon.awssdk.services.dynamodb.model.AttributeValue;
-import software.amazon.awssdk.utils.Pair;
+import software.amazon.awssdk.utils.Validate;
 
 /**
  * This extension implements optimistic locking on record writes by means of a 'record version number' that is used
@@ -61,20 +61,19 @@ public final class VersionedRecordExtension implements DynamoDbEnhancedClientExt
     private static final Function<String, String> VERSIONED_RECORD_EXPRESSION_VALUE_KEY_MAPPER = key -> ":old_" + key + "_value";
     private static final String CUSTOM_METADATA_KEY = "VersionedRecordExtension:VersionAttribute";
     private static final VersionAttribute VERSION_ATTRIBUTE = new VersionAttribute();
-    private static final AttributeValue DEFAULT_VALUE = AttributeValue.fromNul(Boolean.TRUE);
-
-    private final int startAt;
-    private final int incrementBy;
-
-    /**
-     * Creates a new {@link VersionedRecordExtension} using the supplied starting and incrementing value.
-     *
-     * @param startAt the value used to compare if a record is the initial version of a record.
-     * @param incrementBy the amount to increment the version by with each subsequent update.
-     */
-    private VersionedRecordExtension(int startAt, int incrementBy) {
-        this.startAt = startAt;
-        this.incrementBy = incrementBy;
+
+    private final long startAt;
+    private final long incrementBy;
+
+    private VersionedRecordExtension(Long startAt, Long incrementBy) {
+        Validate.isNotNegativeOrNull(startAt, "startAt");
+
+        if (incrementBy != null && incrementBy < 1) {
+            throw new IllegalArgumentException("IncrementBy must be greater than 0.");
+        }
+
+        this.startAt = startAt != null ? startAt : 0L;
+        this.incrementBy = incrementBy != null ? incrementBy : 1L;
     }
 
     public static Builder builder() {
@@ -89,7 +88,7 @@ public static StaticAttributeTag versionAttribute() {
             return VERSION_ATTRIBUTE;
         }
 
-        public static StaticAttributeTag versionAttribute(Integer startAt, Integer incrementBy) {
+        public static StaticAttributeTag versionAttribute(Long startAt, Long incrementBy) {
             return new VersionAttribute(startAt, incrementBy);
         }
     }
@@ -98,15 +97,15 @@ private static final class VersionAttribute implements StaticAttributeTag {
         private static final String START_AT_METADATA_KEY = "VersionedRecordExtension:StartAt";
         private static final String INCREMENT_BY_METADATA_KEY = "VersionedRecordExtension:IncrementBy";
 
-        private final Integer startAt;
-        private final Integer incrementBy;
+        private final Long startAt;
+        private final Long incrementBy;
 
         private VersionAttribute() {
             this.startAt = null;
             this.incrementBy = null;
         }
 
-        private VersionAttribute(Integer startAt, Integer incrementBy) {
+        private VersionAttribute(Long startAt, Long incrementBy) {
             this.startAt = startAt;
             this.incrementBy = incrementBy;
         }
@@ -120,16 +119,13 @@ public Consumer<StaticTableMetadata.Builder> modifyMetadata(String attributeName
                     "is supported.", attributeName, attributeValueType.name()));
             }
 
-            if (startAt != null && startAt < 0) {
-                throw new IllegalArgumentException("StartAt cannot be negative.");
-            }
+            Validate.isNotNegativeOrNull(startAt, "startAt");
 
             if (incrementBy != null && incrementBy < 1) {
                 throw new IllegalArgumentException("IncrementBy must be greater than 0.");
             }
 
-            return metadata -> metadata
-                                       .addCustomMetadataObject(CUSTOM_METADATA_KEY, attributeName)
+            return metadata -> metadata.addCustomMetadataObject(CUSTOM_METADATA_KEY, attributeName)
                                        .addCustomMetadataObject(START_AT_METADATA_KEY, startAt)
                                        .addCustomMetadataObject(INCREMENT_BY_METADATA_KEY, incrementBy)
                                        .markAttributeAsKey(attributeName, attributeValueType);
@@ -145,109 +141,69 @@ public WriteModification beforeWrite(DynamoDbExtensionContext.BeforeWrite contex
             return WriteModification.builder().build();
         }
 
-        Pair<AttributeValue, Expression> updates = getRecordUpdates(versionAttributeKey.get(), context);
-
-        AttributeValue newVersionValue = updates.left();
-        Expression condition = updates.right();
-
         Map<String, AttributeValue> itemToTransform = new HashMap<>(context.items());
-        itemToTransform.put(versionAttributeKey.get(), newVersionValue);
-
-        return WriteModification.builder()
-                                .transformedItem(Collections.unmodifiableMap(itemToTransform))
-                                .additionalConditionalExpression(condition)
-                                .build();
-    }
-
-    private Pair<AttributeValue, Expression> getRecordUpdates(String versionAttributeKey,
-                                                              DynamoDbExtensionContext.BeforeWrite context) {
-        Map<String, AttributeValue> itemToTransform = context.items();
-
-        // Default to NUL if not present to reduce additional checks further along
-        AttributeValue existingVersionValue = itemToTransform.getOrDefault(versionAttributeKey, DEFAULT_VALUE);
 
-        if (isInitialVersion(existingVersionValue, context)) {
-            return createInitialRecord(versionAttributeKey, context);
-        }
-        // Existing record, increment version
-        return updateExistingRecord(versionAttributeKey, existingVersionValue, context);
-    }
-
-    private boolean isInitialVersion(AttributeValue existingVersionValue, DynamoDbExtensionContext.BeforeWrite context) {
-        Optional<Integer> versionStartAtFromAnnotation = context.tableMetadata()
-                                                                .customMetadataObject(VersionAttribute.START_AT_METADATA_KEY,
-                                                                                      Integer.class);
-
-        return isNullAttributeValue(existingVersionValue)
-               || (versionStartAtFromAnnotation.isPresent()
-                   && getExistingVersion(existingVersionValue) == versionStartAtFromAnnotation.get())
-               || getExistingVersion(existingVersionValue) == this.startAt;
-    }
-
-    private Pair<AttributeValue, Expression> createInitialRecord(String versionAttributeKey,
-                                                                 DynamoDbExtensionContext.BeforeWrite context) {
-        Optional<Integer> versionStartAtFromAnnotation = context.tableMetadata()
-                                                                .customMetadataObject(VersionAttribute.START_AT_METADATA_KEY,
-                                                                                      Integer.class);
-
-        AttributeValue newVersionValue = versionStartAtFromAnnotation.isPresent() ?
-                                         incrementVersion(versionStartAtFromAnnotation.get(), context) :
-                                         incrementVersion(this.startAt, context);
-
-
-        String attributeKeyRef = keyRef(versionAttributeKey);
-
-        Expression condition = Expression.builder()
-                                         // Check that the version does not exist before setting the initial value.
-                                         .expression(String.format("attribute_not_exists(%s)", attributeKeyRef))
-                                         .expressionNames(Collections.singletonMap(attributeKeyRef, versionAttributeKey))
-                                         .build();
-
-        return Pair.of(newVersionValue, condition);
-    }
-
-    private Pair<AttributeValue, Expression> updateExistingRecord(String versionAttributeKey,
-                                                                  AttributeValue existingVersionValue,
-                                                                  DynamoDbExtensionContext.BeforeWrite context) {
-        int existingVersion = getExistingVersion(existingVersionValue);
-        AttributeValue newVersionValue = incrementVersion(existingVersion, context);
-
-        String attributeKeyRef = keyRef(versionAttributeKey);
-        String existingVersionValueKey = VERSIONED_RECORD_EXPRESSION_VALUE_KEY_MAPPER.apply(versionAttributeKey);
+        String attributeKeyRef = keyRef(versionAttributeKey.get());
+        AttributeValue newVersionValue;
+        Expression condition;
+        Optional<AttributeValue> existingVersionValue =
+            Optional.ofNullable(itemToTransform.get(versionAttributeKey.get()));
+
+        Optional<Long> versionStartAtFromAnnotation = context.tableMetadata()
+                                                             .customMetadataObject(VersionAttribute.START_AT_METADATA_KEY,
+                                                                                   Long.class);
+
+        Optional<Long> versionIncrementByFromAnnotation = context.tableMetadata()
+                                                                 .customMetadataObject(VersionAttribute.INCREMENT_BY_METADATA_KEY,
+                                                                                       Long.class);
+
+        if (!existingVersionValue.isPresent() || isNullAttributeValue(existingVersionValue.get()) ||
+            (existingVersionValue.get().n() != null &&
+             ((versionStartAtFromAnnotation.isPresent() &&
+               Long.parseLong(existingVersionValue.get().n()) == versionStartAtFromAnnotation.get()) ||
+              Long.parseLong(existingVersionValue.get().n()) == this.startAt))) {
+
+            long startValue = versionStartAtFromAnnotation.orElse(this.startAt);
+            long increment = versionIncrementByFromAnnotation.orElse(this.incrementBy);
+
+            newVersionValue = AttributeValue.builder().n(Long.toString(startValue + increment)).build();
+            condition = Expression.builder()
+                                  .expression(String.format("attribute_not_exists(%s)", attributeKeyRef))
+                                  .expressionNames(Collections.singletonMap(attributeKeyRef, versionAttributeKey.get()))
+                                  .build();
+        } else {
+            // Existing record, increment version
+            if (existingVersionValue.get().n() == null) {
+                // In this case a non-null version attribute is present, but it's not an N
+                throw new IllegalArgumentException("Version attribute appears to be the wrong type. N is required.");
+            }
 
-        Expression condition = Expression.builder()
-                                         // Check that the version matches the existing value before setting the updated value.
-                                         .expression(String.format("%s = %s", attributeKeyRef, existingVersionValueKey))
-                                         .expressionNames(Collections.singletonMap(attributeKeyRef, versionAttributeKey))
-                                         .expressionValues(Collections.singletonMap(existingVersionValueKey,
-                                                                                    existingVersionValue))
-                                         .build();
+            long existingVersion = Long.parseLong(existingVersionValue.get().n());
+            String existingVersionValueKey = VERSIONED_RECORD_EXPRESSION_VALUE_KEY_MAPPER.apply(versionAttributeKey.get());
 
-        return Pair.of(newVersionValue, condition);
-    }
+            long increment = versionIncrementByFromAnnotation.orElse(this.incrementBy);
+            newVersionValue = AttributeValue.builder().n(Long.toString(existingVersion + increment)).build();
 
-    private int getExistingVersion(AttributeValue existingVersionValue) {
-        if (existingVersionValue.n() == null) {
-            throw new IllegalArgumentException("Version attribute appears to be the wrong type. N is required.");
+            condition = Expression.builder()
+                                  .expression(String.format("%s = %s", attributeKeyRef, existingVersionValueKey))
+                                  .expressionNames(Collections.singletonMap(attributeKeyRef, versionAttributeKey.get()))
+                                  .expressionValues(Collections.singletonMap(existingVersionValueKey,
+                                                                             existingVersionValue.get()))
+                                  .build();
         }
 
-        return Integer.parseInt(existingVersionValue.n());
-    }
+        itemToTransform.put(versionAttributeKey.get(), newVersionValue);
 
-    private AttributeValue incrementVersion(int version, DynamoDbExtensionContext.BeforeWrite context) {
-        Optional<Integer> versionIncrementByFromAnnotation = context.tableMetadata()
-                                                            .customMetadataObject(VersionAttribute.INCREMENT_BY_METADATA_KEY,
-                                                                                  Integer.class);
-        if (versionIncrementByFromAnnotation.isPresent()) {
-            return AttributeValue.fromN(Integer.toString(version + versionIncrementByFromAnnotation.get()));
-        }
-        return AttributeValue.fromN(Integer.toString(version + this.incrementBy));
+        return WriteModification.builder()
+                                .transformedItem(Collections.unmodifiableMap(itemToTransform))
+                                .additionalConditionalExpression(condition)
+                                .build();
     }
 
     @NotThreadSafe
     public static final class Builder {
-        private int startAt = 0;
-        private int incrementBy = 1;
+        private Long startAt = 0L;
+        private Long incrementBy = 1L;
 
         private Builder() {
         }
@@ -259,10 +215,7 @@ private Builder() {
          * @param startAt
          * @return the builder instance
          */
-        public Builder startAt(int startAt) {
-            if (startAt < 0) {
-                throw new IllegalArgumentException("StartAt cannot be negative.");
-            }
+        public Builder startAt(Long startAt) {
             this.startAt = startAt;
             return this;
         }
@@ -274,10 +227,7 @@ public Builder startAt(int startAt) {
          * @param incrementBy
          * @return the builder instance
          */
-        public Builder incrementBy(int incrementBy) {
-            if (incrementBy < 1) {
-                throw new IllegalArgumentException("IncrementBy must be greater than 0.");
-            }
+        public Builder incrementBy(Long incrementBy) {
             this.incrementBy = incrementBy;
             return this;
         }
diff --git a/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/extensions/annotations/DynamoDbVersionAttribute.java b/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/extensions/annotations/DynamoDbVersionAttribute.java
@@ -39,14 +39,14 @@
      *
      * @return the starting value
      */
-    int startAt() default 0;
+    long startAt() default 0;
 
     /**
      * The amount to increment the version by with each update.
      * Default value - {@code 1}.
      *
      * @return the increment value
      */
-    int incrementBy() default 1;
+    long incrementBy() default 1;
 
 }
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

Original file line number	Diff line number	Diff line change
`@@ -321,7 +321,7 @@ public String toString() {`
`321`	`321`	`}`
`322`	`322`
`323`	`323`	`/**`
`324`		`- * A builder for {@link Expression}v`
	`324`	`+ * A builder for {@link Expression}`
`325`	`325`	`*/`
`326`	`326`	`@NotThreadSafe`
`327`	`327`	`public static final class Builder {`
Original file line number	Diff line number	Diff line change
`@@ -39,14 +39,14 @@`
`39`	`39`	`*`
`40`	`40`	`* @return the starting value`
`41`	`41`	`*/`
`42`		`- int startAt() default 0;`
	`42`	`+ long startAt() default 0;`
`43`	`43`
`44`	`44`	`/**`
`45`	`45`	`* The amount to increment the version by with each update.`
`46`	`46`	`* Default value - {@code 1}.`
`47`	`47`	`*`
`48`	`48`	`* @return the increment value`
`49`	`49`	`*/`
`50`		`- int incrementBy() default 1;`
	`50`	`+ long incrementBy() default 1;`
`51`	`51`
`52`	`52`	`}`