efcore.pg icon indicating copy to clipboard operation
efcore.pg copied to clipboard

Serialization options for System.Text.Json support

Open brendan-nobadthing opened this issue 5 years ago • 52 comments

Hi,

I'm looking to serialize a model containing Nodatime types into a jsonb data field. Right now this serializes a whole bunch of individual fields from the nodatime type. I see a couple of days ago, jon skeet published a beta for system.text.json serlialisation support for nodatime: https://www.nuget.org/packages/NodaTime.Serialization.SystemTextJson

This looks like just the thing I'm looking for but I'm not sure how to wire this config into the serialization used by npgsql & EF core.

  • the new system.text.serlializer doe not seem to have the same global default engine that NewtonSoft provided.
  • And I've yet to find any configurable interface to the serializer used by npgsql.

Any ideas / suggestions? Thanks!

brendan-nobadthing avatar Nov 04 '19 08:11 brendan-nobadthing

You're correct - in this initial version of JSON support there aren't any hooks for configuring the System.Text.Json serializer. This isn't trivial to do, but I agree it's definitely important.

roji avatar Nov 04 '19 17:11 roji

Hi @roji Thanks for the reply.

After a bit of digging around, I managed to find a workaround. Once I realised that the actual serialization was being done by JsonHandler way down in NpgSql itself, I was able to refer to how the nodatime handlers and mappers are configured to come up with this:

var origJsonbMapping =
    NpgsqlConnection.GlobalTypeMapper.Mappings.Single(m => m.NpgsqlDbType == NpgsqlDbType.Jsonb);
NpgsqlConnection.GlobalTypeMapper.RemoveMapping(origJsonbMapping.PgTypeName);
NpgsqlConnection.GlobalTypeMapper.AddMapping(new NpgsqlTypeMappingBuilder
{
    PgTypeName = origJsonbMapping.PgTypeName,
    NpgsqlDbType = origJsonbMapping.NpgsqlDbType,
    DbTypes = origJsonbMapping.DbTypes,
    ClrTypes = origJsonbMapping.ClrTypes,
    InferredDbType = origJsonbMapping.InferredDbType,
    TypeHandlerFactory = new JsonbHandlerFactory(new JsonSerializerOptions()
        .ConfigureForNodaTime(DateTimeZoneProviders.Serialization))
}.Build());

Works so far for a simple "save stuff, and get the same values back" Integration test. Will report back here if I get any issues once applied to my project.

brendan-nobadthing avatar Nov 10 '19 10:11 brendan-nobadthing

@brendan-ssw great :) Note that this will change the ADO-level serialization options, so documents will be read and written with these options. However, EF Core also generates JSON property names when traversing into a JSON column in a LINQ query, and that's not taken care of by the above.

roji avatar Nov 25 '19 19:11 roji

This is definitely a desired feature for those using their own Converters registered via JsonSerializerOptions.

@brendan-ssw solution works accepting the caveat about query EFFunctions, thanks

sitepodmatt avatar Jan 03 '20 11:01 sitepodmatt

Perhaps the Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime plugin should take a dependency on NodaTime.Serialization.SystemTextJson and configure the JSON Serializer for this scenario by default?

This might be a good interim step that could be released sooner than full configuration options for the JSON serializer.

space-alien avatar May 08 '20 07:05 space-alien

@space-alien I don't think that makes sense - not all users of the NodaTime plugin use System.Text.Json, so there shouldn't be a dependency there. More importantly, the NodaTime plugin wouldn't actually be involved in the serialization in any way, only in the configuration, which again doesn't seem right.

Fortunately there's at least a workaround as @brendan-ssw posted above, but other than that the provider simply needs to expose a way to configure the JSON serializer...

roji avatar May 08 '20 08:05 roji

My thinking was more the other way round - or maybe just back to front, depending on one's perspective..!😅

The NodaTime.Serialization.SystemTextJson package can be thought of as the NodaTime "default/official/recommended" config for System.Text.Json, just delivered as a separate package.

All users of json columns are implicitly relying on System.Text.Json, since that's the built-in JSON serializer in efcore.pg.

So, for a user of the NodaTime efcore.pg plugin, who maps a complex model property to a json column, the built-in NodaTime configuration for System.Text.Json is an appropriate and intuitive default.

If their complex property contains NodaTime types, it's highly likely they will want the JSON serializer to adopt the NodaTime default configuration. If this default configuration is not applied, it's all but certain they will need to apply serializer configuration manually, when this could probably have been avoided.

The Nodatime efcore.pg plugin does not take any responsibility for serialization in this setup. I don't think the plugin's dependency on NodaTime.Serialization.SystemTextJson would add anything unwarranted, since we're already reliant on both of that package's sub-dependencies, namely NodaTime and System.Text.Json.

Of course, this is a separate concern to exposing wider configuration options for the JSON serializer, and a user of the NodaTime plugin could still override any default configuration.

TL;DR: Without bringing in any new sub-dependencies, the Nodatime plugin could apply NodaTime "default" config to the built-in JSON serializer.

(I think! 😀)

space-alien avatar May 08 '20 10:05 space-alien

All users of json columns are implicitly relying on System.Text.Json, since that's the built-in JSON serializer in efcore.pg.

That's true, but System.Text.Json is part of the framework and not an additional external dependencies (since netcoreapp3.0). That's one good reason it exists within EFCore.PG and not as a plugin.

TL;DR: Without bringing in any new sub-dependencies, the Nodatime plugin could apply NodaTime "default" config to the built-in JSON serializer.

How would that work? Wouldn't the NodaTime plugin (Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime) need to depend on NodaTime.Serialization.SystemTextJson? If not, how do the methods of the latter get called?

roji avatar May 08 '20 11:05 roji

Ah, so for netcoreapp3.0 this would actually introduce a dependency on System.Text.Json through NodaTime.Serialization.SystemTextJson, as the latter only targets netstandard2.0?

If that's the case, would this be solved if NodaTime.Serialization.SystemTextJson added a target for netcoreapp3.0?

space-alien avatar May 08 '20 12:05 space-alien

I'm not sure I understand... When targeting netcoreapp3.0, there's no dependency (i.e. nuget package) needed to use System.Text.Json - it's in the box, just like System.Text. I thought you were proposing that Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime take a dependency on NodaTime.Serialization.SystemTextJson in order to configure Npgsql's JSON serialization options to use it...

roji avatar May 08 '20 20:05 roji

I thought you were proposing that Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime take a dependency on NodaTime.Serialization.SystemTextJson in order to configure Npgsql's JSON serialization options to use it...

Yes, I am!

I thought that this wouldn't add any other new dependencies because:

  • NodaTime.Serialization.SystemTextJson only depends on NodaTime and System.Text.Json.
  • And Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime already depends on these two - NodaTime directly; and System.Text.Json is down in Npgsql.

Since NodaTime.Serialization.SystemTextJson can be thought of an 'official' NodaTime package, I felt it would be reasonable to add it.

I'm not too hot on this targeting stuff so perhaps I have misunderstood your earlier comments. Would it not be the case that a user would either be targeting netstandard (in which case Npgsql brings in System.Text.Json), or netcoreapp3.0 (in which case it's in the box, as you say)? I'm sorry if I have misunderstood how this works.

space-alien avatar May 08 '20 21:05 space-alien

My problem is with making Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime depend on NodaTime.Serialization.SystemTextJson, since it wouldn't actually need that package or use it for anything except configuring JSON serialization options. Its current role is to change the mappings of of PostgreSQL date/time types (outside of JSON!) to NodaTime CLR types instead of the built-in BCL types - that's quite different and unrelated.

Also, there are other non-NodaTime needs to configuration System.Text.Json serialization, so something would have to be exposed in any case - once that happens, it doesn't seem like much to ask the user to take a dependency on NodaTime.Serialization.SystemTextJson themselves and do the proper setup.

I'm sorry if I have misunderstood how this works.

No worries at all! This is a good conversation.

roji avatar May 08 '20 21:05 roji

Yes, this makes sense! Thanks for taking the time to discuss this.

space-alien avatar May 08 '20 21:05 space-alien

Punting this out of 5.0, I'm hoping to do a big work cycle on cross-database JSON support, this should be part of that.

roji avatar Aug 29 '20 11:08 roji

Is there already a way to have the EF Core provider resolve the JsonSerializerOptions from the service provider? This issue is agnostic to NodaTime. It's a general EF Core JSON handling one.

IMHO GlobalTypeMapper should be deprecated and replaced with the options pattern, or at least some of it, so that it can be easily configured at app startup stage and used across the app with the DI container.

weitzhandler avatar Nov 26 '20 03:11 weitzhandler

@weitzhandler not all applications are ASP.NET applications, or even DI applications, so requiring the options pattern wouldn't really do. There are also internal performance considerations which favor a global/static approach (such as GlobalTypeMapper) over something that would use DI.

What actual issues do you see with calling a GlobalTypeMapper on application startup?

roji avatar Nov 26 '20 08:11 roji

Thanks for your explanation, convinced me.

weitzhandler avatar Nov 26 '20 15:11 weitzhandler

For our application it would be useful to provide options, too. It would especially be nice if we could somehow query case insensitive for jsonb properties, but I don't know how feasible this is.

mg90707 avatar Jan 20 '21 13:01 mg90707

Any update on this?

In my case, all the domain entities polluted with JsonPropertyName even though it is an infrastructure concern. I have configured Text.Json with CamelCase policy, but this https://github.com/efcore/EFCore.NamingConventions/issues/65#issuecomment-776023203 suggests that the workaround is not feasible.

sreejith-ms avatar Apr 27 '21 11:04 sreejith-ms

Not yet. This is something I'd like to get around to, but haven't had the time.

roji avatar Apr 27 '21 12:04 roji

@roji Is this something up-for-grabs/good-first-issue? Then I can take a look.

sreejith-ms avatar Apr 27 '21 14:04 sreejith-ms

Not entirely sure... The challenge is that there are two places where JSON field names are generated:

  1. When saving/loading documents (serialization). This is managed by System.Text.Json at the ADO.NET level of Npgsql.
  2. When generating query SQL which contains JSON field names (e.g. Where(x => x.SomeJson.Foo == 'bar'). EF Core would need to apply the naming convention itself in the SQL generation phase, probably.

These two layers need to be aligned around the naming convention - it's probably not trivial.

Note that this is really unrelated to EFCore.NamingConventions - that plugin is about table/column names, whereas here we're discussing fields names within JSON documents (which are themselves stored in a column).

roji avatar Apr 27 '21 16:04 roji

@roji It looks not trivial at all. I'm curious about the solution, will this be a part of EntityTypeBuilder or a separate fluent style API that can be used by both EFCore and ADO.NET?

sreejith-ms avatar Apr 28 '21 16:04 sreejith-ms

I don't know yet - designing the solution is part of what needs to be done :) The complexity is unfortunately the main reason I haven't done it yet...

roji avatar Apr 28 '21 20:04 roji

Here's a sample for working around this in 6.0.0 (similar to this above):

var options = new JsonSerializerOptions
{
    // Customize based on needs...
};
NpgsqlConnection.GlobalTypeMapper.AddTypeResolverFactory(new JsonOverrideTypeHandlerResolverFactory(options));

await using var conn = new NpgsqlConnection("Host=localhost;Username=test;Password=test");
await conn.OpenAsync();

using var cmd = new NpgsqlCommand("SELECT $1::text", conn);
cmd.Parameters.Add(new()
{
    Value = new Blog
    {
        Title = "foo",
    }
});
Console.WriteLine(cmd.ExecuteScalar());

class JsonOverrideTypeHandlerResolverFactory : TypeHandlerResolverFactory
{
    private readonly JsonSerializerOptions _options;

    public JsonOverrideTypeHandlerResolverFactory(JsonSerializerOptions options)
        => _options = options;

    public override TypeHandlerResolver Create(NpgsqlConnector connector)
        => new JsonOverrideTypeHandlerResolver(connector, _options);

    public override string? GetDataTypeNameByClrType(Type clrType)
        => null;

    public override TypeMappingInfo? GetMappingByDataTypeName(string dataTypeName)
        => null;

    class JsonOverrideTypeHandlerResolver : TypeHandlerResolver
    {
        readonly JsonHandler _jsonbHandler;

        internal JsonOverrideTypeHandlerResolver(NpgsqlConnector connector, JsonSerializerOptions options)
            => _jsonbHandler ??= new JsonHandler(
                connector.DatabaseInfo.GetPostgresTypeByName("jsonb"),
                connector.TextEncoding,
                isJsonb: true,
                options);

        public override NpgsqlTypeHandler? ResolveByDataTypeName(string typeName)
            => typeName == "jsonb" ? _jsonbHandler : null;

        public override NpgsqlTypeHandler? ResolveByClrType(Type type)
            // You can add any user-defined CLR types which you want mapped to jsonb
            => type == typeof(JsonDocument) || type == typeof(Blog)
                ? _jsonbHandler
                : null;

        public override TypeMappingInfo? GetMappingByDataTypeName(string dataTypeName)
            => null; // Let the built-in resolver do this
    }
}

class Blog
{
    public string? Title { get; set; }
}

roji avatar Oct 17 '21 13:10 roji

Note: this depends on https://github.com/npgsql/npgsql/pull/4045, which was merged after 6.0.0. Use version 6.0.0-rtm-ci.20211017T140717 or higher from the vNext nuget feed.

roji avatar Oct 17 '21 14:10 roji

@roji thanks for a workaround. it's working fine when writing values to the database but when building a query like this

var joes = context.CustomerEntries
    .Where(e => e.Customer.Name == "Joe")
    .ToList();

it's converted to

SELECT c.""Id"", c.""Customer""
FROM ""CustomerEntries"" AS c
WHERE c.""Customer""->>'Name' = 'Joe'

instead of

SELECT c.""Id"", c.""Customer""
FROM ""CustomerEntries"" AS c
WHERE c.""Customer""->>'name' = 'Joe'

Is there a way to also apply camelCase when referencing json fields in linq query?

llRandom avatar Jan 05 '22 11:01 llRandom

@llRandom no - as written above this only works for storing and loading, but not for query SQL generation.

roji avatar Jan 05 '22 11:01 roji

@roji, is this issue still on track for EF Core 7? I would like to store c# class object in jsonb using:

new JsonSerializerOptions
            { DefaultIgnoreCondition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull,
PropertyNamingPolicy = JsonNamingPolicy.CamelCase }

Our use-case (c# class) has a lot of nullable props (100s) and 99% of them are always null. If we can eliminate null props, this would greatly reduce database size.

leonkosak avatar Jul 10 '22 11:07 leonkosak

This is unlikely to make it in for 7.0, it also depends on ongoing JSON work being done in EF Core itself.

But if you just need to specify the options for serialization, you can use the workaround I detailed above.

roji avatar Jul 10 '22 13:07 roji