Refactor InternalEntityEntry (#36165)

Part of #31237

Author: AndriySvyryd

.github/copilot-instructions.md

@@ -86,8 +86,9 @@ If you are not sure, do not guess, just tell that you don't know or ask clarifyi
 
 - Follow the existing test patterns in the corresponding test projects
 - Create both unit tests and functional tests where appropriate
-- Add or modify `SQL` and `C#` baselines for tests when necessary
-- When running the tests specify the test project and let it be rebuilt. 
+- Fix `SQL` and `C#` baselines for tests when necessary by setting the `EF_TEST_REWRITE_BASELINES` env var to `1`
+- Before building or running the tests execute `restore.cmd` or `restore.sh` and `activate.ps1` or `activate.sh` to set up the environment
+- When running the tests specify the test project and let it be rebuilt by not adding `--no-build`
 
 ## Documentation
 

src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs

@@ -6,7 +6,6 @@
 using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Design.Internal;
 using Microsoft.EntityFrameworkCore.Internal;
-using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Query.Internal;
 
@@ -1498,8 +1497,10 @@ private void
         var variableName = parameters.TargetName;
         var mainBuilder = parameters.MainBuilder;
         var unsafeAccessors = new HashSet<string>();
+        var isOnComplexCollection = property.DeclaringType is IReadOnlyComplexType complexType && complexType.ComplexProperty.IsCollection;
 
         if (!property.IsShadowProperty()
+            && !isOnComplexCollection
             && property is not IServiceProperty) // Service properties don't use property accessors
         {
             ClrPropertyGetterFactory.Instance.Create(
@@ -1559,7 +1560,8 @@ private void
                 .DecrementIndent();
         }
 
-        if (property is not IServiceProperty)
+        if (property is not IServiceProperty
+            && !isOnComplexCollection)
         {
             PropertyAccessorsFactory.Instance.Create(
                 property,
@@ -2783,6 +2785,21 @@ private void CreateAnnotations(
                     mainBuilder.AppendLine(";");
                 }
 
+                foreach (var navigation in entityType.GetSkipNavigations())
+                {
+                    var variableName = _code.Identifier(navigation.Name, navigation, parameters.ScopeObjects, capitalize: false);
+
+                    mainBuilder
+                        .Append($"var {variableName} = ")
+                        .Append($"{parameters.TargetName}.FindSkipNavigation({_code.Literal(navigation.Name)})");
+                    if (nullable)
+                    {
+                        mainBuilder.Append("!");
+                    }
+
+                    mainBuilder.AppendLine(";");
+                }
+
                 var runtimeType = (IRuntimeEntityType)entityType;
                 var unsafeAccessors = new HashSet<string>();
 
@@ -2859,17 +2876,18 @@ private void CreateAnnotations(
                     .DecrementIndent();
 
                 AddNamespace(typeof(PropertyCounts), parameters.Namespaces);
-                var counts = runtimeType.Counts;
+                var counts = runtimeType.CalculateCounts();
                 mainBuilder
-                    .Append(parameters.TargetName).AppendLine(".Counts = new PropertyCounts(")
+                    .Append(parameters.TargetName).AppendLine(".SetCounts(new PropertyCounts(")
                     .IncrementIndent()
                     .Append("propertyCount: ").Append(_code.Literal(counts.PropertyCount)).AppendLine(",")
                     .Append("navigationCount: ").Append(_code.Literal(counts.NavigationCount)).AppendLine(",")
                     .Append("complexPropertyCount: ").Append(_code.Literal(counts.ComplexPropertyCount)).AppendLine(",")
+                    .Append("complexCollectionCount: ").Append(_code.Literal(counts.ComplexCollectionCount)).AppendLine(",")
                     .Append("originalValueCount: ").Append(_code.Literal(counts.OriginalValueCount)).AppendLine(",")
                     .Append("shadowCount: ").Append(_code.Literal(counts.ShadowCount)).AppendLine(",")
                     .Append("relationshipCount: ").Append(_code.Literal(counts.RelationshipCount)).AppendLine(",")
-                    .Append("storeGeneratedCount: ").Append(_code.Literal(counts.StoreGeneratedCount)).AppendLine(");")
+                    .Append("storeGeneratedCount: ").Append(_code.Literal(counts.StoreGeneratedCount)).AppendLine("));")
                     .DecrementIndent();
 
                 Check.DebugAssert(

src/EFCore.Relational/Metadata/Internal/RelationalModel.cs

@@ -1,9 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Reflection.Metadata;
 using System.Text;
-using System.Text.Json;
 
 namespace Microsoft.EntityFrameworkCore.Metadata.Internal;
 
@@ -282,7 +280,7 @@ private static void AddDefaultMappings(
                 principalRootEntityType = ownership.PrincipalEntityType;
             }
 
-            if (principalRootEntityType.FindDiscriminatorProperty() is not null) // tph
+            if (principalRootEntityType.FindDiscriminatorProperty() is not null)
             {
                 principalRootEntityType = principalRootEntityType.GetRootType();
             }

src/EFCore/ChangeTracking/CollectionEntry.cs

@@ -109,8 +109,7 @@ public override bool IsModified
 
             if (Metadata is ISkipNavigation skipNavigation)
             {
-                if (InternalEntry.EntityState != EntityState.Unchanged
-                    && InternalEntry.EntityState != EntityState.Detached)
+                if (InternalEntityEntry.EntityState is not EntityState.Unchanged and not EntityState.Detached)
                 {
                     return true;
                 }
@@ -122,11 +121,11 @@ public override bool IsModified
                 {
                     if (joinEntry.EntityType == joinEntityType
                         && stateManager.FindPrincipal(joinEntry, foreignKey) == InternalEntry
-                        && (joinEntry.EntityState == EntityState.Added
-                            || joinEntry.EntityState == EntityState.Deleted
+                        && (joinEntry.EntityState is EntityState.Added
+                            || joinEntry.EntityState is EntityState.Deleted
                             || foreignKey.Properties.Any(joinEntry.IsModified)
                             || inverseForeignKey.Properties.Any(joinEntry.IsModified)
-                            || (stateManager.FindPrincipal(joinEntry, inverseForeignKey)?.EntityState == EntityState.Deleted)))
+                            || (stateManager.FindPrincipal(joinEntry, inverseForeignKey)?.EntityState is EntityState.Deleted)))
                     {
                         return true;
                     }
@@ -231,7 +230,7 @@ public override void Load(LoadOptions options)
 
         if (!IsLoaded)
         {
-            TargetLoader.Load(InternalEntry, options);
+            TargetLoader.Load(InternalEntityEntry, options);
         }
     }
 
@@ -279,7 +278,7 @@ public override Task LoadAsync(LoadOptions options, CancellationToken cancellati
 
         return IsLoaded
             ? Task.CompletedTask
-            : TargetLoader.LoadAsync(InternalEntry, options, cancellationToken);
+            : TargetLoader.LoadAsync(InternalEntityEntry, options, cancellationToken);
     }
 
     /// <summary>
@@ -300,11 +299,11 @@ public override IQueryable Query()
     {
         EnsureInitialized();
 
-        return TargetLoader.Query(InternalEntry);
+        return TargetLoader.Query(InternalEntityEntry);
     }
 
     private void EnsureInitialized()
-        => InternalEntry.GetOrCreateCollection(Metadata, forMaterialization: true);
+        => InternalEntityEntry.GetOrCreateCollection(Metadata, forMaterialization: true);
 
     /// <summary>
     ///     The <see cref="EntityEntry" /> of an entity this navigation targets.
@@ -332,7 +331,7 @@ private void EnsureInitialized()
     [EntityFrameworkInternal]
     protected virtual InternalEntityEntry? GetInternalTargetEntry(object entity)
         => CurrentValue == null
-            || !InternalEntry.CollectionContains(Metadata, entity)
+            || !InternalEntityEntry.CollectionContains(Metadata, entity)
                 ? null
                 : InternalEntry.StateManager.GetOrCreateEntry(entity, Metadata.TargetEntityType);
 

src/EFCore/ChangeTracking/CollectionEntry`.cs

@@ -59,7 +59,7 @@ public CollectionEntry(InternalEntityEntry internalEntry, INavigationBase naviga
     /// </remarks>
     /// <value> An entry for the entity that owns this member. </value>
     public new virtual EntityEntry<TEntity> EntityEntry
-        => new(InternalEntry);
+        => new(InternalEntityEntry);
 
     /// <summary>
     ///     Gets or sets the value currently assigned to this property. If the current value is set using this property,
@@ -73,7 +73,7 @@ public CollectionEntry(InternalEntityEntry internalEntry, INavigationBase naviga
     /// </remarks>
     public new virtual IEnumerable<TRelatedEntity>? CurrentValue
     {
-        get => (IEnumerable<TRelatedEntity>?)this.GetInfrastructure().GetCurrentValue(Metadata);
+        get => (IEnumerable<TRelatedEntity>?)InternalEntry.GetCurrentValue(Metadata);
         set => base.CurrentValue = value;
     }
 
@@ -93,7 +93,7 @@ public CollectionEntry(InternalEntityEntry internalEntry, INavigationBase naviga
     /// </remarks>
     public new virtual IQueryable<TRelatedEntity> Query()
     {
-        InternalEntry.GetOrCreateCollection(Metadata, forMaterialization: true);
+        InternalEntityEntry.GetOrCreateCollection(Metadata, forMaterialization: true);
 
         return (IQueryable<TRelatedEntity>)base.Query();
     }

src/EFCore/ChangeTracking/ComplexPropertyEntry.cs

@@ -28,7 +28,7 @@ public class ComplexPropertyEntry : MemberEntry
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     [EntityFrameworkInternal]
-    public ComplexPropertyEntry(InternalEntityEntry internalEntry, IComplexProperty complexProperty)
+    public ComplexPropertyEntry(IInternalEntry internalEntry, IComplexProperty complexProperty)
         : base(internalEntry, complexProperty)
     {
     }
@@ -48,7 +48,7 @@ public ComplexPropertyEntry(InternalEntityEntry internalEntry, IComplexProperty
     /// </remarks>
     public override bool IsModified
     {
-        get => Metadata.ComplexType.GetFlattenedProperties().Any(property => InternalEntry.IsModified(property));
+        get => Metadata.ComplexType.GetFlattenedProperties().Any(InternalEntry.IsModified);
         set
         {
             foreach (var property in Metadata.ComplexType.GetFlattenedProperties())

src/EFCore/ChangeTracking/ComplexPropertyEntry`.cs

@@ -31,7 +31,7 @@ public class ComplexPropertyEntry<TEntity, TComplexProperty> : ComplexPropertyEn
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     [EntityFrameworkInternal]
-    public ComplexPropertyEntry(InternalEntityEntry internalEntry, IComplexProperty complexProperty)
+    public ComplexPropertyEntry(IInternalEntry internalEntry, IComplexProperty complexProperty)
         : base(internalEntry, complexProperty)
     {
     }
@@ -45,7 +45,7 @@ public ComplexPropertyEntry(InternalEntityEntry internalEntry, IComplexProperty
     ///     examples.
     /// </remarks>
     public new virtual EntityEntry<TEntity> EntityEntry
-        => new(InternalEntry);
+        => new(InternalEntry.EntityEntry);
 
     /// <summary>
     ///     Gets or sets the value currently assigned to this property. If the current value is set using this property,