Multiple SaveChanges calls in entity framework
Asked Answered
W

6

36

I am building my own custom repository, based on entity framework, and I'm creating some extension methods that allow me to save partial view models as entity models so I'm building my own Add and Update methods.

Currently, each method has SaveChanges() from DbContext called at the end which means for every model, one call will be invoked.

I'm building this base DAL pattern for MVC4 sites which means most of the time I will access 1 model, but it does not have to be the case though.

Is it too bad practice to call SaveChanges() for each model when updating i.e. 3 entities or should I add everything first to object context and than do SaveChanges() as some sort of transaction commit?

Wile answered 26/10, 2012 at 7:14 Comment(0)
S
93

I know it's kind of late answer but i found it useful to share.

Now in EF6 it's easier to achieve this by using dbContext.Database.BeginTransaction()

like this :

using (var context = new BloggingContext())
{
    using (var dbContextTransaction = context.Database.BeginTransaction())
    {
        try
        {
            // do your changes
            context.SaveChanges();

            // do another changes
            context.SaveChanges();

            dbContextTransaction.Commit();
        }
        catch (Exception ex)
        {
            //Log, handle or absorbe I don't care ^_^
        }
    }
}

for more information look at this

again it's in EF6 Onwards

Scarlatti answered 26/4, 2014 at 9:21 Comment(9)
no need to call Rollback it automatically call when exception occurHalfhardy
...because the using will dispose the transaction when it leaves scope which will roll it back if Commit hasn't been successfully called on it yet.Butterfish
Here is the proof: #22486989Cemetery
@TrilokChandra Sorry I just noticed your comment.You're right guys, I Updated my answer according to it.Scarlatti
@DFTR That's a pretty general comment - can you be more specific as to what doesn't work with Azure?Ephebe
Support for explicit entity transactions does not work if you're connecting to an azure database context. It throws an unsupported exception.Lory
I’d recommend avoiding this pattern if your logic doesn’t require it. Doing steps between transactions will result in the server holding locks on objects unnecessarily. The effect of this is reduced by only having a single SaveChanges() call per unit of work.Sassaby
is it neccessary to dispose context when rolling back transaction ?Deathless
@Lory Sql Azure does not support user initiated transactions. There is however a simple workaround which is to suspend the SqlAzureExecutionStrategy. You can read more about this limitation here: learn.microsoft.com/en-us/ef/ef6/fundamentals/…Loverly
G
9

It is a bad practice to call SaveChanges multiple times (Without a transaction scope) when the related entities should be persisted in a single transaction. What you have created is a leaky abstraction. Create a separate Unit of Work class or use the ObjectContext/DbContext itself.

Georgiannageorgianne answered 26/10, 2012 at 7:55 Comment(4)
Or... use TransactionScope.Mausoleum
Sometimes, when working with legacy databases, you just have no other options that to call SaveChanges multiple times. For example, your database has a stored procedure or a trigger that generates a very specifically formatted identifier and you need to get that identifier back in the middle of unit-of-work, so you can assign the identifier to some other entity. So, you need to call SaveChanges first to persist the changes to the database and get that identifier, and then again to save the final entities.Shep
@Shep Good point. However, bad legacy technology should not drive bad architectural choices. In the example above, the database will be left in an inconsistent state if the second save crashes or the server reboots right in the middle of the save. The application must be smart enough to leave the database clean and consistent at all times.Loverly
@Loverly Of course, we are using explicit transactions around all the SaveChanges calls to avoid inconsistencies as much as possible. Unfortunately, I live in a country where budget often wins over quality, especially in large government projects. The customer does not accept the idea of improving the legacy datastructures because it might require redesigning existing integrations with other systems which would lead to unplanned expenses. Our application has to work with the data that's already there with as few changes as possible.Shep
B
3

I would strongly advise against calling SaveChanges() in each method. Using the repository pattern and unit of work is the better way forward. Unit of work, allows you to be more efficient with your db calls and also helps you against polluting your db if some data is not valid (e.g. user details is ok, but address fails).

Here's a good tutorial to help you.

http://www.asp.net/mvc/tutorials/getting-started-with-ef-using-mvc/implementing-the-repository-and-unit-of-work-patterns-in-an-asp-net-mvc-application

Bluegrass answered 26/10, 2012 at 11:4 Comment(2)
I'm familiar with UnitOfWork pattern but I find it rather obsolete considering DbContext is UnitOfWork by itself (hover the mouse over it and read description). Implementing UnitOfWork is making abstraction over an existing abstraction of same purpose which just makes things more complicated, not better. Also my model validation is done on controller level, not on database commits, so data is ensured to be valid prior to doing Add/Update. My worries were mostly about performance hit, i.e. is it a difference sending all as one transaction or sending several separate small transacations.Northeastwards
Yes DbContext is a unit of work and is disposable. But if you want to minimize database calls then you will need to implement a unit of work approach. Your IUnitOfWork will have a SaveChanges method, and your UnitOfWork will inherit IUnitOfWork and IDisposableBluegrass
C
3

This is another approach to handle multiple context.SaveChanges() using UnitOfWork that I currently use.

We will hold all context.SaveChanges() method until this last one is called.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore;

namespace DataAccess
{
    public class UnitOfWork : IUnitOfWork
    {
        private readonly Context context;
        private readonly Dictionary<Type, object> repositories = new Dictionary<Type, object>();

        private int beginChangeCount;
        private bool selfManagedTransaction = true;

        public UnitOfWork(Context context)
        {
            this.context = context;
        }     

        //Use generic repo or init the instance of your repos here
        public IGenericRepository<TEntity> GetRepository<TEntity>() where TEntity : BaseEntityModel
        {
            if (repositories.Keys.Contains(typeof(TEntity)))
                return repositories[typeof(TEntity)] as IGenericRepository<TEntity>;

            var repository = new Repository<TEntity>(context);
            repositories.Add(typeof(TEntity), repository);

            return repository;
        }

        public void SaveChanges()
        {           
            if (selfManagedTransaction)
            {
                CommitChanges();
            }
        }

        public void BeginChanges()
        {
            selfManagedTransaction = false;
            Interlocked.Increment(ref beginChangeCount);
        }

        public void CommitChanges()
        {
            if (Interlocked.Decrement(ref beginChangeCount) > 0)
            {
                return;
            }

            beginChangeCount = 0;
            context.SaveChanges();
            selfManagedTransaction = true;
        }
    }
}

Sample using.

Find my comment in the code below

using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Identity;

namespace BusinessServices.Domain
{
    public class AService : BaseBusinessService, IAService
    {
        private readonly IBService BService;
        private readonly ICService CService;
        private readonly IUnitOfWork uow;

        public AService (IBService BService, ICService CService, IUnitOfWork uow)
        {
            this.BService = BService;
            this.CService = CService;
            this.uow = uow;
        }

        public void DoSomeThingComplicated()
        {
            uow.BeginChanges();

            //Create object B - already have uow.SaveChanges() inside
            //still not save to database yet
            BService.CreateB();

            //Create object C  - already have uow.SaveChanges() inside
            //still not save to databse yet
            CService.CreateC();

            //if there are no exceptions, all data will be saved in database
            //else nothing in database
            uow.CommitChanges();

        }
    }
}
Countermine answered 27/7, 2017 at 10:21 Comment(0)
P
0

A new modern approach as articulated here is adviced in such scenarios.

If you're familiar with the TransactionScope class, then you already know how to use a DbContextScope. They're very similar in essence - the only difference is that DbContextScope creates and manages DbContext instances instead of database transactions. But just like TransactionScope, DbContextScope is ambient, can be nested, can have its nesting behaviour disabled and works fine with async execution flows.

public void MarkUserAsPremium(Guid userId)  
{
    using (var dbContextScope = _dbContextScopeFactory.Create())
    {
        var user = _userRepository.Get(userId);
        user.IsPremiumUser = true;
        dbContextScope.SaveChanges();
    }
}

Within a DbContextScope, you can access the DbContext instances that the scope manages in two ways. You can get them via the DbContextScope.DbContexts property like this:

public void SomeServiceMethod(Guid userId)  
{
    using (var dbContextScope = _dbContextScopeFactory.Create())
    {
        var user = dbContextScope.DbContexts.Get<MyDbContext>.Set<User>.Find(userId);
        [...]
        dbContextScope.SaveChanges();
    }
}

But that's of course only available in the method that created the DbContextScope. If you need to access the ambient DbContext instances anywhere else (e.g. in a repository class), you can just take a dependency on IAmbientDbContextLocator, which you would use like this:

public class UserRepository : IUserRepository  
{
    private readonly IAmbientDbContextLocator _contextLocator;

    public UserRepository(IAmbientDbContextLocator contextLocator)
    {
        if (contextLocator == null) throw new ArgumentNullException("contextLocator");
        _contextLocator = contextLocator;
    }

    public User Get(Guid userId)
    {
        return _contextLocator.Get<MyDbContext>.Set<User>().Find(userId);
    }
}
Paleoasiatic answered 8/4, 2016 at 16:1 Comment(2)
Why you check (contextLocator == null) in constructor ? Are you kiddin' ?Hermanhermann
what scope IAmbientDbContextLocator should be: Singleton or Transient? in .NET CoreTellurate
E
0

As per the official tutorial of Entity Framework, it is possible with the BeginTransaction property of database context as:

    using (var context = new SchoolContext())
{
    context.Database.Log = Console.Write;

    using (DbContextTransaction transaction = context.Database.BeginTransaction())
    {
        try
        {
            var standard = context.Standards.Add(new Standard() { StandardName = "1st Grade" });

            context.Students.Add(new Student()
            {
                FirstName = "Rama2",
                StandardId = standard.StandardId
            });
            context.SaveChanges();

            context.Courses.Add(new Course() { CourseName = "Computer Science" });
            context.SaveChanges();

            transaction.Commit();
        }
        catch (Exception ex)
        {
            transaction.Rollback();
            Console.WriteLine("Error occurred.");
        }
    }
}
Expectoration answered 10/1 at 16:6 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.