Approve EF migration scripts to catch index changes

docs/testing.md

@@ -27,6 +27,8 @@ On Windows, [PackagedRavenDBTests](../src/ServiceControlInstaller.Engine.UnitTes
 Persistence tests check assumptions at the persistence seam level by exercising each persister.
 For local setup details, see [Local testing of persistence providers](testing-persistence.md).
 
+The SQL Server and PostgreSQL persistence test projects contain a `SlowMigrationOperationApprovals` test. It approves the migration steps that can block writes on a large table: index, primary key, and unique constraint changes, new foreign keys and check constraints, altered columns, raw SQL, and new columns that are NOT NULL with a default or are stored computed columns. Other steps, such as renames and drops of columns, tables, and constraints, are not listed. The test lists these steps for each migration and leaves out steps on a table that the same migration creates, because that table is empty. Raw SQL is always listed. When the test fails because a step changed, record the change in the upgrade guide of the next release before you approve the new list. If a step changed inside an existing migration, for example through a shared SQL constant, add a new migration instead. Databases that already ran the old migration never run it again.
+
 ## Transport tests
 
 Transport tests run the transport test suite for each transport.

src/ServiceControl.Audit.Persistence.Tests.PostgreSql/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt

@@ -0,0 +1,3 @@
+20260928032745_AddFullTextSearch
+    Sql
+        CREATE INDEX ix_audit_messages_full_text ON audit_messages USING GIN (to_tsvector('simple', headers_json || ' ' || substring(COALESCE(body_text, ''), 1, 262144) || ' ' || replace(replace(COALESCE(message_type, ''), '.', ' '), '+', ' ')))

src/ServiceControl.Audit.Persistence.Tests.PostgreSql/SlowMigrationOperationApprovals.cs

@@ -0,0 +1,29 @@
+namespace ServiceControl.Audit.Persistence.Tests
+{
+    using System;
+    using Microsoft.Extensions.DependencyInjection;
+    using NUnit.Framework;
+    using ServiceControl.Audit.Persistence.EFCore.Abstractions;
+    using ServiceControl.Audit.Persistence.EFCore.DbContexts;
+    using ServiceControl.Audit.Persistence.EFCore.PostgreSql;
+    using TestHelper;
+
+    [TestFixture]
+    class SlowMigrationOperationApprovals
+    {
+        [Test]
+        public void Verify_slow_migration_operations_are_unchanged()
+        {
+            var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000);
+            settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = "Host=unused";
+
+            var services = new ServiceCollection();
+            new PostgreSqlPersistenceConfiguration().Create(settings).AddInstaller(services);
+
+            using var serviceProvider = services.BuildServiceProvider();
+            using var scope = serviceProvider.CreateScope();
+
+            SlowMigrationOperations.Verify(scope.ServiceProvider.GetRequiredService<AuditDbContext>());
+        }
+    }
+}

src/ServiceControl.Audit.Persistence.Tests.SqlServer/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt

@@ -0,0 +1,24 @@
+20260928032751_AddFullTextSearch
+    Sql
+        IF SERVERPROPERTY('IsFullTextInstalled') <> 1
+        BEGIN
+            THROW 50000, 'ServiceControl requires the SQL Server Full-Text Search feature, which is not installed on this instance. Install it and run setup again.', 1;
+        END
+    Sql
+        IF NOT EXISTS (SELECT 1 FROM sys.fulltext_catalogs WHERE name = 'ServiceControlFullTextCatalog')
+        BEGIN
+            BEGIN TRY
+                EXEC('CREATE FULLTEXT CATALOG ServiceControlFullTextCatalog');
+            END TRY
+            BEGIN CATCH
+                IF NOT EXISTS (SELECT 1 FROM sys.fulltext_catalogs WHERE name = 'ServiceControlFullTextCatalog') THROW;
+            END CATCH
+        END
+    Sql
+        IF NOT EXISTS (SELECT 1 FROM sys.fulltext_indexes WHERE object_id = OBJECT_ID('AuditMessages'))
+        BEGIN
+            EXEC('CREATE FULLTEXT INDEX ON AuditMessages(HeadersJson LANGUAGE 0, BodyText LANGUAGE 0)
+                      KEY INDEX IX_AuditMessages_Id
+                      ON ServiceControlFullTextCatalog
+                      WITH (CHANGE_TRACKING AUTO, STOPLIST = OFF)');
+        END

src/ServiceControl.Audit.Persistence.Tests.SqlServer/SlowMigrationOperationApprovals.cs

@@ -0,0 +1,29 @@
+namespace ServiceControl.Audit.Persistence.Tests
+{
+    using System;
+    using Microsoft.Extensions.DependencyInjection;
+    using NUnit.Framework;
+    using ServiceControl.Audit.Persistence.EFCore.Abstractions;
+    using ServiceControl.Audit.Persistence.EFCore.DbContexts;
+    using ServiceControl.Audit.Persistence.EFCore.SqlServer;
+    using TestHelper;
+
+    [TestFixture]
+    class SlowMigrationOperationApprovals
+    {
+        [Test]
+        public void Verify_slow_migration_operations_are_unchanged()
+        {
+            var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000);
+            settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = "Server=unused";
+
+            var services = new ServiceCollection();
+            new SqlServerPersistenceConfiguration().Create(settings).AddInstaller(services);
+
+            using var serviceProvider = services.BuildServiceProvider();
+            using var scope = serviceProvider.CreateScope();
+
+            SlowMigrationOperations.Verify(scope.ServiceProvider.GetRequiredService<AuditDbContext>());
+        }
+    }
+}

src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt

@@ -0,0 +1,23 @@
+20260902040422_AddFullTextSearch
+    Sql
+        CREATE INDEX ix_failed_messages_full_text ON failed_messages USING GIN (to_tsvector('simple', headers_json || ' ' || COALESCE(body_text, '') || ' ' || replace(replace(COALESCE(message_type, ''), '.', ' '), '+', ' ')))
+20260911041328_AddMissingIndexes
+    DropIndex ix_failed_messages_status_last_modified on failed_messages
+    DropIndex ix_failed_message_groups_type_group_id on failed_message_groups
+    AlterColumn failed_messages.message_type text null -> character varying(450) null
+    CreateIndex ix_failed_messages_status_last_modified on failed_messages (status, last_modified) Npgsql:IndexInclude=[first_time_of_failure, last_time_of_failure]
+    CreateIndex ix_failed_messages_status_last_time_of_failure on failed_messages (status, last_time_of_failure)
+    CreateIndex ix_failed_messages_status_message_type_unique_message_id on failed_messages (status, message_type, unique_message_id)
+    CreateIndex ix_failed_message_groups_type_group_id on failed_message_groups (type, group_id) Npgsql:IndexInclude=[title]
+20260915050538_AddAdditionalIndexCoverage
+    DropIndex ix_failed_messages_status_last_modified on failed_messages
+    DropIndex ix_failed_message_groups_type_group_id on failed_message_groups
+    CreateIndex ix_failed_messages_status_last_modified_unique_message_id on failed_messages (status, last_modified, unique_message_id) Npgsql:IndexInclude=[first_time_of_failure, last_time_of_failure]
+    CreateIndex ix_failed_message_groups_type_group_id on failed_message_groups (type, group_id) Npgsql:IndexInclude=[failed_message_unique_id, title]
+20260928064706_WidenMessageIdAndCapFullTextBody
+    AlterColumn failed_messages.message_id character varying(450) null -> text null
+    AlterColumn failed_error_imports.message_id character varying(450) not null -> text not null
+    Sql
+        DROP INDEX IF EXISTS ix_failed_messages_full_text
+    Sql
+        CREATE INDEX ix_failed_messages_full_text ON failed_messages USING GIN (to_tsvector('simple', headers_json || ' ' || substring(COALESCE(body_text, ''), 1, 262144) || ' ' || replace(replace(COALESCE(message_type, ''), '.', ' '), '+', ' ')))

src/ServiceControl.Persistence.Tests.PostgreSql/SlowMigrationOperationApprovals.cs

@@ -0,0 +1,31 @@
+// ReSharper disable once CheckNamespace
+namespace ServiceControl.Persistence.Tests;
+
+using EFCore.PostgreSql;
+using Microsoft.Extensions.DependencyInjection;
+using NUnit.Framework;
+using ServiceControl.Persistence.EFCore.Abstractions;
+using ServiceControl.Persistence.EFCore.DbContexts;
+using TestHelper;
+
+[TestFixture]
+class SlowMigrationOperationApprovals
+{
+    [Test]
+    public void Verify_slow_migration_operations_are_unchanged()
+    {
+        var settings = new PostgreSqlPersisterSettings
+        {
+            ConnectionString = "Host=unused",
+            BodyStorage = new FileSystemBodyStorageSettings { StoragePath = "unused" }
+        };
+
+        var services = new ServiceCollection();
+        new PostgreSqlPersistenceConfiguration().Create(settings).AddInstaller(services);
+
+        using var serviceProvider = services.BuildServiceProvider();
+        using var scope = serviceProvider.CreateScope();
+
+        SlowMigrationOperations.Verify(scope.ServiceProvider.GetRequiredService<ServiceControlDbContext>());
+    }
+}

src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt

@@ -0,0 +1,35 @@
+20260902040432_AddFullTextSearch
+    Sql
+        IF SERVERPROPERTY('IsFullTextInstalled') <> 1
+        BEGIN
+            THROW 50000, 'ServiceControl requires the SQL Server Full-Text Search feature, which is not installed on this instance. Install it and run setup again.', 1;
+        END
+    Sql
+        IF NOT EXISTS (SELECT 1 FROM sys.fulltext_catalogs WHERE name = 'ServiceControlFullTextCatalog')
+        BEGIN
+            BEGIN TRY
+                EXEC('CREATE FULLTEXT CATALOG ServiceControlFullTextCatalog');
+            END TRY
+            BEGIN CATCH
+                IF NOT EXISTS (SELECT 1 FROM sys.fulltext_catalogs WHERE name = 'ServiceControlFullTextCatalog') THROW;
+            END CATCH
+        END
+    Sql
+        IF NOT EXISTS (SELECT 1 FROM sys.fulltext_indexes WHERE object_id = OBJECT_ID('FailedMessages'))
+        BEGIN
+            EXEC('CREATE FULLTEXT INDEX ON FailedMessages(HeadersJson LANGUAGE 0, BodyText LANGUAGE 0)
+                      KEY INDEX PK_FailedMessages
+                      ON ServiceControlFullTextCatalog
+                      WITH (CHANGE_TRACKING AUTO, STOPLIST = OFF)');
+        END
+20260911041253_AddMissingIndexes
+    DropIndex IX_FailedMessages_Status_LastModified on FailedMessages
+    DropIndex IX_FailedMessageGroups_Type_GroupId on FailedMessageGroups
+    AlterColumn FailedMessages.MessageType nvarchar(max) null -> nvarchar(450) null
+    CreateIndex IX_FailedMessages_Status_LastModified on FailedMessages (Status, LastModified) SqlServer:Include=[FirstTimeOfFailure, LastTimeOfFailure]
+    CreateIndex IX_FailedMessages_Status_LastTimeOfFailure on FailedMessages (Status, LastTimeOfFailure)
+    CreateIndex IX_FailedMessages_Status_MessageType_UniqueMessageId on FailedMessages (Status, MessageType, UniqueMessageId)
+    CreateIndex IX_FailedMessageGroups_Type_GroupId on FailedMessageGroups (Type, GroupId) SqlServer:Include=[Title]
+20260928064709_WidenMessageId
+    AlterColumn FailedMessages.MessageId nvarchar(450) null -> nvarchar(max) null
+    AlterColumn FailedErrorImports.MessageId nvarchar(450) not null -> nvarchar(max) not null

src/ServiceControl.Persistence.Tests.SqlServer/SlowMigrationOperationApprovals.cs

@@ -0,0 +1,31 @@
+// ReSharper disable once CheckNamespace
+namespace ServiceControl.Persistence.Tests;
+
+using EFCore.SqlServer;
+using Microsoft.Extensions.DependencyInjection;
+using NUnit.Framework;
+using ServiceControl.Persistence.EFCore.Abstractions;
+using ServiceControl.Persistence.EFCore.DbContexts;
+using TestHelper;
+
+[TestFixture]
+class SlowMigrationOperationApprovals
+{
+    [Test]
+    public void Verify_slow_migration_operations_are_unchanged()
+    {
+        var settings = new SqlServerPersisterSettings
+        {
+            ConnectionString = "Server=unused",
+            BodyStorage = new FileSystemBodyStorageSettings { StoragePath = "unused" }
+        };
+
+        var services = new ServiceCollection();
+        new SqlServerPersistenceConfiguration().Create(settings).AddInstaller(services);
+
+        using var serviceProvider = services.BuildServiceProvider();
+        using var scope = serviceProvider.CreateScope();
+
+        SlowMigrationOperations.Verify(scope.ServiceProvider.GetRequiredService<ServiceControlDbContext>());
+    }
+}

src/TestHelper/SlowMigrationOperations.cs

@@ -0,0 +1,105 @@
+namespace TestHelper;
+
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.Linq;
+using System.Runtime.CompilerServices;
+using System.Text;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Migrations;
+using Microsoft.EntityFrameworkCore.Migrations.Operations;
+using Particular.Approvals;
+
+public static class SlowMigrationOperations
+{
+    const string ChangedMessage =
+        "The migration steps that can block writes on a large table do not match the approved file. Index, key, foreign key, check constraint, column and raw SQL changes can block writes for a long time during an upgrade. " +
+        "If the diff shows a changed step, record it in the upgrade guide of the next release. If the step is in an existing migration, add a new migration instead. " +
+        "Then approve the new list. See docs/testing.md.";
+
+    /// <summary>
+    /// Compares the slow steps in the migrations of <paramref name="context"/> with the approved file for the calling test.
+    /// The slow steps are index, primary key and unique constraint changes, new foreign keys and check constraints, altered columns, raw SQL,
+    /// and new columns that are NOT NULL with a default or are stored computed columns.
+    /// Other steps, such as renames and drops of columns, tables and constraints, are not listed.
+    /// Steps on a table that the same migration creates are left out, because that table starts empty. Raw SQL is always listed.
+    /// When the list differs, it throws an exception that says what to check before approving the new list, with the approval failure as the inner exception.
+    /// </summary>
+    public static void Verify(DbContext context, [CallerFilePath] string callerFilePath = null, [CallerMemberName] string callerMemberName = null)
+    {
+        var steps = Describe(context);
+
+        try
+        {
+            Approver.Verify(steps, callerFilePath: callerFilePath, callerMemberName: callerMemberName);
+        }
+        catch (Exception e)
+        {
+            throw new Exception(ChangedMessage, e);
+        }
+    }
+
+    static string Describe(DbContext context)
+    {
+        var migrationsAssembly = context.GetService<IMigrationsAssembly>();
+        var text = new StringBuilder();
+
+        foreach (var (id, type) in migrationsAssembly.Migrations.OrderBy(migration => migration.Key, StringComparer.Ordinal))
+        {
+            var operations = migrationsAssembly.CreateMigration(type, context.Database.ProviderName).UpOperations;
+            var newTables = operations.OfType<CreateTableOperation>().Select(table => table.Name).ToHashSet(StringComparer.Ordinal);
+            var steps = operations.Select(operation => Describe(operation, newTables)).OfType<string>().ToList();
+
+            if (steps.Count == 0)
+            {
+                continue;
+            }
+
+            text.AppendLine(id);
+            foreach (var step in steps)
+            {
+                text.AppendLine(Indent(step));
+            }
+        }
+
+        return text.ToString();
+    }
+
+    static string Describe(MigrationOperation operation, HashSet<string> newTables) => operation switch
+    {
+        SqlOperation sql => $"Sql{Environment.NewLine}{Indent(sql.Sql)}",
+        ITableMigrationOperation table when newTables.Contains(table.Table) => null,
+        CreateIndexOperation index => $"CreateIndex {index.Name} on {index.Table} ({IndexColumns(index)}){(index.IsUnique ? " unique" : "")}{(index.Filter is null ? "" : $" where {index.Filter}")}{Annotations(index)}",
+        DropIndexOperation index => $"DropIndex {index.Name} on {index.Table}",
+        AddPrimaryKeyOperation key => $"AddPrimaryKey {key.Name} on {key.Table} ({string.Join(", ", key.Columns)}){Annotations(key)}",
+        DropPrimaryKeyOperation key => $"DropPrimaryKey {key.Name} on {key.Table}",
+        AddUniqueConstraintOperation constraint => $"AddUniqueConstraint {constraint.Name} on {constraint.Table} ({string.Join(", ", constraint.Columns)}){Annotations(constraint)}",
+        DropUniqueConstraintOperation constraint => $"DropUniqueConstraint {constraint.Name} on {constraint.Table}",
+        AddCheckConstraintOperation constraint => $"AddCheckConstraint {constraint.Name} on {constraint.Table} ({constraint.Sql})",
+        AddForeignKeyOperation key => $"AddForeignKey {key.Name} on {key.Table} ({string.Join(", ", key.Columns)}) references {key.PrincipalTable} ({string.Join(", ", key.PrincipalColumns ?? [])})",
+        AlterColumnOperation column => $"AlterColumn {column.Table}.{column.Name} {ColumnType(column.OldColumn)} -> {ColumnType(column)}",
+        AddColumnOperation column when CanRewriteEveryRow(column) => $"AddColumn {column.Table}.{column.Name} {ColumnType(column)}{ColumnValue(column)}",
+        _ => null
+    };
+
+    static bool CanRewriteEveryRow(AddColumnOperation column) =>
+        (!column.IsNullable && (column.DefaultValue is not null || column.DefaultValueSql is not null)) || (column.ComputedColumnSql is not null && column.IsStored == true);
+
+    static string ColumnValue(ColumnOperation column) =>
+        column.ComputedColumnSql is not null ? $" computed {column.ComputedColumnSql}"
+        : column.DefaultValueSql is not null ? $" default {column.DefaultValueSql}"
+        : $" default {Convert.ToString(column.DefaultValue, CultureInfo.InvariantCulture)}";
+
+    // An empty IsDescending array means every column is descending.
+    static string IndexColumns(CreateIndexOperation index) => string.Join(", ", index.Columns.Select((column, i) =>
+        index.IsDescending is { } descending && (descending.Length == 0 || descending[i]) ? $"{column} desc" : column));
+
+    static string ColumnType(ColumnOperation column) => $"{column.ColumnType ?? column.ClrType.Name} {(column.IsNullable ? "null" : "not null")}";
+
+    static string Annotations(MigrationOperation operation) => string.Concat(operation.GetAnnotations().Select(annotation =>
+        $" {annotation.Name}={(annotation.Value is IEnumerable<string> values ? $"[{string.Join(", ", values)}]" : annotation.Value)}"));
+
+    static string Indent(string text) => string.Join(Environment.NewLine, text.Split('\n').Select(line => line.TrimEnd('\r')).Select(line => line.Length == 0 ? line : "    " + line));
+}

src/TestHelper/TestHelper.csproj

@@ -5,8 +5,10 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="Microsoft.EntityFrameworkCore.Relational" />
     <PackageReference Include="NUnit" />
     <PackageReference Include="NUnit.Analyzers" />
+    <PackageReference Include="Particular.Approvals" />
     <PackageReference Include="RavenDB.Embedded" />
   </ItemGroup>