Skip to content

Issues/224 atomic counter attribute #3809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: development
Choose a base branch
from

Conversation

irina-herciu
Copy link
Contributor

Introduce support for the [DynamoDBAtomicCounter] attribute in the DynamoDB Object Persistence Model for the AWS SDK for .NET. This feature brings parity with the Java SDK V2, which supports atomic counters via @DynamoDbAtomicCounter.

java DynamoDbAtomicCounter
Using atomic counters

Description

  • New attribute: DynamoDBAtomicCounterAttribute, to mark numeric properties for auto-increment behavior
  • Updates to DynamoDBContext and internal mapping to detect and apply counter increment behavior during save item operation
class Session {
    [DynamoDBHashKey]
    public string Id { get; set; }

    [DynamoDBAtomicCounter]
    public int RetryCount { get; set; }

    // This field starts from 100 and increments by 5 on each save
    [DynamoDBAtomicCounter(delta:5, startValue:100)] 
    public long Views { get; set; }
}

On context.SaveAsync(session), it will automatically be incremented in DynamoDB using the appropriate SET operation.

Motivation and Context

Testing

Integration test added
Unit test to be added for internal methods

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

foreach (var propertyStorage in counterPropertyStorages)
{
Primitive counter;
string versionAttributeName = propertyStorage.AttributeName;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be called counterAttributeName?

string startValueName = $":{propertyStorage.AttributeName}Start";
string deltaValueName = $":{propertyStorage.AttributeName}Delta";
string counterAttributeName = Common.GetAttributeReference(propertyStorage.AttributeName);
asserts += $"{counterAttributeName} = " +
Copy link
Member

Choose a reason for hiding this comment

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

Curious how this compares to Java V2. But if I set the StartValue to 3 and Delta to 5 when I save my first version of the object I get 8. I would expect 3 after the first save and 8 with the second save. If this is consistent with Java we need to make sure the docs on the StartValue and Delta are clear this is how it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

first save will be 3 as the value in ExpressionAttributeValues is CounterStartValue-CounterDelta so that the increment can be done correctly. This is the same behavior as in Java V2.

if (counter != null && counter.Value != null)
{
if (counter.Type != DynamoDBEntryType.Numeric) throw new InvalidOperationException("Atomic Counter property must be numeric.");
IncrementCounter(propertyStorage.MemberType, ref counter, propertyStorage.CounterDelta);
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly this increment is for getting the in memory updated to what the value will be incremented to. But what if the in memory value is behind what is in DynamoDB? If I'm understanding that correctly this seems bad. If I'm misunderstanding this then the logic needs more doc comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants