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();