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

Support custom JsonSerializerOptions in EFCore.MySql.Json.Microsoft #1827

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Storage.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.Extensions.DependencyInjection;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Infrastructure;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Infrastructure.Internal;

namespace Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Design.Internal
{
Expand All @@ -26,6 +28,7 @@ public class MySqlJsonMicrosoftDesignTimeServices : IDesignTimeServices
/// </summary>
public virtual void ConfigureDesignTimeServices(IServiceCollection serviceCollection)
=> serviceCollection
.AddSingleton<IMysqlJsonOptions, DefaultMysqlJsonOptions>()
.AddSingleton<IRelationalTypeMappingSourcePlugin, MySqlJsonMicrosoftTypeMappingSourcePlugin>()
.AddSingleton<IProviderCodeGeneratorPlugin, MySqlJsonMicrosoftCodeGeneratorPlugin>();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Storage.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Infrastructure;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Infrastructure.Internal;
using Pomelo.EntityFrameworkCore.MySql.Query.ExpressionTranslators.Internal;

// ReSharper disable once CheckNamespace
Expand All @@ -33,7 +35,9 @@ public static class MySqlJsonMicrosoftServiceCollectionExtensions
.TryAdd<IMethodCallTranslatorPlugin, MySqlJsonMicrosoftMethodCallTranslatorPlugin>()
.TryAdd<IMemberTranslatorPlugin, MySqlJsonMicrosoftMemberTranslatorPlugin>()
.TryAddProviderSpecificServices(
x => x.TryAddScopedEnumerable<IMySqlJsonPocoTranslator, MySqlJsonMicrosoftPocoTranslator>());
x => x
.TryAddSingleton<IMysqlJsonOptions, DefaultMysqlJsonOptions>()
.TryAddScopedEnumerable<IMySqlJsonPocoTranslator, MySqlJsonMicrosoftPocoTranslator>());

return serviceCollection;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) Pomelo Foundation. All rights reserved.
// Licensed under the MIT. See LICENSE in the project root for license information.

using System.Text.Json;

namespace Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Infrastructure;

public interface IMysqlJsonOptions

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a casing issue; shouldn't it be IMySqlJsonOptions for consistency?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change it

{
JsonSerializerOptions JsonSerializerOptions { get; }
}
Comment on lines +8 to +11
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not convinced that this is the right approach. I think users should have the ability to set JsonSerializerOptions using the model, not injecting them via a service.

They should be able to set a default on the model itself and then also concrete options on (JSON) properties of entities (that override the default options). They also need to be supplied in a JSON provider specific fashion (since we support Microsoft and Newtonsoft JSON implementations). So this requires JSON provider specific extension methods.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
using System.Text.Json;

namespace Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Infrastructure.Internal;

public sealed class DefaultMysqlJsonOptions : IMysqlJsonOptions
{
public JsonSerializerOptions JsonSerializerOptions { get; init; } = new();
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Pomelo.EntityFrameworkCore.MySql.Infrastructure.Internal;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Infrastructure;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Storage.ValueComparison.Internal;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Storage.ValueConversion.Internal;
using Pomelo.EntityFrameworkCore.MySql.Storage.Internal;
Expand All @@ -20,10 +21,14 @@ public class MySqlJsonMicrosoftTypeMappingSourcePlugin : MySqlJsonTypeMappingSou
private static readonly Lazy<MySqlJsonMicrosoftJsonElementValueConverter> _jsonElementValueConverter = new Lazy<MySqlJsonMicrosoftJsonElementValueConverter>();
private static readonly Lazy<MySqlJsonMicrosoftStringValueConverter> _jsonStringValueConverter = new Lazy<MySqlJsonMicrosoftStringValueConverter>();

private readonly IMysqlJsonOptions _mysqlJsonOptions;

public MySqlJsonMicrosoftTypeMappingSourcePlugin(
[NotNull] IMySqlOptions options)
[NotNull] IMySqlOptions options,
[NotNull] IMysqlJsonOptions mysqlJsonOptions)
: base(options)
{
_mysqlJsonOptions = mysqlJsonOptions;
}

protected override Type MySqlJsonTypeMappingType => typeof(MySqlJsonMicrosoftTypeMapping<>);
Expand Down Expand Up @@ -64,7 +69,8 @@ protected override ValueConverter GetValueConverter(Type clrType)
}

return (ValueConverter)Activator.CreateInstance(
typeof(MySqlJsonMicrosoftPocoValueConverter<>).MakeGenericType(clrType));
typeof(MySqlJsonMicrosoftPocoValueConverter<>).MakeGenericType(clrType),
_mysqlJsonOptions.JsonSerializerOptions);
}

protected override ValueComparer GetValueComparer(Type clrType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ namespace Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Storage.ValueConversio
{
public class MySqlJsonMicrosoftPocoValueConverter<T> : ValueConverter<T, string>
{
public MySqlJsonMicrosoftPocoValueConverter()
public MySqlJsonMicrosoftPocoValueConverter(JsonSerializerOptions jsonSerializerOptions)
: base(
v => ConvertToProviderCore(v),
v => ConvertFromProviderCore(v))
v => ConvertToProviderCore(v, jsonSerializerOptions),
v => ConvertFromProviderCore(v, jsonSerializerOptions))
{
}

private static string ConvertToProviderCore(T v)
=> JsonSerializer.Serialize(v);
private static string ConvertToProviderCore(T v, JsonSerializerOptions jsonSerializerOptions)
=> JsonSerializer.Serialize(v, jsonSerializerOptions);

private static T ConvertFromProviderCore(string v)
=> JsonSerializer.Deserialize<T>(v);
private static T ConvertFromProviderCore(string v, JsonSerializerOptions jsonSerializerOptions)
=> JsonSerializer.Deserialize<T>(v, jsonSerializerOptions);
Comment on lines +12 to +23
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure at the moment, that we want to force the use of a jsonSerializerOptions parameter. We might want to make it optional, falling back on the default options.

}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.EntityFrameworkCore.Design.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Storage.Json;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
using Pomelo.EntityFrameworkCore.MySql.Internal;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Infrastructure;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Infrastructure.Internal;
using Pomelo.EntityFrameworkCore.MySql.Json.Microsoft.Storage.Internal;
using Pomelo.EntityFrameworkCore.MySql.Storage.Internal;
using Xunit;
Expand All @@ -29,6 +32,36 @@ public void GenerateSqlLiteral_returns_json_object_literal()
@"]}'", literal);
}

[Fact]
public void GenerateSqlLiteral_returns_json_object_literal_customJsonOptions()
{
var jsonSerializerOptions = new JsonSerializerOptions();
jsonSerializerOptions.Converters.Add(new BoolJsonConverter());

var literal = CreateMySqlTypeMappingSource(new DefaultMysqlJsonOptions { JsonSerializerOptions = jsonSerializerOptions })
.FindMapping(typeof(Customer), "json").GenerateSqlLiteral(SampleCustomer);
Assert.Equal(
"""
'{"Name":"Joe","Age":25,"IsVip":0,"Orders":[{"Price":99.5,"ShippingAddress":"Some address 1","ShippingDate":"2019-10-01T00:00:00"},{"Price":23.1,"ShippingAddress":"Some address 2","ShippingDate":"2019-10-10T00:00:00"}]}'
""",
literal);

}

/// <summary>
/// POC converter, verify that custom JsonSerializerOptions is being used
/// </summary>
private sealed class BoolJsonConverter : JsonConverter<bool>
{
public override bool Read(
ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options
) => reader.TokenType is JsonTokenType.Number && reader.GetInt32() == 1;

public override void Write(
Utf8JsonWriter writer, bool value, JsonSerializerOptions options
) => writer.WriteNumberValue(value ? 1 : 0);
}

[Fact]
public void GenerateSqlLiteral_returns_json_document_literal()
{
Expand Down Expand Up @@ -98,16 +131,22 @@ public class Order

#region Support

private static readonly MySqlTypeMappingSource Mapper = new MySqlTypeMappingSource(
private static MySqlTypeMappingSource CreateMySqlTypeMappingSource(
IMysqlJsonOptions mysqlJsonOptions
) => new MySqlTypeMappingSource(
new TypeMappingSourceDependencies(
new ValueConverterSelector(new ValueConverterSelectorDependencies()),
new JsonValueReaderWriterSource(new JsonValueReaderWriterSourceDependencies()),
Array.Empty<ITypeMappingSourcePlugin>()),
new RelationalTypeMappingSourceDependencies(
new [] {new MySqlJsonMicrosoftTypeMappingSourcePlugin(new MySqlOptions())}),
new[] { new MySqlJsonMicrosoftTypeMappingSourcePlugin(new MySqlOptions(), mysqlJsonOptions) }),
new MySqlOptions()
);

private static readonly MySqlTypeMappingSource Mapper = CreateMySqlTypeMappingSource(
new DefaultMysqlJsonOptions()
);

private static RelationalTypeMapping GetMapping(string storeType) => Mapper.FindMapping(storeType);

private static RelationalTypeMapping GetMapping(Type clrType) => (RelationalTypeMapping)Mapper.FindMapping(clrType);
Expand Down