Register a scoped DbContext automatically when using AddDbContextFactory

[ajcvickers]

Fixes #25164

Results from design investigation:

• AddDbContext registers DbContextOptions as scoped by default. This cannot be consumed from the default singleton factory. Changing the lifetime of DbContextOptions is a breaking change. This prevents us from adding the factory automatically, since it will not work correctly in the default/normal case.
• AddDbContextFactory can automatically add a scoped DbContext registration. This will depend on the singleton options added with the factory, which will work. This is the change we will make.
• Applications that explicitly register a DbContext as well as using AddDbContextFactory will not be impacted, since we always TryAdd in our methods.
• Applications that are already calling AddDbContextFactory first, and then also AddDbContext, would have the registration made by AddDbContext ignored, since AddDbContext uses TryAdd. This would be breaking for these applications. Therefore, if AddDbContextFactory has been called, and then AddDbContext is also called, AddDbContext will replace the registration made by AddDbContextFactory. This could also be breaking in corner cases, but is the best we can do if we want to implement this at all.

[roji]

AddDbContext registers DbContextOptions as scoped by default. This cannot be consumed from the default singleton factory. Changing the lifetime of DbContextOptions is a breaking change. This prevents us from adding the factory automatically, since it will not work correctly in the default/normal case.

Is there a downside to registering a factory as scoped in this case (in other words, always register the factory with the scope of the options)? This would ensure that the factory is consistently available in DI, without an extra gesture.

[roji]

Maybe have two lifetime parameters, one for the factory and one for the scope?

[ajcvickers]

Yes. The default is singleton. Changing the default to scoped would be very breaking. Only registering the context in some cases, depending on the scope of the factory, would be very convoluted.

[roji]

I wasn't suggesting to change any default - just to add a 2nd lifetime parameter to AddDbContextFactory, to allow users to explicitly control scoped vs. transient

[ajcvickers]

How does that help?

[ajcvickers]

Note: if there is significant pushback on this, then I propose we punt on doing anything for 6.0. At the moment, @roji has requested controversial changes, which I am not willing to make in 6.0.

[roji]

Not pushing back or anything (at least not yet :)), just trying to understand why not register a scoped factory from AddDbContext. Definitely not proposing to change the default scope in which we register options etc.

[ajcvickers]

Because scoped factory is almost always wrong. It can't be resolved correctly from the root provider, and so would require creating of a scope to use the factory.

[roji]

Because scoped factory is almost always wrong.

OK, I guess this is where we have a gap - I'd be happy to understand this better...

Unless I'm missing something, from e.g. the ASP.NET controller perspective there's very little difference: whether the injected factory is scoped or singleton doesn't change anything functionally: you can use it to create DbContext instances just the same. There's only the slight perf downside of creating the factory each time (though we already have that for the options IIRC). Scoped factories are also necessary for e.g. multi-tenant scenarios, so they should be OK to use even in non-multi-tenant scenarios, no?

In other words, I'm looking for how having the scoped factory hurts users in some way (aside from the perf thing).

It can't be resolved correctly from the root provider

Right, but neither can the options right now - how is that concretely a problem?

[roji]

@davidfowl wrote:

It wouldn't be possible to inject it into a singleton then

Sure, but then just explicitly set the options' lifetime to singleton, if that's what you need. Why limit people who don't need singleton injection from just using a scoped factory directly?

(also, as I wrote above, we already register the options as scoped, creating the same issue there)

[davidfowl]

I'm not sure why the options are scoped in the first place? Why aren't they singleton? Is it because of how EF uses scopes internally?

Sure, but then just explicitly set the options' lifetime to singleton, if that's what you need. Why limit people who don't need singleton injection from just using a scoped factory directly?

That should be the default. I don't have a pushback against making the lifetime optional.

[roji]

@davidfowl this was done in #8797 (to the best of my understanding to support options depending on other scoped things, i.e. explicitly-specified connections); as @ajcvickers wrote above, changing that would be quite a breaking change.

[ajcvickers]

First, I don't think we should change the services we add based on what lifetimes are provided. If we want to start doing that, then that's a discussion we should have after 6.0. I think it would make things very hard to understand.

Second, using the default lifetimes (by not passing any lifetime parameters) should not break. If we want to start doing that, then that's a discussion we should have after 6.0.

Third, people currently calling both methods should not break. If we want to start doing that, then that's a discussion we should have after 6.0.

By default AddDbContextFactory adds the factory as singleton, and adds the options as singleton. This means it can be resolved from the root provider. Changing this default would require applications that currently don't use a scope to start using a scope. This is way breaking.

By default AddDbContext adds the options as scoped. This is so that the options are re-evaluated for each application scope--typically a request. This allows people to configure a context differently depending on configuration on the current request. If you don't need to do this, then registering the options as singleton is better. But it isn't the default, and changing the default is breaking.

[roji]

@ajcvickers we may have a disconnect - I don't think I'm proposing any of the above things. The only thing I'm proposing is that AddDbContext (always) register a factory with the same lifetime as the options is registers, that's all (so scoped by default). I'm not proposing to change AddDbContextFactory or to change any default lifetime anywhere.

[roji]

(note: I agree we'd need to consider what happens when users call both AddDbContext and AddDbContextFactory and not break (too much), but I'm leaving that aside for the moment, just to understand the larger question)

[ajcvickers]

@roji But how does it help to a convenience method that does something both unexpected and not convenient? Also, should it only do this if you haven't already registered the factory as singleton?

In other words, we're trying to make a change that makes it easier for people to do things right. Instead, this seems like a change that would make it easier for people to do things wrong.

[roji]

But how does it help to a convenience method that does something both unexpected and not convenient?

I'm simply thinking about applications which need both directly-injected DbContexts and factories in different places (e.g. since they occasionally need multiple context instances). What I'm proposing would allow those scenarios to just specify AddDbContext, and everything works.

Re unexpected, I don't think registering a factory from AddDbContext is any more unexpected than registering a DbContext from AddDbContextFactory... Had we designed this from scratch, I probably would have advocated for having just a single method always registering both, but that boat has sailed.

Re not convenient, how so? This would only make the factory available in DI in case the user wishes to get injected with one. If user code doesn't request a factory, they'd never even know about it...

In other words, we're trying to make a change that makes it easier for people to do things right. Instead, this seems like a change that would make it easier for people to do things wrong.

I don't get this - what is wrong about a scoped factory? Doesn't it produce valid DbContext instances which can be used, just like the DbContext instances produced from singleton factories? How would users be hurt here?

[davidfowl]

The scoped factory breaks one of the mainline scenarios of injecting the factory into a hosted service (which is a singleton).

[roji]

@davidfowl I think two orthogonal things are being conflated.

You're proposing changing the default options lifetime to be singleton, so that it (and the corresponding factory) could be injected to singleton services (e.g. hosted service). That's a breaking change which we can consider - I'm not currently discussing that.

Regardless of whether we do that or not, I'm proposing that AddDbContext register a factory with the same lifecycle as the options it registers; this doesn't necessarily mean scoped, though that is the current default. If the user explicitly specifies singleton as the options lifecycle (which they can do today), my proposal would automatically make available a singleton factory as well, making that work with hosted services. Similarly, if we end up doing your breaking proposal above, that would mean that a singleton factory is by default available.

In other words, whether we register a factory from AddDbContext doesn't hurt or break injecting the factory into a hosted service; that's a matter of options lifecycle.

[roji]

Thanks to an offline conversation with @ajcvickers (those are useful), I realized that DI by default does allow resolving scoped services from a singleton service (unless scope vaildation is on), and this is the pit of failure to be avoided. However, according to the ASP.NET documentation:

Scoped services are disposed by the container that created them. If a scoped service is created in the root container, the service's lifetime is effectively promoted to singleton because it's only disposed by the root container when app/server is shut down. Validating service scopes catches these situations when BuildServiceProvider is called.

So if I understand everything correctly, having AddDbContext register the factory (as scoped by default) would make it available to singleton services. It's somewhat odd, but I'm not seeing any actual downside - the end result seems quite similar to explicitly registering the factory as singleton. The advantage would be to make the factory implicitly available - to controllers, singleton services, anyone - without having to discover AddDbContextFactory. @davidfowl do you see any issues with this?

See below for a sample where a scoped factory is resolved from a singleton.

Scoped factory sample

var serviceCollection = new ServiceCollection();

serviceCollection.AddDbContextFactory<BlogContext>(
    o => o.UseSqlServer(@"Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0"),
    ServiceLifetime.Scoped);

serviceCollection.AddSingleton<SomeSingletonService>();

var rootServiceProvider = serviceCollection.BuildServiceProvider();
var someSingletonService = rootServiceProvider.GetRequiredService<SomeSingletonService>();
someSingletonService.DoStuff();

public class SomeSingletonService
{
    private readonly IDbContextFactory<BlogContext> _factory;

    public SomeSingletonService(IDbContextFactory<BlogContext> factory)
        => _factory = factory;

    public void DoStuff()
    {
        using var ctx = _factory.CreateDbContext();
        _ = ctx.Blogs.ToList();
    }
}

public class BlogContext : DbContext
{
    public BlogContext(DbContextOptions<BlogContext> options) : base(options) {}

    public DbSet<Blog> Blogs { get; set; }
}

public class Blog
{
    public int Id { get; set; }
    public string Name { get; set; }
}

[ajcvickers]

Note from triage: merging this as-is for now. @roji will continue follow up after merge.

[davidfowl]

Change this code:

var rootServiceProvider = serviceCollection.BuildServiceProvider();

To this code:

var rootServiceProvider = serviceCollection.BuildServiceProvider(new ServiceProviderOptions { ValidateScopes = true  });

[roji]

@davidfowl right - that will throw. So users has ValidateScopes will get the exception - if they try to resolve the scoped factory from a singleton (resolving from a controller would work since it's all scoped - that's the point of what I'm proposing). Users without ValidateScopes (the default) would just get a functioning factory in all cases, no? Where do you see the problem?

[davidfowl]

@davidfowl right - that will throw. So users has ValidateScopes will get the exception - if they try to resolve the scoped factory from a singleton (resolving from a controller would work since it's all scoped - that's the point of what I'm proposing). Users without ValidateScopes (the default) would just get a functioning factory in all cases, no? Where do you see the problem?

ValidateScopes is default when using the generic host.

[roji]

ValidateScopes is default when using the generic host.

Then once again registering a scoped factory doesn't matter either way...

• If the user uses AddDbContextFactory, everything just works regardless of anything, since it registers a singleton factory.
• If the user uses AddDbContext today (with or without this PR as-is), trying to inject a factory to a hosted service will fail simply because no factory is registered.
• If we do what I propose (implicitly register a scoped factory from AddDbContext), then the same will throw because of scope validation. At that point the user has to use AddDbContextFactory in any case.

So regardless of my proposal, it fails for hosted services with AddDbContext, and works with AddDbContextFactory. I'm just proposing something that will make life easier for the controller (scoped) scenario.

But it seems like we've been talking a lot about this and I'm not managing to make myself understood... Am OK if you guys just want to drop this.

[davidfowl]

@roji I see, that makes sense but I don't know who would use it? Is it for creating multiple dbcontexts within a request?

[roji]

Is it for creating multiple dbcontexts within a request?

Yeah, this would be the primary scenario. This can be needed when you want to have multiple queries running at the same time, e.g. for perf. I'm basically saying that I don't see a reason not to enable this by default, that's all.