Skip to content

Automatically create notification collections when change-tracking proxies are being used #22438

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/EFCore/Metadata/Internal/ClrCollectionAccessorFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<>);
Expand Down
8 changes: 6 additions & 2 deletions src/EFCore/Metadata/Internal/CollectionTypeFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down
30 changes: 16 additions & 14 deletions test/EFCore.Specification.Tests/ManyToManyLoadTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Loading