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

Switch to JSON.NET #1940

Closed
ErazerBrecht opened this issue Aug 2, 2021 · 6 comments
Closed

Switch to JSON.NET #1940

ErazerBrecht opened this issue Aug 2, 2021 · 6 comments

Comments

@ErazerBrecht
Copy link

ErazerBrecht commented Aug 2, 2021

Hello,

This feels more like a question that an actual issue.
But didn't know for sure where to ask otherwise

These are early days for EF Core JSON support, and you'll likely run into some limitations. Please let us know how the current features are working for you and what you'd like to see.

So I'm playing a bit with the JSON support and I really like it. There is one problem I'm having since internally the 'new' System.Text.Json api is used there is no support for polymorphic serialization and deserialization (dotnet/runtime#30083)

I could easily solve this in the following way:

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<RootEntity>()
                .Property(e => e.ChildrenA)
                .HasConversion(
                    v => JsonConvert.SerializeObject(v, JsonSettings.JsonSerializerSettings),
                    v => JsonConvert.DeserializeObject<ImmutableList<AbstractClassA>>(v, JsonSettings.JsonSerializerSettings));
            
            modelBuilder.Entity<RootEntity>()
                .Property(e => e.ChildrenB)
                .HasConversion(
                    v => JsonConvert.SerializeObject(v, JsonSettings.JsonSerializerSettings),
                    v => JsonConvert.DeserializeObject<ImmutableList<AbstractClassB>>(v, JsonSettings.JsonSerializerSettings));
        }
    }

    public static class JsonSettings
    {
        public static readonly JsonSerializerSettings JsonSerializerSettings = new JsonSerializerSettings
        {
            TypeNameHandling = TypeNameHandling.Auto,
            MetadataPropertyHandling = MetadataPropertyHandling.ReadAhead
        };
    }

This perfectly works. But it's a bit tedious to always add this in the model creation.
Is there a better solution like globally replacing System.Text.Json to good 'ol JSON.NET?

Btw I'm using the preview of EF Core 6.

Thanks in advance,
Sincerely, Brecht

@roji
Copy link
Member

roji commented Aug 2, 2021

One main reason why JSON.NET wasn't implemented, was that we don't want to add a reference to JSON.NET from EFCore.PG itself - that would force it upon users and potentially created dependency hell situations (System.Text.Json has the advantage of being built into the framework). EF Core does have a plugin model which allows having external nugets which add the reference, but the JSON support requires certain advanced functionality which isn't possible from a plugin.

Then there's the burden of maintaining two JSON implementations rather than just one... System.Text.Json has generally been improving and closing the gap, so this doesn't really seem like a good way forward.

@ErazerBrecht
Copy link
Author

ErazerBrecht commented Aug 2, 2021

@roji
I completely understand that and System.Text.Json is also the way forward. It's a bit a bummer it will take until .NET 7 before polymorphism is supported.

I guess there is no easy way to override it globally. Or be able to create a custom attribute?

Thx in advance!

@roji
Copy link
Member

roji commented Aug 2, 2021

I guess there is no easy way to override it globally. Or be able to create a custom attribute?

I honestly don't know System.Text.Json well enough...

@NinoFloris
Copy link
Member

There should be converter samples in those issues (though deserialization is a different matter) that you could copy paste into your app but I think that all hinges on #1107?

@ErazerBrecht
Copy link
Author

@NinoFloris Hmm I'll check out 'GlobalTypeMapper' tomorrow, certainly worth a try.

Thx!

@roji roji closed this as completed Aug 2, 2021
@ErazerBrecht
Copy link
Author

@NinoFloris I confirm the NpgsqlConnection.GlobalTypeMapper does the job for now!
Thx for suggesting this!

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