Fix for some minor bugs around JSON mappings

#29214 - Json column: constructor with optional enum-parameter throws ArgumentException

When producing shaper code for json nullable properties with converters we check if the converter handles null, if not we need to peek into the json value and only pass it thru converter if it's not null, and return null otherwise

#29217 - Json: nullable enums on entities mapped to json don't use enum-to-int converter, rather than string, like for non-nullable enums

In convention code unwrap nullable type when testing for enum properties that we then apply converter to string

#29225 - Json: projecting nullable scalar produces debug.assert in SqlExpression ctor

Unwrap nullable type from property before passing it as type to SqlExpression ctor

#29219 - Json: error when trying to materialize json entity with nullable property that is missing in the json string - should materialize the property as null instead

Modify ExtractJsonProperty method so that it used TryGetProperty when trying to extract nullable properties.

Fixes #29214
Fixes #29217
Fixes #29225
Fixes #29219

src/EFCore.Relational/Metadata/Conventions/RelationalMapToJsonConvention.cs

@@ -70,7 +70,7 @@ public virtual void ProcessModelFinalizing(
     {
         foreach (var jsonEntityType in modelBuilder.Metadata.GetEntityTypes().Where(e => e.IsMappedToJson()))
         {
-            foreach (var enumProperty in jsonEntityType.GetDeclaredProperties().Where(p => p.ClrType.IsEnum))
+            foreach (var enumProperty in jsonEntityType.GetDeclaredProperties().Where(p => p.ClrType.UnwrapNullableType().IsEnum))
             {
                 // by default store enums as strings - values should be human-readable
                 enumProperty.Builder.HasConversion(typeof(string));

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ClientMethods.cs

@@ -125,12 +125,19 @@ private static TValue ThrowExtractJsonPropertyException<TValue>(
                 exception);
         }
 
-        private static object? ExtractJsonProperty(JsonElement element, string propertyName, Type returnType)
-        {
-            var jsonElementProperty = element.GetProperty(propertyName);
-
-            return jsonElementProperty.Deserialize(returnType);
-        }
+        private static T? ExtractJsonProperty<T>(JsonElement element, string propertyName, bool nullable)
+            => nullable
+                ? element.TryGetProperty(propertyName, out var jsonValue)
+                    ? jsonValue.Deserialize<T>()
+                    : default
+                : element.GetProperty(propertyName).Deserialize<T>();
+
+        //private static object? ExtractJsonProperty(JsonElement element, string propertyName, Type returnType)
+        //{
+        //    var jsonElementProperty = element.GetProperty(propertyName);
+
+        //    return jsonElementProperty.Deserialize(returnType);
+        //}
 
         private static void IncludeReference<TEntity, TIncludingEntity, TIncludedEntity>(
             QueryContext queryContext,

src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs

@@ -1501,34 +1501,79 @@ private Expression CreateExtractJsonPropertyExpression(
             ParameterExpression jsonElementParameter,
             IProperty property)
         {
+            var nullable = property.IsNullable;
             Expression resultExpression;
             if (property.GetTypeMapping().Converter is ValueConverter converter)
             {
-                resultExpression = Expression.Call(
-                    ExtractJsonPropertyMethodInfo,
-                    jsonElementParameter,
-                    Expression.Constant(property.GetJsonPropertyName()),
-                    Expression.Constant(converter.ProviderClrType));
-
-                if (resultExpression.Type != converter.ProviderClrType)
+                var providerClrType = converter.ProviderClrType.MakeNullable(nullable);
+                if (!property.IsNullable || converter.ConvertsNulls)
                 {
-                    resultExpression = Expression.Convert(resultExpression, converter.ProviderClrType);
+                    resultExpression = Expression.Call(
+                        ExtractJsonPropertyMethodInfo.MakeGenericMethod(providerClrType),
+                        jsonElementParameter,
+                        Expression.Constant(property.GetJsonPropertyName()),
+                        Expression.Constant(nullable));
+
+                    resultExpression = ReplacingExpressionVisitor.Replace(
+                        converter.ConvertFromProviderExpression.Parameters.Single(),
+                        resultExpression,
+                        converter.ConvertFromProviderExpression.Body);
+
+                    if (resultExpression.Type != property.ClrType)
+                    {
+                        resultExpression = Expression.Convert(resultExpression, property.ClrType);
+                    }
                 }
+                else
+                {
+                    // property is nullable and the converter can't handle nulls
+                    // we need to peek into the JSON value and only pass it thru converter if it's not null
+                    var jsonPropertyCall = Expression.Call(
+                        ExtractJsonPropertyMethodInfo.MakeGenericMethod(providerClrType),
+                        jsonElementParameter,
+                        Expression.Constant(property.GetJsonPropertyName()),
+                        Expression.Constant(nullable));
 
-                resultExpression = ReplacingExpressionVisitor.Replace(
-                    converter.ConvertFromProviderExpression.Parameters.Single(),
-                    resultExpression,
-                    converter.ConvertFromProviderExpression.Body);
+                    var jsonPropertyVariable = Expression.Variable(providerClrType);
+                    var jsonPropertyAssignment = Expression.Assign(jsonPropertyVariable, jsonPropertyCall);
+
+                    var testExpression = Expression.NotEqual(
+                        jsonPropertyVariable,
+                        Expression.Default(providerClrType));
+
+                    var ifTrueExpression = (Expression)jsonPropertyVariable;
+                    if (ifTrueExpression.Type != converter.ProviderClrType)
+                    {
+                        ifTrueExpression = Expression.Convert(ifTrueExpression, converter.ProviderClrType);
+                    }
+
+                    ifTrueExpression = ReplacingExpressionVisitor.Replace(
+                        converter.ConvertFromProviderExpression.Parameters.Single(),
+                        ifTrueExpression,
+                        converter.ConvertFromProviderExpression.Body);
+
+                    if (ifTrueExpression.Type != property.ClrType)
+                    {
+                        ifTrueExpression = Expression.Convert(ifTrueExpression, property.ClrType);
+                    }
+
+                    var condition = Expression.Condition(
+                        testExpression,
+                        ifTrueExpression,
+                        Expression.Default(property.ClrType));
+
+                    resultExpression = Expression.Block(
+                        new ParameterExpression[] { jsonPropertyVariable },
+                        new Expression[] { jsonPropertyAssignment, condition });
+                }
             }
             else
             {
-                resultExpression = Expression.Convert(
-                    Expression.Call(
-                        ExtractJsonPropertyMethodInfo,
-                        jsonElementParameter,
-                        Expression.Constant(property.GetJsonPropertyName()),
-                        Expression.Constant(property.ClrType)),
-                    property.ClrType);
+                resultExpression = Expression.Call(
+                    ExtractJsonPropertyMethodInfo.MakeGenericMethod(property.ClrType),
+                    jsonElementParameter,
+                    Expression.Constant(property.GetJsonPropertyName()),
+                    Expression.Constant(nullable));
             }
 
             if (_detailedErrorsEnabled)

src/EFCore.Relational/Query/SqlExpressions/JsonScalarExpression.cs

@@ -26,7 +26,7 @@ public JsonScalarExpression(
         IProperty property,
         IReadOnlyList<PathSegment> path,
         bool nullable)
-        : this(jsonColumn, path, property.ClrType, property.FindRelationalTypeMapping()!, nullable)
+        : this(jsonColumn, path, property.ClrType.UnwrapNullableType(), property.FindRelationalTypeMapping()!, nullable)
     {
     }
 

test/EFCore.Relational.Specification.Tests/Query/JsonQueryAdHocTestBase.cs

@@ -0,0 +1,86 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.Query;
+
+public abstract class JsonQueryAdHocTestBase : NonSharedModelTestBase
+{
+    protected JsonQueryAdHocTestBase(ITestOutputHelper testOutputHelper)
+    {
+        //TestSqlLoggerFactory.SetTestOutputHelper(testOutputHelper);
+    }
+
+    protected override string StoreName
+        => "JsonQueryAdHocTest";
+
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual async Task Optional_json_properties_materialized_as_null_when_the_element_in_json_is_not_present(bool async)
+    {
+        var contextFactory = await InitializeAsync<MyContext29219>(seed: Seed29219);
+        using (var context = contextFactory.CreateContext())
+        {
+            var query = context.Entities.Where(x => x.Id == 3);
+
+            var result = async
+                ? await query.SingleAsync()
+                : query.Single();
+
+            Assert.Equal(3, result.Id);
+            Assert.Equal(null, result.Reference.NullableScalar);
+            Assert.Equal(null, result.Collection[0].NullableScalar);
+        }
+    }
+
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual async Task Can_project_nullable_json_property_when_the_element_in_json_is_not_present(bool async)
+    {
+        var contextFactory = await InitializeAsync<MyContext29219>(seed: Seed29219);
+        using (var context = contextFactory.CreateContext())
+        {
+            var query = context.Entities.OrderBy(x => x.Id).Select(x => x.Reference.NullableScalar);
+
+            var result = async
+                ? await query.ToListAsync()
+                : query.ToList();
+
+            Assert.Equal(3, result.Count);
+            Assert.Equal(11, result[0]);
+            Assert.Equal(null, result[1]);
+            Assert.Equal(null, result[2]);
+        }
+    }
+
+    protected abstract void Seed29219(MyContext29219 ctx);
+
+    protected class MyContext29219 : DbContext
+    {
+        public MyContext29219(DbContextOptions options)
+            : base(options)
+        {
+        }
+
+        public DbSet<MyEntity29219> Entities { get; set; }
+
+        protected override void OnModelCreating(ModelBuilder modelBuilder)
+        {
+            modelBuilder.Entity<MyEntity29219>().Property(x => x.Id).ValueGeneratedNever();
+            modelBuilder.Entity<MyEntity29219>().OwnsOne(x => x.Reference).ToJson();
+            modelBuilder.Entity<MyEntity29219>().OwnsMany(x => x.Collection).ToJson();
+        }
+    }
+
+    public class MyEntity29219
+    {
+        public int Id { get; set; }
+        public MyJsonEntity29219 Reference { get; set; }
+        public List<MyJsonEntity29219> Collection { get; set; }
+    }
+
+    public class MyJsonEntity29219
+    {
+        public int NonNullableScalar { get; set; }
+        public int? NullableScalar { get; set; }
+    }
+}

test/EFCore.Relational.Specification.Tests/Query/JsonQueryFixtureBase.cs

@@ -297,6 +297,7 @@ private static void AssertOwnedBranch(JsonOwnedBranch expected, JsonOwnedBranch
         Assert.Equal(expected.Date, actual.Date);
         Assert.Equal(expected.Fraction, actual.Fraction);
         Assert.Equal(expected.Enum, actual.Enum);
+        Assert.Equal(expected.NullableEnum, actual.NullableEnum);
 
         AssertOwnedLeaf(expected.OwnedReferenceLeaf, actual.OwnedReferenceLeaf);
         Assert.Equal(expected.OwnedCollectionLeaf.Count, actual.OwnedCollectionLeaf.Count);
@@ -345,6 +346,12 @@ public static void AssertAllTypes(JsonOwnedAllTypes expected, JsonOwnedAllTypes
         Assert.Equal(expected.TestUnsignedInt16, actual.TestUnsignedInt16);
         Assert.Equal(expected.TestUnsignedInt32, actual.TestUnsignedInt32);
         Assert.Equal(expected.TestUnsignedInt64, actual.TestUnsignedInt64);
+        Assert.Equal(expected.TestNullableInt32, actual.TestNullableInt32);
+        Assert.Equal(expected.TestEnum, actual.TestEnum);
+        Assert.Equal(expected.TestEnumWithIntConverter, actual.TestEnumWithIntConverter);
+        Assert.Equal(expected.TestNullableEnum, actual.TestNullableEnum);
+        Assert.Equal(expected.TestNullableEnumWithIntConverter, actual.TestNullableEnumWithIntConverter);
+        Assert.Equal(expected.TestNullableEnumWithConverterThatHandlesNulls, actual.TestNullableEnumWithConverterThatHandlesNulls);
     }
 
     protected override string StoreName { get; } = "JsonQueryTest";
@@ -496,6 +503,27 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
             {
                 b.ToJson();
                 b.Property(x => x.TestDecimal).HasPrecision(18, 3);
+                b.Property(x => x.TestEnumWithIntConverter).HasConversion<int>();
+                b.Property(x => x.TestNullableEnumWithIntConverter).HasConversion<int>();
+                b.Property(x => x.TestNullableEnumWithConverterThatHandlesNulls).HasConversion(
+                    new ValueConverter<JsonEnum?, string>(
+                        x => x == null
+                            ? "Null"
+                            : x == JsonEnum.One
+                                ? "One"
+                                : x == JsonEnum.Two
+                                    ? "Two"
+                                    : x == JsonEnum.Three
+                                        ? "Three"
+                                        : "INVALID",
+                        x => x == "One"
+                            ? JsonEnum.One
+                            : x == "Two"
+                                ? JsonEnum.Two
+                                : x == "Three"
+                                    ? JsonEnum.Three
+                                    : null,
+                        convertsNulls: true));
             });
         modelBuilder.Entity<JsonEntityAllTypes>().OwnsMany(
             x => x.Collection, b =>

test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs

@@ -726,7 +726,7 @@ public virtual Task Json_all_types_entity_projection(bool async)
         => AssertQuery(
             async,
             ss => ss.Set<JsonEntityAllTypes>(),
-            entryCount: 3);
+            entryCount: 6);
 
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
@@ -752,6 +752,12 @@ public virtual Task Json_all_types_projection_individual_properties(bool async)
                     x.Reference.TestTimeSpan,
                     x.Reference.TestUnsignedInt16,
                     x.Reference.TestUnsignedInt32,
-                    x.Reference.TestUnsignedInt64
+                    x.Reference.TestUnsignedInt64,
+                    x.Reference.TestNullableInt32,
+                    x.Reference.TestEnum,
+                    x.Reference.TestEnumWithIntConverter,
+                    x.Reference.TestNullableEnum,
+                    x.Reference.TestNullableEnumWithIntConverter,
+                    x.Reference.TestNullableEnumWithConverterThatHandlesNulls,
                 }));
 }

test/EFCore.Relational.Specification.Tests/TestModels/JsonQuery/JsonOwnedAllTypes.cs

@@ -22,4 +22,10 @@ public class JsonOwnedAllTypes
     public ulong TestUnsignedInt64 { get; set; }
     public char TestCharacter { get; set; }
     public sbyte TestSignedByte { get; set; }
+    public int? TestNullableInt32 { get; set; }
+    public JsonEnum TestEnum { get; set; }
+    public JsonEnum TestEnumWithIntConverter { get; set; }
+    public JsonEnum? TestNullableEnum { get; set; }
+    public JsonEnum? TestNullableEnumWithIntConverter { get; set; }
+    public JsonEnum? TestNullableEnumWithConverterThatHandlesNulls { get; set; }
 }

test/EFCore.Relational.Specification.Tests/TestModels/JsonQuery/JsonOwnedBranch.cs

@@ -10,6 +10,8 @@ public class JsonOwnedBranch
 
     public JsonEnum Enum { get; set; }
 
+    public JsonEnum? NullableEnum { get; set; }
+
     public JsonOwnedLeaf OwnedReferenceLeaf { get; set; }
     public List<JsonOwnedLeaf> OwnedCollectionLeaf { get; set; }
 }