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

Exception is thrown when deserializing nullable strongly-typed id #36

Closed
hankovich opened this issue Jul 5, 2021 · 4 comments
Closed

Comments

@hankovich
Copy link

hankovich commented Jul 5, 2021

[StronglyTypedId]
public partial struct GroupId
{
}

public class User
{
    public User(GroupId? groupId)
    {
        GroupId = groupId;
    }

    public GroupId? GroupId { get; }
}

var json = JsonConvert.SerializeObject(new User(groupId: null)); // "{\"GroupId\":null}"
var deserialize = JsonConvert.DeserializeObject<User>(json); // throws Newtonsoft.Json.JsonSerializationException: 'Error converting value {null} to type 'System.Guid'. Path 'GroupId', line 1, position 15.'

The issue is caused by Newtonsoft.Json's behaviour (ReadJson is called on GroupIdNewtonsoftJsonConverter with objectType = Nullable<GroupId>).

A couple of related issues from Newtonsoft.Json:
JamesNK/Newtonsoft.Json#1783
JamesNK/Newtonsoft.Json#2219

Version of StronglyTypeId: 0.2.1
Version of Newtonsoft.Json: tried with both 12.0.3 and 13.0.1

@mvarendorff2
Copy link

Just hit this as well today; I would be willing to implement a fix to this @andrewlock but I have no idea where to start, honestly. Could you give me a pointer or two? Or would that effort not be worth it given that you are working on a 1.x version of this package?

@andrewlock
Copy link
Owner

I believe this is fixed in the latest beta: https://www.nuget.org/packages/StronglyTypedId/1.0.0-beta02, would you mind giving that a try? Make sure to check out this post for breaking changes etc: https://andrewlock.net/rebuilding-stongly-typed-id-as-a-source-generator-1-0-0-beta-release/#breaking-changes

@mvarendorff2
Copy link

mvarendorff2 commented Nov 23, 2021

Hey Andrew, thanks for the quick response! Unfortunately it seems that the issue is not fixed in that version. On top of that the deserializing code uses int for deserializing for a long backed id.

Starting from:

using StronglyTypedIds;

namespace ExampleIds {
    [StronglyTypedId(StronglyTypedIdBackingType.Long, StronglyTypedIdConverter.NewtonsoftJson)]
    public partial struct ExampleId { }
}

the generated code of the ReadJson function (rest omitted for brevity) looks like this:

public override object ReadJson(Newtonsoft.Json.JsonReader reader, System.Type objectType, object existingValue, Newtonsoft.Json.JsonSerializer serializer)
{
    return new ExampleId(serializer.Deserialize<int>(reader));
}

while I would expect it to be something more like this (using long? for deserializing and returning null in the case of a null value read):

public override object? ReadJson(Newtonsoft.Json.JsonReader reader, System.Type objectType, object existingValue, Newtonsoft.Json.JsonSerializer serializer)
{
    var deserializeResult = serializer.Deserialize<long?>(reader);
    return deserializeResult == null ? null : new ExampleId(deserializeResult.Value);
}

@andrewlock
Copy link
Owner

Hey @mvarendorff, you're absolutely right, this is still outstanding.

I've been working on it this evening, and I've fixed the bug in 13f91b1.

I haven't released a new package yet, as there's some other issues I want to tackle at the same time (namely #38, and upgrading to .NET 6 incremental generators), but I should have a new version out soon

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

No branches or pull requests

3 participants