diff --git a/docs/README.md b/docs/README.md index d177340078..8ad624345a 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 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 new file mode 100644 index 0000000000..872a8dc260 --- /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) | `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_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`. diff --git a/docs/usage-report.md b/docs/usage-report.md index 3b56231f9a..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`, `Custom` | `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 | @@ -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 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/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/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..e733b87f58 100644 --- a/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs +++ b/src/ServiceControl.Audit.Persistence.EFCore/DbContexts/AuditDbContext.cs @@ -7,7 +7,6 @@ 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"; public string? Schema { get; } = options.FindExtension()?.Schema; 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 index 6ef1503937..ffae16230a 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,26 @@ 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")); + } + + 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/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/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..3cba5ba45d 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": "sc_primary" + }, { "Name": "ServiceControl/Database/CommandTimeout", "Mandatory": false 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/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..0c6bd6789d 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": "sc_primary" + }, { "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..76f60098af 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 = "sc_primary"; 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..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.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/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 index 26f51e2a5b..dddc4cf3a0 100644 --- a/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/SchemaMustExistTests.cs @@ -16,13 +16,21 @@ 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 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")); + } + + 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 +39,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/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 index 32a09c87e4..e1ad5f3a09 100644 --- a/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs +++ b/src/ServiceControl.Persistence.Tests.SqlServer/SchemaMustExistTests.cs @@ -16,13 +16,21 @@ 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 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")); + } + + 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 +39,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/EFCore/EnvironmentDataConfigurationTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/EnvironmentDataConfigurationTests.cs index 12bda43295..9d2dc47db7 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", "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) { - 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..4f29519d29 --- /dev/null +++ b/src/ServiceControl.Persistence.Tests/EFCore/SchemaConfigurationTests.cs @@ -0,0 +1,75 @@ +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_sc_primary(string configured) + { + Environment.SetEnvironmentVariable(SchemaVariable, configured); + + Assert.That(CreateSettings().Schema, Is.EqualTo("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; +}