fluentmigrator
fluentmigrator copied to clipboard
Separate dependency injection for framework and migrations
When using third party dependency injection frameworks together with .NET Core, you'll end up with two IOC containers. One provided by Microsoft for framework dependencies, and one for the application dependencies. I'll link to some articles of why that is: https://simpleinjector.readthedocs.io/en/latest/servicecollectionintegration.html https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-3.0#default-service-container-replacement
Anyway, it would be nice to be able to explicitly set the implementation of IServiceProvider for the migrations dependency injection. One could argue that the FluentMigration framework IOC registrations belong to the framework of the application, and that the migrations, which are custom written by the integrator, belongs to the applications IOC registrations, and therefor it makes sense being able to separate them.
It is doable already, but the solution becomes quite hacky and dependable on that the registration of the MigrationSource is done accordingly to FluentMigratorServiceCollectionExtensions.AddFluentMigratorCore.
internal static class MigrationRunnerBuilderExtensions
{
internal static IMigrationRunnerBuilder UseSimpleInjectorForMigrationDependencies(
this IMigrationRunnerBuilder migrationRunnerBuilder,
Container container)
{
// In order to use simple injector as dependency resolver, the migration source needs to be overridden.
// The overridden registration can be found here:
// https://github.com/fluentmigrator/fluentmigrator/blob/ab8e602b2a84f529812ef10791bed4740522d2ed/src/FluentMigrator.Runner/FluentMigratorServiceCollectionExtensions.cs#L89
migrationRunnerBuilder.Services.Replace(new ServiceDescriptor(typeof(IMigrationSource),
serviceProvider => new MigrationSource(
serviceProvider.GetRequiredService<IAssemblySource>(),
serviceProvider.GetRequiredService<IMigrationRunnerConventions>(),
container,
serviceProvider.GetServices<IMigrationSourceItem>()), ServiceLifetime.Scoped));
return migrationRunnerBuilder;
}
@Fresa This is thought-provoking. If I understand you correctly, the same issue was just reported for StructureMap by @kYann in #1064 - we discussed this yesterday in the gitter FluentMigrator/FluentMigrator channel. In his case, he assumed that the problem was StructureMap was selecting the wrong constructor since the constructor it was complaining about had an [Obsolete]
attribute: It's easy to see why he might think StructureMap is using the "wrong constructor" as opposed to simply not being able to locate the dependencies to any constructor. In this sense, I think you found the root cause of #1064. Please read through and let me know your thoughts.
Regarding a fix: Give me a few days - I need to read through the links you provided to fully grok this, as I don't think I've encountered this pain point except through @kYann reporting a similar issue. (We frankly don't use DI in FluentMigrator at work - we just use the MSBuild and CommandLine packages at work in order to get "Red blood" in the terminal when things fail as part of a BuildPipeline target that does: Clean, Restore, Build, Drop Existing UnitTest Db, Create New UnitTest Db, Run Migrations against UnitTest Db, Run Unit Tests, Pack, Publish)
My general hunch is your proposal may be better than what I suggested @kYann do in the gitter channel.
I don't believe it's the same issue. I'm no expert in StructureMap, but it seems they have taken the approach to completely replace the builtin container (and seems to have trouble filtering out obsolete constructors), while SimpleInjector cross wires it to be able to pull registrations from the builtin container into SimpleInjector when needed. You can read all about it in the link I posted, and if you want to understand exactly why they took that road there is a good article here.
So to summarize; the problem is that while SimpleInjector can resolve all the FluentMigrator framework components, MigrationSource expects all migrations dependencies to be registered in the builtin one, which is not SimpleInjector, so it fails creating the migration classes.
Thank you, that was a very helpful summary. Sorry for my spastic reply. It takes time going through the daily churn of new issues on this project and sometimes I make inaccurate first guesses as to what the problem is.
Just thinking... Would it be sufficient to just create an overload for AddFluentMigratorCore that takes an IServiceProvider parameter?
I haven't encountered this problem before so I'm not sure on the best approach to handle it. Originally, you worded the issue as just being MigrationSource, but I think the more general problem is how do you let your container take-over completely. Right?
I think framework dependencies and application dependencies should be separated. If you haven't already, you should definitely read the articles from SimpleInjector explaining why they have separated framework/third party dependencies from application dependencies. I think their conclusions make sense. I would also not be surprised if there are more DI frameworks that have similar designs.
If you added an overload to AddFluentMigratorCore with another IServiceProvider, would it not be a bit confusing for the integrator considering AddFluentMigratorCore is an extension method on the builtin DI registrator?
I read it, but I need time to think. I respect SimpleInjector authors.
My initial thoughts are: I thought by Microsoft DI existing and ASP.NET Core eliminating static state, we had gotten rid of "DI tunneling" issues (e.g., recording CreatedBy on a EF object using ASP.NET Identity in ASP.NET Classic required tunneling because ASP.NET threading model doesn't gaurantee you'll be on the same thread throughout a request's lifecycle, and if you wanted to add EF objects to a database in parallel, you had to use multiple threads, which broke the ASP.NET Classic Identity chain). After reading the SimpleInjector article, it seems it merely pushed some tunneling up a level. It's cleaner, but still exists. As best as I can tell, it has to exist.
The remaining questions are likely around this piece of the article:
Simple Injector’s auto cross wiring has the following limitations:
finally:
If you added an overload to AddFluentMigratorCore with another IServiceProvider, would it not be a bit confusing for the integrator considering AddFluentMigratorCore is an extension method on the builtin DI registrator?
I think the general intent is to do something similar to UseSimpleInjector. In that sense, I guess we come full circle and I'm starting to see how your "hack" code above can be cleaned up a bit and generalized.
Note to self:
It's possible Migrations should be loaded via AssemblyLoadContext as "plug-ins" such that if you want to keep all Migrations from ground zero in a greenfield project and never practice "baselining" your database, you can have one Migration Assembly which references EFCore 2.2 and another Migration Assembly which references EFCore 3.0.
This is in theory possible to do, especially once this issue is resolved.
It would also be nice to just read .deps.json in the currently executing assembly's folder and gather those instead of passing the assemblies on the command line. These ambient project references then become loaded in situ through deps.json rather than specifying anything explicitly. The negative would be that there is no way to control loading order of assemblies into the AssemblyLoadContext, unless you wrote an assembly which implements IAssemblySource and had that stitch together the how to load migrations in situ.
Just a brain dump....
@Fresa @fubar-coder Is the real issue that we're calling .BuildServiceProvider(false);
, rather than implementing IHostedService
for the core logic and letting the HostBuilder
instantiate the service provider? The IHostedService can then bootstrap its own container and be fed any Microsoft dependencies it needs, right?