Re-subscribe to notifying collection when the collection instance is …

…changed (#28567)

src/EFCore/ChangeTracking/Internal/IInternalEntityEntrySubscriber.cs

@@ -31,4 +31,20 @@ public interface IInternalEntityEntrySubscriber
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     void Unsubscribe(InternalEntityEntry entry);
+
+    /// <summary>
+    ///     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.
+    /// </summary>
+    void SubscribeCollectionChanged(InternalEntityEntry entry, INavigationBase navigation);
+
+    /// <summary>
+    ///     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.
+    /// </summary>
+    void UnsubscribeCollectionChanged(InternalEntityEntry entry, INavigationBase navigation);
 }

src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs

@@ -1810,6 +1810,13 @@ public void HandleINotifyPropertyChanging(
         foreach (var propertyBase in GetNotificationProperties(EntityType, eventArgs.PropertyName))
         {
             StateManager.InternalEntityEntryNotifier.PropertyChanging(this, propertyBase);
+
+            if (propertyBase is INavigationBase navigation
+                && navigation.IsCollection
+                && GetCurrentValue(propertyBase) != null)
+            {
+                StateManager.Dependencies.InternalEntityEntrySubscriber.UnsubscribeCollectionChanged(this, navigation);
+            }
         }
     }
 
@@ -1826,6 +1833,13 @@ public void HandleINotifyPropertyChanged(
         foreach (var propertyBase in GetNotificationProperties(EntityType, eventArgs.PropertyName))
         {
             StateManager.InternalEntityEntryNotifier.PropertyChanged(this, propertyBase, setModified: true);
+
+            if (propertyBase is INavigationBase navigation
+                && navigation.IsCollection
+                && GetCurrentValue(propertyBase) != null)
+            {
+                StateManager.Dependencies.InternalEntityEntrySubscriber.SubscribeCollectionChanged(this, navigation);
+            }
         }
     }
 

src/EFCore/ChangeTracking/Internal/InternalEntityEntrySubscriber.cs

@@ -44,8 +44,7 @@ public virtual bool SnapshotAndSubscribe(InternalEntityEntry entry)
                      .Concat<INavigationBase>(entityType.GetSkipNavigations())
                      .Where(n => n.IsCollection))
         {
-            AsINotifyCollectionChanged(entry, navigation, entityType, changeTrackingStrategy).CollectionChanged
-                += entry.HandleINotifyCollectionChanged;
+            SubscribeCollectionChanged(entry, navigation);
         }
 
         if (changeTrackingStrategy != ChangeTrackingStrategy.ChangedNotifications)
@@ -60,6 +59,16 @@ public virtual bool SnapshotAndSubscribe(InternalEntityEntry entry)
         return true;
     }
 
+    /// <summary>
+    ///     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.
+    /// </summary>
+    public virtual void SubscribeCollectionChanged(InternalEntityEntry entry, INavigationBase navigation)
+        => AsINotifyCollectionChanged(entry, navigation, entry.EntityType, entry.EntityType.GetChangeTrackingStrategy()).CollectionChanged
+            += entry.HandleINotifyCollectionChanged;
+
     /// <summary>
     ///     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
@@ -77,8 +86,7 @@ public virtual void Unsubscribe(InternalEntityEntry entry)
                          .Concat<INavigationBase>(entityType.GetSkipNavigations())
                          .Where(n => n.IsCollection))
             {
-                AsINotifyCollectionChanged(entry, navigation, entityType, changeTrackingStrategy).CollectionChanged
-                    -= entry.HandleINotifyCollectionChanged;
+                UnsubscribeCollectionChanged(entry, navigation);
             }
 
             if (changeTrackingStrategy != ChangeTrackingStrategy.ChangedNotifications)
@@ -92,6 +100,18 @@ public virtual void Unsubscribe(InternalEntityEntry entry)
         }
     }
 
+    /// <summary>
+    ///     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.
+    /// </summary>
+    public virtual void UnsubscribeCollectionChanged(
+        InternalEntityEntry entry,
+        INavigationBase navigation)
+        => AsINotifyCollectionChanged(entry, navigation, entry.EntityType, entry.EntityType.GetChangeTrackingStrategy()).CollectionChanged
+            -= entry.HandleINotifyCollectionChanged;
+
     private static INotifyCollectionChanged AsINotifyCollectionChanged(
         InternalEntityEntry entry,
         INavigationBase navigation,

test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs

@@ -5768,9 +5768,11 @@ public virtual void Can_add_and_remove_a_new_relationship_with_payload(
     }
 
     [ConditionalTheory]
-    [InlineData(false)]
-    [InlineData(true)]
-    public virtual async Task Can_replace_dependent_with_many_to_many(bool async)
+    [InlineData(false, false)]
+    [InlineData(false, true)]
+    [InlineData(true, false)]
+    [InlineData(true, true)]
+    public virtual async Task Can_replace_dependent_with_many_to_many(bool createNewCollection, bool async)
     {
         var principalKey = 0;
         var newRightKey = 0;
@@ -5784,7 +5786,11 @@ await ExecuteWithStrategyInTransactionAsync(
 
                 principal.Reference = leftEntity;
 
-                leftEntity.ThreeSkipFull ??= CreateCollection<EntityThree>();
+                if (leftEntity.ThreeSkipFull == null || createNewCollection)
+                {
+                    leftEntity.ThreeSkipFull = CreateCollection<EntityThree>();
+                }
+
                 leftEntity.ThreeSkipFull.Add(rightEntity);
 
                 _ = async
@@ -5816,7 +5822,11 @@ await ExecuteWithStrategyInTransactionAsync(
 
                 principal.Reference = newLeftEntity;
 
-                newLeftEntity.ThreeSkipFull ??= CreateCollection<EntityThree>();
+                if (newLeftEntity.ThreeSkipFull == null || createNewCollection)
+                {
+                    newLeftEntity.ThreeSkipFull = CreateCollection<EntityThree>();
+                }
+
                 newLeftEntity.ThreeSkipFull.Add(newRightEntity);
 
                 if (RequiresDetectChanges)

test/EFCore.Tests/ChangeTracking/Internal/InternalEntryEntrySubscriberTest.cs

@@ -291,7 +291,7 @@ public void Entry_subscribes_to_INotifyPropertyChanging_and_INotifyPropertyChang
         Assert.Empty(testListener.Changing);
         Assert.Empty(testListener.Changed);
 
-        entity.RelatedCollection = new List<ChangedOnlyNotificationEntity>();
+        entity.RelatedCollection = new ObservableHashSet<ChangedOnlyNotificationEntity>();
 
         var property = entry.EntityType.FindNavigation("RelatedCollection");
         Assert.Same(property, testListener.Changing.Single().Item2);
@@ -421,6 +421,56 @@ public void Entry_unsubscribes_to_INotifyCollectionChanged()
         Assert.Same(entries[2], testListener.CollectionChanged.Skip(2).Single().Item1);
     }
 
+    [ConditionalFact] // Issue #26023
+    public void Entry_re_subscribes_to_INotifyCollectionChanged_when_collection_instance_changes()
+    {
+        var contextServices = InMemoryTestHelpers.Instance.CreateContextServices(
+            new ServiceCollection().AddScoped<INavigationFixer, TestNavigationListener>(),
+            BuildModel());
+
+        var testListener = contextServices
+            .GetRequiredService<IEnumerable<INavigationFixer>>()
+            .OfType<TestNavigationListener>()
+            .Single();
+
+        var entities = new List<FullNotificationEntity>();
+        var entries = new List<InternalEntityEntry>();
+        for (var i = 0; i < 10; i++)
+        {
+            entities.Add(
+                new FullNotificationEntity { Id = i + 1, RelatedCollection = new ObservableHashSet<ChangedOnlyNotificationEntity>() });
+            entries.Add(contextServices.GetRequiredService<IStateManager>().GetOrCreateEntry(entities[i]));
+            entries[i].SetEntityState(EntityState.Unchanged);
+        }
+
+        var navigation = entries[0].EntityType.FindNavigation("RelatedCollection");
+
+        Assert.Empty(testListener.CollectionChanged);
+
+        entities[2].RelatedCollection.Add(new ChangedOnlyNotificationEntity());
+        entities[5].RelatedCollection.Add(new ChangedOnlyNotificationEntity());
+
+        Assert.Equal(2, testListener.CollectionChanged.Count);
+        Assert.All(testListener.CollectionChanged, e => Assert.Same(e.Item2, navigation));
+        Assert.Same(entries[2], testListener.CollectionChanged.First().Item1);
+        Assert.Same(entries[5], testListener.CollectionChanged.Skip(1).Single().Item1);
+
+        foreach (var entity in entities)
+        {
+            entity.RelatedCollection = new ObservableHashSet<ChangedOnlyNotificationEntity>();
+        }
+
+        entities[2].RelatedCollection.Add(new ChangedOnlyNotificationEntity());
+        entities[5].RelatedCollection.Add(new ChangedOnlyNotificationEntity());
+
+        Assert.Equal(4, testListener.CollectionChanged.Count);
+        Assert.All(testListener.CollectionChanged, e => Assert.Same(e.Item2, navigation));
+        Assert.Same(entries[2], testListener.CollectionChanged.First().Item1);
+        Assert.Same(entries[5], testListener.CollectionChanged.Skip(1).First().Item1);
+        Assert.Same(entries[2], testListener.CollectionChanged.Skip(2).First().Item1);
+        Assert.Same(entries[5], testListener.CollectionChanged.Skip(3).Single().Item1);
+    }
+
     [ConditionalTheory]
     [InlineData(true)]
     [InlineData(false)]