13

Background: I have WCF service with SimpleInjector as IoC which creates instance of DbContext per WCF request.

Backend itself is CQRS. CommandHandlers have a lot of decorators (validation, authorization, logging, some common rules for different handler groups etc) and one of them is Transaction Decorator:

public class TransactionCommandHandlerDecorator<TCommand> : ICommandHandler<TCommand> 
    where TCommand : ICommand
{
    private readonly ICommandHandler<TCommand> _handler;
    private readonly IMyDbContext _context;
    private readonly IPrincipal _principal;

    public TransactionCommandHandlerDecorator(ICommandHandler<TCommand> handler,
        IMyDbContext context, IPrincipal principal)
    {
        _handler = handler;
        _context = context;
        _principal = principal;
    }

    void ICommandHandler<TCommand>.Handle(TCommand command)
    {
        using (var transaction = _context.Database.BeginTransaction())
        {
            try
            {
                var user = _context.User.Single(x => x.LoginName == _principal.Identity.Name);
                _handler.Handle(command);
                _context.SaveChangesWithinExplicitTransaction(user);
                transaction.Commit();
            }
            catch (Exception ex)
            {
                transaction.Rollback();
                throw;
            }
        }
    }
}

Problem occurs when any command tries to chain execute another command within the same WCF request. I got an expected exception at this line:

using (var transaction = _context.Database.BeginTransaction())

because my DbContext instance already has a transaction.

Is there any way to check current transaction existence?

Szer
  • 3,426
  • 3
  • 16
  • 36
  • 2
    You are missing a [holistic abstraction](http://qujck.com/commands-and-queries-are-holistic-abstractions/) – qujck Jan 12 '16 at 18:30
  • @qujck after reading this article I don't understand what the difference between `IQuery` and `IDataQuery`. Both of it returns data. Why we need second interface for that? Is there some examples? – Szer Jan 13 '16 at 09:53
  • They do both return data but these different abstractions are only important when it comes to decorating your code. You need one abstraction that owns the entire operation, an abstraction that can be wrapped with something like a `TransactionDecorator`. You have other, lower-level abstractions that can be decorated with cross-cutting concerns such as `AuthoriseDecorator` or `LoggingDecorator` that are not concerned with the atomic operation. – qujck Jan 13 '16 at 11:12
  • 1
    @qujck finally got it. Great solution, I'll definitely do it. Thanks! – Szer Jan 13 '16 at 11:24

2 Answers2

29

I think you're looking for the CurrentTransaction property of the DbContext:

var transaction = db.Database.CurrentTransaction;

Then you can do a check like this:

using(var transaction = db.Database.CurrentTransaction ?? db.Database.BeginTransaction())
{
   ...
}

However I'm not sure how you can know when to commit the transaction if it's being used by concurrent methods.

Alexander Derck
  • 13,818
  • 5
  • 54
  • 76
  • I greatly upvote your answer because for some reason I had 6.0.0 version of EF which doesn't have this property. After Update-Package EF goes to 6.1.3 and property is there. Many thanks! – Szer Jan 13 '16 at 09:48
  • @Szer yes it hasn't been in EF for long :) – Alexander Derck Jan 13 '16 at 09:50
  • I realize that your answer is better straightforward 'answer' to my question, but I should admit that real answer is 'bad design' which is what @Ric.Net said. – Szer Jan 13 '16 at 09:57
7

Instead of using the transaction from the DbContext of Entity Framework you could or maybe should use the TransactionScope class which creates an ambient transaction scope and manages transactions of all connections made to the (SQL) database under the covers.

It even would put a direct SqlCommand in the same transaction if you would use the exact (case-sensitive) connectionstring for the SqlCommand. Messages writen to the MessageQueue are also encapsulated in the same transaction

It even could manage connections to different databases at the same time. It uses the DTC windows service for this. Beware that this is a pain to configure if needed. Normally, with a single DB connection (or multiple connections to the same DB) you won't need the DTC.

The TransactionScopeCommandHandlerDecorator implementation is trivial:

public class TransactionScopeCommandHandlerDecorator<TCommand> 
        : ICommandHandler<TCommand>
{
    private readonly ICommandHandler<TCommand> decoratee;

    public TransactionScopeCommandHandlerDecorator(ICommandHandler<TCommand> decoratee)
    {
        this.decoratee = decoratee;
    }

    public void Handle(TCommand command)
    {
        using (var scope = new TransactionScope())
        {
            this.decoratee.Handle(command);

            scope.Complete();
        }
    }
}

But: As qujck already mentioned in the comments, you are missing the concept of ICommandHandler as an atomic operation. One commandhandler should never reference another commandhandler. Not only is this bad for transactions, but also consider this:

Imagine the application grows and you would refactor some of your commandhandlers to a background thread, which will run in some windows service. In this windows service a PerWcfOperation lifestyle is not available. You would need a LifeTimeScope lifestyle for you commandhandlers now. Because your design allows it, which is great by the way!, you would typicaly wrap your commandhandlers in a LifetimeScopeCommandHandler decorator to start the LifetimeScope. In your current design where a single commandhandler references other commandhandlers you will run into a problem, because every commandhandler will be created in its own scope a thus gets an other DbContext injected than the other commandhandlers!

So you need to do some redesign and make your commandhandlers holistic abstractions and create a lower level abstraction for doing the DbContext operations.

SeeR
  • 2,158
  • 1
  • 20
  • 35
Ric .Net
  • 5,540
  • 1
  • 20
  • 39
  • This is the right approach, +1. `It even would put a direct SqlCommand in the same transaction if you would use the exact (case-sensitive) connectionstring for the SqlCommand` this is not guaranteed. Careful, this tends to fail under load. – usr Jan 12 '16 at 22:33
  • @usr, are you sure? I guess you are. Can you point to some documentation about this? In some scenarios we rely heavily on this... So many thanks for this note! – Ric .Net Jan 13 '16 at 06:55
  • @Ric.Net many thanks! I'll consider redesign of chain commands. But I should mention it is not easy. In my case when client send command AddEntityA, server should add entity A to database and after that it should also add entity B. AddEntityB command has a lot of rules and checks and own decorator so if entity B cannot be created entity A also wont be added to database. Thats why I execute command inside command – Szer Jan 13 '16 at 09:43
  • @Ric.Net if you open multiple connections in the same TS you often get lucky because the conn pool might take the same physical connection to SQL Server. Then, you don't escalate to MSDTC. But under load the pool cannot necessarily oblige and might return a fresh connection or a different one. Then you randomly escalate (under load when the app has to work most importantly). This is the devil. Such nasty design. – usr Jan 13 '16 at 12:55
  • @usr Nasty design indeed! brrrrrr... Thank you very much for this info. I'll keep this in mind! – Ric .Net Jan 13 '16 at 13:17
  • @Szer I understand, you should really consider the design of holistic abstractions or switch to using the [Domain Event Pattern](https://lostechies.com/jimmybogard/2014/05/13/a-better-domain-events-pattern/). A Simple Injector related answer can be found [here](http://stackoverflow.com/questions/30625363/implementing-domain-event-handler-pattern-in-c-sharp-with-simple-injector) – Ric .Net Jan 13 '16 at 13:49
  • @usr I've been using these non-escalated DTC transactions as Ric describes them for years and newer experienced any problems with them. So unless you can point at some resource for proof, I would see this is bullocks or at least some bug that has been patched years ago. – Steven Jan 13 '16 at 14:16
  • @Steven I have investigated/tested this in detail and it's also confirmed from official sources. Since nobody has ever written this up comprehensively it's hard to quickly find something to link to. I believe I have answered this a few times and explained it in detail. You can reproduce this as follows: `OpenTS(); UseNewConn(); OpenAndHoldUnrelatedConnection(); UseNewConn();`. The second UseNewConn will escalate because OpenAndHoldUnrelatedConnection burns the connection that the first UseNewConn put back into the pool. – usr Jan 13 '16 at 15:09
  • @Ric.Net, Lets say all command handlers are decorated with a lifetime scope and my command handler can not reference other command handlers. What option do I have to fire a command handler from a command handler? – janhartmann Jan 18 '16 at 11:59
  • @janhartmann You shouldn't fire a commandhandler from a commandhandler. I suggest reading the referenced blog about holistic abstractions. – Ric .Net Jan 18 '16 at 12:49
  • Thanks, still have a bit of a trouble understanding the differences between those without a concrete example. I have spoken @qujck who will make a follow-up on the blog post. – janhartmann Jan 18 '16 at 12:54
  • @Ric.Net Years ago this approach had got us so excited: the simplicity of ambient transactions was something out of this world. Until one day after some Windows update the DTC service on the application box has stopped working with the DTC service on the database box. Needless to say that we have spent hours and hours learning all sorts of obscure commands to make DTCs ping each other and firewall ports, we ended up never really being able to find a root cause, since all troubleshooting steps have passed. It was a shock of realization that simplicity of a black box comes at a sudden cost. – timmi4sa Jun 03 '20 at 04:06
  • @timmi4sa My comment in the answer that it is a pain to configure DTC was not because it was nice to do ;-). However with single DB connections, which is at least most of my projects it is simple and the DTC won't ever be used. – Ric .Net Jun 03 '20 at 10:58