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

System.Text.Json and dynamic types #15762

Closed
gvkries opened this issue Apr 15, 2024 · 2 comments · Fixed by #15816
Closed

System.Text.Json and dynamic types #15762

gvkries opened this issue Apr 15, 2024 · 2 comments · Fixed by #15816
Labels
Milestone

Comments

@gvkries
Copy link
Contributor

gvkries commented Apr 15, 2024

Describe the bug

Bug #15718 reveals issues that arise with the upgrade to System.Text.Json when using dynamic typing. Previously, Newtonsoft.Json handled many automatic conversions for JSON values, facilitating the use of dynamic typing with various target types. For example, in

, content field values could be cast to DateTime.

With the new dynamic support added by @jtkech, these conversions are absent. All values are returned as parsed by System.Text.Json and converted to an object, which only returns types defined by JsonValueKind. This behavior is implemented here: JsonDynamicValue.cs#L28.

This change breaks all dynamic usages with specific target types.

To Reproduce

See #15718 for steps how to reproduce this error, but the underlaying problem is not limited to GraphQL.

As another example, the following unit test passes with Newtonsoft.Jsonbut fails now with STJ:

[Fact]
public void ShouldDeserializeDateTimeFields()
{
    var contentItem = new ContentItem();
    contentItem.GetOrCreate<MyPart>();
    contentItem.Alter<MyPart>(x => x.Text = "test");
    contentItem.Alter<MyPart>(x =>
    {
        x.GetOrCreate<MyDateTimeField>("myField");
        x.Alter<MyDateTimeField>("myField", f => f.Value = new DateTime(2024, 1, 1, 10, 42, 0));
    });

    var json = JsonConvert.SerializeObject(contentItem);

    var contentItem2 = JConvert.DeserializeObject<ContentItem>(json);

    Assert.NotNull(contentItem2.Content.MyPart);
    Assert.NotNull(contentItem2.Content.MyPart.myField);
    Assert.Equal(new DateTime(2024, 1, 1, 10, 42, 0), (DateTime?)contentItem2.Content.MyPart.myField.Value);
}

public class MyDateTimeField : ContentField
{
    public DateTime? Value { get; set; }
}

Expected behavior

To maintain compatibility and not disrupt existing functionality that uses dynamic types, conversions similar to those provided by Newtonsoft.Json should still be possible.

@Piedone Piedone added this to the 1.9 milestone Apr 15, 2024
@hishamco
Copy link
Member

I remember @hyzx86 did some PRs for STJ with dynamic, right?

@hyzx86
Copy link
Contributor

hyzx86 commented Apr 17, 2024

I remember @hyzx86 did some PRs for STJ with dynamic, right?

Yes, but that is not include DateTime value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants