feat(persistence): add Microsoft SQL Server as an opt-in database provider - #1374
Open
maxiar wants to merge 1 commit into
Open
feat(persistence): add Microsoft SQL Server as an opt-in database provider#1374maxiar wants to merge 1 commit into
maxiar wants to merge 1 commit into
Conversation
…vider The stack now runs on PostgreSQL or SQL Server, selected entirely by `DatabaseOptions:Provider` — no hardcoded provider strings remain. Both migrations assemblies ship in every build, so switching is a config change rather than a rescaffold. Requires SQL Server 2025 (17.x) or Azure SQL: JSON columns map to the native `json` type at compatibility level 170, which does not exist on 2019/2022. Provider seam - Entity configurations now declare portable intent (`HasJsonColumn`, `HasNotDeletedFilter`, `AsTrigramSearchIndex`, …) which `HeroProviderConventions` resolves per provider. Implemented as an EF model-finalizing convention so it runs regardless of where a subclass calls `base.OnModelCreating`. - `WhereSearch` replaces the 16 Npgsql-only `EF.Functions.ILike` call sites. - MSSQL outbox claim via `UPDLOCK, READPAST, OUTPUT` — a real equivalent of `FOR UPDATE SKIP LOCKED`, not the previous degraded single-dispatcher fallback. - Hangfire stale-lock cleanup, OpenTelemetry instrumentation and the DbMigrator lock (`sp_getapplock`) all gained SQL Server paths. New `FSH.Starter.Migrations.MSSQL` with one consolidated `Initial` per DbContext, plus hand-written DDL EF has no API for: `CREATE JSON INDEX` on audit payloads and the chat full-text catalog. Aspire (`DbProvider=MSSQL`), Docker Compose (`--profile mssql`), Testcontainers (`FSH_TEST_DB_PROVIDER=MSSQL`) and `fsh new --db-provider mssql` all understand the choice. Fixes found along the way, all masked by PostgreSQL - `AmbientDbTransactionRegistry` never ran: its methods did not match `IDbTransactionInterceptor`, whose default no-op implementations bound instead. The outbox therefore never enlisted in the business transaction. Npgsql hides this by associating commands with the connection's open transaction; SqlClient throws. Guarded by a new reflection test. - Guid v7 ids do not sort chronologically on SQL Server (`uniqueidentifier` compares the last six bytes first), which silently broke chat cursor pagination. Message paging now sorts on a persisted `char(36)` key. - `EnableRetryOnFailure` on the MSSQL branch would have broken every transactional publish; its execution strategy refuses user-initiated transactions. - `appsettings.Production.json` was missing `DatabaseOptions:MigrationsAssembly`. - `ConnectionStringValidator` reported success for unknown providers. Testcontainers is bumped 4.11.0 -> 4.14.0 because the older line drags in SSH.NET 2025.1.0 (GHSA-q939-rpr3-3284) and fails NuGetAudit under TreatWarningsAsErrors — main does not currently build without this. `MigrationDriftTests` compares each context's live model against that provider's own snapshot, so a change that only gets one provider's migration fails the build. Maintaining both is not mandatory: `FshMaintainedDbProviders` decides which providers are enforced and which only report. PostgreSQL is unchanged: all 11 DbContexts remain drift-free against the existing migrations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Heads-up for sequencing: #1376 (a database-backed Data Protection key store) depends on this PR and cannot merge before it. Two concrete things it takes from here:
Nothing is needed from you here; flagging it so the ordering is visible rather than discovered when #1376 fails to build. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Microsoft SQL Server as a second, config-selected database provider.
DatabaseOptions:Providerswitches the whole stack — there are no hardcoded provider strings left. Both migrations assemblies ship in every build, so moving between engines is a config change, not a rescaffold.The provider seam
Entity configurations stop naming Postgres types and declare intent instead;
HeroProviderConventionsresolves it per provider:HasColumnType("jsonb").HasJsonColumn()jsonbjsonHasColumnType("text").HasUnboundedTextColumn()textnvarchar(max)HasFilter("\"IsDeleted\" = FALSE").HasNotDeletedFilter()"IsDeleted" = FALSE[IsDeleted] = 0.HasMethod("gin").HasOperators(…).AsTrigramSearchIndex()It runs as an EF model-finalizing convention, so it applies regardless of where a subclass calls
base.OnModelCreating. Queries useWhereSearch(...)instead of the 16 Npgsql-onlyEF.Functions.ILikecall sites.Also provider-aware now: the outbox claim (
UPDLOCK, READPAST, OUTPUT— a realSKIP LOCKEDequivalent, replacing the degraded single-dispatcher fallback), Hangfire stale-lock cleanup, OpenTelemetry instrumentation, and the DbMigrator lock (sp_getapplock).Aspire (
DbProvider=MSSQL), Docker Compose (--profile mssql), Testcontainers (FSH_TEST_DB_PROVIDER=MSSQL) andfsh new --db-provider mssqlall understand the choice.Four defects found on the way — three pre-existing, all masked by PostgreSQL
AmbientDbTransactionRegistry's methods did not matchIDbTransactionInterceptor; the interface's default no-op implementations bound instead, so the registry stayed permanently empty and the outbox never enlisted in the business transaction. Npgsql hides this by associating commands with the connection's open transaction — SqlClient throws. Fixed, plus a reflection test that fails if a signature drifts again.uniqueidentifiercompares the last six bytes first, so chat cursor pagination returned wrongly-ordered pages, silently. Reproduced in a container (ORDER BY Idgavet3 t2 t1instead oft1 t2 t3) and fixed with a persistedchar(36)sort key.EnableRetryOnFailureon the MSSQL branch would have broken every transactional publish — its execution strategy refuses user-initiated transactions.appsettings.Production.jsonwas missingDatabaseOptions:MigrationsAssembly;ConnectionStringValidatorreported success for unknown providers.Capability differences on SQL Server
FREETEXTTABLEwhere Full-Text Search is installed, UNIONed with aLIKEpass over the last few minutes — SQL Server populates a full-text index asynchronously, so without it a just-sent message would be unsearchable. Falls back to a plainLIKEscan where FTS is absent (the official container has none; Azure SQL and full installs do).Source/UserNamesubstring search scans — no trigram equivalent onnvarchar(max).jsonb_path_opsGIN on Postgres,CREATE JSON INDEXon SQL Server.Guard against one-sided migrations
MigrationDriftTestscompares each context's live model against that provider's own snapshot, so a change that only gets one provider's migration fails the build with the exactmigrations addcommand. Maintaining both is not mandatory:FshMaintainedDbProvidersinDirectory.Build.propsdecides which providers are enforced and which only report, so a project that settles on one engine is never blocked by the other.Verification
The drift guard was also proven to fail: adding a property with only the PostgreSQL migration generated leaves the build red for MSSQL alone.
PostgreSQL is unchanged — all 11 DbContexts remain drift-free against the existing migrations.
One unrelated change, and why it is here
Testcontainersis bumped 4.11.0 → 4.14.0. The 4.11.0 line drags in SSH.NET 2025.1.0, which carries a high-severity advisory (GHSA-q939-rpr3-3284) and failsNuGetAuditunderTreatWarningsAsErrors—maindoes not currently build because of it, verified on a clean checkout with none of this branch applied. Since this PR adds a Testcontainers package of its own, leaving the family split was not an option. Happy to split it out if you would rather take it separately.Follow-up
Per golden rule #10, the docs site (
fullstackhero/docs) needs a companion PR: a database-providers page and a changelog entry. Content is drafted; say the word and I will open it.Branched from
main, not from my fork'sdevelop, so it carries only this feature and none of my fork-local commits.