Identity resolution for loading detached entities

Part of #10042

Avoids duplicates when loading collections that already contain some entities. Entities queried using NoTrackingWithIdentityResolution get this behavior when lazy-loading.

src/EFCore.Proxies/Proxies/Internal/ProxyFactory.cs

@@ -79,10 +79,7 @@ public virtual object CreateLazyLoadingProxy(
             throw new InvalidOperationException(ProxiesStrings.ProxyServicesMissing);
         }
 
-        return CreateLazyLoadingProxy(
-            entityType,
-            context.GetService<ILazyLoader>(),
-            constructorArguments);
+        return CreateLazyLoadingProxy(entityType, loader, constructorArguments);
     }
 
     private object CreateLazyLoadingProxy(

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

@@ -1,10 +1,8 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Collections;
 using System.Runtime.CompilerServices;
 using System.Text.Json;
-using System.Xml.Linq;
 using Microsoft.EntityFrameworkCore.Internal;
 using Microsoft.EntityFrameworkCore.Query.Internal;
 
@@ -308,7 +306,8 @@ void ProcessCurrentElementRow()
                     if (!trackingQuery)
                     {
                         fixup(entity, relatedEntity);
-                        if (inverseNavigation != null)
+                        if (inverseNavigation != null
+                            && !inverseNavigation.IsCollection)
                         {
                             inverseNavigation.SetIsLoadedWhenNoTracking(relatedEntity);
                         }
@@ -477,7 +476,9 @@ private static async Task PopulateSplitIncludeCollectionAsync<TIncludingEntity,
                 // Execute and fetch data reader
                 var dataReader = await executionStrategy.ExecuteAsync(
                         (queryContext, relationalCommandCache, readerColumns, detailedErrorsEnabled),
-                        ((RelationalQueryContext, RelationalCommandCache, IReadOnlyList<ReaderColumn?>?, bool) tup, CancellationToken cancellationToken)
+                        (
+                                (RelationalQueryContext, RelationalCommandCache, IReadOnlyList<ReaderColumn?>?, bool) tup,
+                                CancellationToken cancellationToken)
                             => InitializeReaderAsync(tup.Item1, tup.Item2, tup.Item3, tup.Item4, cancellationToken),
                         verifySucceeded: null,
                         queryContext.CancellationToken)
@@ -800,7 +801,9 @@ private static async Task PopulateSplitCollectionAsync<TCollection, TElement, TR
                 // Execute and fetch data reader
                 var dataReader = await executionStrategy.ExecuteAsync(
                         (queryContext, relationalCommandCache, readerColumns, detailedErrorsEnabled),
-                        ((RelationalQueryContext, RelationalCommandCache, IReadOnlyList<ReaderColumn?>?, bool) tup, CancellationToken cancellationToken)
+                        (
+                                (RelationalQueryContext, RelationalCommandCache, IReadOnlyList<ReaderColumn?>?, bool) tup,
+                                CancellationToken cancellationToken)
                             => InitializeReaderAsync(tup.Item1, tup.Item2, tup.Item3, tup.Item4, cancellationToken),
                         verifySucceeded: null,
                         queryContext.CancellationToken)
@@ -929,7 +932,7 @@ private static void IncludeJsonEntityCollection<TIncludingEntity, TIncludedColle
 
             if (nullable)
             {
-                return default(TEntity);
+                return default;
             }
 
             throw new InvalidOperationException(
@@ -966,7 +969,7 @@ private static void IncludeJsonEntityCollection<TIncludingEntity, TIncludedColle
                 return result;
             }
 
-            return default(TResult);
+            return default;
         }
 
         private static async Task TaskAwaiter(Func<Task>[] taskFactories)

src/EFCore/ChangeTracking/CollectionEntry.cs

@@ -211,7 +211,16 @@ public override bool IsModified
     /// </summary>
     /// <remarks>
     ///     <para>
-    ///         Note that entities that are already being tracked are not overwritten with new data from the database.
+    ///         If the the entity represented by this entry is tracked, then entities with the same primary key value are not replaced
+    ///         by new entities or overwritten with new data from the database. If the entity entity represented by this entry is not
+    ///         tracked and the collection already contains entities, then calling this method will result in duplicate
+    ///         instances in the collection or inverse collection for any entities with the same key value.
+    ///         Use <see cref="LoadWithIdentityResolution" /> to avoid getting these duplicates.
+    ///     </para>
+    ///     <para>
+    ///         For tracked entities, this method behaves in the same way and has the same performance as
+    ///         <see cref="LoadWithIdentityResolution" />. For entities that are not tracked, this method can be faster than
+    ///         <see cref="LoadWithIdentityResolution" />.
     ///     </para>
     ///     <para>
     ///         See <see href="https://aka.ms/efcore-docs-entity-entries">Accessing tracked entities in EF Core</see>
@@ -224,7 +233,35 @@ public override void Load()
 
         if (!IsLoaded)
         {
-            TargetLoader.Load(InternalEntry);
+            TargetLoader.Load(InternalEntry, forceIdentityResolution: false);
+        }
+    }
+
+    /// <summary>
+    ///     Loads the entities referenced by this navigation property, unless <see cref="NavigationEntry.IsLoaded" />
+    ///     is already set to true.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         Entities with the same primary key value are not replaced by new entities or overwritten with new data from the database.
+    ///         This navigation and its inverse will not contain duplicate entities.
+    ///     </para>
+    ///     <para>
+    ///         For tracked entities, this method behaves in the same way and has the same performance as
+    ///         <see cref="Load" />. For entities that are not tracked, this method can be slower than <see cref="Load" />.
+    ///     </para>
+    ///     <para>
+    ///         See <see href="https://aka.ms/efcore-docs-entity-entries">Accessing tracked entities in EF Core</see>
+    ///         and <see href="https://aka.ms/efcore-docs-load-related-data">Loading related entities</see> for more information and examples.
+    ///     </para>
+    /// </remarks>
+    public override void LoadWithIdentityResolution()
+    {
+        EnsureInitialized();
+
+        if (!IsLoaded)
+        {
+            TargetLoader.Load(InternalEntry, forceIdentityResolution: true);
         }
     }
 
@@ -234,7 +271,16 @@ public override void Load()
     /// </summary>
     /// <remarks>
     ///     <para>
-    ///         Note that entities that are already being tracked are not overwritten with new data from the database.
+    ///         If the the entity represented by this entry is tracked, then entities with the same primary key value are not replaced
+    ///         by new entities or overwritten with new data from the database. If the entity entity represented by this entry is not
+    ///         tracked and the collection already contains entities, then calling this method will result in duplicate
+    ///         instances in the collection or inverse collection for any entities with the same key value.
+    ///         Use <see cref="LoadWithIdentityResolutionAsync" /> to avoid getting these duplicates.
+    ///     </para>
+    ///     <para>
+    ///         For tracked entities, this method behaves in the same way and has the same performance as
+    ///         <see cref="LoadWithIdentityResolutionAsync" />. For entities that are not tracked, this method can be faster than
+    ///         <see cref="LoadWithIdentityResolutionAsync" />.
     ///     </para>
     ///     <para>
     ///         Multiple active operations on the same context instance are not supported. Use <see langword="await" /> to ensure
@@ -254,7 +300,41 @@ public override Task LoadAsync(CancellationToken cancellationToken = default)
 
         return IsLoaded
             ? Task.CompletedTask
-            : TargetLoader.LoadAsync(InternalEntry, cancellationToken);
+            : TargetLoader.LoadAsync(InternalEntry, forceIdentityResolution: false, cancellationToken);
+    }
+
+    /// <summary>
+    ///     Loads entities referenced by this navigation property, unless <see cref="NavigationEntry.IsLoaded" />
+    ///     is already set to true.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         Entities with the same primary key value are not replaced by new entities or overwritten with new data from the database.
+    ///         This navigation and its inverse will not contain duplicate entities.
+    ///     </para>
+    ///     <para>
+    ///         For tracked entities, this method behaves in the same way and has the same performance as
+    ///         <see cref="LoadAsync" />. For entities that are not tracked, this method can be slower than <see cref="LoadAsync" />.
+    ///     </para>
+    ///     <para>
+    ///         Multiple active operations on the same context instance are not supported. Use <see langword="await" /> to ensure
+    ///         that any asynchronous operations have completed before calling another method on this context.
+    ///     </para>
+    ///     <para>
+    ///         See <see href="https://aka.ms/efcore-docs-entity-entries">Accessing tracked entities in EF Core</see>
+    ///         and <see href="https://aka.ms/efcore-docs-load-related-data">Loading related entities</see> for more information and examples.
+    ///     </para>
+    /// </remarks>
+    /// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
+    /// <returns>A task that represents the asynchronous save operation.</returns>
+    /// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
+    public override Task LoadWithIdentityResolutionAsync(CancellationToken cancellationToken = default)
+    {
+        EnsureInitialized();
+
+        return IsLoaded
+            ? Task.CompletedTask
+            : TargetLoader.LoadAsync(InternalEntry, forceIdentityResolution: true, cancellationToken);
     }
 
     /// <summary>

src/EFCore/ChangeTracking/NavigationEntry.cs

@@ -85,12 +85,21 @@ private static INavigationBase GetNavigation(InternalEntityEntry internalEntry,
     }
 
     /// <summary>
-    ///     Loads the entity or entities referenced by this navigation property, unless <see cref="IsLoaded" />
+    ///     Loads the entities referenced by this navigation property, unless <see cref="NavigationEntry.IsLoaded" />
     ///     is already set to true.
     /// </summary>
     /// <remarks>
     ///     <para>
-    ///         Note that entities that are already being tracked are not overwritten with new data from the database.
+    ///         If the the entity represented by this entry is tracked, then entities with the same primary key value are not replaced
+    ///         by new entities or overwritten with new data from the database. If the entity entity represented by this entry is not
+    ///         tracked and the collection already contains entities, then calling this method will result in duplicate
+    ///         instances in the collection or inverse collection for any entities with the same key value.
+    ///         Use <see cref="LoadWithIdentityResolution" /> to avoid getting these duplicates.
+    ///     </para>
+    ///     <para>
+    ///         For tracked entities, this method behaves in the same way and has the same performance as
+    ///         <see cref="LoadWithIdentityResolution" />. For entities that are not tracked, this method can be faster than
+    ///         <see cref="LoadWithIdentityResolution" />.
     ///     </para>
     ///     <para>
     ///         See <see href="https://aka.ms/efcore-docs-entity-entries">Accessing tracked entities in EF Core</see>
@@ -100,12 +109,41 @@ private static INavigationBase GetNavigation(InternalEntityEntry internalEntry,
     public abstract void Load();
 
     /// <summary>
-    ///     Loads the entity or entities referenced by this navigation property, unless <see cref="IsLoaded" />
+    ///     Loads the entities referenced by this navigation property, unless <see cref="NavigationEntry.IsLoaded" />
     ///     is already set to true.
     /// </summary>
     /// <remarks>
     ///     <para>
-    ///         Note that entities that are already being tracked are not overwritten with new data from the database.
+    ///         Entities with the same primary key value are not replaced by new entities or overwritten with new data from the database.
+    ///         This navigation and its inverse will not contain duplicate entities.
+    ///     </para>
+    ///     <para>
+    ///         For tracked entities, this method behaves in the same way and has the same performance as
+    ///         <see cref="Load" />. For entities that are not tracked, this method can be slower than <see cref="Load" />.
+    ///     </para>
+    ///     <para>
+    ///         See <see href="https://aka.ms/efcore-docs-entity-entries">Accessing tracked entities in EF Core</see>
+    ///         and <see href="https://aka.ms/efcore-docs-load-related-data">Loading related entities</see> for more information and examples.
+    ///     </para>
+    /// </remarks>
+    public abstract void LoadWithIdentityResolution();
+
+    /// <summary>
+    ///     Loads entities referenced by this navigation property, unless <see cref="NavigationEntry.IsLoaded" />
+    ///     is already set to true.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         If the the entity represented by this entry is tracked, then entities with the same primary key value are not replaced
+    ///         by new entities or overwritten with new data from the database. If the entity entity represented by this entry is not
+    ///         tracked and the collection already contains entities, then calling this method will result in duplicate
+    ///         instances in the collection or inverse collection for any entities with the same key value.
+    ///         Use <see cref="LoadWithIdentityResolutionAsync" /> to avoid getting these duplicates.
+    ///     </para>
+    ///     <para>
+    ///         For tracked entities, this method behaves in the same way and has the same performance as
+    ///         <see cref="LoadWithIdentityResolutionAsync" />. For entities that are not tracked, this method can be faster than
+    ///         <see cref="LoadWithIdentityResolutionAsync" />.
     ///     </para>
     ///     <para>
     ///         Multiple active operations on the same context instance are not supported. Use <see langword="await" /> to ensure
@@ -117,10 +155,37 @@ private static INavigationBase GetNavigation(InternalEntityEntry internalEntry,
     ///     </para>
     /// </remarks>
     /// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
-    /// <returns>A task that represents the asynchronous operation.</returns>
+    /// <returns>A task that represents the asynchronous save operation.</returns>
     /// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
     public abstract Task LoadAsync(CancellationToken cancellationToken = default);
 
+    /// <summary>
+    ///     Loads entities referenced by this navigation property, unless <see cref="NavigationEntry.IsLoaded" />
+    ///     is already set to true.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         Entities with the same primary key value are not replaced by new entities or overwritten with new data from the database.
+    ///         This navigation and its inverse will not contain duplicate entities.
+    ///     </para>
+    ///     <para>
+    ///         For tracked entities, this method behaves in the same way and has the same performance as
+    ///         <see cref="LoadAsync" />. For entities that are not tracked, this method can be slower than <see cref="LoadAsync" />.
+    ///     </para>
+    ///     <para>
+    ///         Multiple active operations on the same context instance are not supported. Use <see langword="await" /> to ensure
+    ///         that any asynchronous operations have completed before calling another method on this context.
+    ///     </para>
+    ///     <para>
+    ///         See <see href="https://aka.ms/efcore-docs-entity-entries">Accessing tracked entities in EF Core</see>
+    ///         and <see href="https://aka.ms/efcore-docs-load-related-data">Loading related entities</see> for more information and examples.
+    ///     </para>
+    /// </remarks>
+    /// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
+    /// <returns>A task that represents the asynchronous save operation.</returns>
+    /// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
+    public abstract Task LoadWithIdentityResolutionAsync(CancellationToken cancellationToken = default);
+
     /// <summary>
     ///     Returns the query that would be used by <see cref="Load" /> to load entities referenced by
     ///     this navigation property.