1

I am in the process of refactoring and improving a codebase. One of the major missing features is transactional safety and certain errors arising from each repository having its own DbContext.

The current setup is as follows:

  • The BLL (FooManager) connects to as many repositories (FooRepository, BarRepository) as it wants to.
  • Each repository has its own DbContext.
  • Ninject is used for constructor injection.

What I want to create is as follows:

  • The BLL (FooManager) has an injected UnitOfWork
  • The UnitOfWork class holds a reference to all repositories (Note: I am aware of the possibility of having the repositories register themselves to the unit of work, more on that later.)
  • A UnitOfWork's repositories should all use the same DbContext.
  • We must ensure that the DbContext is disposed of even if the FooManager is kept alive after its method was called.

But I'm encountering a few issues here. I initially wanted to post these as separate questions but I suspect that any relevant answer needs to take all these considerations into account. I've found individual answer to each question (which I will mention when relevant) but I haven't found a solution that ticks all the boxes.


How to ensure that the DbContext is shared between repositories?

Currently, the DbContext is injected into the repository constructor. I see other options, but I can't fully solve any of them.

  • I could set NInject to treat the DbContext as a singleton. However, then two units of work will also share the same DbContext.
  • I could inject the DbContext into the UnitOfWork constructor, but I'm unsure how to then pass the context to the underlying repositories.
    • Setting a public property feels dirty.
    • Using a constructor would mean that the UOW constructs the repository instead of NInject, which feels like an antipattern.
    • Having the repositories each point to the context in the UOW requires the UOW injecting itself into the repositories. The question remains the same, regardless of whether I choose to pass the context or the UOW itself to the underlying repositories.

How to ensure that contexts get closed?

I want to stay away from using NInject's .InRequestScope() feature, because the company has in the past shown to reuse its libraries in different forms. In the past, we've encountered issues because a BLL never disposed of its context.

Initially, it was developed for a REST web API, so the context was disposed of at the end of the request. But when the BLL was later reused as part of a Windows service, the contexts were never disposed of because the "request" never finished (since the service was active at all times). It took us many weeks of work to trace bugs that were caused by this (we kept seeing ghost data enter the database).

I want to avoid it in the new project, so the proposed solution needs to work both for a web service and a windows service. I am already relying on factory injection here, provided by a NInject extension:

public class FooManager
{
    private Func<UnitOfWork> createUOW;

    public FooManager(Func<UnitOfWork> uowFactory)
    {
       createUOW = uowFactory;
    }

    public object PublicMethod1()
    {
        using (var uow = createUOW())
        {
            // ...
        }
    }

    public object PublicMethod2()
    {
        using (var uow = createUOW())
        {
            // ...
        }
    }
}

The goal is to ensure that each "call" to the business logic gets its own unique UOW (and therefore unique context). This means that in a Windows service, where the FooManager may be kept alive for an extended time, that the underlying context will still be refreshed every time the FooManager is called upon.


How to connect the UOW and the repositories?

My first attempt was to simply inject every repository via the UOW constructor. While that is technically possible, I'm apprehensive of having to bloat the constructor to using more than 30 different repositories. It's going to be hell to maintain in the future.

I can't just inject the IKernel itself into the UOW. The DI and BLL (and DAL) are separate projects, and the DI already depends on the BLL (and DAL) so I can't have the BLL (and DAL) depend on the DI project as well as that would create a circular dependency.

I've come across solutions to have the repository register itself with the unit of work, but I'm unsure if this actually improves things in my case.

  • While I do have a Repository<T> that all repositories inherit from, almost all repositories heavily rely on an additional FooRepository which considerably extends the available method. The FooManager can't just use Repository<Foo>, it should be using FooRepository.
  • If I understand it correctly, this would also still require the BLL to instantiate the repositories it needs. But I'm specifically trying to prevent this behavior. I want the BLL to only work with a UOW directly.
    • I'm aware that the BLL will still be aware of which repositories of the UOW it wishes to use, but I don't want the BLL to be responsible for instantiating the repositories.

One last mention I want to make is that complexity matters. I've fought long and hard for the opportunity to refactor the codebase, but management is liable to cancel the whole change if it takes too long or becomes too complex for the current developers to understand/properly work with. This means that I probably won't be able to implement a perfect design.

The main requirements are:

  • Transactional behavior
  • Ensuring that a UOW uses a single db context in all of its repositories.
  • Ensuring that every BLL-method uses its own UOW/context and correctly disposes of it at the end of the method.
  • For all other concerns, the needed change from the current setup should be minimized.

While I have found some solutions to the problems listed above, I cannot find one that aligns all needed requirements.

Am I missing something here, or am I trying to go about it the wrong way?

  • This deserves a better answer than I've got time to write at the moment. However when I did this I had an ISessionManager which provided the current UoW on request and was a dependency of the repositories. The SessionManager implementation could be stored in memory or http context depending on the application. – Liath Aug 24 '18 at 9:56
  • 1
    @Liath: It sounds interesting, but don't think I'm fully getting what you mean. An extended answer would be welcome if and when you could find the time (or if someone else wishes to elaborate) – Flater Aug 24 '18 at 9:57
  • If I was doing this i would have each UOW have it's own instance of each required repository, scoped to the UOW life cycle just as the DbContext is. Then when the repositories are created you just inject the DbContext into the constructor. – user1450877 Aug 24 '18 at 11:27
  • @user1450877: Each UOW can have its own repository objects, I have no issue with that. It's actually a requirement, since all the repositories of a single UOW should share the same context, and the repositories of another UOW should share a different context. That means that two UOWs cannot logically use the same repository (as they need to have different contexts) "you just inject the DbContext into the constructor" That is exactly what the question is about, I'm asking how to correctly achieve this. – Flater Aug 24 '18 at 12:30
  • @Flater scope the DBContext to the lifecycle of your unit of work planetgeek.ch/2010/12/08/… – user1450877 Aug 24 '18 at 15:38
0

The credit should go to @user1450877 but he's not actually posting an answer. The suggestion to use a named scope indeed solved the issue.

Reference link, I used the first of the three options.

I installed the NInject.Extensions.NamedScope plugin, and then created a string key to act as the name of the scope:

private const string UnitOfWorkScope = "UNITOFWORKSCOPE";

And then changed my bindings to use the correct scope:

kernel.Bind<IUnitOfWork>()
      .To<UnitOfWork>()
      .DefinesNamedScope(UnitOfWorkScope);

kernel.Bind<IFooRepository>()
      .To<FooRepository>();

kernel.Bind<MyDbContext>()
      .ToSelf()
      .InNamedScope(UnitOfWorkScope);

This achieves exactly what I wanted, a DbContext that behaves like a singleton in scope of every IUnitOfWork that is instantiated.

-5

Your repository should be at database level not table level.

ie instead of FooRepo.Get() and BarRepo.Get() you should have DbRepo.GetFoo() and DbRepo.GetBar()

This allows you to deal with the most common UoW problems where you are working on a single database with FK connected tables.

The repository can handle the UoW logic and expose an appropriate method

If you have cross database UoW then you have to see whether your language of choice supports cross DB TransactionScopes or if you can implement them yourself. Opening a transaction on each server and checking success of actions on the other before closing your own.

Moving the UoW logic out of the Business Layer and into the Data Layer is the correct approach. "Undo those writes because this thing failed" is a Data Layer concern, not business logic.

So on a practical note:

  1. Transactional behavior
  2. Ensuring that a UOW uses a single db context in all of its repositories.
  3. Ensuring that every BLL-method uses its own UOW/context and correctly disposes of it at the end of the method.
  4. For all other concerns, the needed change from the current setup should be minimized.

Simply moving current Business method to a DBRepo in the Data Layer which hides your existing TableRepos from the app allows you to

  1. use a transaction
  2. instantiate and keep as private variables your TableRepo classes using the same database context
  3. ensures that the Method controls its own UoW/transaction and correctly disposes of it
  4. has involved only copy/paste and public -> private/internal
  • 1
    We are talking about 30+ repositories each with their own custom methods (on average, I'd say 10 custom methods per repository, and 15 methods shared by all repositories). I'm not going to be putting those in a single monolithic class. You're suggesting to effectively use the UOW as the repository. – Flater Aug 24 '18 at 10:39
  • keep the classes, just make them internal and expose through a single interface if you like – Ewan Aug 24 '18 at 10:40
  • 1
    That still requires around 300 copy/pasted "pass-through" methods to be implemented in the uow, which still creates a monolithic class. Any future change to a repository can be doubled (since the UOW must be updated as well), and it provides no additional benefits Whether I call myUOW.FooRepository.Get() or myUOW.GetFoos() is irrelevant in regards to the question. – Flater Aug 24 '18 at 10:44
  • Furthermore, if I keep the (now internal) classes as per your suggetion, then you haven't actually answered the question on how to pass the db context to these (internal) classes. – Flater Aug 24 '18 at 10:46
  • 1
    (1) Currently, each repository has its own db context. It cannot simply be registered as a singleton as the db context does need to vary between separate BLL methods. (2) As I am using a single db context with SaveChanges(), this means there will already be transational safety on the context provided that the same context is reused. The Unit Of Work effectively serves as the scope for the uniqueness of the db context. (3) The UOW will further help prevent BLL constructor bloat when many repositories are called upon, as is now the case in e.g. an ImportManager that affects many tables. – Flater Aug 24 '18 at 11:03

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.