Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lifestyle Scope Question with DbContext ASP.NET Core Integration #398

Closed
tylerlrhodes opened this issue Apr 2, 2017 · 10 comments
Closed
Labels

Comments

@tylerlrhodes
Copy link

Sorry if this has been answered before.

I'm trying to integrate Simple Injector into an ASP.NET Core MVC project.

It's gotten complicated in that it uses Identity, so if I'm correct that means I have to "cross-wire" some of the objects to be resolved by the ASP.NET DI Container so that Simple Injector can resolve them. I'm new to DI in general, so I apologize in advance for getting some of the terms incorrect.

Anyway, I've gotten it working for the most part, but I'm having trouble with the Lifestyle's. My understanding is that a DbContext is supposed to essentially live for a single web request, and that would mean Lifestyle.Scoped. However, when I set this as the lifestyle, I get an exception in other places accessing that context that I cannot access a disposed instance.

If I set it to a Singleton, then it works, but I don't think this is correct. I put the code

https://gist.github.com/tylerlrhodes/e8acce9487b03cdf7cee7c9ff48db2dd

You can see what I've got in the above gist.

Thanks in advance

-Tyler

@tylerlrhodes
Copy link
Author

tylerlrhodes commented Apr 2, 2017

I was able to get it working with the correct Lifestyle with the following function from another issue:

    private Func<T> GetAspNetServiceProvider<T>(IApplicationBuilder app)
    {
      var appServices = app.ApplicationServices;
      var accessor = appServices.GetRequiredService<IHttpContextAccessor>();
      return () => {
        var services = accessor.HttpContext != null
            ? accessor.HttpContext.RequestServices
            : this._container.IsVerifying ? appServices : null;
        if (services == null) throw new InvalidOperationException("No HttpContext");
        return services.GetRequiredService<T>();
      };
    }

I created another Gist if anyone cares to see:

https://gist.github.com/tylerlrhodes/fb44067f92f6bd0f013f0d40af681961

@tylerlrhodes
Copy link
Author

tylerlrhodes commented Apr 3, 2017

This does leave me with one question however. Is it possible to get an instance of one of the registered objects using this function from a spot in the code which is outside of a request?

For instance, I have a piece of code that is run on startup which requires access to the UserManager and RoleManager objects which are resolved through the DI.

It calls a method like:

using (AsyncScopedLifestyle.BeginScope(container))
{
    UserManager<ApplicationUser> userManager =
        container.GetInstance<UserManager<ApplicationUser>>();
    RoleManager<IdentityRole> roleManager = container.GetInstance<RoleManager<IdentityRole>>();

    string userName = configuration["Data:AdminUser:Name"];
    string email = configuration["Data:AdminUser:Email"];
    string password = configuration["Data:Adminuser:Password"];
    string role = configuration["Data:AdminUser:Role"];

    if (await userManager.FindByNameAsync(userName) == null)

But with that GetAspNetServiceProvider function, when this method is called it fails because its not during a HTTP Request from what I can tell.

When I go through the .NET Core DI to try and get it (which used to work) the objects seem to be disposed as soon as they are created, leading to an exception.

Any ideas?

Thanks

@dotnetjunkie
Copy link
Collaborator

dotnetjunkie commented Apr 3, 2017

As you already noticed, the problem lies in this line:

_container.Register<BlogDbContext>(() => app.ApplicationServices.GetService<BlogDbContext>(),
    Lifestyle.Singleton);

There are two problems with this registration. First of all, you use the Singleton lifestyle, which means that Simple Injector will hold on to this BlogDbContext forever. But even if you didn't do this, since you are requesting that service from the ApplicationServices, you'll get a single instance. IMO this is a major design flaw in how ASP.NET Core works; it will allow you to resolve instances that are registered as transient or scoped from ApplicationServices, while they will effectively become singletons. ASP.NET Core contains no safety net for this.

Your GetAspNetServiceProvider extension method solves this problem, but the Simple Injector integration package for ASP.NET Core however already solves this for you by supplying GetRequestService and GetRequiredRequestService extension methods out of the box:

_container.Register<BlogDbContext>(() => app.GetRequiredRequestService<BlogDbContext>(),
    Lifestyle.Scoped);

Typically however you should strive to register your application components, such as your BlogDbContext inside Simple Injector, instead of inside the framework's configuration system. So if possible, you might want to turn things around, i.e. cross-wire the DbContext in the identity configuration, instead of cross-wiring this in Simple Injector. I'm however not that familiar with Identity Framework, and I'm not sure whether this is easily possible.

Another option is to not cross-wire your DbContext at all, but simply register your DbContext as Scoped in Simple Injector as well. This does mean that you might get 2 DbContext instances during a single request, but it's questionable whether that would be a problem, since all your application code will get its own DbContext and only Identity Framework services will get their own. This shouldn't be a problem, because Identity calls SaveChanges on that DbContext, so it might actually be useful to give it its own scoped instance. It prevents Identity Framework from calling SaveChanges on your DbContext in the middle of your own business operation.

But with that GetAspNetServiceProvider function, when this method is called it fails because its not during a HTTP Request from what I can tell.

That's because your extension method (and Simple Injector's GetRequiredRequestService require there to be an active request. You are probably running your operations at application startup, or on a background thread. Letting your DbContext be managed by Simple Injector makes this much easier, because you can explicitly wrap your background (or startup) code in a Simple Injector Scope and this allows Simple Injector to resolve your scoped instances.

Also note that there should typically be no need to use ThreadScopedLifestyle. Even on a background thread, you can start an AsyncScopedLifestyle by calling AsyncScopedLifestyle.BeginScope(Container). AsyncScopedLifestyle works evenly well when executed on just a single thread. This removes the need to have a hybrid lifestyle.

@tylerlrhodes
Copy link
Author

tylerlrhodes commented Apr 3, 2017

After looking at the source for Identity and taking 45 minutes to try to wire up all the required services in Simple Inject I gave up and will live with the cross-wiring for now.

Unfortunately, that bit of code that runs on startup when there isn't an active request also needs Identity, which is a bit of a pain otherwise I would wire it up just using Simple Inject as you recommended.

If I run that startup code before the Simple Inject container takes over and the cross-wiring takes place it runs just fine using the .NET Core DI so that's a workable solution for now.

Thanks for taking the time to respond! This little excursion into Simple Injector came from reading your blog entries on CQRS which I thought were great.

If it weren't for Identity this would have been a lot easier, but I get the feeling figuring out how to wire up all of their services in Simple Injector is going to be more trouble than it's worth.

@dotnetjunkie
Copy link
Collaborator

dotnetjunkie commented Apr 4, 2017

After looking at the source for Identity and taking 45 minutes to try to wire up all the required services

This is typically not a path you should pursue. 3rd party libraries that use IServiceCollection to wire up their dependencies, should typically keep using the built-in container. Trying to move those registrations to your application container will lead to a lot of complications and especially a lot of trouble with the next update of that library. You'll be duplicating all its registrations. Instead, try to keep these worlds as separated as you can. This means that at some point you might need to request a service from the built-in container or vice versa. But only do so, in case your application actually needs that service directly.

I get the feeling figuring out how to wire up all of their services in Simple Injector is going to be more trouble than it's worth.

Absolutely. This not something that you should do. Keep your application registrations separated from framework and 3rd party registrations. Those have to be in the built-in container to prevent falling into the Conforming Container trap.

Unfortunately, that bit of code that runs on startup when there isn't an active request also needs Identity

When code runs at startup, it might not be a problem that Identity components are resolved as singleton, since this would be the only time they are used. In case you really need to resolve both (scoped) application components and (scoped) Identity components at startup or on a background thread, you might need to create a Core DI scope next to your Simple Injector scope. You can do this by resolving an IServiceScopeFactory from app.ApplicationServices, call CreateScope on it and resolve the service from that scope (using GetRequiredService<T>()).

@tylerlrhodes
Copy link
Author

That makes sense. Thanks for the explanation again.

I've created a scope using the Core DI container for the Identity bits that run on startup and that seems to work just fine.

Just so I understand though, with Simple Injector's Asp.net Core Mvc integration, if I'm not mistaken, it takes over for the creation of controllers -- since I have a few controllers that rely on constructor injection to get an instance of Identity classes, this is why I need to cross-wire a few of their services for it to work -- correct?

@dotnetjunkie
Copy link
Collaborator

if I'm not mistaken, it takes over for the creation of controllers -- since I have a few controllers that rely on constructor injection to get an instance of Identity classes, this is why I need to cross-wire a few of their services for it to work -- correct?

Simple Injector indeed takes over the creation of controllers. This makes controllers application components; that your application container is in control over. Once you need to inject framework abstractions (that are registered in the ASP.NET configuration system opposed to in Simple Injector) into your application components, that's the time you start to cross-wire.

Cross-wiring can however often be something as simple as the following:

public void Configure(IApplicationBuilder app, IHttpContextAccessor accessor)
{
    container.RegisterSingleton<IUserContext>(new AspNetUserContextAdapter(accessor));
}

Here we use a singleton service (the IHttpContextAccessor) by injecting it into a singleton adapter (the AspNetUserContextAdapter) by creating that adapter by hand.

As a general rule of thumb, I would say that your application components should not depend on framework abstractions (i.e. if you follow the Dependency Inversion Principle). Instead only specially crafted adapters, that are part of your Composition Root, should depend on framework stuff.

This however isn't always feasible, and it also depends on how you view the system. Your controllers, can very well be simple infrastructural glue that they just adapt between framework stuff and the application code. In such case it could make a lot of sense to inject framework dependencies into controllers and that means that you probably need to cross-wire framework dependencies into the application container by registering those dependencies directly.

If this is the case, you could even view controllers as non-application components and let them be built-up by the built-in configuration system. This especially works well if controllers hardly have any application-specific abstractions. This especially works well if you apply an CQRS-style application model, like described here and here, since this would typically leave your controllers with just two application-specific dependencies (i.e. ICommandDispatcher and IQueryDispatcher) that get cross-wired in the built-in configuration system.

In case you are writing a Web API (opposed to an MVC application) is to remove the controller layer completely and replace it with a thin piece of infrastructure that just forwards calls to your command handlers and query handlers. Again, this requires this CQRS-style of programming. Examples of this can be in this Github repository, but examples of how to apply this to ASP.NET Core are currently missing.

Long story short, there are quite a few options and views on this.

@tylerlrhodes
Copy link
Author

tylerlrhodes commented Apr 8, 2017

So I've gotten the chance to play around with this a little more, and I've noticed something that seems peculiar.

In my ASP.NET Core app as I mentioned before I call a few functions during startup that runs at startup, while there is not an HTTP session in play.

For instance:

BlogDbContext.CreateAdminAccount(app.ApplicationServices, Configuration).Wait();
BlogDbContext.CreateAuthorRole(app.ApplicationServices).Wait();

InitializeContainer(app);

_container.Verify();

The calls to CreateAdminAccount and CreateAuthorRole use the built in ASP.NET DI to resolve some Identity components that need to be seeded on startup.

This works fine. However, when I place the calls to these functions after _container.Verify(), such as:

InitializeContainer(app);

_container.Verify();

BlogDbContext.CreateAdminAccount(app.ApplicationServices, Configuration).Wait();
BlogDbContext.CreateAuthorRole(app.ApplicationServices).Wait();

I get an exception, such as:

Inner Exception 1:

ObjectDisposedException: Cannot access a disposed object.

It seems that as long as I call them before Verify, there isn't a problem. I'm not sure why calling them after Verify would trigger this error -- would this be expected behavior?

It isn't that big of a deal regardless, I just wouldn't expect verify to have an impact there.

Thanks again,

Tyler

@dotnetjunkie
Copy link
Collaborator

I bet you cross-wired your DbContext in Simple Injector as Scoped and this DbContext is resolved from the built-in application components. Verify tests the creation of components and will dispose scoped registrations afterwards.

@tylerlrhodes
Copy link
Author

Yes, that's exactly how I registered the DbContext and the Identity components.

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants