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?

Acquiescence

Just for clarification, do you understand what a circular dependency is? – Hobnailed

Also, this appears to be an XY problem. What is the ultimate goal you are trying to achieve? Why do you need access to the user manager within the application context? – Hobnailed

@Hobnailed I want to update each entity with created/modified by on every create/update with the current user. I know I could pass in the user to my Save() method but it would be much easier to just have it in the context in one place instead – Acquiescence

UserManager<ApplicationUser> and ApplicationContext in your example have explicit dependencies on each other, resulting in a circular dependency. when resolving ApplicationContext it has to create an UserManager<ApplicationUser> which needs a ApplicationContext. You see where that is going? – Hobnailed

Having a dependency on a user manager inside a database context doesn’t sound like a good idea. You probably should have a service instead that depends on both the database context and the user manager. – Lavallee

@Acquiescence You already have access to Users within the application context. make your query directly on that with the the current request's user id. no need to reference the user manager at all. – Hobnailed

@Hobnailed where does it say that UserManager<ApplicationUser> is dependant on ApplicationContext ? – Acquiescence

@Acquiescence check it's constructor – Hobnailed

@Hobnailed thanks a mill, didn't realise I had access to Users in the context already! Just for clarification though, I still don't see where UserManager is dependant on the context. Here is the ctor: public UserManager(IUserStore<TUser> store, IOptions<IdentityOptions> optionsAccessor, IPasswordHasher<TUser> passwordHasher, IEnumerable<IUserValidator<TUser>> userValidators, IEnumerable<IPasswordValidator<TUser>> passwordValidators, ILookupNormalizer keyNormalizer, IdentityErrorDescriber errors, IServiceProvider services, ILogger<UserManager<TUser>> logger); – Acquiescence

UserManager<T> depends on UserStore<T> which depends on the database context that’s registered with ASP.NET Core Identity, which happens to be your database context that depends on the user manager. – Lavallee

@Acquiescence you should also keep the ApplicationContext constructor lean and not try to access the user or make queries within there. extract the user and make your queries within the target methods as the request would have been fully realized by then. It should basically only have _contextAccessor = contextAccessor; the rest should be done on one of the crud operations. – Hobnailed

Answer 1

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.

Lavallee

Answer 2

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.

Ausgleich

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();

Appal