Skip to content

Conversation

@irina-herciu
Copy link
Contributor

@irina-herciu irina-herciu commented Nov 18, 2025

Description

New Request Object Types

Introduced three new request classes under Amazon.DynamoDBv2.DocumentModel:

public class PutItemDocumentOperationRequest { ... }
public class UpdateItemDocumentOperationRequest { ... }
public class DeleteItemDocumentOperationRequest { ... }

Added new overloads to Table:

Task<Document> PutItemAsync(PutItemDocumentOperationRequest request, CancellationToken cancellationToken = default);
Task<Document> UpdateItemAsync(UpdateItemDocumentOperationRequest request, CancellationToken cancellationToken = default);
Task<Document> DeleteItemAsync(DeleteItemDocumentOperationRequest request, CancellationToken cancellationToken = default);

Motivation and Context

The existing Amazon.DynamoDBv2.DocumentModel.Table API relies heavily on method overloads for common operations such as PutItem, UpdateItem, and DeleteItem.
This design:

Obscures caller intent and reduces discoverability.

Makes it difficult to extend support for optional features like projections, and expressions.

Uses legacy constructs (Expected, AttributeUpdates, etc.) that are incompatible with expression-based syntax used in modern DynamoDB APIs.

To modernize this surface and align with other AWS SDKs, this PR introduces strongly-typed request objects for PutItem, UpdateItem, and DeleteItem operations in the Document Model layer.

Testing

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

@irina-herciu irina-herciu changed the title Add Request Object Pattern and Expression-Based Migration for DynamoDB Document Model Add Request Object Pattern for DynamoDB Document Model Nov 19, 2025
wip

wip

implement sync update method and unit tests

PutItemOperationRequest

integration tests
@irina-herciu irina-herciu force-pushed the features/atomicOperations branch from 1965390 to fc6c71f Compare November 19, 2025 11:52
@irina-herciu irina-herciu marked this pull request as ready for review November 19, 2025 11:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a Request Object Pattern for the DynamoDB Document Model API, modernizing the API surface by replacing overload-heavy methods with strongly-typed request objects. This change makes the API more extensible and aligns with modern SDK patterns by supporting expression-based syntax for operations like PutItem, UpdateItem, DeleteItem, GetItem, and Scan.

Key changes:

  • Adds new request classes (PutItemDocumentOperationRequest, UpdateItemDocumentOperationRequest, DeleteItemDocumentOperationRequest, GetItemDocumentOperationRequest, ScanDocumentOperationRequest)
  • Implements a pipeline architecture (DocumentOperationPipeline) for consistent request processing
  • Adds new method overloads accepting request objects to the Table class

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
DocumentOperationRequest.cs Defines new request object classes for Get, Put, Update, Delete, and Scan operations
DocumentOperationPipeline.cs Implements pipeline infrastructure for validating, mapping, and executing document operations
Table.cs Adds internal helper methods to bridge request objects with the pipeline
Table.Sync.cs Adds synchronous public API methods and interface definitions for request objects
Table.Async.cs Adds asynchronous public API methods for request objects
Expression.cs Adds new methods for applying conditional and update expressions separately
TableTests.cs Comprehensive unit tests for the new request pattern
DocumentTests.cs Integration tests validating end-to-end behavior
DataModelTests.cs Unrelated test for TTL functionality
Util.cs Minor whitespace fix

Comment on lines +38 to +45
public TResult ExecuteSync(THighLevelRequest request)
{
Validate(request);
var low = Map(request);
ApplyExpressions(request, low);
var resp = InvokeSync(low);
return PostProcess(request, resp);
}
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

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

The synchronous execution path is missing a call to Table.UpdateRequestUserAgentDetails(low, isAsync: false) before invoking the service. This call is present in the async path (line 58) and is consistently used throughout the existing codebase for telemetry tracking. Add Table.UpdateRequestUserAgentDetails(low, isAsync: false); after line 42.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +587 to +595
/// <inheritdoc/>
public async Task<Document> DeleteItemAsync(IDictionary<string, DynamoDBEntry> key, DeleteItemOperationConfig config, CancellationToken cancellationToken = default(CancellationToken))
{
return await DeleteHelperAsync(MakeKey(key), config, cancellationToken).ConfigureAwait(false);
var operationName = DynamoDBTelemetry.ExtractOperationName(nameof(Table), nameof(DeleteItemAsync));
using (DynamoDBTelemetry.CreateSpan(TracerProvider, operationName, spanKind: SpanKind.CLIENT))
{
return await DeleteHelperAsync(MakeKey(key), config, cancellationToken).ConfigureAwait(false);
}
}
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

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

Inconsistent indentation detected. Lines 587-595 have extra indentation compared to the rest of the file. This should align with the same indentation level as other methods in the class.

Copilot uses AI. Check for mistakes.
Comment on lines +1125 to +1162
[TestMethod]
[TestCategory("DynamoDBv2")]
public void TestContext_SaveItem_WithTTL()
{
TableCache.Clear();
CleanupTables();
TableCache.Clear();

// Create context and table if needed
CreateContext(DynamoDBEntryConversion.V2, true);

// Define TTL to be 2 minutes in the future
var ttlEpoch = DateTimeOffset.UtcNow.AddMinutes(2).ToUnixTimeSeconds();

var item = new TtlTestItem
{
Id = 1,
Data = "Test with TTL",
Ttl = ttlEpoch
};

Context.Save(item);

// Load immediately, should exist
var loaded = Context.Load<TtlTestItem>(item.Id);
Assert.IsNotNull(loaded);
Assert.AreEqual(item.Id, loaded.Id);
Assert.AreEqual(item.Data, loaded.Data);
Assert.AreEqual(item.Ttl, loaded.Ttl);

item.Ttl = DateTimeOffset.UtcNow.AddMinutes(3).ToUnixTimeSeconds();
Context.Save(item);
var loaded2 = Context.Load<TtlTestItem>(item.Id);
Assert.IsNotNull(loaded2);
Assert.AreEqual(item.Id, loaded2.Id);
Assert.AreEqual(item.Data, loaded2.Data);
Assert.AreEqual(item.Ttl, loaded2.Ttl);
}
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

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

This test appears to be unrelated to the PR's purpose of adding Request Object Pattern for DynamoDB Document Model. The test is about TTL functionality in the DataModel (not DocumentModel) and should likely be in a separate PR. According to the PR description, this PR should only contain changes related to introducing request objects for PutItem, UpdateItem, and DeleteItem operations in the Document Model layer.

Copilot generated this review using guidance from repository custom instructions.
@GarrettBeatty GarrettBeatty changed the base branch from main to development November 22, 2025 01:17
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