From 9194b5a442d97431cd157fc4a809654f142c8cc9 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Wed, 1 Jun 2022 13:41:39 +0100 Subject: [PATCH] Interceptor for identity resolution when attaching graphs Part of #626 Fixes #20124 This is essentially a call back that happens when an instance withe the same key as an existing tracked instance is attached. The existing instance is always retained, but the interceptor gets the chance to change property values of the tracked instance. This will typically be used to copy over values from the duplicate instance (i.e. painting from a detached entity) or just skip the duplicate. Interceptor implementations are provided to do these two things. --- .../Storage/Internal/InMemoryTable.cs | 2 +- .../IPrincipalKeyValueFactory.cs | 4 +- .../ChangeTracking/Internal/DependentsMap.cs | 2 +- .../Internal/EntityGraphAttacher.cs | 57 ++- .../ChangeTracking/Internal/IIdentityMap.cs | 8 + .../ChangeTracking/Internal/IStateManager.cs | 23 ++ .../ChangeTracking/Internal/IdentityMap.cs | 20 +- .../Internal/InternalEntityEntry.cs | 11 + .../Internal/NavigationFixer.cs | 18 +- .../ChangeTracking/Internal/StateManager.cs | 87 +++++ .../Internal/StateManagerDependencies.cs | 12 +- .../CopyingIdentityResolutionInterceptor.cs | 42 +++ .../IIdentityResolutionInterceptor.cs | 42 +++ .../Diagnostics/ISaveChangesInterceptor.cs | 2 +- ...IdentityResolutionInterceptorAggregator.cs | 35 ++ .../SkippingIdentityResolutionInterceptor.cs | 23 ++ .../EntityFrameworkServicesBuilder.cs | 1 + .../GraphUpdates/GraphUpdatesTestBase.cs | 59 ++- .../GraphUpdatesTestBaseMiscellaneous.cs | 135 ++++++- .../ProxyGraphUpdatesFixtureBase.cs | 109 ++++++ .../ProxyGraphUpdatesTestBaseMiscellaneous.cs | 120 ++++++ .../ChangeTracking/ChangeTrackerTest.cs | 60 ++- .../ChangeTracking/Internal/FixupTest.cs | 351 +++++++++++++++++- .../Internal/StateManagerTest.cs | 161 ++++++++ .../TestUtilities/FakeStateManager.cs | 16 +- 25 files changed, 1309 insertions(+), 91 deletions(-) create mode 100644 src/EFCore/Diagnostics/CopyingIdentityResolutionInterceptor.cs create mode 100644 src/EFCore/Diagnostics/IIdentityResolutionInterceptor.cs create mode 100644 src/EFCore/Diagnostics/Internal/IdentityResolutionInterceptorAggregator.cs create mode 100644 src/EFCore/Diagnostics/SkippingIdentityResolutionInterceptor.cs diff --git a/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs b/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs index 57d7bbb1c67..5c771d60648 100644 --- a/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs +++ b/src/EFCore.InMemory/Storage/Internal/InMemoryTable.cs @@ -332,7 +332,7 @@ public virtual void BumpValueGenerators(object?[] row) } private TKey CreateKey(IUpdateEntry entry) - => _keyValueFactory.CreateFromCurrentValues(entry); + => _keyValueFactory.CreateFromCurrentValues(entry)!; private static object? SnapshotValue(IProperty property, ValueComparer? comparer, IUpdateEntry entry) { diff --git a/src/EFCore/ChangeTracking/IPrincipalKeyValueFactory.cs b/src/EFCore/ChangeTracking/IPrincipalKeyValueFactory.cs index 1ab47112cf0..ed4b54be8be 100644 --- a/src/EFCore/ChangeTracking/IPrincipalKeyValueFactory.cs +++ b/src/EFCore/ChangeTracking/IPrincipalKeyValueFactory.cs @@ -45,7 +45,7 @@ public interface IPrincipalKeyValueFactory /// /// The entry tracking an entity instance. /// The key value. - TKey CreateFromCurrentValues(IUpdateEntry entry); + TKey? CreateFromCurrentValues(IUpdateEntry entry); /// /// Finds the first null key value in the given entry and returns the associated . @@ -59,7 +59,7 @@ public interface IPrincipalKeyValueFactory /// /// The entry tracking an entity instance. /// The key value. - TKey CreateFromOriginalValues(IUpdateEntry entry); + TKey? CreateFromOriginalValues(IUpdateEntry entry); /// /// Creates a key object from the relationship snapshot key values in the given entry. diff --git a/src/EFCore/ChangeTracking/Internal/DependentsMap.cs b/src/EFCore/ChangeTracking/Internal/DependentsMap.cs index 33ad77eb36c..69c04e3cb81 100644 --- a/src/EFCore/ChangeTracking/Internal/DependentsMap.cs +++ b/src/EFCore/ChangeTracking/Internal/DependentsMap.cs @@ -126,7 +126,7 @@ private bool TryCreateFromCurrentValues(IUpdateEntry entry, [NotNullWhen(true)] /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual IEnumerable GetDependents(IUpdateEntry principalEntry) - => _map.TryGetValue(_principalKeyValueFactory.CreateFromCurrentValues(principalEntry), out var dependents) + => _map.TryGetValue(_principalKeyValueFactory.CreateFromCurrentValues(principalEntry)!, out var dependents) ? dependents : Enumerable.Empty(); diff --git a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs index ef69f970168..562c6006373 100644 --- a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs +++ b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs @@ -12,6 +12,7 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal; public class EntityGraphAttacher : IEntityGraphAttacher { private readonly IEntityEntryGraphIterator _graphIterator; + private HashSet? _visited; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -49,10 +50,12 @@ public virtual void AttachGraph( null), PaintAction); + _visited = null; rootEntry.StateManager.CompleteAttachGraph(); } catch { + _visited = null; rootEntry.StateManager.AbortAttachGraph(); throw; } @@ -84,22 +87,25 @@ await _graphIterator.TraverseGraphAsync( PaintActionAsync, cancellationToken).ConfigureAwait(false); + _visited = null; rootEntry.StateManager.CompleteAttachGraph(); } catch { + _visited = null; rootEntry.StateManager.AbortAttachGraph(); throw; } } - private static bool PaintAction( + private bool PaintAction( EntityEntryGraphNode<(EntityState TargetState, EntityState StoreGenTargetState, bool Force)> node) { SetReferenceLoaded(node); var internalEntityEntry = node.GetInfrastructure(); - if (internalEntityEntry.EntityState != EntityState.Detached) + if (internalEntityEntry.EntityState != EntityState.Detached + || (_visited != null && _visited.Contains(internalEntityEntry.Entity))) { return false; } @@ -108,24 +114,32 @@ private static bool PaintAction( var (isGenerated, isSet) = internalEntityEntry.IsKeySet; - internalEntityEntry.SetEntityState( - isSet - ? (isGenerated ? storeGenTargetState : targetState) - : EntityState.Added, // Key can only be not-set if it is store-generated - acceptChanges: true, - forceStateWhenUnknownKey: force ? targetState : null); + if (internalEntityEntry.ResolveToExistingEntry(node.InboundNavigation, node.SourceEntry?.GetInfrastructure())) + { + (_visited ??= new HashSet()).Add(internalEntityEntry.Entity); + } + else + { + internalEntityEntry.SetEntityState( + isSet + ? (isGenerated ? storeGenTargetState : targetState) + : EntityState.Added, // Key can only be not-set if it is store-generated + acceptChanges: true, + forceStateWhenUnknownKey: force ? targetState : null); + } return true; } - private static async Task PaintActionAsync( + private async Task PaintActionAsync( EntityEntryGraphNode<(EntityState TargetState, EntityState StoreGenTargetState, bool Force)> node, CancellationToken cancellationToken) { SetReferenceLoaded(node); var internalEntityEntry = node.GetInfrastructure(); - if (internalEntityEntry.EntityState != EntityState.Detached) + if (internalEntityEntry.EntityState != EntityState.Detached + || (_visited != null && _visited.Contains(internalEntityEntry.Entity))) { return false; } @@ -134,14 +148,21 @@ private static async Task PaintActionAsync( var (isGenerated, isSet) = internalEntityEntry.IsKeySet; - await internalEntityEntry.SetEntityStateAsync( - isSet - ? (isGenerated ? storeGenTargetState : targetState) - : EntityState.Added, // Key can only be not-set if it is store-generated - acceptChanges: true, - forceStateWhenUnknownKey: force ? targetState : null, - cancellationToken: cancellationToken) - .ConfigureAwait(false); + if (internalEntityEntry.ResolveToExistingEntry(node.InboundNavigation, node.SourceEntry?.GetInfrastructure())) + { + (_visited ??= new HashSet()).Add(internalEntityEntry.Entity); + } + else + { + await internalEntityEntry.SetEntityStateAsync( + isSet + ? (isGenerated ? storeGenTargetState : targetState) + : EntityState.Added, // Key can only be not-set if it is store-generated + acceptChanges: true, + forceStateWhenUnknownKey: force ? targetState : null, + cancellationToken: cancellationToken) + .ConfigureAwait(false); + } return true; } diff --git a/src/EFCore/ChangeTracking/Internal/IIdentityMap.cs b/src/EFCore/ChangeTracking/Internal/IIdentityMap.cs index 1235ccc6a8c..39b2761bc83 100644 --- a/src/EFCore/ChangeTracking/Internal/IIdentityMap.cs +++ b/src/EFCore/ChangeTracking/Internal/IIdentityMap.cs @@ -35,6 +35,14 @@ public interface IIdentityMap /// bool Contains(IForeignKey foreignKey, in ValueBuffer valueBuffer); + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + InternalEntityEntry? TryGetEntry(InternalEntityEntry entry); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/ChangeTracking/Internal/IStateManager.cs b/src/EFCore/ChangeTracking/Internal/IStateManager.cs index 7e7b5d0da39..1f69a9b167a 100644 --- a/src/EFCore/ChangeTracking/Internal/IStateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/IStateManager.cs @@ -252,6 +252,29 @@ void RecordReferencedUntrackedEntity( INavigationBase navigation, InternalEntityEntry referencedFromEntry); + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + void UpdateReferencedUntrackedEntity( + object referencedEntity, + object newReferencedEntity, + INavigationBase navigation, + InternalEntityEntry referencedFromEntry); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + bool ResolveToExistingEntry( + InternalEntityEntry newEntry, + INavigationBase? navigation, + InternalEntityEntry? referencedFromEntry); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/ChangeTracking/Internal/IdentityMap.cs b/src/EFCore/ChangeTracking/Internal/IdentityMap.cs index 42dd890b6dd..655887c02dc 100644 --- a/src/EFCore/ChangeTracking/Internal/IdentityMap.cs +++ b/src/EFCore/ChangeTracking/Internal/IdentityMap.cs @@ -83,6 +83,20 @@ public virtual bool Contains(IForeignKey foreignKey, in ValueBuffer valueBuffer) => foreignKey.GetDependentKeyValueFactory()!.TryCreateFromBuffer(valueBuffer, out var key) && _identityMap.ContainsKey(key); + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + public virtual InternalEntityEntry? TryGetEntry(InternalEntityEntry entry) + { + var key = PrincipalKeyValueFactory.CreateFromCurrentValues(entry); + return key != null && _identityMap.TryGetValue(key, out var existingEntry) + ? existingEntry + : null; + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -194,7 +208,7 @@ public virtual bool Contains(IForeignKey foreignKey, in ValueBuffer valueBuffer) /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual void AddOrUpdate(InternalEntityEntry entry) - => Add(PrincipalKeyValueFactory.CreateFromCurrentValues(entry), entry, updateDuplicate: true); + => Add(PrincipalKeyValueFactory.CreateFromCurrentValues(entry)!, entry, updateDuplicate: true); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -203,7 +217,7 @@ public virtual void AddOrUpdate(InternalEntityEntry entry) /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual void Add(InternalEntityEntry entry) - => Add(PrincipalKeyValueFactory.CreateFromCurrentValues(entry), entry); + => Add(PrincipalKeyValueFactory.CreateFromCurrentValues(entry)!, entry); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -371,7 +385,7 @@ public virtual void Clear() /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual void Remove(InternalEntityEntry entry) - => Remove(PrincipalKeyValueFactory.CreateFromCurrentValues(entry), entry); + => Remove(PrincipalKeyValueFactory.CreateFromCurrentValues(entry)!, entry); /// /// 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 index 444ecec374d..daa39e16919 100644 --- a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs +++ b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs @@ -428,6 +428,17 @@ private void SetServiceProperties(EntityState oldState, EntityState newState) } } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + public bool ResolveToExistingEntry( + INavigationBase? navigation, + InternalEntityEntry? referencedFromEntry) + => StateManager.ResolveToExistingEntry(this, navigation, referencedFromEntry); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index 4d4da6d7479..87122438810 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -955,14 +955,24 @@ private void DelayedFixup( FixupToDependent(entry, referencedEntry, navigation.ForeignKey, setModified, fromQuery); } } - else if (referencedEntry.Entity == navigationValue) + else { - FixupToDependent(entry, referencedEntry, navigation.ForeignKey, setModified, fromQuery); + FixupToDependent( + entry, + referencedEntry, + navigation.ForeignKey, + referencedEntry.Entity == navigationValue && setModified, + fromQuery); } } - else if (referencedEntry.Entity == navigationValue) + else { - FixupToPrincipal(entry, referencedEntry, navigation.ForeignKey, setModified, fromQuery); + FixupToPrincipal( + entry, + referencedEntry, + navigation.ForeignKey, + referencedEntry.Entity == navigationValue && setModified, + fromQuery); FixupSkipNavigations(entry, navigation.ForeignKey, fromQuery); } diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index 96ace17161e..e3101a7ab1b 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -1,6 +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.Collections; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata.Internal; @@ -29,6 +30,7 @@ public class StateManager : IStateManager private readonly IModel _model; private readonly IDatabase _database; private readonly IConcurrencyDetector? _concurrencyDetector; + private readonly IIdentityResolutionInterceptor? _resolutionInterceptor; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -60,6 +62,8 @@ public StateManager(StateManagerDependencies dependencies) UpdateLogger = dependencies.UpdateLogger; _changeTrackingLogger = dependencies.ChangeTrackingLogger; + + _resolutionInterceptor = dependencies.Interceptors.Aggregate(); } /// @@ -731,6 +735,89 @@ public virtual void RecordReferencedUntrackedEntity( danglers.Add(Tuple.Create(navigation, referencedFromEntry)); } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + public virtual void UpdateReferencedUntrackedEntity( + object referencedEntity, + object newReferencedEntity, + INavigationBase navigation, + InternalEntityEntry referencedFromEntry) + { + if (_referencedUntrackedEntities != null + && _referencedUntrackedEntities.TryGetValue(referencedEntity, out var danglers)) + { + _referencedUntrackedEntities.Remove(referencedEntity); + + if (!_referencedUntrackedEntities.TryGetValue(newReferencedEntity, out var newDanglers)) + { + newDanglers = new List>(); + _referencedUntrackedEntities.Add(newReferencedEntity, newDanglers); + } + + foreach (var dangler in danglers) + { + newDanglers.Add(dangler); + } + } + } + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + public virtual bool ResolveToExistingEntry( + InternalEntityEntry newEntry, + INavigationBase? navigation, + InternalEntityEntry? referencedFromEntry) + { + if (_resolutionInterceptor != null) + { + var needsTracking = false; + foreach (var key in newEntry.EntityType.GetKeys()) + { + var existingEntry = FindIdentityMap(key)?.TryGetEntry(newEntry); + if (existingEntry != null) + { + _resolutionInterceptor.UpdateTrackedInstance( + Context, + new EntityEntry(existingEntry), + newEntry.Entity); + + if (navigation != null) + { + UpdateReferencedUntrackedEntity( + newEntry.Entity, + existingEntry.Entity, + navigation, + referencedFromEntry!); + + var navigationValue = referencedFromEntry![navigation]; + if (navigationValue != null && navigation.IsCollection) + { + navigation.GetCollectionAccessor()!.Remove(referencedFromEntry.Entity, newEntry.Entity); + } + } + + InternalEntityEntryNotifier.StateChanged(existingEntry, EntityState.Detached, fromQuery: false); + } + else + { + needsTracking = true; + } + } + + return !needsTracking; + } + + return false; + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/ChangeTracking/Internal/StateManagerDependencies.cs b/src/EFCore/ChangeTracking/Internal/StateManagerDependencies.cs index e3517f58ab7..f5c59224140 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManagerDependencies.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManagerDependencies.cs @@ -63,7 +63,8 @@ public StateManagerDependencies( ILoggingOptions loggingOptions, IDiagnosticsLogger updateLogger, IDiagnosticsLogger changeTrackingLogger, - INavigationFixer navigationFixer) + INavigationFixer navigationFixer, + IInterceptors interceptors) { InternalEntityEntrySubscriber = internalEntityEntrySubscriber; InternalEntityEntryNotifier = internalEntityEntryNotifier; @@ -81,6 +82,7 @@ public StateManagerDependencies( UpdateLogger = updateLogger; ChangeTrackingLogger = changeTrackingLogger; NavigationFixer = navigationFixer; + Interceptors = interceptors; } /// @@ -212,4 +214,12 @@ public StateManagerDependencies( /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public INavigationFixer NavigationFixer { get; init; } + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// 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. + /// + public IInterceptors Interceptors { get; } } diff --git a/src/EFCore/Diagnostics/CopyingIdentityResolutionInterceptor.cs b/src/EFCore/Diagnostics/CopyingIdentityResolutionInterceptor.cs new file mode 100644 index 00000000000..baad84f3bbe --- /dev/null +++ b/src/EFCore/Diagnostics/CopyingIdentityResolutionInterceptor.cs @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Diagnostics; + +/// +/// A that copies property values from the new entity instance into the +/// tracked entity instance. +/// +public class CopyingIdentityResolutionInterceptor : IIdentityResolutionInterceptor +{ + /// + /// Called when a attempts to track a new instance of an entity with the same primary key value as + /// an already tracked instance. This implementation copies property values from the new entity instance into the + /// tracked entity instance. + /// + /// The is use. + /// The entry for the existing tracked entity instance. + /// The new entity instance, which will be discarded after this call. + public virtual void UpdateTrackedInstance(DbContext context, EntityEntry existingEntry, object newInstance) + { + var tempEntry = context.Entry(newInstance); + + if (existingEntry.State == EntityState.Added) + { + foreach (var propertyEntry in tempEntry.Properties.Where( + e => e.Metadata.GetBeforeSaveBehavior() != PropertySaveBehavior.Throw)) + { + existingEntry.Property(propertyEntry.Metadata.Name).CurrentValue = propertyEntry.CurrentValue; + } + } + else + { + foreach (var propertyEntry in tempEntry.Properties.Where( + e => e.Metadata.GetAfterSaveBehavior() != PropertySaveBehavior.Throw)) + { + existingEntry.Property(propertyEntry.Metadata.Name).CurrentValue = propertyEntry.CurrentValue; + } + } + + } +} diff --git a/src/EFCore/Diagnostics/IIdentityResolutionInterceptor.cs b/src/EFCore/Diagnostics/IIdentityResolutionInterceptor.cs new file mode 100644 index 00000000000..f35aaf15400 --- /dev/null +++ b/src/EFCore/Diagnostics/IIdentityResolutionInterceptor.cs @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Diagnostics; + +/// +/// Allows interception of identity resolution conflicts when the starts tracking new entity +/// instances. +/// +/// +/// +/// A can only track one entity instance with any given primary key value. This means multiple instances +/// of an entity with the same key value must be resolved to a single instance. An interceptor of this type can be used to do +/// this. It is called with the existing tracked instance and the new instance and must apply any property values and relationship +/// changes from the new instance into the existing instance. The new instance is then discarded. +/// +/// +/// Use +/// to register application interceptors. +/// +/// +/// Extensions can also register interceptors in the internal service provider. +/// If both injected and application interceptors are found, then the injected interceptors are run in the +/// order that they are resolved from the service provider, and then the application interceptors are run last. +/// +/// +/// See EF Core interceptors +/// and EF Core change tracking for more information and examples. +/// +/// +public interface IIdentityResolutionInterceptor : IInterceptor +{ + /// + /// Called when a attempts to track a new instance of an entity with the same primary key value as + /// an already tracked instance. This method must apply any property values and relationship changes from the new instance + /// into the existing instance. The new instance is then discarded. + /// + /// The is use. + /// The entry for the existing tracked entity instance. + /// The new entity instance, which will be discarded after this call. + void UpdateTrackedInstance(DbContext context, EntityEntry existingEntry, object newInstance); +} diff --git a/src/EFCore/Diagnostics/ISaveChangesInterceptor.cs b/src/EFCore/Diagnostics/ISaveChangesInterceptor.cs index 41228332c54..56e12cd1296 100644 --- a/src/EFCore/Diagnostics/ISaveChangesInterceptor.cs +++ b/src/EFCore/Diagnostics/ISaveChangesInterceptor.cs @@ -8,7 +8,7 @@ namespace Microsoft.EntityFrameworkCore.Diagnostics; /// /// /// -/// Command interceptors can be used to view, change, or suppress execution of the SaveChanges call and +/// SaveChanges interceptors can be used to view, change, or suppress execution of the SaveChanges call and /// modify the result before it is returned to EF. /// /// diff --git a/src/EFCore/Diagnostics/Internal/IdentityResolutionInterceptorAggregator.cs b/src/EFCore/Diagnostics/Internal/IdentityResolutionInterceptorAggregator.cs new file mode 100644 index 00000000000..09241cacb89 --- /dev/null +++ b/src/EFCore/Diagnostics/Internal/IdentityResolutionInterceptorAggregator.cs @@ -0,0 +1,35 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Diagnostics.Internal; + +/// +/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to +/// the same compatibility standards as public APIs. It may be changed or removed without notice in +/// 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. +/// +public class IdentityResolutionInterceptorAggregator : InterceptorAggregator +{ + /// + protected override IIdentityResolutionInterceptor CreateChain(IEnumerable interceptors) + => new CompositeIdentityResolutionInterceptor(interceptors); + + private sealed class CompositeIdentityResolutionInterceptor : IIdentityResolutionInterceptor + { + private readonly IIdentityResolutionInterceptor[] _interceptors; + + public CompositeIdentityResolutionInterceptor(IEnumerable interceptors) + { + _interceptors = interceptors.ToArray(); + } + + public void UpdateTrackedInstance(DbContext context, EntityEntry existingEntry, object newInstance) + { + for (var i = 0; i < _interceptors.Length; i++) + { + _interceptors[i].UpdateTrackedInstance(context, existingEntry, newInstance); + } + } + } +} diff --git a/src/EFCore/Diagnostics/SkippingIdentityResolutionInterceptor.cs b/src/EFCore/Diagnostics/SkippingIdentityResolutionInterceptor.cs new file mode 100644 index 00000000000..97ed1a6a596 --- /dev/null +++ b/src/EFCore/Diagnostics/SkippingIdentityResolutionInterceptor.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Diagnostics; + +/// +/// A that ignores the new instance and retains property values from the existing +/// tracked instance. +/// +public class SkippingIdentityResolutionInterceptor : IIdentityResolutionInterceptor +{ + /// + /// Called when a attempts to track a new instance of an entity with the same primary key value as + /// an already tracked instance. This implementation does nothing, such that property values from the existing tracked + /// instance are retained. + /// + /// The is use. + /// The entry for the existing tracked entity instance. + /// The new entity instance, which will be discarded after this call. + public virtual void UpdateTrackedInstance(DbContext context, EntityEntry existingEntry, object newInstance) + { + } +} diff --git a/src/EFCore/Infrastructure/EntityFrameworkServicesBuilder.cs b/src/EFCore/Infrastructure/EntityFrameworkServicesBuilder.cs index f2670cc30d5..ea9af49a8d5 100644 --- a/src/EFCore/Infrastructure/EntityFrameworkServicesBuilder.cs +++ b/src/EFCore/Infrastructure/EntityFrameworkServicesBuilder.cs @@ -272,6 +272,7 @@ public virtual EntityFrameworkServicesBuilder TryAddCoreServices() TryAdd(typeof(IDiagnosticsLogger<>), typeof(DiagnosticsLogger<>)); TryAdd(); TryAdd(); + TryAdd(); TryAdd(); TryAdd(); TryAdd(p => p.GetRequiredService()); diff --git a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs index 214fd99cb94..3e64b1472c4 100644 --- a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs +++ b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBase.cs @@ -31,6 +31,9 @@ public virtual bool ForceClientNoAction public virtual bool NoStoreCascades => false; + public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder) + => base.AddOptions(builder).AddInterceptors(new CopyingIdentityResolutionInterceptor()); + protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context) { modelBuilder.Entity( @@ -697,23 +700,33 @@ protected virtual IQueryable ModifyQueryRoot(IQueryable query) => query; protected Root LoadRequiredGraph(DbContext context) + => QueryRequiredGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredGraph(DbContext context) => ModifyQueryRoot(context.Set()) .Include(e => e.RequiredChildren).ThenInclude(e => e.Children) .Include(e => e.RequiredSingle).ThenInclude(e => e.Single) - .OrderBy(e => e.Id) - .Single(IsTheRoot); + .OrderBy(e => e.Id); protected Root LoadOptionalGraph(DbContext context) + => QueryOptionalGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryOptionalGraph(DbContext context) => ModifyQueryRoot(context.Set()) .Include(e => e.OptionalChildren).ThenInclude(e => e.Children) .Include(e => e.OptionalChildren).ThenInclude(e => e.CompositeChildren) .Include(e => e.OptionalSingle).ThenInclude(e => e.Single) .Include(e => e.OptionalSingleDerived).ThenInclude(e => e.Single) .Include(e => e.OptionalSingleMoreDerived).ThenInclude(e => e.Single) - .OrderBy(e => e.Id) - .Single(IsTheRoot); + .OrderBy(e => e.Id); protected Root LoadRequiredNonPkGraph(DbContext context) + => QueryRequiredNonPkGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredNonPkGraph(DbContext context) => ModifyQueryRoot(context.Set()) .Include(e => e.RequiredNonPkSingle).ThenInclude(e => e.Single) .Include(e => e.RequiredNonPkSingleDerived).ThenInclude(e => e.Single) @@ -721,19 +734,25 @@ protected Root LoadRequiredNonPkGraph(DbContext context) .Include(e => e.RequiredNonPkSingleMoreDerived).ThenInclude(e => e.Single) .Include(e => e.RequiredNonPkSingleMoreDerived).ThenInclude(e => e.Root) .Include(e => e.RequiredNonPkSingleMoreDerived).ThenInclude(e => e.DerivedRoot) - .OrderBy(e => e.Id) - .Single(IsTheRoot); + .OrderBy(e => e.Id); protected Root LoadRequiredAkGraph(DbContext context) + => QueryRequiredAkGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredAkGraph(DbContext context) => ModifyQueryRoot(context.Set()) .Include(e => e.RequiredChildrenAk).ThenInclude(e => e.Children) .Include(e => e.RequiredChildrenAk).ThenInclude(e => e.CompositeChildren) .Include(e => e.RequiredSingleAk).ThenInclude(e => e.Single) .Include(e => e.RequiredSingleAk).ThenInclude(e => e.SingleComposite) - .OrderBy(e => e.Id) - .Single(IsTheRoot); + .OrderBy(e => e.Id); protected Root LoadOptionalAkGraph(DbContext context) + => QueryOptionalAkGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryOptionalAkGraph(DbContext context) => ModifyQueryRoot(context.Set()) .Include(e => e.OptionalChildrenAk).ThenInclude(e => e.Children) .Include(e => e.OptionalChildrenAk).ThenInclude(e => e.CompositeChildren) @@ -741,10 +760,13 @@ protected Root LoadOptionalAkGraph(DbContext context) .Include(e => e.OptionalSingleAk).ThenInclude(e => e.SingleComposite) .Include(e => e.OptionalSingleAkDerived).ThenInclude(e => e.Single) .Include(e => e.OptionalSingleAkMoreDerived).ThenInclude(e => e.Single) - .OrderBy(e => e.Id) - .Single(IsTheRoot); + .OrderBy(e => e.Id); protected Root LoadRequiredNonPkAkGraph(DbContext context) + => QueryRequiredNonPkAkGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredNonPkAkGraph(DbContext context) => ModifyQueryRoot(context.Set()) .Include(e => e.RequiredNonPkSingleAk).ThenInclude(e => e.Single) .Include(e => e.RequiredNonPkSingleAkDerived).ThenInclude(e => e.Single) @@ -752,23 +774,28 @@ protected Root LoadRequiredNonPkAkGraph(DbContext context) .Include(e => e.RequiredNonPkSingleAkMoreDerived).ThenInclude(e => e.Single) .Include(e => e.RequiredNonPkSingleAkMoreDerived).ThenInclude(e => e.Root) .Include(e => e.RequiredNonPkSingleAkMoreDerived).ThenInclude(e => e.DerivedRoot) - .OrderBy(e => e.Id) - .Single(IsTheRoot); + .OrderBy(e => e.Id); protected Root LoadOptionalOneToManyGraph(DbContext context) + => QueryOptionalOneToManyGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryOptionalOneToManyGraph(DbContext context) => ModifyQueryRoot(context.Set()) .Include(e => e.OptionalChildren).ThenInclude(e => e.Children) .Include(e => e.OptionalChildren).ThenInclude(e => e.CompositeChildren) .Include(e => e.OptionalChildrenAk).ThenInclude(e => e.Children) .Include(e => e.OptionalChildrenAk).ThenInclude(e => e.CompositeChildren) - .OrderBy(e => e.Id) - .Single(IsTheRoot); + .OrderBy(e => e.Id); protected Root LoadRequiredCompositeGraph(DbContext context) + => QueryRequiredCompositeGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredCompositeGraph(DbContext context) => ModifyQueryRoot(context.Set()) .Include(e => e.RequiredCompositeChildren).ThenInclude(e => e.CompositeChildren) - .OrderBy(e => e.Id) - .Single(IsTheRoot); + .OrderBy(e => e.Id); protected static void AssertEntries(IReadOnlyList expectedEntries, IReadOnlyList actualEntries) { diff --git a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs index 3206447a50c..12f324386fa 100644 --- a/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs +++ b/test/EFCore.Specification.Tests/GraphUpdates/GraphUpdatesTestBaseMiscellaneous.cs @@ -73,10 +73,9 @@ public virtual async Task Saving_multiple_modified_entities_with_the_same_key_do } else { - Assert.Equal( - CoreStrings.IdentityConflictSensitive(nameof(College), $"{{Id: {college.Id}}}"), - Assert.Throws( - () => context.Entry(college).State = EntityState.Modified).Message); + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Deleted, context.Entry(college).State); + Assert.Equal(EntityState.Unchanged, context.Entry(city).State); } return Task.CompletedTask; @@ -1570,4 +1569,132 @@ public virtual void Can_add_multiple_dependents_when_multiple_possible_principal + hiddenAreaTask.Choices.Count(e => e.Id == taskChoice.Id)); } }); + + [ConditionalFact] + public void Cab_attach_full_required_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + AssertNavigations(trackedRoot); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_optional_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadOptionalGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryOptionalGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + AssertNavigations(trackedRoot); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_non_PK_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredNonPkGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredNonPkGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + AssertNavigations(trackedRoot); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_AK_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredAkGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredAkGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + AssertNavigations(trackedRoot); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_optional_AK_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadOptionalAkGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryOptionalAkGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + AssertNavigations(trackedRoot); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_non_PK_AK_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredNonPkAkGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredNonPkAkGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + AssertNavigations(trackedRoot); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_one_to_many_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadOptionalOneToManyGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryOptionalOneToManyGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + AssertNavigations(trackedRoot); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_composite_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredCompositeGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredCompositeGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + AssertNavigations(trackedRoot); + + Assert.Equal(0, context.SaveChanges()); + }); } diff --git a/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesFixtureBase.cs b/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesFixtureBase.cs index ef2ac6984e4..e4eba970f92 100644 --- a/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesFixtureBase.cs +++ b/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesFixtureBase.cs @@ -25,6 +25,9 @@ public abstract class ProxyGraphUpdatesFixtureBase : SharedStoreFixtureBase false; + public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder) + => base.AddOptions(builder).AddInterceptors(new CopyingIdentityResolutionInterceptor()); + protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context) { modelBuilder.Entity( @@ -681,6 +684,112 @@ protected Expression> IsTheRoot protected Root LoadRoot(DbContext context) => context.Set().Single(IsTheRoot); + protected Root LoadRequiredGraph(DbContext context) + => QueryRequiredGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredGraph(DbContext context) + => context.Set() + .Include(e => e.RequiredChildren).ThenInclude(e => e.Children) + .Include(e => e.RequiredSingle).ThenInclude(e => e.Single) + .OrderBy(e => e.Id); + + protected Root LoadOptionalGraph(DbContext context) + => QueryOptionalGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryOptionalGraph(DbContext context) + => context.Set() + .Include(e => e.OptionalChildren).ThenInclude(e => e.Children) + .Include(e => e.OptionalChildren).ThenInclude(e => e.CompositeChildren) + .Include(e => e.OptionalSingle).ThenInclude(e => e.Single) + .Include(e => e.OptionalSingleDerived).ThenInclude(e => e.Single) + .Include(e => e.OptionalSingleMoreDerived).ThenInclude(e => e.Single) + .OrderBy(e => e.Id); + + protected Root LoadRequiredNonPkGraph(DbContext context) + => QueryRequiredNonPkGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredNonPkGraph(DbContext context) + => context.Set() + .Include(e => e.RequiredNonPkSingle).ThenInclude(e => e.Single) + .Include(e => e.RequiredNonPkSingleDerived).ThenInclude(e => e.Single) + .Include(e => e.RequiredNonPkSingleDerived).ThenInclude(e => e.Root) + .Include(e => e.RequiredNonPkSingleMoreDerived).ThenInclude(e => e.Single) + .Include(e => e.RequiredNonPkSingleMoreDerived).ThenInclude(e => e.Root) + .Include(e => e.RequiredNonPkSingleMoreDerived).ThenInclude(e => e.DerivedRoot) + .OrderBy(e => e.Id); + + protected Root LoadRequiredAkGraph(DbContext context) + => QueryRequiredAkGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredAkGraph(DbContext context) + => context.Set() + .Include(e => e.RequiredChildrenAk).ThenInclude(e => e.Children) + .Include(e => e.RequiredChildrenAk).ThenInclude(e => e.CompositeChildren) + .Include(e => e.RequiredSingleAk).ThenInclude(e => e.Single) + .Include(e => e.RequiredSingleAk).ThenInclude(e => e.SingleComposite) + .OrderBy(e => e.Id); + + protected Root LoadOptionalAkGraph(DbContext context) + => QueryOptionalAkGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryOptionalAkGraph(DbContext context) + => context.Set() + .Include(e => e.OptionalChildrenAk).ThenInclude(e => e.Children) + .Include(e => e.OptionalChildrenAk).ThenInclude(e => e.CompositeChildren) + .Include(e => e.OptionalSingleAk).ThenInclude(e => e.Single) + .Include(e => e.OptionalSingleAk).ThenInclude(e => e.SingleComposite) + .Include(e => e.OptionalSingleAkDerived).ThenInclude(e => e.Single) + .Include(e => e.OptionalSingleAkMoreDerived).ThenInclude(e => e.Single) + .OrderBy(e => e.Id); + + protected Root LoadRequiredNonPkAkGraph(DbContext context) + => QueryRequiredNonPkAkGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredNonPkAkGraph(DbContext context) + => context.Set() + .Include(e => e.RequiredNonPkSingleAk).ThenInclude(e => e.Single) + .Include(e => e.RequiredNonPkSingleAkDerived).ThenInclude(e => e.Single) + .Include(e => e.RequiredNonPkSingleAkDerived).ThenInclude(e => e.Root) + .Include(e => e.RequiredNonPkSingleAkMoreDerived).ThenInclude(e => e.Single) + .Include(e => e.RequiredNonPkSingleAkMoreDerived).ThenInclude(e => e.Root) + .Include(e => e.RequiredNonPkSingleAkMoreDerived).ThenInclude(e => e.DerivedRoot) + .OrderBy(e => e.Id); + + protected Root LoadOptionalOneToManyGraph(DbContext context) + => QueryOptionalOneToManyGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryOptionalOneToManyGraph(DbContext context) + => context.Set() + .Include(e => e.OptionalChildren).ThenInclude(e => e.Children) + .Include(e => e.OptionalChildren).ThenInclude(e => e.CompositeChildren) + .Include(e => e.OptionalChildrenAk).ThenInclude(e => e.Children) + .Include(e => e.OptionalChildrenAk).ThenInclude(e => e.CompositeChildren) + .OrderBy(e => e.Id); + + protected Root LoadRequiredCompositeGraph(DbContext context) + => QueryRequiredCompositeGraph(context) + .Single(IsTheRoot); + + protected IOrderedQueryable QueryRequiredCompositeGraph(DbContext context) + => context.Set() + .Include(e => e.RequiredCompositeChildren).ThenInclude(e => e.CompositeChildren) + .OrderBy(e => e.Id); + + protected static void AssertEntries(IReadOnlyList expectedEntries, IReadOnlyList actualEntries) + { + var newEntities = new HashSet(actualEntries.Select(ne => ne.Entity)); + var missingEntities = expectedEntries.Select(e => e.Entity).Where(e => !newEntities.Contains(e)).ToList(); + Assert.Equal(Array.Empty(), missingEntities); + Assert.Equal(expectedEntries.Count, actualEntries.Count); + } + public class Root { protected Root() diff --git a/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesTestBaseMiscellaneous.cs b/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesTestBaseMiscellaneous.cs index 043155d8f4b..3617ce25d03 100644 --- a/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesTestBaseMiscellaneous.cs +++ b/test/EFCore.Specification.Tests/GraphUpdates/ProxyGraphUpdatesTestBaseMiscellaneous.cs @@ -338,4 +338,124 @@ public virtual void Sometimes_not_calling_DetectChanges_when_required_does_not_t Assert.Null(dependent.BadCustomer); Assert.Empty(principal.BadOrders); }); + + [ConditionalFact] + public void Cab_attach_full_required_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_optional_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadOptionalGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryOptionalGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_non_PK_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredNonPkGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredNonPkGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_AK_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredAkGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredAkGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_optional_AK_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadOptionalAkGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryOptionalAkGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_non_PK_AK_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredNonPkAkGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredNonPkAkGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_one_to_many_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadOptionalOneToManyGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryOptionalOneToManyGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + + Assert.Equal(0, context.SaveChanges()); + }); + + [ConditionalFact] + public void Cab_attach_full_required_composite_graph_of_duplicates() + => ExecuteWithStrategyInTransaction( + context => + { + var trackedRoot = LoadRequiredCompositeGraph(context); + var entries = context.ChangeTracker.Entries().ToList(); + + context.Attach(QueryRequiredCompositeGraph(context).AsNoTracking().Single(IsTheRoot)); + + AssertEntries(entries, context.ChangeTracker.Entries().ToList()); + + Assert.Equal(0, context.SaveChanges()); + }); } diff --git a/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs b/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs index c91101817e5..3a9836538df 100644 --- a/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs +++ b/test/EFCore.Tests/ChangeTracking/ChangeTrackerTest.cs @@ -1929,7 +1929,7 @@ public void DetectChanges_reparents_even_when_immediate_cascade_enabled(bool del [InlineData(true, true)] public void Dependent_FKs_are_not_nulled_when_principal_is_detached(bool delayCascade, bool trackNewDependents) { - using var context = new EarlyLearningCenter(); + using var context = new EarlyLearningCenter(new CopyingIdentityResolutionInterceptor()); var category = new OptionalCategory { @@ -1973,47 +1973,38 @@ public void Dependent_FKs_are_not_nulled_when_principal_is_detached(bool delayCa { newCategory.Products = new List { - new() { Id = 1 }, - new() { Id = 2 }, - new() { Id = 3 } + new() { Id = 1, CategoryId = category.Id }, + new() { Id = 2, CategoryId = category.Id }, + new() { Id = 3, CategoryId = category.Id } }; } - if (trackNewDependents) - { - Assert.Equal( - CoreStrings.IdentityConflict(nameof(OptionalProduct), "{'Id'}"), - Assert.Throws(() => context.Attach(newCategory)).Message); - } - else - { - context.Update(newCategory); + context.Update(newCategory); - Assert.Equal(4, context.ChangeTracker.Entries().Count()); + Assert.Equal(4, context.ChangeTracker.Entries().Count()); - categoryEntry = context.Entry(newCategory); - product0Entry = context.Entry(newCategory.Products[0]); - product1Entry = context.Entry(newCategory.Products[1]); - product2Entry = context.Entry(newCategory.Products[2]); + categoryEntry = context.Entry(newCategory); + product0Entry = context.Entry(newCategory.Products[0]); + product1Entry = context.Entry(newCategory.Products[1]); + product2Entry = context.Entry(newCategory.Products[2]); - Assert.Equal(EntityState.Modified, categoryEntry.State); + Assert.Equal(EntityState.Modified, categoryEntry.State); - Assert.Equal(EntityState.Unchanged, product0Entry.State); - Assert.Equal(EntityState.Unchanged, product1Entry.State); - Assert.Equal(EntityState.Unchanged, product2Entry.State); + Assert.Equal(EntityState.Unchanged, product0Entry.State); + Assert.Equal(EntityState.Unchanged, product1Entry.State); + Assert.Equal(EntityState.Unchanged, product2Entry.State); - Assert.Same(newCategory.Products[0], category.Products[0]); - Assert.Same(newCategory.Products[1], category.Products[1]); - Assert.Same(newCategory.Products[2], category.Products[2]); + Assert.Same(newCategory.Products[0], category.Products[0]); + Assert.Same(newCategory.Products[1], category.Products[1]); + Assert.Same(newCategory.Products[2], category.Products[2]); - Assert.Same(newCategory, newCategory.Products[0].Category); - Assert.Same(newCategory, newCategory.Products[1].Category); - Assert.Same(newCategory, newCategory.Products[2].Category); + Assert.Same(newCategory, newCategory.Products[0].Category); + Assert.Same(newCategory, newCategory.Products[1].Category); + Assert.Same(newCategory, newCategory.Products[2].Category); - Assert.Equal(newCategory.Id, product0Entry.Property("CategoryId").CurrentValue); - Assert.Equal(newCategory.Id, product1Entry.Property("CategoryId").CurrentValue); - Assert.Equal(newCategory.Id, product2Entry.Property("CategoryId").CurrentValue); - } + Assert.Equal(newCategory.Id, product0Entry.Property("CategoryId").CurrentValue); + Assert.Equal(newCategory.Id, product1Entry.Property("CategoryId").CurrentValue); + Assert.Equal(newCategory.Id, product2Entry.Property("CategoryId").CurrentValue); } [ConditionalTheory] // Issues #16546 #25360; Change reverted in #27174. @@ -3179,10 +3170,12 @@ private class DependentGN private class EarlyLearningCenter : DbContext { + private readonly IInterceptor[] _interceptors; private readonly IServiceProvider _serviceProvider; - public EarlyLearningCenter() + public EarlyLearningCenter(params IInterceptor[] interceptors) { + _interceptors = interceptors; _serviceProvider = InMemoryTestHelpers.Instance.CreateServiceProvider(); } @@ -3287,6 +3280,7 @@ public override bool GeneratesTemporaryValues protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder + .AddInterceptors(_interceptors) .UseInternalServiceProvider(_serviceProvider) .UseInMemoryDatabase(nameof(EarlyLearningCenter)); } diff --git a/test/EFCore.Tests/ChangeTracking/Internal/FixupTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/FixupTest.cs index ff2649a25bd..d02bf64e7de 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/FixupTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/FixupTest.cs @@ -2226,6 +2226,296 @@ public void Navigation_fixup_is_non_destructive_to_existing_graphs() Assert.Equal(5, context.ChangeTracker.Entries().Count()); } + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Attaching_dependent_with_duplicate_principal_resolves(bool copy) + { + using var context = new FixupContext( + copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var originalCategory = new Category(1) { Value = "Original" }; + var originalProduct = new Product(1, 0) { Value = "Original" }; + originalCategory.AddProduct(originalProduct); + context.Attach(originalCategory); + + var newProduct = new Product(2, 0) { Value = "New" }; + var newCategory = new Category(1) { Value = "New" }; + newProduct.SetCategory(newCategory); + + context.Attach(newProduct); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalCategory).State); + Assert.Equal(EntityState.Unchanged, context.Entry(originalProduct).State); + Assert.Equal(EntityState.Unchanged, context.Entry(newProduct).State); + + Assert.Equal(copy ? "New" : "Original", originalCategory.Value); + Assert.Equal("Original", originalProduct.Value); + Assert.Equal("New", newProduct.Value); + + Assert.Equal(1, originalProduct.CategoryId); + Assert.Equal(1, newProduct.CategoryId); + Assert.Same(originalCategory, originalProduct.Category); + Assert.Same(originalCategory, newProduct.Category); + Assert.Equal(2, originalCategory.Products.Count); + Assert.Contains(originalProduct, originalCategory.Products); + Assert.Contains(newProduct, originalCategory.Products); + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Attaching_duplicate_dependent_with_duplicate_principal_resolves(bool copy) + { + using var context = new FixupContext( + copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var originalCategory = new Category(1) { Value = "Original" }; + var originalProduct = new Product(1, 0) { Value = "Original" }; + originalCategory.AddProduct(originalProduct); + context.Attach(originalCategory); + + var newProduct = new Product(1, 1) { Value = "New" }; + var newCategory = new Category(1) { Value = "New" }; + newProduct.SetCategory(newCategory); + + context.Attach(newProduct); + + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalCategory).State); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalProduct).State); + + Assert.Equal(copy ? "New" : "Original", originalCategory.Value); + Assert.Equal(copy ? "New" : "Original", originalProduct.Value); + + Assert.Equal(1, originalProduct.CategoryId); + Assert.Same(originalCategory, originalProduct.Category); + Assert.Equal(1, originalCategory.Products.Count); + Assert.Contains(originalProduct, originalCategory.Products); + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Attaching_principal_with_duplicate_dependent_resolves(bool copy) + { + using var context = new FixupContext( + copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var originalCategory = new Category(1) { Value = "Original" }; + var originalProduct = new Product(1, 0) { Value = "Original" }; + originalCategory.AddProduct(originalProduct); + context.Attach(originalCategory); + + var newProduct = new Product(1, 2) { Value = "New" }; + var newCategory = new Category(2) { Value = "New" }; + newCategory.AddProduct(newProduct); + + context.Attach(newCategory); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Unchanged, context.Entry(originalCategory).State); + Assert.Equal(EntityState.Unchanged, context.Entry(newCategory).State); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalProduct).State); + + Assert.Equal("Original", originalCategory.Value); + Assert.Equal("New", newCategory.Value); + Assert.Equal(copy ? "New" : "Original", originalProduct.Value); + + if (copy) + { + Assert.Equal(2, originalProduct.CategoryId); + Assert.Same(newCategory, originalProduct.Category); + Assert.Equal(0, originalCategory.Products.Count); + Assert.Equal(1, newCategory.Products.Count); + Assert.Contains(originalProduct, newCategory.Products); + } + else + { + Assert.Equal(1, originalProduct.CategoryId); + Assert.Same(originalCategory, originalProduct.Category); + Assert.Equal(1, originalCategory.Products.Count); + Assert.Contains(originalProduct, originalCategory.Products); + Assert.Equal(0, newCategory.Products.Count); + } + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Attaching_duplicate_principal_with_duplicate_dependent_resolves(bool copy) + { + using var context = new FixupContext( + copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var originalCategory = new Category(1) { Value = "Original" }; + var originalProduct = new Product(1, 0) { Value = "Original" }; + originalCategory.AddProduct(originalProduct); + context.Attach(originalCategory); + + var newProduct = new Product(1, 1) { Value = "New" }; + var newCategory = new Category(1) { Value = "New" }; + newCategory.AddProduct(newProduct); + + context.Attach(newCategory); + + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalCategory).State); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalProduct).State); + + Assert.Equal(copy ? "New" : "Original", originalCategory.Value); + Assert.Equal(copy ? "New" : "Original", originalProduct.Value); + + Assert.Equal(1, originalProduct.CategoryId); + Assert.Same(originalCategory, originalProduct.Category); + Assert.Equal(1, originalCategory.Products.Count); + Assert.Contains(originalProduct, originalCategory.Products); + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Attaching_one_to_one_dependent_with_duplicate_principal_resolves(bool copy) + { + using var context = new FixupContext( + copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var originalParent = new Parent(1) { Value = "Original" }; + var originalChild = new Child(1, 0) { Value = "Original" }; + originalParent.SetChild(originalChild); + context.Attach(originalParent); + + var newChild = new Child(2, 0) { Value = "New" }; + var newParent = new Parent(1) { Value = "New" }; + newChild.SetParent(newParent); + + context.Attach(newChild); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalParent).State); + Assert.Equal(EntityState.Deleted, context.Entry(originalChild).State); + Assert.Equal(EntityState.Unchanged, context.Entry(newChild).State); + + Assert.Equal(copy ? "New" : "Original", originalParent.Value); + Assert.Equal("Original", originalChild.Value); + Assert.Equal("New", newChild.Value); + Assert.Equal(1, newChild.ParentId); + Assert.Same(originalParent, newChild.Parent); + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Attaching_one_to_one_duplicate_dependent_with_duplicate_principal_resolves(bool copy) + { + using var context = new FixupContext( + copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var originalParent = new Parent(1) { Value = "Original" }; + var originalChild = new Child(1, 0) { Value = "Original" }; + originalParent.SetChild(originalChild); + context.Attach(originalParent); + + var newChild = new Child(1, 1) { Value = "New" }; + var newParent = new Parent(1) { Value = "New" }; + newChild.SetParent(newParent); + + context.Attach(newChild); + + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalParent).State); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalChild).State); + + Assert.Equal(copy ? "New" : "Original", originalParent.Value); + Assert.Equal(copy ? "New" : "Original", originalChild.Value); + + Assert.Equal(1, originalChild.ParentId); + Assert.Same(originalParent, originalChild.Parent); + Assert.Same(originalChild, originalParent.Child); + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Attaching_one_to_one_principal_with_duplicate_dependent_resolves(bool copy) + { + using var context = new FixupContext( + copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var originalParent = new Parent(1) { Value = "Original" }; + var originalChild = new Child(1, 0) { Value = "Original" }; + originalParent.SetChild(originalChild); + context.Attach(originalParent); + + var newChild = new Child(1, 0) { Value = "New" }; + var newParent = new Parent(2) { Value = "New" }; + newParent.SetChild(newChild); + + context.Attach(newParent); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Unchanged, context.Entry(originalParent).State); + Assert.Equal(EntityState.Unchanged, context.Entry(newParent).State); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalChild).State); + + Assert.Equal("Original", originalParent.Value); + Assert.Equal("New", newParent.Value); + Assert.Equal(copy ? "New" : "Original", originalChild.Value); + + Assert.Equal(2, originalChild.ParentId); + Assert.Same(newParent, originalChild.Parent); + Assert.Same(originalChild, newParent.Child); + Assert.Null(originalParent.Child); + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Attaching_one_to_one_duplicate_principal_with_duplicate_dependent_resolves(bool copy) + { + using var context = new FixupContext( + copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var originalParent = new Parent(1) { Value = "Original" }; + var originalChild = new Child(1, 0) { Value = "Original" }; + originalParent.SetChild(originalChild); + context.Attach(originalParent); + + var newChild = new Child(1, 1) { Value = "New" }; + var newParent = new Parent(1) { Value = "New" }; + newParent.SetChild(newChild); + + context.Attach(newParent); + + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalParent).State); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(originalChild).State); + + Assert.Equal(copy ? "New" : "Original", originalParent.Value); + Assert.Equal(copy ? "New" : "Original", originalChild.Value); + + Assert.Equal(1, originalChild.ParentId); + Assert.Same(originalParent, originalChild.Parent); + Assert.Same(originalChild, originalParent.Child); + } + [ConditionalFact] public void Comparable_entities_that_comply_are_tracked_correctly() { @@ -2571,9 +2861,42 @@ public void Detached_entity_is_not_replaced_by_tracked_entity() Assert.Throws(() => context.Add(a)).Message); } + [ConditionalTheory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task Detached_entity_can_be_replaced_by_tracked_entity(bool async, bool copy) + { + using var context = new BadBeeContext( + nameof(BadBeeContext), copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var b1 = new EntityB { EntityBId = 1, Value = "b1" }; + context.BEntities.Attach(b1); + + var b2 = new EntityB { EntityBId = 1, Value = "b2" }; + + var a = new EntityA { EntityAId = 1, EntityB = b2 }; + + _ = async ? await context.AddAsync(a) : context.Add(a); + + Assert.Equal(2, context.ChangeTracker.Entries().Count()); + Assert.Equal(EntityState.Added, context.Entry(a).State); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(b1).State); + + Assert.Same(a, b1.EntityA); + Assert.Same(b1, a.EntityB); + Assert.Equal(b1.EntityBId, a.EntityBId); + + Assert.Equal(copy ? "b2" : "b1", b1.Value); + } + private class EntityB { public int EntityBId { get; set; } + public string Value { get; set; } public EntityA EntityA { get; set; } } @@ -2586,15 +2909,19 @@ private class EntityA private class BadBeeContext : DbContext { + private readonly IInterceptor[] _interceptors; private readonly string _databaseName; - public BadBeeContext(string databaseName) + public BadBeeContext(string databaseName, params IInterceptor[] interceptors) { + _interceptors = interceptors; _databaseName = databaseName; } protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) - => optionsBuilder.UseInMemoryDatabase(_databaseName); + => optionsBuilder + .AddInterceptors(_interceptors) + .UseInMemoryDatabase(_databaseName); public DbSet AEntities { get; set; } public DbSet BEntities { get; set; } @@ -2648,6 +2975,8 @@ public Parent(int id) public int Id => _id; + public string Value { get; set; } + // ReSharper disable once ConvertToAutoPropertyWithPrivateSetter public Child Child => _child; @@ -2673,6 +3002,8 @@ public Child(int id, int parentId) public int Id => _id; + public string Value { get; set; } + // ReSharper disable once ConvertToAutoPropertyWithPrivateSetter public int ParentId => _parentId; @@ -2784,6 +3115,8 @@ public Category(int id) _id = id; } + public string Value { get; set; } + // ReSharper disable once ConvertToAutoProperty public int Id => _id; @@ -2793,7 +3126,7 @@ public ICollection Products => _products; public void AddProduct(Product product) - => (_products ?? (_products = new List())).Add(product); + => (_products ??= new List()).Add(product); } private class Product @@ -2825,6 +3158,8 @@ public int CategoryId public void SetCategoryId(int categoryId) => _categoryId = categoryId; + public string Value { get; set; } + // ReSharper disable once ConvertToAutoPropertyWithPrivateSetter public Category Category => _category; @@ -2883,10 +3218,17 @@ public void SetProduct(Product product) private sealed class FixupContext : DbContext { + private readonly IInterceptor[] _interceptors; private readonly string _databaseName; - public FixupContext(string databaseName = null) + public FixupContext(params IInterceptor[] interceptors) + : this(null, interceptors) + { + } + + public FixupContext(string databaseName, params IInterceptor[] interceptors) { + _interceptors = interceptors; _databaseName = databaseName; ChangeTracker.AutoDetectChangesEnabled = false; } @@ -2968,6 +3310,7 @@ protected internal override void OnModelCreating(ModelBuilder modelBuilder) protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder + .AddInterceptors(_interceptors) .UseInternalServiceProvider(InMemoryFixture.DefaultServiceProvider) .UseInMemoryDatabase(_databaseName ?? nameof(FixupContext)); } diff --git a/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs index c98fbc02c3b..965b3eb4185 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs @@ -41,6 +41,52 @@ public void Identity_conflict_throws_for_primary_key() new SingleKey { Id = 77, AlternateId = 67 })).Message); } + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Identity_conflict_can_be_resolved(bool copy) + { + using var context = new IdentityConflictContext(copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var entity = new SingleKey { Id = 77, AlternateId = 66, Value = "Existing" }; + context.Attach(entity); + context.Attach(new SingleKey { Id = 77, AlternateId = 66, Value = "New" }); + + Assert.Single(context.ChangeTracker.Entries()); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(entity).State); + Assert.Equal(copy ? "New" : "Existing", entity.Value); + } + + [ConditionalFact] + public void Resolving_identity_conflict_for_primary_key_cannot_change_alternate_key() + { + using var context = new IdentityConflictContext(new NaiveCopyingIdentityResolutionInterceptor()); + + context.Attach(new SingleKey { Id = 77, AlternateId = 66 }); + + Assert.Equal( + CoreStrings.KeyReadOnly(nameof(SingleKey.AlternateId), nameof(SingleKey)), + Assert.Throws( + () => context.Attach( + new SingleKey { Id = 77, AlternateId = 67 })).Message); + } + + [ConditionalFact] + public void Resolving_identity_conflict_for_primary_key_throws_if_alternate_key_changes() + { + using var context = new IdentityConflictContext(new SkippingIdentityResolutionInterceptor()); + + context.Attach(new SingleKey { Id = 77, AlternateId = 66 }); + + Assert.Equal( + CoreStrings.IdentityConflict("SingleKey", "{'Id'}"), + Assert.Throws( + () => context.Attach( + new SingleKey { Id = 77, AlternateId = 67 })).Message); + } + [ConditionalFact] public void Identity_conflict_throws_for_alternate_key() { @@ -55,6 +101,40 @@ public void Identity_conflict_throws_for_alternate_key() new SingleKey { Id = 78, AlternateId = 66 })).Message); } + [ConditionalFact] + public void Resolving_identity_conflict_for_alternate_key_cannot_change_primary_key() + { + using var context = new IdentityConflictContext(new NaiveCopyingIdentityResolutionInterceptor()); + + context.Attach(new SingleKey { Id = 77, AlternateId = 66 }); + Assert.Equal( + CoreStrings.KeyReadOnly(nameof(SingleKey.Id), nameof(SingleKey)), + Assert.Throws( + () => context.Attach( + new SingleKey { Id = 78, AlternateId = 66 })).Message); + } + + private class NaiveCopyingIdentityResolutionInterceptor : IIdentityResolutionInterceptor + { + public void UpdateTrackedInstance(DbContext context, EntityEntry existingEntry, object newInstance) + { + existingEntry.CurrentValues.SetValues(newInstance); + } + } + + [ConditionalFact] + public void Resolving_identity_conflict_for_alternate_key_throws_if_primary_key_changes() + { + using var context = new IdentityConflictContext(new SkippingIdentityResolutionInterceptor()); + + context.Attach(new SingleKey { Id = 77, AlternateId = 66 }); + Assert.Equal( + CoreStrings.IdentityConflict("SingleKey", "{'AlternateId'}"), + Assert.Throws( + () => context.Attach( + new SingleKey { Id = 78, AlternateId = 66 })).Message); + } + [ConditionalFact] public void Identity_conflict_throws_for_owned_primary_key() { @@ -82,6 +162,40 @@ public void Identity_conflict_throws_for_owned_primary_key() })).Message); } + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Identity_conflict_can_be_resolved_for_owned(bool copy) + { + using var context = new IdentityConflictContext(copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var owned = new SingleKeyOwned { Value = "Existing" }; + context.Attach( + new SingleKey + { + Id = 77, + AlternateId = 66, + Owned = owned + }); + + var duplicateOwned = new SingleKeyOwned { Value = "New" }; + context.Entry(duplicateOwned).Property("SingleKeyId").CurrentValue = 77; + + context.Attach( + new SingleKey + { + Id = 78, + AlternateId = 67, + Owned = duplicateOwned + }); + + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(owned).State); + Assert.Equal(copy ? "New" : "Existing", owned.Value); + } + [ConditionalFact] public void Identity_conflict_throws_for_composite_primary_key() { @@ -108,6 +222,39 @@ public void Identity_conflict_throws_for_composite_primary_key() })).Message); } + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public void Identity_conflict_can_be_resolved_for_composite_primary_key(bool copy) + { + using var context = new IdentityConflictContext(copy + ? new CopyingIdentityResolutionInterceptor() + : new SkippingIdentityResolutionInterceptor()); + + var entity = new CompositeKey + { + Id1 = 77, + Id2 = 78, + AlternateId1 = 66, + AlternateId2 = 67, + Value = "Existing" + }; + context.Attach(entity); + context.Attach( + new CompositeKey + { + Id1 = 77, + Id2 = 78, + AlternateId1 = 66, + AlternateId2 = 67, + Value = "New" + }); + + Assert.Single(context.ChangeTracker.Entries()); + Assert.Equal(copy ? EntityState.Modified : EntityState.Unchanged, context.Entry(entity).State); + Assert.Equal(copy ? "New" : "Existing", entity.Value); + } + [ConditionalFact] public void Identity_conflict_throws_for_composite_alternate_key() { @@ -372,8 +519,16 @@ protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBu private class IdentityConflictContext : DbContext { + private readonly IInterceptor[] _interceptors; + + public IdentityConflictContext(params IInterceptor[] interceptors) + { + _interceptors = interceptors; + } + protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder + .AddInterceptors(_interceptors) .UseInMemoryDatabase(nameof(IdentityConflictContext)) .UseInternalServiceProvider(InMemoryFixture.DefaultServiceProvider); @@ -401,10 +556,12 @@ protected internal override void OnModelCreating(ModelBuilder modelBuilder) private class SingleKeyOwned { + public string Value { get; set; } } private class CompositeKeyOwned { + public string Value { get; set; } } private class SingleKey @@ -412,6 +569,8 @@ private class SingleKey public int? Id { get; set; } public int? AlternateId { get; set; } + public string Value { get; set; } + public SingleKeyOwned Owned { get; set; } } @@ -423,6 +582,8 @@ private class CompositeKey public int? AlternateId1 { get; set; } public int? AlternateId2 { get; set; } + public string Value { get; set; } + public CompositeKeyOwned Owned { get; set; } } diff --git a/test/EFCore.Tests/TestUtilities/FakeStateManager.cs b/test/EFCore.Tests/TestUtilities/FakeStateManager.cs index 385cb439506..807580ccb4a 100644 --- a/test/EFCore.Tests/TestUtilities/FakeStateManager.cs +++ b/test/EFCore.Tests/TestUtilities/FakeStateManager.cs @@ -116,9 +116,6 @@ public InternalEntityEntry StartTrackingFromQuery( in ValueBuffer valueBuffer) => throw new NotImplementedException(); - public void BeginTrackingQuery() - => throw new NotImplementedException(); - public InternalEntityEntry TryGetEntry(IKey key, object[] keyValues) => throw new NotImplementedException(); @@ -154,6 +151,19 @@ public void RecordReferencedUntrackedEntity( InternalEntityEntry referencedFromEntry) => throw new NotImplementedException(); + public void UpdateReferencedUntrackedEntity( + object referencedEntity, + object newReferencedEntity, + INavigationBase navigation, + InternalEntityEntry referencedFromEntry) + => throw new NotImplementedException(); + + public bool ResolveToExistingEntry( + InternalEntityEntry newEntry, + INavigationBase navigation, + InternalEntityEntry referencedFromEntry) + => throw new NotImplementedException(); + public IEnumerable> GetRecordedReferrers(object referencedEntity, bool clear) => throw new NotImplementedException();