1
\$\begingroup\$

We are using Dapper as our ORM and have chosen the repository pattern for organizing our operations. This has worked very well for us but I'd like to confirm that this configuration is capable of sharing and disposing of SQL connections properly.

Startup.cs

services.AddScoped(_ => new DeploymentManagerDbConnection(
    new SqlConnection(Configuration.GetConnectionString("DeploymentManagement"))
));

services.AddScoped<IDeploymentManagerDbContext, DeploymentManagerDbContext>();
services.AddTransient<IChangeRequestRepository, ChangeRequestRepository>();
services.AddTransient<IBuildRepository, BuildRepository>();
services.AddTransient<IBatchRepository, BatchRepository>();
services.AddTransient<IUserRepository, UserRepository>();

DeploymentManagerDbConnection.cs

public DeploymentManagerDbConnection(IDbConnection dbConnection) : base(dbConnection)
{

}

DbConnection.cs

public class DbConnection : IDisposable
{
    private bool _disposed;

    public DbConnection(IDbConnection dbConnection)
    {
        Connection = dbConnection;
    }

    public IDbConnection Connection { get; }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!_disposed)
            if (disposing)
                Connection.Dispose();
        _disposed = true;
    }
}

DeploymentManagerDbContext.cs

public class DeploymentManagerDbContext : IDeploymentManagerDbContext
{
    private readonly IServiceProvider _serviceProvider;

    public DeploymentManagerDbContext(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider;
    }

    public IChangeRequestRepository ChangeRequests =>
        _serviceProvider.GetService(typeof(IChangeRequestRepository)) as IChangeRequestRepository;

    public IBuildRepository Build =>
        _serviceProvider.GetService(typeof(IBuildRepository)) as IBuildRepository;

    public IBatchRepository Batch =>
        _serviceProvider.GetService(typeof(IBatchRepository)) as IBatchRepository;

    public IUserRepository Users =>
        _serviceProvider.GetService(typeof(IUserRepository)) as IUserRepository;
}

UserRepository.cs

public class UserRepository : IUserRepository
{
    private readonly IDbConnection _dbConnection;

    public UserRepository(DeploymentManagerDbConnection deploymentManagerDbConnection)
    {
        _dbConnection = deploymentManagerDbConnection?.Connection;
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ I would recommand moving the Startup code into an extension or you can put it under a namespace, and use Reflection to get all instances and register them, try to automate things. The connectionString should be used inside the DeploymentManagerDbContext and not the Repository. if used on Repository then what purpose the DeploymentManagerDbContext serves ? \$\endgroup\$
    – iSR5
    Sep 8, 2021 at 5:41

1 Answer 1

3
\$\begingroup\$

Extra boiler plating

To start with unless there is missing code from DeploymentManagerDbConnection.cs or DbConnection.cs they aren't doing anything interesting or cleaver with the underlying IDbConnction you're wrapping. If anything I would say to directly use it as is.

  • Startup.cs
services.AddScoped<IDbConnection>(_ => 
    new SqlConnection(Configuration.GetConnectionString("DeploymentManagement"))
));
  • UserRepository.cs
public UserRepository(IDbConnection connection)
{
    _dbConnection = connection;
}

Using IServiceProvider outside of Startup.cs

In almost all cases it isn't recommended to be accessing the di container directly. Registrations or dynamic factories should know about the container IServiceProvider, but other wise it really shouldn't leak out to other places.

That being said DeploymentManagerDbContext looks like a factory but is far too simple to justify IServiceProvider leaking.

  • Register Disposables as Func<T>

To preface, I would expect your interfaces like IUserRepository to inherit from IDisposable and in turn would call the dispose method on your IDbConnection.

What I would expect for your use-case for your repositories, you want to build them at runtime where ever you need them, but also want to follow/use DI to wire them up correctly. This is where Func<T> comes in.

  • Startup.cs
services.AddTransient<IBuildRepository, BuildRepository>();
services.AddTransient<Func<IBuildRepository>>(_ => _.GetService<IBuildRepository>);

If you find you're seeing a bit of duplication, an extension method can cut down on this.

  • RegistrationExtentions.cs
public static IServiceCollection AddRepository<TInterface, TImplimentation>(this IServiceCollection services)
    where TInterface : class
    where TImplimentation: TInterface
{
    services.AddTransient<TInterface, TImplimentation>();
    services.AddTransient<Func<TInterface>>(_ => _.GetService<TInterface>);
    return services;
}

However, if you don't want to create helper functions for this type of extra registrations, I'd recommend using Lamar as it auto-adds a type build rule for Func<T> and even Lazy<T>.

  • Startup.cs
services.AddRepository<IBuildRepository, BuildRepository>()
    .AddRepository<IBatchRepository, BatchRepository>();
  • Back to DeploymentManagerDbContext

We can now directly ask from the container for a Buildable repository.

//this class is also entirely optional to keep or throw
public class DeploymentManagerDbContext : IDeploymentManagerDbContext
{

    public DeploymentManagerDbContext(
    Func<IChangeRequestRepository> change,
    Func<IBuildRepository> build,
    Func<IBatchRepository> batch,
    Func<IUserRepository> user)
    {
        _change = change;
        _build = build;
        _batch = batch;
        _user = user;
    }
    
    private readonly Func<IChangeRequestRepository> _change;
    public IChangeRequestRepository ChangeRequests => _change();
    //skipping the other 3, just follow the pattern
}
  • Usage Now in any code file you can directly ask for what you want built.
public class ProcessController : Controller
{
    private readonly IDeploymentManagerDbContext _contextFactory;
    public ProcessController(IDeploymentManagerDbContext contextFactory)
    {
        _contextFactory = contextFactory;
    }

    public void DoWork()
    {
         using userContext = _contextFactory.Users;
         //sample only for this purpose there are better ways to do authentication
         userContext.IsValidUser(Request.Context.CurrentUser);

         using buildContext = _contextFactory.Build;
         buildContext.BuildFoo();

         using batchContext = _contextFactory.Batch;
         batchContext.BatchBar();
    }
}
  • Without IDeploymentManagerDbContext Since the registrations are at the DI level you can also just directly ask for a specific builder.
public class ProcessController : Controller
{
    //skipping other fields
    private readonly Func<IBuildRepository> _build;
    public ProcessController(
    Func<IBuildRepository> build,
    Func<IBatchRepository> batch,
    Func<IUserRepository> user)
    {
        //skipping assignments
    }

    public void DoWork()
    {
         using userContext = _users();
         //sample only for this purpose there are better ways to do authentication
         userContext.IsValidUser(Request.Context.CurrentUser);

         using buildContext = _build();
         buildContext.BuildFoo();

         using batchContext = _batch();
         batchContext.BatchBar();
    }
}
\$\endgroup\$
11
  • \$\begingroup\$ Hi @Michael Rieger. Thanks for your review. Just to clarify, DeploymentManagerDbConnection exists to create separation between multiple database connections. The database connection may not be the same between these repositories so by specifying DeploymentManagerDbConnection instead of SomeOtherDbConnection, the idea was that DI would be able to inject the appropriate connection. Do you have any thoughts on this approach? \$\endgroup\$ Sep 10, 2021 at 17:53
  • \$\begingroup\$ @AdamChubbuck doing this in 3 parts it might be a bit of a leaky abstraction where a class knows which implementation of an interface to use but the sample from this answer does what you expect without extra hoops while still keeping this stuff in Startup.cs. \$\endgroup\$ Sep 10, 2021 at 19:51
  • \$\begingroup\$ this answer can look simpler but introduces a lot of redundant types., but had the benefit of types don't directly try to say i want the SpecificImplimentation for interface x \$\endgroup\$ Sep 10, 2021 at 19:51
  • \$\begingroup\$ this answer is more direct by only adding 1 extra type but make you implement some property to tag/filter the right implementation you want. \$\endgroup\$ Sep 10, 2021 at 19:51
  • 1
    \$\begingroup\$ No problem @Michael Rieger. So just to sum everything up, the repository interfaces should implement IDisposable and be added as transients. IDeploymentManagerDbContext should be injected as scoped, although it doesn't really matter much since the associated repositories will be retrieved as new instances regardless. And DeploymentManagerDbConnection and DbConnection can be removed in favor of a transient IDBConnection instance. \$\endgroup\$ Sep 14, 2021 at 16:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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