Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 9, 2026

Summary

This PR successfully addresses the issue where Elsa.Persistence.Dapper depends on FluentMigrator.Runner meta-package, which transitively pulls in all database provider packages (Db2, Oracle, PostgreSQL, SQL Server, Snowflake, etc.), significantly increasing application size and publish time.

Changes Made:

1. Core Library Changes (Elsa.Persistence.Dapper)

  • ✅ Replaced FluentMigrator.Runner with FluentMigrator.Runner.Core in project file
  • ✅ Removed convenience methods UseSqlite() and UseSqlServer() from DapperMigrationsFeature
  • ✅ Fixed pre-existing bug: Replaced System.Data.SqlClient with Microsoft.Data.SqlClient
  • ✅ Added comprehensive XML documentation to ConfigureRunner property

2. Package Management (Directory.Packages.props)

  • ✅ Added version entries for:
    • FluentMigrator.Runner.Core (7.2.0)
    • FluentMigrator.Runner.SqlServer (7.2.0)
    • FluentMigrator.Runner.SQLite (7.2.0)

3. Consuming Application Updates (Elsa.Server.Web)

  • ✅ Added explicit package references for database-specific runners
  • ✅ Updated migration configuration to use direct provider configuration
  • ✅ Refactored to reduce code duplication
  • ✅ Fixed lambda to not return value (Action should not return)

Benefits:

  • Reduced application size: No longer pulls in unused database provider packages (~75+ MB for some providers)
  • Faster build/publish times: Fewer dependencies to download and process
  • More explicit dependencies: Applications explicitly declare which database providers they use
  • Better flexibility: Consuming applications can choose any database provider supported by FluentMigrator

Breaking Change Notice:

⚠️ This is a breaking change for applications using DapperMigrationsFeature.UseSqlite() or UseSqlServer() methods.

Migration Guide for Consumers:

Before:

elsa.UseDapper(dapper =>
{
    dapper.UseMigrations(feature =>
    {
        feature.UseSqlServer(); // or feature.UseSqlite()
    });
});

After:

// 1. Add package reference to your .csproj:
<PackageReference Include="FluentMigrator.Runner.SqlServer" />
// or
<PackageReference Include="FluentMigrator.Runner.SQLite" />

// 2. Update configuration:
elsa.UseDapper(dapper =>
{
    dapper.UseMigrations(feature =>
    {
        feature.ConfigureRunner = builder =>
        {
            var runnerBuilder = useSqlServer 
                ? builder.AddSqlServer()
                : builder.AddSQLite();
            
            runnerBuilder
                .WithGlobalConnectionString(sp => sp.GetRequiredService<IDbConnectionProvider>().GetConnectionString())
                .WithMigrationsIn(typeof(Elsa.Persistence.Dapper.Migrations.Management.Initial).Assembly);
        };
    });
});

Testing:

  • ✅ Built successfully: Elsa.Persistence.Dapper project
  • ✅ Verified dependency changes: Only FluentMigrator.Runner.Core is now included
  • ✅ Built consuming application: Elsa.Server.Web workbench
  • ✅ Code review completed and all feedback addressed

Security Summary:

No security vulnerabilities were introduced by these changes. The changes are purely dependency management and do not affect runtime behavior or data handling.


Addresses issue #112 raised by the community regarding FluentMigrator.Runner meta-package pulling all providers unnecessarily.

Original prompt

This section details on the original issue you should resolve

<issue_title>Elsa.Dapper depends on FluentMigrator.Runner (pulls all providers)</issue_title>
<issue_description>I noticed that Elsa.Persistence.Dapper.csproj depends on FluentMigrator.Runner.

Since FluentMigrator.Runner is a meta-package, this causes all FluentMigrator database providers to be pulled transitively into consuming applications (Db2, Postgres, Oracle, SqlServer, Snowflake, etc.), even if they are not used.

One practical side effect is that large native dependencies are installed as well (e.g. DB2 clidriver ~75 MB), increasing application size and publish time.

From what I can tell, Elsa.Dapper only needs the core runner infrastructure, not database-specific providers.

Would it be possible to replace:

<PackageReference Include="FluentMigrator.Runner" />

with:

<PackageReference Include="FluentMigrator.Runner.Core" />

This would avoid pulling unused providers and let applications choose the database they actually need.
Thanks for maintaining Elsa and for considering this improvement 🙂
</issue_description>

Comments on the Issue (you are @copilot in this section)

@sfmskywalker That makes perfect sense 👍

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits January 9, 2026 21:59
- Changed Elsa.Persistence.Dapper to depend on FluentMigrator.Runner.Core instead of FluentMigrator.Runner (meta-package)
- This prevents pulling in all database provider packages (Db2, Oracle, Postgres, SqlServer, Snowflake, etc.)
- Removed UseSqlite() and UseSqlServer() convenience methods from DapperMigrationsFeature
- Updated consuming applications to add specific runner packages and configure directly
- Fixed pre-existing bug: replaced System.Data.SqlClient with Microsoft.Data.SqlClient package reference

Co-authored-by: sfmskywalker <[email protected]>
Copilot AI changed the title [WIP] Replace FluentMigrator.Runner with FluentMigrator.Runner.Core Replace FluentMigrator.Runner meta-package with FluentMigrator.Runner.Core Jan 9, 2026
Copilot AI requested a review from sfmskywalker January 9, 2026 22:09
@sfmskywalker sfmskywalker marked this pull request as ready for review January 10, 2026 06:11
Copilot AI review requested due to automatic review settings January 10, 2026 06:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses issue #112 by replacing the FluentMigrator.Runner meta-package with FluentMigrator.Runner.Core in the Elsa.Persistence.Dapper library to avoid pulling in all database provider dependencies (~75+ MB of unused native dependencies). This is a breaking change that requires consuming applications to explicitly declare which database providers they need.

Changes:

  • Replaced FluentMigrator.Runner with FluentMigrator.Runner.Core dependency in core library
  • Removed UseSqlite() and UseSqlServer() convenience methods from DapperMigrationsFeature
  • Updated consuming application to explicitly configure database providers via ConfigureRunner

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Directory.Packages.props Added FluentMigrator.Runner.Core, FluentMigrator.Runner.SqlServer, and FluentMigrator.Runner.SQLite package versions (7.2.0)
src/modules/persistence/Elsa.Persistence.Dapper/Elsa.Persistence.Dapper.csproj Replaced FluentMigrator.Runner with FluentMigrator.Runner.Core; replaced System.Data.SqlClient with Microsoft.Data.SqlClient
src/modules/persistence/Elsa.Persistence.Dapper/Features/DapperMigrationsFeature.cs Removed UseSqlite() and UseSqlServer() convenience methods
src/workbench/Elsa.Server.Web/Elsa.Server.Web.csproj Added explicit FluentMigrator.Runner.SqlServer and FluentMigrator.Runner.SQLite package references
src/workbench/Elsa.Server.Web/Program.cs Updated migration configuration to use ConfigureRunner with explicit database provider calls

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sfmskywalker
Copy link
Member

@copilot apply changes based on the comments in this thread and make sure everything builds and tests pass.

- Remove return statement from lambda in Program.cs (Action<T> should not return)
- Add comprehensive XML documentation to ConfigureRunner property explaining breaking change and migration path

Co-authored-by: sfmskywalker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Elsa.Dapper depends on FluentMigrator.Runner (pulls all providers)

2 participants