Automatically create notification collections when change-tracking pr…

…oxies are being used Fixes #21684
dotnet · Sep 7, 2020 · bcf2afb · bcf2afb
1 parent 26c8e1c
commit bcf2afb
Show file tree

Hide file tree

Showing 14 changed files with 394 additions and 319 deletions.
diff --git a/src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs b/src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs
@@ -153,7 +153,11 @@ private static IClrCollectionAccessor CreateGeneric<TEntity, TCollection, TEleme
                 setterDelegateForMaterialization = CreateSetterDelegate(entityParameter, memberInfoForMaterialization, valueParameter);
             }
 
-            var concreteType = new CollectionTypeFactory().TryFindTypeToInstantiate(typeof(TEntity), typeof(TCollection));
+            var concreteType = new CollectionTypeFactory().TryFindTypeToInstantiate(
+                typeof(TEntity),
+                typeof(TCollection),
+                navigation.DeclaringEntityType.Model[CoreAnnotationNames.FullChangeTrackingNotificationsRequiredAnnotation] != null);
+
             if (concreteType != null)
             {
                 var isHashSet = concreteType.IsGenericType && concreteType.GetGenericTypeDefinition() == typeof(HashSet<>);

diff --git a/src/EFCore/Metadata/Internal/CollectionTypeFactory.cs b/src/EFCore/Metadata/Internal/CollectionTypeFactory.cs
@@ -23,7 +23,10 @@ public class CollectionTypeFactory
         ///     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 Type TryFindTypeToInstantiate([NotNull] Type entityType, [NotNull] Type collectionType)
+        public virtual Type TryFindTypeToInstantiate(
+            [NotNull] Type entityType,
+            [NotNull] Type collectionType,
+            bool requireFullNotifications)
         {
             // Code taken from EF6. The rules are:
             // If the collection is defined as a concrete type with a public parameterless constructor, then create an instance of that type
@@ -48,7 +51,8 @@ public virtual Type TryFindTypeToInstantiate([NotNull] Type entityType, [NotNull
                 }
             }
 
-            if (typeof(INotifyPropertyChanged).IsAssignableFrom(entityType))
+            if (requireFullNotifications
+                || typeof(INotifyPropertyChanged).IsAssignableFrom(entityType))
             {
                 var observableHashSetOfT = typeof(ObservableHashSet<>).MakeGenericType(elementType);
                 if (collectionType.IsAssignableFrom(observableHashSetOfT))

diff --git a/test/EFCore.Specification.Tests/ManyToManyLoadTestBase.cs b/test/EFCore.Specification.Tests/ManyToManyLoadTestBase.cs
@@ -2,6 +2,8 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.EntityFrameworkCore.ChangeTracking;
@@ -144,24 +146,24 @@ public virtual void Attached_collections_are_not_marked_as_loaded(EntityState st
                     b =>
                     {
                         b.Id = 7776;
-                        b.TwoSkip.Add(new EntityTwo { Id = 7777 });
-                        b.TwoSkipShared.Add(new EntityTwo { Id = 7778 });
-                        b.SelfSkipPayloadLeft.Add(new EntityOne { Id = 7779 });
-                        b.SelfSkipPayloadRight.Add(new EntityOne { Id = 7780 });
-                        b.BranchSkip.Add(new EntityBranch { Id = 7781 });
-                        b.ThreeSkipPayloadFull.Add(new EntityThree { Id = 7782 });
-                        b.ThreeSkipPayloadFullShared.Add(new EntityThree { Id = 7783 });
+                        b.TwoSkip = new ObservableCollection<EntityTwo> { new EntityTwo { Id = 7777 } };
+                        b.TwoSkipShared = new ObservableCollection<EntityTwo> { new EntityTwo { Id = 7778 } };
+                        b.SelfSkipPayloadLeft = new ObservableCollection<EntityOne> { new EntityOne { Id = 7779 } };
+                        b.SelfSkipPayloadRight = new ObservableCollection<EntityOne> { new EntityOne { Id = 7780 } };
+                        b.BranchSkip = new ObservableCollection<EntityBranch> { new EntityBranch { Id = 7781 } };
+                        b.ThreeSkipPayloadFull = new ObservableCollection<EntityThree> { new EntityThree { Id = 7782 } };
+                        b.ThreeSkipPayloadFullShared = new ObservableCollection<EntityThree> { new EntityThree { Id = 7783 } };
                     })
                 : new EntityOne
                 {
                     Id = 7776,
-                    TwoSkip = { new EntityTwo { Id = 7777 } },
-                    TwoSkipShared = { new EntityTwo { Id = 7778 } },
-                    SelfSkipPayloadLeft = { new EntityOne { Id = 7779 } },
-                    SelfSkipPayloadRight = { new EntityOne { Id = 7780 } },
-                    BranchSkip = { new EntityBranch { Id = 7781 } },
-                    ThreeSkipPayloadFull = { new EntityThree { Id = 7782 } },
-                    ThreeSkipPayloadFullShared = { new EntityThree { Id = 7783 } },
+                    TwoSkip = new List<EntityTwo> { new EntityTwo { Id = 7777 } },
+                    TwoSkipShared = new List<EntityTwo> { new EntityTwo { Id = 7778 } },
+                    SelfSkipPayloadLeft = new List<EntityOne> { new EntityOne { Id = 7779 } },
+                    SelfSkipPayloadRight = new List<EntityOne> { new EntityOne { Id = 7780 } },
+                    BranchSkip = new List<EntityBranch> { new EntityBranch { Id = 7781 } },
+                    ThreeSkipPayloadFull = new List<EntityThree> { new EntityThree { Id = 7782 } },
+                    ThreeSkipPayloadFullShared = new List<EntityThree> { new EntityThree { Id = 7783 } }
                 };
 
             context.Attach(left);