Implement RejectChanges() in DbContext

[ice1e0]

Story

There are many user requests for a function in DbContext class to reject or discard changes. See as well:

• https://stackoverflow.com/questions/5466677/undo-changes-in-entity-framework-entities
• https://stackoverflow.com/questions/16437083/dbcontext-discard-changes-without-disposing/16437305

Microsoft sample code: How to undo the changes in Entity Framework 4.1 and later

I first implemented a wrong version which I found online which caused terrible data loss. I was able to fix it with a similar solution as the Microsoft sample code is, see above.

Request

I would appreciate if a function DbContext.RejectChanges() or a similar one would be implemented in the official version of Entity Framework Core.

Further technical details

EF Core version: 2.1.4
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows
IDE: Visual Studio 2017 15.8.6

[mohamedmahmoudalemam]

I think this may be possible if there exists a state(backup) of database before and after each call for SaveChanges, and RejectChanges(state) return the database to the marked state.

[ajcvickers]

The recommended approach is to use a single DbContext instance for each unit-of-work, disposing it when done. If you need to discard all changes in a unit-of-work, then do so by disposing the context without saving.

[StrangeWill]

A use case I ran into:

• Attempting to do a lot of EF work
• Upon error, create a log entry and log it to the database
• If an error happens in the middle of EF work, the EF state will be invalid (missing objects, not cleaned out other objects, etc.)
• Log insert usually fails due to prior state

My options are:

• Inject a DbContext factory instead of a DbContext, request a new instance from the factory when logging errors, pray that the original DbContext isn't holding onto anything transaction-wise.
• Loop through the change tracker dropping everything.
• Inject the entire IServiceScopeFactory and manually create a new scope every time I need to dump a DbContext state (which sure, isn't often, but still).

Issue with 1) is that EF Core doesn't have a recommended way to even do this: #10694 (comment), so it's both a recommended way to accomplish a task, while being unsupported.

I agree 2) is kind of gross, without the context being designed for that (at least that's what I got from this issue), it may be doing some awful things we don't want it to. If it was a first-class supported flow this is would be super easy.

3. Is probably the "most DI friendly" while basically requiring us to be very explicit in scope, within basically every try/catch block that may need to throw out EF's state having it's own scope which is a bit rough.

---

Maybe I'm just missing something... but it seems like all options have pretty poor downsides of either support or code maintainability, so I think some more supported solution to this should be considered a bit more.

[giovinazzo-kevin]

+1. Not all DbContexts are made equal and some are long-lived objects that can't be disposed of very easily. Some are singletons, even, and they can't be disposed of without refactoring them into not being singletons in the first place.

Replacing them with factories and providers sure sounds like a pain - one that should be mitigated by the framework in this case.
Furthermore, given the SaveChanges syntax it would only make sense for there to be a DiscardChanges as well.

Another option would be to let each DbContext expose a method through which you can create a scoped sub-context, integrating the factory pattern seamlessly.

It's also worth mentioning that ScopedTransactions don't work at all here, despite old StackOverflow threads suggesting otherwise, and that manually iterating the ChangeTracker's entries sounds pretty risky, if not plain wrong.

This thread in particular is a great example of how confusing this all is - no two answers are the same!

[syndicatedshannon]

I have to agree with G3 and other requests here.

Because the UoW pattern crosses knowledge boundaries, we can't really ensure we don't save at the end of our scoped operation without a UoW-bound flag. RejectChanges() is one way that could be provided.

Otherwise, we have to ensure our exceptions surface all the way up, beyond the last SaveChanges() call, which breaks other SoC.

[syndicatedshannon]

Also, FYI, I see that RejectChanges() could easily lead to misunderstandings about the state of the context. Can we then apply more changes after rejection? etc. My prior comment about boundaries still applies, and I'm not sure what the solution is.

[dradovic]

As a possible work-around I would like to mention this library: EFCore.DbContextFactory.

[rmihael]

With Blazor Server, I have a common flow like this:

1. Create a DB context using the factory class (as recommended by https://docs.microsoft.com/en-us/aspnet/core/blazor/blazor-server-ef-core?view=aspnetcore-5.0#new-dbcontext-instances-5x);
2. Use Attach to bring some entities into the tracking context;
3. Make some changes in entities;
4. Call SaveChanges to persist these changes;
5. Dispose the DB context.

In case of any exception during step 3, I really want to roll back all modified entities to the state as they were initially. Currently, I am using the trick from https://stackoverflow.com/a/22098063/685808, but it feels hackish. With the rise of Blazor's popularity, that kind of workflow will happen more often. It makes sense to either provide some support for it in EF Core or suggest some workarounds in the documentation.

[KernelMC]

Does DbContext.ChangeTracker.Clear() work for this purpose? According the documentation it avoids to overhead of looping through the change tracker and detaching entities one by one. The downside to this is that all entities are detached, even those that were unmodified.

[nicholasyin]

What if my first call to SaveChanges failed, then I wanted to keep using the same context to do some other work, such as logging to database? In this case, my second call to SaveChanges, which is supposed to do ONLY logging would fail, because it would include all the previous changes that have failed the first call. The whole thing is indeed a unit of work.

[syndicatedshannon]

@nicholasyin : In that specific case I recommend using a separate context for logging. which probably has other benefits in your app as well.

[nicholasyin]

@nicholasyin : In that specific case I recommend using a separate context for logging. which probably has other benefits in your app as well.

Logging is just an example. The point is to have a unified calling style to save changes. So it should accommodate for both normal and exceptional scenarios. Therefore, using a separate context would require me to have a second context anyway, even for normal cases.

[syndicatedshannon]

I hear you @nicholasyin . I appreciate your desire to keep the "number of contexts" down, but you really probably DO want two contexts for that sort of operation. You want to be able to complete logging operations independent of business transactions.

Can you give a different example where that wouldn't be the case?

[syndicatedshannon]

logging operations should probably be fire-and-forget, not dependent on saving business data... and the logger definitely shouldn't have access to the context where business work is being done.

Not that you would do so, but I've seen a developer call .SaveChanges on a logger with shared context before. Not pretty.

[nicholasyin]

@syndicatedshannon Thanks for the reply. As I mentioned, logging is simple an example and I'm aware of the proper way of implementing logging. After all, the original intention is about the necessity of a RejectChanges() method or some mechanism to reset the context to a 'clean' state. Retrieving a new context instance would probably suffice and function the same as resetting an existing context to the clean state.

[jtheisen]

This really should exist, and the semantics of RejectChanges should be the same as it used to be in EF Ria Services: Afterwards, the context can be used again.

This is needed when building data-entry-style user interfaces (in Desktop and Blazor apps): The user checks a checkbox, leading to a save operation. If that fails, the change needs to be reverted, so the user interface isn't incorrectly displaying the change after that. The knowledge of what the original state was is known to EF, so EF should cover that job.

The alternative would be to create a new context, attach a clone of the object as modified and only on success modify the original object for the user interface. This is cumbersome in this simple example, it gets absurd if we're talking about more complex scenarios like additions and removals.

The missing reload functionality is related to this.

[jtheisen]

For those souls finding this ticket, here's an implementation that will work for EF7 and later:

        static void RevertChangesEf7(this DbContext context)
        {
            foreach (var entry in context.ChangeTracker.Entries().ToArray())
            {
                switch (entry.State)
                {
                    case EntityState.Added:
                        entry.State = EntityState.Detached;
                        break;
                    case EntityState.Deleted:
                    case EntityState.Modified:
                        entry.State = EntityState.Modified;
                        entry.State = EntityState.Unchanged;
                        break;
                    default:
                        break;
                }
            }
        }

This will, on EF7, revert all properties of all entities, including the navigation properties.

This code was given on StackOverflow a long time ago, but it only started working properly very recently, because earlier versions of EF did no fix the navigation properties on resetting the entity states - although it did change them in the first place when modifying non-navigational properties.

I implemented a more complex version of this for EF Core 6 (before I realized it was a trivial problem on EF7) which I have put into my Reconciler library so that the tests can live somewhere.

[DamirLisak]

@jtheisen : This workaround doesn't restore navigation properties if you use Lazy Loading with Microsoft.EntityFrameworkCore.Proxies or Microsoft.EntityFrameworkCore.Infrastructure.Internal.LazyLoader.

@ajcvickers :
Why is this tagged with "Closed as not planned"?
This feature, the refreshing of entity objects (navigation properties and collections) and the change tracker worked well in EF4/5/6. Why is Microsoft reluctant to implement important functions that have always worked for stateful applications?
In this context, I would just like to remind you of the following issues:
#16491
#31819