Skip to content

Fix TransactWrite<T>.AddSaveItem when T does NOT contain DynamoDbProperty Attributes #3614

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

Merged

Conversation

irina-herciu
Copy link
Contributor

Description

fix: Fixed an issue where TransactWrite.AddSaveItem throws when T does NOT contain DynamoDbProperty Attributes

given that T does not contain any DynamoDbProperty, in order to use TransactWriteItem API to save the entity, it will require to perform PutItem operation instead of an UpdateItem operation.

Motivation and Context

Issue #3095

Testing

Tested locally using below code:

[Fact]
public async Task GivenCreateMultiTableTransactWrite_WhenAddSaveItem_ShouldSucceed()
{
DynamoDBOperationConfig TransactionDynamoDbOperationConfig = new DynamoDBOperationConfig()
{
SkipVersionCheck = true,
ConditionalOperator = ConditionalOperatorValues.And
};

 var table1 = new Table1() { Id = "Id2", RelatedId = "Id1"};
 var table2 = new Table2() { Id = "Id1", RelatedId = "Id2"};

 IDynamoDBContext dbContext = new DynamoDBContext(new AmazonDynamoDBClient());

 TransactWrite<Table1> t1 = dbContext.CreateTransactWrite<Table1>(TransactionDynamoDbOperationConfig);
 t1.AddSaveItem(table1);

 TransactWrite<Table2> t2 = dbContext.CreateTransactWrite<Table2>(TransactionDynamoDbOperationConfig);
 t2.AddSaveItem(table2);

 MultiTableTransactWrite transaction = dbContext.CreateMultiTableTransactWrite(t1,t2);
 await transaction.ExecuteAsync();

}

[DynamoDBTable("Table1")]
public class Table1
{
[DynamoDBHashKey("Id")]
public string Id { get; set; } = string.Empty;

[DynamoDBRangeKey("RelatedId")]
public string RelatedId { get; set; } = string.Empty;

}

[DynamoDBTable("Table2")]
public class Table2
{
[DynamoDBHashKey("Id")]
public string Id { get; set; } = string.Empty;

[DynamoDBRangeKey("RelatedId")]
public string RelatedId { get; set; } = string.Empty;

[DynamoDBProperty("NonKey")]
public string NonKey { get; set; } = string.Empty;

}

Screenshots (if appropriate)

Types of changes

  • [ x] 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

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

License

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

@dscpinheiro dscpinheiro changed the base branch from main to main-staging January 20, 2025 13:43
@irina-herciu irina-herciu marked this pull request as ready for review January 21, 2025 08:30
@afroz429 afroz429 requested review from normj and philasmar January 22, 2025 18:37
Copy link
Member

@normj normj left a comment

Choose a reason for hiding this comment

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

We need a test for this change. I would suggest adding a test to sdk\test\Services\DynamoDBv2\IntegrationTests\DataModelTests.cs.

Also as you create PRs you need to add a change file so the release system will know to update the version and generate the change log. For this PR you would add a file generator/.DevConfigs/<random-guid>.json here with the following content. Feel free to tweak the message.

{
  "services": [
    {
      "serviceName": "DynamoDBv2",
      "type": "patch",
      "changeLogMessages": [
        "Fixed issue with TransactWrite using the DataModel not handling when only keys are being saved."
      ]
    }
  ]
}

Update TransactWrite.cs

use putItem operation for key only entities
@irina-herciu irina-herciu force-pushed the issues/3095_TransactWrite branch from f20be80 to 377983c Compare January 27, 2025 10:46
@normj
Copy link
Member

normj commented Jan 27, 2025

Changes look good. Running the change through the internal validation system.

Copy link
Member

@normj normj left a comment

Choose a reason for hiding this comment

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

Internal dry run successful.

@dscpinheiro dscpinheiro merged commit 67d8df8 into aws:main-staging Jan 28, 2025
@dscpinheiro
Copy link
Contributor

@irina-herciu Thanks again for the contribution, this PR will be included in today's release of the SDK (I'll update the linked issue with the version number once the new package is available).

@dscpinheiro dscpinheiro mentioned this pull request Feb 6, 2025
1 task
@irina-herciu irina-herciu deleted the issues/3095_TransactWrite branch June 2, 2025 10:26
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.

4 participants