List shared-identity entries for STETs

Remove null-checking code for IProperty.GetValueComparer() Make DebugString terser for Indexes and keys on STETs Remove extra UsingEntity in ManyToManyQueryFixtureBase Fixes #23418
dotnet · Sep 14, 2021 · 122d038 · 122d038
1 parent 0458d08
commit 122d038
Show file tree

Hide file tree

Showing 28 changed files with 714 additions and 738 deletions.
diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
@@ -2018,12 +2018,11 @@ protected virtual void TrackData(
 
                             var sourceValue = sourceEntry.GetCurrentValue(sourceProperty);
                             var targetValue = entry.GetCurrentValue(targetProperty);
-                            var comparer = targetProperty.GetValueComparer()
-                                ?? sourceProperty.GetValueComparer();
+                            var comparer = targetProperty.GetValueComparer();
 
                             var modelValuesChanged
                                 = sourceProperty.ClrType.UnwrapNullableType() == targetProperty.ClrType.UnwrapNullableType()
-                                && comparer?.Equals(sourceValue, targetValue) == false;
+                                && comparer.Equals(sourceValue, targetValue) == false;
 
                             if (!modelValuesChanged)
                             {

diff --git a/...uery/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs b/...uery/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs
@@ -1877,23 +1877,16 @@ private static async Task TaskAwaiter(Func<Task>[] taskFactories)
 
             private static bool CompareIdentifiers(IReadOnlyList<ValueComparer> valueComparers, object[] left, object[] right)
             {
-                if (valueComparers != null)
+                // Ignoring size check on all for perf as they should be same unless bug in code.
+                for (var i = 0; i < left.Length; i++)
                 {
-                    // Ignoring size check on all for perf as they should be same unless bug in code.
-                    for (var i = 0; i < left.Length; i++)
+                    if (!valueComparers[i].Equals(left[i], right[i]))
                     {
-                        if (valueComparers[i] != null
-                            ? !valueComparers[i].Equals(left[i], right[i])
-                            : !Equals(left[i], right[i]))
-                        {
-                            return false;
-                        }
+                        return false;
                     }
-
-                    return true;
                 }
 
-                return StructuralComparisons.StructuralEqualityComparer.Equals(left, right);
+                return true;
             }
 
             private sealed class CollectionShaperFindingExpressionVisitor : ExpressionVisitor

diff --git a/src/EFCore.Relational/Update/ColumnModification.cs b/src/EFCore.Relational/Update/ColumnModification.cs
@@ -423,7 +423,7 @@ public virtual void AddSharedColumnModification(IColumnModification modification
             _sharedColumnModifications ??= new List<IColumnModification>();
 
             if (UseCurrentValueParameter
-                && !StructuralComparisons.StructuralEqualityComparer.Equals(Value, modification.Value))
+                && !modification.Property.GetValueComparer().Equals(Value, modification.Value))
             {
                 if (_sensitiveLoggingEnabled)
                 {
@@ -447,7 +447,7 @@ public virtual void AddSharedColumnModification(IColumnModification modification
             }
 
             if (UseOriginalValueParameter
-                && !StructuralComparisons.StructuralEqualityComparer.Equals(OriginalValue, modification.OriginalValue))
+                && !modification.Property.GetValueComparer().Equals(OriginalValue, modification.OriginalValue))
             {
                 if (Entry.EntityState == EntityState.Modified
                     && modification.Entry.EntityState == EntityState.Added

diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs
@@ -270,7 +270,9 @@ private List<IColumnModification> GenerateColumnModifications()
 
                     if (entry.SharedIdentityEntry != null)
                     {
-                        var sharedTableMapping = GetTableMapping(entry.SharedIdentityEntry.EntityType);
+                        var sharedTableMapping = entry.EntityType != entry.SharedIdentityEntry.EntityType
+                            ? GetTableMapping(entry.SharedIdentityEntry.EntityType)
+                            : tableMapping;
                         if (sharedTableMapping != null)
                         {
                             InitializeSharedColumns(entry.SharedIdentityEntry, sharedTableMapping, updating, sharedTableColumnMap);
@@ -482,16 +484,7 @@ public void RecordValue(IProperty property, IUpdateEntry entry)
                         break;
                     case EntityState.Added:
                         _currentValue = entry.GetCurrentValue(property);
-
-                        var comparer = property.GetValueComparer();
-                        if (comparer == null)
-                        {
-                            _write = !Equals(_originalValue, _currentValue);
-                        }
-                        else
-                        {
-                            _write = !comparer.Equals(_originalValue, _currentValue);
-                        }
+                        _write = !property.GetValueComparer().Equals(_originalValue, _currentValue);
 
                         break;
                     case EntityState.Deleted:
@@ -512,7 +505,8 @@ public bool TryPropagate(IProperty property, IUpdateEntry entry)
                 if (_write
                     && (entry.EntityState == EntityState.Unchanged
                         || (entry.EntityState == EntityState.Modified && !entry.IsModified(property))
-                        || (entry.EntityState == EntityState.Added && Equals(_originalValue, entry.GetCurrentValue(property)))))
+                        || (entry.EntityState == EntityState.Added
+                            && property.GetValueComparer().Equals(_originalValue, entry.GetCurrentValue(property)))))
                 {
                     entry.SetStoreGeneratedValue(property, _currentValue);
 

diff --git a/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs b/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs
@@ -208,21 +208,7 @@ private void LocalDetectChanges(InternalEntityEntry entry)
                     var current = entry[property];
                     var original = entry.GetOriginalValue(property);
 
-                    var comparer = property.GetValueComparer();
-
-                    if (comparer == null)
-                    {
-                        if (!Equals(current, original))
-                        {
-                            SetPropertyModified();
-                        }
-                    }
-                    else if (!comparer.Equals(current, original))
-                    {
-                        SetPropertyModified();
-                    }
-
-                    void SetPropertyModified()
+                    if (!property.GetValueComparer().Equals(current, original))
                     {
                         if (entry.EntityState == EntityState.Deleted)
                         {

diff --git a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs
@@ -124,7 +124,7 @@ private static bool PaintAction(
                     ? (isGenerated ? storeGenTargetState : targetState)
                     : EntityState.Added, // Key can only be not-set if it is store-generated
                 acceptChanges: true,
-                forceStateWhenUnknownKey: force ? (EntityState?)targetState : null);
+                forceStateWhenUnknownKey: force ? targetState : null);
 
             return true;
         }

diff --git a/src/EFCore/ChangeTracking/Internal/EntityReferenceMap.cs b/src/EFCore/ChangeTracking/Internal/EntityReferenceMap.cs
@@ -178,10 +178,7 @@ public virtual bool TryGet(
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
         public virtual int GetCountForState(
-            bool added = false,
-            bool modified = false,
-            bool deleted = false,
-            bool unchanged = false)
+            bool added, bool modified, bool deleted, bool unchanged, bool countSharedIdentity)
         {
             var count = 0;
 
@@ -200,7 +197,9 @@ public virtual int GetCountForState(
             if (deleted
                 && _deletedReferenceMap != null)
             {
-                count += _deletedReferenceMap.Count;
+                count += countSharedIdentity
+                    ? _deletedReferenceMap.Count
+                    : _deletedReferenceMap.Count(p => p.Value.SharedIdentityEntry == null);
             }
 
             if (unchanged
@@ -213,7 +212,7 @@ public virtual int GetCountForState(
             {
                 foreach (var map in _sharedTypeReferenceMap)
                 {
-                    count += map.Value.GetCountForState(added, modified, deleted, unchanged);
+                    count += map.Value.GetCountForState(added, modified, deleted, unchanged, countSharedIdentity);
                 }
             }
 
@@ -227,10 +226,7 @@ public virtual int GetCountForState(
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
         public virtual IEnumerable<InternalEntityEntry> GetEntriesForState(
-            bool added = false,
-            bool modified = false,
-            bool deleted = false,
-            bool unchanged = false)
+            bool added, bool modified, bool deleted, bool unchanged, bool returnSharedIdentity)
         {
             // Perf sensitive
 
@@ -298,7 +294,8 @@ var numberOfStates
             return GetEntriesForState(
                 added, modified, deleted, unchanged,
                 hasSharedTypes,
-                returnAdded, returnModified, returnDeleted, returnUnchanged);
+                returnAdded, returnModified, returnDeleted, returnUnchanged,
+                returnSharedIdentity);
         }
 
         private IEnumerable<InternalEntityEntry> GetEntriesForState(
@@ -310,7 +307,8 @@ private IEnumerable<InternalEntityEntry> GetEntriesForState(
             bool returnAdded,
             bool returnModified,
             bool returnDeleted,
-            bool returnUnchanged)
+            bool returnUnchanged,
+            bool returnSharedIdentity)
         {
             if (returnAdded)
             {
@@ -332,7 +330,11 @@ private IEnumerable<InternalEntityEntry> GetEntriesForState(
             {
                 foreach (var entry in _deletedReferenceMap!.Values)
                 {
-                    yield return entry;
+                    if (entry.SharedIdentityEntry == null
+                        || returnSharedIdentity)
+                    {
+                        yield return entry;
+                    }
                 }
             }
 
@@ -348,13 +350,9 @@ private IEnumerable<InternalEntityEntry> GetEntriesForState(
             {
                 foreach (var subMap in _sharedTypeReferenceMap!.Values)
                 {
-                    foreach (var entry in subMap.GetEntriesForState(added, modified, deleted, unchanged))
+                    foreach (var entry in subMap.GetEntriesForState(added, modified, deleted, unchanged, returnSharedIdentity))
                     {
-                        if ((entry.SharedIdentityEntry == null
-                            || entry.EntityState != EntityState.Deleted))
-                        {
-                            yield return entry;
-                        }
+                        yield return entry;
                     }
                 }
             }

diff --git a/src/EFCore/ChangeTracking/Internal/IStateManager.cs b/src/EFCore/ChangeTracking/Internal/IStateManager.cs
@@ -149,7 +149,8 @@ IEnumerable<InternalEntityEntry> GetEntriesForState(
             bool added = false,
             bool modified = false,
             bool deleted = false,
-            bool unchanged = false);
+            bool unchanged = false,
+            bool returnSharedIdentity = false);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -161,7 +162,8 @@ int GetCountForState(
             bool added = false,
             bool modified = false,
             bool deleted = false,
-            bool unchanged = false);
+            bool unchanged = false,
+            bool returnSharedIdentity = false);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
@@ -1391,13 +1391,7 @@ public void HandleNullForeignKey(
         }
 
         private static Func<object?, object?, bool> ValuesEqualFunc(IProperty property)
-        {
-            var comparer = property.GetValueComparer();
-
-            return comparer != null
-                ? (Func<object?, object?, bool>)((l, r) => comparer.Equals(l, r))
-                : (l, r) => Equals(l, r);
-        }
+            => property.GetValueComparer().Equals;
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

diff --git a/src/EFCore/ChangeTracking/Internal/OriginalValues.cs b/src/EFCore/ChangeTracking/Internal/OriginalValues.cs
@@ -104,11 +104,7 @@ public void AcceptChanges(InternalEntityEntry entry)
             }
 
             private static object? SnapshotValue(IProperty property, object? value)
-            {
-                var comparer = property.GetValueComparer();
-
-                return comparer == null ? value : comparer.Snapshot(value);
-            }
+                => property.GetValueComparer().Snapshot(value);
 
             public bool IsEmpty
                 => _values == null;

diff --git a/src/EFCore/ChangeTracking/Internal/SidecarValues.cs b/src/EFCore/ChangeTracking/Internal/SidecarValues.cs
@@ -50,11 +50,7 @@ public void SetValue(IProperty property, object? value, int index)
             }
 
             private static object? SnapshotValue(IProperty property, object? value)
-            {
-                var comparer = property.GetValueComparer();
-
-                return comparer == null ? value : comparer.Snapshot(value);
-            }
+                => property.GetValueComparer().Snapshot(value);
 
             public bool IsEmpty
                 => _values == null;

diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs
@@ -415,7 +415,7 @@ public virtual InternalEntityEntry StartTrackingFromQuery(
                     ? throwOnTypeMismatch
                         ? throw new InvalidOperationException(
                             CoreStrings.TrackingTypeMismatch(entry.EntityType.DisplayName(), entityType.DisplayName()))
-                        : (InternalEntityEntry?)null
+                        : null
                     : entry
                 : null;
 
@@ -496,8 +496,9 @@ public virtual int GetCountForState(
             bool added = false,
             bool modified = false,
             bool deleted = false,
-            bool unchanged = false)
-            => _entityReferenceMap.GetCountForState(added, modified, deleted, unchanged);
+            bool unchanged = false,
+            bool countSharedIdentity = false)
+            => _entityReferenceMap.GetCountForState(added, modified, deleted, unchanged, countSharedIdentity);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -506,7 +507,7 @@ public virtual int GetCountForState(
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
         public virtual int Count
-            => GetCountForState(added: true, modified: true, deleted: true, unchanged: true);
+            => GetCountForState(added: true, modified: true, deleted: true, unchanged: true, countSharedIdentity: true);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -518,8 +519,9 @@ public virtual IEnumerable<InternalEntityEntry> GetEntriesForState(
             bool added = false,
             bool modified = false,
             bool deleted = false,
-            bool unchanged = false)
-            => _entityReferenceMap.GetEntriesForState(added, modified, deleted, unchanged);
+            bool unchanged = false,
+            bool returnSharedIdentity = false)
+            => _entityReferenceMap.GetEntriesForState(added, modified, deleted, unchanged, returnSharedIdentity);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -528,7 +530,7 @@ public virtual IEnumerable<InternalEntityEntry> GetEntriesForState(
         ///     doing so can result in application failures when updating to a new Entity Framework Core release.
         /// </summary>
         public virtual IEnumerable<InternalEntityEntry> Entries
-            => GetEntriesForState(added: true, modified: true, deleted: true, unchanged: true);
+            => GetEntriesForState(added: true, modified: true, deleted: true, unchanged: true, returnSharedIdentity: true);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

diff --git a/src/EFCore/ChangeTracking/Internal/StateManagerExtensions.cs b/src/EFCore/ChangeTracking/Internal/StateManagerExtensions.cs
@@ -24,12 +24,13 @@ public static IReadOnlyList<InternalEntityEntry> ToListForState(
             bool added = false,
             bool modified = false,
             bool deleted = false,
-            bool unchanged = false)
+            bool unchanged = false,
+            bool returnSharedIdentity = false)
         {
             var list = new List<InternalEntityEntry>(
-                stateManager.GetCountForState(added, modified, deleted, unchanged));
+                stateManager.GetCountForState(added, modified, deleted, unchanged, returnSharedIdentity));
 
-            foreach (var entry in stateManager.GetEntriesForState(added, modified, deleted, unchanged))
+            foreach (var entry in stateManager.GetEntriesForState(added, modified, deleted, unchanged, returnSharedIdentity))
             {
                 list.Add(entry);
             }
@@ -45,6 +46,6 @@ public static IReadOnlyList<InternalEntityEntry> ToListForState(
         /// </summary>
         public static IReadOnlyList<InternalEntityEntry> ToList(
             this IStateManager stateManager)
-            => stateManager.ToListForState(added: true, modified: true, deleted: true, unchanged: true);
+            => stateManager.ToListForState(added: true, modified: true, deleted: true, unchanged: true, returnSharedIdentity: true);
     }
 }
diff --git a/src/EFCore/Extensions/PropertyBaseExtensions.cs b/src/EFCore/Extensions/PropertyBaseExtensions.cs
@@ -33,7 +33,7 @@ public static bool IsShadowProperty(this IPropertyBase property)
         ///     when throwing exceptions about keys, indexes, etc. that use the properties.
         /// </summary>
         /// <param name="properties"> The properties to format. </param>
-        /// <param name="includeTypes"> If true, then type names are included in the string. The default is <see langword="false" />.</param>
+        /// <param name="includeTypes"> If true, then type names are included in the string. The default is <see langword="false" />. </param>
         /// <returns> The string representation. </returns>
         public static string Format(this IEnumerable<IReadOnlyPropertyBase> properties, bool includeTypes = false)
             => "{"

diff --git a/src/EFCore/Metadata/IReadOnlyIndex.cs b/src/EFCore/Metadata/IReadOnlyIndex.cs
@@ -68,7 +68,7 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt
                     ", ",
                     Properties.Select(
                         p => singleLine
-                            ? p.DeclaringEntityType.DisplayName() + "." + p.Name
+                            ? p.DeclaringEntityType.DisplayName(omitSharedType: true) + "." + p.Name
                             : p.Name));
 
             builder.Append(" " + Name ?? "<unnamed>");

diff --git a/src/EFCore/Metadata/IReadOnlyKey.cs b/src/EFCore/Metadata/IReadOnlyKey.cs
@@ -69,7 +69,7 @@ string ToDebugString(MetadataDebugStringOptions options = MetadataDebugStringOpt
             builder.AppendJoin(
                 ", ", Properties.Select(
                     p => singleLine
-                        ? p.DeclaringEntityType.DisplayName() + "." + p.Name
+                        ? p.DeclaringEntityType.DisplayName(omitSharedType: true) + "." + p.Name
                         : p.Name));
 
             if (IsPrimaryKey())