Problem with PerRequestUnitOfWorkScope

I am using 2 different repositories in the same mvc app (for 2 different databases) using different units of work.

Each repository is using a PerRequestUnitOfWorkScope e.g. PerRequestUnitOfWorkScope<DocsUnitOfWork> and PerRequestUnitOfWorkScope<CmsUnitOfWork>.  This is causing intermitant errors as one scope will occassionaly end up with the wrong value for Current.

Looking at the (old) source, the problem seems to be in the implimentation of current:

  public sealed class PerRequestUnitOfWorkScope<TUnitOfWork> : UnitOfWorkScopeBase<TUnitOfWork>
    where TUnitOfWork : class, IUnitOfWork, new()
  {

    public const string ItemsKey = "__LightSpeedUnitOfWork__";

    public override TUnitOfWork Current
    {
      get
      {
        var current = (TUnitOfWork)HttpContext.Current.Items[ItemsKey];

        if (current == null)
        {
          HttpContext.Current.Items[ItemsKey] = current = LightSpeedContext.CreateUnitOfWork();
        }

        return current;
      }
    }

...

}

The problem is that ItemsKey is constant so if you have 2 different uows in play, occassionally the wrong one will end up being accessed.

To fix it, ItemsKey should be dependant on the uow type used to create the PerRequestUnitOfWorkScope.

Pretty please.

Sean

Yes - this is the intended behavior. The PerRequestUnitOfWorkScope's semantics are 'unit of work per request,' so it is expecting to provide the single unit of work associated with the current request and accordingly will blow up if that isnt of the same type as you previously dealt with. That said, a few people have hit this problem before and have shared some examples back on how to cater for multiple typed UnitOfWork instances.

Here is an example of a custom implementation which covers this: http://www.mindscapehq.com/forums/Post.aspx?ThreadID=2037&PostID=5449 - let us know if you need any additional help here.

Jeremy

Jeremy, I am disappoint.

I am disappoint that I spent hours tracking down a bug in my code that turns out to be a bug in your code.  And I am disappoint that you are giving me the "It's a feature not a bug" line. Please forgive me if I lapse into sarcasm occassionaly.

Serriously?  Your offical position is that using multiple unit of work types should throw a completely unhelpful exception?  And that it should be inconsistant with PerThreadUnitOfWorkScope which works the way you would expect?

At the moment if you have PerRequestUnitOfWorkScope<AUnitOfWork> & PerRequestUnitOfWorkScope<BUnitOfWork> then you get an "invalid cast" exception UNLESS AUnitOfWork decends from BUnitOfWork or vice versa AND you use them in the correct order. How is that helpful to anyone?

In reality, you are limited to using just one unit of work type or getting inexplicable error messages.

I suggest that you change the semantics 'unit of work per type per request,'.  This would work exactly the same one as currently when using 1 type (the only scenario that works atm) and would also let in work with multiple unit of work types.  I fail to see the downside.

It would also bring PerRequestUnitOfWorkScope into line with PerThreadUnitOfWorkScope which is 'unit of work per type per thread'.

If you can't do that, then at least change Current to do a type check before returning, and then throw a more usefull error such as 'PerRequestUnitOfWorkScope doesn't support multiple unit of work types so write your own'.  That will at least save the next poor sap.

Unhappy Sean

Hi Sean,

Thanks for the honest feedback, it is always appreciated. Its unfortunate to hear that it cost you some time but as previously mentioned that is the way it was designed. We will have a think about the improving this going forward, in the meantime I will add in a better exception since that is a pretty reasonable suggestion.

Jeremy

Thanks for at least making the error change.  However I think designed to fail on multiple types is a pretty poor design.  For reference I have included the fixed class for the next person to run into this problem.  I have highlighed the extensive changes required.

    public class PerRequestPerTypeUnitOfWorkScope<TUnitOfWork> : UnitOfWorkScopeBase<TUnitOfWork>
      where TUnitOfWork : class, IUnitOfWork, new()
    {
        // public const string ItemsKey = "__FixedLightSpeedUnitOfWork__";
        public static string ItemsKey = "__FixedLightSpeedUnitOfWork__" + typeof(TUnitOfWork).ToString();


        public PerRequestPerTypeUnitOfWorkScope(LightSpeedContext<TUnitOfWork> lightSpeedContext)
            : base(lightSpeedContext)
        {
        }

        public override TUnitOfWork Current
        {
            get
            {
                var current = (TUnitOfWork)HttpContext.Current.Items[ItemsKey];

                if (current == null)
                {
                    HttpContext.Current.Items[ItemsKey] = current = LightSpeedContext.CreateUnitOfWork();
                }

                return current;
            }
        }

        public override bool HasCurrent
        {
            get { return HttpContext.Current.Items.Contains(ItemsKey); }
        }
    }

Hi Sean,

Just an update to let you know we will be putting in an updated version of this class for 4.0 which supports multiple instances of the UnitOfWork each stored in its own ItemsKey.

The reason we dont want to change this for 3.x is due to the change in behavior around disposal of the UnitOfWork instances and the fact that some people make use of the ItemsKey instance currently outside of the PerRequestUnitOfWorkScope. With the new version (which as you highlighted doesnt require much in the way of code change to the class itself) you will need to explicitely make sure each TUnitOfWork is correctly disposed, so we will be adding this in to the release docs for 4.0 as a breaking change.

Thanks again for your feedback :)

Jeremy

Good to hear.  Thanks

I've just run into this issue. Glad to see it is fixed!

Are you guys making the same change to PerThreadUnitOfWorkScope to support multiple unit of work types?

Actually the change we made for the PerRequest scope isnt actually required for PerThreadUnitOfWorkScope, so you can already have multiple unit of work types.

Jeremy