2
\$\begingroup\$

I am using ASP.NET Core 3.1 and have been assigned a task to verify the count of changes done using SaveChanges(). It is expected that the developer should know how many records will be changed before-hand when SaveChanges() will be called.

To implement it, I have created an extension method for DbContext called SaveChangesAndVerify(int expectedChangeCount) where I am using transaction and equating this parameter with the return value of SaveChanges(). If the values match, the transaction is committed and if it doesn't match, the transaction is rolled back.

Please check the code below and let me know if it would work and if there are any considerations that I need to make. Also, is there a better way to do this?

public static class DbContextExtensions
{
    public static int SaveChangesAndVerify(this DbContext context, int expectedChangeCount)
    {
        context.Database.BeginTransaction();
        var actualChangeCount = context.SaveChanges();
        if (actualChangeCount == expectedChangeCount)
        {
            context.Database.CommitTransaction();
            return actualChangeCount;
        }
        else
        {
            context.Database.RollbackTransaction();
            throw new DbUpdateException($"Expected count {expectedChangeCount} did not match actual count {actualChangeCount} while saving the changes.");
        }
    }

    public static async Task<int> SaveChangesAndVerifyAsync(this DbContext context, int expectedChangeCount, CancellationToken cancellationToken = default)
    {
        await context.Database.BeginTransactionAsync();
        var actualChangeCount = await context.SaveChangesAsync();
        if(actualChangeCount == expectedChangeCount)
        {
            context.Database.CommitTransaction();
            return actualChangeCount;
        }
        else
        {
            context.Database.RollbackTransaction();
            throw new DbUpdateException($"Expected count {expectedChangeCount} did not match actual count {actualChangeCount} while saving the changes.");
        }
    }
}

A sample usage would be like context.SaveChangesAndVerify(1) where a developer is expecting only 1 record to update.

\$\endgroup\$
2
\$\begingroup\$

I can't think of another way of doing this - only the database knows how many rows have been affected. That said, your code has some problems.

You need to dispose of your transaction when you're done with it. This has 2 additional benefits:

  1. You don't need to rollback manually
  2. You don't need to worry about exceptions in SaveChanges (you haven't handled that at all at the moment).

Let's look at how that changes things:

public static int SaveChangesAndVerify(
    this DbContext context,
    int expectedChangeCount)
{
    using (var transaction = context.Database.BeginTransaction())
    {
        var actualChangeCount = context.SaveChanges();
        if (actualChangeCount == expectedChangeCount)
        {
            transaction.Commit();
            return actualChangeCount;
        }
        throw new DbUpdateException($"Expected count {expectedChangeCount} did not match actual count {actualChangeCount} while saving the changes.");
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Thank you for reviewing the code. \$\endgroup\$ Sep 17 '20 at 10:26
1
\$\begingroup\$

No need to do this at all.

For one, Entity Framework keeps track of expected changes itself, probably better than you can do it. EF knows how many rows are expected to be changed by a SaveChanges() call. If there is a difference it will throw a DbUpdateConcurrencyException exception and roll back the transaction. For example when a record is deleted while it's not present in the database anymore:

DbUpdateConcurrencyException: Database operation expected to affect 1 row(s) but actually affected 0 row(s). Data may have been modified or deleted since entities were loaded. See http://go.microsoft.com/fwlink/?LinkId=527962 for information on understanding and handling optimistic concurrency exceptions.

As you see, EF assumes that exceptions like this are caused by concurrency, but you can easily raise one by committing an entity with a non-existing primary key value.

Which means, secondly, that you don't need to start and commit a transaction yourself. SavaChanges manages its own transaction and won't commit anything if there's an exception.

So here's the reviewed code:

Yep, you can remove the methods completely. Call SaveChanges(Async) where you need it and handle exceptions as usual.

As a matter of fact, the line throw new DbUpdateException should never be reached: if there is a difference, SaveChanges() will already have thrown and if the line is reached you probably made a mistake in determining expectedChangeCount.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.