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

Pattern for seeding database with EF7 in ASP.NET 5 #3070

Closed
ducalai opened this issue Sep 9, 2015 · 16 comments
Closed

Pattern for seeding database with EF7 in ASP.NET 5 #3070

ducalai opened this issue Sep 9, 2015 · 16 comments
Assignees
Milestone

Comments

@ducalai
Copy link

ducalai commented Sep 9, 2015

Hi

I am playing around with MVC Music Store and I try to access Entities from DbContext once it has been initialized with very last line SampleData.InitializeMusicStoreDatabaseAsync(app.ApplicationServices).Wait() in Startup.cs file.

If right after that static call, I reach DbContext using var db = app.ApplicationServices.GetService(), then every access to db.[Entity].ToListAsync() generates an "Object reference not set to an instance of an object" exception.

This exception is not thrown if I remove the using block when accessing the DbContext in SampleData.InitializeMusicStoreDatabaseAsync.

This exception was not thrown in beta6. I am currently running beta7 version. Is that a bug or am I doing anything wrong?

Thanks for your help
Alain

@rowanmiller
Copy link
Contributor

Can you provide sample code that we can use to reproduce the issue you are seeing?

@divega
Copy link
Contributor

divega commented Sep 10, 2015

If I understand what @ducalai says correctly, his code might be trying to use a DbContext instance from the same DI scope that has already been disposed by the using statement in SampleData.InitializeMusicStoreDatabaseAsync(). This is expected not to work, but the NullRefException is not expected.

@ducalai
Copy link
Author

ducalai commented Sep 11, 2015

Yes you are right. I do understand that in the same DI scope, if context has been disposed by the using statement, this is not expected to work.

If you want a simple sample code, just add those two following lines of code in MVC Music Store sample project, after the very last line of Configure(IApplicationBuilder app) method of MusicStore.xproj:

//Get context
var context = app.ApplicationServices.GetRequiredService();
var albums = context.Albums.ToListAsync();//This line throws NullRefException

So what is the correct way to access DbContext content in that current scope? Is the using statement incorrect and I should rely on DI to dispose DbContext at the end of scope? Will this scope last untill next application restart?

@divega
Copy link
Contributor

divega commented Sep 11, 2015

Yes, applying a using statement on a context whose lifetime is controlled by DI is not the best idea because the context instance is supposed to be shared by any code running in the same request. Of course, in tge unmodified version of MusicStore this wasn't causing any problems because nothing else was trying to use the context.

I believe the code in Startup.Configure() gets executed in a request scope so the context wouldn't be kept alive for the whole application lifetime, but I am not 100% sure (@HaoK do you know from the top of your head?).

For what it is worth the pattern that creates the database on start up used in MusicStore isn't something we would recommend for production deployment in real world applications.
I believe from the top

@HaoK
Copy link
Member

HaoK commented Sep 11, 2015

Configure actually is run when building the application pipeline, so there is no request at this point. The request scope gets added automatically to the pipeline via the IStartupFilter around the pipeline that is build by Configure, but this SampleData.Initialize call is being run as a sideeffect of building the pipline using the ApplicationServices that's being passed in from the IAppBuilder.

@divega
Copy link
Contributor

divega commented Sep 11, 2015

Thanks for the details. Hence I assume the DbContext obtained here from DI will be kept alive for the duration of the application, right?

@ducalai
Copy link
Author

ducalai commented Sep 11, 2015

Seems that this SampleData.Initialize call in Configure method is not the optimal way to initialize database. Do you have a more recommended solution?

@divega
Copy link
Contributor

divega commented Sep 11, 2015

@rowanmiller has put together a sample of a seeding method and its usage.

Notice the sample does the seeding in Startup.Configure() so it has the same issues we have discussed in this thread. We will look at this problem and come back to you when we have a solution.

@divega
Copy link
Contributor

divega commented Sep 11, 2015

@rowanmiller and I discussed this briefly and we think there isn't yet a great place to do this kind of work in an ASP.NET 5.0 application. But we are going to try to work around it by obtaining the DbContext from a scoped service provider (similar to what is exposed later to each Web request) so that DbContext instance used for seeding shouldn't stay in memory after the service scope is disposed. E.g. the code in his sample would change to look somewhat like this:

using(var serviceScope = app.ApplicationServices
              .GetRequiredService<IServiceScopeFactory>()
              .CreateScope())
{
    var context = serviceScope.ServiceProvider.GetService<UnicornStoreContext>();
...

@divega divega changed the title DbContext no more accessible Pattern for seeding database with EF7 in ASP.NET 5 Sep 11, 2015
@ducalai
Copy link
Author

ducalai commented Sep 11, 2015

Sounds great! Thanks for your work. I will wait for your news

@HaoK
Copy link
Member

HaoK commented Sep 11, 2015

Feels like we are missing an OnApplicationStart feature that takes care of this, (creates a scope, calls the delegate) Altho the scope might just be an EF thing

@rowanmiller
Copy link
Contributor

👍

@divega
Copy link
Contributor

divega commented Sep 12, 2015

@HaoK Assuming I have understood everything correctly it seems that any service that is registered as scoped will be created with the wrong lifetime if used in Startup.Configure(). Maybe the EF seeding scenario helps make that more evident but I don't think it is just an EF issue.

Can you help by articulating this in a bug and gauge the chances of us fixing it? (the fix may be adding a separate method like the OnApplicationStart() you mentioned, but I think it would be good to discuss if we need to do anything about consuming scoped services from Startup.Configure() or if this is something that just needs to be documented).

@HaoK
Copy link
Member

HaoK commented Sep 12, 2015

Well, the general issue is that there is no such thing as a scope in the ApplicationServices, this bug is sort of related, where it would be nice if we at least detected and threw an exception when trying to resolve a scoped service from ApplicationServices, I hit this before in identity and MVC hit this behavior as well, so its definitely an easy thing to stumble into:

aspnet/DependencyInjection#88

@YZahringer
Copy link

Hello,

Another problematic case is when you use a Timer. During the callback there have no scope. So the used instances are always the same, because you have to use the ServiceProvider of the application, so always the same context.

What would be the best way to use DI with a Timer? The Timer sould perhaps create a scope during the callback and pass it in parameter? Use IServiceScopeFactory in the callback?

@rowanmiller rowanmiller mentioned this issue Sep 17, 2015
10 tasks
@rowanmiller rowanmiller added this to the Discussions milestone Sep 18, 2015
@divega
Copy link
Contributor

divega commented Sep 23, 2015

I have filed a new issue at https://github.com/aspnet/Hosting/issues/373 for first class support of application code that runs at startup. In the meanwhile, running code within a service scope in Startup.Configure() seems to be the best workaround:

using(var serviceScope = app.ApplicationServices
              .GetRequiredService<IServiceScopeFactory>()
              .CreateScope())
{
    var context = serviceScope.ServiceProvider.GetService<UnicornStoreContext>();
...

This workaround is now implemented in the MusicStore sample.

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

No branches or pull requests

6 participants