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

Update to Reduce magic in the interaction of EF Core and DI #4750

Closed
wants to merge 1 commit into from

Conversation

ajcvickers
Copy link
Contributor

See issue #4668. This is an update following a discussion yesterday that moves all configuration onto DbContextOptions and makes this the only object passed to the DbContext constructor. This allows the builder pattern that already exists for DbContextOptions to be used for external services and for setting the internal service provider. AddDbContext does this automatically for external services, but a DbContextOptions object can also be easily built and either configured in DI or passed directly to the context with new.

In addition, the UseMyProvider methods have been updated to use nested closure for provider-specific configuration. This makes for a very nice chaining story.

There is a new overload of AddDbContext that makes the application service provider available to teh configuration delegate. This is rarely needed, but one use is to set the application service provider to be used as the internal EF service provider if this is needed/desired.

Still to do:

  • Look at negative cases when services are not setup and create better exception messages.
  • Rename AddSqlServer, etc to AddEntityFrameworkSqlServer, etc.
  • Extract diagnostics as one of the external services on DbContextOptions.

See issue #4668. This is an update following a discussion yesterday that moves all configuration onto DbContextOptions and makes this the only object passed to the DbContext constructor. This allows the builder pattern that already exists for DbContextOptions to be used for external services and for setting the internal service provider. AddDbContext does this automatically for external services, but a DbContextOptions object can also be easily built and either configured in DI or passed directly to the context with new.

In addition, the UseMyProvider methods have been updated to use nested closure for provider-specific configuration. This makes for a very nice chaining story.

There is a new overload of AddDbContext that makes the application service provider available to teh configuration delegate. This is rarely needed, but one use is to set the application service provider to be used as the internal EF service provider if this is needed/desired.

Still to do:
- Look at negative cases when services are not setup and create better exception messages.
- Rename AddSqlServer, etc to AddEntityFrameworkSqlServer, etc.
- Extract diagnostics as one of the external services on DbContextOptions.
/// <returns> The options builder so that further configuration can be chained. </returns>
public static DbContextOptionsBuilder UseSqlServer(
[NotNull] this DbContextOptionsBuilder optionsBuilder,
[NotNull] string connectionString,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this just happen in the action now? Or, is this the pattern for required config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. This is the pattern for required config.

@anpete
Copy link
Contributor

anpete commented Mar 10, 2016

Question: Do we still need the DbContextOptions/Builder that are generic on TContext? What are they used for?

@ajcvickers
Copy link
Contributor Author

Creating DbContextOptions that are specific to a given context type such that an application can have multiple context types registered in D.I. and have each one resolve the correct options.

@anpete
Copy link
Contributor

anpete commented Mar 10, 2016

:shipit: 🎉

@ajcvickers
Copy link
Contributor Author

Merged

@Grinderofl
Copy link
Contributor

Referencing #4784

This change has caused application service provider resolution to fail when adding plain DbContext (rather than subclassed one) because the default constructor is preferred. May be for DependencyInjection

ajcvickers added a commit that referenced this pull request Mar 22, 2016
Because when people use the same service provider for both the application and our internal services and don't create a derived DbContext class, then the two registrations can often collide. See #4750.

The fix is to use the type ICurrentDbContext internally which wraps the context for the current scope.
@bricelam bricelam deleted the ChuggyChogger0308 branch April 4, 2016 16:21
@rmja
Copy link

rmja commented Apr 7, 2016

@rowanmiller One could argue that this change should be emphasized on the https://github.com/aspnet/Announcements repo, as it breaks code when one resolves the db provider in AddDbContext<T>() like this:

services.AddDbContext<MyContext>(options =>
{
        options.UseSqlServer(connectionString);
});

As MyContext now requires a constructor similar to this one:

public MyContext(DbContextOptions options)
        : base(options)
{
}

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

Successfully merging this pull request may close these issues.

5 participants