VersionedRecordExtension does not support 0 as a starting value

[akiesler]

Describe the bug

The VersionedRecordExtension says it will increment a version and verify that the version in DynamoDB is equal to the previous value when updating. However, the documentation is unclear that the starting value must be null and cannot be 0 which would be a reasonable starting version. Using a value of 0 results in the conditional check failing when attempting to put the item.

Expected Behavior

I would expect the extension to treat 0 the same as null acting as a clear starting point for the versioning. This would result in an update expression where the version attribute is checked not to exist before updating.

Current Behavior

The DynamoDB client throws a ConditionalUpdateFailedException because the the version field does not exist in the record.

Reproduction Steps

A simple test case modified from the SDK:

   @Test
    public void beforeWrite_initialVersionDueToExplicitZero_expressionAndTransformedItemIsCorrect() {
        FakeItem fakeItem = createUniqueFakeItem();

        Map<String, AttributeValue> inputMap =
            new HashMap<>(FakeItem.getTableSchema().itemToMap(fakeItem, true));
        inputMap.put("version", AttributeValue.builder().n("0").build());

        Map<String, AttributeValue> fakeItemWithInitialVersion =
            new HashMap<>(FakeItem.getTableSchema().itemToMap(fakeItem, true));
        fakeItemWithInitialVersion.put("version", AttributeValue.builder().n("1").build());

        WriteModification result =
            versionedRecordExtension.beforeWrite(DefaultDynamoDbExtensionContext
                                                     .builder()
                                                     .items(inputMap)
                                                     .tableMetadata(FakeItem.getTableMetadata())
                                                     .operationContext(PRIMARY_CONTEXT).build());

        assertThat(result.transformedItem(), is(fakeItemWithInitialVersion));
        assertThat(result.additionalConditionalExpression(),
                   is(Expression.builder()
                                .expression("attribute_not_exists(#AMZN_MAPPED_version)")
                                .expressionNames(singletonMap("#AMZN_MAPPED_version", "version"))
                                .build()));
    }

Possible Solution

Update the check in beforeWrite for the VersionedRecordExtension to include a check for 0.
If there is concern that this may break backwards compatibility it could be added behind a feature flag in the extension itself.

Additional Information/Context

No response

AWS Java SDK version used

2.20.43

JDK version used

openjdk version "1.8.0_362"

Operating System and version

macOS 12.6.4

[debora-ito]

Not sure I'd consider this a bug, but I agree that documentation is lacking.
We are actively working on a more comprehensive documentation for everything DynamoDB Enhanced Client, but I think we should clarify the behavior in the Javadoc too.

@akiesler what do you think?

[akiesler]

I definitely agree the javadoc is unclear on this point as it does not specify that it must be nullable.

This attribute must be an 'integer' type numeric (long or integer), and you need to tag it as the version attribute.

I do feel that this is a defect that should be addressed. If this is not considered a bug could we reclassify this as a feature request?
It's very common to want to have a non-null integer field that we know will always be set. Making it nullable means that we must then know when it can be null and check for the edge case.

[akiesler]

I was recently bitten by this issue again. I don't think a documentation update is sufficient and instead there should be an option to enable a specific starting value (default of 0) for users to specify how to start counting their versions. I think for backwards compatibility the logic could be updated to treat null as 0 to prevent teams from having to re-write their entire codebases (Option 1). If there are concerns about the backwards compatibility it could optionally be something that clients opt into (Option 2).

I see two possible options of how it could be implemented either at the annotation level or at the extension level. For the first option (Option A) each record type could define the starting value they want to use. This would require updating each record type but would also grant clients the ability to flexibility set the starting point to whatever value they need.

@DynamoDbVersionAttribute( startAt=0 )

The second option (Option B) would be specified at extension creation time to set a consistent starting point for all records.

VersionedRecordExtension.builder()
    .startAt(0)
    .build();

What would be concerns around Option 1 of changing the default behavior?
If Option 2 is preferred what is the preferred means to enable the behavior?

[akiesler]

@debora-ito is there a strong opposition to this approach of supporting a 0 starting value?

[debora-ito]

Fixed via #6019.

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.