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

IDbContextFactory async contract vs covariance #26630

Closed
MikeAmputer opened this issue Nov 11, 2021 · 9 comments
Closed

IDbContextFactory async contract vs covariance #26630

MikeAmputer opened this issue Nov 11, 2021 · 9 comments

Comments

@MikeAmputer
Copy link

Just a quick question. Was it worth it to add IDbContextFactory.CreateDbContextAsync in #24768 sacrificing the covariance?
I used to inherit sqlite contexts for tests in my solution. I will find the way to deal with the new contract, but I'm really curious.

@ajcvickers
Copy link
Member

ajcvickers commented Nov 16, 2021

@MikeAmputer In retrospect, it's possible we should not have added this; it might turn out to be YAGNI. Could you provide some more details on how you were using the covariance?

@MikeAmputer
Copy link
Author

MikeAmputer commented Nov 17, 2021

@ajcvickers thank you for your reply. Here is an example of covariance usage:

public class TransactionalContextFactory<TContext> : ITransactionalContextFactory<TContext>
    where TContext : DbContext
{
    protected readonly IDbContextFactory<TContext> ContextFactory;

    public TransactionalContextFactory(IDbContextFactory<TContext> contextFactory)
    {
        ContextFactory = contextFactory ?? throw new ArgumentNullException(nameof(contextFactory));
    }

    // ITransactionalContextFactory<TContext> implementation
    public virtual TransactionalContext<TContext> Create(...) { ... }
}
internal class TestTransactionalContextFactory<TContext, TContextStub> : TransactionalContextFactory<TContext>
    where TContext : DbContext
    where TContextStub : TContext
{
    public TestTransactionalContextFactory(IDbContextFactory<TContextStub> contextFactory)
        : base(contextFactory) { } // here

    // ITransactionalContextFactory<TContext> implementation override
}

DI consumer classes get ITransactionalContextFactory<ProductionContext> that builds a TransactionalContext<InMemSqliteContext> in tests (InMemSqliteContext inherits ProductionContext).

@ajcvickers
Copy link
Member

@MikeAmputer Thanks. We discussed, and while maybe we should not have added this API (time will tell), we don't think making a breaking change to remove it would be the best thing to do now.

@wertzui
Copy link
Contributor

wertzui commented Nov 29, 2021

@ajcvickers could you elaborate why the covariance needed to be removed for this feature to work?
Or would it be possible to reintroduce the covariance and still keep the async feature?

For me this breaks my library which basically adds a Set<TEntity>() method to IDbContextFactory<DbContext>
The signature is this

public static IQueryable<TEntity> Set<TEntity>(this IDbContextFactory<DbContext> contextFactory)
            where TEntity : class

That way, the consumers can call it like this

await factory.Set<MyEntity>().Where(...).Select(...).ToListAsync();

Because the covariance is gone now, I need to change the signature to this

public static IQueryable<TEntity> Set<TContext, TEntity>(this IDbContextFactory<TContext> contextFactory)
            where TContext: DbContext
            where TEntity : class

This forces the consumers to call it like this

await factory.Set<MyContext, MyEntity>().Where(...).Select(...).ToListAsync();

So they already have a IDbContextFactory<MyContext> and they need to repeat that generic type every time they call the Set method which seems redundant.

@ajcvickers
Copy link
Member

@wertzui Because async methods return Task<TContext>, not TContext.

@wertzui
Copy link
Contributor

wertzui commented Nov 29, 2021

Thanks!
However I would still vote to reintroduce the covariance :)

@georg-jung
Copy link

georg-jung commented Jan 17, 2022

I used the covariance too. It was usable in EF Core 5 but is removed with EF Core 6. If I haven't overlooked it it is not listed in the list of breaking changes though.

My use case (if I'm doing it wrong please point me in the right direction):

  • I have a solution that consists of some libraries and multiple executable entrypoints that share code by using the libraries (one windows desktop app, one console app that executes cron tasks)

  • One project contains the model, one contains my DbContext etc., one contains shared logic that works with a DbContext instance

  • My desktop project does dapper-like queries for types that make sense for some views (i.e. hold aggregated data) but aren't known to the database/aren't contained in the schema. Thus, the client project inherits the solution-wide context type ApplicationDbContext as ClientDbContext and register the "query types" in the model as keyless entity types. The client also consumes services from the logic library that depend on IDbContextFactory<ApplicationDbContext>. I can't get this kind of factory from DI now that the covariance is missing. I used to have this line in my Startupclass before:

    services.AddTransient<IDbContextFactory<ApplicationDbContext>>(sp => sp.GetRequiredService<IDbContextFactory<ClientDbContext>>());
    
  • Thus, upgrading from EF Core 5 to 6 is breaking for me. This change isn't listed as breaking though.

Am I doing anything wrong/cumbersome/...? I would have thought what I do - inheriting a common DbContext to specialize - would be rather typical.

@georg-jung
Copy link

georg-jung commented Jan 17, 2022

For others facing the same difficulties as I did, following workaround works for me:

I previously had in my DI config:

    services.AddTransient<IDbContextFactory<ApplicationDbContext>>(
        sp => sp.GetRequiredService<IDbContextFactory<ClientDbContext>>());

I created a helper factory class like this:

    internal class CovariantDbContextFactory<TContextOut, TContextIn>
        : IDbContextFactory<TContextOut>
        where TContextOut : DbContext
        where TContextIn : TContextOut
    {
        private readonly IDbContextFactory<TContextIn> _contextInFactory;

        public CovariantDbContextFactory(IDbContextFactory<TContextIn> contextInFactory)
        {
            _contextInFactory = contextInFactory;
        }

        public TContextOut CreateDbContext()
            => _contextInFactory.CreateDbContext();

        public async Task<TContextOut> CreateDbContextAsync(CancellationToken cancellationToken = default)
            => await _contextInFactory.CreateDbContextAsync(cancellationToken);
    }

And register it in DI:

	services.AddTransient<CovariantDbContextFactory<ApplicationDbContext, ClientDbContext>>();
	services.AddTransient<IDbContextFactory<ApplicationDbContext>>(
        sp => sp.GetRequiredService<CovariantDbContextFactory<ApplicationDbContext, ClientDbContext>>());

Now my old EF Core 5 code works without changes.

Anyone: If this approach has considerable flaws I'm overlooking, I am happy if you point me to it.

@PhilipBeinggs
Copy link

@georg-jung your elegant solution (OK, massive hack!) is another example of "there is no problem in computer science that can't be solved by using another layer of indirection" 😉

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

No branches or pull requests

5 participants