-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Add AddDbContextFactory to allow registering a factory for creating contexts #21246
Conversation
…ontexts Fixes #18575 Mostly this is copied from @JeremyLikness's sample, with tests added. It evolved a bit as I was testing. Most significantly, we now register an IDbContextFactory interface, for which we provide a default implementation. This allows the factory to be replaced by one specific for the application, which can be more performant because it doesn't use ActivatorUtilities. Overloads have been added for this. I believe we can also add overloads to allow the factory to resolve from a context pool, but I will file a new issue for this.
/cc @JeremyLikness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This basically makes Reset()
obsolete
We should show custom factory implementation in the docs as that will probably be at least twice as performant
@AndriySvyryd Which |
I meant to say |
@AndriySvyryd I've come to the conclusion that it won't matter how many ways we have to create a context instance--some people are still going to want to detach all entity instances. If they are going to do it anyway, then |
/// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
/// </summary> | ||
public virtual TContext CreateDbContext() | ||
=> ActivatorUtilities.CreateInstance<TContext>(_provider); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ajcvickers for better performance you should stash and ObjectFactory in a field by calling ActivatorUtilities.CreateFactory<TContext>()
. Then you can use that factory to create instances of TContext. It's much more optimized and avoids the constructor selection per call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it going to be possible to use both This doesn't seem possible right now as the DbContextFactory can't consume DbContextOptions because it's set as a scoped service by the call to
|
@davidkarlsson Yes, this will work, but an overload of AddDbContext that allows the options lifetime to be set must be used, and the lifetime should be set to Singleton. So we can understand how this feature is going to be used, could you provide some more details on why you want to do this? |
@ajcvickers The reasons I would want to do this might be based on issues with my underlying design and the lifetime of the services I use.. I have multiple projects (ASP.NET and Windows services) that share singletons in which I would like to use the DbContextFactory for creating new contexts since they are not always used by the Windows service for example so injecting them in the constructor would be wasteful. At the same time I would like to continue injecting request scoped contexts directly in the controllers of the ASP.NET project instead of using the factory there so I don't have to dispose the contexts manually. In the singletons I'm just creating new instances with the parameterless constructor at the moment and use I'm also thinking that I might want to use the context pooling feature in the future which isn't possible with my current setup because the context class has a parameterless constructor. If I could use the DbContextFactory instead this would allow me to use the context pooling feature because I wouldn't need parameterless constructor any longer. |
I agree it should be possible to use both at the same time. It's really common to want to use a DbContext per request and the factory for singletons e.g. https://github.com/dotnet-presentations/aspnetcore-app-workshop/blob/b9cde864b6917c10fbef979a4806dd1a658030a8/src/FrontEnd/Services/AdminService.cs#L9-L44 |
@davidkarlsson Thanks for the info--your case makes sense and aligns with the reason we made this possible. |
@ajcvickers But it isn't possible right now unless I'm missing something? Changing the ServiceLifetime for contexts injected directly to Singleton with the |
@davidkarlsson Only the lifetime of the options needs to change from the default. For example: .AddDbContext<WoolacombeContext>(
b => b.UseInMemoryDatabase(nameof(WoolacombeContext)), ServiceLifetime.Scoped, ServiceLifetime.Singleton)
.AddDbContextFactory<WoolacombeContext>(
b => b.UseInMemoryDatabase(nameof(WoolacombeContext))) |
@ajcvickers I see! I totally missed that that was possible. Thank you! |
@ajcvickers I'd missed this was possible too and spent a while done the rabbit-hole before coming across this thread. I think a lot of people will be using the DbContextFactory approach with existing code and may run up against lifetime errors. Would it be worth adding something in the docs specifically about the need to set the lifetimes for I've stuck a Q&A on stack overflow to try and surface this too. |
@tomRedox Thanks--I added a note to the item tracking documentation. |
The alternative to the above solution is to register the factory as scoped as well: .AddDbContext<WoolacombeContext>(
b => b.UseInMemoryDatabase(nameof(WoolacombeContext)), ServiceLifetime.Scoped)
.AddDbContextFactory<WoolacombeContext>(
b => b.UseInMemoryDatabase(nameof(WoolacombeContext)), ServiceLifetime.Scoped) |
Fixes #18575
Mostly this is copied from @JeremyLikness's sample, with tests added. It evolved a bit as I was testing. Most significantly, we now register an IDbContextFactory interface, for which we provide a default implementation. This allows the factory to be replaced by one specific for the application, which can be more performant because it doesn't use ActivatorUtilities. Overloads have been added for this.
I believe we can also add overloads to allow the factory to resolve from a context pool, but I will file a new issue for this.