Magic Scope

[carbonrobot]

It's difficult to pin down just what this project is attempting to solve. You seem to have taken the approach of managing the lifetime of the context outside of the business transaction, and in doing so introduced some magic on top of what should be a simple unit of work.

Take the example listed as a basic service call:

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

In the above example, its unclear what context the _userRepository will be ultimately using. It could be argued that it doesn't matter, because inject should be handling that properly, and I could even agree with that myself. Where it gets sticky is the call to dbContextScope.SaveChanges(). The repository is using a context that is hidden from the caller, but then the caller commits the transaction through a different interface. There is no consistency in that pattern and consists of behind the scenes magic. It's unclear to the caller what affects will occur when SaveChanges() are called.

How many contexts will be affected? How many changes will persist? How does the caller gracefully handle exceptions?

These problems persist when making a direct call through DbContexts as well.

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

This example seems to confound the situation even further. Here we explicitly get the context we want, but instead of calling SaveChanges on that instance, we call it on the scope (which contains all DbContexts. Just what side effects could occur is not obvious to the caller.

This seems to be counter to your statement

Your services must be in control of the business transaction boundary

Although you make an exception for handling the DBContext lifetime, which I agree with completely, I argue that the service no longer controls the transaction boundary. You could potentially make several unrelated business transactions under the same context without the safety of a clear unit of work.

In your blog, you mention this consequence

A service method must use the same DbContext instance throughout the duration of a business transaction. This is so that all the changes made to your persistent model are tracked and either committed to the underlying data store or rolled back in an atomic manner.

Although its perfectly OK to handle the lifetime of the DbContext outside of a single transaction, your code is calling SaveChanges on multiple DBContext instances which could potentially commit unwanted transactions.

Take the following example:

_service.TransferMoney(2000, account);
_service.WithdrawMoney(1200);

Let's say within the TransferMoney transaction, the code returns prior to calling SaveChanges() for whatever contrived reason we like, possibly some business rule or exception.

The client code then calls WithdrawMoney, and locally the DbContext believes it has enough funds to cover the transaction, so it succeeds until the call to SaveChanges() occurs.

At this point, one of several things could occur:

1. The database throws an exception, at which point bubbles up the stack and reports that WithdrawMoney has thrown an exception. Since the problem didn't actually occur in this method, we are now stuck with an akward way to handle this scenario as the proper course of action can not be determined.
2. The database does not throw an exception, and allows the overdraft of funds, leaving the account with -800 remaining.
3. Depending on the internal implementation of the two methods, it may actually persist both transactions, even if it should not have persisted the first one.

Obviously, the correct approach would be to send both transactions to the same service method as follows:

var transactions = new List<Transaction>(){
    new TransferMoneyTransaction(2000, account),
    new WithdrawMoneyTransaction(1200)
};
_service.Commit(transactions);

In this instance, one would only need a single unit of work within that service method. Which brings me back to the question, "What does this project solve?"

If one needs to reuse the DbContext across multiple service methods, that means the caller is now in control of business transactions outside of the service itself, which tells me those would be better served by wrapping them into their own service method.

I mean this as constructive criticism, and I'm only looking for your thoughts on the proper use case. In your article you clearly state

There is of course no such thing as one-size-fits-all

and go on to details the pros/cons of each approach. However, you didn't list the pros/cons of your own approach.

[nolisj]

I've just recently come across this library and still trying to understand the whole thing. But given that, here's my two cents on some of your concerns based on my current understanding.

1. The Fund Transfer scenario (ie. TransferMoney and WithdrawMoney; btw, I think you meant DepositMoney and WithdrawMoney since those are what's involved in a fund transfer). In this case, based on the sample code of mehdime, the closest steps will probably be to do the ff:
   • Create the DepositMoney method (with its own SaveChanges call since I think this is a distinct operation on its own);
   • Create the WithdrawMoney method (with its own SaveChanges call since I think this is a distinct operation on its own too);
   • Create a FundTransfer method that calls the DepositMoney and WithdrawMoney methods inside. The FundTransfer method will create its own scope which becomes the parent scope of the two (2) "child" methods. This way the commit and the rollback will be consistent.

That's the way I understand it. I haven't tested it yet but theoretically it should work.

My main concern however is different. It's related to this example code:

var user = dbContextScope.DbContexts.Get.Set.Find(userId);

(note: angle brackets are being removed in the sample code above; MyDbContext and User are missing);

It is the fact that in a Service method I can access a different DbContext and therefore directly call the Find, for example, on the entity on that different context, even if my design involves a repository. Ideally, if I wrapped my data access calls in a Repository, all of my data access calls should be encapsulated in that repository class and my service class simply calls the methods in the Repository. I think the way it is implemented right now is a bit leaky.

I wonder what will happen if the DbContexts collection is hidden from the caller.

Thanks.

[ryanlangton]

I find readonly scopes to be particularly "magic" in their usage.

 public UserDto GetUser(Guid userId)
 {
    using (var dbContextScope = _dbContextScopeFactory.CreateReadOnly())
    {
        var user = _userRepository.Get(userId);
        var userDto = _mapper.Map<UserDto>(user);
        return userDto;
    }
}

Why was I required to create a scope? It's not used explicitly anywhere. This leads to fragile code where refactorings can cause run time errors.

[MarcusChamberlain]

For anyone coming across this question now, an answer the question by carbonrobot.

To transfer money, you would not have:

_service.TransferMoney(2000, account);
_service.WithdrawMoney(1200);

because as he points out, each of those two services would each call SaveChanges(), creating a request that is not atomic and can't be rolled back. Instead the way to handle this would be the following:

using (var dbContextScope = _dbContextScopeFactory.Create())
{
  TransferMoney(2000, account);
  WithdrawMoney(1200);
  dbContextScope.SaveChanges();
}

void TransferMoney(int amount, Account account)
{
  using (var dbContextScope = _dbContextScopeFactory.Create())
  {
    //transfer money...

    dbContextScope.SaveChanges();
  }
}

void WithdrawMoney(int amount)
{
  using (var dbContextScope = _dbContextScopeFactory.Create())
  {
    //withdraw money...

    dbContextScope.SaveChanges();
  }
}

TransferMoney and Withdraw money still include their own SaveChanges() calls (because they can still be called on their own), but the SaveChanges method is ignored because DbContextScope detects an ambient DbContext that has already been created! This means you can avoid situations of messy and confusing DbContext scopes.

In comparison to the answer by nolisj. In that example you would have seperate method called FundTransfer with its own SaveChanges() call, but they wouldn't be able to implement TransferMoney or WithdrawMoney because they already implement their own DbContexts and call SaveChanges(). That's where without DbContextScope it starts to get messy. (Edit: I misread the post by nolisj, his method is correct)

Likewise, in carbonrobots final example:

var transactions = new List<Transaction>(){
    new TransferMoneyTransaction(2000, account),
    new WithdrawMoneyTransaction(1200)
};
_service.Commit(transactions);

Would only work if TransferMoneyTransaction and WithdrawMoneyTransaction are designed not to be used on their own and do not call Commit/SaveChanges themselves, which could lead to a messy codebase when developers still need to use those actions on their own.

You can also see the power of this in the CreateListOfUsers method in the sample project where a service can either call CreateUser() on its own and the user will be persisted back to the database, or they can create a new ambient DbContextScope and then call CreateUser several times in a loop - it's the exact same method being called, but now because the ambient scope exists CreateUser() wont actually save the changes back to the DB - it's only once the loop is completed that the service then calls SaveChanges() to persist the updates in a single request for all the new users.

Hope that helps!

[nolisj]

Boolean0101,

Thanks for your input. If you read closely, your answer is the same as mine. Your two independent "child" functions (TransferMoney and WithdrawMoney) become contained in a "parent" (aka container) function. It's just that you did not name your parent function. In my case, the DepositMoney and WithdrawMoney are the child functions which are called in the FundTransfer function. Each child function is independent (since you can deposit or withdraw separately) and can commit their own individual operations if called individually but can also participate in the fund transfer scenario if you so desire (which means there's a DbContextScope in the FundTransfer parent function also) . I hope that clarifies what I meant.

[MarcusChamberlain]

Hi nolisj

Right you are - I misread your last bullet point as not calling SaveChanges on itself and thought you were relying on the child methods to persist changes, which I realize wasn't what you meant. I think my post still adds some details to the answer though, so I'll update it to make it more clear.