Dependency Injection circular dependency .NET Core 2.0

Question

I want my ApplicationContext constructor to have the UserManager as a parameter, but I am having trouble with dependency injection.

Code:

public class ApplicationContext : IdentityDbContext<ApplicationUser>
{
    private IHttpContextAccessor _contextAccessor { get; set; }
    public ApplicationUser ApplicationUser { get; set; }
    private UserManager<ApplicationUser> _userManager;

    public ApplicationContext(DbContextOptions<ApplicationContext> options, IHttpContextAccessor contextAccessor, UserManager<ApplicationUser> userManager)
        : base(options)
    {
        _contextAccessor = contextAccessor;
        var user = _contextAccessor.HttpContext.User;
        _userManager = userManager;
        ApplicationUser = _userManager.Users.FirstOrDefault(u => u.Id == _userManager.GetUserId(user));
    }
}

And in startup.cs:

public void ConfigureServices(IServiceCollection services)
{
    // Add framework services.
    services.AddDbContext<ApplicationContext>(options =>
        options.UseSqlServer(Configuration.GetConnectionString("DefaultConnection"), b => b.MigrationsAssembly("RCI.App")));

    services.AddIdentity<ApplicationUser, IdentityRole>()
        .AddEntityFrameworkStores<ApplicationContext>()
        .AddDefaultTokenProviders();

    services.AddAuthentication();

    services.AddMvc();

    // Add application services.
    services.AddTransient<IEmailSender, AuthMessageSender>();
    services.AddTransient<ISmsSender, AuthMessageSender>();
    services.AddTransient<IHttpContextAccessor, HttpContextAccessor>();

    services.AddOptions();

}

Error message:

A circular dependency was detected for the service of type 'Microsoft.AspNetCore.Identity.UserManager`1[RCI.App.Models.ApplicationUser]'.

Could anyone point out what I'm doing wrong?

Answer 1

If you don't actually need the UserManager in the constructor, you can store a reference to the IServiceProvider instead:

private IHttpContextAccessor _contextAccessor { get; set; }
public ApplicationUser ApplicationUser { get; set; }
private IServiceProvider _services;

public ApplicationContext(DbContextOptions<ApplicationContext> options,
    IHttpContextAccessor contextAccessor, IServiceProvider services)
    : base(options)
{
    _contextAccessor = contextAccessor;
    var user = _contextAccessor.HttpContext.User;
    _services = services;
}

Then later, when you actually need the ApplicationUser, call e.g. GetRequiredService<ApplicationUser>() (defined in Microsoft.Extensions.DependencyInjection):

var manager = _services.GetRequiredService<UserManager<ApplicationUser>>();
var user = manager.Users.FirstOrDefault(u => u.Id == _userManager.GetUserId(user));

Of course, you can use a Lazy<T> to lazy-load the manager or user the first time and then store a reference to it.

In general, @poke is correct about re-architecting to avoid such circular dependencies, but leaving this answer here in case someone else has a similar problem and refactoring isn't an option.

Answer 2

Circular dependencies are usually a sign for an improper application design, which should be revised. As I already mentioned in the comments, having a database context that depends on the user manager does not seem to be a good idea. This makes me assume that your database context does too much and likely violates the single-responsibility principle.

Just looking at the dependencies of your database context, you are already adding too much application specific state in there: You not only have a dependency on the user manager, but also on the HTTP context accessor; and you are resolving the HTTP context also immediately in the constructor (which is generally not the best idea).

From your code excerpt it seems that you want to retrieve the current user for later use. If you want to use this for example to filter queries for the user, then you should think about whether it’s really a good idea to statically bake this into the database context instance. Consider accepting an ApplicationUser inside methods instead. That way, you get rid of all those dependencies, you make your database context better testable (since the user is no longer a state of the context), and you also make the single responsibility of the context clearer:

public IList<Thing> GetThings (ApplicationUser user)
{
    // just an example…
    return Things.Where(t => t.UserId == user.Id).ToList();
}

Note that this is also inversion of control. Instead of having the database context actively retrieve the user it should query for (which would add another responsibility, violating the SRP), it expects to be passed the user it should query for, moving the control to the calling code.

Now, if you query stuff for the current user very often, it might become somewhat annoying to resolve the current user in a controller and then pass it to the database context. In that case, create a service to no longer repeat yourself. That service can then depend on the database context and other things to figure out the current user.

But just clearing up your database context from stuff it shouldn’t do is enough to fix this circular dependency.

Answer 3

Many Thanks to Toby, for solution. You can also use Lazy<IMyService> to prevent calling _services.GetRequiredService<UserManager<ApplicationUser>>() each time u wanna use it.

private IHttpContextAccessor _contextAccessor { get; set; }
public ApplicationUser ApplicationUser { get; set; }
private Lazy<UserManager<ApplicationUser>> _userManager;

public ApplicationContext(DbContextOptions<ApplicationContext> options,
    IHttpContextAccessor contextAccessor, IServiceProvider services)
    : base(options)
{
    _contextAccessor = contextAccessor;
    var user = _contextAccessor.HttpContext.User;
    _userManager = new Lazy<UserManager<ApplicationUser>>(() =>
                services.GetRequiredService<UserManager<ApplicationUser>>());
}

and when u wanna use it just say :

_userManager.value.doSomeThing();