Refactor InternalEntityEntry

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>();
 
@@ -2866,6 +2883,7 @@ private void CreateAnnotations(
                     .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(",")

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 (isTph)
             {
                 principalRootEntityType = principalRootEntityType.GetRootType();
             }

src/EFCore/ChangeTracking/CollectionEntry.cs

@@ -109,8 +109,8 @@ public override bool IsModified
 
             if (Metadata is ISkipNavigation skipNavigation)
             {
-                if (InternalEntry.EntityState != EntityState.Unchanged
-                    && InternalEntry.EntityState != EntityState.Detached)
+                if (InternalEntityEntry.EntityState != EntityState.Unchanged
+                    && InternalEntityEntry.EntityState != EntityState.Detached)
                 {
                     return true;
                 }
@@ -231,7 +231,7 @@ public override void Load(LoadOptions options)
 
         if (!IsLoaded)
         {
-            TargetLoader.Load(InternalEntry, options);
+            TargetLoader.Load(InternalEntityEntry, options);
         }
     }
 
@@ -279,7 +279,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 +300,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 +332,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,

src/EFCore/ChangeTracking/Internal/ArrayPropertyValues.cs

@@ -108,7 +108,7 @@ public override void SetValues(PropertyValues propertyValues)
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public override IReadOnlyList<IProperty> Properties
-        => _properties ??= EntityType.GetFlattenedProperties().ToList();
+        => _properties ??= [.. EntityType.GetFlattenedProperties()];
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

src/EFCore/ChangeTracking/Internal/ChangeDetector.cs

@@ -38,7 +38,7 @@ public ChangeDetector(
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual void PropertyChanged(InternalEntityEntry entry, IPropertyBase propertyBase, bool setModified)
+    public virtual void PropertyChanged(IInternalEntry entry, IPropertyBase propertyBase, bool setModified)
     {
         if (entry.EntityState == EntityState.Detached
             || propertyBase is IServiceProperty)
@@ -62,16 +62,16 @@ public virtual void PropertyChanged(InternalEntityEntry entry, IPropertyBase pro
         else if (propertyBase.GetRelationshipIndex() != -1
                  && propertyBase is INavigationBase navigation)
         {
-            DetectNavigationChange(entry, navigation);
+            DetectNavigationChange((InternalEntityEntry)entry, navigation);
         }
     }
 
-    private static void ThrowIfKeyChanged(InternalEntityEntry entry, IProperty property)
+    private static void ThrowIfKeyChanged(IInternalEntry entry, IProperty property)
     {
         if (property.IsKey()
             && property.GetAfterSaveBehavior() == PropertySaveBehavior.Throw)
         {
-            throw new InvalidOperationException(CoreStrings.KeyReadOnly(property.Name, entry.EntityType.DisplayName()));
+            throw new InvalidOperationException(CoreStrings.KeyReadOnly(property.Name, entry.StructuralType.DisplayName()));
         }
     }
 
@@ -81,15 +81,15 @@ private static void ThrowIfKeyChanged(InternalEntityEntry entry, IProperty prope
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public virtual void PropertyChanging(InternalEntityEntry entry, IPropertyBase propertyBase)
+    public virtual void PropertyChanging(IInternalEntry entry, IPropertyBase propertyBase)
     {
         if (entry.EntityState == EntityState.Detached
             || propertyBase is IServiceProperty)
         {
             return;
         }
 
-        if (!entry.EntityType.UseEagerSnapshots())
+        if (!entry.StructuralType.UseEagerSnapshots())
         {
             if (propertyBase is IProperty asProperty
                 && asProperty.GetOriginalValueIndex() != -1)
@@ -99,7 +99,7 @@ public virtual void PropertyChanging(InternalEntityEntry entry, IPropertyBase pr
 
             if (propertyBase.GetRelationshipIndex() != -1)
             {
-                entry.EnsureRelationshipSnapshot();
+                ((InternalEntityEntry)entry).EnsureRelationshipSnapshot();
             }
         }
     }
@@ -183,35 +183,27 @@ public virtual void DetectChanges(InternalEntityEntry entry)
         }
     }
 
-    private bool DetectChanges(InternalEntityEntry entry, HashSet<InternalEntityEntry> visited)
+    private void DetectChanges(InternalEntityEntry entry, HashSet<InternalEntityEntry> visited)
     {
-        var changesFound = false;
-
-        if (entry.EntityState != EntityState.Detached)
+        if (entry.EntityState == EntityState.Detached)
         {
-            foreach (var foreignKey in entry.EntityType.GetForeignKeys())
-            {
-                var principalEntry = entry.StateManager.FindPrincipal(entry, foreignKey);
-
-                if (principalEntry != null
-                    && !visited.Contains(principalEntry))
-                {
-                    visited.Add(principalEntry);
+            return;
+        }
 
-                    if (DetectChanges(principalEntry, visited))
-                    {
-                        changesFound = true;
-                    }
-                }
-            }
+        foreach (var foreignKey in entry.EntityType.GetForeignKeys())
+        {
+            var principalEntry = entry.StateManager.FindPrincipal(entry, foreignKey);
 
-            if (LocalDetectChanges(entry))
+            if (principalEntry != null
+                && !visited.Contains(principalEntry))
             {
-                changesFound = true;
+                visited.Add(principalEntry);
+
+                DetectChanges(principalEntry, visited);
             }
         }
 
-        return changesFound;
+        LocalDetectChanges(entry);
     }
 
     private bool LocalDetectChanges(InternalEntityEntry entry)
@@ -274,7 +266,7 @@ private bool LocalDetectChanges(InternalEntityEntry entry)
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public bool DetectValueChange(InternalEntityEntry entry, IProperty property)
+    public bool DetectValueChange(IInternalEntry entry, IProperty property)
     {
         var current = entry[property];
         var original = entry.GetOriginalValue(property);
@@ -296,26 +288,33 @@ public bool DetectValueChange(InternalEntityEntry entry, IProperty property)
         return false;
     }
 
-    private void LogChangeDetected(InternalEntityEntry entry, IProperty property, object? original, object? current)
+    private void LogChangeDetected(IInternalEntry entry, IProperty property, object? original, object? current)
     {
         if (_loggingOptions.IsSensitiveDataLoggingEnabled)
         {
-            _logger.PropertyChangeDetectedSensitive(entry, property, original, current);
+            if (entry is InternalEntityEntry entityEntry)
+            {
+                _logger.PropertyChangeDetectedSensitive(entityEntry, property, original, current);
+            }
         }
         else
         {
-            _logger.PropertyChangeDetected(entry, property, original, current);
+            if (entry is InternalEntityEntry entityEntry)
+            {
+                _logger.PropertyChangeDetected(entityEntry, property, original, current);
+            }
         }
     }
 
-    private bool DetectKeyChange(InternalEntityEntry entry, IProperty property)
+    private bool DetectKeyChange(IInternalEntry entry, IProperty property)
     {
         if (property.GetRelationshipIndex() < 0)
         {
             return false;
         }
 
-        var snapshotValue = entry.GetRelationshipSnapshotValue(property);
+        var entityEntry = (InternalEntityEntry)entry;
+        var snapshotValue = entityEntry.GetRelationshipSnapshotValue(property);
         var currentValue = entry[property];
 
         var comparer = property.GetKeyValueComparer();
@@ -326,19 +325,19 @@ private bool DetectKeyChange(InternalEntityEntry entry, IProperty property)
         {
             var keys = property.GetContainingKeys();
             var foreignKeys = property.GetContainingForeignKeys()
-                .Where(fk => fk.DeclaringEntityType.IsAssignableFrom(entry.EntityType));
+                .Where(fk => fk.DeclaringEntityType.IsAssignableFrom(entityEntry.EntityType));
 
             if (_loggingOptions.IsSensitiveDataLoggingEnabled)
             {
-                _logger.ForeignKeyChangeDetectedSensitive(entry, property, snapshotValue, currentValue);
+                _logger.ForeignKeyChangeDetectedSensitive(entityEntry, property, snapshotValue, currentValue);
             }
             else
             {
-                _logger.ForeignKeyChangeDetected(entry, property, snapshotValue, currentValue);
+                _logger.ForeignKeyChangeDetected(entityEntry, property, snapshotValue, currentValue);
             }
 
-            entry.StateManager.InternalEntityEntryNotifier.KeyPropertyChanged(
-                entry, property, keys, foreignKeys, snapshotValue, currentValue);
+            entityEntry.StateManager.InternalEntityEntryNotifier.KeyPropertyChanged(
+                entityEntry, property, keys, foreignKeys, snapshotValue, currentValue);
 
             return true;
         }
@@ -487,7 +486,6 @@ public virtual void SetEvents(
     public virtual void OnDetectingEntityChanges(InternalEntityEntry internalEntityEntry)
     {
         var @event = DetectingEntityChanges;
-
         if (@event != null)
         {
             var changeTracker = internalEntityEntry.StateManager.Context.ChangeTracker;