Fix to #34293 - Json: error when trying to materialize json entity wi…

…th nullable property that is null in the json string - should materialize the property as null instead (#35040)

Problem was that when computing nullability of a JSON entity we were using faulty logic - checking if the foreign key is required, rather than if its required dependent. This lead to optional entities being incorrectly marked as required and cause validation error when JSON contained null.

Fixes #34293

src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

@@ -1650,12 +1650,14 @@ Expression ExpandOwnedNavigation(INavigation navigation)
                 {
                     var newJsonQueryExpression = jsonQueryExpression.BindNavigation(navigation);
 
+                    Debug.Assert(!navigation.IsOnDependent, "JSON navigations should always be from principal do dependent");
+
                     return navigation.IsCollection
                         ? newJsonQueryExpression
                         : new RelationalStructuralTypeShaperExpression(
                             navigation.TargetEntityType,
                             newJsonQueryExpression,
-                            nullable: shaper.IsNullable || !navigation.ForeignKey.IsRequired);
+                            nullable: shaper.IsNullable || !navigation.ForeignKey.IsRequiredDependent);
                 }
 
                 var entityProjectionExpression = GetEntityProjectionExpression(shaper);

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

@@ -1475,12 +1475,14 @@ private Expression CreateJsonShapers(
             foreach (var ownedNavigation in entityType.GetNavigations().Where(
                          n => n.TargetEntityType.IsMappedToJson() && n.ForeignKey.IsOwnership && n == n.ForeignKey.PrincipalToDependent))
             {
+                Debug.Assert(!ownedNavigation.IsOnDependent, "JSON navigations should always be from principal do dependent");
+
                 // we need to build entity shapers and fixup separately
                 // we don't know the order in which data comes, so we need to read through everything
                 // before we can do fixup safely
                 var innerShaper = CreateJsonShapers(
                     ownedNavigation.TargetEntityType,
-                    nullable || !ownedNavigation.ForeignKey.IsRequired,
+                    nullable || !ownedNavigation.ForeignKey.IsRequiredDependent,
                     jsonReaderDataShaperLambdaParameter,
                     keyValuesShaperLambdaParameter,
                     parentEntityExpression: null,

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

@@ -380,6 +380,156 @@ public class SubRound
 
     #endregion
 
+    #region 34293
+
+    [ConditionalFact]
+    public virtual async Task Project_entity_with_optional_json_entity_owned_by_required_json()
+    {
+        var contextFactory = await InitializeAsync<Context34293>(
+            onModelCreating: OnModelCreating34293,
+            seed: ctx => ctx.Seed());
+
+        using var context = contextFactory.CreateContext();
+        var entityProjection = await context.Set<Context34293.Entity>().ToListAsync();
+
+        Assert.Equal(3, entityProjection.Count);
+    }
+
+    [ConditionalFact]
+    public virtual async Task Project_required_json_entity()
+    {
+        var contextFactory = await InitializeAsync<Context34293>(
+            onModelCreating: OnModelCreating34293,
+            seed: ctx => ctx.Seed());
+
+        using var context = contextFactory.CreateContext();
+
+        var rootProjection = await context.Set<Context34293.Entity>().AsNoTracking().Where(x => x.Id != 3).Select(x => x.Json).ToListAsync();
+        Assert.Equal(2, rootProjection.Count);
+
+        var branchProjection = await context.Set<Context34293.Entity>().AsNoTracking().Where(x => x.Id != 3).Select(x => x.Json.Required).ToListAsync();
+        Assert.Equal(2, rootProjection.Count);
+
+        var badRootProjectionMessage = (await Assert.ThrowsAsync<InvalidOperationException>(
+            () => context.Set<Context34293.Entity>().AsNoTracking().Where(x => x.Id == 3).Select(x => x.Json).ToListAsync())).Message;
+        Assert.Equal(RelationalStrings.JsonRequiredEntityWithNullJson(nameof(Context34293.JsonBranch)), badRootProjectionMessage);
+
+        var badBranchProjectionMessage = (await Assert.ThrowsAsync<InvalidOperationException>(
+            () => context.Set<Context34293.Entity>().AsNoTracking().Where(x => x.Id == 3).Select(x => x.Json.Required).ToListAsync())).Message;
+        Assert.Equal(RelationalStrings.JsonRequiredEntityWithNullJson(nameof(Context34293.JsonBranch)), badBranchProjectionMessage);
+    }
+
+    [ConditionalFact]
+    public virtual async Task Project_optional_json_entity_owned_by_required_json_entity()
+    {
+        var contextFactory = await InitializeAsync<Context34293>(
+            onModelCreating: OnModelCreating34293,
+            seed: ctx => ctx.Seed());
+
+        using var context = contextFactory.CreateContext();
+        var leafProjection = await context.Set<Context34293.Entity>().AsNoTracking().Select(x => x.Json.Required.Optional).ToListAsync();
+        Assert.Equal(3, leafProjection.Count);
+    }
+
+    protected class Context34293(DbContextOptions options) : DbContext(options)
+    {
+        public DbSet<Entity> Entities { get; set; }
+
+        public class Entity
+        {
+            public int Id { get; set; }
+            public JsonRoot Json { get; set; }
+        }
+
+        public class JsonRoot
+        {
+            public DateTime Date { get; set; }
+
+            public JsonBranch Required { get; set; }
+        }
+
+        public class JsonBranch
+        {
+            public int Number { get; set; }
+            public JsonLeaf Optional { get; set; }
+        }
+
+        public class JsonLeaf
+        {
+            public string Name { get; set; }
+        }
+
+        public async Task Seed()
+        {
+            // everything - ok
+            var e1 = new Entity
+            {
+                Id = 1,
+                Json = new JsonRoot
+                {
+                    Date = new DateTime(2001, 1, 1),
+                    Required = new JsonBranch
+                    {
+                        Number = 1,
+                        Optional = new JsonLeaf { Name = "optional 1" }
+                    }
+                }
+            };
+
+            // null leaf - ok (optional nav)
+            var e2 = new Entity
+            {
+                Id = 2,
+                Json = new JsonRoot
+                {
+                    Date = new DateTime(2002, 2, 2),
+                    Required = new JsonBranch
+                    {
+                        Number = 2,
+                        Optional = null
+                    }
+                }
+            };
+
+            // null branch - invalid (required nav)
+            var e3 = new Entity
+            {
+                Id = 3,
+                Json = new JsonRoot
+                {
+                    Date = new DateTime(2003, 3, 3),
+                    Required = null,
+                }
+            };
+
+            Entities.AddRange(e1, e2, e3);
+            await SaveChangesAsync();
+        }
+    }
+
+    protected virtual void OnModelCreating34293(ModelBuilder modelBuilder)
+    {
+        modelBuilder.Entity<Context34293.Entity>(
+            b =>
+            {
+                b.Property(x => x.Id).ValueGeneratedNever();
+                b.OwnsOne(
+                    x => x.Json, b =>
+                    {
+                        b.ToJson().HasColumnType(JsonColumnType);
+                        b.OwnsOne(x => x.Required, bb =>
+                        {
+                            bb.OwnsOne(x => x.Optional);
+                            bb.Navigation(x => x.Optional).IsRequired(false);
+                        });
+                        b.Navigation(x => x.Required).IsRequired(true);
+                    });
+                b.Navigation(x => x.Json).IsRequired(true);
+            });
+    }
+
+    #endregion
+
     #region 34960
 
     [ConditionalFact]