Pomelo.EntityFrameworkCore.MySql
Pomelo.EntityFrameworkCore.MySql copied to clipboard
Pomelo for Newtonsoft.Json is not using serialization settings when generating property paths in MySQL Functions that search JSON values
Steps to reproduce
using Pomelo and Newtonsoft.Json if you have an entity with a column marked as json when querying for a field in this column, Pomelo generates a query that uses a MySQL function to deal with JSON values but when the path to the property is generated it does not take into account that the properties may be being serialized in a format other than the default. Pomelo does take into account the JsonProperty attribute but with Newtonsoft.Json you can also configure globally how you want to serialize property names (for example in camelCase) and it is not being taken into account
EXAMPLE
Given:
- the entity:
public class Person
{
public int Id { get; set; }
public string Name { get; set; }
public DataContainer Container { get; set; }
}
public class IntData
{
public int IntValue { get; set; }
}
public class StringData
{
public string StringValue { get; set; }
}
public class DataContainer
{
public IntData IntData { get; set; }
public StringData StringData { get; set; }
}
- the entity configuration:
modelBuilder.Entity<Person>().HasKey(x => x.Id);
modelBuilder.Entity<Person>().Property(x => x.Container).HasColumnType("json");
modelBuilder.Entity<Person>().ToTable("People");
- and this configuration
JsonConvert.DefaultSettings = () =>
{
var settings = new JsonSerializerSettings();
settings.Converters.Add(new Newtonsoft.Json.Converters.StringEnumConverter());
settings.NullValueHandling = NullValueHandling.Include;
settings.ContractResolver = new CamelCasePropertyNamesContractResolver(); // <-----
settings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore;
settings.DateTimeZoneHandling = DateTimeZoneHandling.Local;
return settings;
};
services.AddDbContext<TestDbContext>(o =>
o
.UseMySql(Constants.MySqlDbCnx, ServerVersion.AutoDetect(Constants.MySqlDbCnx), mySqlOpts =>
{
mySqlOpts.UseNewtonsoftJson();
mySqlOpts.MigrationsAssembly(typeof(Program).Assembly.GetName().Name);
})
.LogTo(Console.WriteLine, LogLevel.Information)
.EnableSensitiveDataLogging()
.EnableDetailedErrors()
);
an entity instance like:
var p1 = new Person
{
Id = 1,
Name = "Person 1",
Container = new DataContainer
{
IntData = new IntData { IntValue = 34 },
StringData = new StringData { StringValue = "hi" }
}
};
will be saved on the db like:
| Id | Name | Container |
|----|----------|--------------------------------------------------------------------|
| 1 | Person 1 | {"intData": {"intValue": 34}, "stringData": {"stringValue": "hi"}} |
(note the camelCase usage in property names)
the query:
var p1 = await db.Set<Person>().FirstAsync(x => x.Container.StringData.StringValue == "h1")
is translated to:
SELECT `p`.`Id`, `p`.`Container`, `p`.`Name`
FROM `People` AS `p`
WHERE JSON_EXTRACT(`p`.`Container`, '$.StringData.StringValue') = 'hi'
LIMIT 1
and will return 0 rows in response, wrongly, due to '$.StringData.StringValue'
if we change the definition of the entities by adding the JsonProperty attribute it works fine, but it is very inconvenient and goes against the idea of defining this behavior globally
given:
public class IntData
{
[JsonProperty("intValue")]
public int IntValue { get; set; }
}
public class StringData
{
[JsonProperty("stringValue")]
public string StringValue { get; set; }
}
public class DataContainer
{
[JsonProperty("intData")]
public IntData IntData { get; set; }
[JsonProperty("stringData")]
public StringData StringData { get; set; }
}
the serialization of the entity will be the same and the query is translated to:
SELECT `p`.`Id`, `p`.`Container`, `p`.`Name`
FROM `People` AS `p`
WHERE JSON_EXTRACT(`p`.`Container`, '$.stringData.stringValue') = 'hi'
LIMIT 1
and it will return 1 row
also if we do not use the global configuration
/*
JsonConvert.DefaultSettings = () =>
{
var settings = new JsonSerializerSettings();
settings.Converters.Add(new Newtonsoft.Json.Converters.StringEnumConverter());
settings.NullValueHandling = NullValueHandling.Include;
settings.ContractResolver = new CamelCasePropertyNamesContractResolver();
settings.ReferenceLoopHandling = ReferenceLoopHandling.Ignore;
settings.DateTimeZoneHandling = DateTimeZoneHandling.Local;
return settings;
};
*/
the entity will be saved like:
| Id | Name | Container |
|----|----------|--------------------------------------------------------------------|
| 1 | Person 1 | {"IntData": {"IntValue": 34}, "StringData": {"StringValue": "hi"}} |
and the query is translated to:
SELECT `p`.`Id`, `p`.`Container`, `p`.`Name`
FROM `People` AS `p`
WHERE JSON_EXTRACT(`p`.`Container`, '$.StringData.StringValue') = 'hi'
LIMIT 1
retrieving 1 row as expected
The issue
If Newtonsoft.Json is configured globally, Pomelo does not use the serialization convention for property names defined there when generating the paths for MySQL Json functions, resulting in invalid queries and/or not returning the expected values
Suggestion
I reviewed the Pomelo source code a bit and although I can't assimilate it 100%, I have seen that somewhere, for example, in MySqlJsonPocoTranslator you do:
https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/27f227388a5e1b68292f6c649227bdf0637598d7/src/EFCore.MySql/Query/ExpressionTranslators/Internal/MySqlJsonPocoTranslator.cs#L45-L48
and
https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/27f227388a5e1b68292f6c649227bdf0637598d7/src/EFCore.MySql.Json.Newtonsoft/Query/Internal/MySqlJsonNewtonsoftPocoTranslator.cs#L20-L21
so, maybe you could check with the Newtonsoft.Json global configuration like this:
public override string GetJsonPropertyName(MemberInfo member)
{
var attributeName = member.GetCustomAttribute<JsonPropertyAttribute>()?.PropertyName;
if (!string.IsNullOrWhiteSpace(attributeName))
return attributeName;
if (JsonConvert.DefaultSettings?.Invoke()?.ContractResolver is DefaultContractResolver resolver)
return resolver.GetResolvedPropertyName(member.Name);
return new DefaultContractResolver().GetResolvedPropertyName(member.Name);
}
Further technical details
MySQL version: 8.0.32-mysql (docker image) Operating system: windows (host), linux (container) Pomelo.EntityFrameworkCore.MySql version: 7.0.0 Pomelo.EntityFrameworkCore.MySql.Json.Newtonsoft: 7.0.0 Microsoft.AspNetCore.App version: 6.0.18
PS: I attached a code where this error can be reproduced : PomeloTest.zip
any news about this?
Hi @trejjam, do you think this will be solved with #1827 ?
My PR is only extending System.Text.Json support.
But I do not know if it fixes query building. It probably can be fixed using configuration from my PR
@jjavierdguezas First of all, great issue report!
I think we should allow users to configure default JSON settings via the model, and then allow them to override those for entity properties or using the MySqlJsonNewtonsoftPocoValueConverter<T>.
There might be other areas in an app (or even within a DbContext model) that need to use JSON (de-/)serialization with different settings.