Skip to content

Commit 1a7c8b1

Browse files
Approve EF migration scripts to catch index changes (#6045)
* Add migration script approval tests for PostgreSQL and SQL Server * Add tests for slow migration operations and remove obsolete migration scripts * Enhance SlowMigrationOperation approvals to include new foreign keys, check constraints, and NOT NULL columns with defaults in migration tests
1 parent 77449c3 commit 1a7c8b1

11 files changed

Lines changed: 314 additions & 0 deletions

‎docs/testing.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ On Windows, [PackagedRavenDBTests](../src/ServiceControlInstaller.Engine.UnitTes
2727
Persistence tests check assumptions at the persistence seam level by exercising each persister.
2828
For local setup details, see [Local testing of persistence providers](testing-persistence.md).
2929

30+
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.
31+
3032
## Transport tests
3133

3234
Transport tests run the transport test suite for each transport.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
20260928032745_AddFullTextSearch
2+
Sql
3+
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, ''), '.', ' '), '+', ' ')))
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
namespace ServiceControl.Audit.Persistence.Tests
2+
{
3+
using System;
4+
using Microsoft.Extensions.DependencyInjection;
5+
using NUnit.Framework;
6+
using ServiceControl.Audit.Persistence.EFCore.Abstractions;
7+
using ServiceControl.Audit.Persistence.EFCore.DbContexts;
8+
using ServiceControl.Audit.Persistence.EFCore.PostgreSql;
9+
using TestHelper;
10+
11+
[TestFixture]
12+
class SlowMigrationOperationApprovals
13+
{
14+
[Test]
15+
public void Verify_slow_migration_operations_are_unchanged()
16+
{
17+
var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000);
18+
settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = "Host=unused";
19+
20+
var services = new ServiceCollection();
21+
new PostgreSqlPersistenceConfiguration().Create(settings).AddInstaller(services);
22+
23+
using var serviceProvider = services.BuildServiceProvider();
24+
using var scope = serviceProvider.CreateScope();
25+
26+
SlowMigrationOperations.Verify(scope.ServiceProvider.GetRequiredService<AuditDbContext>());
27+
}
28+
}
29+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
20260928032751_AddFullTextSearch
2+
Sql
3+
IF SERVERPROPERTY('IsFullTextInstalled') <> 1
4+
BEGIN
5+
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;
6+
END
7+
Sql
8+
IF NOT EXISTS (SELECT 1 FROM sys.fulltext_catalogs WHERE name = 'ServiceControlFullTextCatalog')
9+
BEGIN
10+
BEGIN TRY
11+
EXEC('CREATE FULLTEXT CATALOG ServiceControlFullTextCatalog');
12+
END TRY
13+
BEGIN CATCH
14+
IF NOT EXISTS (SELECT 1 FROM sys.fulltext_catalogs WHERE name = 'ServiceControlFullTextCatalog') THROW;
15+
END CATCH
16+
END
17+
Sql
18+
IF NOT EXISTS (SELECT 1 FROM sys.fulltext_indexes WHERE object_id = OBJECT_ID('AuditMessages'))
19+
BEGIN
20+
EXEC('CREATE FULLTEXT INDEX ON AuditMessages(HeadersJson LANGUAGE 0, BodyText LANGUAGE 0)
21+
KEY INDEX IX_AuditMessages_Id
22+
ON ServiceControlFullTextCatalog
23+
WITH (CHANGE_TRACKING AUTO, STOPLIST = OFF)');
24+
END
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
namespace ServiceControl.Audit.Persistence.Tests
2+
{
3+
using System;
4+
using Microsoft.Extensions.DependencyInjection;
5+
using NUnit.Framework;
6+
using ServiceControl.Audit.Persistence.EFCore.Abstractions;
7+
using ServiceControl.Audit.Persistence.EFCore.DbContexts;
8+
using ServiceControl.Audit.Persistence.EFCore.SqlServer;
9+
using TestHelper;
10+
11+
[TestFixture]
12+
class SlowMigrationOperationApprovals
13+
{
14+
[Test]
15+
public void Verify_slow_migration_operations_are_unchanged()
16+
{
17+
var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000);
18+
settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = "Server=unused";
19+
20+
var services = new ServiceCollection();
21+
new SqlServerPersistenceConfiguration().Create(settings).AddInstaller(services);
22+
23+
using var serviceProvider = services.BuildServiceProvider();
24+
using var scope = serviceProvider.CreateScope();
25+
26+
SlowMigrationOperations.Verify(scope.ServiceProvider.GetRequiredService<AuditDbContext>());
27+
}
28+
}
29+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
20260902040422_AddFullTextSearch
2+
Sql
3+
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, ''), '.', ' '), '+', ' ')))
4+
20260911041328_AddMissingIndexes
5+
DropIndex ix_failed_messages_status_last_modified on failed_messages
6+
DropIndex ix_failed_message_groups_type_group_id on failed_message_groups
7+
AlterColumn failed_messages.message_type text null -> character varying(450) null
8+
CreateIndex ix_failed_messages_status_last_modified on failed_messages (status, last_modified) Npgsql:IndexInclude=[first_time_of_failure, last_time_of_failure]
9+
CreateIndex ix_failed_messages_status_last_time_of_failure on failed_messages (status, last_time_of_failure)
10+
CreateIndex ix_failed_messages_status_message_type_unique_message_id on failed_messages (status, message_type, unique_message_id)
11+
CreateIndex ix_failed_message_groups_type_group_id on failed_message_groups (type, group_id) Npgsql:IndexInclude=[title]
12+
20260915050538_AddAdditionalIndexCoverage
13+
DropIndex ix_failed_messages_status_last_modified on failed_messages
14+
DropIndex ix_failed_message_groups_type_group_id on failed_message_groups
15+
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]
16+
CreateIndex ix_failed_message_groups_type_group_id on failed_message_groups (type, group_id) Npgsql:IndexInclude=[failed_message_unique_id, title]
17+
20260928064706_WidenMessageIdAndCapFullTextBody
18+
AlterColumn failed_messages.message_id character varying(450) null -> text null
19+
AlterColumn failed_error_imports.message_id character varying(450) not null -> text not null
20+
Sql
21+
DROP INDEX IF EXISTS ix_failed_messages_full_text
22+
Sql
23+
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, ''), '.', ' '), '+', ' ')))
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// ReSharper disable once CheckNamespace
2+
namespace ServiceControl.Persistence.Tests;
3+
4+
using EFCore.PostgreSql;
5+
using Microsoft.Extensions.DependencyInjection;
6+
using NUnit.Framework;
7+
using ServiceControl.Persistence.EFCore.Abstractions;
8+
using ServiceControl.Persistence.EFCore.DbContexts;
9+
using TestHelper;
10+
11+
[TestFixture]
12+
class SlowMigrationOperationApprovals
13+
{
14+
[Test]
15+
public void Verify_slow_migration_operations_are_unchanged()
16+
{
17+
var settings = new PostgreSqlPersisterSettings
18+
{
19+
ConnectionString = "Host=unused",
20+
BodyStorage = new FileSystemBodyStorageSettings { StoragePath = "unused" }
21+
};
22+
23+
var services = new ServiceCollection();
24+
new PostgreSqlPersistenceConfiguration().Create(settings).AddInstaller(services);
25+
26+
using var serviceProvider = services.BuildServiceProvider();
27+
using var scope = serviceProvider.CreateScope();
28+
29+
SlowMigrationOperations.Verify(scope.ServiceProvider.GetRequiredService<ServiceControlDbContext>());
30+
}
31+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
20260902040432_AddFullTextSearch
2+
Sql
3+
IF SERVERPROPERTY('IsFullTextInstalled') <> 1
4+
BEGIN
5+
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;
6+
END
7+
Sql
8+
IF NOT EXISTS (SELECT 1 FROM sys.fulltext_catalogs WHERE name = 'ServiceControlFullTextCatalog')
9+
BEGIN
10+
BEGIN TRY
11+
EXEC('CREATE FULLTEXT CATALOG ServiceControlFullTextCatalog');
12+
END TRY
13+
BEGIN CATCH
14+
IF NOT EXISTS (SELECT 1 FROM sys.fulltext_catalogs WHERE name = 'ServiceControlFullTextCatalog') THROW;
15+
END CATCH
16+
END
17+
Sql
18+
IF NOT EXISTS (SELECT 1 FROM sys.fulltext_indexes WHERE object_id = OBJECT_ID('FailedMessages'))
19+
BEGIN
20+
EXEC('CREATE FULLTEXT INDEX ON FailedMessages(HeadersJson LANGUAGE 0, BodyText LANGUAGE 0)
21+
KEY INDEX PK_FailedMessages
22+
ON ServiceControlFullTextCatalog
23+
WITH (CHANGE_TRACKING AUTO, STOPLIST = OFF)');
24+
END
25+
20260911041253_AddMissingIndexes
26+
DropIndex IX_FailedMessages_Status_LastModified on FailedMessages
27+
DropIndex IX_FailedMessageGroups_Type_GroupId on FailedMessageGroups
28+
AlterColumn FailedMessages.MessageType nvarchar(max) null -> nvarchar(450) null
29+
CreateIndex IX_FailedMessages_Status_LastModified on FailedMessages (Status, LastModified) SqlServer:Include=[FirstTimeOfFailure, LastTimeOfFailure]
30+
CreateIndex IX_FailedMessages_Status_LastTimeOfFailure on FailedMessages (Status, LastTimeOfFailure)
31+
CreateIndex IX_FailedMessages_Status_MessageType_UniqueMessageId on FailedMessages (Status, MessageType, UniqueMessageId)
32+
CreateIndex IX_FailedMessageGroups_Type_GroupId on FailedMessageGroups (Type, GroupId) SqlServer:Include=[Title]
33+
20260928064709_WidenMessageId
34+
AlterColumn FailedMessages.MessageId nvarchar(450) null -> nvarchar(max) null
35+
AlterColumn FailedErrorImports.MessageId nvarchar(450) not null -> nvarchar(max) not null
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// ReSharper disable once CheckNamespace
2+
namespace ServiceControl.Persistence.Tests;
3+
4+
using EFCore.SqlServer;
5+
using Microsoft.Extensions.DependencyInjection;
6+
using NUnit.Framework;
7+
using ServiceControl.Persistence.EFCore.Abstractions;
8+
using ServiceControl.Persistence.EFCore.DbContexts;
9+
using TestHelper;
10+
11+
[TestFixture]
12+
class SlowMigrationOperationApprovals
13+
{
14+
[Test]
15+
public void Verify_slow_migration_operations_are_unchanged()
16+
{
17+
var settings = new SqlServerPersisterSettings
18+
{
19+
ConnectionString = "Server=unused",
20+
BodyStorage = new FileSystemBodyStorageSettings { StoragePath = "unused" }
21+
};
22+
23+
var services = new ServiceCollection();
24+
new SqlServerPersistenceConfiguration().Create(settings).AddInstaller(services);
25+
26+
using var serviceProvider = services.BuildServiceProvider();
27+
using var scope = serviceProvider.CreateScope();
28+
29+
SlowMigrationOperations.Verify(scope.ServiceProvider.GetRequiredService<ServiceControlDbContext>());
30+
}
31+
}
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
namespace TestHelper;
2+
3+
using System;
4+
using System.Collections.Generic;
5+
using System.Globalization;
6+
using System.Linq;
7+
using System.Runtime.CompilerServices;
8+
using System.Text;
9+
using Microsoft.EntityFrameworkCore;
10+
using Microsoft.EntityFrameworkCore.Infrastructure;
11+
using Microsoft.EntityFrameworkCore.Migrations;
12+
using Microsoft.EntityFrameworkCore.Migrations.Operations;
13+
using Particular.Approvals;
14+
15+
public static class SlowMigrationOperations
16+
{
17+
const string ChangedMessage =
18+
"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. " +
19+
"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. " +
20+
"Then approve the new list. See docs/testing.md.";
21+
22+
/// <summary>
23+
/// Compares the slow steps in the migrations of <paramref name="context"/> with the approved file for the calling test.
24+
/// The slow steps are index, primary key and unique constraint changes, new foreign keys and check constraints, altered columns, raw SQL,
25+
/// and new columns that are NOT NULL with a default or are stored computed columns.
26+
/// Other steps, such as renames and drops of columns, tables and constraints, are not listed.
27+
/// Steps on a table that the same migration creates are left out, because that table starts empty. Raw SQL is always listed.
28+
/// 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.
29+
/// </summary>
30+
public static void Verify(DbContext context, [CallerFilePath] string callerFilePath = null, [CallerMemberName] string callerMemberName = null)
31+
{
32+
var steps = Describe(context);
33+
34+
try
35+
{
36+
Approver.Verify(steps, callerFilePath: callerFilePath, callerMemberName: callerMemberName);
37+
}
38+
catch (Exception e)
39+
{
40+
throw new Exception(ChangedMessage, e);
41+
}
42+
}
43+
44+
static string Describe(DbContext context)
45+
{
46+
var migrationsAssembly = context.GetService<IMigrationsAssembly>();
47+
var text = new StringBuilder();
48+
49+
foreach (var (id, type) in migrationsAssembly.Migrations.OrderBy(migration => migration.Key, StringComparer.Ordinal))
50+
{
51+
var operations = migrationsAssembly.CreateMigration(type, context.Database.ProviderName).UpOperations;
52+
var newTables = operations.OfType<CreateTableOperation>().Select(table => table.Name).ToHashSet(StringComparer.Ordinal);
53+
var steps = operations.Select(operation => Describe(operation, newTables)).OfType<string>().ToList();
54+
55+
if (steps.Count == 0)
56+
{
57+
continue;
58+
}
59+
60+
text.AppendLine(id);
61+
foreach (var step in steps)
62+
{
63+
text.AppendLine(Indent(step));
64+
}
65+
}
66+
67+
return text.ToString();
68+
}
69+
70+
static string Describe(MigrationOperation operation, HashSet<string> newTables) => operation switch
71+
{
72+
SqlOperation sql => $"Sql{Environment.NewLine}{Indent(sql.Sql)}",
73+
ITableMigrationOperation table when newTables.Contains(table.Table) => null,
74+
CreateIndexOperation index => $"CreateIndex {index.Name} on {index.Table} ({IndexColumns(index)}){(index.IsUnique ? " unique" : "")}{(index.Filter is null ? "" : $" where {index.Filter}")}{Annotations(index)}",
75+
DropIndexOperation index => $"DropIndex {index.Name} on {index.Table}",
76+
AddPrimaryKeyOperation key => $"AddPrimaryKey {key.Name} on {key.Table} ({string.Join(", ", key.Columns)}){Annotations(key)}",
77+
DropPrimaryKeyOperation key => $"DropPrimaryKey {key.Name} on {key.Table}",
78+
AddUniqueConstraintOperation constraint => $"AddUniqueConstraint {constraint.Name} on {constraint.Table} ({string.Join(", ", constraint.Columns)}){Annotations(constraint)}",
79+
DropUniqueConstraintOperation constraint => $"DropUniqueConstraint {constraint.Name} on {constraint.Table}",
80+
AddCheckConstraintOperation constraint => $"AddCheckConstraint {constraint.Name} on {constraint.Table} ({constraint.Sql})",
81+
AddForeignKeyOperation key => $"AddForeignKey {key.Name} on {key.Table} ({string.Join(", ", key.Columns)}) references {key.PrincipalTable} ({string.Join(", ", key.PrincipalColumns ?? [])})",
82+
AlterColumnOperation column => $"AlterColumn {column.Table}.{column.Name} {ColumnType(column.OldColumn)} -> {ColumnType(column)}",
83+
AddColumnOperation column when CanRewriteEveryRow(column) => $"AddColumn {column.Table}.{column.Name} {ColumnType(column)}{ColumnValue(column)}",
84+
_ => null
85+
};
86+
87+
static bool CanRewriteEveryRow(AddColumnOperation column) =>
88+
(!column.IsNullable && (column.DefaultValue is not null || column.DefaultValueSql is not null)) || (column.ComputedColumnSql is not null && column.IsStored == true);
89+
90+
static string ColumnValue(ColumnOperation column) =>
91+
column.ComputedColumnSql is not null ? $" computed {column.ComputedColumnSql}"
92+
: column.DefaultValueSql is not null ? $" default {column.DefaultValueSql}"
93+
: $" default {Convert.ToString(column.DefaultValue, CultureInfo.InvariantCulture)}";
94+
95+
// An empty IsDescending array means every column is descending.
96+
static string IndexColumns(CreateIndexOperation index) => string.Join(", ", index.Columns.Select((column, i) =>
97+
index.IsDescending is { } descending && (descending.Length == 0 || descending[i]) ? $"{column} desc" : column));
98+
99+
static string ColumnType(ColumnOperation column) => $"{column.ColumnType ?? column.ClrType.Name} {(column.IsNullable ? "null" : "not null")}";
100+
101+
static string Annotations(MigrationOperation operation) => string.Concat(operation.GetAnnotations().Select(annotation =>
102+
$" {annotation.Name}={(annotation.Value is IEnumerable<string> values ? $"[{string.Join(", ", values)}]" : annotation.Value)}"));
103+
104+
static string Indent(string text) => string.Join(Environment.NewLine, text.Split('\n').Select(line => line.TrimEnd('\r')).Select(line => line.Length == 0 ? line : " " + line));
105+
}

0 commit comments

Comments
 (0)