From fda3987c0bebad3f062d60c178a7cdfd8de161bd Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Sun, 6 Sep 2026 23:27:43 -0700 Subject: [PATCH] fix(efcore): terminate raw SQL statements in the PostgreSQL and SQLite Runtime V3_6 migrations (#8039) * fix(efcore): terminate raw SQL statements in the PostgreSQL and SQLite Runtime V3_6 migrations The V3_6 Runtime migration's two DROP INDEX statements were emitted without a trailing semicolon on PostgreSQL and SQLite, so `dotnet ef migrations script` produced syntactically invalid SQL (an unterminated statement inside the idempotent DO $EF$ block, and unseparated statements in the plain script). MigrateAsync() was unaffected because EF executes each Sql() call individually. Add the missing terminators and a regression test that generates the Runtime migration script offline for both providers (idempotent and plain forms) and asserts the DROP INDEX statements are properly terminated. Refs #7912 Co-Authored-By: Claude Fable 5.1 * test(efcore): share migration-script setup and cover the schema-prefixed statements Co-Authored-By: Claude Fable 5.1 * test(efcore): cover schema-prefixed migration statements without static state Co-Authored-By: Claude Fable 5.1 --------- Co-authored-by: Claude Fable 5.1 --- .../Migrations/Runtime/20251204150341_V3_6.cs | 4 +- .../Migrations/Runtime/20251204150006_V3_6.cs | 4 +- .../Elsa.Persistence.EFCore.UnitTests.csproj | 2 + .../V3_6RuntimeMigrationTests.cs | 121 ++++++++++++++++++ 4 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 test/unit/Elsa.Persistence.EFCore.UnitTests/V3_6RuntimeMigrationTests.cs diff --git a/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs index efb1be610..67b39e4c0 100644 --- a/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.PostgreSql/Migrations/Runtime/20251204150341_V3_6.cs @@ -26,8 +26,8 @@ namespace Elsa.Persistence.EFCore.PostgreSql.Migrations.Runtime unique: true); var schemaPrefix = _schema.Schema != null ? $"\"{_schema.Schema}\"." : ""; - migrationBuilder.Sql($"DROP INDEX IF EXISTS {schemaPrefix}\"IX_WorkflowExecutionLogRecord_ActivityNodeId\""); - migrationBuilder.Sql($"DROP INDEX IF EXISTS {schemaPrefix}\"IX_ActivityExecutionRecord_ActivityNodeId\""); + migrationBuilder.Sql($"DROP INDEX IF EXISTS {schemaPrefix}\"IX_WorkflowExecutionLogRecord_ActivityNodeId\";"); + migrationBuilder.Sql($"DROP INDEX IF EXISTS {schemaPrefix}\"IX_ActivityExecutionRecord_ActivityNodeId\";"); } /// diff --git a/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs b/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs index 3d46f9498..f3b3c2702 100644 --- a/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs +++ b/src/modules/Elsa.Persistence.EFCore.Sqlite/Migrations/Runtime/20251204150006_V3_6.cs @@ -25,8 +25,8 @@ namespace Elsa.Persistence.EFCore.Sqlite.Migrations.Runtime columns: new[] { "WorkflowDefinitionId", "Hash", "ActivityId", "TenantId" }, unique: true); - migrationBuilder.Sql("DROP INDEX IF EXISTS \"IX_WorkflowExecutionLogRecord_ActivityNodeId\""); - migrationBuilder.Sql("DROP INDEX IF EXISTS \"IX_ActivityExecutionRecord_ActivityNodeId\""); + migrationBuilder.Sql("DROP INDEX IF EXISTS \"IX_WorkflowExecutionLogRecord_ActivityNodeId\";"); + migrationBuilder.Sql("DROP INDEX IF EXISTS \"IX_ActivityExecutionRecord_ActivityNodeId\";"); } /// diff --git a/test/unit/Elsa.Persistence.EFCore.UnitTests/Elsa.Persistence.EFCore.UnitTests.csproj b/test/unit/Elsa.Persistence.EFCore.UnitTests/Elsa.Persistence.EFCore.UnitTests.csproj index 50b51c57e..777107d4b 100644 --- a/test/unit/Elsa.Persistence.EFCore.UnitTests/Elsa.Persistence.EFCore.UnitTests.csproj +++ b/test/unit/Elsa.Persistence.EFCore.UnitTests/Elsa.Persistence.EFCore.UnitTests.csproj @@ -11,6 +11,8 @@ + + diff --git a/test/unit/Elsa.Persistence.EFCore.UnitTests/V3_6RuntimeMigrationTests.cs b/test/unit/Elsa.Persistence.EFCore.UnitTests/V3_6RuntimeMigrationTests.cs new file mode 100644 index 000000000..89f30d0a9 --- /dev/null +++ b/test/unit/Elsa.Persistence.EFCore.UnitTests/V3_6RuntimeMigrationTests.cs @@ -0,0 +1,121 @@ +using Elsa.Persistence.EFCore; +using Elsa.Persistence.EFCore.Extensions; +using Elsa.Persistence.EFCore.Modules.Runtime; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.Extensions.DependencyInjection; + +namespace Elsa.Persistence.EFCore.UnitTests; + +/// +/// Regression tests for the V3_6 Runtime migration's raw SQL statements. `dotnet ef migrations script` +/// previously produced invalid SQL because the `DROP INDEX` statements were not terminated with a +/// semicolon, which corrupts both the idempotent PL/pgSQL block and the plain script. +/// +public class V3_6RuntimeMigrationTests +{ + private const string DropIndexPrefix = "DROP INDEX IF EXISTS"; + private const string WorkflowExecutionLogRecordIndexName = "IX_WorkflowExecutionLogRecord_ActivityNodeId"; + private const string ActivityExecutionRecordIndexName = "IX_ActivityExecutionRecord_ActivityNodeId"; + + [Theory] + [InlineData("Elsa")] + [InlineData("custom_schema")] + public void GenerateScript_PostgreSql_Idempotent_TerminatesDropIndexStatements(string schema) + { + var script = GeneratePostgreSqlScript(MigrationsSqlGenerationOptions.Idempotent, schema); + + AssertDropIndexStatementsAreTerminated(script, schema, requireTrailingEndIf: true); + } + + [Theory] + [InlineData("Elsa")] + [InlineData("custom_schema")] + public void GenerateScript_PostgreSql_Plain_TerminatesDropIndexStatements(string schema) + { + var script = GeneratePostgreSqlScript(MigrationsSqlGenerationOptions.Default, schema); + + AssertDropIndexStatementsAreTerminated(script, schema); + } + + // Note: EF Core's SQLite provider does not support generating idempotent migration scripts + // (SqliteHistoryRepository.GetEndIfScript throws NotSupportedException), so only the plain + // script form is exercised here. The SQLite migration also never schema-qualifies its DROP + // INDEX statements, so there is no schema-prefixed variant to cover. + [Fact] + public void GenerateScript_Sqlite_Plain_TerminatesDropIndexStatements() + { + var script = GenerateSqliteScript(MigrationsSqlGenerationOptions.Default); + + AssertDropIndexStatementsAreTerminated(script, schema: null); + } + + private static string GeneratePostgreSqlScript(MigrationsSqlGenerationOptions options, string schema) + { + var migrationsAssembly = typeof(Elsa.Persistence.EFCore.PostgreSql.Migrations.Runtime.V3_6).Assembly; + var contextOptions = new ElsaDbContextOptions { SchemaName = schema }; + + return GenerateScript( + builder => builder.UseElsaPostgreSql(migrationsAssembly, "Host=unused", contextOptions), + fromMigration: "20250530104953_V3_5", + toMigration: "20251204150341_V3_6", + options); + } + + private static string GenerateSqliteScript(MigrationsSqlGenerationOptions options) + { + var migrationsAssembly = typeof(Elsa.Persistence.EFCore.Sqlite.Migrations.Runtime.V3_6).Assembly; + + return GenerateScript( + builder => builder.UseElsaSqlite(migrationsAssembly, "Data Source=:memory:"), + fromMigration: "20250530104854_V3_5", + toMigration: "20251204150006_V3_6", + options); + } + + private static string GenerateScript(Action> configureProvider, string fromMigration, string toMigration, MigrationsSqlGenerationOptions options) + { + var optionsBuilder = new DbContextOptionsBuilder(); + configureProvider(optionsBuilder); + var dbContextOptions = optionsBuilder.Options; + + using var dbContext = new RuntimeElsaDbContext(dbContextOptions, CreateServiceProvider()); + var migrator = dbContext.GetService(); + + return migrator.GenerateScript(fromMigration: fromMigration, toMigration: toMigration, options: options); + } + + // ElsaDbContextOptions.SchemaName always falls back to ElsaDbContextBase.ElsaSchema ("Elsa") when it + // isn't set, so the branch the V3_6 migration guards against with + // `_schema.Schema != null ? "\"{schema}\"." : ""` - a null schema - is unreachable through the + // public options and is therefore not covered here. + private static IServiceProvider CreateServiceProvider() => new ServiceCollection().BuildServiceProvider(); + + private static void AssertDropIndexStatementsAreTerminated(string script, string? schema, bool requireTrailingEndIf = false) + { + AssertDropIndexStatementIsTerminated(script, WorkflowExecutionLogRecordIndexName, schema, requireTrailingEndIf); + AssertDropIndexStatementIsTerminated(script, ActivityExecutionRecordIndexName, schema, requireTrailingEndIf); + } + + private static void AssertDropIndexStatementIsTerminated(string script, string indexName, string? schema, bool requireTrailingEndIf) + { + var expectedStatement = schema != null + ? $"{DropIndexPrefix} \"{schema}\".\"{indexName}\";" + : $"{DropIndexPrefix} \"{indexName}\";"; + + var lines = script.Split('\n').Select(l => l.Trim()).ToList(); + var dropLineIndex = lines.FindIndex(l => l.StartsWith(DropIndexPrefix, StringComparison.Ordinal) && l.Contains(indexName, StringComparison.Ordinal)); + + Assert.True(dropLineIndex >= 0, $"Expected to find a 'DROP INDEX IF EXISTS' statement for \"{indexName}\" in the generated script:\n{script}"); + Assert.Equal(expectedStatement, lines[dropLineIndex]); + + if (!requireTrailingEndIf) + return; + + // The statement inside the DO $EF$ ... IF NOT EXISTS(...) THEN END IF; block must be + // terminated before the following END IF; line, otherwise the PL/pgSQL block fails to parse. + var endIfLineIndex = lines.FindIndex(dropLineIndex, l => l.Equals("END IF;", StringComparison.Ordinal)); + Assert.True(endIfLineIndex >= 0, $"Expected an 'END IF;' line after the DROP INDEX statement for \"{indexName}\":\n{script}"); + } +}