Saving DBContext with UnitOfWork pattern automatically

Question

So using the UnitOfWork pattern in MVC I don't want to have to call unitOfWork save method every time I modify an object. Usually with UnitOfWork you do something like this:

    if (ModelState.IsValid)
    {
        var user = new User()
        {
            Id = Guid.NewGuid(),
            Username = model.Username,
            Email = model.Email,
            Password = model.HashedPassword()
        };
        unitOfWork.UserRepository.Insert(user);
        unitOfWork.Save();
    }

I'd like to remove the "unitOfWork.Save();" line and just know that it will save every time an action completes. So I added a save clause to the Dispose method of my UnitOfWork:

    protected virtual void Dispose(bool disposing)
    {
        if (context.ChangeTracker.HasChanges())
        {
            context.SaveChangesAsync();
        }
        if (!this.isDisposed)
        {
            if (disposing)
            {
                context.Dispose();
            }
        }
        this.isDisposed = true;
    }

And of course my controller calls the dispose:

    protected override void Dispose(bool disposing)
    {
        unitOfWork.Dispose();
        base.Dispose(disposing);
    }

I'm confident that the HasChanges() method works as expected and SaveChangesAsync() is called, yet this doesn't work. I'm thinking that it could be something to do with the thread created by SaveChangesAsync not completing because it the object it depends on is disposed?

But if that's the case then SaveChangesAsync must be dangerous, because even if you used it in the controller action it could get transaction locked for a few seconds and find that the context has been disposed before it gets a chance to save.

So what am I doing wrong? I can't seem to find an established way of doing this, yet I can't imagine that everyone who uses the unit of work pattern has to remember to manually call the Save method every time they modify any objects.

But what if there are errors? The Dispose() is a little late in the chain to handle/report them. All in all this is just a bad idea. — H H, Jul 05 '15 at 12:19
This unit of work usage is an [anti-pattern](http://blog.sapiensworks.com/post/2014/06/04/Unit-Of-Work-is-the-new-Singleton.aspx) — MikeSW, Jul 05 '15 at 12:33

score 2 · Answer 1 · answered Jul 05 '15 at 12:28

Although I agree that using Dispose() to save changes is a bad idea, the reason things are not working is because Dispose() is not an async method. So calling context.SaveChangesAsync() directly followed by a context.Dispose() will probably dispose the context while async processing is still going on or has yet to start in the background.

You should at the very least switch to SaveChanges().

score 1 · Accepted Answer · edited May 23 '17 at 11:45

1

As another possibility, since you state you want each action to save automatically, you could do the save somewhere like here on ending the request:

MVC End Request

The pattern which does this (in one way or another) is actually called

Unit of Work per request

you can find more details about it online, and it is suitable for some web applications (altough comes with many variations)

edited May 23 '17 at 11:45

Community

1
1

answered Jul 05 '15 at 12:29

Denis Biondic

7,943
5
48
79

Denis Biondic · Answer 3 · 2015-07-05T12:20:42.533

0

Calling SaveChanges() for multiple operations together (meaning calling it only once) has many advantages, but also not every application needs those advantages. If you have a base repository class somewhere, why don't you define it with Add / Remove / Update operations where each at the end would call SaveChanges()?

Something like:

public abstract RepositoryBase<T>
{
    public void Add(T entity)
    {
        context.Add(entity);
        context.SaveChanges();
    }

    public void Remove(T entity)
    {
        context.Remove(entity);
        context.SaveChanges();
    }

    etc...
}

Using Dispose for Saving changes I find really bad IMO.

Note on role of Unit of Work after this change: the only role it would have is being a wrapper over a context, making sure all repositories share the same context, but its original main responsibility will be gone.

edited Jul 05 '15 at 12:20

answered Jul 05 '15 at 12:13

Denis Biondic

7,943
5
48
79

Thanks for the info, could you go into a bit more detail? Why is it good to use save changes multiple times? I've always tried to ensure that there is only one database update transaction during a request, assuming that that's more efficient. And what is it about disposing to save changes that you don't like- is it that the principle is wrong, that a dispose method should only dispose? – Jansky Jul 05 '15 at 12:19
I also meant calling save changes is good when called less times, check the edit I clarified a bit on the top. As for the Dispose, it is a clear violation of at least several principles – Denis Biondic Jul 05 '15 at 12:22
I added another different answer which might add another perspective – Denis Biondic Jul 05 '15 at 12:31

Saving DBContext with UnitOfWork pattern automatically

3 Answers3