From 239ae979bc95033e00e11044e4512fc0030f6852 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Thu, 22 Oct 2026 14:34:21 +0800 Subject: [PATCH 1/6] Add default schemas that are unique --- docs/README.md | 1 + docs/sql-persister-schemas.md | 12 ++++ docs/usage-report.md | 4 +- .../PostgreSqlAuditDbContext.cs | 5 ++ .../persistence.manifest | 3 +- .../SqlServerAuditDbContext.cs | 5 ++ .../persistence.manifest | 3 +- .../EFPersistenceConfigurationBase.cs | 4 +- .../Abstractions/EFPersisterSettings.cs | 1 + .../DbContexts/AuditDbContext.cs | 18 ++++- .../Implementation/DatabaseSetup.cs | 2 + .../EFCore/SchemaConfigurationTests.cs | 40 +++++++++++ .../EFCore/SchemaMustExistTests.cs | 61 ++++++++++++++-- .../PostgreSqlDatabaseMigrator.cs | 1 + .../PostgreSqlPersistence.cs | 4 +- .../PostgreSqlServiceControlDbContext.cs | 5 ++ .../persistence.manifest | 5 ++ .../SqlServerDatabaseMigrator.cs | 1 + .../SqlServerPersistence.cs | 4 +- .../SqlServerServiceControlDbContext.cs | 5 ++ .../persistence.manifest | 5 ++ .../EFPersistenceConfigurationBase.cs | 4 +- .../Abstractions/EFPersisterSettings.cs | 3 +- .../DbContexts/ServiceControlDbContext.cs | 17 +++++ .../EFEnvironmentDataProvider.cs | 2 +- .../Infrastructure/SchemaOptionsExtension.cs | 3 +- .../SchemaMustExistTests.cs | 61 ++++++++++++++-- .../TestSchema.cs | 3 + .../SchemaMustExistTests.cs | 61 ++++++++++++++-- .../TestSchema.cs | 3 + .../EnvironmentDataConfigurationTests.cs | 8 +++ .../EFCore/SchemaConfigurationTests.cs | 72 +++++++++++++++++++ 32 files changed, 389 insertions(+), 37 deletions(-) create mode 100644 docs/sql-persister-schemas.md create mode 100644 src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs create mode 100644 src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs diff --git a/docs/README.md b/docs/README.md index d177340078..690f42fa00 100644 --- a/docs/README.md +++ b/docs/README.md @@ -32,6 +32,7 @@ This section points to sources that explain why ServiceControl is designed the w - [Handling unavailable runtime dependencies](handling-unavailable-runtime-dependencies.md) — how instances react when a dependency is unavailable - [Telemetry](telemetry.md) — how to configure telemetry export and which metrics instances emit - [Throughput collection](throughput-collection.md) — how usage data is collected and why the throughput queue has a fixed name +- [SQL persister schemas](sql-persister-schemas.md) — which schema each SQL persister uses by default and why a primary and an audit instance cannot share one - [Usage report contents](usage-report.md): every key the usage report carries, every setting it leaves out, and why ## Decisions and rationale diff --git a/docs/sql-persister-schemas.md b/docs/sql-persister-schemas.md new file mode 100644 index 0000000000..c82f23963e --- /dev/null +++ b/docs/sql-persister-schemas.md @@ -0,0 +1,12 @@ +# SQL persister schemas + +The SQL Server and PostgreSQL persisters keep all of an instance's tables in one schema, set by `Database/Schema`. A blank or absent setting selects a default: + +| Instance | Default schema | +|---|---| +| Primary (error) | `psw_sc` | +| Audit | `psw_audit` | + +* ServiceControl does not create the schema. Create it, and grant the configured user rights on it, before running setup. +* A primary and an audit instance cannot share a schema. Setup fails before it applies any migration when the other instance's migrations history table is in the target schema: the primary rejects `__AuditMigrationsHistory`, and the audit instance rejects `__EFMigrationsHistory`. +* The primary instance's usage report lists `Storage.Schema` as `Default` for `psw_sc` and `Custom` for other configured schemas. See [Usage report contents](usage-report.md). diff --git a/docs/usage-report.md b/docs/usage-report.md index 3b56231f9a..60057f856b 100644 --- a/docs/usage-report.md +++ b/docs/usage-report.md @@ -88,7 +88,7 @@ Releases up to 6.21.2 report `SqlVersion` as the full text of `@@VERSION` on SQL | `Storage.FullTextSearch` | `Enabled`, `Disabled` | `EnableFullTextSearchOnBodies`, RavenDB only. Always `Enabled` on SQL Server and PostgreSQL, which do not read the setting | 6.20.0 | | `Storage.BodyStorage.Type` | `RavenAttachments`, `FileSystem`, `AzureBlob`, `S3` | the persister | 6.20.0 | | `Limits.MaxBodySizeToStore` | bytes | `MaxBodySizeToStore`, SQL Server and PostgreSQL only | 6.20.0 | -| `Storage.Schema` | `Default`, `Custom` | `Database/Schema`, SQL Server and PostgreSQL only | Unreleased | +| `Storage.Schema` | `Default` when the schema is `psw_sc`, `Custom` for other configured schemas | `Database/Schema`, SQL Server and PostgreSQL only | Unreleased | | `Storage.LogLevel` | `None`, `Information`, `Operations` | `RavenDBLogLevel`, RavenDB only | Unreleased | | `Storage.CommandTimeoutSeconds` | `Default` or number | `Database/CommandTimeout`, SQL Server and PostgreSQL only | Unreleased | | `Storage.QueryTimeoutSeconds` | `Default` or number | `QueryTimeoutInSeconds` | Unreleased | @@ -237,7 +237,7 @@ The ingestion and health keys follow the rules in [Error ingestion health](#erro | `Audit.SameMachine` | `True`, `False`, `Mixed`, `NotApplicable`, `Unknown` | the machine name hashes. `NotApplicable` when either instance runs in a container | Unreleased | | `Audit.DatabaseSharing` | `SameSchema`, `SameDatabase`, `SameServer`, `SeparateServer`, `NotApplicable`, `Unknown` | the storage identity hashes. `NotApplicable` when the storage engines differ. Several instances report the most shared class | Unreleased | -`Audit.Storage.SizeGB` covers the whole schema. When the primary and an audit instance share a schema, both report the same size, and `Audit.DatabaseSharing` is `SameSchema`. +`Audit.Storage.SizeGB` covers the whole schema. A primary and an audit instance on SQL Server or PostgreSQL cannot share a schema, so `SameSchema` is not reported for that pairing; see [SQL persister schemas](sql-persister-schemas.md). #### From the configuration response diff --git a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs index 4f65c76342..5f71dac00e 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs @@ -45,4 +45,9 @@ public override async Task SchemaExists(string schema, CancellationToken c await Database .SqlQueryRaw("""SELECT CASE WHEN EXISTS (SELECT 1 FROM pg_namespace WHERE nspname = {0}) THEN 1 ELSE 0 END AS "Value" """, schema) .SingleAsync(cancellationToken) == 1; + + protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => + await Database + .SqlQueryRaw("""SELECT CASE WHEN EXISTS (SELECT 1 FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE c.relkind = 'r' AND c.relname = {0} AND n.nspname = COALESCE(CAST({1} AS text), current_schema())) THEN 1 ELSE 0 END AS "Value" """, table, Schema ?? (object)DBNull.Value) + .SingleAsync(cancellationToken) == 1; } diff --git a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest index abff0adcaf..1c0fa78545 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest +++ b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest @@ -11,7 +11,8 @@ }, { "Name": "ServiceControl.Audit/Database/Schema", - "Mandatory": false + "Mandatory": false, + "DefaultValue": "psw_audit" }, { "Name": "ServiceControl.Audit/Database/CommandTimeout", diff --git a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs index 829680fecc..17f57dc763 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs @@ -32,4 +32,9 @@ public override async Task SchemaExists(string schema, CancellationToken c await Database .SqlQueryRaw("SELECT CASE WHEN SCHEMA_ID({0}) IS NULL THEN 0 ELSE 1 END AS [Value]", schema) .SingleAsync(cancellationToken) == 1; + + protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => + await Database + .SqlQueryRaw("SELECT CASE WHEN EXISTS (SELECT 1 FROM sys.tables WHERE name = {0} AND schema_id = SCHEMA_ID(COALESCE(CAST({1} AS sysname), SCHEMA_NAME()))) THEN 1 ELSE 0 END AS [Value]", table, Schema ?? (object)DBNull.Value) + .SingleAsync(cancellationToken) == 1; } diff --git a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest index 8c7628c10f..cc70c28d0e 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest +++ b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest @@ -11,7 +11,8 @@ }, { "Name": "ServiceControl.Audit/Database/Schema", - "Mandatory": false + "Mandatory": false, + "DefaultValue": "psw_audit" }, { "Name": "ServiceControl.Audit/Database/CommandTimeout", diff --git a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs index 4b1bb5da46..3ab25809af 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs @@ -41,11 +41,11 @@ public static EFPersisterSettings CreateSettings(PersistenceSettings settings) }; } - static string? ReadSchema(IDictionary specific) + static string ReadSchema(IDictionary specific) { if (!specific.TryGetValue(SchemaKey, out var schema) || string.IsNullOrWhiteSpace(schema)) { - return null; + return EFPersisterSettings.DefaultSchema; } try diff --git a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs index f441412deb..26b4136388 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs @@ -7,6 +7,7 @@ public class EFPersisterSettings public static readonly TimeSpan MigrationCommandTimeout = TimeSpan.FromMinutes(40); public const int DefaultCommandTimeout = 30; + public const string DefaultSchema = "psw_audit"; public required string ConnectionString { get; init; } diff --git a/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs b/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs index 284e90b81e..6f63bc9d68 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs @@ -7,9 +7,11 @@ namespace ServiceControl.Audit.Persistence.EFCore.DbContexts; public abstract class AuditDbContext(DbContextOptions options) : DbContext(options) { - // Not EF Core's default, so an audit instance can share a schema with a primary instance. public const string MigrationsHistoryTableName = "__AuditMigrationsHistory"; + // EF Core's default history table name, which identifies a primary instance that wrongly shares this schema. + const string PrimaryMigrationsHistoryTableName = "__EFMigrationsHistory"; + public string? Schema { get; } = options.FindExtension()?.Schema; public DbSet AuditMessages { get; set; } @@ -44,4 +46,18 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) public abstract bool IsDuplicateKeyException(DbUpdateException exception); public abstract Task SchemaExists(string schema, CancellationToken cancellationToken = default); + + /// + /// Whether the table exists in the configured schema, or in the provider default schema when none is configured. + /// + protected abstract Task TableExists(string table, CancellationToken cancellationToken = default); + + public async Task RequireNoPrimaryMigrationsHistory(CancellationToken cancellationToken = default) + { + if (await TableExists(PrimaryMigrationsHistoryTableName, cancellationToken)) + { + throw new InvalidOperationException( + $"The schema '{Schema ?? "(provider default)"}' already contains the primary migrations history table '{PrimaryMigrationsHistoryTableName}'. A primary instance and an audit instance cannot share a schema. Configure a different Database/Schema for this instance, create the schema, grant the configured user rights on it, and run setup again."); + } + } } diff --git a/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs b/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs index e36630c426..bac90a2a0a 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs @@ -26,6 +26,8 @@ public async Task StartAsync(CancellationToken cancellationToken = default) $"The configured schema '{dbContext.Schema}' does not exist in the database. ServiceControl does not create schemas, the same way it does not create the database. Create the schema, grant the configured user rights on it, and run setup again."); } + await dbContext.RequireNoPrimaryMigrationsHistory(cancellationToken); + logger.LogInformation("Starting audit database migration"); await dbContext.Database.MigrateAsync(cancellationToken); diff --git a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs new file mode 100644 index 0000000000..c9a36437f2 --- /dev/null +++ b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs @@ -0,0 +1,40 @@ +namespace ServiceControl.Audit.Persistence.Tests +{ + using System; + using NUnit.Framework; + using ServiceControl.Audit.Persistence.EFCore.Abstractions; + + [TestFixture] + class SchemaConfigurationTests + { + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + public void Absent_or_blank_schema_falls_back_to_psw_audit(string configured) + { + var settings = CreateSettings(); + if (configured != null) + { + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = configured; + } + + Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo("psw_audit")); + } + + [Test] + public void Configured_schema_is_used() + { + var settings = CreateSettings(); + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = "legacy"; + + Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo("legacy")); + } + + static PersistenceSettings CreateSettings() + { + var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000); + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = "Server=nowhere"; + return settings; + } + } +} diff --git a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs index 6ef1503937..56f62e8872 100644 --- a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs +++ b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs @@ -6,6 +6,7 @@ namespace ServiceControl.Audit.Persistence.Tests using Microsoft.Extensions.Hosting; using NUnit.Framework; using ServiceControl.Audit.Persistence.EFCore.Abstractions; + using ServiceControl.Persistence.Tests; [TestFixture] class SchemaMustExistTests @@ -14,19 +15,71 @@ class SchemaMustExistTests public async Task Setup_fails_when_the_configured_schema_does_not_exist() { var testsConfiguration = new PersistenceTestsConfiguration(); + var connectionString = await testsConfiguration.GetConnectionString(); var schema = $"sc_absent_{Guid.NewGuid():n}"; + var exception = Assert.ThrowsAsync(() => Setup(testsConfiguration, connectionString, schema)); + + Assert.That(exception.Message, Does.Contain(schema).And.Contain("does not exist")); + } + + [Test] + public async Task Setup_fails_when_the_schema_holds_the_primary_migrations_history_table() + { + var testsConfiguration = new PersistenceTestsConfiguration(); + var connectionString = await testsConfiguration.GetConnectionString(); + var schema = $"sc_shared_{Guid.NewGuid():n}"; + await TestSchema.Create(connectionString, schema); + + try + { + await TestSchema.CreateTable(connectionString, schema, "__EFMigrationsHistory"); + + var exception = Assert.ThrowsAsync(() => Setup(testsConfiguration, connectionString, schema)); + + Assert.That(exception.Message, Does.Contain(schema).And.Contain("__EFMigrationsHistory")); + } + finally + { + await TestSchema.Drop(connectionString, schema); + } + } + + [Test] + public async Task Setup_ignores_the_primary_migrations_history_table_in_another_schema() + { + var testsConfiguration = new PersistenceTestsConfiguration(); + var connectionString = await testsConfiguration.GetConnectionString(); + var schema = $"sc_audit_{Guid.NewGuid():n}"; + var primarySchema = $"sc_primary_{Guid.NewGuid():n}"; + await TestSchema.Create(connectionString, schema); + await TestSchema.Create(connectionString, primarySchema); + + try + { + await TestSchema.CreateTable(connectionString, primarySchema, "__EFMigrationsHistory"); + + Assert.DoesNotThrowAsync(() => Setup(testsConfiguration, connectionString, schema)); + } + finally + { + await TestSchema.Drop(connectionString, schema); + await TestSchema.Drop(connectionString, primarySchema); + } + } + + static async Task Setup(PersistenceTestsConfiguration testsConfiguration, string connectionString, string schema) + { var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000); - settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = await testsConfiguration.GetConnectionString(); + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = connectionString; settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = schema; var hostBuilder = Host.CreateApplicationBuilder(); testsConfiguration.CreateConfiguration().Create(settings).AddInstaller(hostBuilder.Services); using var host = hostBuilder.Build(); - var exception = Assert.ThrowsAsync(() => host.StartAsync()); - - Assert.That(exception.Message, Does.Contain(schema).And.Contain("does not exist")); + await host.StartAsync(); + await host.StopAsync(); } } } diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs index 1a87d914ec..e8d98ddb7a 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs @@ -15,6 +15,7 @@ public async Task ApplyMigrations(CancellationToken cancellationToken = default) dbContext.Database.SetCommandTimeout(EFPersisterSettings.MigrationCommandTimeout); await RequireSchema(cancellationToken); + await dbContext.RequireNoAuditMigrationsHistory(cancellationToken); await dbContext.Database.MigrateAsync(cancellationToken); dbContext.Database.SetCommandTimeout(previousTimeout); diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs index 423f1d8ba8..0abc56670e 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs @@ -73,9 +73,7 @@ void ConfigureDbContext(IServiceCollection services) options.ReplaceService(); options.ReplaceService(); - // HasDefaultSchema moves every table, so the model is meant to differ from the - // snapshot the migrations were scaffolded against. A default installation keeps the - // check, where a difference really would mean a migration is missing. + // Schema stamping intentionally differs from the schema-less snapshot; PendingModelChangesTests checks migration drift. options.ConfigureWarnings(warnings => warnings.Ignore(RelationalEventId.PendingModelChangesWarning)); } diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContext.cs b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContext.cs index 21ca3b0b4c..c2b9eb08cc 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContext.cs +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContext.cs @@ -49,4 +49,9 @@ public override bool IsDuplicateKeyException(DbUpdateException exception) } return false; } + + protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => + await Database + .SqlQueryRaw("""SELECT CASE WHEN EXISTS (SELECT 1 FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE c.relkind = 'r' AND c.relname = {0} AND n.nspname = COALESCE(CAST({1} AS text), current_schema())) THEN 1 ELSE 0 END AS "Value" """, table, Schema ?? (object)DBNull.Value) + .SingleAsync(cancellationToken) == 1; } diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest b/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest index 433c31a5ae..6d161b9b16 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest @@ -9,6 +9,11 @@ "Name": "ServiceControl/Database/ConnectionString", "Mandatory": true }, + { + "Name": "ServiceControl/Database/Schema", + "Mandatory": false, + "DefaultValue": "psw_sc" + }, { "Name": "ServiceControl/Database/CommandTimeout", "Mandatory": false diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs index bdd297c290..96a165f1bc 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs @@ -15,6 +15,7 @@ public async Task ApplyMigrations(CancellationToken cancellationToken = default) dbContext.Database.SetCommandTimeout(EFPersisterSettings.MigrationCommandTimeout); await RequireSchema(cancellationToken); + await dbContext.RequireNoAuditMigrationsHistory(cancellationToken); await dbContext.Database.MigrateAsync(cancellationToken); dbContext.Database.SetCommandTimeout(previousTimeout); diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs index 8f6a8f78dd..74e6074ce7 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs @@ -73,9 +73,7 @@ void ConfigureDbContext(IServiceCollection services) options.ReplaceService(); options.ReplaceService(); - // HasDefaultSchema moves every table, so the model is meant to differ from the - // snapshot the migrations were scaffolded against. A default installation keeps the - // check, where a difference really would mean a migration is missing. + // Schema stamping intentionally differs from the schema-less snapshot; PendingModelChangesTests checks migration drift. options.ConfigureWarnings(warnings => warnings.Ignore(RelationalEventId.PendingModelChangesWarning)); } diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs index 91e578f75f..62b982032f 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs @@ -27,4 +27,9 @@ public override bool IsDuplicateKeyException(DbUpdateException exception) } return false; } + + protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => + await Database + .SqlQueryRaw("SELECT CASE WHEN EXISTS (SELECT 1 FROM sys.tables WHERE name = {0} AND schema_id = SCHEMA_ID(COALESCE(CAST({1} AS sysname), SCHEMA_NAME()))) THEN 1 ELSE 0 END AS [Value]", table, Schema ?? (object)DBNull.Value) + .SingleAsync(cancellationToken) == 1; } diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest b/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest index 9c1f04c024..89e1a8dae4 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest @@ -9,6 +9,11 @@ "Name": "ServiceControl/Database/ConnectionString", "Mandatory": true }, + { + "Name": "ServiceControl/Database/Schema", + "Mandatory": false, + "DefaultValue": "psw_sc" + }, { "Name": "ServiceControl/Database/CommandTimeout", "Mandatory": false diff --git a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs index 5bf874154a..af4547b9c5 100644 --- a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs +++ b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersistenceConfigurationBase.cs @@ -52,13 +52,13 @@ public PersistenceSettings CreateSettings(SettingsRootNamespace settingsRootName protected abstract EFPersisterSettings CreateSettings(string connectionString, BodyStorageSettings bodyStorage); - static string? ReadSchema(SettingsRootNamespace settingsRootNamespace) + static string ReadSchema(SettingsRootNamespace settingsRootNamespace) { var schema = SettingsReader.Read(settingsRootNamespace, SchemaKey); if (string.IsNullOrWhiteSpace(schema)) { - return null; + return EFPersisterSettings.DefaultSchema; } try diff --git a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs index 1d57e8de46..799afe6b4d 100644 --- a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs +++ b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs @@ -5,6 +5,7 @@ public abstract class EFPersisterSettings : PersistenceSettings public static readonly TimeSpan MigrationCommandTimeout = TimeSpan.FromMinutes(40); public const int DefaultCommandTimeout = 30; + public const string DefaultSchema = "psw_sc"; public const int DefaultExternalIntegrationsDispatchingBatchSize = 100; public static readonly TimeSpan DefaultEventsRetentionPeriod = TimeSpan.FromDays(14); @@ -13,7 +14,7 @@ public abstract class EFPersisterSettings : PersistenceSettings public required string ConnectionString { get; set; } /// - /// The schema the persister owns, or null to use the provider default. Validated on assignment + /// The schema the persister owns, or null to use the provider default. Configuration falls back to . Validated on assignment /// because it reaches raw DDL and dialect SQL by interpolation. /// public string? Schema diff --git a/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs b/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs index d9a40b9b14..a29cc1bad1 100644 --- a/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs +++ b/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs @@ -7,6 +7,9 @@ namespace ServiceControl.Persistence.EFCore.DbContexts; public abstract class ServiceControlDbContext(DbContextOptions options) : DbContext(options) { + // Not EF Core's default, so it identifies an audit instance that wrongly shares this schema. + const string AuditMigrationsHistoryTableName = "__AuditMigrationsHistory"; + /// /// The configured schema, or null when the provider default is in use. Null keeps the model /// identical to the one the migrations were scaffolded against. @@ -79,4 +82,18 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) } public abstract bool IsDuplicateKeyException(DbUpdateException exception); + + /// + /// Whether the table exists in the configured schema, or in the provider default schema when none is configured. + /// + protected abstract Task TableExists(string table, CancellationToken cancellationToken = default); + + public async Task RequireNoAuditMigrationsHistory(CancellationToken cancellationToken = default) + { + if (await TableExists(AuditMigrationsHistoryTableName, cancellationToken)) + { + throw new InvalidOperationException( + $"The schema '{Schema ?? "(provider default)"}' already contains the audit migrations history table '{AuditMigrationsHistoryTableName}'. A primary instance and an audit instance cannot share a schema. Configure a different Database/Schema for this instance, create the schema, grant the configured user rights on it, and run setup again."); + } + } } diff --git a/src/ServiceControl.Persistence.EFCore/Implementation/EFEnvironmentDataProvider.cs b/src/ServiceControl.Persistence.EFCore/Implementation/EFEnvironmentDataProvider.cs index 0b3809c362..0df58796dc 100644 --- a/src/ServiceControl.Persistence.EFCore/Implementation/EFEnvironmentDataProvider.cs +++ b/src/ServiceControl.Persistence.EFCore/Implementation/EFEnvironmentDataProvider.cs @@ -43,7 +43,7 @@ public IEnumerable GetData() Value("Storage.FullTextSearch", () => settings.EnableFullTextSearchOnBodies ? "Enabled" : "Disabled"), Value("Storage.BodyStorage.Type", () => BodyStorageType(settings.BodyStorage)), Value("Limits.MaxBodySizeToStore", () => settings.BodyStorage.MaxBodySizeToStore.ToString(CultureInfo.InvariantCulture)), - Value("Storage.Schema", () => settings.Schema is null ? "Default" : "Custom"), + Value("Storage.Schema", () => settings.Schema is null or EFPersisterSettings.DefaultSchema ? "Default" : "Custom"), Value("Storage.CommandTimeoutSeconds", () => WhenConfigured(EFPersistenceConfigurationBase.CommandTimeoutKey, () => Number(settings.CommandTimeout))), Value("Storage.QueryTimeoutSeconds", () => WhenConfigured(QueryTimeLimit.SettingName, () => Seconds(settings.QueryTimeout))), Value("Storage.SubscriptionCacheSeconds", () => WhenConfigured(EFPersistenceConfigurationBase.SubscriptionCacheDurationKey, () => Seconds(settings.SubscriptionCacheDuration))), diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/SchemaOptionsExtension.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/SchemaOptionsExtension.cs index fa87c17920..4944b1de38 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/SchemaOptionsExtension.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/SchemaOptionsExtension.cs @@ -7,8 +7,7 @@ namespace ServiceControl.Persistence.EFCore.Infrastructure; /// Carries the configured schema into EF Core, where the model builder and the migrations SQL /// generator both need it. An options extension rather than a constructor argument because the /// migrations generator is resolved from the provider's own service provider and cannot see -/// application services. Present only when a schema is configured, so a default installation -/// builds exactly the model and the SQL it did before. +/// application services. Omitted for schema-less design-time contexts so migration snapshots remain schema-agnostic. /// public sealed class SchemaOptionsExtension(string schema) : IDbContextOptionsExtension { diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs index 26f51e2a5b..965b0b2e12 100644 --- a/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs @@ -16,13 +16,61 @@ class SchemaMustExistTests [Test] public async Task Migration_fails_when_the_configured_schema_does_not_exist() { - var settings = new PostgreSqlPersisterSettings + var settings = await CreateSettings($"sc_absent_{Guid.NewGuid():n}"); + + var exception = Assert.ThrowsAsync(() => ApplyMigrations(settings)); + Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("does not exist")); + } + + [Test] + public async Task Migration_fails_when_the_schema_holds_the_audit_migrations_history_table() + { + var settings = await CreateSettings($"sc_shared_{Guid.NewGuid():n}"); + await TestSchema.Create(settings.ConnectionString, settings.Schema); + + try + { + await TestSchema.CreateTable(settings.ConnectionString, settings.Schema, "__AuditMigrationsHistory"); + + var exception = Assert.ThrowsAsync(() => ApplyMigrations(settings)); + Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("__AuditMigrationsHistory")); + } + finally + { + await TestSchema.Drop(settings.ConnectionString, settings.Schema); + } + } + + [Test] + public async Task Migration_ignores_the_audit_migrations_history_table_in_another_schema() + { + var settings = await CreateSettings($"sc_primary_{Guid.NewGuid():n}"); + var auditSchema = $"sc_audit_{Guid.NewGuid():n}"; + await TestSchema.Create(settings.ConnectionString, settings.Schema); + await TestSchema.Create(settings.ConnectionString, auditSchema); + + try { - ConnectionString = await PostgreSqlSharedContainer.GetConnectionStringAsync(), - Schema = $"sc_absent_{Guid.NewGuid():n}", - BodyStorage = new FileSystemBodyStorageSettings { StoragePath = Path.GetTempPath() } - }; + await TestSchema.CreateTable(settings.ConnectionString, auditSchema, "__AuditMigrationsHistory"); + Assert.DoesNotThrowAsync(() => ApplyMigrations(settings)); + } + finally + { + await TestSchema.Drop(settings.ConnectionString, settings.Schema); + await TestSchema.Drop(settings.ConnectionString, auditSchema); + } + } + + static async Task CreateSettings(string schema) => new() + { + ConnectionString = await PostgreSqlSharedContainer.GetConnectionStringAsync(), + Schema = schema, + BodyStorage = new FileSystemBodyStorageSettings { StoragePath = Path.GetTempPath() } + }; + + static async Task ApplyMigrations(PostgreSqlPersisterSettings settings) + { var services = new ServiceCollection(); services.AddLogging(); new PostgreSqlPersistenceConfiguration().Create(settings).AddInstaller(services); @@ -31,7 +79,6 @@ public async Task Migration_fails_when_the_configured_schema_does_not_exist() using var scope = provider.CreateScope(); var migrator = scope.ServiceProvider.GetRequiredService(); - var exception = Assert.ThrowsAsync(() => migrator.ApplyMigrations()); - Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("does not exist")); + await migrator.ApplyMigrations(); } } diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/TestSchema.cs b/src/ServiceControl.Persistence.Tests.PostgreSql/TestSchema.cs index ced8cf43e0..23bd9cae35 100644 --- a/src/ServiceControl.Persistence.Tests.PostgreSql/TestSchema.cs +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/TestSchema.cs @@ -9,6 +9,9 @@ static class TestSchema public static Task Create(string connectionString, string schema, CancellationToken cancellationToken = default) => Execute(connectionString, $"CREATE SCHEMA IF NOT EXISTS \"{schema}\"", cancellationToken); + public static Task CreateTable(string connectionString, string schema, string table, CancellationToken cancellationToken = default) => + Execute(connectionString, $"CREATE TABLE \"{schema}\".\"{table}\" (id int)", cancellationToken); + public static Task Drop(string connectionString, string schema, CancellationToken cancellationToken = default) => Execute(connectionString, $"DROP SCHEMA IF EXISTS \"{schema}\" CASCADE", cancellationToken); diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs index 32a09c87e4..13b8a5a4f5 100644 --- a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs +++ b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs @@ -16,13 +16,61 @@ class SchemaMustExistTests [Test] public async Task Migration_fails_when_the_configured_schema_does_not_exist() { - var settings = new SqlServerPersisterSettings + var settings = await CreateSettings($"sc_absent_{Guid.NewGuid():n}"); + + var exception = Assert.ThrowsAsync(() => ApplyMigrations(settings)); + Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("does not exist")); + } + + [Test] + public async Task Migration_fails_when_the_schema_holds_the_audit_migrations_history_table() + { + var settings = await CreateSettings($"sc_shared_{Guid.NewGuid():n}"); + await TestSchema.Create(settings.ConnectionString, settings.Schema); + + try + { + await TestSchema.CreateTable(settings.ConnectionString, settings.Schema, "__AuditMigrationsHistory"); + + var exception = Assert.ThrowsAsync(() => ApplyMigrations(settings)); + Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("__AuditMigrationsHistory")); + } + finally + { + await TestSchema.Drop(settings.ConnectionString, settings.Schema); + } + } + + [Test] + public async Task Migration_ignores_the_audit_migrations_history_table_in_another_schema() + { + var settings = await CreateSettings($"sc_primary_{Guid.NewGuid():n}"); + var auditSchema = $"sc_audit_{Guid.NewGuid():n}"; + await TestSchema.Create(settings.ConnectionString, settings.Schema); + await TestSchema.Create(settings.ConnectionString, auditSchema); + + try { - ConnectionString = await SqlServerSharedContainer.GetConnectionStringAsync(), - Schema = $"sc_absent_{Guid.NewGuid():n}", - BodyStorage = new FileSystemBodyStorageSettings { StoragePath = Path.GetTempPath() } - }; + await TestSchema.CreateTable(settings.ConnectionString, auditSchema, "__AuditMigrationsHistory"); + Assert.DoesNotThrowAsync(() => ApplyMigrations(settings)); + } + finally + { + await TestSchema.Drop(settings.ConnectionString, settings.Schema); + await TestSchema.Drop(settings.ConnectionString, auditSchema); + } + } + + static async Task CreateSettings(string schema) => new() + { + ConnectionString = await SqlServerSharedContainer.GetConnectionStringAsync(), + Schema = schema, + BodyStorage = new FileSystemBodyStorageSettings { StoragePath = Path.GetTempPath() } + }; + + static async Task ApplyMigrations(SqlServerPersisterSettings settings) + { var services = new ServiceCollection(); services.AddLogging(); new SqlServerPersistenceConfiguration().Create(settings).AddInstaller(services); @@ -31,7 +79,6 @@ public async Task Migration_fails_when_the_configured_schema_does_not_exist() using var scope = provider.CreateScope(); var migrator = scope.ServiceProvider.GetRequiredService(); - var exception = Assert.ThrowsAsync(() => migrator.ApplyMigrations()); - Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("does not exist")); + await migrator.ApplyMigrations(); } } diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/TestSchema.cs b/src/ServiceControl.Persistence.Tests.SqlServer/TestSchema.cs index 4dcd38c039..b534746201 100644 --- a/src/ServiceControl.Persistence.Tests.SqlServer/TestSchema.cs +++ b/src/ServiceControl.Persistence.Tests.SqlServer/TestSchema.cs @@ -10,6 +10,9 @@ static class TestSchema public static Task Create(string connectionString, string schema, CancellationToken cancellationToken = default) => Execute(connectionString, CreateSchemaSql, schema, cancellationToken); + public static Task CreateTable(string connectionString, string schema, string table, CancellationToken cancellationToken = default) => + Execute(connectionString, $"DECLARE @create nvarchar(max) = N'CREATE TABLE ' + QUOTENAME(@schema) + N'.' + QUOTENAME(N'{table}') + N' (Id int)'; EXEC sp_executesql @create;", schema, cancellationToken); + // SQL Server has no DROP SCHEMA CASCADE, so the objects have to go first, and the full-text // index has to go before the table it is on. public static Task Drop(string connectionString, string schema, CancellationToken cancellationToken = default) => diff --git a/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs index 12bda43295..885b15a250 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs @@ -62,6 +62,14 @@ public async Task Should_report_configured_values_in_the_unit_their_keys_name() } } + [Test] + public async Task Should_report_the_default_schema_as_default() + { + var data = await GetData(new TestPersisterSettings { ConnectionString = "Host=localhost", BodyStorage = new FileSystemBodyStorageSettings { StoragePath = "/var/bodies" }, Schema = EFPersisterSettings.DefaultSchema }); + + Assert.That(data["Storage.Schema"], Is.EqualTo("Default")); + } + [Test] public async Task Should_report_a_custom_schema_without_naming_it() { diff --git a/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs new file mode 100644 index 0000000000..8ba4b16579 --- /dev/null +++ b/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs @@ -0,0 +1,72 @@ +namespace ServiceControl.Persistence.Tests; + +using System; +using NUnit.Framework; +using ServiceControl.Configuration; +using ServiceControl.Persistence.EFCore.Abstractions; + +[TestFixture] +[NonParallelizable] +class SchemaConfigurationTests +{ + const string SchemaVariable = "SERVICECONTROL_DATABASE_SCHEMA"; + + static readonly string[] Variables = + [ + "SERVICECONTROL_DATABASE_CONNECTIONSTRING", + "SERVICECONTROL_ERRORRETENTIONPERIOD", + "SERVICECONTROL_MESSAGEBODY_STORAGETYPE", + "SERVICECONTROL_MESSAGEBODY_FILESYSTEM_STORAGEPATH", + SchemaVariable + ]; + + [SetUp] + public void SetUp() + { + TearDown(); + Environment.SetEnvironmentVariable("SERVICECONTROL_DATABASE_CONNECTIONSTRING", "Server=nowhere"); + Environment.SetEnvironmentVariable("SERVICECONTROL_ERRORRETENTIONPERIOD", "10.00:00:00"); + Environment.SetEnvironmentVariable("SERVICECONTROL_MESSAGEBODY_STORAGETYPE", "FileSystem"); + Environment.SetEnvironmentVariable("SERVICECONTROL_MESSAGEBODY_FILESYSTEM_STORAGEPATH", "/var/bodies"); + } + + [TearDown] + public void TearDown() + { + foreach (var variable in Variables) + { + Environment.SetEnvironmentVariable(variable, null); + } + } + + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + public void Absent_or_blank_schema_falls_back_to_psw_sc(string configured) + { + Environment.SetEnvironmentVariable(SchemaVariable, configured); + + Assert.That(CreateSettings().Schema, Is.EqualTo("psw_sc")); + } + + [Test] + public void Configured_schema_is_used() + { + Environment.SetEnvironmentVariable(SchemaVariable, "legacy"); + + Assert.That(CreateSettings().Schema, Is.EqualTo("legacy")); + } + + static EFPersisterSettings CreateSettings() => + (EFPersisterSettings)new TestPersistenceConfiguration().CreateSettings(new SettingsRootNamespace("ServiceControl")); + + sealed class TestPersistenceConfiguration : EFPersistenceConfigurationBase + { + public override IPersistence Create(PersistenceSettings settings) => throw new NotSupportedException(); + + protected override EFPersisterSettings CreateSettings(string connectionString, BodyStorageSettings bodyStorage) => + new TestPersisterSettings { ConnectionString = connectionString, BodyStorage = bodyStorage }; + } + + sealed class TestPersisterSettings : EFPersisterSettings; +} From 3c1f19559768c8919f4134b6d7c4e898bcfad7bd Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Fri, 23 Oct 2026 09:51:21 +0800 Subject: [PATCH 2/6] Explicitly load config in tests --- .../PostgreSqlEnvironmentDataTests.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ServiceControl.Audit.Persistence.Tests.PostgreSql/PostgreSqlEnvironmentDataTests.cs b/src/ServiceControl.Audit.Persistence.Tests.PostgreSql/PostgreSqlEnvironmentDataTests.cs index e14534561d..eed0e5b76c 100644 --- a/src/ServiceControl.Audit.Persistence.Tests.PostgreSql/PostgreSqlEnvironmentDataTests.cs +++ b/src/ServiceControl.Audit.Persistence.Tests.PostgreSql/PostgreSqlEnvironmentDataTests.cs @@ -14,6 +14,7 @@ namespace ServiceControl.Audit.Persistence.Tests using ServiceControl.Audit.Persistence.EFCore.DbContexts; using ServiceControl.Audit.Persistence.EFCore.Entities; using ServiceControl.Audit.Persistence.EFCore.Infrastructure; + using ServiceControl.Audit.Persistence.EFCore.PostgreSql; class PostgreSqlEnvironmentDataTests : EFPersistenceTestFixture { @@ -63,14 +64,15 @@ public async Task Counts_messages_across_the_partitions_once_analysed() [Test] public async Task Measures_and_identifies_the_search_path_schema_when_none_is_configured() { - var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000); - settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = - new NpgsqlConnectionStringBuilder(configuration.ConnectionString) { SearchPath = configuration.Schema }.ConnectionString; - var services = new ServiceCollection(); services.AddLogging(); services.AddSingleton(configuration.TimeProvider); - configuration.CreateConfiguration().Create(settings).AddPersistence(services); + new PostgreSqlPersistence(new EFPersisterSettings + { + ConnectionString = new NpgsqlConnectionStringBuilder(configuration.ConnectionString) { SearchPath = configuration.Schema }.ConnectionString, + AuditRetentionPeriod = TimeSpan.FromDays(1), + MaxBodySizeToStore = 100000 + }).AddPersistence(services); await using var unconfigured = services.BuildServiceProvider(); var identity = await unconfigured.GetRequiredService().GetIdentity(TestTimeoutCancellationToken); From f8dd9fb8dafe95dd69e59232258372f7bafc4d79 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Fri, 23 Oct 2026 11:18:06 +0800 Subject: [PATCH 3/6] Simplify default schema names --- docs/sql-persister-schemas.md | 8 ++++--- docs/usage-report.md | 2 +- .../persistence.manifest | 2 +- .../persistence.manifest | 2 +- .../Abstractions/EFPersisterSettings.cs | 2 +- .../EFCore/SchemaConfigurationTests.cs | 15 +++++++------ .../persistence.manifest | 2 +- .../persistence.manifest | 2 +- .../Abstractions/EFPersisterSettings.cs | 2 +- .../EnvironmentDataConfigurationTests.cs | 21 +++++++------------ .../EFCore/SchemaConfigurationTests.cs | 15 +++++++------ 11 files changed, 38 insertions(+), 35 deletions(-) diff --git a/docs/sql-persister-schemas.md b/docs/sql-persister-schemas.md index c82f23963e..811f47b114 100644 --- a/docs/sql-persister-schemas.md +++ b/docs/sql-persister-schemas.md @@ -4,9 +4,11 @@ The SQL Server and PostgreSQL persisters keep all of an instance's tables in one | Instance | Default schema | |---|---| -| Primary (error) | `psw_sc` | -| Audit | `psw_audit` | +| Primary (error) | `sc` | +| Audit | `audit` | * ServiceControl does not create the schema. Create it, and grant the configured user rights on it, before running setup. * A primary and an audit instance cannot share a schema. Setup fails before it applies any migration when the other instance's migrations history table is in the target schema: the primary rejects `__AuditMigrationsHistory`, and the audit instance rejects `__EFMigrationsHistory`. -* The primary instance's usage report lists `Storage.Schema` as `Default` for `psw_sc` and `Custom` for other configured schemas. See [Usage report contents](usage-report.md). +* The primary instance's usage report lists `Storage.Schema` as `Default` for `sc` and `Custom` for other configured schemas. See [Usage report contents](usage-report.md). + +ServiceControl does not rename or move existing schemas or tables. An instance that already holds data must set `Database/Schema` to the schema that contains it, such as `dbo` on SQL Server, `public` on PostgreSQL, or a custom schema. Otherwise, the instance uses the default schema instead of the schema containing its existing data. diff --git a/docs/usage-report.md b/docs/usage-report.md index 60057f856b..ef4ebf65ee 100644 --- a/docs/usage-report.md +++ b/docs/usage-report.md @@ -88,7 +88,7 @@ Releases up to 6.21.2 report `SqlVersion` as the full text of `@@VERSION` on SQL | `Storage.FullTextSearch` | `Enabled`, `Disabled` | `EnableFullTextSearchOnBodies`, RavenDB only. Always `Enabled` on SQL Server and PostgreSQL, which do not read the setting | 6.20.0 | | `Storage.BodyStorage.Type` | `RavenAttachments`, `FileSystem`, `AzureBlob`, `S3` | the persister | 6.20.0 | | `Limits.MaxBodySizeToStore` | bytes | `MaxBodySizeToStore`, SQL Server and PostgreSQL only | 6.20.0 | -| `Storage.Schema` | `Default` when the schema is `psw_sc`, `Custom` for other configured schemas | `Database/Schema`, SQL Server and PostgreSQL only | Unreleased | +| `Storage.Schema` | `Default` when the schema is `sc`, `Custom` for other configured schemas | `Database/Schema`, SQL Server and PostgreSQL only | Unreleased | | `Storage.LogLevel` | `None`, `Information`, `Operations` | `RavenDBLogLevel`, RavenDB only | Unreleased | | `Storage.CommandTimeoutSeconds` | `Default` or number | `Database/CommandTimeout`, SQL Server and PostgreSQL only | Unreleased | | `Storage.QueryTimeoutSeconds` | `Default` or number | `QueryTimeoutInSeconds` | Unreleased | diff --git a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest index 1c0fa78545..db0733241e 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest +++ b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest @@ -12,7 +12,7 @@ { "Name": "ServiceControl.Audit/Database/Schema", "Mandatory": false, - "DefaultValue": "psw_audit" + "DefaultValue": "audit" }, { "Name": "ServiceControl.Audit/Database/CommandTimeout", diff --git a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest index cc70c28d0e..7aa197ebb9 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest +++ b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest @@ -12,7 +12,7 @@ { "Name": "ServiceControl.Audit/Database/Schema", "Mandatory": false, - "DefaultValue": "psw_audit" + "DefaultValue": "audit" }, { "Name": "ServiceControl.Audit/Database/CommandTimeout", diff --git a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs index 26b4136388..9be6017ee0 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs @@ -7,7 +7,7 @@ public class EFPersisterSettings public static readonly TimeSpan MigrationCommandTimeout = TimeSpan.FromMinutes(40); public const int DefaultCommandTimeout = 30; - public const string DefaultSchema = "psw_audit"; + public const string DefaultSchema = "audit"; public required string ConnectionString { get; init; } diff --git a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs index c9a36437f2..4a8addd06d 100644 --- a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs +++ b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs @@ -10,7 +10,7 @@ class SchemaConfigurationTests [TestCase(null)] [TestCase("")] [TestCase(" ")] - public void Absent_or_blank_schema_falls_back_to_psw_audit(string configured) + public void Absent_or_blank_schema_falls_back_to_audit(string configured) { var settings = CreateSettings(); if (configured != null) @@ -18,16 +18,19 @@ public void Absent_or_blank_schema_falls_back_to_psw_audit(string configured) settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = configured; } - Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo("psw_audit")); + Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo("audit")); } - [Test] - public void Configured_schema_is_used() + [TestCase("legacy")] + [TestCase("dbo")] + [TestCase("public")] + [TestCase("psw_audit")] + public void Configured_schema_is_used(string schema) { var settings = CreateSettings(); - settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = "legacy"; + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = schema; - Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo("legacy")); + Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo(schema)); } static PersistenceSettings CreateSettings() diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest b/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest index 6d161b9b16..6c8287ccce 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest @@ -12,7 +12,7 @@ { "Name": "ServiceControl/Database/Schema", "Mandatory": false, - "DefaultValue": "psw_sc" + "DefaultValue": "sc" }, { "Name": "ServiceControl/Database/CommandTimeout", diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest b/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest index 89e1a8dae4..b9cf575a8a 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest @@ -12,7 +12,7 @@ { "Name": "ServiceControl/Database/Schema", "Mandatory": false, - "DefaultValue": "psw_sc" + "DefaultValue": "sc" }, { "Name": "ServiceControl/Database/CommandTimeout", diff --git a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs index 799afe6b4d..941683d9f6 100644 --- a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs +++ b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs @@ -5,7 +5,7 @@ public abstract class EFPersisterSettings : PersistenceSettings public static readonly TimeSpan MigrationCommandTimeout = TimeSpan.FromMinutes(40); public const int DefaultCommandTimeout = 30; - public const string DefaultSchema = "psw_sc"; + public const string DefaultSchema = "sc"; public const int DefaultExternalIntegrationsDispatchingBatchSize = 100; public static readonly TimeSpan DefaultEventsRetentionPeriod = TimeSpan.FromDays(14); diff --git a/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs index 885b15a250..3662e3fd79 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs @@ -62,23 +62,18 @@ public async Task Should_report_configured_values_in_the_unit_their_keys_name() } } - [Test] - public async Task Should_report_the_default_schema_as_default() - { - var data = await GetData(new TestPersisterSettings { ConnectionString = "Host=localhost", BodyStorage = new FileSystemBodyStorageSettings { StoragePath = "/var/bodies" }, Schema = EFPersisterSettings.DefaultSchema }); - - Assert.That(data["Storage.Schema"], Is.EqualTo("Default")); - } - - [Test] - public async Task Should_report_a_custom_schema_without_naming_it() + [TestCase(null, "Default")] + [TestCase("sc", "Default")] + [TestCase("psw_sc", "Custom")] + [TestCase("contoso", "Custom")] + public async Task Should_report_the_schema_without_naming_it(string schema, string expected) { - var data = await GetData(new TestPersisterSettings { ConnectionString = "Host=localhost", BodyStorage = new FileSystemBodyStorageSettings { StoragePath = "/var/bodies" }, Schema = "contoso" }); + var data = await GetData(new TestPersisterSettings { ConnectionString = "Host=localhost", BodyStorage = new FileSystemBodyStorageSettings { StoragePath = "/var/bodies" }, Schema = schema }); using (Assert.EnterMultipleScope()) { - Assert.That(data["Storage.Schema"], Is.EqualTo("Custom")); - Assert.That(string.Join("|", data.Values), Does.Not.Contain("contoso")); + Assert.That(data["Storage.Schema"], Is.EqualTo(expected)); + Assert.That(string.Join("|", data.Values), Does.Not.Contain("psw_sc").And.Not.Contain("contoso")); } } diff --git a/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs index 8ba4b16579..6fd1cc5017 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs @@ -42,19 +42,22 @@ public void TearDown() [TestCase(null)] [TestCase("")] [TestCase(" ")] - public void Absent_or_blank_schema_falls_back_to_psw_sc(string configured) + public void Absent_or_blank_schema_falls_back_to_sc(string configured) { Environment.SetEnvironmentVariable(SchemaVariable, configured); - Assert.That(CreateSettings().Schema, Is.EqualTo("psw_sc")); + Assert.That(CreateSettings().Schema, Is.EqualTo("sc")); } - [Test] - public void Configured_schema_is_used() + [TestCase("legacy")] + [TestCase("dbo")] + [TestCase("public")] + [TestCase("psw_sc")] + public void Configured_schema_is_used(string schema) { - Environment.SetEnvironmentVariable(SchemaVariable, "legacy"); + Environment.SetEnvironmentVariable(SchemaVariable, schema); - Assert.That(CreateSettings().Schema, Is.EqualTo("legacy")); + Assert.That(CreateSettings().Schema, Is.EqualTo(schema)); } static EFPersisterSettings CreateSettings() => From fe0a4e3bb2652dcdf4798a45d12530c57b69787c Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Fri, 23 Oct 2026 12:59:25 +0800 Subject: [PATCH 4/6] Don't forbid separate schemas --- docs/README.md | 2 +- docs/sql-persister-schemas.md | 4 +- docs/usage-report.md | 2 +- .../PostgreSqlAuditDbContext.cs | 5 --- .../SqlServerAuditDbContext.cs | 5 --- .../DbContexts/AuditDbContext.cs | 17 ------- .../Implementation/DatabaseSetup.cs | 2 - .../EFCore/SchemaMustExistTests.cs | 45 ------------------- .../PostgreSqlDatabaseMigrator.cs | 1 - .../PostgreSqlServiceControlDbContext.cs | 5 --- .../SqlServerDatabaseMigrator.cs | 1 - .../SqlServerServiceControlDbContext.cs | 5 --- .../DbContexts/ServiceControlDbContext.cs | 17 ------- .../SchemaMustExistTests.cs | 40 ----------------- .../TestSchema.cs | 3 -- .../SchemaMustExistTests.cs | 40 ----------------- .../TestSchema.cs | 3 -- 17 files changed, 4 insertions(+), 193 deletions(-) diff --git a/docs/README.md b/docs/README.md index 690f42fa00..8ad624345a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -32,7 +32,7 @@ This section points to sources that explain why ServiceControl is designed the w - [Handling unavailable runtime dependencies](handling-unavailable-runtime-dependencies.md) — how instances react when a dependency is unavailable - [Telemetry](telemetry.md) — how to configure telemetry export and which metrics instances emit - [Throughput collection](throughput-collection.md) — how usage data is collected and why the throughput queue has a fixed name -- [SQL persister schemas](sql-persister-schemas.md) — which schema each SQL persister uses by default and why a primary and an audit instance cannot share one +- [SQL persister schemas](sql-persister-schemas.md) — which schema each SQL persister uses by default, and that sharing one between a primary and an audit instance is not recommended - [Usage report contents](usage-report.md): every key the usage report carries, every setting it leaves out, and why ## Decisions and rationale diff --git a/docs/sql-persister-schemas.md b/docs/sql-persister-schemas.md index 811f47b114..5f4a8d29f0 100644 --- a/docs/sql-persister-schemas.md +++ b/docs/sql-persister-schemas.md @@ -7,8 +7,8 @@ The SQL Server and PostgreSQL persisters keep all of an instance's tables in one | Primary (error) | `sc` | | Audit | `audit` | -* ServiceControl does not create the schema. Create it, and grant the configured user rights on it, before running setup. -* A primary and an audit instance cannot share a schema. Setup fails before it applies any migration when the other instance's migrations history table is in the target schema: the primary rejects `__AuditMigrationsHistory`, and the audit instance rejects `__EFMigrationsHistory`. +* ServiceControl will attempt to create the configured schema if it does not already exist. If using a connection without this permission it should be created in advance with the appropriate rights granted to the connection user. +* A primary and an audit instance can share one schema, but sharing is not recommended. * The primary instance's usage report lists `Storage.Schema` as `Default` for `sc` and `Custom` for other configured schemas. See [Usage report contents](usage-report.md). ServiceControl does not rename or move existing schemas or tables. An instance that already holds data must set `Database/Schema` to the schema that contains it, such as `dbo` on SQL Server, `public` on PostgreSQL, or a custom schema. Otherwise, the instance uses the default schema instead of the schema containing its existing data. diff --git a/docs/usage-report.md b/docs/usage-report.md index ef4ebf65ee..95c2996f0f 100644 --- a/docs/usage-report.md +++ b/docs/usage-report.md @@ -237,7 +237,7 @@ The ingestion and health keys follow the rules in [Error ingestion health](#erro | `Audit.SameMachine` | `True`, `False`, `Mixed`, `NotApplicable`, `Unknown` | the machine name hashes. `NotApplicable` when either instance runs in a container | Unreleased | | `Audit.DatabaseSharing` | `SameSchema`, `SameDatabase`, `SameServer`, `SeparateServer`, `NotApplicable`, `Unknown` | the storage identity hashes. `NotApplicable` when the storage engines differ. Several instances report the most shared class | Unreleased | -`Audit.Storage.SizeGB` covers the whole schema. A primary and an audit instance on SQL Server or PostgreSQL cannot share a schema, so `SameSchema` is not reported for that pairing; see [SQL persister schemas](sql-persister-schemas.md). +`Audit.Storage.SizeGB` covers the whole schema. A primary and an audit instance on SQL Server or PostgreSQL can share a schema, which is not recommended and is not the default; see [SQL persister schemas](sql-persister-schemas.md). #### From the configuration response diff --git a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs index 5f71dac00e..4f65c76342 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs @@ -45,9 +45,4 @@ public override async Task SchemaExists(string schema, CancellationToken c await Database .SqlQueryRaw("""SELECT CASE WHEN EXISTS (SELECT 1 FROM pg_namespace WHERE nspname = {0}) THEN 1 ELSE 0 END AS "Value" """, schema) .SingleAsync(cancellationToken) == 1; - - protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => - await Database - .SqlQueryRaw("""SELECT CASE WHEN EXISTS (SELECT 1 FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE c.relkind = 'r' AND c.relname = {0} AND n.nspname = COALESCE(CAST({1} AS text), current_schema())) THEN 1 ELSE 0 END AS "Value" """, table, Schema ?? (object)DBNull.Value) - .SingleAsync(cancellationToken) == 1; } diff --git a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs index 17f57dc763..829680fecc 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs @@ -32,9 +32,4 @@ public override async Task SchemaExists(string schema, CancellationToken c await Database .SqlQueryRaw("SELECT CASE WHEN SCHEMA_ID({0}) IS NULL THEN 0 ELSE 1 END AS [Value]", schema) .SingleAsync(cancellationToken) == 1; - - protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => - await Database - .SqlQueryRaw("SELECT CASE WHEN EXISTS (SELECT 1 FROM sys.tables WHERE name = {0} AND schema_id = SCHEMA_ID(COALESCE(CAST({1} AS sysname), SCHEMA_NAME()))) THEN 1 ELSE 0 END AS [Value]", table, Schema ?? (object)DBNull.Value) - .SingleAsync(cancellationToken) == 1; } diff --git a/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs b/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs index 6f63bc9d68..e733b87f58 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs @@ -9,9 +9,6 @@ public abstract class AuditDbContext(DbContextOptions options) : DbContext(optio { public const string MigrationsHistoryTableName = "__AuditMigrationsHistory"; - // EF Core's default history table name, which identifies a primary instance that wrongly shares this schema. - const string PrimaryMigrationsHistoryTableName = "__EFMigrationsHistory"; - public string? Schema { get; } = options.FindExtension()?.Schema; public DbSet AuditMessages { get; set; } @@ -46,18 +43,4 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) public abstract bool IsDuplicateKeyException(DbUpdateException exception); public abstract Task SchemaExists(string schema, CancellationToken cancellationToken = default); - - /// - /// Whether the table exists in the configured schema, or in the provider default schema when none is configured. - /// - protected abstract Task TableExists(string table, CancellationToken cancellationToken = default); - - public async Task RequireNoPrimaryMigrationsHistory(CancellationToken cancellationToken = default) - { - if (await TableExists(PrimaryMigrationsHistoryTableName, cancellationToken)) - { - throw new InvalidOperationException( - $"The schema '{Schema ?? "(provider default)"}' already contains the primary migrations history table '{PrimaryMigrationsHistoryTableName}'. A primary instance and an audit instance cannot share a schema. Configure a different Database/Schema for this instance, create the schema, grant the configured user rights on it, and run setup again."); - } - } } diff --git a/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs b/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs index bac90a2a0a..e36630c426 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs @@ -26,8 +26,6 @@ public async Task StartAsync(CancellationToken cancellationToken = default) $"The configured schema '{dbContext.Schema}' does not exist in the database. ServiceControl does not create schemas, the same way it does not create the database. Create the schema, grant the configured user rights on it, and run setup again."); } - await dbContext.RequireNoPrimaryMigrationsHistory(cancellationToken); - logger.LogInformation("Starting audit database migration"); await dbContext.Database.MigrateAsync(cancellationToken); diff --git a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs index 56f62e8872..ffae16230a 100644 --- a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs +++ b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs @@ -23,51 +23,6 @@ public async Task Setup_fails_when_the_configured_schema_does_not_exist() Assert.That(exception.Message, Does.Contain(schema).And.Contain("does not exist")); } - [Test] - public async Task Setup_fails_when_the_schema_holds_the_primary_migrations_history_table() - { - var testsConfiguration = new PersistenceTestsConfiguration(); - var connectionString = await testsConfiguration.GetConnectionString(); - var schema = $"sc_shared_{Guid.NewGuid():n}"; - await TestSchema.Create(connectionString, schema); - - try - { - await TestSchema.CreateTable(connectionString, schema, "__EFMigrationsHistory"); - - var exception = Assert.ThrowsAsync(() => Setup(testsConfiguration, connectionString, schema)); - - Assert.That(exception.Message, Does.Contain(schema).And.Contain("__EFMigrationsHistory")); - } - finally - { - await TestSchema.Drop(connectionString, schema); - } - } - - [Test] - public async Task Setup_ignores_the_primary_migrations_history_table_in_another_schema() - { - var testsConfiguration = new PersistenceTestsConfiguration(); - var connectionString = await testsConfiguration.GetConnectionString(); - var schema = $"sc_audit_{Guid.NewGuid():n}"; - var primarySchema = $"sc_primary_{Guid.NewGuid():n}"; - await TestSchema.Create(connectionString, schema); - await TestSchema.Create(connectionString, primarySchema); - - try - { - await TestSchema.CreateTable(connectionString, primarySchema, "__EFMigrationsHistory"); - - Assert.DoesNotThrowAsync(() => Setup(testsConfiguration, connectionString, schema)); - } - finally - { - await TestSchema.Drop(connectionString, schema); - await TestSchema.Drop(connectionString, primarySchema); - } - } - static async Task Setup(PersistenceTestsConfiguration testsConfiguration, string connectionString, string schema) { var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000); diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs index e8d98ddb7a..1a87d914ec 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs @@ -15,7 +15,6 @@ public async Task ApplyMigrations(CancellationToken cancellationToken = default) dbContext.Database.SetCommandTimeout(EFPersisterSettings.MigrationCommandTimeout); await RequireSchema(cancellationToken); - await dbContext.RequireNoAuditMigrationsHistory(cancellationToken); await dbContext.Database.MigrateAsync(cancellationToken); dbContext.Database.SetCommandTimeout(previousTimeout); diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContext.cs b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContext.cs index c2b9eb08cc..21ca3b0b4c 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContext.cs +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContext.cs @@ -49,9 +49,4 @@ public override bool IsDuplicateKeyException(DbUpdateException exception) } return false; } - - protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => - await Database - .SqlQueryRaw("""SELECT CASE WHEN EXISTS (SELECT 1 FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE c.relkind = 'r' AND c.relname = {0} AND n.nspname = COALESCE(CAST({1} AS text), current_schema())) THEN 1 ELSE 0 END AS "Value" """, table, Schema ?? (object)DBNull.Value) - .SingleAsync(cancellationToken) == 1; } diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs index 96a165f1bc..bdd297c290 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs @@ -15,7 +15,6 @@ public async Task ApplyMigrations(CancellationToken cancellationToken = default) dbContext.Database.SetCommandTimeout(EFPersisterSettings.MigrationCommandTimeout); await RequireSchema(cancellationToken); - await dbContext.RequireNoAuditMigrationsHistory(cancellationToken); await dbContext.Database.MigrateAsync(cancellationToken); dbContext.Database.SetCommandTimeout(previousTimeout); diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs index 62b982032f..91e578f75f 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContext.cs @@ -27,9 +27,4 @@ public override bool IsDuplicateKeyException(DbUpdateException exception) } return false; } - - protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => - await Database - .SqlQueryRaw("SELECT CASE WHEN EXISTS (SELECT 1 FROM sys.tables WHERE name = {0} AND schema_id = SCHEMA_ID(COALESCE(CAST({1} AS sysname), SCHEMA_NAME()))) THEN 1 ELSE 0 END AS [Value]", table, Schema ?? (object)DBNull.Value) - .SingleAsync(cancellationToken) == 1; } diff --git a/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs b/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs index a29cc1bad1..d9a40b9b14 100644 --- a/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs +++ b/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs @@ -7,9 +7,6 @@ namespace ServiceControl.Persistence.EFCore.DbContexts; public abstract class ServiceControlDbContext(DbContextOptions options) : DbContext(options) { - // Not EF Core's default, so it identifies an audit instance that wrongly shares this schema. - const string AuditMigrationsHistoryTableName = "__AuditMigrationsHistory"; - /// /// The configured schema, or null when the provider default is in use. Null keeps the model /// identical to the one the migrations were scaffolded against. @@ -82,18 +79,4 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) } public abstract bool IsDuplicateKeyException(DbUpdateException exception); - - /// - /// Whether the table exists in the configured schema, or in the provider default schema when none is configured. - /// - protected abstract Task TableExists(string table, CancellationToken cancellationToken = default); - - public async Task RequireNoAuditMigrationsHistory(CancellationToken cancellationToken = default) - { - if (await TableExists(AuditMigrationsHistoryTableName, cancellationToken)) - { - throw new InvalidOperationException( - $"The schema '{Schema ?? "(provider default)"}' already contains the audit migrations history table '{AuditMigrationsHistoryTableName}'. A primary instance and an audit instance cannot share a schema. Configure a different Database/Schema for this instance, create the schema, grant the configured user rights on it, and run setup again."); - } - } } diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs index 965b0b2e12..dddc4cf3a0 100644 --- a/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs @@ -22,46 +22,6 @@ public async Task Migration_fails_when_the_configured_schema_does_not_exist() Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("does not exist")); } - [Test] - public async Task Migration_fails_when_the_schema_holds_the_audit_migrations_history_table() - { - var settings = await CreateSettings($"sc_shared_{Guid.NewGuid():n}"); - await TestSchema.Create(settings.ConnectionString, settings.Schema); - - try - { - await TestSchema.CreateTable(settings.ConnectionString, settings.Schema, "__AuditMigrationsHistory"); - - var exception = Assert.ThrowsAsync(() => ApplyMigrations(settings)); - Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("__AuditMigrationsHistory")); - } - finally - { - await TestSchema.Drop(settings.ConnectionString, settings.Schema); - } - } - - [Test] - public async Task Migration_ignores_the_audit_migrations_history_table_in_another_schema() - { - var settings = await CreateSettings($"sc_primary_{Guid.NewGuid():n}"); - var auditSchema = $"sc_audit_{Guid.NewGuid():n}"; - await TestSchema.Create(settings.ConnectionString, settings.Schema); - await TestSchema.Create(settings.ConnectionString, auditSchema); - - try - { - await TestSchema.CreateTable(settings.ConnectionString, auditSchema, "__AuditMigrationsHistory"); - - Assert.DoesNotThrowAsync(() => ApplyMigrations(settings)); - } - finally - { - await TestSchema.Drop(settings.ConnectionString, settings.Schema); - await TestSchema.Drop(settings.ConnectionString, auditSchema); - } - } - static async Task CreateSettings(string schema) => new() { ConnectionString = await PostgreSqlSharedContainer.GetConnectionStringAsync(), diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/TestSchema.cs b/src/ServiceControl.Persistence.Tests.PostgreSql/TestSchema.cs index 23bd9cae35..ced8cf43e0 100644 --- a/src/ServiceControl.Persistence.Tests.PostgreSql/TestSchema.cs +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/TestSchema.cs @@ -9,9 +9,6 @@ static class TestSchema public static Task Create(string connectionString, string schema, CancellationToken cancellationToken = default) => Execute(connectionString, $"CREATE SCHEMA IF NOT EXISTS \"{schema}\"", cancellationToken); - public static Task CreateTable(string connectionString, string schema, string table, CancellationToken cancellationToken = default) => - Execute(connectionString, $"CREATE TABLE \"{schema}\".\"{table}\" (id int)", cancellationToken); - public static Task Drop(string connectionString, string schema, CancellationToken cancellationToken = default) => Execute(connectionString, $"DROP SCHEMA IF EXISTS \"{schema}\" CASCADE", cancellationToken); diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs index 13b8a5a4f5..e1ad5f3a09 100644 --- a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs +++ b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs @@ -22,46 +22,6 @@ public async Task Migration_fails_when_the_configured_schema_does_not_exist() Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("does not exist")); } - [Test] - public async Task Migration_fails_when_the_schema_holds_the_audit_migrations_history_table() - { - var settings = await CreateSettings($"sc_shared_{Guid.NewGuid():n}"); - await TestSchema.Create(settings.ConnectionString, settings.Schema); - - try - { - await TestSchema.CreateTable(settings.ConnectionString, settings.Schema, "__AuditMigrationsHistory"); - - var exception = Assert.ThrowsAsync(() => ApplyMigrations(settings)); - Assert.That(exception.Message, Does.Contain(settings.Schema).And.Contain("__AuditMigrationsHistory")); - } - finally - { - await TestSchema.Drop(settings.ConnectionString, settings.Schema); - } - } - - [Test] - public async Task Migration_ignores_the_audit_migrations_history_table_in_another_schema() - { - var settings = await CreateSettings($"sc_primary_{Guid.NewGuid():n}"); - var auditSchema = $"sc_audit_{Guid.NewGuid():n}"; - await TestSchema.Create(settings.ConnectionString, settings.Schema); - await TestSchema.Create(settings.ConnectionString, auditSchema); - - try - { - await TestSchema.CreateTable(settings.ConnectionString, auditSchema, "__AuditMigrationsHistory"); - - Assert.DoesNotThrowAsync(() => ApplyMigrations(settings)); - } - finally - { - await TestSchema.Drop(settings.ConnectionString, settings.Schema); - await TestSchema.Drop(settings.ConnectionString, auditSchema); - } - } - static async Task CreateSettings(string schema) => new() { ConnectionString = await SqlServerSharedContainer.GetConnectionStringAsync(), diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/TestSchema.cs b/src/ServiceControl.Persistence.Tests.SqlServer/TestSchema.cs index b534746201..4dcd38c039 100644 --- a/src/ServiceControl.Persistence.Tests.SqlServer/TestSchema.cs +++ b/src/ServiceControl.Persistence.Tests.SqlServer/TestSchema.cs @@ -10,9 +10,6 @@ static class TestSchema public static Task Create(string connectionString, string schema, CancellationToken cancellationToken = default) => Execute(connectionString, CreateSchemaSql, schema, cancellationToken); - public static Task CreateTable(string connectionString, string schema, string table, CancellationToken cancellationToken = default) => - Execute(connectionString, $"DECLARE @create nvarchar(max) = N'CREATE TABLE ' + QUOTENAME(@schema) + N'.' + QUOTENAME(N'{table}') + N' (Id int)'; EXEC sp_executesql @create;", schema, cancellationToken); - // SQL Server has no DROP SCHEMA CASCADE, so the objects have to go first, and the full-text // index has to go before the table it is on. public static Task Drop(string connectionString, string schema, CancellationToken cancellationToken = default) => From f3dd9807baf68f1dab0f6ed257eefa67783602b7 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Fri, 23 Oct 2026 13:19:33 +0800 Subject: [PATCH 5/6] Rename error migrations table --- docs/sql-persister-schemas.md | 2 ++ .../PostgreSqlPersistence.cs | 8 +------ ...ostgreSqlServiceControlDbContextFactory.cs | 3 ++- .../SqlServerPersistence.cs | 8 +------ ...SqlServerServiceControlDbContextFactory.cs | 3 ++- .../DbContexts/ServiceControlDbContext.cs | 2 ++ .../MigrationsHistoryTableTests.cs | 21 +++++++++++++++++++ .../MigrationsHistoryTableTests.cs | 21 +++++++++++++++++++ 8 files changed, 52 insertions(+), 16 deletions(-) create mode 100644 src/ServiceControl.Persistence.Tests.PostgreSql/MigrationsHistoryTableTests.cs create mode 100644 src/ServiceControl.Persistence.Tests.SqlServer/MigrationsHistoryTableTests.cs diff --git a/docs/sql-persister-schemas.md b/docs/sql-persister-schemas.md index 5f4a8d29f0..80571d2706 100644 --- a/docs/sql-persister-schemas.md +++ b/docs/sql-persister-schemas.md @@ -12,3 +12,5 @@ The SQL Server and PostgreSQL persisters keep all of an instance's tables in one * The primary instance's usage report lists `Storage.Schema` as `Default` for `sc` and `Custom` for other configured schemas. See [Usage report contents](usage-report.md). ServiceControl does not rename or move existing schemas or tables. An instance that already holds data must set `Database/Schema` to the schema that contains it, such as `dbo` on SQL Server, `public` on PostgreSQL, or a custom schema. Otherwise, the instance uses the default schema instead of the schema containing its existing data. + +The primary instance records applied migrations in the `__ErrorMigrationsHistory` table of its schema, as the audit instance does with `__AuditMigrationsHistory`. An existing primary database created with the default `__EFMigrationsHistory` table must have that table renamed to `__ErrorMigrationsHistory` before the instance starts. Otherwise, setup runs the migrations again and fails on the existing tables. diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs index 0abc56670e..1e4269791f 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs @@ -50,13 +50,7 @@ void ConfigureDbContext(IServiceCollection services) { npgsqlOptions.CommandTimeout(settings.CommandTimeout); - if (settings.Schema is not null) - { - // Its own history table per schema, so schemas sharing a database migrate - // independently. Without this EF leaves the history table unqualified and every - // schema reads the same one. - npgsqlOptions.MigrationsHistoryTable(HistoryRepository.DefaultTableName, settings.Schema); - } + npgsqlOptions.MigrationsHistoryTable(ServiceControlDbContext.MigrationsHistoryTableName, settings.Schema); if (settings.EnableRetryOnFailure) { diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContextFactory.cs b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContextFactory.cs index 5a17acb93f..a7dc16016a 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContextFactory.cs +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlServiceControlDbContextFactory.cs @@ -2,6 +2,7 @@ namespace ServiceControl.Persistence.EFCore.PostgreSql; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Design; +using ServiceControl.Persistence.EFCore.DbContexts; /// /// Design-time factory used by the dotnet-ef tooling (e.g. dotnet ef migrations add). @@ -14,7 +15,7 @@ public PostgreSqlServiceControlDbContext CreateDbContext(string[] args) ?? "Host=localhost;Port=5432;Database=servicecontrol;Username=postgres;Password=postgres"; var optionsBuilder = new DbContextOptionsBuilder(); - optionsBuilder.UseNpgsql(connectionString); + optionsBuilder.UseNpgsql(connectionString, npgsqlOptions => npgsqlOptions.MigrationsHistoryTable(ServiceControlDbContext.MigrationsHistoryTableName)); return new PostgreSqlServiceControlDbContext(optionsBuilder.Options); } diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs index 74e6074ce7..72c9a59c94 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs @@ -50,13 +50,7 @@ void ConfigureDbContext(IServiceCollection services) { sqlOptions.CommandTimeout(settings.CommandTimeout); - if (settings.Schema is not null) - { - // Its own history table per schema, so schemas sharing a database migrate - // independently. Without this EF leaves the history table unqualified and every - // schema reads the same one. - sqlOptions.MigrationsHistoryTable(HistoryRepository.DefaultTableName, settings.Schema); - } + sqlOptions.MigrationsHistoryTable(ServiceControlDbContext.MigrationsHistoryTableName, settings.Schema); if (settings.EnableRetryOnFailure) { diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContextFactory.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContextFactory.cs index 2be9b44199..a3bfd8056b 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContextFactory.cs +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerServiceControlDbContextFactory.cs @@ -2,6 +2,7 @@ namespace ServiceControl.Persistence.EFCore.SqlServer; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Design; +using ServiceControl.Persistence.EFCore.DbContexts; /// /// Design-time factory used by the dotnet-ef tooling (e.g. dotnet ef migrations add). @@ -14,7 +15,7 @@ public SqlServerServiceControlDbContext CreateDbContext(string[] args) ?? "Server=localhost;Database=ServiceControl;Trusted_Connection=True;TrustServerCertificate=True"; var optionsBuilder = new DbContextOptionsBuilder(); - optionsBuilder.UseSqlServer(connectionString); + optionsBuilder.UseSqlServer(connectionString, sqlOptions => sqlOptions.MigrationsHistoryTable(ServiceControlDbContext.MigrationsHistoryTableName)); return new SqlServerServiceControlDbContext(optionsBuilder.Options); } diff --git a/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs b/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs index d9a40b9b14..f75cb06f76 100644 --- a/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs +++ b/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs @@ -7,6 +7,8 @@ namespace ServiceControl.Persistence.EFCore.DbContexts; public abstract class ServiceControlDbContext(DbContextOptions options) : DbContext(options) { + public const string MigrationsHistoryTableName = "__ErrorMigrationsHistory"; + /// /// The configured schema, or null when the provider default is in use. Null keeps the model /// identical to the one the migrations were scaffolded against. diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/MigrationsHistoryTableTests.cs b/src/ServiceControl.Persistence.Tests.PostgreSql/MigrationsHistoryTableTests.cs new file mode 100644 index 0000000000..b8105d443c --- /dev/null +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/MigrationsHistoryTableTests.cs @@ -0,0 +1,21 @@ +// ReSharper disable once CheckNamespace +namespace ServiceControl.Persistence.Tests; + +using EFCore.PostgreSql; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using NUnit.Framework; + +[TestFixture] +class MigrationsHistoryTableTests +{ + [Test] + public void The_history_table_is_named_for_the_error_instance() + { + using var dbContext = new PostgreSqlServiceControlDbContextFactory().CreateDbContext([]); + + var createScript = dbContext.GetService().GetCreateScript(); + + Assert.That(createScript, Does.Contain("__ErrorMigrationsHistory")); + } +} diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/MigrationsHistoryTableTests.cs b/src/ServiceControl.Persistence.Tests.SqlServer/MigrationsHistoryTableTests.cs new file mode 100644 index 0000000000..a26850b0f1 --- /dev/null +++ b/src/ServiceControl.Persistence.Tests.SqlServer/MigrationsHistoryTableTests.cs @@ -0,0 +1,21 @@ +// ReSharper disable once CheckNamespace +namespace ServiceControl.Persistence.Tests; + +using EFCore.SqlServer; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using NUnit.Framework; + +[TestFixture] +class MigrationsHistoryTableTests +{ + [Test] + public void The_history_table_is_named_for_the_error_instance() + { + using var dbContext = new SqlServerServiceControlDbContextFactory().CreateDbContext([]); + + var createScript = dbContext.GetService().GetCreateScript(); + + Assert.That(createScript, Does.Contain("__ErrorMigrationsHistory")); + } +} From d635c17216c29b7a5cb646babc0526ca7516a1db Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Fri, 23 Oct 2026 13:30:53 +0800 Subject: [PATCH 6/6] Change the defaults again --- docs/sql-persister-schemas.md | 8 ++++---- docs/usage-report.md | 2 +- .../persistence.manifest | 2 +- .../persistence.manifest | 2 +- .../Abstractions/EFPersisterSettings.cs | 2 +- .../EFCore/SchemaConfigurationTests.cs | 4 ++-- .../persistence.manifest | 2 +- .../persistence.manifest | 2 +- .../Abstractions/EFPersisterSettings.cs | 2 +- .../EFCore/EnvironmentDataConfigurationTests.cs | 3 ++- .../EFCore/SchemaConfigurationTests.cs | 4 ++-- 11 files changed, 17 insertions(+), 16 deletions(-) diff --git a/docs/sql-persister-schemas.md b/docs/sql-persister-schemas.md index 80571d2706..872a8dc260 100644 --- a/docs/sql-persister-schemas.md +++ b/docs/sql-persister-schemas.md @@ -4,13 +4,13 @@ The SQL Server and PostgreSQL persisters keep all of an instance's tables in one | Instance | Default schema | |---|---| -| Primary (error) | `sc` | -| Audit | `audit` | +| Primary (error) | `sc_primary` | +| Audit | `sc_audit` | * ServiceControl will attempt to create the configured schema if it does not already exist. If using a connection without this permission it should be created in advance with the appropriate rights granted to the connection user. * A primary and an audit instance can share one schema, but sharing is not recommended. -* The primary instance's usage report lists `Storage.Schema` as `Default` for `sc` and `Custom` for other configured schemas. See [Usage report contents](usage-report.md). +* The primary instance's usage report lists `Storage.Schema` as `Default` for `sc_primary` and `Custom` for other configured schemas. See [Usage report contents](usage-report.md). ServiceControl does not rename or move existing schemas or tables. An instance that already holds data must set `Database/Schema` to the schema that contains it, such as `dbo` on SQL Server, `public` on PostgreSQL, or a custom schema. Otherwise, the instance uses the default schema instead of the schema containing its existing data. -The primary instance records applied migrations in the `__ErrorMigrationsHistory` table of its schema, as the audit instance does with `__AuditMigrationsHistory`. An existing primary database created with the default `__EFMigrationsHistory` table must have that table renamed to `__ErrorMigrationsHistory` before the instance starts. Otherwise, setup runs the migrations again and fails on the existing tables. +The primary instance records applied migrations in the `__ErrorMigrationsHistory` table of its schema, as the audit instance does with `__AuditMigrationsHistory`. diff --git a/docs/usage-report.md b/docs/usage-report.md index 95c2996f0f..1c8ded17b5 100644 --- a/docs/usage-report.md +++ b/docs/usage-report.md @@ -88,7 +88,7 @@ Releases up to 6.21.2 report `SqlVersion` as the full text of `@@VERSION` on SQL | `Storage.FullTextSearch` | `Enabled`, `Disabled` | `EnableFullTextSearchOnBodies`, RavenDB only. Always `Enabled` on SQL Server and PostgreSQL, which do not read the setting | 6.20.0 | | `Storage.BodyStorage.Type` | `RavenAttachments`, `FileSystem`, `AzureBlob`, `S3` | the persister | 6.20.0 | | `Limits.MaxBodySizeToStore` | bytes | `MaxBodySizeToStore`, SQL Server and PostgreSQL only | 6.20.0 | -| `Storage.Schema` | `Default` when the schema is `sc`, `Custom` for other configured schemas | `Database/Schema`, SQL Server and PostgreSQL only | Unreleased | +| `Storage.Schema` | `Default` when the schema is `sc_primary`, `Custom` for other configured schemas | `Database/Schema`, SQL Server and PostgreSQL only | Unreleased | | `Storage.LogLevel` | `None`, `Information`, `Operations` | `RavenDBLogLevel`, RavenDB only | Unreleased | | `Storage.CommandTimeoutSeconds` | `Default` or number | `Database/CommandTimeout`, SQL Server and PostgreSQL only | Unreleased | | `Storage.QueryTimeoutSeconds` | `Default` or number | `QueryTimeoutInSeconds` | Unreleased | diff --git a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest index db0733241e..2da848c499 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest +++ b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/persistence.manifest @@ -12,7 +12,7 @@ { "Name": "ServiceControl.Audit/Database/Schema", "Mandatory": false, - "DefaultValue": "audit" + "DefaultValue": "sc_audit" }, { "Name": "ServiceControl.Audit/Database/CommandTimeout", diff --git a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest index 7aa197ebb9..6a36218884 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest +++ b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/persistence.manifest @@ -12,7 +12,7 @@ { "Name": "ServiceControl.Audit/Database/Schema", "Mandatory": false, - "DefaultValue": "audit" + "DefaultValue": "sc_audit" }, { "Name": "ServiceControl.Audit/Database/CommandTimeout", diff --git a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs index 9be6017ee0..cee4ba0869 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/Abstractions/EFPersisterSettings.cs @@ -7,7 +7,7 @@ public class EFPersisterSettings public static readonly TimeSpan MigrationCommandTimeout = TimeSpan.FromMinutes(40); public const int DefaultCommandTimeout = 30; - public const string DefaultSchema = "audit"; + public const string DefaultSchema = "sc_audit"; public required string ConnectionString { get; init; } diff --git a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs index 4a8addd06d..5c94838aa7 100644 --- a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs +++ b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs @@ -10,7 +10,7 @@ class SchemaConfigurationTests [TestCase(null)] [TestCase("")] [TestCase(" ")] - public void Absent_or_blank_schema_falls_back_to_audit(string configured) + public void Absent_or_blank_schema_falls_back_to_sc_audit(string configured) { var settings = CreateSettings(); if (configured != null) @@ -18,7 +18,7 @@ public void Absent_or_blank_schema_falls_back_to_audit(string configured) settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = configured; } - Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo("audit")); + Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo("sc_audit")); } [TestCase("legacy")] diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest b/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest index 6c8287ccce..3cba5ba45d 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest @@ -12,7 +12,7 @@ { "Name": "ServiceControl/Database/Schema", "Mandatory": false, - "DefaultValue": "sc" + "DefaultValue": "sc_primary" }, { "Name": "ServiceControl/Database/CommandTimeout", diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest b/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest index b9cf575a8a..0c6bd6789d 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest @@ -12,7 +12,7 @@ { "Name": "ServiceControl/Database/Schema", "Mandatory": false, - "DefaultValue": "sc" + "DefaultValue": "sc_primary" }, { "Name": "ServiceControl/Database/CommandTimeout", diff --git a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs index 941683d9f6..76f60098af 100644 --- a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs +++ b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs @@ -5,7 +5,7 @@ public abstract class EFPersisterSettings : PersistenceSettings public static readonly TimeSpan MigrationCommandTimeout = TimeSpan.FromMinutes(40); public const int DefaultCommandTimeout = 30; - public const string DefaultSchema = "sc"; + public const string DefaultSchema = "sc_primary"; public const int DefaultExternalIntegrationsDispatchingBatchSize = 100; public static readonly TimeSpan DefaultEventsRetentionPeriod = TimeSpan.FromDays(14); diff --git a/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs index 3662e3fd79..9d2dc47db7 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs @@ -63,7 +63,8 @@ public async Task Should_report_configured_values_in_the_unit_their_keys_name() } [TestCase(null, "Default")] - [TestCase("sc", "Default")] + [TestCase("sc_primary", "Default")] + [TestCase("sc", "Custom")] [TestCase("psw_sc", "Custom")] [TestCase("contoso", "Custom")] public async Task Should_report_the_schema_without_naming_it(string schema, string expected) diff --git a/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs index 6fd1cc5017..4f29519d29 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs @@ -42,11 +42,11 @@ public void TearDown() [TestCase(null)] [TestCase("")] [TestCase(" ")] - public void Absent_or_blank_schema_falls_back_to_sc(string configured) + public void Absent_or_blank_schema_falls_back_to_sc_primary(string configured) { Environment.SetEnvironmentVariable(SchemaVariable, configured); - Assert.That(CreateSettings().Schema, Is.EqualTo("sc")); + Assert.That(CreateSettings().Schema, Is.EqualTo("sc_primary")); } [TestCase("legacy")]