Code review updates

dotnet · Aug 11, 2020 · 4178007 · 4178007
1 parent a8ee6ff
commit 4178007
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 24 deletions.
diff --git a/src/EFCore/ChangeTracking/CollectionEntry.cs b/src/EFCore/ChangeTracking/CollectionEntry.cs
@@ -103,36 +103,40 @@ public override bool IsModified
 
                     var joinEntityType = skipNavigation.JoinEntityType;
                     var foreignKey = skipNavigation.ForeignKey;
+                    var inverseForeignKey = skipNavigation.Inverse.ForeignKey;
                     foreach (var joinEntry in stateManager.Entries)
                     {
                         if (joinEntry.EntityType == joinEntityType
-                            && joinEntry.EntityState != EntityState.Unchanged
-                            && joinEntry.EntityState != EntityState.Detached
-                            && stateManager.FindPrincipal(joinEntry, foreignKey) == InternalEntry)
+                            && stateManager.FindPrincipal(joinEntry, foreignKey) == InternalEntry
+                            && (joinEntry.EntityState == EntityState.Added
+                                || joinEntry.EntityState == EntityState.Deleted
+                                || foreignKey.Properties.Any(joinEntry.IsModified)
+                                || inverseForeignKey.Properties.Any(joinEntry.IsModified)
+                                || (stateManager.FindPrincipal(joinEntry, inverseForeignKey)?.EntityState == EntityState.Deleted)))
                         {
                             return true;
                         }
                     }
                 }
-
-                var navigationValue = CurrentValue;
-                if (navigationValue != null)
+                else
                 {
-                    var targetEntityType = Metadata.TargetEntityType;
-                    var foreignKey = Metadata is INavigation navigation
-                        ? navigation.ForeignKey
-                        : ((ISkipNavigation)Metadata).ForeignKey;
-
-                    foreach (var relatedEntity in navigationValue)
+                    var navigationValue = CurrentValue;
+                    if (navigationValue != null)
                     {
-                        var relatedEntry = stateManager.TryGetEntry(relatedEntity, targetEntityType);
+                        var targetEntityType = Metadata.TargetEntityType;
+                        var foreignKey = ((INavigation)Metadata).ForeignKey;
 
-                        if (relatedEntry != null
-                            && (relatedEntry.EntityState == EntityState.Added
-                                || relatedEntry.EntityState == EntityState.Deleted
-                                || (foreignKey.Properties.Any(relatedEntry.IsModified))))
+                        foreach (var relatedEntity in navigationValue)
                         {
-                            return true;
+                            var relatedEntry = stateManager.TryGetEntry(relatedEntity, targetEntityType);
+
+                            if (relatedEntry != null
+                                && (relatedEntry.EntityState == EntityState.Added
+                                    || relatedEntry.EntityState == EntityState.Deleted
+                                    || foreignKey.Properties.Any(relatedEntry.IsModified)))
+                            {
+                                return true;
+                            }
                         }
                     }
                 }
@@ -147,10 +151,9 @@ public override bool IsModified
                 {
                     var joinEntityType = skipNavigation.JoinEntityType;
                     var foreignKey = skipNavigation.ForeignKey;
-                    foreach (var joinEntry in stateManager.Entries.Where(
+                    foreach (var joinEntry in stateManager
+                        .GetEntriesForState(added: !value, modified: !value, deleted: !value, unchanged: value).Where(
                             e => e.EntityType == joinEntityType
-                                && e.EntityState != EntityState.Detached
-                                && (value == (e.EntityState == EntityState.Unchanged))
                                 && stateManager.FindPrincipal(e, foreignKey) == InternalEntry)
                         .ToList())
                     {

diff --git a/test/EFCore.Tests/ChangeTracking/SkipCollectionEntryTest.cs b/test/EFCore.Tests/ChangeTracking/SkipCollectionEntryTest.cs
@@ -266,12 +266,18 @@ public void Can_get_and_set_current_value_start_tracking_generic(bool useExplici
         }
 
         [ConditionalTheory]
-        [InlineData(false)]
-        [InlineData(true)]
-        public void IsModified_tracks_detects_deletion_of_related_entity(bool useExplicitPk)
+        [InlineData(false, CascadeTiming.Immediate)]
+        [InlineData(true, CascadeTiming.Immediate)]
+        [InlineData(false, CascadeTiming.OnSaveChanges)]
+        [InlineData(true, CascadeTiming.OnSaveChanges)]
+        [InlineData(false, CascadeTiming.Never)]
+        [InlineData(true, CascadeTiming.Never)]
+        public void IsModified_tracks_detects_deletion_of_related_entity(bool useExplicitPk, CascadeTiming cascadeTiming)
         {
             using var context = useExplicitPk ? new ExplicitFreezerContext() : new FreezerContext();
 
+            context.ChangeTracker.CascadeDeleteTiming = cascadeTiming;
+
             var cherry1 = new Cherry { Id = 1 };
             var cherry2 = new Cherry { Id = 2 };
             var chunky1 = new Chunky { Id = 1 };