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

DynamoDBv2: Document.FromAttributeMap Fails to Handle M or L if NULL is set to false #1887

Closed
david-beckman opened this issue Jul 23, 2021 · 5 comments
Labels
bug This issue is a bug. dynamodb p2 This is a standard priority issue queued

Comments

@david-beckman
Copy link

david-beckman commented Jul 23, 2021

Steps to reproduce:

  1. Populate a attribute map Dictionary<string, AttributeValue> such that at least one of the AttributeValue instances has M or L set. Also update said AttributeValue such that NULL is false. An easy way to do that is via JSON Deserialization:
{
    "Account": {
        "B": null,
        "BOOL": false,
        "IsBOOLSet": false,
        "BS": [],
        "L": [],
        "IsLSet": false,
        "M": {
            "Name": {
                "B": null,
                "BOOL": false,
                "IsBOOLSet": false,
                "BS": [],
                "L": [],
                "IsLSet": false,
                "M": {},
                "IsMSet": false,
                "N": null,
                "NS": [],
                "NULL": false,
                "S": "Test Account",
                "SS": []
            }
        },
        "IsMSet": true,
        "N": null,
        "NS": [],
        "NULL": false,
        "S": null,
        "SS": []
    }
}
  1. Create a Document from this attribute map via Document.FromAttributeMap

Expected Behavior:
The value is set to the correct Document instance

Actual Behavior:
The value is set to DynamoDBNull

Technical Details
In the AttributeValueToDynamoDBEntry method, TryToDynamoDBNull is called prior to TryToDynamoDBList or TryToDocument and it is returning DynamoDBNull incorrectly. The call to attributeValue.IsSetNULL should be attributeValue.NULL instead.

@ashishdhingra ashishdhingra added bug This issue is a bug. dynamodb needs-reproduction This issue needs reproduction. labels Jul 23, 2021
@david-beckman
Copy link
Author

david-beckman commented Jul 23, 2021

Here is a unit test that will expose the issue:

#r "nuget:AWSSDK.DynamoDBv2/3.7.0.45"
#r "nuget:xunit/2.4.1"

using System.Text.Json;

using Amazon.DynamoDBv2.DocumentModel;
using Amazon.DynamoDBv2.Model;

using Xunit;

var mapJson = @"{
    ""Account"": {
        ""B"": null,
        ""BOOL"": false,
        ""IsBOOLSet"": false,
        ""BS"": [],
        ""L"": [],
        ""IsLSet"": false,
        ""M"": {
            ""Name"": {
                ""B"": null,
                ""BOOL"": false,
                ""IsBOOLSet"": false,
                ""BS"": [],
                ""L"": [],
                ""IsLSet"": false,
                ""M"": {},
                ""IsMSet"": false,
                ""N"": null,
                ""NS"": [],
                ""NULL"": false,
                ""S"": ""Test Account"",
                ""SS"": []
            }
        },
        ""IsMSet"": true,
        ""N"": null,
        ""NS"": [],
        ""NULL"": false,
        ""S"": null,
        ""SS"": []
    }
}";

var map = JsonSerializer.Deserialize<Dictionary<string, AttributeValue>>(mapJson);
var doc = Document.FromAttributeMap(map);

Assert.NotNull(doc);
Assert.True(doc.ContainsKey("Account"));
Assert.NotNull(doc["Account"]);

var accountDoc = doc["Account"].AsDocument();
Assert.NotNull(accountDoc);
Assert.True(accountDoc.ContainsKey("Name"));
Assert.Equal("Test Account", accountDoc["Name"]);

@ashishdhingra
Copy link
Contributor

ashishdhingra commented Jul 23, 2021

@david-beckman I'm able to see that Document with Key Account is returned with value Amazon.DynamoDBv2.DocumentModel.DynamoDBNull, i.e., there is it is not of type Amazon.DynamoDBv2.DocumentModel.Document. Removing "NULL": false from Account key works fine.

@ashishdhingra
Copy link
Contributor

Also revisit #1980 since it appears to be a related issue.

@hunanniu hunanniu added A and removed B labels Mar 25, 2022
@ashishdhingra ashishdhingra added p2 This is a standard priority issue and removed A labels Nov 1, 2022
@david-beckman
Copy link
Author

I re-ran that unit test above and it now passes; it looks like it was fixed between 3.7.3.23 & 3.7.3.24. I am closing this issue.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

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. dynamodb p2 This is a standard priority issue queued
Projects
None yet
Development

No branches or pull requests

3 participants