diff --git a/docs/testing.md b/docs/testing.md index af20a9d4f9..f7a995eebc 100644 --- a/docs/testing.md +++ b/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. diff --git a/src/ServiceControl.Audit.Persistence.Tests.PostgreSql/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt b/src/ServiceControl.Audit.Persistence.Tests.PostgreSql/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt new file mode 100644 index 0000000000..48a74dddcc --- /dev/null +++ b/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, ''), '.', ' '), '+', ' '))) diff --git a/src/ServiceControl.Audit.Persistence.Tests.PostgreSql/SlowMigrationOperationApprovals.cs b/src/ServiceControl.Audit.Persistence.Tests.PostgreSql/SlowMigrationOperationApprovals.cs new file mode 100644 index 0000000000..2119122cc9 --- /dev/null +++ b/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()); + } + } +} diff --git a/src/ServiceControl.Audit.Persistence.Tests.SqlServer/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt b/src/ServiceControl.Audit.Persistence.Tests.SqlServer/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt new file mode 100644 index 0000000000..16e598b9a5 --- /dev/null +++ b/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 diff --git a/src/ServiceControl.Audit.Persistence.Tests.SqlServer/SlowMigrationOperationApprovals.cs b/src/ServiceControl.Audit.Persistence.Tests.SqlServer/SlowMigrationOperationApprovals.cs new file mode 100644 index 0000000000..493c8c6a87 --- /dev/null +++ b/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()); + } + } +} diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt b/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt new file mode 100644 index 0000000000..55879e2ebc --- /dev/null +++ b/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, ''), '.', ' '), '+', ' '))) diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/SlowMigrationOperationApprovals.cs b/src/ServiceControl.Persistence.Tests.PostgreSql/SlowMigrationOperationApprovals.cs new file mode 100644 index 0000000000..535be80ef4 --- /dev/null +++ b/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()); + } +} diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt b/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/SlowMigrationOperationApprovals.Verify_slow_migration_operations_are_unchanged.approved.txt new file mode 100644 index 0000000000..9f6affc840 --- /dev/null +++ b/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 diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/SlowMigrationOperationApprovals.cs b/src/ServiceControl.Persistence.Tests.SqlServer/SlowMigrationOperationApprovals.cs new file mode 100644 index 0000000000..719705d1bb --- /dev/null +++ b/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()); + } +} diff --git a/src/TestHelper/SlowMigrationOperations.cs b/src/TestHelper/SlowMigrationOperations.cs new file mode 100644 index 0000000000..89e21da883 --- /dev/null +++ b/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."; + + /// + /// Compares the slow steps in the migrations of 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. + /// + 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(); + 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().Select(table => table.Name).ToHashSet(StringComparer.Ordinal); + var steps = operations.Select(operation => Describe(operation, newTables)).OfType().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 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 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)); +} diff --git a/src/TestHelper/TestHelper.csproj b/src/TestHelper/TestHelper.csproj index b6077f151d..ec9704188c 100644 --- a/src/TestHelper/TestHelper.csproj +++ b/src/TestHelper/TestHelper.csproj @@ -5,8 +5,10 @@ + +