DDB Enhanced: Allow custom versioning

[RanVaknin]

Motivation and Context

This PR builds on the work started in issue #3894 where the VersionedRecordExtension didn't support starting version numbers at 0, requiring clients to use Integer objects (with null initial values) rather than long primitives. As mentioned by @akiesler in the original issue, developers might expect versions to start at 0 and increment from there, rather than having a special case where the value must be initialized to null.

This implementation allows the extension to be more flexible by allowing:

• Starting versions at 0 (or any non-negative long)
• Configuring custom increment values (only positive numbers)
• Supporting these configs both at the extension level and via annotations

Modifications

• Refactored the VersionedRecordExtension to support explicit startAt and incrementBy values via builder methods (making it opt-in).
• Expanded the DynamoDbVersionAttribute annotation to support startAt and incrementBy parameters
• Added validation to prevent negative startAt values and non-positive incrementBy values

Testing

• Custom startAt and incrementBy values through both builder and annotations
• Validation of illegal values (negative startAt, zero/negative incrementBy)
• Precedence rules between annotation and builder configurations
• Version incrementation for both initial and existing records
• Edge cases with different configuration combinations

[akiesler]

Thank you for picking up this change! It will greatly simplify our version handling. Please let me know if I can do anything to help.

[dagnir]

• This is a very complex expression. Let's move this to a separate function so it's easier to reason about and we can document it

[zoewangg]

+1. Why do we need to check existingVersionValue.get().n() != null && ((versionStartAtFromAnnotation.isPresent() && Long.parseLong(existingVersionValue.get().n()) == versionStartAtFromAnnotation.get()) || Long.parseLong(existingVersionValue.get().n()) == this.startAt)))?

Suggesting the following

if (existingValueNotAvailable(existingAttributeValue)) {
    long startVersion = versionStartAtFromAnnotation.orElse(this.startAt);
    long incrementVersion = versionIncrementByFromAnnotation.orElse(this.incrementBy);
  ...
} else {
  ....
}

[RanVaknin]

we need the expanded evaluation to see what constitutes an "initial version".
There are three ways a version can be considered "initial":

1. The version doesn't exist in DynamoDB yet (existing behavior)
2. The version is explicitly set to null (existing behavior)
3. The version equals a configured start value (new behavior)

For case 3, the extension must check if the version matches either:

• The start value from the annotation (@DynamoDbVersionAttribute(startAt = 3))
• The start value from the builder (builder().startAt(3).build())

[dagnir]

It doesn't look like the original version handled this, but how do we deal with integer overflow? Originally incrementBy was always 1 so it's unlikely to happen, but with incrementBy being configurable now, it's technically more likely now if the user sets a relatively large value.

Can we check how v1's mapper handles this (if at all)?

[RanVaknin]

Doesn't seem like v1 had any sort of integer overflow protection?

https://github.com/aws/aws-sdk-java/blob/c34aa7f14d7598f033091b7b9be96ee82d2c3860/aws-java-sdk-dynamodb/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/DynamoDBMapper.java#L2454-L2470

[dagnir]

Thanks for looking! I'd still like us to add protection for 2.x if possible

[zoewangg]

Oh I see, I completely missed this. So just to double check, in order to make it backwards compatible, the new behavior should be like the following:

• if startAt is not configured: we only add version if the version attribute does not exist or is null
• if startAt is configured and the version attribute is equal to startAt: we will treat it as initial version

Is that correct?

[RanVaknin]

@zoewangg yes you are correct.

[zoewangg]

+1. Why do we need to check existingVersionValue.get().n() != null && ((versionStartAtFromAnnotation.isPresent() && Long.parseLong(existingVersionValue.get().n()) == versionStartAtFromAnnotation.get()) || Long.parseLong(existingVersionValue.get().n()) == this.startAt)))?

Suggesting the following

if (existingValueNotAvailable(existingAttributeValue)) {
    long startVersion = versionStartAtFromAnnotation.orElse(this.startAt);
    long incrementVersion = versionIncrementByFromAnnotation.orElse(this.incrementBy);
  ...
} else {
  ....
}

[sonarqubecloud]

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[dagnir]

Delete

[zoewangg]

Use of Optional as method parameter is a bit code smell. https://github.com/aws/aws-sdk-java-v2/blob/master/docs/design/UseOfOptional.md

[zoewangg]

Oh I see, I completely missed this. So just to double check, in order to make it backwards compatible, the new behavior should be like the following:

• if startAt is not configured: we only add version if the version attribute does not exist or is null
• if startAt is configured and the version attribute is equal to startAt: we will treat it as initial version

Is that correct?

[zoewangg]

Can we add functional tests as well that tests end to end functionality with dynamodb local? https://github.com/aws/aws-sdk-java-v2/blob/master/services-custom/dynamodb-enhanced/src/test/java/software/amazon/awssdk/enhanced/dynamodb/functionaltests/VersionedRecordTest.java