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..33ea3d2eeb --- /dev/null +++ b/docs/sql-persister-schemas.md @@ -0,0 +1,16 @@ +# 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) | None: the connection or provider default, such as `dbo` on SQL Server and `public` on PostgreSQL | +| Audit | `sc_audit` | + +* During setup, ServiceControl attempts to create a missing schema, whether selected by default or configured through `Database/Schema`. If the configured database user lacks permission to create schemas, setup fails with a database permission error. In that case, create the schema and grant the user the required rights before running setup. +* A primary and an audit instance cannot share a schema within the same database. Setup checks the target schema in the connected database before applying migrations: the primary rejects `__AuditMigrationsHistory`, and the audit instance rejects `__EFMigrationsHistory`. +* The primary instance's usage report lists `Storage.Schema` as `Default` when no schema is configured and `Custom` for any configured schema, including `sc_primary`. See [Usage report contents](usage-report.md). + +ServiceControl does not rename or move existing schemas or tables. An instance that already holds data in a named schema, such as `sc_primary` or a custom schema, must set `Database/Schema` to that 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`. diff --git a/docs/usage-report.md b/docs/usage-report.md index 3b56231f9a..7baad9478c 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 no schema is configured, `Custom` for any configured schema | `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 within the same database, so `SameSchema` is not reported for that pairing. Using the same schema name in separate databases does not mean the instances share a schema; 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..1a2619d809 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore.PostgreSql/PostgreSqlAuditDbContext.cs @@ -41,8 +41,8 @@ public override bool IsDuplicateKeyException(DbUpdateException exception) return false; } - public override async Task SchemaExists(string schema, CancellationToken cancellationToken = default) => + protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => await Database - .SqlQueryRaw("""SELECT CASE WHEN EXISTS (SELECT 1 FROM pg_namespace WHERE nspname = {0}) THEN 1 ELSE 0 END AS "Value" """, schema) + .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..2da848c499 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": "sc_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..34c03d54db 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore.SqlServer/SqlServerAuditDbContext.cs @@ -28,8 +28,8 @@ public override bool IsDuplicateKeyException(DbUpdateException exception) return false; } - public override async Task SchemaExists(string schema, CancellationToken cancellationToken = default) => + protected override async Task TableExists(string table, CancellationToken cancellationToken = default) => await Database - .SqlQueryRaw("SELECT CASE WHEN SCHEMA_ID({0}) IS NULL THEN 0 ELSE 1 END AS [Value]", schema) + .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..6a36218884 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": "sc_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..cee4ba0869 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 = "sc_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..796f5285ec 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; } @@ -43,5 +45,17 @@ 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 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..41c0966eb7 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/Implementation/DatabaseSetup.cs @@ -20,11 +20,7 @@ public async Task StartAsync(CancellationToken cancellationToken = default) var dbContext = scope.ServiceProvider.GetRequiredService(); dbContext.Database.SetCommandTimeout(EFPersisterSettings.MigrationCommandTimeout); - if (dbContext.Schema is not null && !await dbContext.SchemaExists(dbContext.Schema, cancellationToken)) - { - throw new InvalidOperationException( - $"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.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); 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..5c94838aa7 --- /dev/null +++ b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaConfigurationTests.cs @@ -0,0 +1,43 @@ +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_sc_audit(string configured) + { + var settings = CreateSettings(); + if (configured != null) + { + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = configured; + } + + Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo("sc_audit")); + } + + [TestCase("legacy")] + [TestCase("dbo")] + [TestCase("public")] + [TestCase("psw_audit")] + public void Configured_schema_is_used(string schema) + { + var settings = CreateSettings(); + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = schema; + + Assert.That(EFPersistenceConfigurationBase.CreateSettings(settings).Schema, Is.EqualTo(schema)); + } + + 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 deleted file mode 100644 index 6ef1503937..0000000000 --- a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaMustExistTests.cs +++ /dev/null @@ -1,32 +0,0 @@ -namespace ServiceControl.Audit.Persistence.Tests -{ - using System; - using System.Threading.Tasks; - using Microsoft.Extensions.DependencyInjection; - using Microsoft.Extensions.Hosting; - using NUnit.Framework; - using ServiceControl.Audit.Persistence.EFCore.Abstractions; - - [TestFixture] - class SchemaMustExistTests - { - [Test] - public async Task Setup_fails_when_the_configured_schema_does_not_exist() - { - var testsConfiguration = new PersistenceTestsConfiguration(); - var schema = $"sc_absent_{Guid.NewGuid():n}"; - - var settings = new PersistenceSettings(TimeSpan.FromDays(1), true, 100000); - settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.ConnectionStringKey] = await testsConfiguration.GetConnectionString(); - 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")); - } - } -} diff --git a/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaSetupTests.cs b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaSetupTests.cs new file mode 100644 index 0000000000..d758095561 --- /dev/null +++ b/src/ServiceControl.Audit.Persistence.Tests/EFCore/SchemaSetupTests.cs @@ -0,0 +1,90 @@ +namespace ServiceControl.Audit.Persistence.Tests +{ + using System; + using System.Threading.Tasks; + using Microsoft.Extensions.DependencyInjection; + using Microsoft.Extensions.Hosting; + using NUnit.Framework; + using ServiceControl.Audit.Persistence.EFCore.Abstractions; + using ServiceControl.Persistence.Tests; + + [TestFixture] + class SchemaSetupTests + { + [Test] + public async Task Setup_creates_the_configured_schema_when_it_does_not_exist() + { + var testsConfiguration = new PersistenceTestsConfiguration(); + var connectionString = await testsConfiguration.GetConnectionString(); + var schema = $"sc_absent_{Guid.NewGuid():n}"; + + try + { + Assert.DoesNotThrowAsync(() => Setup(testsConfiguration, connectionString, schema)); + } + finally + { + await TestSchema.Drop(connectionString, schema); + } + } + + [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] = connectionString; + settings.PersisterSpecificSettings[EFPersistenceConfigurationBase.SchemaKey] = schema; + + var hostBuilder = Host.CreateApplicationBuilder(); + testsConfiguration.CreateConfiguration().Create(settings).AddInstaller(hostBuilder.Services); + using var host = hostBuilder.Build(); + + 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..3852dd8f65 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlDatabaseMigrator.cs @@ -14,31 +14,11 @@ public async Task ApplyMigrations(CancellationToken cancellationToken = default) var previousTimeout = dbContext.Database.GetCommandTimeout(); dbContext.Database.SetCommandTimeout(EFPersisterSettings.MigrationCommandTimeout); - await RequireSchema(cancellationToken); + await dbContext.RequireNoAuditMigrationsHistory(cancellationToken); await dbContext.Database.MigrateAsync(cancellationToken); dbContext.Database.SetCommandTimeout(previousTimeout); logger.LogInformation("PostgreSQL database migration completed"); } - - // EF Core would create the schema on its way to creating the migrations history table, which - // would turn a misspelled Database/Schema into a silently empty instance rather than an error. - async Task RequireSchema(CancellationToken cancellationToken) - { - if (dbContext.Schema is null) - { - return; - } - - var exists = await dbContext.Database - .SqlQueryRaw("""SELECT CASE WHEN EXISTS (SELECT 1 FROM pg_namespace WHERE nspname = {0}) THEN 1 ELSE 0 END AS "Value" """, dbContext.Schema) - .SingleAsync(cancellationToken); - - if (exists == 0) - { - throw new InvalidOperationException( - $"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."); - } - } } diff --git a/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs b/src/ServiceControl.Persistence.EFCore.PostgreSql/PostgreSqlPersistence.cs index 423f1d8ba8..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) { @@ -73,9 +67,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/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.PostgreSql/persistence.manifest b/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest index 433c31a5ae..f021224da5 100644 --- a/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest +++ b/src/ServiceControl.Persistence.EFCore.PostgreSql/persistence.manifest @@ -9,6 +9,10 @@ "Name": "ServiceControl/Database/ConnectionString", "Mandatory": true }, + { + "Name": "ServiceControl/Database/Schema", + "Mandatory": false + }, { "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..a3a34cabdd 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerDatabaseMigrator.cs @@ -14,31 +14,11 @@ public async Task ApplyMigrations(CancellationToken cancellationToken = default) var previousTimeout = dbContext.Database.GetCommandTimeout(); dbContext.Database.SetCommandTimeout(EFPersisterSettings.MigrationCommandTimeout); - await RequireSchema(cancellationToken); + await dbContext.RequireNoAuditMigrationsHistory(cancellationToken); await dbContext.Database.MigrateAsync(cancellationToken); dbContext.Database.SetCommandTimeout(previousTimeout); logger.LogInformation("SQL Server database migration completed"); } - - // EF Core would create the schema on its way to creating the migrations history table, which - // would turn a misspelled Database/Schema into a silently empty instance rather than an error. - async Task RequireSchema(CancellationToken cancellationToken) - { - if (dbContext.Schema is null) - { - return; - } - - var exists = await dbContext.Database - .SqlQueryRaw("SELECT CASE WHEN SCHEMA_ID({0}) IS NULL THEN 0 ELSE 1 END AS [Value]", dbContext.Schema) - .SingleAsync(cancellationToken); - - if (exists == 0) - { - throw new InvalidOperationException( - $"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."); - } - } } diff --git a/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs b/src/ServiceControl.Persistence.EFCore.SqlServer/SqlServerPersistence.cs index 8f6a8f78dd..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) { @@ -73,9 +67,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/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.SqlServer/persistence.manifest b/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest index 9c1f04c024..faa1f24c3e 100644 --- a/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest +++ b/src/ServiceControl.Persistence.EFCore.SqlServer/persistence.manifest @@ -9,6 +9,10 @@ "Name": "ServiceControl/Database/ConnectionString", "Mandatory": true }, + { + "Name": "ServiceControl/Database/Schema", + "Mandatory": false + }, { "Name": "ServiceControl/Database/CommandTimeout", "Mandatory": false diff --git a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs index 1d57e8de46..0d58e3e76d 100644 --- a/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs +++ b/src/ServiceControl.Persistence.EFCore/Abstractions/EFPersisterSettings.cs @@ -13,7 +13,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 connection or provider default. 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..9ff490aa65 100644 --- a/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs +++ b/src/ServiceControl.Persistence.EFCore/DbContexts/ServiceControlDbContext.cs @@ -7,6 +7,10 @@ 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"; + 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. @@ -79,4 +83,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 and run setup again."); + } + } } 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/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.PostgreSql/SchemaMustExistTests.cs b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs deleted file mode 100644 index 26f51e2a5b..0000000000 --- a/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs +++ /dev/null @@ -1,37 +0,0 @@ -// ReSharper disable once CheckNamespace -namespace ServiceControl.Persistence.Tests; - -using System; -using System.IO; -using System.Threading.Tasks; -using EFCore.PostgreSql; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using NUnit.Framework; -using ServiceControl.Persistence.EFCore.Abstractions; - -[TestFixture] -class SchemaMustExistTests -{ - [Test] - public async Task Migration_fails_when_the_configured_schema_does_not_exist() - { - var settings = new PostgreSqlPersisterSettings - { - ConnectionString = await PostgreSqlSharedContainer.GetConnectionStringAsync(), - Schema = $"sc_absent_{Guid.NewGuid():n}", - BodyStorage = new FileSystemBodyStorageSettings { StoragePath = Path.GetTempPath() } - }; - - var services = new ServiceCollection(); - services.AddLogging(); - new PostgreSqlPersistenceConfiguration().Create(settings).AddInstaller(services); - - await using var provider = services.BuildServiceProvider(); - 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")); - } -} diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaSetupTests.cs b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaSetupTests.cs new file mode 100644 index 0000000000..ddf899f5c8 --- /dev/null +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaSetupTests.cs @@ -0,0 +1,90 @@ +// ReSharper disable once CheckNamespace +namespace ServiceControl.Persistence.Tests; + +using System; +using System.IO; +using System.Threading.Tasks; +using EFCore.PostgreSql; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using NUnit.Framework; +using ServiceControl.Persistence.EFCore.Abstractions; + +[TestFixture] +class SchemaSetupTests +{ + [Test] + public async Task Migration_creates_the_configured_schema_when_it_does_not_exist() + { + var settings = await CreateSettings($"sc_absent_{Guid.NewGuid():n}"); + + try + { + Assert.DoesNotThrowAsync(() => ApplyMigrations(settings)); + } + finally + { + await TestSchema.Drop(settings.ConnectionString, settings.Schema); + } + } + + [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(), + 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); + + await using var provider = services.BuildServiceProvider(); + using var scope = provider.CreateScope(); + var migrator = scope.ServiceProvider.GetRequiredService(); + + 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/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")); + } +} diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs deleted file mode 100644 index 32a09c87e4..0000000000 --- a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs +++ /dev/null @@ -1,37 +0,0 @@ -// ReSharper disable once CheckNamespace -namespace ServiceControl.Persistence.Tests; - -using System; -using System.IO; -using System.Threading.Tasks; -using EFCore.SqlServer; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using NUnit.Framework; -using ServiceControl.Persistence.EFCore.Abstractions; - -[TestFixture] -class SchemaMustExistTests -{ - [Test] - public async Task Migration_fails_when_the_configured_schema_does_not_exist() - { - var settings = new SqlServerPersisterSettings - { - ConnectionString = await SqlServerSharedContainer.GetConnectionStringAsync(), - Schema = $"sc_absent_{Guid.NewGuid():n}", - BodyStorage = new FileSystemBodyStorageSettings { StoragePath = Path.GetTempPath() } - }; - - var services = new ServiceCollection(); - services.AddLogging(); - new SqlServerPersistenceConfiguration().Create(settings).AddInstaller(services); - - await using var provider = services.BuildServiceProvider(); - 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")); - } -} diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaSetupTests.cs b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaSetupTests.cs new file mode 100644 index 0000000000..bd75b98896 --- /dev/null +++ b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaSetupTests.cs @@ -0,0 +1,90 @@ +// ReSharper disable once CheckNamespace +namespace ServiceControl.Persistence.Tests; + +using System; +using System.IO; +using System.Threading.Tasks; +using EFCore.SqlServer; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using NUnit.Framework; +using ServiceControl.Persistence.EFCore.Abstractions; + +[TestFixture] +class SchemaSetupTests +{ + [Test] + public async Task Migration_creates_the_configured_schema_when_it_does_not_exist() + { + var settings = await CreateSettings($"sc_absent_{Guid.NewGuid():n}"); + + try + { + Assert.DoesNotThrowAsync(() => ApplyMigrations(settings)); + } + finally + { + await TestSchema.Drop(settings.ConnectionString, settings.Schema); + } + } + + [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(), + 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); + + await using var provider = services.BuildServiceProvider(); + using var scope = provider.CreateScope(); + var migrator = scope.ServiceProvider.GetRequiredService(); + + 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..863d158462 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs @@ -62,15 +62,19 @@ public async Task Should_report_configured_values_in_the_unit_their_keys_name() } } - [Test] - public async Task Should_report_a_custom_schema_without_naming_it() + [TestCase(null, "Default")] + [TestCase("sc_primary", "Custom")] + [TestCase("sc", "Custom")] + [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 new file mode 100644 index 0000000000..79292f3cf5 --- /dev/null +++ b/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs @@ -0,0 +1,76 @@ +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_is_null(string configured) + { + Environment.SetEnvironmentVariable(SchemaVariable, configured); + + Assert.That(CreateSettings().Schema, Is.Null); + } + + [TestCase("sc_primary")] + [TestCase("legacy")] + [TestCase("dbo")] + [TestCase("public")] + [TestCase("psw_sc")] + public void Configured_schema_is_used(string schema) + { + Environment.SetEnvironmentVariable(SchemaVariable, schema); + + Assert.That(CreateSettings().Schema, Is.EqualTo(schema)); + } + + 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; +}