From 670ae6003e69618a5ce3ea319903f7b523112956 Mon Sep 17 00:00:00 2001 From: "Stuart Blackler (@im5tu)" Date: Wed, 14 Jan 2026 10:39:39 +0000 Subject: [PATCH 1/3] Added missing properties. Fixed dynamo ops --- .../ReturnValuesOnConditionCheckFailure.cs | 17 + .../Models/ItemCollectionMetrics.cs | 18 + .../Batch/BatchWriteItemResponse.cs | 5 + .../DeleteItem/DeleteItemBuilder.cs | 27 +- .../DeleteItem/DeleteItemRequest.cs | 6 + .../DeleteItem/DeleteItemResponse.cs | 5 + .../Operations/PutItem/PutItemBuilder.cs | 28 +- .../Operations/PutItem/PutItemRequest.cs | 6 + .../Operations/Query/QueryBuilder.cs | 63 +- .../Operations/Scan/ScanBuilder.cs | 16 +- .../TransactConditionCheckItem.cs | 9 +- .../Transactions/TransactDeleteItem.cs | 9 +- .../Transactions/TransactGetBuilder.cs | 12 + .../Transactions/TransactGetRequest.cs | 9 +- .../Transactions/TransactPutItem.cs | 9 +- .../Transactions/TransactUpdateItem.cs | 9 +- .../UpdateItem/UpdateItemBuilder.cs | 28 +- .../UpdateItem/UpdateItemRequest.cs | 8 +- .../UpdateItem/UpdateItemResponse.cs | 5 + .../ConditionExpressionValuesTests.cs | 547 ++++++++++++++++++ 20 files changed, 803 insertions(+), 33 deletions(-) create mode 100644 src/Clients/Goa.Clients.Dynamo/Enums/ReturnValuesOnConditionCheckFailure.cs create mode 100644 src/Clients/Goa.Clients.Dynamo/Models/ItemCollectionMetrics.cs create mode 100644 tests/Clients/Goa.Clients.Dynamo.Tests/ConditionExpressionValuesTests.cs diff --git a/src/Clients/Goa.Clients.Dynamo/Enums/ReturnValuesOnConditionCheckFailure.cs b/src/Clients/Goa.Clients.Dynamo/Enums/ReturnValuesOnConditionCheckFailure.cs new file mode 100644 index 00000000..f52a99c6 --- /dev/null +++ b/src/Clients/Goa.Clients.Dynamo/Enums/ReturnValuesOnConditionCheckFailure.cs @@ -0,0 +1,17 @@ +namespace Goa.Clients.Dynamo.Enums; + +/// +/// Specifies how to return attribute values when a condition check fails. +/// +public enum ReturnValuesOnConditionCheckFailure +{ + /// + /// Nothing is returned. (This is the default.) + /// + NONE, + + /// + /// Returns all attributes of the item as they appeared before the operation. + /// + ALL_OLD +} diff --git a/src/Clients/Goa.Clients.Dynamo/Models/ItemCollectionMetrics.cs b/src/Clients/Goa.Clients.Dynamo/Models/ItemCollectionMetrics.cs new file mode 100644 index 00000000..c3766732 --- /dev/null +++ b/src/Clients/Goa.Clients.Dynamo/Models/ItemCollectionMetrics.cs @@ -0,0 +1,18 @@ +namespace Goa.Clients.Dynamo.Models; + +/// +/// Information about item collections, if any, that were affected by the operation. +/// +public class ItemCollectionMetrics +{ + /// + /// The partition key value of the item collection. + /// + public Dictionary? ItemCollectionKey { get; set; } + + /// + /// An estimate of item collection size, in gigabytes. This value is a two-element array + /// containing a lower bound and an upper bound for the estimate. + /// + public List? SizeEstimateRangeGB { get; set; } +} diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Batch/BatchWriteItemResponse.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Batch/BatchWriteItemResponse.cs index 358358da..af4d2d12 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Batch/BatchWriteItemResponse.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Batch/BatchWriteItemResponse.cs @@ -23,4 +23,9 @@ public class BatchWriteItemResponse /// The write capacity units consumed by the BatchWriteItem operation. /// public List? ConsumedCapacity { get; set; } + + /// + /// A list of tables that were processed by BatchWriteItem and, for each table, information about any item collections that were affected by individual DeleteItem or PutItem operations. + /// + public Dictionary>? ItemCollectionMetrics { get; set; } } \ No newline at end of file diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemBuilder.cs index 1720230d..e0ccf299 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemBuilder.cs @@ -44,10 +44,18 @@ public DeleteItemBuilder WithCondition(Condition condition) _request.ConditionExpression += " AND " + condition.Expression; } - _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); - _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + if (condition.ExpressionNames.Count > 0) + { + _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); + } + + if (condition.ExpressionValues.Count > 0) + { + _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + } + return this; } @@ -90,6 +98,17 @@ public DeleteItemBuilder WithReturnItemCollectionMetrics(ReturnItemCollectionMet return this; } + /// + /// Specifies how to return attribute values when a conditional check fails. + /// + /// The return values on condition check failure setting. + /// The DeleteItemBuilder instance for method chaining. + public DeleteItemBuilder WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure returnValuesOnConditionCheckFailure) + { + _request.ReturnValuesOnConditionCheckFailure = returnValuesOnConditionCheckFailure; + return this; + } + /// /// Builds and returns the configured DeleteItemRequest. /// diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemRequest.cs b/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemRequest.cs index 506cef7d..548e5ca6 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemRequest.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemRequest.cs @@ -48,4 +48,10 @@ public class DeleteItemRequest /// Determines whether item collection metrics are returned. /// public ReturnItemCollectionMetrics ReturnItemCollectionMetrics { get; set; } = ReturnItemCollectionMetrics.NONE; + + /// + /// Specifies how to return attribute values when a conditional check fails. + /// Use ALL_OLD to return all attributes of the item as they appeared before the operation. + /// + public ReturnValuesOnConditionCheckFailure ReturnValuesOnConditionCheckFailure { get; set; } = ReturnValuesOnConditionCheckFailure.NONE; } diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemResponse.cs b/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemResponse.cs index 23abfd96..072e3393 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemResponse.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemResponse.cs @@ -16,4 +16,9 @@ public class DeleteItemResponse /// The number of capacity units consumed by the operation. /// public double? ConsumedCapacityUnits { get; set; } + + /// + /// Information about item collections, if any, that were affected by the operation. + /// + public Dictionary>? ItemCollectionMetrics { get; set; } } \ No newline at end of file diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemBuilder.cs index 041e447d..430949c2 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemBuilder.cs @@ -47,10 +47,19 @@ public PutItemBuilder WithAttribute(string attributeName, AttributeValue value) public PutItemBuilder WithCondition(Condition condition) { _request.ConditionExpression = condition.Expression; - _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); - _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + + if (condition.ExpressionNames.Count > 0) + { + _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); + } + + if (condition.ExpressionValues.Count > 0) + { + _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + } + return this; } @@ -76,6 +85,17 @@ public PutItemBuilder WithReturnValues(ReturnValues returnValues) return this; } + /// + /// Specifies how to return attribute values when a conditional check fails. + /// + /// The return values on condition check failure setting. + /// The PutItemBuilder instance for method chaining. + public PutItemBuilder WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure returnValuesOnConditionCheckFailure) + { + _request.ReturnValuesOnConditionCheckFailure = returnValuesOnConditionCheckFailure; + return this; + } + /// /// Builds and returns the configured PutItemRequest. /// diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemRequest.cs b/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemRequest.cs index ee07f6d2..2ee18891 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemRequest.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemRequest.cs @@ -47,4 +47,10 @@ public class PutItemRequest /// Determines whether item collection metrics are returned. /// public ReturnItemCollectionMetrics ReturnItemCollectionMetrics { get; set; } = ReturnItemCollectionMetrics.NONE; + + /// + /// Specifies how to return attribute values when a conditional check fails. + /// Use ALL_OLD to return all attributes of the item as they appeared before the operation. + /// + public ReturnValuesOnConditionCheckFailure ReturnValuesOnConditionCheckFailure { get; set; } = ReturnValuesOnConditionCheckFailure.NONE; } diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Query/QueryBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Query/QueryBuilder.cs index 9c2641ba..efaeb822 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Query/QueryBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Query/QueryBuilder.cs @@ -1,3 +1,4 @@ +using System.Text.RegularExpressions; using Goa.Clients.Dynamo.Enums; using Goa.Clients.Dynamo.Models; using Goa.Core; @@ -8,7 +9,7 @@ namespace Goa.Clients.Dynamo.Operations.Query; /// Fluent builder for constructing DynamoDB Query requests with a user-friendly API. /// /// The name of the table to query. -public class QueryBuilder(string tableName) +public partial class QueryBuilder(string tableName) { private readonly QueryRequest _request = new() { @@ -31,10 +32,18 @@ public QueryBuilder WithKey(Condition condition) _request.KeyConditionExpression += " AND " + condition.Expression; } - _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); - _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + if (condition.ExpressionNames.Count > 0) + { + _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); + } + + if (condition.ExpressionValues.Count > 0) + { + _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + } + return this; } @@ -54,20 +63,44 @@ public QueryBuilder WithConsistentRead(bool consistentRead = true) /// /// The name of the index to query, or null to query the base table. /// The QueryBuilder instance for method chaining. + /// Thrown when indexName is not between 3-255 characters or contains invalid characters. public QueryBuilder WithIndex(string? indexName) { - _request.IndexName = string.IsNullOrWhiteSpace(indexName) ? null : indexName; + if (string.IsNullOrWhiteSpace(indexName)) + { + _request.IndexName = null; + return this; + } + + ArgumentOutOfRangeException.ThrowIfLessThan(indexName.Length, 3, nameof(indexName)); + ArgumentOutOfRangeException.ThrowIfGreaterThan(indexName.Length, 255, nameof(indexName)); + + if (!IndexNamePattern().IsMatch(indexName)) + { + throw new ArgumentException("Index name must match pattern [a-zA-Z0-9_.-]+.", nameof(indexName)); + } + + _request.IndexName = indexName; return this; } + [GeneratedRegex("^[a-zA-Z0-9_.-]+$")] + private static partial Regex IndexNamePattern(); + /// /// Sets the maximum number of items to return from the query. /// /// The maximum number of items to return, or null for no limit. /// The QueryBuilder instance for method chaining. + /// Thrown when limit is less than 1. public QueryBuilder WithLimit(int? limit) { - _request.Limit = limit > 0 ? limit : null; + if (limit.HasValue) + { + ArgumentOutOfRangeException.ThrowIfLessThan(limit.Value, 1, nameof(limit)); + } + + _request.Limit = limit; return this; } @@ -175,10 +208,18 @@ public QueryBuilder WithFilter(Condition condition) _request.FilterExpression += " AND " + condition.Expression; } - _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); - _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + if (condition.ExpressionNames.Count > 0) + { + _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); + } + + if (condition.ExpressionValues.Count > 0) + { + _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + } + return this; } diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Scan/ScanBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Scan/ScanBuilder.cs index 74c4a120..9231ffde 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Scan/ScanBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Scan/ScanBuilder.cs @@ -31,10 +31,18 @@ public ScanBuilder WithFilter(Condition condition) _request.FilterExpression += " AND " + condition.Expression; } - _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); - _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + if (condition.ExpressionNames.Count > 0) + { + _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); + } + + if (condition.ExpressionValues.Count > 0) + { + _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + } + return this; } diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactConditionCheckItem.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactConditionCheckItem.cs index d9c47c51..e7a31e52 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactConditionCheckItem.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactConditionCheckItem.cs @@ -1,4 +1,5 @@ -using Goa.Clients.Dynamo.Models; +using Goa.Clients.Dynamo.Enums; +using Goa.Clients.Dynamo.Models; namespace Goa.Clients.Dynamo.Operations.Transactions; @@ -31,4 +32,10 @@ public class TransactConditionCheckItem /// One or more substitution tokens for attribute names in an expression. /// public Dictionary? ExpressionAttributeNames { get; set; } + + /// + /// Specifies how to return attribute values when a conditional check fails. + /// Use ALL_OLD to return all attributes of the item as they appeared before the operation. + /// + public ReturnValuesOnConditionCheckFailure ReturnValuesOnConditionCheckFailure { get; set; } = ReturnValuesOnConditionCheckFailure.NONE; } \ No newline at end of file diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactDeleteItem.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactDeleteItem.cs index e65c11d0..d93eca34 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactDeleteItem.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactDeleteItem.cs @@ -1,4 +1,5 @@ -using Goa.Clients.Dynamo.Models; +using Goa.Clients.Dynamo.Enums; +using Goa.Clients.Dynamo.Models; namespace Goa.Clients.Dynamo.Operations.Transactions; @@ -31,4 +32,10 @@ public class TransactDeleteItem /// One or more substitution tokens for attribute names in an expression. /// public Dictionary? ExpressionAttributeNames { get; set; } + + /// + /// Specifies how to return attribute values when a conditional check fails. + /// Use ALL_OLD to return all attributes of the item as they appeared before the operation. + /// + public ReturnValuesOnConditionCheckFailure ReturnValuesOnConditionCheckFailure { get; set; } = ReturnValuesOnConditionCheckFailure.NONE; } \ No newline at end of file diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactGetBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactGetBuilder.cs index 3798a7c1..c8ab5981 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactGetBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactGetBuilder.cs @@ -1,3 +1,4 @@ +using Goa.Clients.Dynamo.Enums; using Goa.Clients.Dynamo.Models; namespace Goa.Clients.Dynamo.Operations.Transactions; @@ -70,6 +71,17 @@ public TransactGetBuilder WithGet(string tableName, Dictionary + /// Determines the level of detail about consumed capacity to return. + /// + /// The level of consumed capacity information to return. + /// The TransactGetBuilder instance for method chaining. + public TransactGetBuilder WithReturnConsumedCapacity(ReturnConsumedCapacity returnConsumedCapacity) + { + _request.ReturnConsumedCapacity = returnConsumedCapacity; + return this; + } + /// /// Builds and returns the configured TransactGetRequest. /// diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactGetRequest.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactGetRequest.cs index 4f580024..29249342 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactGetRequest.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactGetRequest.cs @@ -1,4 +1,6 @@ -namespace Goa.Clients.Dynamo.Operations.Transactions; +using Goa.Clients.Dynamo.Enums; + +namespace Goa.Clients.Dynamo.Operations.Transactions; /// /// Request for transactional get operations. @@ -9,4 +11,9 @@ public class TransactGetRequest /// An ordered array of up to 100 TransactGetItem objects, each of which contains a Get operation. /// public List TransactItems { get; set; } = new(); + + /// + /// Determines the level of detail about provisioned throughput consumption that is returned in the response. + /// + public ReturnConsumedCapacity ReturnConsumedCapacity { get; set; } = ReturnConsumedCapacity.NONE; } \ No newline at end of file diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactPutItem.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactPutItem.cs index 0a20bd9f..0b9ecdda 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactPutItem.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactPutItem.cs @@ -1,4 +1,5 @@ -using Goa.Clients.Dynamo.Models; +using Goa.Clients.Dynamo.Enums; +using Goa.Clients.Dynamo.Models; namespace Goa.Clients.Dynamo.Operations.Transactions; @@ -31,4 +32,10 @@ public class TransactPutItem /// One or more substitution tokens for attribute names in an expression. /// public Dictionary? ExpressionAttributeNames { get; set; } + + /// + /// Specifies how to return attribute values when a conditional check fails. + /// Use ALL_OLD to return all attributes of the item as they appeared before the operation. + /// + public ReturnValuesOnConditionCheckFailure ReturnValuesOnConditionCheckFailure { get; set; } = ReturnValuesOnConditionCheckFailure.NONE; } \ No newline at end of file diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactUpdateItem.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactUpdateItem.cs index 680ff78a..5b8b0d28 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactUpdateItem.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Transactions/TransactUpdateItem.cs @@ -1,4 +1,5 @@ -using Goa.Clients.Dynamo.Models; +using Goa.Clients.Dynamo.Enums; +using Goa.Clients.Dynamo.Models; namespace Goa.Clients.Dynamo.Operations.Transactions; @@ -36,4 +37,10 @@ public class TransactUpdateItem /// One or more substitution tokens for attribute names in an expression. /// public Dictionary? ExpressionAttributeNames { get; set; } + + /// + /// Specifies how to return attribute values when a conditional check fails. + /// Use ALL_OLD to return all attributes of the item as they appeared before the operation. + /// + public ReturnValuesOnConditionCheckFailure ReturnValuesOnConditionCheckFailure { get; set; } = ReturnValuesOnConditionCheckFailure.NONE; } \ No newline at end of file diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemBuilder.cs index dca52d2b..9c687b6a 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemBuilder.cs @@ -803,10 +803,19 @@ public UpdateItemBuilder RemoveFromNumberSet(string attributeName, params decima public UpdateItemBuilder WithCondition(Condition condition) { _request.ConditionExpression = condition.Expression; - _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); - _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); - _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + + if (condition.ExpressionNames.Count > 0) + { + _request.ExpressionAttributeNames ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeNames.Merge(condition.ExpressionNames); + } + + if (condition.ExpressionValues.Count > 0) + { + _request.ExpressionAttributeValues ??= new(StringComparer.OrdinalIgnoreCase); + _request.ExpressionAttributeValues.Merge(condition.ExpressionValues); + } + return this; } @@ -832,6 +841,17 @@ public UpdateItemBuilder WithReturnValues(ReturnValues returnValues) return this; } + /// + /// Specifies how to return attribute values when a conditional check fails. + /// + /// The return values on condition check failure setting. + /// The UpdateItemBuilder instance for method chaining. + public UpdateItemBuilder WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure returnValuesOnConditionCheckFailure) + { + _request.ReturnValuesOnConditionCheckFailure = returnValuesOnConditionCheckFailure; + return this; + } + #endregion #region Build diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemRequest.cs b/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemRequest.cs index fbbf1551..2d009143 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemRequest.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemRequest.cs @@ -31,7 +31,7 @@ public class UpdateItemRequest /// /// One or more values that can be substituted in an expression. /// - public Dictionary? ExpressionAttributeValues { get; set; }= new(StringComparer.OrdinalIgnoreCase); + public Dictionary? ExpressionAttributeValues { get; set; } /// /// One or more substitution tokens for attribute names in an expression. @@ -52,4 +52,10 @@ public class UpdateItemRequest /// Determines whether item collection metrics are returned. /// public ReturnItemCollectionMetrics ReturnItemCollectionMetrics { get; set; } = ReturnItemCollectionMetrics.NONE; + + /// + /// Specifies how to return attribute values when a conditional check fails. + /// Use ALL_OLD to return all attributes of the item as they appeared before the operation. + /// + public ReturnValuesOnConditionCheckFailure ReturnValuesOnConditionCheckFailure { get; set; } = ReturnValuesOnConditionCheckFailure.NONE; } diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemResponse.cs b/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemResponse.cs index 70a7dcb8..d7ba2e10 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemResponse.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemResponse.cs @@ -16,4 +16,9 @@ public class UpdateItemResponse /// The number of capacity units consumed by the operation. /// public double? ConsumedCapacityUnits { get; set; } + + /// + /// Information about item collections, if any, that were affected by the operation. + /// + public Dictionary>? ItemCollectionMetrics { get; set; } } \ No newline at end of file diff --git a/tests/Clients/Goa.Clients.Dynamo.Tests/ConditionExpressionValuesTests.cs b/tests/Clients/Goa.Clients.Dynamo.Tests/ConditionExpressionValuesTests.cs new file mode 100644 index 00000000..987f8091 --- /dev/null +++ b/tests/Clients/Goa.Clients.Dynamo.Tests/ConditionExpressionValuesTests.cs @@ -0,0 +1,547 @@ +using Goa.Clients.Dynamo.Models; +using Goa.Clients.Dynamo.Operations; +using Goa.Clients.Dynamo.Operations.DeleteItem; +using Goa.Clients.Dynamo.Operations.PutItem; +using Goa.Clients.Dynamo.Operations.Query; +using Goa.Clients.Dynamo.Operations.Scan; +using Goa.Clients.Dynamo.Operations.UpdateItem; + +namespace Goa.Clients.Dynamo.Tests; + +/// +/// Tests to ensure that ExpressionAttributeValues is properly handled for all Condition methods. +/// These tests prevent regression of the bug where empty ExpressionAttributeValues ({}) was sent to DynamoDB, +/// which causes a ValidationException. +/// +public class ConditionExpressionValuesTests +{ + #region PutItemBuilder Tests + + [Test] + public async Task PutItemBuilder_WithCondition_AttributeExists_DoesNotSetExpressionAttributeValues() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value" }) + .WithCondition(Condition.AttributeExists("pk")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_exists(#pk)"); + await Assert.That(request.ExpressionAttributeNames!["#pk"]).IsEqualTo("pk"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task PutItemBuilder_WithCondition_AttributeNotExists_DoesNotSetExpressionAttributeValues() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value" }) + .WithCondition(Condition.AttributeNotExists("pk")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_not_exists(#pk)"); + await Assert.That(request.ExpressionAttributeNames!["#pk"]).IsEqualTo("pk"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task PutItemBuilder_WithCondition_Equals_SetsExpressionAttributeValues() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value" }) + .WithCondition(Condition.Equals("version", 1)); + + var request = builder.Build(); + + await Assert.That(request.ExpressionAttributeValues is not null).IsTrue(); + await Assert.That(request.ExpressionAttributeValues!.Count).IsEqualTo(1); + } + + #endregion + + #region DeleteItemBuilder Tests + + [Test] + public async Task DeleteItemBuilder_WithCondition_AttributeExists_DoesNotSetExpressionAttributeValues() + { + var builder = new DeleteItemBuilder("TestTable") + .WithKey("pk", "value") + .WithCondition(Condition.AttributeExists("lockToken")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_exists(#lockToken)"); + await Assert.That(request.ExpressionAttributeNames!["#lockToken"]).IsEqualTo("lockToken"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task DeleteItemBuilder_WithCondition_AttributeNotExists_DoesNotSetExpressionAttributeValues() + { + var builder = new DeleteItemBuilder("TestTable") + .WithKey("pk", "value") + .WithCondition(Condition.AttributeNotExists("lockToken")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_not_exists(#lockToken)"); + await Assert.That(request.ExpressionAttributeNames!["#lockToken"]).IsEqualTo("lockToken"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task DeleteItemBuilder_WithCondition_Equals_SetsExpressionAttributeValues() + { + var builder = new DeleteItemBuilder("TestTable") + .WithKey("pk", "value") + .WithCondition(Condition.Equals("version", 1)); + + var request = builder.Build(); + + await Assert.That(request.ExpressionAttributeValues is not null).IsTrue(); + await Assert.That(request.ExpressionAttributeValues!.Count).IsEqualTo(1); + } + + #endregion + + #region UpdateItemBuilder Tests + + [Test] + public async Task UpdateItemBuilder_WithCondition_AttributeExists_DoesNotSetExpressionAttributeValues() + { + var builder = new UpdateItemBuilder("TestTable") + .WithKey("pk", "value") + .WithCondition(Condition.AttributeExists("lockToken")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_exists(#lockToken)"); + await Assert.That(request.ExpressionAttributeNames!["#lockToken"]).IsEqualTo("lockToken"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task UpdateItemBuilder_WithCondition_AttributeNotExists_DoesNotSetExpressionAttributeValues() + { + var builder = new UpdateItemBuilder("TestTable") + .WithKey("pk", "value") + .WithCondition(Condition.AttributeNotExists("lockToken")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_not_exists(#lockToken)"); + await Assert.That(request.ExpressionAttributeNames!["#lockToken"]).IsEqualTo("lockToken"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task UpdateItemBuilder_WithCondition_Equals_SetsExpressionAttributeValues() + { + var builder = new UpdateItemBuilder("TestTable") + .WithKey("pk", "value") + .WithCondition(Condition.Equals("version", 1)); + + var request = builder.Build(); + + await Assert.That(request.ExpressionAttributeValues is not null).IsTrue(); + await Assert.That(request.ExpressionAttributeValues!.Count).IsEqualTo(1); + } + + #endregion + + #region QueryBuilder WithKey Tests + + [Test] + public async Task QueryBuilder_WithKey_AttributeExists_DoesNotSetExpressionAttributeValues() + { + var builder = new QueryBuilder("TestTable") + .WithKey(Condition.AttributeExists("pk")); + + var request = builder.Build(); + + await Assert.That(request.KeyConditionExpression).IsEqualTo("attribute_exists(#pk)"); + await Assert.That(request.ExpressionAttributeNames!["#pk"]).IsEqualTo("pk"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task QueryBuilder_WithKey_AttributeNotExists_DoesNotSetExpressionAttributeValues() + { + var builder = new QueryBuilder("TestTable") + .WithKey(Condition.AttributeNotExists("pk")); + + var request = builder.Build(); + + await Assert.That(request.KeyConditionExpression).IsEqualTo("attribute_not_exists(#pk)"); + await Assert.That(request.ExpressionAttributeNames!["#pk"]).IsEqualTo("pk"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task QueryBuilder_WithKey_Equals_SetsExpressionAttributeValues() + { + var builder = new QueryBuilder("TestTable") + .WithKey(Condition.Equals("pk", "value")); + + var request = builder.Build(); + + await Assert.That(request.ExpressionAttributeValues is not null).IsTrue(); + await Assert.That(request.ExpressionAttributeValues!.Count).IsEqualTo(1); + } + + #endregion + + #region QueryBuilder WithFilter Tests + + [Test] + public async Task QueryBuilder_WithFilter_AttributeExists_DoesNotAddToExpressionAttributeValues() + { + var builder = new QueryBuilder("TestTable") + .WithKey(Condition.Equals("pk", "value")) + .WithFilter(Condition.AttributeExists("data")); + + var request = builder.Build(); + + await Assert.That(request.FilterExpression).IsEqualTo("attribute_exists(#data)"); + await Assert.That(request.ExpressionAttributeNames!["#data"]).IsEqualTo("data"); + // ExpressionAttributeValues should only have the key condition value, not filter + await Assert.That(request.ExpressionAttributeValues!.Count).IsEqualTo(1); + } + + [Test] + public async Task QueryBuilder_WithFilter_AttributeNotExists_DoesNotAddToExpressionAttributeValues() + { + var builder = new QueryBuilder("TestTable") + .WithKey(Condition.Equals("pk", "value")) + .WithFilter(Condition.AttributeNotExists("data")); + + var request = builder.Build(); + + await Assert.That(request.FilterExpression).IsEqualTo("attribute_not_exists(#data)"); + await Assert.That(request.ExpressionAttributeNames!["#data"]).IsEqualTo("data"); + // ExpressionAttributeValues should only have the key condition value, not filter + await Assert.That(request.ExpressionAttributeValues!.Count).IsEqualTo(1); + } + + [Test] + public async Task QueryBuilder_WithFilter_Only_AttributeExists_DoesNotSetExpressionAttributeValues() + { + // Use AttributeExists for both key and filter to test pure empty values scenario + var builder = new QueryBuilder("TestTable") + .WithKey(Condition.AttributeExists("pk")) + .WithFilter(Condition.AttributeExists("data")); + + var request = builder.Build(); + + await Assert.That(request.KeyConditionExpression).IsEqualTo("attribute_exists(#pk)"); + await Assert.That(request.FilterExpression).IsEqualTo("attribute_exists(#data)"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + #endregion + + #region ScanBuilder WithFilter Tests + + [Test] + public async Task ScanBuilder_WithFilter_AttributeExists_DoesNotSetExpressionAttributeValues() + { + var builder = new ScanBuilder("TestTable") + .WithFilter(Condition.AttributeExists("data")); + + var request = builder.Build(); + + await Assert.That(request.FilterExpression).IsEqualTo("attribute_exists(#data)"); + await Assert.That(request.ExpressionAttributeNames!["#data"]).IsEqualTo("data"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task ScanBuilder_WithFilter_AttributeNotExists_DoesNotSetExpressionAttributeValues() + { + var builder = new ScanBuilder("TestTable") + .WithFilter(Condition.AttributeNotExists("data")); + + var request = builder.Build(); + + await Assert.That(request.FilterExpression).IsEqualTo("attribute_not_exists(#data)"); + await Assert.That(request.ExpressionAttributeNames!["#data"]).IsEqualTo("data"); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task ScanBuilder_WithFilter_Equals_SetsExpressionAttributeValues() + { + var builder = new ScanBuilder("TestTable") + .WithFilter(Condition.Equals("status", "active")); + + var request = builder.Build(); + + await Assert.That(request.ExpressionAttributeValues is not null).IsTrue(); + await Assert.That(request.ExpressionAttributeValues!.Count).IsEqualTo(1); + } + + #endregion + + #region All Condition Methods Coverage Tests + + [Test] + public async Task Condition_NotEquals_HasExpressionValues() + { + var condition = Condition.NotEquals("attr", "value"); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_GreaterThan_HasExpressionValues() + { + var condition = Condition.GreaterThan("attr", 10); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_GreaterThanOrEquals_HasExpressionValues() + { + var condition = Condition.GreaterThanOrEquals("attr", 10); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_LessThan_HasExpressionValues() + { + var condition = Condition.LessThan("attr", 10); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_LessThanOrEquals_HasExpressionValues() + { + var condition = Condition.LessThanOrEquals("attr", 10); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_Between_HasExpressionValues() + { + var condition = Condition.Between("attr", 1, 10); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(2); + } + + [Test] + public async Task Condition_BeginsWith_HasExpressionValues() + { + var condition = Condition.BeginsWith("attr", "prefix"); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_Contains_HasExpressionValues() + { + var condition = Condition.Contains("attr", "value"); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_NotContains_HasExpressionValues() + { + var condition = Condition.NotContains("attr", "value"); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_SizeEquals_HasExpressionValues() + { + var condition = Condition.SizeEquals("attr", 5); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_SizeNotEquals_HasExpressionValues() + { + var condition = Condition.SizeNotEquals("attr", 5); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_SizeGreaterThan_HasExpressionValues() + { + var condition = Condition.SizeGreaterThan("attr", 5); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_SizeGreaterThanOrEquals_HasExpressionValues() + { + var condition = Condition.SizeGreaterThanOrEquals("attr", 5); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_SizeLessThan_HasExpressionValues() + { + var condition = Condition.SizeLessThan("attr", 5); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_SizeLessThanOrEquals_HasExpressionValues() + { + var condition = Condition.SizeLessThanOrEquals("attr", 5); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_AttributeType_HasExpressionValues() + { + var condition = Condition.AttributeType("attr", "S"); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_In_HasExpressionValues() + { + var condition = Condition.In("attr", "value1", "value2", "value3"); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(3); + } + + [Test] + public async Task Condition_AttributeExists_HasNoExpressionValues() + { + var condition = Condition.AttributeExists("attr"); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(0); + await Assert.That(condition.ExpressionNames.Count).IsEqualTo(1); + } + + [Test] + public async Task Condition_AttributeNotExists_HasNoExpressionValues() + { + var condition = Condition.AttributeNotExists("attr"); + await Assert.That(condition.ExpressionValues.Count).IsEqualTo(0); + await Assert.That(condition.ExpressionNames.Count).IsEqualTo(1); + } + + #endregion + + #region Composite Condition Tests + + [Test] + public async Task Condition_And_TwoConditions_CombinesValues() + { + var left = Condition.Equals("a", "1"); + var right = Condition.Equals("b", "2"); + var combined = Condition.And(left, right); + + await Assert.That(combined.ExpressionValues.Count).IsEqualTo(2); + await Assert.That(combined.ExpressionNames.Count).IsEqualTo(2); + } + + [Test] + public async Task Condition_And_WithEmptyValueCondition_HasPartialValues() + { + var left = Condition.Equals("a", "1"); + var right = Condition.AttributeExists("b"); + var combined = Condition.And(left, right); + + await Assert.That(combined.ExpressionValues.Count).IsEqualTo(1); + await Assert.That(combined.ExpressionNames.Count).IsEqualTo(2); + } + + [Test] + public async Task Condition_And_TwoEmptyValueConditions_HasNoValues() + { + var left = Condition.AttributeExists("a"); + var right = Condition.AttributeNotExists("b"); + var combined = Condition.And(left, right); + + await Assert.That(combined.ExpressionValues.Count).IsEqualTo(0); + await Assert.That(combined.ExpressionNames.Count).IsEqualTo(2); + } + + [Test] + public async Task Condition_And_ParamsArray_CombinesAll() + { + var combined = Condition.And( + Condition.Equals("a", "1"), + Condition.Equals("b", "2"), + Condition.AttributeExists("c") + ); + + await Assert.That(combined.ExpressionValues.Count).IsEqualTo(2); + await Assert.That(combined.ExpressionNames.Count).IsEqualTo(3); + } + + [Test] + public async Task Condition_Or_TwoConditions_CombinesValues() + { + var left = Condition.Equals("a", "1"); + var right = Condition.Equals("b", "2"); + var combined = Condition.Or(left, right); + + await Assert.That(combined.ExpressionValues.Count).IsEqualTo(2); + await Assert.That(combined.ExpressionNames.Count).IsEqualTo(2); + } + + [Test] + public async Task Condition_Or_WithEmptyValueCondition_HasPartialValues() + { + var left = Condition.Equals("a", "1"); + var right = Condition.AttributeExists("b"); + var combined = Condition.Or(left, right); + + await Assert.That(combined.ExpressionValues.Count).IsEqualTo(1); + await Assert.That(combined.ExpressionNames.Count).IsEqualTo(2); + } + + [Test] + public async Task Condition_Or_ParamsArray_CombinesAll() + { + var combined = Condition.Or( + Condition.Equals("a", "1"), + Condition.Equals("b", "2"), + Condition.AttributeNotExists("c") + ); + + await Assert.That(combined.ExpressionValues.Count).IsEqualTo(2); + await Assert.That(combined.ExpressionNames.Count).IsEqualTo(3); + } + + #endregion + + #region Builder with Composite Conditions Tests + + [Test] + public async Task PutItemBuilder_WithCondition_AndComposite_WithEmptyValues_DoesNotSetExpressionAttributeValues() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value" }) + .WithCondition(Condition.And( + Condition.AttributeExists("a"), + Condition.AttributeNotExists("b") + )); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_exists(#a) AND attribute_not_exists(#b)"); + await Assert.That(request.ExpressionAttributeNames!.Count).IsEqualTo(2); + await Assert.That(request.ExpressionAttributeValues is null).IsTrue(); + } + + [Test] + public async Task PutItemBuilder_WithCondition_AndComposite_WithMixedValues_SetsOnlyNonEmptyValues() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value" }) + .WithCondition(Condition.And( + Condition.AttributeNotExists("pk"), + Condition.Equals("version", 1) + )); + + var request = builder.Build(); + + await Assert.That(request.ExpressionAttributeNames!.Count).IsEqualTo(2); + await Assert.That(request.ExpressionAttributeValues is not null).IsTrue(); + await Assert.That(request.ExpressionAttributeValues!.Count).IsEqualTo(1); + } + + #endregion +} From 7100af5f6ddb207d4a60ff8464f4b0c4b88b40c7 Mon Sep 17 00:00:00 2001 From: "Stuart Blackler (@im5tu)" Date: Wed, 14 Jan 2026 11:18:28 +0000 Subject: [PATCH 2/3] PR Feedback --- .../DeleteItem/DeleteItemBuilder.cs | 8 +- .../Operations/PutItem/PutItemBuilder.cs | 15 ++- .../Operations/Query/QueryBuilder.cs | 3 +- .../UpdateItem/UpdateItemBuilder.cs | 15 ++- .../Serialization/DynamoJsonContext.cs | 2 + .../BuilderChainingTests.cs | 78 ++++++++++++- .../ConditionExpressionValuesTests.cs | 103 ++++++++++++++++++ 7 files changed, 219 insertions(+), 5 deletions(-) diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemBuilder.cs index e0ccf299..184d8f45 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/DeleteItem/DeleteItemBuilder.cs @@ -30,18 +30,24 @@ public DeleteItemBuilder WithKey(string attributeName, AttributeValue value) /// /// Sets a condition expression that must be satisfied for the delete operation to succeed. + /// Multiple conditions are combined with AND. /// /// The condition that must be met. /// The DeleteItemBuilder instance for method chaining. public DeleteItemBuilder WithCondition(Condition condition) { + if (string.IsNullOrEmpty(condition.Expression)) + { + return this; + } + if (string.IsNullOrEmpty(_request.ConditionExpression)) { _request.ConditionExpression = condition.Expression; } else { - _request.ConditionExpression += " AND " + condition.Expression; + _request.ConditionExpression = $"({_request.ConditionExpression}) AND ({condition.Expression})"; } if (condition.ExpressionNames.Count > 0) diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemBuilder.cs index 430949c2..4e2901d5 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/PutItem/PutItemBuilder.cs @@ -41,12 +41,25 @@ public PutItemBuilder WithAttribute(string attributeName, AttributeValue value) /// /// Sets a condition expression that must be satisfied for the put operation to succeed. + /// Multiple conditions are combined with AND. /// /// The condition that must be met. /// The PutItemBuilder instance for method chaining. public PutItemBuilder WithCondition(Condition condition) { - _request.ConditionExpression = condition.Expression; + if (string.IsNullOrEmpty(condition.Expression)) + { + return this; + } + + if (string.IsNullOrEmpty(_request.ConditionExpression)) + { + _request.ConditionExpression = condition.Expression; + } + else + { + _request.ConditionExpression = $"({_request.ConditionExpression}) AND ({condition.Expression})"; + } if (condition.ExpressionNames.Count > 0) { diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/Query/QueryBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/Query/QueryBuilder.cs index efaeb822..0a1e77b3 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/Query/QueryBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/Query/QueryBuilder.cs @@ -63,7 +63,8 @@ public QueryBuilder WithConsistentRead(bool consistentRead = true) /// /// The name of the index to query, or null to query the base table. /// The QueryBuilder instance for method chaining. - /// Thrown when indexName is not between 3-255 characters or contains invalid characters. + /// Thrown when indexName length is less than 3 or greater than 255 characters. + /// Thrown when indexName contains invalid characters (must match pattern [a-zA-Z0-9_.-]+). public QueryBuilder WithIndex(string? indexName) { if (string.IsNullOrWhiteSpace(indexName)) diff --git a/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemBuilder.cs b/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemBuilder.cs index 9c687b6a..b307c75a 100644 --- a/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemBuilder.cs +++ b/src/Clients/Goa.Clients.Dynamo/Operations/UpdateItem/UpdateItemBuilder.cs @@ -797,12 +797,25 @@ public UpdateItemBuilder RemoveFromNumberSet(string attributeName, params decima /// /// Sets a condition expression that must be satisfied for the update operation to succeed. + /// Multiple conditions are combined with AND. /// /// The condition that must be met. /// The UpdateItemBuilder instance for method chaining. public UpdateItemBuilder WithCondition(Condition condition) { - _request.ConditionExpression = condition.Expression; + if (string.IsNullOrEmpty(condition.Expression)) + { + return this; + } + + if (string.IsNullOrEmpty(_request.ConditionExpression)) + { + _request.ConditionExpression = condition.Expression; + } + else + { + _request.ConditionExpression = $"({_request.ConditionExpression}) AND ({condition.Expression})"; + } if (condition.ExpressionNames.Count > 0) { diff --git a/src/Clients/Goa.Clients.Dynamo/Serialization/DynamoJsonContext.cs b/src/Clients/Goa.Clients.Dynamo/Serialization/DynamoJsonContext.cs index d034a39c..ce125a46 100644 --- a/src/Clients/Goa.Clients.Dynamo/Serialization/DynamoJsonContext.cs +++ b/src/Clients/Goa.Clients.Dynamo/Serialization/DynamoJsonContext.cs @@ -59,7 +59,9 @@ namespace Goa.Clients.Dynamo.Serialization; [JsonSerializable(typeof(CapacityDetail))] [JsonSerializable(typeof(ReturnConsumedCapacity))] [JsonSerializable(typeof(ReturnValues))] +[JsonSerializable(typeof(ReturnValuesOnConditionCheckFailure))] [JsonSerializable(typeof(ReturnItemCollectionMetrics))] +[JsonSerializable(typeof(ItemCollectionMetrics))] [JsonSerializable(typeof(Select))] [JsonSerializable(typeof(Dictionary))] [JsonSerializable(typeof(List))] diff --git a/tests/Clients/Goa.Clients.Dynamo.Tests/BuilderChainingTests.cs b/tests/Clients/Goa.Clients.Dynamo.Tests/BuilderChainingTests.cs index 5488a9d2..0cd2fa87 100644 --- a/tests/Clients/Goa.Clients.Dynamo.Tests/BuilderChainingTests.cs +++ b/tests/Clients/Goa.Clients.Dynamo.Tests/BuilderChainingTests.cs @@ -3,8 +3,10 @@ using Goa.Clients.Dynamo.Operations; using Goa.Clients.Dynamo.Operations.Batch; using Goa.Clients.Dynamo.Operations.DeleteItem; +using Goa.Clients.Dynamo.Operations.PutItem; using Goa.Clients.Dynamo.Operations.Query; using Goa.Clients.Dynamo.Operations.Scan; +using Goa.Clients.Dynamo.Operations.UpdateItem; namespace Goa.Clients.Dynamo.Tests; @@ -106,7 +108,7 @@ public async Task DeleteItemBuilder_WithCondition_MultipleConditions_CombinesWit var request = builder.Build(); await Assert.That(request.ConditionExpression) - .IsEqualTo("attribute_exists(#lockToken) AND #version = :version"); + .IsEqualTo("(attribute_exists(#lockToken)) AND (#version = :version)"); await Assert.That(request.ExpressionAttributeNames!["#lockToken"]).IsEqualTo("lockToken"); await Assert.That(request.ExpressionAttributeNames!["#version"]).IsEqualTo("version"); await Assert.That(request.ExpressionAttributeValues![":version"].N).IsEqualTo("1"); @@ -217,4 +219,78 @@ public async Task DeleteItemBuilder_WithReturnConsumedCapacity_SetsCapacity() await Assert.That(request.ReturnConsumedCapacity).IsEqualTo(ReturnConsumedCapacity.INDEXES); await Assert.That(request.ReturnItemCollectionMetrics).IsEqualTo(ReturnItemCollectionMetrics.SIZE); } + + [Test] + public async Task DeleteItemBuilder_WithReturnValuesOnConditionCheckFailure_AllOld_SetsValue() + { + var builder = new DeleteItemBuilder("TestTable") + .WithKey("pk", new AttributeValue { S = "value1" }) + .WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD); + + var request = builder.Build(); + + await Assert.That(request.ReturnValuesOnConditionCheckFailure).IsEqualTo(ReturnValuesOnConditionCheckFailure.ALL_OLD); + } + + [Test] + public async Task DeleteItemBuilder_WithReturnValuesOnConditionCheckFailure_None_SetsValue() + { + var builder = new DeleteItemBuilder("TestTable") + .WithKey("pk", new AttributeValue { S = "value1" }) + .WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.NONE); + + var request = builder.Build(); + + await Assert.That(request.ReturnValuesOnConditionCheckFailure).IsEqualTo(ReturnValuesOnConditionCheckFailure.NONE); + } + + [Test] + public async Task PutItemBuilder_WithReturnValuesOnConditionCheckFailure_AllOld_SetsValue() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value1" }) + .WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD); + + var request = builder.Build(); + + await Assert.That(request.ReturnValuesOnConditionCheckFailure).IsEqualTo(ReturnValuesOnConditionCheckFailure.ALL_OLD); + } + + [Test] + public async Task PutItemBuilder_WithReturnValuesOnConditionCheckFailure_None_SetsValue() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value1" }) + .WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.NONE); + + var request = builder.Build(); + + await Assert.That(request.ReturnValuesOnConditionCheckFailure).IsEqualTo(ReturnValuesOnConditionCheckFailure.NONE); + } + + [Test] + public async Task UpdateItemBuilder_WithReturnValuesOnConditionCheckFailure_AllOld_SetsValue() + { + var builder = new UpdateItemBuilder("TestTable") + .WithKey("pk", new AttributeValue { S = "value1" }) + .Set("status", "active") + .WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD); + + var request = builder.Build(); + + await Assert.That(request.ReturnValuesOnConditionCheckFailure).IsEqualTo(ReturnValuesOnConditionCheckFailure.ALL_OLD); + } + + [Test] + public async Task UpdateItemBuilder_WithReturnValuesOnConditionCheckFailure_None_SetsValue() + { + var builder = new UpdateItemBuilder("TestTable") + .WithKey("pk", new AttributeValue { S = "value1" }) + .Set("status", "active") + .WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.NONE); + + var request = builder.Build(); + + await Assert.That(request.ReturnValuesOnConditionCheckFailure).IsEqualTo(ReturnValuesOnConditionCheckFailure.NONE); + } } diff --git a/tests/Clients/Goa.Clients.Dynamo.Tests/ConditionExpressionValuesTests.cs b/tests/Clients/Goa.Clients.Dynamo.Tests/ConditionExpressionValuesTests.cs index 987f8091..56315a9a 100644 --- a/tests/Clients/Goa.Clients.Dynamo.Tests/ConditionExpressionValuesTests.cs +++ b/tests/Clients/Goa.Clients.Dynamo.Tests/ConditionExpressionValuesTests.cs @@ -544,4 +544,107 @@ public async Task PutItemBuilder_WithCondition_AndComposite_WithMixedValues_Sets } #endregion + + #region Multiple Condition Parentheses Tests + + [Test] + public async Task UpdateItemBuilder_WithCondition_MultipleConditions_CombinesWithParentheses() + { + var builder = new UpdateItemBuilder("TestTable") + .WithKey("pk", "value") + .Set("status", "active") + .WithCondition(Condition.AttributeExists("lockToken")) + .WithCondition(Condition.Equals("version", 1)); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression) + .IsEqualTo("(attribute_exists(#lockToken)) AND (#version = :version)"); + } + + [Test] + public async Task PutItemBuilder_WithCondition_MultipleConditions_CombinesWithParentheses() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value" }) + .WithCondition(Condition.AttributeNotExists("pk")) + .WithCondition(Condition.Equals("version", 1)); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression) + .IsEqualTo("(attribute_not_exists(#pk)) AND (#version = :version)"); + } + + [Test] + public async Task DeleteItemBuilder_WithCondition_MultipleConditions_CombinesWithParentheses() + { + var builder = new DeleteItemBuilder("TestTable") + .WithKey("pk", "value") + .WithCondition(Condition.AttributeExists("lockToken")) + .WithCondition(Condition.Equals("version", 1)); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression) + .IsEqualTo("(attribute_exists(#lockToken)) AND (#version = :version)"); + } + + [Test] + public async Task UpdateItemBuilder_WithCondition_ThreeConditions_NestsParentheses() + { + var builder = new UpdateItemBuilder("TestTable") + .WithKey("pk", "value") + .Set("status", "active") + .WithCondition(Condition.AttributeExists("a")) + .WithCondition(Condition.AttributeExists("b")) + .WithCondition(Condition.AttributeExists("c")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression) + .IsEqualTo("((attribute_exists(#a)) AND (attribute_exists(#b))) AND (attribute_exists(#c))"); + } + + [Test] + public async Task UpdateItemBuilder_WithCondition_EmptyCondition_IsIgnored() + { + var builder = new UpdateItemBuilder("TestTable") + .WithKey("pk", "value") + .Set("status", "active") + .WithCondition(new Condition(string.Empty, [], [])) + .WithCondition(Condition.AttributeExists("lockToken")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_exists(#lockToken)"); + } + + [Test] + public async Task PutItemBuilder_WithCondition_EmptyCondition_IsIgnored() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value" }) + .WithCondition(new Condition(string.Empty, [], [])) + .WithCondition(Condition.AttributeNotExists("pk")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_not_exists(#pk)"); + } + + [Test] + public async Task DeleteItemBuilder_WithCondition_EmptyCondition_IsIgnored() + { + var builder = new DeleteItemBuilder("TestTable") + .WithKey("pk", "value") + .WithCondition(new Condition(string.Empty, [], [])) + .WithCondition(Condition.AttributeExists("lockToken")); + + var request = builder.Build(); + + await Assert.That(request.ConditionExpression).IsEqualTo("attribute_exists(#lockToken)"); + } + + #endregion } From 27e9cc5fe07fc1f0a254241d8ba08cca4ffe6780 Mon Sep 17 00:00:00 2001 From: "Stuart Blackler (@im5tu)" Date: Wed, 14 Jan 2026 11:25:47 +0000 Subject: [PATCH 3/3] PR Feedback --- .../BuilderChainingTests.cs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/Clients/Goa.Clients.Dynamo.Tests/BuilderChainingTests.cs b/tests/Clients/Goa.Clients.Dynamo.Tests/BuilderChainingTests.cs index 0cd2fa87..43fa2cf5 100644 --- a/tests/Clients/Goa.Clients.Dynamo.Tests/BuilderChainingTests.cs +++ b/tests/Clients/Goa.Clients.Dynamo.Tests/BuilderChainingTests.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using Goa.Clients.Dynamo.Enums; using Goa.Clients.Dynamo.Models; using Goa.Clients.Dynamo.Operations; @@ -7,6 +8,7 @@ using Goa.Clients.Dynamo.Operations.Query; using Goa.Clients.Dynamo.Operations.Scan; using Goa.Clients.Dynamo.Operations.UpdateItem; +using Goa.Clients.Dynamo.Serialization; namespace Goa.Clients.Dynamo.Tests; @@ -293,4 +295,44 @@ public async Task UpdateItemBuilder_WithReturnValuesOnConditionCheckFailure_None await Assert.That(request.ReturnValuesOnConditionCheckFailure).IsEqualTo(ReturnValuesOnConditionCheckFailure.NONE); } + + [Test] + public async Task DeleteItemBuilder_WithReturnValuesOnConditionCheckFailure_AllOld_SerializesCorrectly() + { + var builder = new DeleteItemBuilder("TestTable") + .WithKey("pk", new AttributeValue { S = "value1" }) + .WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD); + + var request = builder.Build(); + var json = JsonSerializer.Serialize(request, DynamoJsonContext.Default.DeleteItemRequest); + + await Assert.That(json).Contains("\"ReturnValuesOnConditionCheckFailure\":\"ALL_OLD\""); + } + + [Test] + public async Task PutItemBuilder_WithReturnValuesOnConditionCheckFailure_AllOld_SerializesCorrectly() + { + var builder = new PutItemBuilder("TestTable") + .WithItem(new Dictionary { ["pk"] = "value1" }) + .WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD); + + var request = builder.Build(); + var json = JsonSerializer.Serialize(request, DynamoJsonContext.Default.PutItemRequest); + + await Assert.That(json).Contains("\"ReturnValuesOnConditionCheckFailure\":\"ALL_OLD\""); + } + + [Test] + public async Task UpdateItemBuilder_WithReturnValuesOnConditionCheckFailure_AllOld_SerializesCorrectly() + { + var builder = new UpdateItemBuilder("TestTable") + .WithKey("pk", new AttributeValue { S = "value1" }) + .Set("status", "active") + .WithReturnValuesOnConditionCheckFailure(ReturnValuesOnConditionCheckFailure.ALL_OLD); + + var request = builder.Build(); + var json = JsonSerializer.Serialize(request, DynamoJsonContext.Default.UpdateItemRequest); + + await Assert.That(json).Contains("\"ReturnValuesOnConditionCheckFailure\":\"ALL_OLD\""); + } }