DynamoDB AttributeValue roundtrip of empty number or string sets

[danielmarbach]

Describe the bug

When serializing content with empty Lists (L) or empty Maps (M) it is possible to force those by manually setting IsLSet or IsMSet to true which then wraps things internally in an AlwaysSendList or AlwaysSendDictionary. This is necessary to allow setting list and map types to an empty value without running into

Amazon.DynamoDBv2.AmazonDynamoDBException : Supplied AttributeValue is empty, must contain exactly one of the supported datatypes

Unfortunately, it is not possible today to use empty string sets (SS), number sets (NS) or binary sets (BS) since they have no public way of enforcing something like IsSSSet. The SDK currently has equivalent internal methods that make a bunch of assumptions that seem incompatible with setting those sets to empty values.

When you try to set any of these types to an empty set type, the following exception is thrown

Amazon.DynamoDBv2.AmazonDynamoDBException : Supplied AttributeValue is empty, must contain exactly one of the supported datatypes

Expected Behavior

Having the possibility to explicitly set/write sets of strings, sets of numbers or sets of binary data to empty.

Current Behavior

A

Amazon.DynamoDBv2.AmazonDynamoDBException : Supplied AttributeValue is empty, must contain exactly one of the supported datatypes

is thrown

Reproduction Steps

Set either SS, BS or NS to an empty list and try to write it against dynamodb.

Possible Solution

No response

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

Latest

Targeted .NET Platform

All

Operating System and version

All

[danielmarbach]

I think it would make sense to align on the attribute value the SS, BS and NS property with corresponding IsXYSet properties and extend the InteralSDKUtils to support ISet<T> types. This could be done in a non-breaking way.

[normj]

In V4 the IsSet properties lose the meaningfulness because the collections will default to null instead of empty. Then the SDK can figure out if something is set depending if the property is null or not.

[danielmarbach]

I understand that but we have this problem with 3.x

[normj]

Sorry I thought you were talking about V4.

If you want to submit a PR to add the IsSet you basically add the properties to this customization file and then run the generator. The generator is in this solution and then you have the ServiceClientGenerator project as the startup project and run it. In the PR you only include the customization file not the generated code.

[danielmarbach]

@normj why is this being treated as a feature and not a bug? It is currently not possible to roundtrip empty sets as outlined in the description.

[DavidBoike]

@normj I'm one of @danielmarbach's colleagues. I have raised #3299 which I hope properly implements your suggestion.

[normj]

@DavidBoike Your PR looks good. I'll try and get this out soon.

@danielmarbach My logic was yes it is an architecture bug in the SDK with how SDK handles collections and the ability to know if a collection has a value or not. We are addressing the bug in V4 by changing the default of collections to be null. We aren't going to add IsSet methods all over the public API in V3 so adding customizations for specific properties I view as a feature request. I agree though it could be either way and I will get @DavidBoike PR out as soon as I can.

[normj]

Version 3.7.302.23 of the AWSSDK.DynamoDBv2 package went out today with @DavidBoike PR. You can see the generated code checked in as well here: https://github.com/aws/aws-sdk-net/blob/main/sdk/src/Services/DynamoDBv2/Generated/Model/AttributeValue.cs#L428

I believe that solves this issue and closing the issue.

[github-actions]

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

[danielmarbach]

@normj I think this issue needs to be reopened.

Using the latest SDK you pushed yesterday the service doesn't accept empty NS, BS or SS values. The simplest possible repro is:

        var client = new DynamoDBClient();
        await client.BatchWriteItemAsync(new BatchWriteItemRequest(new Dictionary<string, List<WriteRequest>>
        {
            {
                "SomeTable", [
                    new(new PutRequest(new Dictionary<string, AttributeValue>
                    {
                        { "PK", new AttributeValue(Guid.NewGuid().ToString()) },
                        { "SK", new AttributeValue(Guid.NewGuid().ToString()) },
                        { "Data", new AttributeValue { NS = [], IsNSSet = true } }
                    }))
                ]
            }
        }));

that blows with

Amazon.DynamoDBv2.AmazonDynamoDBException: One or more parameter values were invalid: An number set  may not be empty
 ---> Amazon.Runtime.Internal.HttpErrorResponseException: Exception of type 'Amazon.Runtime.Internal.HttpErrorResponseException' was thrown.
   at Amazon.Runtime.HttpWebRequestMessage.ProcessHttpResponseMessage(HttpResponseMessage responseMessage)
   at Amazon.Runtime.HttpWebRequestMessage.GetResponseAsync(CancellationToken cancellationToken)
   at Amazon.Runtime.Internal.HttpHandler`1.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.Unmarshaller.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.ErrorHandler.InvokeAsync[T](IExecutionContext executionContext)
   --- End of inner exception stack trace ---
   at Amazon.Runtime.Internal.HttpErrorResponseExceptionHandler.HandleExceptionStream(IRequestContext requestContext, IWebResponseData httpErrorResponse, HttpErrorResponseException exception, Stream responseStream)
   at Amazon.Runtime.Internal.HttpErrorResponseExceptionHandler.HandleExceptionAsync(IExecutionContext executionContext, HttpErrorResponseException exception)
   at Amazon.Runtime.Internal.ExceptionHandler`1.HandleAsync(IExecutionContext executionContext, Exception exception)
   at Amazon.Runtime.Internal.ErrorHandler.ProcessExceptionAsync(IExecutionContext executionContext, Exception exception)
   at Amazon.Runtime.Internal.ErrorHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.CallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.Signer.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.EndpointDiscoveryHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.EndpointDiscoveryHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.CredentialsRetriever.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.RetryHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.RetryHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.CallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.CallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.ErrorCallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.MetricsHandler.InvokeAsync[T](IExecutionContext executionContext)
   at NServiceBus.PersistenceTesting.When_saving_saga_with_empty_list.ShouldSave() in /home/danielmarbach/Projects/NServiceBus.Persistence.DynamoDB/src/NServiceBus.Persistence.DynamoDB.PersistenceTests/When_saving_saga_with_empty_list.cs:line 18

Here is a PR that shows the same on our end

Particular/NServiceBus.Persistence.DynamoDB#604

[normj]

@danielmarbach I should have remembered when you initially asked about adding the IsSet methods but on the service side it doesn't support empty sets for an AttributeValue. So the IsSet methods added probably have little value other then consistency. Here is the line from the PutItem API reference docs: https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_PutItem.html#DDB-PutItem-request-Item

Empty String and Binary attribute values are allowed. Attribute values of type String and Binary must 
have a length greater than zero if the attribute is used as a key attribute for a table or index. Set 
type attributes cannot be empty.

[danielmarbach]

@normj

Empty String and Binary attribute values are allowed. Attribute values of type String and Binary must have a length greater than zero if the attribute is used as a key attribute for a table or index.

That to me, reads as they are allowed in the case above because I'm not using it as a PK or SK key nor do I have an index defined. Or in other words: This reads to me as empty values are allowed unless you use them as SK, PK or in an index. Can you clarify?

[normj]

@danielmarbach I think the doc is written confusingly joining 2 different restrictions. Sets and the other collection types can not even be used as the PK or SK. The line Set type attributes cannot be empty. is independent of the beginning part talking about how empty string and binary attributes are not allowed for the PK and SK.