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

Need good explanation about D.I. usage in EF Core architecture #7267

Closed
rpokrovskij opened this issue Dec 18, 2016 · 2 comments
Closed

Need good explanation about D.I. usage in EF Core architecture #7267

rpokrovskij opened this issue Dec 18, 2016 · 2 comments
Labels
closed-no-further-action The issue is closed and no further action is planned.

Comments

@rpokrovskij
Copy link

rpokrovskij commented Dec 18, 2016

Do you have any explanation why you implement dependency injection for DbContext two different ways?

  1. Through constructor as DbContextOptions but not directly(!) throut options builder
  2. Through GetService().AddProvider(loggerProvider);
    I can't understand the logic behind of all those decisions.

You see, logger configuration currently should be done through GetService<ILoggerFactory>().AddProvider
At other hand sql generator is injected through DbContextOptionsBuilder...

  public class MyDbContext : DbContext
   {
       private static DbContextOptions<MyDbContext> CreateOptions(string connectionString)
       {
           var optionsBuilder = new DbContextOptionsBuilder<MyDbContext>();
           optionsBuilder.UseSqlite("Filename=./blog.db");
            //....
           var options = optionsBuilder.Options;
           return options;
       }
public MyDbContext(string connectionString, MyLoggerProvider loggerProvider)
           : base(CreateOptions(connectionString))
       {
           this.GetService<ILoggerFactory>().AddProvider(loggerProvider); // this is crazy
       }
}

Other questions

  1. Both DI are not interchangeable. I can't add loggerProvider through options and setup UseSqlite through GetService (setup to use sqlLite facade)

  2. What was wrong with simply passing ILoggerProvider through DbContext constructor since as I see Microsoft.EntityFrameworkCore depends on Microsoft.Extensions.Logging and can use ILoggerProvider directly?

  3. Why you need both "DbContextOptionsBuilder" and DbContextOptions... they are defined in the same assembly. Why not to add UseSqlite method to DbContextOptions directly?

It looks like overdesigned but may be I do not see hidden treasure of all of this?

@ajcvickers
Copy link
Member

@rpokrovskij DbContext is designed to be usable both as a service registered in a container and as type that can just be newed up and used. We wanted to keep the pattern for configuring EF as similar as possible for both situations, and as such it is always done through a DbContextOptionsBuilder. Typically an application makes calls to the builder either in AddDbContext when registering the context in a container, or in OnConfiguring when not using D.I.

I'm not sure what you are attempting to do with logging. When the context is resolved from a D,I. container, logging is typically configured on the D.I. container without the involvement of EF. This is usually pretty clean and doesn't require any calls to GetService. On the other hand, if you don't have a D.I. container, then hooking up a logger provider is not as clean and requires that you access EF's internal service provider with GetService. Making this better is partially tracked by issue #1199.

DbContextOptions objects are immutable. The builder helps maintain this immutability while also adding an extensible API for their construction.

I've done my best to answer your questions but I wasn't able to follow everything you are saying. You might want to read https://blog.oneunicorn.com/2016/10/24/ef-core-1-1-creating-dbcontext-instances/ and/or some of the other related posts on my blog to get a more in-depth view of how EF Core interacts with D.I.

@ajcvickers ajcvickers added the closed-no-further-action The issue is closed and no further action is planned. label Dec 19, 2016
@rpokrovskij
Copy link
Author

rpokrovskij commented Dec 20, 2016

I have read the article and... it doesn't calm me.

What is DI:

// dependency injection, fantasy code
ILogger logger = new MyLogger(); // ILogger  defined somewhere in EFCore 
ISqlFacade sqlFacade= new SqliteFacade(connectionString);  // ISqlFacade defined somewhere in EFCore 
var dbContext = new MyDbContext(logger , sqlFacade); // MyDbContext : DbContext  declared in separate assembly

I expect that base DbContext class constructor should accept both ILogger and ISqlFacade (and all other dependencies).

All other tricks - that constructor can accept DI container or dbContext can pick dependencies from service locator, or di container can be picked from service locator, etc. are inessential, are second-level additional superconstructions.

Class constructor has been a convenient way to declare dependencies, isn't it? Where they are in case of DbContext?

So I see that problem: instead of passing "ILogger" and "ISqlFacade" to DbContext (what is DI as it is) I should do really strange things: setup logger through service locator, and "SqlFacade" as a state of "option builder" (that generates stateless options.. hmm.. where is profit?).

          var optionsBuilder = new DbContextOptionsBuilder<MyDbContext>();
           optionsBuilder.UseSqlite("Filename=./blog.db");
           var options = optionsBuilder.Options;
           // send options to DbContext constructor

But if you say that one day logger will be configured from options builder - it calms me. At least DI will be consistent.

@rpokrovskij rpokrovskij changed the title Need good explanation about IoC pattern usage in EF Core architecture Need good explanation about D.I. usage in EF Core architecture Dec 20, 2016
@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned.
Projects
None yet
Development

No branches or pull requests

2 participants