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

Improve experience for creating multiple instances of a DbContext in the same request #4441

Closed
divega opened this issue Jan 30, 2016 · 14 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@divega
Copy link
Contributor

divega commented Jan 30, 2016

While responding to aspnet/DependencyInjection#352, I had what I think is a new idea (in the context of EF) that could help improve this experience. Here is a summary:

  1. Add to the framework a new "factory" class, e.g. let's for now call it DbContextFactory<TContext> class. Its shape would look somewhat like this:

    public class DbContextFactory<TContext>
    {
      public DbContextFactory(
        IServiceProvider serviceProvider, 
        DbContextOptions<TContext> options)
      {
        ...
      }
      public TContext Create()
      {
        ...
      }
    }
  2. The AddDbContext() method would register this in DI.

  3. Users that need a single context per request scope would still resolve the TContext directly in constructors and users that need to create multiple instances could resolve the factory and then create the individual instances manually using the Create() method.

A few details/decision points:

  1. Note that this does not replace the internal DbContextActivator mechanism but would be mostly sugar built on top of it (we still need a static place to stash the IServiceProvider).
  2. There are possibly other variations on the pattern to consider, e.g. make the class non-generic and the Create<TContext>() method generic, plus change where we pass the DbContextOptions, add a params object[] argument to Create(), etc.
  3. We can change the registration in AddDbContext() to always go through an instance of this class or just leave it as it is now.
  4. We actually already have the IDbContextFactory<T> type that this class could implement and that would become the actual service interface but it is currently used for different purposes. We need to understand if there is synergy in having the same type for all purposes of if it makes sense to look elsewhere.
  5. We need to decide whether the type that uses would resolve should represent just a factory or be more of a lifetime manager for DbContext instance, e.g. does it just know how to create instances that then the user owns disposing at appropriate times or should it be an IDisposable itself that keeps records of instances returned and that takes care of disposing it. If we go with the later then it would be a higher level service that would depend on a IDbContextFactory<T> for the creation.

@ajcvickers I don't remember if we ever discussed doing something like this. Do you?

Another improvement in this area would be the ability to specify transient scoping when calling AddDbContext.

@TrabacchinLuigi
Copy link

👍 would be very nice !

@ajcvickers
Copy link
Member

@divega I don't remember discussing anything like this. I think it could work.

@rowanmiller
Copy link
Contributor

BTW we already have IDbContextFactory with a similar API surface, so maybe this overlaps with that in some way.

@divega
Copy link
Contributor Author

divega commented Feb 1, 2016

Need to decide if this "factory" should be IDisposable and remember instances it created to flow the call to Dispose() them or if the user would own calling Dispose() on each of them.

@brendan-nobadthing
Copy link

Need to decide if this "factory" should be IDisposable and remember instances it created

Whenever I've tackled exactly this question before I've always gone with:

  • Factories make instances of things. they never hold on to them
  • If I want to hold onto a context and later Dispose it, I make a "DbContextManager" or "DbContextScope" this would, of course have the Factory injected in so it knows how to make new instances.

Also, I've found the IDbContextFactory interface already provided is good for this purpose.

I believe previous EF versions had some odd assembly scanning logic so that if EF found an IDBContextFactory in the same assembly as your DbContext, EF internals would use that to instantiate DbContexts.- Worth keeping in mind if your intended factory was for the layer above EF.
I assume this has been changed in EF Core for a more explicit Dependency Injection approach?

@divega
Copy link
Contributor Author

divega commented Feb 4, 2016

Factories make instances of things. they never hold on to them

@brendan-ssw totally agreed. If it handles the lifetime of the DbContext it is not just a factory anymore. That is why I started wrapping the word "factory" in quotes 😄

I will update the original description to account for this.

I believe previous EF versions had some odd assembly scanning logic so that if EF found an IDBContextFactory in the same assembly as your DbContext, EF internals would use that to instantiate DbContexts.- Worth keeping in mind if your intended factory was for the layer above EF.
I assume this has been changed in EF Core for a more explicit Dependency Injection approach?

We still have scanning logic in our design time tools, e.g. see
https://github.com/aspnet/EntityFramework/blob/dev/src/Microsoft.EntityFrameworkCore.Commands/Design/DbContextOperations.cs

I can't recall at the moment why this is needed but @bricelam for sure will.

@bricelam
Copy link
Contributor

bricelam commented Feb 4, 2016

IDbContextFactory is the design-time escape hatch for when we 1) can't create your service container, and 2) your DbContext doesn't have a default constructor.

@brendan-nobadthing
Copy link

@bricelam
Thanks, your comments have seriously helped my understanding here.
I typically use DI (usually with autofac) for everything and usually find that I can get everything working just how I want it for my app's runtime but then stuggle to use the same dependency chain when trying to invoke migrations.

So, is there any way to configure the service container when invoking the dnx ef migration commands?

Right now my EFCore test project has a DbContext with an empty constructor just so I can run migrations.
With using a DbContextFactory in this "escape hatch" way, I can't help feeling that this just passes the same problem down the chain: what if I want to inject a dependency into my DbContextFactory?

@bricelam
Copy link
Contributor

bricelam commented Feb 8, 2016

Think of IDbContextFactory as an application entry point. Just like the runtime entry point, it will need to setup the service container (e.g. by calling Startup.ConfigureServices)

@divega
Copy link
Contributor Author

divega commented Feb 8, 2016

Just to repeat @bricelam's previous comment, we only use IDbContextFactory when we cannot figure out other ways to create the service container. We have conventions that look for Startup.ConfigureServices() and...

@divega
Copy link
Contributor Author

divega commented Feb 8, 2016

(hit "Comment" too soon) command parameters you can use to tell EF Migrations where to find the startup project at design-time so that we can create the same service container configuration you would have at runtime.

@divega
Copy link
Contributor Author

divega commented Mar 24, 2016

Per #4668 we removed magic injection for parameter-less constructors. The most intuitive way of creating non-scoped context instances (i.e. using the new keyword) should now work just fine. It should also be easy enough to explain that you can resolve the DbContextOptions and pass it to constructors explicitly.

Given that, I suspect there may not be a big future for context factories at runtime.

I still think that IDbContextFactory is a weird name for something that is only supposed to help at design time, but I think that is a different issue.

I propose we close this. Thoughts?

Cc @rowanmiller @ajcvickers

@ajcvickers
Copy link
Member

@divega Agree it can be closed.

@rowanmiller
Copy link
Contributor

👍

@divega divega closed this as completed May 9, 2016
@bricelam bricelam modified the milestones: Backlog, 2.1.0 Jan 2, 2018
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 26, 2018
@ajcvickers ajcvickers modified the milestones: 2.1.0-preview1, 1.0.0 Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

6 participants