Blazor CRUD scaffolder should dispose the created DbContext instances

[danroth27]

The Blazor CRUD scaffolder was updated to use IDbContextFactory to create DbContext instance as per the documented Blazor guidance. However, the DbContext created on the Index page to setup the QuickGrid doesn't get disposed:

@inject IDbContextFactory<MoviesAppContext> DbFactory
...
<QuickGrid Class="table" Items="DbFactory.CreateDbContext().Movie">

I think instead we need to create a DbContext that is scoped to the lifetime of the Index page and disposed when the component gets disposed, as described here: https://learn.microsoft.com/aspnet/core/blazor/blazor-ef-core. This way, the DbContext remains activity while it is in use by the QuickGrid and then gets disposed when the page gets disposed.

@JeremyLikness @guardrex @javiercn

[danroth27]

I think we want something like this:

@inject IDbContextFactory<MoviesAppContext> DbFactory
@implements IDisposable
...
<QuickGrid Class="table" Items="DB.Movie">
...
@code {
    protected override void OnInitialized()
    {
        DB = DbFactory.CreateDbContext();
    }

    MoviesAppContext DB { get; set; } = default!;

    public void Dispose()
    {
        DB.Dispose();
    }
}

[guardrex]

I asked on the tutorial PR about ...

• A field works, too. Is that an antipattern?
• Async init and disposal works, too. Is that an antipattern?

@inject IDbContextFactory<MoviesAppContext> DbFactory
@implements IAsyncDisposable
...
<QuickGrid Class="table" Items="context.{MODEL}">
...
@code {
    MoviesAppContext context { get; set; } = default!;

    protected override async Task OnInitializedAsync()
    {
        context = await DbFactory.CreateDbContext();
    }

    public async ValueTask DisposeAsync() => await context.DisposeAsync();
}

[guardrex]

... and can the updates be backported (PLZ 🙏) to 8.0? I'd ❤️ to avoid versioning right out of the gate with the brand new tutorial. I think the answer will be 'yes' because the current code for 8.0 isn't correct.

Also note that (thus far) we're going with the word "context" for the db context in the EF Core-Blazor doc. Jeremy has them as properties tho ... not fields. Here's an example btw ...

https://github.com/dotnet/blazor-samples/blob/main/8.0/BlazorWebAppEFCore/Components/Pages/EditContact.razor

... and he isn't doing async init and disposal, but I'm not sure why. I need to know if what I'm angling on doing with async is a stinky 🦖💩👃 ... which wouldn't be surprising 🙈.

[danroth27]

@guardrex Good questions. I think using a context field is fine. As for going async, if we do that then we also need to handle the case where OnInitializedAsync returns but the Task hasn't completed yet 😝.

@JeremyLikness I think we need guidance from the EF Core team on whether to use sync/async here.

@javiercn I'd like to get your review and thoughts on this code as well.

[guardrex]

handle the case where OnInitializedAsync returns but the Task hasn't completed yet

I was hoping that since the items are bound that the grid would load ✨ automagically ✨ when the Task completes. If only life were that easy! 😄

Noting here to make sure it doesn't get lost WRT the incorrect guidance at ...

https://learn.microsoft.com/en-us/aspnet/core/blazor/components/quickgrid?view=aspnetcore-8.0&tabs=visual-studio#entity-framework-core-ef-core-data-source

You guys will want to update that in the QG sample content, too. The doc content came from here ...

https://aspnet.github.io/quickgridsamples/datasources/

... in the Entity Framework IQueryable data section.

[danroth27]

The guidance from @ajcvickers is to use sync CreateDbContext and async DisposeAsync.

So, I think that means this is the pattern we want to go with:

@inject IDbContextFactory<MoviesAppContext> DbFactory
@implements IAsyncDisposable
...
<QuickGrid Class="table" Items="context.{MODEL}">
...
@code {
    MoviesAppContext context = default!;

    protected override void OnInitialized()
    {
        context = DbFactory.CreateDbContext();
    }

    public async ValueTask DisposeAsync() => await context.DisposeAsync();
}

@deepchoudhery Can we get a quick turnaround on this one?

[guardrex]

BTW @danroth27 and @deepchoudhery ...

• Can the context be a field?
• Can it be private to match the docs convention?

private {CONTEXT} context = default!;

[danroth27]

Can the context be a field?

Oh, yes, sorry about that. Updated above.

Can it be private to match the docs convention?

I think the coding style should ideally be controlled by the developer. @deepchoudhery Does scaffolding honor the editor config coding style settings?

[guardrex]

I can remark on it in the tutorial. To see it without the access mod and then all other docs including the access mod, it's confusing if I don't remark on it.

[guardrex]

Conversely, we could have a long-range goal of dropping the private access mod from Blazor docs and samples.

[danroth27]

Conversely, we could have a long-range goal of dropping the private access mod from Blazor docs and samples.

I think we're free to use in the docs whatever coding style we think is most appropriate (ideally aligned with the default coding style used in our products). We just can't assume that coding style will be the same as the one the user decided to use.

[guardrex]

Leaving an additional note here that I think a using-scoped context was to be avoided. If so, the other CRUD components will move away from it ...

using var context = DbFactory.CreateDbContext();

🤔 Actually, I'm not sure now. I know you left them on the last round of tutorial updates. I don't recall if these cases where a QuickGrid isn't in use if the using-scope will be ok.

[danroth27]

🤔 Actually, I'm not sure now. I know you left them on the last round of tutorial updates. I don't recall if these cases where a QuickGrid isn't in use if the using-scope will be ok.

I think I checked all the other places where a using-scoped context was used, and they seemed fine. Only the QuickGrid case seemed to require a longer lifetime for the context.

[deepchoudhery]

Closing since addressed