Efcore: Implement change tracking proxies

[ajcvickers]

Using the lazy-loading proxy infrastructure coupled with INotifyPropertyChanging and INotifyPropertyChanged.

Also, consider partial notifications, for example to detect setting a navigation property or FK to null--see this discussion: https://github.com/aspnet/EntityFramework6/issues/452

[orionstudt]

I'd like to give this a go if that's not a problem. Is this on any existing roadmaps? I can write up a plan here and I'll probably have a couple simple questions before I start anything.

[ajcvickers]

@orionstudt This isn't currently planned for an upcoming release; a good PR is certainly something we would be interested in. Writing a plan here would be a good place to start.

[orionstudt]

@ajcvickers Alright, cool. Here is my initial thoughts/plan:

• Add an additional boolean switch to proxies extension, ProxiesOptionsExtension, UseChangeDetectionProxies.

• An additional third switch, not sure what to call it. Something like CheckEquality - default to true. The default behavior being if the incoming value is the same as the current value than no change notification will be raised. Setting to false should cover the setting a navigation or FK to null scenario.

• Modify logic in ProxyBindingRewriter to indicate different convention. If UseChangeDetectionProxies is enabled it will require all properties to be virtual, not just navigations. Also if only UseChangeDetectionProxies is enabled we won't need to setup lazy loader service property.

• Write 2 additional castle interceptors, 1 for INotifyPropertyChanged & 1 for INotifyPropertyChanging.

• ProxyFactory will now rely on ProxiesOptionsExtension in order to determine how to build the proxies (which interfaces to proxy, and which interceptors to use).

• Will use IEntityType.GetChangeTrackingStrategy() to determine which INPCs to proxy on each entity, if any.

Questions:

1. If the entity already implements one of the notify interfaces, should we just not proxy that interface and leave it up to the consumer to raise those events? Edit: alternatively we could just continue to proxy and potentially raise 2 events if the consumer is already raising 1.

2. In a similar vein, for collection navigations we can bubble a collection change up to the parent entity if the underlying collection is one of the observable collection types. But it will be up to the consumer to instantiate that collection as one of the observable types in their entity's constructor, is that ok?

3. With what I've currently written out, if a consumer enables UseChangeDetectionProxies the default ChangeDetectionStrategy will still be Snapshot so none of the entities will receive the INPC proxies. Should we also be updating the ChangeDetectionStrategy to some other default value when change detection proxies are first enabled?

[ajcvickers]

@orionstudt Thanks--this seems like a well thought out approach, and generally in the correct direction. I will discuss your questions with the team and get back to you.

[orionstudt]

I will start this probably over the holiday or shortly thereafter and get something together for you guys to review. Happy holidays!

[ajcvickers]

@orionstudt Sounds good.

[orionstudt]

@ajcvickers Made an initial draft PR with the current state of the work and copied the questions over there as I figured that might be a better place for any discussion.

[JonPSmith]

Hi @orionstudt and @ajcvickers ,

I'm updating my book Entity Framework Core in Action to EF Core 5 and I will soon updating the part about INotifyPropertyChanged. Can you point me to an classes/unit tests that use this feature so I can understand it.

No rush.

[orionstudt]

Hi @JonPSmith. All the code for this feature is in the src/EFCore.Proxies project and the tests are in test/EFCore.Proxies.Tests.

Specifically most of the code is in:
ProxyFactory.cs
PropertyChangedInterceptor.cs
PropertyChangingInterceptor.cs

[ajcvickers]

@JonPSmith See also ProxyGraphUpdatesTestBaseOneToOne and related, and also ManyToManyTrackingProxySqlServerTest

[JonPSmith]

Hi @orionstudt and @ajcvickers ,

That should keep me busy. I will work though this and maybe get you to check that I have understand it correctly.

[JonPSmith]

Hi @orionstudt and @ajcvickers ,

I am writing about change tracking proxies. What I have worked out so far are:

• It works like Lazy Loading version that uses the UseLazyLoadingProxies() builder command.
• You add builder.UseChangeTrackingProxies() to the creation the DbContextOptionsBuilder
• Every property has to be to be virtual in all entities that are linked to the DbContext

Things I don't understand are

• How do I set up the ChangingAndChangedNotifications? Is it linked to the Proxy Create??

Questions I have

• I don't think you can have a mix of normal entity classes (i.e. not using the ChangeTrackingProxies and not having virtual properties) and entity classes that use ChangeTrackingProxies in the same DbContext. Is that correct?
• Can you have UseLazyLoadingProxies() and .UseChangeTrackingProxies() at the same time?

[orionstudt]

How do I set up the ChangingAndChangedNotifications? Is it linked to the Proxy Create??

I'm not sure I'm understanding this question correctly but, the ChangeTrackingStrategy will default to ChangingAndChangedNotifications when you call .UseChangeTrackingProxies(). This is done in Microsoft.EntityFrameworkCore.Proxies.Internal.ProxyChangeTrackingConvention.

I don't think you can have a mix of normal entity classes (i.e. not using the ChangeTrackingProxies and not having virtual properties) and entity classes that use ChangeTrackingProxies in the same DbContext. Is that correct?

That is correct. This was a design decision from the EFCore team. @ajcvickers discussed this here & here.

Can you have UseLazyLoadingProxies() and .UseChangeTrackingProxies() at the same time?

Yes this should absolutely be ok. You can see in Microsoft.EntityFrameworkCore.Proxies.Internal.ProxyFactory that on proxy creation it will pass in interceptors for one, the other, or both.

Hope that helps.

[JonPSmith]

Hi @orionstudt,

Thanks for coming back to me. I have worked it out now:

• If I am using ChangingAndChangedNotifications I need to create the entity class via the CreateProxy method.
• If I am using ChangingNotifications I can create the entity class with a normal new.

That what works in my unit test but is that what you recommend? The use of the CreateProxy method isn't obvious and I suspect you need some documentation for that.

It also looks like you can't have a ChangingAndChangedNotificationsWithOriginalValues version. Not sure you need one but just checking that I haven't missed it.

Thanks for the other answers. They all make sense.

[ajcvickers]

@JonPSmith To add to what @orionstudt said, this is the case where I wouldn't expect to set different values for ChangeTrackingStrategy. The proxies will always implement both changing and changed notifications, I don't think there would be any value in telling EF to not use them. That being said, ChangingAndChangedNotificationsWithOriginalValues would make sense to set.

I filed #21920