dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.49k stars 3.13k forks source link

Events and interception (aka lifecycle hooks) #626

Open divega opened 9 years ago

divega commented 9 years ago

Done in 2.1

Done in 3.1

Done in 5.0

Done in 6.0

Done in 7.0

Backlog

Note: below is a copy of a very old EF specification and reflects thinking from several years ago. A lot of things aren't valid anymore.

We define EF Core lifecycle hooks as the general feature that enables an application or library to sign up to be invoked or notified whenever certain interesting conditions or actions occur as part of the lifecycle of entities, properties, associations, queries, context instances, and other elements in the Entity Framework stack.

For example:

  1. An application can provide a method that will be invoked automatically whenever an object is about to be saved, or it can subscribe to an event that fires when an object is created and its properties initialized, etc.
  2. A framework extension can register an interceptor that gives it an opportunity to rewrite query expression trees before they get translated by EF. This could be used to validate whether a user has access to specific information or to filter query results based on per DbContext filter (see #6440).
  3. Execute SQL after a DbConnection is opened (to use features such as SQL Server App Role)

The need for lifecycle hooks

We want to enable customers to write business logic that triggers in the different stages of the lifecycle of these objects, following well factored coding patterns. We also want framework writers to be able to use these hooks to extend EF Core in useful ways.

In previous versions of Entity Framework we already exposed a few lifecycle hooks. For instance, we had the AssociationChanged and ObjectStateManagerChanged events since the first version, and the ObjectMaterialized event was added in EF4. Up until EF6.x many of the existing hooks are not exposed in the DbContext API. In EF6 we also added several low level extensibility points in Interception that can be used too as lifecycle hooks.

There is a continuum of capabilities related and overlapping with lifecycle hooks, e.g.:

Name Pri Location Cancel or override Description & sample scenario
QueryExecuting 0 DbContext Yes Query interception, custom query caching.
QueryExecuted 3 DbContext No When Execute happened, before the reader is read. Tracing?
QueryCompleted 3 DbContext No After DbDataReader is closed. Tracing?
EntityStateChanged 0 DbContext No Signals all state changes
EntityStateChanging 3 DbContext ? Undo changes or change proposed values before they are set?
ConnectionProvisioning 2 DbContext Yes Execute additional code to make sure the connection is alive, or do logging
ConnectionReleasing 2 DbContext Yes Cleanup something done during Ensure / StartUsingConnection
ConnectionOpened 2   No More likely for tracing. Since SqlClient has fixed invalid connection pools, then this is lower priority
ConnectionOpening 1 DbContext Yes Slightly simpler to use than Ensure/Start, would not require user to check current state. Could also be used for tracing.
ConnectionClosed 1   No  
ConnectionClosing 1 DbContext Yes Slightly simpler to use than Release/Stop, would not require user to check the initial state. Could also be used for tracing.
OnModelCreating 0 DbContext Yes Tweak model before it is cached.
OnModelCreated 1 DbContext Yes Signal that the model is done and execute some custom code, possibly related to caching logic. . Issue: do we need this for ObjectContext? Issue: if the user is going to implement his own caching, we should have an abstract class or interface for that.
ModelCacheLookup 2 DbContext Yes Implement your own caching logic. Tracing?
ModelCacheHit 2 DbContext Yes Execute additional code when the model is found in the cache. Tracing?
EntityLoading 1 DbContext, DbEntityEntry No After object instance is created but before its properties are initialized. Can be used to reset a flag that will be set in newly created instances but shouldn’t be set during initialization, i.e. for validation.
EntityLoaded 0 DbContext, DbEntityEntry No Can be used to setup anything after an object has been materialized, i.e. event handlers, flags, etc.
CollectionLoading 1 DbContext, DbEntityEntry DbCollectiohnEntry No Can be used to setup anything on a collection after it is created but before it is populated. Issue: Could be used to provide your own collection?
CollectionLoading 1 Context, Entity or Collection No Can be used to setup anything on a collection after it has been created and populated, i.e. listeners for its changed event.
ObjectTypeResolving 1 Context Yes Could be used to specify a different type than the original one, i.e. to implement your own proxy mechanism. It should be per type but could return a Func<T> that returns a new instance and the result could be compiled into materialization delegates.
CollectionTypeResolving 1 Context Yes Something similar to ObjectTypeResolving but for collections. Could be used to replace the default collection type with a custom proxy collection with additional functionality (i.e. paging, fine grained lazy load).
Virtual OnSavingChanges Medium DbContext No Can be used to re-implement SaveChanges but still invoke the existing SavingChanges event
SavedChanges Low Context No Could be used to execute cleanup code after SaveChanges. For instance, to call AcceptChanges on each STE change tracker. It is lower priority because virtual SaveChanges covers most scenarios.
EntityStateChanging Low Context, Entity Yes For an entity instance or type in particular we could avoid putting in the modified state. So even if the properties are read-write, the context ignores changes to this entity. Could be also used to suspend fixup on an entity that is being detached.
EntityStateChanged High Context, Entity No Executes logic after an entity has been put in a certain state. Can be used to setup property values, restore state after the changing event.
PropertyChanging Low Context, Entity Yes Any time a property is about to be changed by the framework or any party, if notification or interception is enabled by the entity type. Should make original and new value available. Should also work for navigation, scalar and complex types properties. Tracing?
PropertyChanged High Context, Entity No Any time a change in a property value change is detected.
PropertyLoading High Context, Entity, Collection Yes Intercepts, overrides de loading of a property. Could be used to support loading of properties using stored procedures.
PropertyLoaded Medium Context, Entity, Collection No Tracing?
Writetable IsLoaded High Context, Entity Yes Allows cancelling the loading of a property.
CollectionChanging Medium Context Yes Any time a collection is about to be changed by the framework or any party, if interception is enabled
CollectionChanged Medium Context, Entity No Any time a change to a collection has been detected.
AssociationChanging Low Context, RelatedEnd Yes Can be used to prevent an association from being changed, or to execute business logic when the association is about to change.
AssociationChanged Medium Context, RelatedEnd No Can be used to execute additional logic after an association is changed, i.e. user can explicitly cascade relationships removals into dependent removals, workaround current databinding shortcomings.
RowValidate , RowValidateAdded, RowValidateModified, RowValidateDeleted Medium Context Yes Storage level version of ObjectValidate. Tracing?
SavingChanges event 0 DbContext ? Currently only available on ObjectContext. Should make trigger OnSavingChanges method protected.

Existing hooks

Name Description & sample scenario
virtual Dispose This can be used to do additional cleanup, i.e. on entity instances.
virtual SaveChanges Can be used to execute additional logic before, after or instead of saving changes.

Some open issues:

  1. Need to prototype some coding patterns and try them.
  2. Is logging and tracing part of this API? It seems that ideally we should have the same level of flexibility for hooking mechanisms with logging and tracing as we end up having with this API.
  3. Second level cache should probably expose its hooks through the same mechanisms.
  4. Should we provide low level query interception points with the same mechanisms, i.e. as command tress and store commands? Should we do the same for CUD store commands? Would need to make sure those work well with caching.
  5. Can we get some level of support for async execution of queries with this hook?
  6. Should we provide enough lifecycle hooks to implement custom fixup logic?
  7. Areas of overlap with other extensibilities: read and write properties in object mapping, proxy type creation (can be imperative vs. event driven), equality and snapshot comparisons for change tracking extensibility.
  8. What about customizing identity resolution?
  9. Is Logging and Tracing part of the lifecycle hooks
  10. Is Query interception part of the lifecycle hooks
  11. Even without query interception we should expose when we are about to execute (imagine a profiling tool that measures how much query compilation costs).
  12. Is ContinueOnConflict part of the lifecycle hooks
  13. How do we improve diagnostics? Can we have OnError
  14. Need to do prioritization, costing and scoping
  15. Should we have a fine grained version of CollectionAdding / CollectionRemoving with support for magic methods on the entities to enable collection patterns? We would need a pattern for Contains checks also.
  16. There is a conversation about splitting AssociationChanged this event into properties and collection changes. However, there should be a way to tell the difference between a scalar property change and a nav prop. Should we make AssociationChanged more accessible and add AssociationChanging? This would provide a way to intercept changes in associations independently of cardinality, constraints, etc.
  17. AssociationChanging would need the entity and collection types to collaborate to avoid changes from being made to the graph.
ajcvickers commented 6 years ago

@sidshetye Thanks for the feedback--I will certainly pass it on to those who make the calls on the release dates.

ajcvickers commented 6 years ago

For anyone following this issue. in order to help us prioritize work can you let us know which hooks are important to you and what you intend to do with the hooks?

dazinator commented 6 years ago

@ajcvickers During a save operation of a business object, my parent / root business object typically calls SaveChanges() but only after passing the DbContext instance to all its child business objects in the graph, so that they can add / update / delete entities on the DbContext in order to take part in the transaction. Once the entire business object graph has worked with the DbContext instance, the root business object will call SaveChanges(). At that point, the database can generate values for all of those persisted entities. I would like for my Child business objects in the graph to be able to register their interest in entities that they have added / updated, so that after SaveChanges is called, they can get notified, and get access the new state of the entity now that it has any database generated values (such as primary keys etc) in order to update their own state to match.

popcatalin81 commented 6 years ago

A very useful use case of Lifecycle hooks is to implement Revisions (I've used this in several applications with EF6)

How it works: when a user edits an entity IE: Post, during save the EntityState is changed from Modified to Added and the RevisionNr is increased. This way as simple workflow, Load, Edit, Commit is used (that does not require copying object values and adding them as new) to create a Revisions based system that always inserts new revisions (instead of updating the original entity).

A second use case is to compute complex properties eagerly. Like decode a binary blob and update a specific graph (In cases where this cannot be done lazily), IE: loading complex scheduling rules and updating a scheduling graph (this will be done eagerly)

A third use case is Detailed Change Auditing (usually the more enterprisey the project, the, more this is required)

nphmuller commented 6 years ago

Our use case is to implement global query caching. Basically we'd need a hook to capture a read query just before its executed. If the key does not exist in the cache the query will be executed on the database and the result will be cached.

The other hook we'd need is when a transaction is committed (or rolled back), so we can invalidate the proper items in the cache. Overriding SaveChanges() will probably also work instead of this hook.

See also: https://github.com/aspnet/EntityFrameworkCore/issues/5858#issuecomment-306441806

In EF6 we used https://github.com/moozzyk/EFCache and we implemented our own Redis provider for this.

markusschaber commented 6 years ago

We have some "StatusService", where clients can subscribe (via .NET events or WebSocket) so they get notified when something changes. This includes some hysteresis / rate limiting of notifications. Part of the changes are changes to (a subset of) our database entities, so they can react accordingly (e. G. update their UI, or schedule synchronization of some data, etc...). So an "this database entity has changed" event is what we'd need. (Currently, we do this via overwriting SaveChanges[Async]() Methods which triggers events on an injected singleton service, and subscribers use the dbcontext ChangeTracker to check which entities they're interested.)

codetuner commented 6 years ago

We have several uses for them: 1) In the OnSaving event (which we trigger from an overloaded SaveChanges) we update simple tracking information (UpdatedBy, UpdatedWhen,...). 2) A more advanced version of the tracking logs old and new values of changed properties in new entities. 3) Some objects have complex graphs of subobjects associated which we prefer to store as a JSON string in a single database field. We then use both OnObjectMaterialized and OnSaving to deserialize/serialize the JSON string and populate the entity. I have described this solution in detail on https://www.codeproject.com/Articles/1210354/Hybrid-Storage-relationalplusdocument-with-Entity 4) Though there should probably be a better solution for this one, but we capture the OnSaving event to throw an exception when an entity(type) is considered read-only and has a Changed state.

markusschaber commented 6 years ago

@codetuner For your use case 3 (JSON): https://github.com/aspnet/EntityFrameworkCore/issues/4021 may help there, and I could also envision a different implementation (with conversion logic in the Entity) which works without overriding those methods or LifeCycle hooks.

michaelaird commented 6 years ago

We have 2 cases

  1. Database.Connection.StateChange - we hook into Connection.Open to issue a command to set the current TenantId for multi-tenancy.
  2. ObjectContext.ObjectMaterialized - we hook into the loading of certain object types so we can call DateTime.SpecifyKind to specify that datetimes are in UTC (instead of Unspecified)

1 is really our primary concern, 2 is a nice to have.

sjb-sjb commented 6 years ago

Per my posts of Jul 13 and Dec 4, I am using hooks on the DbContext to detect entity state transitions. The ask is to include ILocalViewListener or similar in the public API.

I use the hooks primarily for two purposes

(a) to inform an observable list of entities of a change in entity state, so that the list can be maintained appropriately (e.g. entities added or removed from the selected set of entities). The observable list is being used as a view model, more or less like a customized LocalView but not attached to a DbContext. It is a free-standing list of entities that is being accessed by the end user. Individual units of work are sent in from various parts of the program and the list provides an integrated view of the resulting entities.

(b) to inform the view model of an individual entity that there has been a change of entity state. This is being used in an entity view model system along the lines of the one in Template 10 but including entity state information. When a property is changed in an entity, a unit of work is opened and the view model reflects a modified status for the property; when SaveChanges is used to complete the unit of work, the entity state transition hook is used to inform the view model that the property is no longer dirty (i.e. the value has been updated). This is very convenient because the callback provides exact information about the specific entity that was changed (or whose changes were rolled back), and there is no tight coupling between the part of the program that submitted the SaveChanges and the part of the program that is using the entity view model.

Broadly speaking, hooking into the entity state change provides a way of driving view model updates and maintaining a single consistent internal state (a) without having to explicitly tie together the receiving view and view model with the UI or other program code that was used to generate and submit the entity changes and (b) without having to reproduce the logic that EF uses to track state and to commit or rollback changes.

These uses are somewhat similar to the objective that @markusschaber cited ("changes to (a subset of) our database entities, so they can react accordingly e.g. update their UI, or schedule synchronization of some data") and to the use that @codetuner described ("logs old and new values of changed properties in new entities") although we are updating UI instead of logging.

Note: in this application, concurrency contention is low so it is OK for a copy of the data to stay around in the app for a while.

SidShetye commented 6 years ago

We need these/equivalents from ObjectContext ...

  1. ObjectMaterialized
  2. SavingChanges

And preferably another hook after SavingChanges completes (maybe SavedChanges ?) Hope to see these soon!

markusschaber commented 6 years ago

@michaelaird I think a general option to specify whether Timestamp columns are UTC or local would be a better alternative here, what do you think? (Maybe this should be discussed somewhere else?)

michaelaird commented 6 years ago

I think they should deprecate DateTime and ‘bless’ NodaTime (the way they did with Json.net). But that is definitely a discussion for somewhere else.

That said we jump through hoops to translate nodatime types to datetime for storage so a mechanism to define those translations at the dbcontext would be hugely useful.

On Fri, Jan 19, 2018 at 3:02 AM Markus Schaber notifications@github.com wrote:

@michaelaird https://github.com/michaelaird I think a general option to specify whether Timestamp columns are UTC or local would be a better alternative here, what do you think? (Maybe this should be discussed somewhere else?)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aspnet/EntityFrameworkCore/issues/626#issuecomment-358893510, or mute the thread https://github.com/notifications/unsubscribe-auth/AANX6YLjFey2wq0V4r-xY9rCQwM30d9qks5tMEwegaJpZM4Cesel .

javiercampos commented 6 years ago

@michaelaird you can workaround point 2 with something like:

public static class DateTimeMapper
{
    public static DateTime SetUtc(DateTime value)
    {
        return DateTime.SpecifyKind(value, DateTimeKind.Utc);
    }
}
public class MyMaterializerSource : EntityMaterializerSource
{
    private static readonly MethodInfo SetUtcMethod = typeof(DateTimeMapper).GetTypeInfo().GetMethod(nameof(DateTimeMapper.SetUtc));
    public override Expression CreateReadValueExpression(Expression valueBuffer, Type type, int index, IProperty property = null)
    {
        if (type == typeof(DateTime))
        {
            return Expression.Call(
                SetUtcMethod,
                base.CreateReadValueExpression(valueBuffer, type, index, property)
            );
        }
        return base.CreateReadValueExpression(valueBuffer, type, index, property);
    }
}

And when setting your DbContextOptions, on your DbContextOptionsBuilder:

options.UseSqlServer(/* ... */);
options.ReplaceService<IEntityMaterializerSource, MyMaterializerSource>();

I agree it could be more user friendly with a hook like ObjectMaterialized, but it works.

For point 1, you can pass your own connection if you don't want the DbContext to create it for you and handle StateChange on it (for example, with UseSqlServer, using this overload), for example:

var myConnection = new SqlConnection(connectionString);
myConnection.StateChange += (s,e) => { /* */ }
/* then, in your options builder */
options.UseSqlServer(myConnection);
javiercampos commented 6 years ago

@sidshetye just out of curiosity, what would you need on a SavingChanges hook that can't be done by overriding SaveChanges/SaveChangesAsync?

You could implement that easily, with something like:

public class MyDbContext : DbContext
{
    public event EventHandler SavingChanges;
    public event EventHandler SavedChanges;

    public override int SaveChanges(bool acceptAllChangesOnSuccess)
    {
        SavingChanges?.Invoke(this, EventArgs.Empty);
        var returnValue = base.SaveChanges(acceptAllChangesOnSuccess);
        SavedChanges?.Invoke(this, EventArgs.Empty);
        return returnValue;
    }

    public override async Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken = default(CancellationToken))
    {
        SavingChanges?.Invoke(this, EventArgs.Empty);
        var returnValue = await base.SaveChangesAsync(acceptAllChangesOnSuccess, cancellationToken);
        SavedChanges?.Invoke(this, EventArgs.Empty);
        return returnValue;
    }
}

Maybe there's something I'm missing?

popcatalin81 commented 6 years ago

@javiercampos I'm thinking a lifecycle hook inside EF could potentially allow access to the Update pipeline allowing you to modify the resulted SQL, or modify the way store generated values are read back, making this feature much more powerful and making more scenarios possible than overriding SaveChanges. For example instead of single Update statement, you could send multiple statements interleaved with other updates. You cannot do this by overriding SaveChanges.

javiercampos commented 6 years ago

@popcatalin81 oh yes, I definitely agree as many "advanced scenarios" lifecycle hooks as possible should be implemented (and in a user-friendly way if possible), that's why I'm following this thread. Was just curious about @sidshetye's request (he was asking for the equivalent to ObjectContext.SavingChanges, and a possible SavedChanges event which are simple enough to implement yourself, not to overload the busy team :smile: ).

sjb-sjb commented 6 years ago

@javiercampos , the challenge with just overriding SaveChanges and SaveChangesAsync is that you then have to keep track of the entities that were changed. While it is true that you can copy this information from ChangeTracker before the save and then iterate through it again after the save, it seems more natural to me to have the system notify you of the changes in each entity. In addition this approach will not notify you of changes that are not due to SaveChanges, e.g. due to a property assignment, a revert changes (state = Unmodified) change, etc. Again you can build this yourself but, given that the ChangeTracker is already figuring all this out, it makes more sense to simply have the system notify you when there is a change.

About 'overloading the busy team', I agree they are busy (!!) and doing a fantastic job. On the other hand, this is a hook that is already there (in the ILocalViewListener service) that just needs to be included in the public API.

javiercampos commented 6 years ago

@sjb-sjb again, yes, but that's not what ObjectContext.SavingChanges did, and that's what I was replying to (@sidshetye specifically asked for the equivalent to ObjectContext.SavingChanges)

SidShetye commented 6 years ago

@javiercampos our data encryption platform (see http://www.crypteron.com) supports C# and within C#, we also support Entity Framework. We need SavedChanges to make it user friendly for our common Entity Framework customers. Because we can then directly plug into the hooks EF provides (plug and play) rather than us telling them "... and don't forget this snippet of custom code" - which busy developers always seem to forget! Specifically, we need this hook to cryptographically verify data integrity after saving to storage (SQL in EF's case). For contrast, the Java Persistence API (JPA) has such hooks, so our guidance there is very simple and I'd like to keep my C# customers equally happy! I raised this since adding a hook is simpler when already adding other hooks and seems the implementation could be straight forward.

About ObjectContext: We're not married to this specific interface but it's what we've used for our EF6 integrations, so we're going by that. I checked with my team and we're using the following API's. A same/improved API would be desired

In addition we're using these APIs DbContext.Configuration.AutoDetectChangesEnabled DbContext.Entry(..) DbEntityEntry.OriginalValues DbEntityEntry.CurrentValues

javiercampos commented 6 years ago

@sidshetye I see, that makes sense... in the meantime, one possible workaround (which involves work from the customer though) would be making a base DbContext from which they must inherit to use your framework. Not ideal on your specific case, yes, but better than having your customers override SaveChanges themselves

ajcvickers commented 6 years ago

Thanks for all the feedback!

Notes for triage:

ajcvickers commented 6 years ago

Decision from triage: we will attempt to get state change events in for 2.1 as a stretch goal. Moving other events out for now.

dazinator commented 6 years ago

Just sharing a quick and dirty implementation to get the type of notification I needed for grabbing ID values after a save:

   // DbContext declared as partial class so implementation not lost if rescaffolding from db.
    public partial class FooDbContext 
    {
        private readonly List<Action> _notifySavedList = new List<Action>();

        protected void NotifySavedChanges()
        {
            foreach (var item in _notifySavedList)
            {
                item();
            }
            _notifySavedList.Clear();
        }  

        public void OnAfterSaveChanges(Action p)
        {
            _notifySavedList.Add(p);
        }

        public override int SaveChanges()
        {
            var result = base.SaveChanges();
            NotifySavedChanges();
            return result;
        }

        public override int SaveChanges(bool acceptAllChangesOnSuccess)
        {
            var result = base.SaveChanges(acceptAllChangesOnSuccess);
            NotifySavedChanges();
            return result;
        }

        public override async Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken = default(CancellationToken))
        {
            var result = await base.SaveChangesAsync(acceptAllChangesOnSuccess, cancellationToken);
            NotifySavedChanges();
            return result;
        }

        public override async Task<int> SaveChangesAsync(CancellationToken cancellationToken = default(CancellationToken))
        {
            var result = await base.SaveChangesAsync(cancellationToken);
            NotifySavedChanges();
            return result;
        }

       public override void Dispose()
        {           
            _notifySavedList.Clear();
            base.Dispose();
        }

}

Use case looks something like this:


public class FooChildBusinessObject
{         
           public int Id { get; private set; }      
           public string Name { get; set; }      

           public bool IsNew { get; set; }     // Whatever logic you use to track inserts vs updates.

           public void Update(DbContext dbContext)
           {
               if(IsNew)
               {
                        var fooEntity = new Foo();
                        fooEntity.Name = Name;
                        dbContext.Foo.Add(fooEntity);
                        dbContext.OnAfterSaveChanges(() =>
                        {
                             // grab the database generated ID..
                             Id = fooEntity.Id;
                        });
               }
              else
              {
                        var fooEntity = dbContext.Foo.Single((f)=>f.Id == Id);
                        fooEntity.Name = Name;
              }       
          }
}

public class RootBusinessObject 
{
    public FooChildBusinessObject Foo { get; set; }   

    public void Save()
    {
          // This is some parent / root object saving children in its object graph.
          // It calls SaveChanges() after all objects in the graph have added their changes to the dbcontext.
         using (var dbContext = GetDbContext())
         {            
              Foo.Update(dbContext);
              dbContext .SaveChanges();       
              // Foo.Id should now be populated!      
         } 
    }     

}          
SidShetye commented 6 years ago

@ajcvickers Is there documentation for committed public API for entity state changes? EntityFrameworkCore/src/EFCore/ChangeTracking/ChangeTracker.cs seems to be evolving on which APIs are internal and which ones are public. We're investigating if we can make any progress till SavingChanges and ObjectMaterialized (and hopefully SavedChanges) are implemented ... hopefully not too far away!

ajcvickers commented 6 years ago

@sidshetye Docs are not done yet--tracking issue is https://github.com/aspnet/EntityFramework.Docs/issues/661. The public surface as it is now is what we plan to ship for 2.1, unless we get significant feedback from the prereleases that cause us to re-think.

sjb-sjb commented 6 years ago

Thanks for this!

A few comments based on looking at ChangeTracker.cs and also based on the experience I've had with the earlier, nonpublic version (LocalViewListener).

ajcvickers commented 6 years ago

@sjb-sjb Thanks for the comments--I referenced them from the docs issue.

With regard to the Tracked event, that was the result of a long discussion on whether or not something being tracked for the first time is just a state change from Detached to something else, or whether it is a distinct event. Specifically, are there common scenarios where they would be used independently. My initial thought was that it was all just one state change event, but the ultimate outcome of the discussion was a team decision to create two different events. So you will never get state changed events from Detached to something else--those will instead be Tracked events.

Can you file a new issue for your last point? I think this is useful information for EF to provide. However, keep in mind that when normal non-notifying entity types are used, then it requires DetectChanges to run to determine if there are any entities in a given state, and this always involves scanning all entities, so this may never be super-performant unless notifying entities are being used.

markusschaber commented 6 years ago

Is there any docs on "notifying entities"? This is the first time I read about them...

ajcvickers commented 6 years ago

@markusschaber Docs are being tracked as https://github.com/aspnet/EntityFramework.Docs/issues/531

In a nutshell, have your entities implement INotifyPropertyChanging and INotifyPropertyChanged, and then use ObservableCollection for collection navigation properties. There are a quite a few places in the tests that you could look at.

sjb-sjb commented 6 years ago

@ajcvickers thanks for the comments.

I think that the FromQuery flag in the tracked event is useful. It looks like this will let you know whether the tracking is due to materialization or not. I could imagine this being extended to indicate whether, for example, tracking was caused by (a) an explicit assignment to the EntityEntry.State or by being the head entity in an Add, Update, etc, or (b) was due to the entity being reachable from a head entity that was tracked due to Add, Update, etc.

It is a little bit odd that we have a NewState in the entity transition event but do not have it in the Tracked event. Presumably the state of a newly tracked entity could be either Unchanged or Added, for example. If the user prevents entity state changes during the transition event and during the tracked event, then there is no need for NewState since it will remain equal to the EntityEntry.State. On the other hand since this locking is not built into EF there is always the possibility that the state could change. In conclusion it would make sense to me to add NewState to the tracked event. Caveat: I'm doing some guessing here since I haven't tried the new interface.

I will create the issue as you requested. As relates to the efficiency of state testing, as a matter of fact I am using notifying entities. One reason in fact for using notifying entities is to avoid the inefficiency of DetectChanges. Another reason, however, is the same as the reason for using EF state callbacks, namely to build a view-model system. If you are going to implement a view-model system then you have to notify property changes anyway, so you may as well leverage these notifications for use by EntityFramework tracking. Concerning the state change, I attach the entity to a context the first time a property is assigned and then the state callbacks are used to manage the entity's presence and sorting in observable collections supporting the interfaces required by ListView (this is a view-model activity and, I might say, not a very simple one -- especially if the collection is paged).

I'll tell you an overall comment. I realize that EF is focused on data access, that is it's raison d'etre. At the same time I don't think MS should ignore the fact that data access operates within UI programs (and within web services, etc.) As a consumer of MS products I think MS has done a fantastic job on the data access part, but it is really surprising to me that little has been done on integrating this into a coherent program-building paradigm. Specifically I feel MS should put effort into building a really professional view-model layer that fits on top of EF. Many people are out there cooking up their own, it is very duplicative and I imagine in most cases leading to not nearly as robust a solution.

Wain123 commented 6 years ago

Are you planning to have an event after entities are created and the context is available to be used again?

After I load an entity from the DB, I want to initalize it, which would look something like this:

ctx.ChangeTracker.Tracked += (sender, e) => {
    ((BOBase)e.Entry.Entity).Init();
};

The problem is, if Init() attempts to execute a DB query, it will throw

System.InvalidOperationException: 'A second operation started on this context before a previous
operation completed. Any instance members are not guaranteed to be thread safe.'

because the Tracked event is invoked while the context is still busy with an operation.

If a query starts tracking multiple entities, I'd want an interception point after the whole query is completed (so, the context can be used again), not after each individual entity is loaded.

ajcvickers commented 6 years ago

@Wain123 We discussed such an event back in the EF4 days, but it is very problematic to implement because it would have to fire only after each entire query has been enumerated, which is something that is controlled by the caller. So at that time we decided that it was better for the application to define and control such events. That way the application can know when it has finished executing whatever queries it needs, even if that is several, and then fire and event to other parts of the application that should respond.

Wain123 commented 6 years ago

@ajcvickers Ok, that's inconvenient, maybe there's another approach - is there a way to not get the InvalidOperationException when executing a DB query inside the ChangeTracker.Tracked event? The context becomes available very soon after the Tracked event (it's available inside the loop where I'm enumerating the query), so could this Tracked event be sent slightly later to allow me to Init each entity right as it becomes tracked?

javiercampos commented 6 years ago

Did any of this got through into 2.1? If so, any docs available?

I'm particulary interested on an ObjectMaterialized replacement... I see you have now the Tracked event on the ChangeTracker, but I'm afraid that will not do if you make a query with AsNoTracking().

Is there any "official" hook (apart from EntityMaterializerSource which, although it's what we are using, is internal and has actually changed from 2.0 to 2.1) to get when entities are created?

ajcvickers commented 6 years ago

@javiercampos The only new hooks added in 2.1 were for the Tracked and StateChanged events. There is no ObjectMaterialized yet--the main reason being that the code and patterns in this area are changing with the introduction of constructor binding and factory support. However, how all the API hooks up and when to use the event over a binding, if ever, is till to be determined. See #10789

AFDevMike commented 6 years ago

The InternalEntityEntry received via IEntityStateListener's StateChanged doesn't seem to contain the values set by the database itself such as those which are set by .ValueGeneratedOnAddOrUpdate().

Should they? (No I learned)

Update - This was a fundamental misunderstanding on my part. Entities are not updated from the Db after SaveChanges[Async]() call unless they are specific properties such as Primary Keys or RowVersion(I don't know the full extent but certainly Primary Keys). Therefore properties which are not the above and are .ValueGeneratedOnAddOrUpdate() cannot be expected to be current.

dazinator commented 6 years ago

@jdevmike - I worked around this using the hand rolled solution I posted above. If I should be replacing this with a new EF mechanism please let me know.

SidShetye commented 6 years ago

@ajcvickers when can we expect ObjectMaterialized and SavingChanges equivalents to be released? Waiting but hoping it's not a long wait 🤞

tourili commented 6 years ago

Hi @divega

What about intercepting connection like we used to do on EF6 using IDbConnectionInterceptor. Interception is not for logging only!

My asp core app requires executing session context EXEC sp_set_session_context ... on the server each time database connection is opened.

My only workaround now is to hook on the event Database.GetDbConnection().StateChange. which is my last option.

Thanks for help

SidShetye commented 5 years ago

@ajcvickers Following up on the April 7th comment; our proof-of-concept with EF Core 2.1 confirms that ChangeTracker.Tracked and ChangeTracker.StateChanged events in a DbContext are insufficient for us to get the job done on EF Core 2.1. Coming from EF 6.0, we did have this hunch. But can now confirm that we are indeed blocked from supporting EF Core 2.x till SavingChanges and ObjectMaterialized (and hopefully SavedChanges) make the release. Would appreciate you sharing your roadmap on this so we can adjust ours accordingly.

divega commented 5 years ago

@SidShetye re Tracked and StateChanged, are they insufficient because they don't trigger when non-tracked objects are materialized or because you need to be able to perform operations that can potentially use the context to execute queries while processing the event?

SidShetye commented 5 years ago

@divega for us they are insufficient because there are no event hooks at the moment of entity materialization from the db (like ObjectMaterialized) as well as at the moment just prior to the entity hitting the network (like SavingChanges). A 3rd event hook that fired after an entity (e.g. SavedChanges) has been written to the network would also be extremely helpful.

sjb-sjb commented 5 years ago

@SidShetye concerning the 3rd event hook, given that before the SaveChanges the state will be Modified, Added or Deleted and afterward it will be Unchanged or Detached, can you not use the StateChanged event?

dazinator commented 5 years ago

@sjb-sjb Just to play devils advocate, given you can detach and attach entities, and change the state of entities yourself, a state changing event is not necessarily any guarantee that the entity was actually written to the network right?

SidShetye commented 5 years ago

@sjb-sjb : Our reason is the same as @dazinator i.e. the state transitions

(Modified | Added | Deleted ) => (Unchanged | Detached)

could happen for certain permutations of user-level usage patterns even when the entity doesn't actually hit the network.

sjb-sjb commented 5 years ago

Yeah, I agree if you let the user do anything then state transitions won't correspond to writes. I could see it happening with a library intended to complement EF for example.

In my case I just don't let them do anything weird, and if they do then they pretty much just get what they deserve 😏.

juanmarl commented 5 years ago

@Wain123 When you materialize a query with many entities you get a bunch of ChangeTracker.Tracked events, one per each entity in the query result. With this code you can call Init() when the object graph is completely materialized:

(You still can't use the object context to make further queries, but you can eager load everything you need in the same graph and then Initialize it on due time. Another option would be to use directly a DbCommand or another instance of the context for these queries).


        private void ChangeTracker_Tracked(object sender, EntityTrackedEventArgs e)
        {
            if (!e.FromQuery)
                return;

            var entity = e.Entry.Entity;

            if (entity is IInitializable initializable)
                _initializableEntities.Add(initializable); //keep until materialization completes and then call Init()

            if (_dbConnection.State == ConnectionState.Closed) //last entity in graph
            {
                 e.Entry.State = EntryState.Detached;
                 e.Entry.State = EntryState.Unchanged; //force change tracker to fix all entry navigations

                 _initializableEntities.ForEach(x => x.Init());
                 _initializableEntities.Clear();
            }
        }
sjb-sjb commented 5 years ago

Just a comment based on some recent experience and reviewing the various comments above.

Many of the comments / issues raised are connected to the fact that Tracked / StateChanged events occur during materialization or during SaveChanges. For example we see a number of comments seeking initialization hooks, which would be addressed if we knew that state transitions occurred after completion of queries or completion of saves (writes).

What I recently realized is that the processing of state changes during the process of materialization or during the process of SaveChanges can lead to inconsistent application state because the various entities in the application (for example a set of entities currently attached to a context and being saved) will be at different stages of being processed by EF. The example I ran into was a SaveChanges of a graph of entities in the context. Some of the entities had been saved and their store-generated ID's set, while other dependent entities had foreign keys (and navigation properties) onto these saved entities but the foreign keys had not yet been updated to match the store-generated id's. That is, the foreign key was a negative number (EF-generated placeholder) while the referenced entity pointed to by the navigation property had a positive id (store-generated). Clearly a bad situation.

Thinking about this, I wonder if I need to put the Tracked / StateChanged events into a producer/consumer queue and pull them off asynchronously for processing after the materialization or SaveChanges is complete. The asynchrony could also help to avoid accidental re-entrancy while processing the state changes. This leads back to the question of having a hook for when materialization or SaveChanges is complete.

If people feel it would be useful to have these events be processed asynchronously after the materialization or save is complete then maybe it could be considered for inclusion as an option in EF.

sjb-sjb commented 5 years ago

On the other hand it is also true that sometime you do want the synchronous invocation. For example this is useful if you are trying to track state information that is synchronized with EF state information.