Split Elsa.Persistence.Dapper project #131

[heku]

No description provided.

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[sfmskywalker]

@greptile please review

[greptile-apps]

Greptile Summary

This PR splits the monolithic Elsa.Persistence.Dapper project into four focused packages: a shared core (Elsa.Persistence.Dapper) and three database-specific packages (Elsa.Persistence.Dapper.Sqlite, Elsa.Persistence.Dapper.SqlServer, Elsa.Persistence.Dapper.PostgreSql). Each provider's dialect, connection provider, and (for SQLite) type handlers are relocated into their respective package with updated namespaces.

• Core package slimmed down — driver-specific NuGet references (Npgsql, Microsoft.Data.SqlClient, Microsoft.Data.Sqlite.Core, Azure.Identity) are removed from the base project and moved to their dedicated packages.
• GuidHandler registration fixed — previously registered in DapperFeature (always, regardless of provider), it is now correctly registered in SqliteDbConnectionProvider's static constructor so it only applies when SQLite is in use.
• Breaking change: no default provider — DapperFeature.DbConnectionProvider previously defaulted to a SQLite instance; it now defaults to null, meaning callers who do not explicitly assign it will get an ArgumentNullException from AddSingleton at startup without a helpful error message.

Confidence Score: 3/5

Safe to merge only after adding a null guard in DapperFeature.Apply(); without it, any consumer that calls UseDapper() without explicitly assigning DbConnectionProvider will crash at startup with an opaque ArgumentNullException.

The structural split is clean and the workbench project always assigns DbConnectionProvider explicitly, so the included sample works fine. However, the removal of the default SQLite provider leaves the public API in a state where omitting a single property assignment produces an unhelpful crash at DI build time.

src/modules/persistence/Elsa.Persistence.Dapper/Features/DapperFeature.cs — the null DbConnectionProvider is the key concern before merging.

Important Files Changed

Filename	Overview
src/modules/persistence/Elsa.Persistence.Dapper/Features/DapperFeature.cs	Removed the default SQLite provider and GuidHandler registration; DbConnectionProvider now defaults to null, which will throw ArgumentNullException at startup if not explicitly assigned.
src/modules/persistence/Elsa.Persistence.Dapper.Sqlite/Services/SqliteDbConnectionProvider.cs	Moved to its own project; GuidHandler now correctly registered in the static constructor alongside DateTimeOffsetHandler and TimeSpanHandler; Dialect changed to auto-property initializer.
src/modules/persistence/Elsa.Persistence.Dapper/Elsa.Persistence.Dapper.csproj	Removed database-driver packages (Npgsql, SqlClient, Sqlite.Core, Azure.Identity); only Dapper and FluentMigrator remain, which is the correct leaner base.
src/modules/persistence/Elsa.Persistence.Dapper.PostgreSql/Services/PostgreSqlDbConnectionProvider.cs	Moved to its own project with updated namespace; Dialect changed to auto-property initializer.
src/modules/persistence/Elsa.Persistence.Dapper.SqlServer/Services/SqlServerDbConnectionProvider.cs	Moved to its own project with updated namespace and fixed doc comment; Dialect changed to auto-property initializer.
src/workbench/Elsa.Server.Web/Program.cs	Updated using directives to reference new per-provider namespaces; DbConnectionProvider is always set explicitly here.
Elsa.Extensions.sln	Three new C# projects added under the correct persistence solution folder.

Comments Outside Diff (1)

1. src/modules/persistence/Elsa.Persistence.Dapper/Features/DapperFeature.cs, line 21-27 (link)

   Null DbConnectionProvider causes ArgumentNullException at startup

   DbConnectionProvider now defaults to null (no initializer) and Apply() calls Services.AddSingleton(DbConnectionProvider) unconditionally. IServiceCollection.AddSingleton<T>(Func<IServiceProvider,T> factory) throws ArgumentNullException when the factory is null, so any project that calls UseDapper() without explicitly assigning DbConnectionProvider will crash at DI container build time with a confusing error. A null-guard with a meaningful message in Apply() would make this failure much easier to diagnose.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/modules/persistence/Elsa.Persistence.Dapper/Features/DapperFeature.cs
   Line: 21-27
   
   Comment:
   **Null `DbConnectionProvider` causes `ArgumentNullException` at startup**
   
   `DbConnectionProvider` now defaults to `null` (no initializer) and `Apply()` calls `Services.AddSingleton(DbConnectionProvider)` unconditionally. `IServiceCollection.AddSingleton<T>(Func<IServiceProvider,T> factory)` throws `ArgumentNullException` when the factory is `null`, so any project that calls `UseDapper()` without explicitly assigning `DbConnectionProvider` will crash at DI container build time with a confusing error. A null-guard with a meaningful message in `Apply()` would make this failure much easier to diagnose.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/modules/persistence/Elsa.Persistence.Dapper/Features/DapperFeature.cs:21-27
**Null `DbConnectionProvider` causes `ArgumentNullException` at startup**

`DbConnectionProvider` now defaults to `null` (no initializer) and `Apply()` calls `Services.AddSingleton(DbConnectionProvider)` unconditionally. `IServiceCollection.AddSingleton<T>(Func<IServiceProvider,T> factory)` throws `ArgumentNullException` when the factory is `null`, so any project that calls `UseDapper()` without explicitly assigning `DbConnectionProvider` will crash at DI container build time with a confusing error. A null-guard with a meaningful message in `Apply()` would make this failure much easier to diagnose.

_{Reviews (1): Last reviewed commit: "Split Elsa.Persistence.Dapper project #1..." | Re-trigger Greptile}