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

Inheritance problem with DbContext subclasses requiring constructor to supply DbContextOptions<DerivedContext> #7533

Closed
Ettery opened this issue Feb 2, 2017 · 28 comments
Labels
area-dbcontext closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@Ettery
Copy link

Ettery commented Feb 2, 2017

I am building web APIs in ASP.NET Core 1.1.

I have a number different databases (for different systems) which have common base schemas for configuration items such as Configuration, Users and groups (about 25 tables in all). I am trying to avoid duplicating the quite extensive EF model configuration for the shared part of the model by inheriting from a base class.

     InvestContext     BatchContext
             ^               ^
             |               |
             _________________
                     |      
            ConfigurationContext
                     ^
                     |      
                 DbContext

However, this does not work because of the Entity Framework (EF) requirement to pass DbContextOptions<DerivedContext> as a parameter to the constructor, where DerivedContext must match the type of the context the constructor is called on. The parameter must then be passed down to the base DbContext by calling :base(param).

So when (for example) InvestContext is initialised with DbContextOptions<InvestContext>, it calls base(DbContextOptions<InvestContext>) and EF throws an error because the call to the ConfigurationContext constructor is receiving a parameter of type DbContextOptions<InvestContext> instead of the required type DbContextOptions<ConfigurationContext>.

I can work around this by building a new DbContextOptions<ConfigurationContext> from the DbContextOptions<InvestContext> object and passing that to base, but it seems like a hack and results in DbContext being initialised with DbContextOptions<ConfigurationContext> for any context which inherits from ConfigurationContext.

So I guess I'm asking:

  1. Is that hack I've described a problem, or can I use it (ie. will there be unexpected effect of DbContext receiving DbContextOptions<ConfigurationContext> when InvestContext is constructed?
  2. Is this limitation on inheritance intended, or just a side effect of the design choice requiring DbContextOptions<DerivedContext>
  3. Is there perhaps a better way to do this which would allow for a clean inheritance model?

Thanks, and thanks for all the good work - Peter

@ajcvickers
Copy link
Member

Note for triage: we should update the check to allow TContext to be a type that is derived from the current context type.

@Ettery
Copy link
Author

Ettery commented Feb 7, 2017

@ajcvickers that seems like a good solution, thanks. In the meantime, do you know whether my workaround of building a new DbContextOptions of the type required by the base class an acceptable way to use inheritance for now, or it is likely to cause problems?

@ajcvickers
Copy link
Member

@Ettery I don't think that will cause problems.

@Ettery
Copy link
Author

Ettery commented Feb 8, 2017

@ajcvickers - Hmmm, digging into the EF code, it looks like the ContextType of the DbContextOptions may be used in ContextPooling and other services? And if it's not being used, why is EF enforcing it? I think my workaround may cause some strange problems, and the fix you have suggested is really important.

@ajcvickers
Copy link
Member

@Ettery I don't think there will be any issues. Context pooling requires that the options passed to the context instance are not modified in OnConfiguring. Also, context pooling is only used if you explicit configure it to be used. Can you point to some specific places in the code where you think there will be a problem?

The reason for the check is because we found that when code registers multiple contexts in D.I. but then uses a constructor that depends on the non-generic DbContext it was really hard to figure out why one of the contexts was using the wrong options. So we want to make sure that the options resolved from D.I. is really for the context being used.

@Ettery
Copy link
Author

Ettery commented Feb 9, 2017

@ajcvickers thankyou, I think you are right, I picked up that TContext was being used in the pooling, but thats not coming from the DbContextOptions and I can't find anywhere there the TContext on DbContextOptions is used other than that initial check. Thanks for your help, I'll use my 'hack' for now. So great to have the code to work through, and thanks for all your work.

@ajcvickers
Copy link
Member

@Ettery I have been trying to reproduce this but have not been able to. The code already allows the TContext generic type to be of a derived DbContext type--see test code below. Can you post the exact exception message and stack trace you are seeing?

public class Program
{
    public static void Main()
    {
        var appServiceProivder = new ServiceCollection()
            .AddDbContext<DerivedContext2>()
            .BuildServiceProvider();

        using (var serviceScope = appServiceProivder
            .GetRequiredService<IServiceScopeFactory>()
            .CreateScope())
        {
            var context = serviceScope.ServiceProvider.GetService<DerivedContext2>();

            Debug.Assert(context.GetService<IDbContextOptions>().GetType() == typeof(DbContextOptions<DerivedContext2>));
        }
    }
}

public class DerivedContext1 : DbContext
{
    public DerivedContext1(DbContextOptions options)
        : base(options)
    {
    }
}

public class DerivedContext2 : DerivedContext1
{
    public DerivedContext2(DbContextOptions<DerivedContext2> options)
        : base(options)
    {
    }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder.UseInMemoryDatabase(nameof(DerivedContext2));
}

@ajcvickers
Copy link
Member

EF Team Triage: Closing this issue as the requested additional details have not been provided and we have been unable to reproduce it.

@ajcvickers ajcvickers removed this from the 2.0.0 milestone Mar 2, 2017
@theCuriousOne
Copy link

@ajcvickers I am momentarily facing the same issue. In you example you need to have

DbContextOptions

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

and the the compiler will complain

@ajcvickers
Copy link
Member

@theCuriousOne When you say "you need to have" can you be more specific? Need to have for what reason?

@theCuriousOne
Copy link

@ajcvickers I meant "you need to have" in order to reproduce the error. My current issue is that I need to extend an DbContext with exactly that kind of code.

@ajcvickers
Copy link
Member

@theCuriousOne I'm curious as to why you need to use that kind of code--I can see it would be problematic, and I can see some scenarios where it might be needed, but I am curious what your scenarios are so that we can make an assessment of the severity of the issue and react appropriately in finding workaround and considering ways to fix.

@theCuriousOne
Copy link

theCuriousOne commented Nov 28, 2017

@ajcvickers There is a framework build with dotnet core, and I need to extend upon it. And do not know how to do it otherwise.

@ajcvickers
Copy link
Member

@theCuriousOne I see; so the DerivedContext1 is code that you cannot change? I'll re-open this issue to consider what to do.

@sam-wheat
Copy link

@ajcvickers What is the specific reason that DbContextOptions<T> is required for the derived class but the base only receives DbContextOptions?

@ajcvickers
Copy link
Member

@sam-wheat It's to do with the way the Microsoft.Extensions.DependencyInjection works. AddDbContext registers DbContextOptions<MyDbContext> which is then resolved in the MyDbContext constructor by declaring a DbContextOptions<MyDbContext> parameter. This allows AddDbContext to be called for multiple different context types while still allowing them to be resolved by D.I. correctly. If only the non-generic DbContextOptions was used for this, then resolution of the correct options for the correct context type would not work when AddDbContext is called multiple times.

This means that any context type resolved from D.I. should have a DbContextOptions<MyDbContext> parameter. However, if the context is only acting as a base class and not being resolved explicitly from D.I., then it can accept the non-generic DbContextOptions because there are no D.I. resolution issues.

The reason I re-opened this issue is because this pattern doesn't work well if the application needs to resolve both two derived context types and their common base context type from D.I.

@sam-wheat
Copy link

sam-wheat commented Dec 11, 2017

@ajcvickers I imagine the resolution matching should take place at the connection string level since an instance of DBContextOptions may theoretically be used for multiple contexts:

string connString1 = "Db1";
string connString2 = "Db2";

MSSQLOptions = new DBContextOptions(string connString) // UseSqlServer(...)

Db1: DbContext {}
Db2: DbContext {}

Both Db1 and Db2 should be able to accept MSSQLOptions.

MSSQLOptions = new DBContextOptions() // UseSqlServer(...)
MySQLOptions = new DBContextOptions() // UseMySQL(...)

Matching the connection string to the right DBContextOptions makes sense in this scenario also. Again, the DbContext does not need to know which type of DBContextOptions is passed in.
I suspect the team is designing EF around the Microsoft DI framework..... Last I looked at it, it is woefully inadequate for these types of resolution issues. I hope Microsoft.DI is not limiting the functionality of EF!

I know you are aware of AutoFac and resoultion using keys. I have actually written a nuget package around this functionality as it is very powerful and useful in cases such as the one we are discussing now.

@ajcvickers
Copy link
Member

@sam-wheat If a different D.I. framework is being used, then it should be possible to set it up any way you want so long as it resolves the context with the options you want. However, you'll likely have to write some custom code to do this, since AddDbContext (which is what this is for) is written for use with the Microsoft D.I. abstractions and is limited to what those abstractions support. EF Core in general is not limited in that way from an application perspective, although is does also use the D.I. abstractions internally, but that is an implementation detail and isolated from context resolution by the application.

@greggbjensen
Copy link

greggbjensen commented Dec 22, 2017

I was able to resolve this without a hack by providing a protected constructor that uses DbContextOptions without any type. Making the second constructor protected ensures that it will not get used by DI.

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

    protected MainDbContext(DbContextOptions options)
        : base(options)
    {
    }
}

public class SubDbContext : MainDbContext
{
    public SubDbContext (DbContextOptions<SubDbContext> options)
        : base(options)
    {
    }
}

ajcvickers added a commit that referenced this issue Dec 28, 2017
Use both a public constructor for the typed options and a protected constructor for the un-typed options on a DbContext type that is intended to be both instantiated and inherited from in the same service provider.
@ajcvickers
Copy link
Member

@greggbjensen Thanks! I added a test for this case and I'm going to close this issue since that pattern looks pretty good to me.

@ajcvickers ajcvickers added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed type-investigation labels Dec 28, 2017
@theCuriousOne
Copy link

@greggbjensen Thank you for your input/help. I haven't tested it yet. According to your code, the pattern implies that the base class have to be change, which I believe it is not a very nice solution, but something I can live with :) I would suggest for this pattern to be added in some documentation (@ajcvickers ) since it is a guideline for building a class that needs to be overwritten.

@ajcvickers
Copy link
Member

@theCuriousOne Created dotnet/EntityFramework.Docs#594 to document this.

ajcvickers added a commit that referenced this issue Jan 2, 2018
Use both a public constructor for the typed options and a protected constructor for the un-typed options on a DbContext type that is intended to be both instantiated and inherited from in the same service provider.
@granthoff1107
Copy link

granthoff1107 commented Jan 31, 2018

@ajcvickers, this seems to have more side effect.
It appears if you reuse you DbContextOptions, It also reuses the Connection pool

Which would explain why I get Concurrency Issues in this issue
#10820
Switching to use @greggbjensen pattern seems to fix the issue, but I have not fully tested it

@sschleicher208
Copy link

I know this is old, but I ran into this issue as well. Many folks use scaffold-dbcontext to generate their context and then either use a derived dbcontext to perform all the decorating and other stuff that would otherwise get overridden on the next call to scaffold-dbcontext. Since scaffold-dbcontext generates the constructor that causes the issue, we continually would have to add another constructor to it so that we can use our derived dbcontext without errors each time.

@simeyla
Copy link

simeyla commented Mar 24, 2019

@sschleicher208 If you're using scaffold-dbcontext you can just add any additional constructors or 'decorating' in a partial class.

But if you're using connection pooling you'll actually get an error if you have two constructors (which the scaffolder created). So to avoid manually editing the code I'm generating my scaffolded class as XYZDBContextBase and then inheriting from that.

In fact I only need subclasses to allow me to set different options for different scenarios - such as longer timeouts for reporting but still be able to use pooling and DI.

@sschleicher208
Copy link

@simeyla That's what I used to do as well, but then I came across an issue when I had to provide additional code in the OnModelCreating method to add a few custom Query() methods based on views in the database along with some special annotations. This caused me to go down the derived context route so that I could override that method to provide what I needed since partial classes didn't allow me to do that (at least not for me).

@bar-codeoasis
Copy link

rename your context from scaffold to name + "FromScaffold"
and use the old name to fix the problem
for example
old name = "MyContext"
new name = "MyContextFromScaffold"
and leave all the inject ext be the same, with MyContext

public class MyContext : MyContextFromScaffold { public satwirContext(DbContextOptions<MyContext> options) : base(options) { } } public partial class MyContextFromScaffold { public MyContextFromScaffold(DbContextOptions options) : base(options) { } }

terry-delph added a commit to terry-delph/elsa-core that referenced this issue Jan 14, 2020
- Added new protected constructor based on following post
- dotnet/efcore#7533
sfmskywalker pushed a commit to elsa-workflows/elsa-core that referenced this issue Jan 14, 2020
* Added MySqlContext
* Added MySqlContextFactory
* Added Pomelo.EntityFrameworkCore.MySql nuget package
* Added initial MySqlContext migrations

* Added support for creating derived ElsaContexts:
- Added new protected constructor based on following post
- dotnet/efcore#7533

* Added Sample 24 - MySql provider with migration execution
* Updated other samples that needed to define the derived ElsaContext

* Added summary to ElsaContext constructor

* Updated persistence/Elsa.Persistence.EntityFrameworkCore MD file with MySql example
@n-ate
Copy link

n-ate commented Jul 30, 2021

If the reason you can't / do-not-want to alter the first inheritor of DbContext is because it is scaffolded then you're in luck. Here is a workaround:

  1. Make a partial of the generated class that inherits from DbContext. Let's call it DataContextBase.
  2. Add a new constructor to the partial like so:
    public DataContextBase(DbContextOptions options) : base(options) { }
  3. Now call the partial constructor via base from your derived class. Let's call it DataContext:
    public DataContext(DbContextOptions<DataContext>) : base(options) { }

Inheritance:

DbContext (1 source: nuget)
     🠗
partial DataContextBase (2 sources: generated, and manual)
     🠗
DataContext (1 source: manual)

@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dbcontext closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests