dohertj2/ScadaBridge
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(db): full-history idempotent migration script runs clean from an empty DB (#127)

Browse Source 
`dotnet ef migrations script --idempotent` from migration 0 stopped at
AddSiteIdToDataConnections (Msg 207 'SiteId') and, past it, hit Msg 156/178/137
on MigrateParametersToJsonSchema's CREATE FUNCTIONs and Msg 207 on the
ExecutionId / ParentExecutionId filtered indexes. Same root cause as #126: the
script compiles a whole migration as one batch behind IF ... BEGIN guards.
(The Msg 1911 'Category' / later @legacy errors in the issue were cascades of
those failures and disappear once they are fixed.)

- New MigrationSql.Deferred(sql): wraps a body in EXEC(N'...') with quotes
  doubled, so it compiles at execution time as its own batch. Migrate() runs
  the identical text.
- Deferred: AddSiteIdToDataConnections step 3 (data move + cursor, DECLAREs kept
  in the same body); MigrateParametersToJsonSchema's three CREATE FUNCTIONs and
  the UPDATE that calls them; AddAuditLogExecutionId / AddAuditLogParentExecutionId
  filtered partition-aligned indexes. No model/snapshot change.
- IdempotentMigrationScriptTests: new IdempotentScript_FromEmpty_RunsClean_AndMatchesMigrate
  runs the full script on an empty DB batch by batch, re-runs it as a no-op, and
  compares a schema snapshot (columns, defaults, indexes incl. filter + data space,
  FKs, checks, modules, principals, permissions, partitioning, per-table row
  count + checksum) against a Migrate()-built DB. Fails on the unfixed
  migrations with Msg 207 'SiteId'. Batch failures now name the migration.

Schema equivalence: Migrate() before vs after this change, and the fixed
from-empty script, all produce byte-identical snapshots (939 lines); the
57->head script still runs clean and lands on the same snapshot; the seeded
data path through AddSiteIdToDataConnections + MigrateParametersToJsonSchema
gives identical rows via the fixed script and the unfixed Migrate().

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-11-12 07:25:37 -05:00
parent 7bc350818c
commit 08ef94bb21
6 changed files with 272 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/Migrations/IdempotentMigrationScriptTests.cs
+213 -11
View File
@@ -16,8 +16,9 @@ namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests.Migrations;
/// batch before any guard runs. A raw-SQL statement that references a column added earlier in
/// the same migration therefore fails with Msg 207 even though the guard would have let the
/// ADD COLUMN run first — <c>database update</c> / <c>Migrate()</c> never see it because they
/// send one command at a time. The tests in this class migrate the fixture database down and
/// back up, so they own their fixture and leave the database fully migrated.
/// send one command at a time. The baseline test migrates the fixture database down and back
/// up, so this class owns its fixture and leaves the database fully migrated; the from-empty
/// test (ScadaBridge#127) builds and drops two databases of its own next to the fixture's.
/// </remarks>
public sealed class IdempotentMigrationScriptTests : IClassFixture<MsSqlMigrationFixture>
{
@@ -27,6 +28,8 @@ public sealed class IdempotentMigrationScriptTests : IClassFixture<MsSqlMigratio
private static readonly Regex BatchSeparator = new(@"^\s*GO\s*$", RegexOptions.Multiline | RegexOptions.IgnoreCase);
private static readonly Regex MigrationIdInBatch = new(@"\[MigrationId\] = N'([^']+)'");
private readonly MsSqlMigrationFixture _fixture;
/// <summary>Creates the test class with its own migrated SQL Server database.</summary>
@@ -70,13 +73,199 @@ public sealed class IdempotentMigrationScriptTests : IClassFixture<MsSqlMigratio
}
}
private ScadaBridgeDbContext CreateContext()
=> new(new DbContextOptionsBuilder<ScadaBridgeDbContext>()
.UseSqlServer(_fixture.ConnectionString, sql => sql.CommandTimeout(600)).Options);
private async Task RunScriptAsync(string script)
/// <summary>
/// From an EMPTY database (a fresh install done from the generated script) the full-history
/// idempotent script runs batch by batch without an error, applies every migration, re-runs
/// as a no-op, and leaves exactly the schema <c>Migrate()</c> builds (ScadaBridge#127).
/// </summary>
/// <remarks>
/// Early migrations carry raw SQL that only <c>Migrate()</c> could run: references to columns
/// added earlier in the same migration, <c>CREATE FUNCTION</c> inside the script's
/// <c>IF … BEGIN</c> guards, and <c>DECLARE</c>d variables. They now go through
/// <c>MigrationSql.Deferred</c>. The schema comparison
/// guards the other half of that fix: the script path and the <c>Migrate()</c> path must
/// converge on the same tables, columns, defaults, indexes, keys, constraints, modules,
/// principals, permissions, partitioning and seed-row counts.
/// </remarks>
/// <returns>A task that represents the asynchronous operation.</returns>
[SkippableFact]
public async Task IdempotentScript_FromEmpty_RunsClean_AndMatchesMigrate()
{
await using var conn = new SqlConnection(_fixture.ConnectionString);
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
var scriptDb = _fixture.DatabaseName + "_script";
var migrateDb = _fixture.DatabaseName + "_migrate";
var scriptConn = ForDatabase(scriptDb);
var migrateConn = ForDatabase(migrateDb);
await using var ctx = CreateContext();
var all = ctx.Database.GetMigrations().ToList();
var script = ctx.GetService<IMigrator>().GenerateScript(
options: MigrationsSqlGenerationOptions.Idempotent);
try
{
await CreateEmptyDatabaseAsync(scriptDb);
await CreateEmptyDatabaseAsync(migrateDb);
await RunScriptAsync(scriptConn, script);
Assert.Equal(all, await ReadHistoryAsync(scriptConn));
// Second run against the fully-migrated database: every guard is false.
await RunScriptAsync(scriptConn, script);
Assert.Equal(all, await ReadHistoryAsync(scriptConn));
await using (var migrateCtx = CreateContext(migrateConn))
{
await migrateCtx.Database.MigrateAsync();
}
var expected = await ReadSchemaSnapshotAsync(migrateConn);
var actual = await ReadSchemaSnapshotAsync(scriptConn);
Assert.True(expected.Count > 500, $"schema snapshot looks empty ({expected.Count} lines)");
var onlyMigrate = expected.Except(actual, StringComparer.Ordinal).ToList();
var onlyScript = actual.Except(expected, StringComparer.Ordinal).ToList();
Assert.True(
onlyMigrate.Count == 0 && onlyScript.Count == 0,
"Schema differs between Migrate() and the idempotent script." +
$"\nOnly via Migrate():\n {string.Join("\n ", onlyMigrate)}" +
$"\nOnly via script:\n {string.Join("\n ", onlyScript)}");
}
finally
{
await DropDatabaseAsync(scriptDb);
await DropDatabaseAsync(migrateDb);
}
}
private ScadaBridgeDbContext CreateContext() => CreateContext(_fixture.ConnectionString);
private static ScadaBridgeDbContext CreateContext(string connectionString)
=> new(new DbContextOptionsBuilder<ScadaBridgeDbContext>()
.UseSqlServer(connectionString, sql => sql.CommandTimeout(600)).Options);
private string ForDatabase(string database)
=> new SqlConnectionStringBuilder(_fixture.ConnectionString) { InitialCatalog = database }.ConnectionString;
private async Task CreateEmptyDatabaseAsync(string database)
{
await using var conn = new SqlConnection(ForDatabase("master"));
await conn.OpenAsync();
await using var cmd = conn.CreateCommand();
cmd.CommandTimeout = 120;
// Name is the fixture's guid-suffixed name plus a fixed suffix: no injection surface.
cmd.CommandText = $"CREATE DATABASE [{database}];";
await cmd.ExecuteNonQueryAsync();
}
private async Task DropDatabaseAsync(string database)
{
try
{
SqlConnection.ClearAllPools();
await using var conn = new SqlConnection(ForDatabase("master"));
await conn.OpenAsync();
await using var cmd = conn.CreateCommand();
cmd.CommandTimeout = 120;
cmd.CommandText =
$"IF DB_ID(N'{database}') IS NOT NULL BEGIN " +
$"ALTER DATABASE [{database}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE; DROP DATABASE [{database}]; END";
await cmd.ExecuteNonQueryAsync();
}
catch (SqlException)
{
// Best-effort: the fixture's orphan sweep clears anything left behind (same prefix).
}
}
/// <summary>
/// One sorted line per schema fact. System-generated constraint names (DF__…_hex) are
/// masked; everything else, including module bodies (hashed), is compared verbatim.
/// </summary>
private static async Task<List<string>> ReadSchemaSnapshotAsync(string connectionString)
{
await using var conn = new SqlConnection(connectionString);
await conn.OpenAsync();
var lines = await ReadLinesAsync(conn, SchemaSnapshotSql);
lines.AddRange(await ReadLinesAsync(conn, RowCountSnapshotSql));
lines.Sort(StringComparer.Ordinal);
return lines;
}
private static async Task<List<string>> ReadLinesAsync(SqlConnection conn, string sql)
{
await using var cmd = conn.CreateCommand();
cmd.CommandTimeout = 120;
cmd.CommandText = sql;
var lines = new List<string>();
await using var reader = await cmd.ExecuteReaderAsync();
while (await reader.ReadAsync())
{
lines.Add(reader.GetString(0));
}
return lines;
}
private const string SchemaSnapshotSql = """
SET NOCOUNT ON;
CREATE TABLE #t(line nvarchar(max) COLLATE Latin1_General_BIN);
INSERT #t(line)
SELECT 'COL|' + s.name + '.' + t.name + '|' + CAST(c.column_id AS varchar) + '|' + c.name + '|' + ty.name + '|' + CAST(c.max_length AS varchar) + '|' + CAST(c.precision AS varchar) + '|' + CAST(c.scale AS varchar) + '|' + CAST(c.is_nullable AS varchar) + '|' + CAST(c.is_identity AS varchar) + '|' + ISNULL(c.collation_name,'') + '|' + ISNULL(cc.definition,'') + '|' + ISNULL(CASE WHEN dc.is_system_named=1 THEN '<sys>' ELSE dc.name END,'') + '|' + ISNULL(dc.definition,'')
FROM sys.tables t JOIN sys.schemas s ON s.schema_id=t.schema_id JOIN sys.columns c ON c.object_id=t.object_id
JOIN sys.types ty ON ty.user_type_id=c.user_type_id
LEFT JOIN sys.computed_columns cc ON cc.object_id=c.object_id AND cc.column_id=c.column_id
LEFT JOIN sys.default_constraints dc ON dc.object_id=c.default_object_id;
INSERT #t(line)
SELECT 'IDX|' + s.name + '.' + o.name + '|' + ISNULL(i.name,'<heap>') + '|' + i.type_desc + '|' + CAST(i.is_unique AS varchar) + '|' + CAST(i.is_primary_key AS varchar) + '|' + CAST(i.is_unique_constraint AS varchar) + '|' + ISNULL(i.filter_definition,'') + '|' + ISNULL((SELECT name FROM sys.data_spaces ds WHERE ds.data_space_id=i.data_space_id),'') + '|' +
ISNULL(STUFF((SELECT ',' + c.name + CASE WHEN ic.is_descending_key=1 THEN ' DESC' ELSE '' END + CASE WHEN ic.is_included_column=1 THEN ' INCL' ELSE '' END
FROM sys.index_columns ic JOIN sys.columns c ON c.object_id=ic.object_id AND c.column_id=ic.column_id
WHERE ic.object_id=i.object_id AND ic.index_id=i.index_id ORDER BY ic.is_included_column, ic.key_ordinal, c.name FOR XML PATH('')),1,1,''),'')
FROM sys.indexes i JOIN sys.objects o ON o.object_id=i.object_id JOIN sys.schemas s ON s.schema_id=o.schema_id WHERE o.is_ms_shipped=0;
INSERT #t(line)
SELECT 'FK|' + fk.name + '|' + OBJECT_SCHEMA_NAME(fk.parent_object_id) + '.' + OBJECT_NAME(fk.parent_object_id) + '->' + OBJECT_SCHEMA_NAME(fk.referenced_object_id) + '.' + OBJECT_NAME(fk.referenced_object_id) + '|' + fk.delete_referential_action_desc + '|' + fk.update_referential_action_desc + '|' +
STUFF((SELECT ',' + COL_NAME(fkc.parent_object_id,fkc.parent_column_id) + '=' + COL_NAME(fkc.referenced_object_id,fkc.referenced_column_id) FROM sys.foreign_key_columns fkc WHERE fkc.constraint_object_id=fk.object_id ORDER BY fkc.constraint_column_id FOR XML PATH('')),1,1,'')
FROM sys.foreign_keys fk;
INSERT #t(line)
SELECT 'CK|' + OBJECT_NAME(parent_object_id) + '|' + CASE WHEN is_system_named=1 THEN '<sys>' ELSE name END + '|' + definition FROM sys.check_constraints;
INSERT #t(line)
SELECT 'OBJ|' + o.type_desc + '|' + s.name + '.' + o.name + '|' + ISNULL(OBJECT_NAME(o.parent_object_id),'') FROM sys.objects o JOIN sys.schemas s ON s.schema_id=o.schema_id WHERE o.is_ms_shipped=0 AND o.type NOT IN ('D','C');
INSERT #t(line)
SELECT 'MOD|' + OBJECT_SCHEMA_NAME(m.object_id) + '.' + OBJECT_NAME(m.object_id) + '|' + CONVERT(varchar(64), HASHBYTES('SHA2_256', m.definition), 2) + '|' + CAST(m.uses_ansi_nulls AS varchar) + CAST(m.uses_quoted_identifier AS varchar) + '|' + ISNULL(CAST(m.is_schema_bound AS varchar),'') + '|' + ISNULL(USER_NAME(m.execute_as_principal_id),'') FROM sys.sql_modules m;
INSERT #t(line)
SELECT 'SCH|' + s.name + '|' + USER_NAME(s.principal_id) FROM sys.schemas s WHERE s.schema_id < 16384;
INSERT #t(line)
SELECT 'PRN|' + p.name + '|' + p.type_desc + '|' + ISNULL(p.default_schema_name,'') FROM sys.database_principals p WHERE p.principal_id BETWEEN 5 AND 16383;
INSERT #t(line)
SELECT 'ROLEMEM|' + USER_NAME(role_principal_id) + '|' + USER_NAME(member_principal_id) FROM sys.database_role_members;
INSERT #t(line)
SELECT 'PERM|' + USER_NAME(dp.grantee_principal_id) + '|' + dp.class_desc + '|' + ISNULL(CASE dp.class WHEN 1 THEN OBJECT_SCHEMA_NAME(dp.major_id) + '.' + OBJECT_NAME(dp.major_id) WHEN 3 THEN SCHEMA_NAME(dp.major_id) END,'') + '|' + ISNULL(COL_NAME(dp.major_id, NULLIF(dp.minor_id,0)),'') + '|' + dp.permission_name + '|' + dp.state_desc FROM sys.database_permissions dp WHERE dp.grantee_principal_id > 4 OR dp.class <> 0;
INSERT #t(line)
SELECT 'SEQ|' + name + '|' + CAST(start_value AS varchar) FROM sys.sequences;
INSERT #t(line)
SELECT 'TT|' + name FROM sys.types WHERE is_user_defined=1;
INSERT #t(line)
SELECT 'PF|' + pf.name + '|' + pf.type_desc + '|' + CAST(pf.boundary_value_on_right AS varchar) + '|' + ISNULL(STUFF((SELECT ',' + CONVERT(nvarchar(100), prv.value, 126) FROM sys.partition_range_values prv WHERE prv.function_id=pf.function_id ORDER BY prv.boundary_id FOR XML PATH('')),1,1,''),'') FROM sys.partition_functions pf;
INSERT #t(line)
SELECT 'PS|' + ps.name + '|' + pf.name FROM sys.partition_schemes ps JOIN sys.partition_functions pf ON pf.function_id=ps.function_id;
SELECT line FROM #t ORDER BY line;
""";
// Row count + content checksum per table (seed rows such as LdapGroupMappings).
private const string RowCountSnapshotSql = """
SET NOCOUNT ON;
DECLARE @sql nvarchar(max) = N'';
SELECT @sql = @sql + N'SELECT ''DATA|' + s.name + '.' + t.name + '|'' + CAST(COUNT(*) AS varchar) + ''|'' + ISNULL(CAST(CHECKSUM_AGG(BINARY_CHECKSUM(*)) AS varchar),'''') FROM ' + QUOTENAME(s.name) + '.' + QUOTENAME(t.name) + N' UNION ALL '
FROM sys.tables t JOIN sys.schemas s ON s.schema_id=t.schema_id ORDER BY s.name, t.name;
SET @sql = LEFT(@sql, LEN(@sql) - 10) + N' ORDER BY 1';
EXEC(@sql);
""";
private Task RunScriptAsync(string script) => RunScriptAsync(_fixture.ConnectionString, script);
private static async Task RunScriptAsync(string connectionString, string script)
{
await using var conn = new SqlConnection(connectionString);
await conn.OpenAsync();
foreach (var batch in BatchSeparator.Split(script))
{
@@ -88,13 +277,26 @@ public sealed class IdempotentMigrationScriptTests : IClassFixture<MsSqlMigratio
await using var cmd = conn.CreateCommand();
cmd.CommandTimeout = 600;
cmd.CommandText = batch;
await cmd.ExecuteNonQueryAsync();
try
{
await cmd.ExecuteNonQueryAsync();
}
catch (SqlException ex)
{
// Name the migration so a failure points straight at the file to fix.
var migration = MigrationIdInBatch.Match(batch);
throw new InvalidOperationException(
$"Idempotent script batch for migration '{(migration.Success ? migration.Groups[1].Value : "?")}' " +
$"failed: Msg {ex.Number}: {ex.Message}", ex);
}
}
}
private async Task<List<string>> ReadHistoryAsync()
private Task<List<string>> ReadHistoryAsync() => ReadHistoryAsync(_fixture.ConnectionString);
private static async Task<List<string>> ReadHistoryAsync(string connectionString)
{
await using var conn = new SqlConnection(_fixture.ConnectionString);
await using var conn = new SqlConnection(connectionString);
await conn.OpenAsync();
await using var cmd = conn.CreateCommand();
cmd.CommandText = "SELECT MigrationId FROM __EFMigrationsHistory ORDER BY MigrationId";