Skip to content
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

DynamoDb record deserialization does not work with empty Map values #1888

Closed
1 task
danny-zegel-zocdoc opened this issue Nov 27, 2024 · 6 comments
Closed
1 task
Labels
bug This issue is a bug. module/lambda-client-lib needs-review p2 This is a standard priority issue

Comments

@danny-zegel-zocdoc
Copy link

Describe the bug

When using DefaultLambdaJsonSerializer to deserialize DynamoDb events that have nested Map type attributes that are empty it parses them AttributeValue objects that don't have any value set.

var serializer = new Amazon.Lambda.Serialization.SystemTextJson.DefaultLambdaJsonSerializer();
var dynamoEvent = serializer.Deserialize<Amazon.DynamoDBv2.Model.Record>(memoryStream);

for example the following payload

{
    "awsRegion": "us-east-1",
    "eventID": "9b997d0a-9512-46b7-a239-093431b36204",
    "eventName": "MODIFY",
    "userIdentity": null,
    "recordFormat": "application/json",
    "dynamodb": {
        "ApproximateCreationDateTime": 1732710435144,
        "Keys": {
            "PK": {
                "S": "my pk value"
            },
            "SK": {
                "S": "my sk value"
            }
        },
        "NewImage": {
            "MyData": {
                "M": {
                    "Selections": {
                        "M": {}
                    }
                }
            },
            "SK": {
                "S": "my sk value"
            },
            "PK": {
                "S": "my pk value"
            }
        },
        "OldImage": { },
        "SizeBytes": 30
    },
    "eventSource": "aws:dynamodb"
}

deserialized Selections to an AttributeValue object with nothing set
Screenshot 2024-11-27 at 4 24 43 PM

Regression Issue

  • Select this option if this issue appears to be a regression.

Expected Behavior

I would expect DefaultLambdaJsonSerializer to Deserialize empty Maps to AttributeValue objects with IsMSet set to true.

Current Behavior

DefaultLambdaJsonSerializer Deserializes empty Maps to AttributeValue objects with IsMSet set to false.

Reproduction Steps

var json = """
           {
               "awsRegion": "us-east-1",
               "eventID": "9b997d0a-9512-46b7-a239-093431b36204",
               "eventName": "MODIFY",
               "userIdentity": null,
               "recordFormat": "application/json",
               "dynamodb": {
                   "ApproximateCreationDateTime": 1732710435144,
                   "Keys": {
                       "PK": {
                           "S": "my pk value"
                       },
                       "SK": {
                           "S": "my sk value"
                       }
                   },
                   "NewImage": {
                       "MyData": {
                           "M": {
                               "Selections": {
                                   "M": {}
                               }
                           }
                       },
                       "SK": {
                           "S": "my sk value"
                       },
                       "PK": {
                           "S": "my pk value"
                       }
                   },
                   "OldImage": { },
                   "SizeBytes": 30
               },
               "eventSource": "aws:dynamodb"
           }
           """;
var serializer = new Amazon.Lambda.Serialization.SystemTextJson.DefaultLambdaJsonSerializer();
var dynamoEvent = serializer.Deserialize<Amazon.DynamoDBv2.Model.Record>(new MemoryStream(Encoding.UTF8.GetBytes(json)));
dynamoEvent.Dynamodb.NewImage["MyData"].M["Selections"]!.IsMSet.Should().BeTrue();

Possible Solution

No response

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

Amazon.Lambda.Serialization.SystemTextJson Version 2.4.4
AWSSDK.DynamoDBv2 Version 3.7.301.10

Targeted .NET Platform

.NET 8

Operating System and version

macOS Somona

@danny-zegel-zocdoc danny-zegel-zocdoc added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 27, 2024
@ashishdhingra ashishdhingra added module/lambda-client-lib p2 This is a standard priority issue investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Nov 27, 2024
@ashishdhingra
Copy link
Contributor

@danny-zegel-zocdoc Good afternoon. I used the below code for reproducing the issue:

public void FunctionHandlerNew(DynamoDBEvent input, ILambdaContext context)
{
    context.Logger.LogLine("OK");
}

with the below event payload:

{
  "Records": [
    {
               "awsRegion": "us-east-1",
               "eventID": "9b997d0a-9512-46b7-a239-093431b36204",
               "eventName": "MODIFY",
               "userIdentity": null,
               "recordFormat": "application/json",
               "dynamodb": {
                   "ApproximateCreationDateTime": 1732710435144,
                   "Keys": {
                       "PK": {
                           "S": "my pk value"
                       },
                       "SK": {
                           "S": "my sk value"
                       }
                   },
                   "NewImage": {
                       "MyData": {
                           "M": {
                               "Selections": {
                                   "M": {}
                               }
                           }
                       },
                       "SK": {
                           "S": "my sk value"
                       },
                       "PK": {
                           "S": "my pk value"
                       }
                   },
                   "OldImage": { },
                   "SizeBytes": 30
               },
               "eventSource": "aws:dynamodb"
           }
  ]
}

Inspecting the value of input.Records[0].Dynamodb.NewImage["MyData"].M["Selections"] shows the following in Immediate window:

{Amazon.Lambda.DynamoDBEvents.DynamoDBEvent.AttributeValue}
    B: null
    BOOL: null
    BS: null
    L: null
    M: Count = 0
    N: null
    NS: null
    NULL: null
    S: null
    SS: null

The dependency on SDK was removed in 4329775 where it now uses it's own Amazon.Lambda.DynamoDBEvents.DynamoDBEvent.AttributeValue POCO class (similar to other event packages in this repository).

I notice that you are using Amazon.DynamoDBv2.Model.Record from the AWS SDK for .NET instead. This leverages AWS .NET SDK auto-generated Amazon.DynamoDBv2.Model.Record model class and accessing AttributeValue.IsMSet would use Amazon.Util.Internal.InternalSDKUtils.GetIsSet(), which returns true only when the Dictionary is not empty.

Are you expecting IsMSet to return true only because collection M is initialized to 0 elements? If yes, do you expect M to be set as null since it had 0 elements in input JSON. I tried setting AWSConfigs.InitializeCollections = false; before execution, it still initialized collection to empty collection. This initialization is handled by JsonSerializer.

I'm unsure if we should be returning IsMSet value as true in case on empty collection (even in upcoming v4 version of AWS .NET SDK). CC @normj for inputs.

Thanks,
Ashish

@ashishdhingra ashishdhingra added needs-review and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Nov 27, 2024
@danny-zegel-zocdoc
Copy link
Author

danny-zegel-zocdoc commented Nov 27, 2024

Thanks you for the response @ashishdhingra

Being that DynamoDb does support empty Map values (unlike string set as DDB does not support empty sets) I would expect the SDK to respect it as well and differentiate between nulls and empty dictionaries.

In regards to the AttributeValue implementation, it does contain the IsMSet property which is backed by InternalSDKUtils.GetIsSet and InternalSDKUtils.SetIsSet which utilizes the AlwaysSendDictionary type to differentiate between empty Map value and unset Map values.

Seems to me there are a couple easy options for the fix:

  1. Define a JsonConverter for AttributeValue which would invoke IsMSet = true when deserializing the M typed AttributeValue object when the json has { "M": { } }, and add the converter to DefaultLambdaJsonSerializer.
  2. Update the setter method for AttributeValue.M to either call SetIsSet or perhaps just wrap the dictionary it receives in AlwaysSendDictionary.

just my 2 cents but the first option i described is likely less risky but the second one would make this work as expected out of the box for arbitrary serializers (say someone wanted to use Newtonsoft or whatever).

This also seems to be a problem for AttributeValue.L, AttributeValue.BS & AttributeValue.NS

@danny-zegel-zocdoc
Copy link
Author

@ashishdhingra regarding your point about the dependency changes and using Amazon.Lambda.DynamoDBEvents.DynamoDBEvent.AttributeValue, i see that if I use that type it does deserialize properly and in fact Amazon.DynamoDBv2.Model.Record seems equivalent to Amazon.Lambda.DynamoDBEvents.DynamoDBEvent.DynamodbStreamRecord.

However I'm using the deserialized output with Amazon.DynamoDBv2.DocumentModel.Document.FromAttributeMap() (in conjunction with DynamoDBContext.FromDocument()) which expects Dictionary<string, Amazon.DynamoDBv2.Model.AttributeValue> and not Dictionary<string, Amazon.Lambda.DynamoDBEvents.DynamoDBEvent.AttributeValue> so deserializing to the Amazon.Lambda.DynamoDBEvents types doesn't exactly work for my use-case.

@danny-zegel-zocdoc
Copy link
Author

danny-zegel-zocdoc commented Nov 27, 2024

Seems to me there are a couple easy options for the fix:

  1. Define a JsonConverter for AttributeValue to invoke IsMSet = true when deserializing the M types AttributeValue object when the json has { "M": { } }, and add the converter to DefaultLambdaJsonSerializer.

If such a JsonConverter were just made available I could add it to an instance of DefaultLambdaJsonSerializer myself even if it's not built it into DefaultLambdaJsonSerializer by default.

@normj
Copy link
Member

normj commented Dec 21, 2024

Closing up on this issue. The types in the SDK like Amazon.DynamoDBv2.Model.AttributeValue were never meant to be used for Lambda event serialization. They are generated specifically with corresponding SDK marshallers for how communicating back and forth with the service directly. Sometimes the SDK types match close enough to the Lambda event that they are used but there is no guarantee.

Due to the SDK's pattern of initializing all collection properties to an empty collection we have to have all of the IsSet properties which are managed by the SDK's marshallers. When using a plan Json serializer it wouldn't know how to handle the IsSet properties. In the upcoming V4 of the SDK we are changing the SDK's behavior to have collection properties initialized to null. In that case the IsSet logic is simpler by just checking if the property is null. You can try the upcoming V4 behavior by setting the property AWSConfigs.InitializeCollections to false.

There are no plans to focus on making SDK types compatible as Lambda events. That is a contract we are not comfortable making. What it sounds like you need is for use to make some progress on this issue requesting a FromJson or some mechanism for converting from the Lambda event to the SDK type. I would suggest adding your plus/comment there to help prioritize that work. #1700

@normj normj closed this as completed Dec 21, 2024
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. module/lambda-client-lib needs-review p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

3 participants