Materialize required complex collection absent from JSON document as empty

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

@@ -58,6 +58,9 @@ private static readonly MethodInfo CollectionAccessorGetOrCreateMethodInfo
         private static readonly MethodInfo CollectionAccessorAddMethodInfo
             = typeof(IClrCollectionAccessor).GetTypeInfo().GetDeclaredMethod(nameof(IClrCollectionAccessor.Add))!;
 
+        private static readonly MethodInfo CollectionAccessorCreateMethodInfo
+            = typeof(IClrCollectionAccessor).GetTypeInfo().GetDeclaredMethod(nameof(IClrCollectionAccessor.Create))!;
+
         private static readonly PropertyInfo ObjectArrayIndexerPropertyInfo
             = typeof(object[]).GetProperty("Item")!;
 
@@ -1609,6 +1612,7 @@ private Expression CreateJsonShapers(
             var innerShapersMap = new Dictionary<string, Expression>();
             var innerFixupMap = new Dictionary<string, LambdaExpression>();
             var trackingInnerFixupMap = new Dictionary<string, LambdaExpression>();
+            var innerAbsentPropertyFixupMap = new Dictionary<string, LambdaExpression>();
 
             // Go over all structural properties (complex properties and navigations - if we're an (owned) entity), which represent JSON
             // nested types; generate shapers and fixup to wire the materialized related instance into the parent's property.
@@ -1716,6 +1720,15 @@ private Expression CreateJsonShapers(
 
                     innerFixupMap[navigationJsonPropertyName] = fixup;
 
+                    // A required complex collection absent from the document (e.g. added to the type after the row was persisted) must
+                    // materialize as empty rather than null; an explicit JSON null is still materialized as null. See #38625.
+                    if (nestedStructuralProperty is IComplexProperty { IsNullable: false }
+                        && !nestedStructuralProperty.IsShadowProperty())
+                    {
+                        innerAbsentPropertyFixupMap[navigationJsonPropertyName] =
+                            GenerateAbsentCollectionFixupForJson(structuralType.ClrType, nestedStructuralProperty);
+                    }
+
                     var trackedFixup = Lambda(
                         Block(typeof(void), expressionsForTracking),
                         shaperEntityParameter,
@@ -1750,7 +1763,8 @@ private Expression CreateJsonShapers(
                 jsonReaderDataShaperLambdaParameter,
                 innerShapersMap,
                 innerFixupMap,
-                trackingInnerFixupMap).Rewrite(structuralTypeShaperMaterializer);
+                trackingInnerFixupMap,
+                innerAbsentPropertyFixupMap).Rewrite(structuralTypeShaperMaterializer);
 
             var entityShaperMaterializerVariable = Variable(
                 structuralTypeShaperMaterializer.Type,
@@ -1899,7 +1913,8 @@ private sealed class JsonEntityMaterializerRewriter(
             ParameterExpression jsonReaderDataParameter,
             IDictionary<string, Expression> innerShapersMap,
             IDictionary<string, LambdaExpression> innerFixupMap,
-            IDictionary<string, LambdaExpression> trackingInnerFixupMap)
+            IDictionary<string, LambdaExpression> trackingInnerFixupMap,
+            IDictionary<string, LambdaExpression> innerAbsentPropertyFixupMap)
             : ExpressionVisitor
         {
             private static readonly PropertyInfo JsonEncodedTextEncodedUtf8BytesProperty
@@ -1912,6 +1927,9 @@ private static readonly MethodInfo JsonEncodedTextEncodeMethod
             // which happens at the end (after we read everything to guarantee that we can instantiate the entity
             private readonly Dictionary<string, ParameterExpression> _navigationVariableMap = [];
 
+            // tracks whether a navigation appeared in the JSON document at all, to distinguish absent from explicitly null
+            private readonly Dictionary<string, ParameterExpression> _navigationReadVariableMap = [];
+
             public BlockExpression Rewrite(BlockExpression jsonEntityShaperMaterializer)
                 => (BlockExpression)VisitBlock(jsonEntityShaperMaterializer);
 
@@ -2090,38 +2108,57 @@ void ProcessFixup(IDictionary<string, LambdaExpression> fixupMap)
                     {
                         foreach (var fixup in fixupMap)
                         {
-                            var navigationEntityParameter = _navigationVariableMap[fixup.Key];
+                            var navigationVariable = _navigationVariableMap[fixup.Key];
 
                             // Inject the fixup code for each property; we have this as a set of lambdas in the fixup map.
                             // In the normal case, simply Invoke the lambda, passing it the structural type to be fixed up as a parameter.
                             // This unfortunately doesn't work on value types (where a copy would be mutated), so for them,
                             // we unwrap the lambda and integrate its body directly.
                             // We should ideally do this for all cases (no need for the extra lambda Invoke), but there are some issues around us writing
                             // to readonly fields.
-                            if (jsonStructuralTypeVariable.Type
-                                .IsValueType /*&& Nullable.GetUnderlyingType(jsonStructuralTypeVariable.Type) is null*/)
-                            {
-                                var fixupBody = ReplacingExpressionVisitor.Replace(
+                            var isValueType = jsonStructuralTypeVariable.Type
+                                .IsValueType /*&& Nullable.GetUnderlyingType(jsonStructuralTypeVariable.Type) is null*/;
+
+                            var fixupExpression = isValueType
+                                ? ReplacingExpressionVisitor.Replace(
                                     originals: [fixup.Value.Parameters[0], fixup.Value.Parameters[1]],
-                                    replacements: [jsonStructuralTypeVariable, _navigationVariableMap[fixup.Key]],
-                                    fixup.Value.Body);
+                                    replacements: [jsonStructuralTypeVariable, navigationVariable],
+                                    fixup.Value.Body)
+                                : Invoke(fixup.Value, jsonStructuralTypeVariable, navigationVariable);
 
-                                finalBlockExpressions.Add(fixupBody);
+                            if (innerAbsentPropertyFixupMap.TryGetValue(fixup.Key, out var absentPropertyFixup))
+                            {
+                                Check.DebugAssert(
+                                    absentPropertyFixup.Parameters is [{ } absentFixupParameter]
+                                    && absentFixupParameter.Type == jsonStructuralTypeVariable.Type,
+                                    "The absent-property fixup must take only the instance being materialized");
+                                Check.DebugAssert(
+                                    _navigationReadVariableMap.ContainsKey(fixup.Key),
+                                    "No read flag was generated for a property with an absent-property fixup");
+
+                                var absentPropertyFixupExpression = isValueType
+                                    ? ReplacingExpressionVisitor.Replace(
+                                        absentPropertyFixup.Parameters[0], jsonStructuralTypeVariable, absentPropertyFixup.Body)
+                                    : Invoke(absentPropertyFixup, jsonStructuralTypeVariable);
+
+                                fixupExpression = IfThenElse(
+                                    _navigationReadVariableMap[fixup.Key],
+                                    fixupExpression,
+                                    absentPropertyFixupExpression);
                             }
-                            else
+
+                            if (!isValueType)
                             {
                                 // If the structural type being fixed up is nullable, then we need to add null checks before we run fixup logic.
                                 // For regular entities, whose fixup is done as part of the "Materialize*" method, the checks are done there
                                 // (the same will be done for the "optimized" scenario, where we populate properties directly rather than store in variables).
                                 // But in this case fixups are standalone, so the null safety must be added here.
-                                finalBlockExpressions.Add(
-                                    IfThen(
-                                        NotEqual(jsonStructuralTypeVariable, Constant(null, jsonStructuralTypeVariable.Type)),
-                                        Invoke(
-                                            fixup.Value,
-                                            jsonStructuralTypeVariable,
-                                            _navigationVariableMap[fixup.Key])));
+                                fixupExpression = IfThen(
+                                    NotEqual(jsonStructuralTypeVariable, Constant(null, jsonStructuralTypeVariable.Type)),
+                                    fixupExpression);
                             }
+
+                            finalBlockExpressions.Add(fixupExpression);
                         }
                     }
                 }
@@ -2211,6 +2248,15 @@ void ProcessFixup(IDictionary<string, LambdaExpression> fixupMap)
 
                         _navigationVariableMap[innerShaperMapElementKey] = propertyVariable;
 
+                        Expression markAsRead = Empty();
+                        if (innerAbsentPropertyFixupMap.ContainsKey(innerShaperMapElementKey))
+                        {
+                            var propertyReadVariable = Variable(typeof(bool));
+                            finalBlockVariables.Add(propertyReadVariable);
+                            _navigationReadVariableMap[innerShaperMapElementKey] = propertyReadVariable;
+                            markAsRead = Assign(propertyReadVariable, Constant(true));
+                        }
+
                         var moveNext = Call(managerVariable, Utf8JsonReaderManagerMoveNextMethod);
                         var captureState = Call(managerVariable, Utf8JsonReaderManagerCaptureStateMethod);
                         var assignment = Assign(propertyVariable, innerShaperMapElement.Value);
@@ -2227,6 +2273,7 @@ void ProcessFixup(IDictionary<string, LambdaExpression> fixupMap)
                                 captureState,
                                 assignment,
                                 managerRecreation,
+                                markAsRead,
                                 Empty()));
                     }
 
@@ -2907,6 +2954,28 @@ private Expression GetOrCreateCollectionObjectLambda(Type entityType, IPropertyB
                 prm);
         }
 
+        private LambdaExpression GenerateAbsentCollectionFixupForJson(Type clrType, IPropertyBase structuralProperty)
+        {
+            var entityParameter = Parameter(clrType);
+            var setter = structuralProperty.GetMemberInfo(forMaterialization: true, forSet: true);
+
+            return Lambda(
+                Block(
+                    typeof(void),
+                    entityParameter.MakeMemberAccess(setter)
+                        .Assign(
+                            Convert(
+                                Call(
+                                    _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
+                                        structuralProperty.GetCollectionAccessor(),
+                                        LiftableConstantExpressionHelpers.BuildClrCollectionAccessorLambda(structuralProperty),
+                                        structuralProperty.Name + "StructuralPropertyCollectionAccessor",
+                                        typeof(IClrCollectionAccessor)),
+                                    CollectionAccessorCreateMethodInfo),
+                                setter.GetMemberType()))),
+                entityParameter);
+        }
+
         private Expression AddToCollectionStructuralProperty(
             ParameterExpression entity,
             ParameterExpression relatedEntity,

src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs

@@ -1721,10 +1721,17 @@ void CheckForNullComplexProperties()
         {
             foreach (var complexProperty in structuralType.GetFlattenedComplexProperties())
             {
+                Check.DebugAssert(
+                    complexProperty.DeclaringType is not IComplexType { ComplexProperty.IsCollection: true }
+                    || complexProperty.DeclaringType == structuralType,
+                    "Properties of a complex collection element type are only flattened into the element's own entry");
+
                 if (!complexProperty.IsNullable
                     && this[complexProperty] == null
                     && complexProperty.ComplexType.GetProperties().Any(p => !p.IsNullable)
                     && (complexProperty.DeclaringType is not IComplexType complexType
+                        // A collection element has its own entry; its containing property lives on the parent's entry
+                        || complexType.ComplexProperty.IsCollection
                         || GetCurrentValue(complexType.ComplexProperty) != null))
                 {
                     throw new InvalidOperationException(

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

@@ -371,6 +371,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
                 b.ToJson();
                 b.Property(x => x.Name).HasJsonPropertyName("1!NOT VALID;");
                 b.Property(x => x.Name2).HasJsonPropertyName("1-NOT VALID!");
+                b.ComplexCollection(x => x.Items, cb => cb.HasJsonPropertyName("1!NOT VALID COLLECTION;"));
             });
     }
 
@@ -385,6 +386,12 @@ public class InvalidNameNestedEntity
     {
         public string Name { get; set; } = "";
         public string Name2 { get; set; } = "";
+        public List<InvalidNameNestedItem> Items { get; set; } = [];
+    }
+
+    public class InvalidNameNestedItem
+    {
+        public string Value { get; set; } = "";
     }
 
     [Fact]

test/EFCore.Relational.Specification.Tests/Update/ComplexCollectionJsonUpdateTestBase.cs

@@ -667,6 +667,111 @@ public virtual Task Set_nullable_complex_property_with_nested_collection_to_null
                 }
             });
 
+    [Fact] // Issue #38625
+    public virtual Task Complex_collection_absent_from_json_is_materialized_as_empty()
+        => TestHelpers.ExecuteWithStrategyInTransactionAsync(
+            CreateContext,
+            UseTransaction,
+            async context =>
+            {
+                // Simulate a row persisted before the Others sub-collection was added to the type,
+                // so the key is absent from the stored document.
+                await SetStoredDocumentAsync(
+                    context, "Widgets", "Deep", id: 1, """{"Mid":{"Items":[{"Title":"Item1","Inner":[{"Value":"inner-0"}]}]}}""");
+
+                // Tracking and no-tracking queries go through separate fixup paths.
+                AssertMaterialized(await context.Set<WidgetWithDeepJson>().OrderBy(w => w.Id).FirstAsync());
+                AssertMaterialized(await context.Set<WidgetWithDeepJson>().AsNoTracking().OrderBy(w => w.Id).FirstAsync());
+
+                static void AssertMaterialized(WidgetWithDeepJson widget)
+                {
+                    var item = Assert.Single(widget.Deep.Mid.Items);
+
+                    Assert.Equal("Item1", item.Title);
+                    Assert.Equal("inner-0", Assert.Single(item.Inner).Value);
+
+                    // A complex collection that is absent from the JSON document materializes as an empty collection, not null.
+                    Assert.NotNull(item.Others);
+                    Assert.Empty(item.Others);
+                }
+            });
+
+    [Fact] // Issue #38625
+    public virtual Task Complex_collection_explicitly_null_in_json_is_materialized_as_null()
+        => TestHelpers.ExecuteWithStrategyInTransactionAsync(
+            CreateContext,
+            UseTransaction,
+            async context =>
+            {
+                // Unlike an absent key, an explicit null in the document is preserved.
+                await SetStoredDocumentAsync(
+                    context, "Widgets", "Deep", id: 1,
+                    """{"Mid":{"Items":[{"Title":"Item1","Inner":[{"Value":"inner-0"}],"Others":null}]}}""");
+
+                var widget = await context.Set<WidgetWithDeepJson>().OrderBy(w => w.Id).FirstAsync();
+                Assert.Null(Assert.Single(widget.Deep.Mid.Items).Others);
+
+                var untrackedWidget = await context.Set<WidgetWithDeepJson>().AsNoTracking().OrderBy(w => w.Id).FirstAsync();
+                Assert.Null(Assert.Single(untrackedWidget.Deep.Mid.Items).Others);
+            });
+
+    [Fact] // Issue #38625
+    public virtual Task Save_changes_after_loading_row_with_complex_collection_absent_from_json()
+        => TestHelpers.ExecuteWithStrategyInTransactionAsync(
+            CreateContext,
+            UseTransaction,
+            async context =>
+            {
+                await SetStoredDocumentAsync(
+                    context, "Widgets", "Deep", id: 1, """{"Mid":{"Items":[{"Title":"Item1","Inner":[{"Value":"inner-0"}]}]}}""");
+
+                var widget = await context.Set<WidgetWithDeepJson>().OrderBy(w => w.Id).FirstAsync();
+
+                // Modifying an unrelated scalar must not make the row unsaveable.
+                widget.Deep.Mid.Items[0].Title = "Item1-updated";
+
+                ClearLog();
+                await context.SaveChangesAsync();
+            },
+            async context =>
+            {
+                using (SuspendRecordingEvents())
+                {
+                    var widget = await context.Set<WidgetWithDeepJson>().OrderBy(w => w.Id).FirstAsync();
+                    var item = Assert.Single(widget.Deep.Mid.Items);
+                    Assert.Equal("Item1-updated", item.Title);
+                    Assert.Empty(item.Others);
+                }
+            });
+
+    [Fact] // Issue #38625
+    public virtual Task Saving_null_required_complex_collection_in_complex_collection_element_throws()
+        => TestHelpers.ExecuteWithStrategyInTransactionAsync(
+            CreateContext,
+            UseTransaction,
+            async context =>
+            {
+                var widget = await context.Set<WidgetWithDeepJson>().OrderBy(w => w.Id).FirstAsync();
+                widget.Deep.Mid.Items[0].Others = null!;
+
+                Assert.Equal(
+                    CoreStrings.NullRequiredComplexProperty(nameof(DeepItem), nameof(DeepItem.Others)),
+                    (await Assert.ThrowsAsync<InvalidOperationException>(() => context.SaveChangesAsync())).Message);
+            });
+
+    private static Task SetStoredDocumentAsync(DbContext context, string table, string column, int id, string json)
+    {
+        // Identifiers are delimited by the provider so that any provider can run this test; the document is inlined rather than
+        // parameterized so that providers with a dedicated JSON store type need no cast. Braces are escaped for string.Format.
+        var sqlGenerationHelper = context.GetService<ISqlGenerationHelper>();
+
+        return context.Database.ExecuteSqlRawAsync(
+            $"UPDATE {Q(table)} SET {Q(column)} = '{json.Replace("{", "{{").Replace("}", "}}")}' WHERE {Q("Id")} = {id}");
+
+        string Q(string name)
+            => sqlGenerationHelper.DelimitIdentifier(name);
+    }
+
     protected virtual void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
         => facade.UseTransaction(transaction.GetDbTransaction());