diff --git a/infra/docker/backend-dotnet.Dockerfile b/infra/docker/backend-dotnet.Dockerfile index 007be4da..ff090405 100644 --- a/infra/docker/backend-dotnet.Dockerfile +++ b/infra/docker/backend-dotnet.Dockerfile @@ -122,6 +122,11 @@ COPY --from=frontend-build /app/src/frontend/build /app/wwwroot # externally (e.g. golang-migrate run by a Helm pre-job). COPY src/backend/clickhouse/migrations /app/clickhouse-migrations +# Postgres analytics migration files (HOL-26) — applied at startup by +# PostgresMigrationService when the Postgres analytics backend is enabled. +# Disable via PostgresAnalytics__Migrations__Disabled=true. +COPY src/dotnet/src/HoldFast.Data.Postgres/Migrations /app/postgres-analytics-migrations + # Default port — matches Go backend convention ENV ASPNETCORE_URLS=http://+:8082 EXPOSE 8082 diff --git a/src/dotnet/HoldFast.Backend.slnx b/src/dotnet/HoldFast.Backend.slnx index b698bfe0..b0445e94 100644 --- a/src/dotnet/HoldFast.Backend.slnx +++ b/src/dotnet/HoldFast.Backend.slnx @@ -3,6 +3,7 @@ + diff --git a/src/dotnet/src/HoldFast.Api/HoldFast.Api.csproj b/src/dotnet/src/HoldFast.Api/HoldFast.Api.csproj index fca58f3c..c540da9f 100644 --- a/src/dotnet/src/HoldFast.Api/HoldFast.Api.csproj +++ b/src/dotnet/src/HoldFast.Api/HoldFast.Api.csproj @@ -4,6 +4,7 @@ + diff --git a/src/dotnet/src/HoldFast.Api/Program.cs b/src/dotnet/src/HoldFast.Api/Program.cs index d44c2b2d..c353f21f 100644 --- a/src/dotnet/src/HoldFast.Api/Program.cs +++ b/src/dotnet/src/HoldFast.Api/Program.cs @@ -175,6 +175,22 @@ req.RequestUri is null || builder.Configuration.GetSection("ClickHouse:Migrations")); builder.Services.AddHostedService(); +// ── Postgres analytics (HOL-26 scaffolding) ────────────────────────── +// Companion analytics backend; lives alongside ClickHouse rather than +// replacing it (the Storage:Analytics switch in HOL-34 chooses one at +// runtime). For HOL-26 the PG migration runner is the only thing wired +// up — its job is to ensure the analytics schema + schema_migrations +// table exist on a fresh Postgres so HOL-29+ implementations have a +// place to add their per-domain tables. +// +// Disabled by default so existing deployments don't get extra startup +// work; opt in via PostgresAnalytics__Migrations__Disabled=false. +builder.Services.Configure( + builder.Configuration.GetSection("PostgresAnalytics")); +builder.Services.Configure( + builder.Configuration.GetSection("PostgresAnalytics:Migrations")); +builder.Services.AddHostedService(); + // ── Storage ─────────────────────────────────────────────────────────── builder.Services.Configure( builder.Configuration.GetSection("Storage")); diff --git a/src/dotnet/src/HoldFast.Api/appsettings.json b/src/dotnet/src/HoldFast.Api/appsettings.json index b528c17a..cfb62183 100644 --- a/src/dotnet/src/HoldFast.Api/appsettings.json +++ b/src/dotnet/src/HoldFast.Api/appsettings.json @@ -19,6 +19,12 @@ "ReadonlyUsername": "default", "ReadonlyPassword": "" }, + "PostgresAnalytics": { + "Schema": "analytics", + "Migrations": { + "Disabled": false + } + }, "Storage": { "Type": "filesystem", "FilesystemRoot": "/tmp/holdfast-storage" diff --git a/src/dotnet/src/HoldFast.Data.Postgres/HoldFast.Data.Postgres.csproj b/src/dotnet/src/HoldFast.Data.Postgres/HoldFast.Data.Postgres.csproj new file mode 100644 index 00000000..98ca3e79 --- /dev/null +++ b/src/dotnet/src/HoldFast.Data.Postgres/HoldFast.Data.Postgres.csproj @@ -0,0 +1,50 @@ + + + + + + + + + + + + + + + + net10.0 + enable + enable + + + + + + + + + + PreserveNewest + + + + diff --git a/src/dotnet/src/HoldFast.Data.Postgres/Migrations/0001_create_analytics_schema.up.sql b/src/dotnet/src/HoldFast.Data.Postgres/Migrations/0001_create_analytics_schema.up.sql new file mode 100644 index 00000000..cdff4e70 --- /dev/null +++ b/src/dotnet/src/HoldFast.Data.Postgres/Migrations/0001_create_analytics_schema.up.sql @@ -0,0 +1,14 @@ +-- HOL-26: Postgres analytics schema bootstrap. +-- +-- The PostgresMigrationService.EnsureSchemaAsync method already creates this +-- schema before applying any migration (so the migration runner itself can +-- insert into analytics.schema_migrations). This file exists so the schema +-- creation is also recorded in the migration history — operators reading +-- *.up.sql get a complete schema-of-record story without needing to know +-- about the runner's bootstrap step. +CREATE SCHEMA IF NOT EXISTS analytics; + +COMMENT ON SCHEMA analytics IS + 'HoldFast analytics tables (logs, traces, sessions, errors, metrics). ' + 'Owned by HoldFast.Data.Postgres. Separate from public schema which ' + 'holds the relational data managed by HoldFast.Data + EF Core.'; diff --git a/src/dotnet/src/HoldFast.Data.Postgres/Migrations/0002_create_schema_migrations.up.sql b/src/dotnet/src/HoldFast.Data.Postgres/Migrations/0002_create_schema_migrations.up.sql new file mode 100644 index 00000000..419f163e --- /dev/null +++ b/src/dotnet/src/HoldFast.Data.Postgres/Migrations/0002_create_schema_migrations.up.sql @@ -0,0 +1,20 @@ +-- HOL-26: Migration tracking table. +-- +-- PostgresMigrationService.EnsureMigrationsTableAsync also creates this table +-- before applying any migration; this file exists for the same documentary +-- reason as 0001 — *.up.sql files together describe the full schema state. +-- +-- Schema parity with golang-migrate's postgres driver (version + dirty), +-- plus an applied_at column for operator convenience. +CREATE TABLE IF NOT EXISTS analytics.schema_migrations +( + version BIGINT PRIMARY KEY, + dirty BOOLEAN NOT NULL DEFAULT false, + applied_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +COMMENT ON TABLE analytics.schema_migrations IS + 'HOL-26: tracks applied analytics-schema migrations. ' + 'dirty=true rows indicate a partial-failure mid-migration that needs ' + 'manual investigation (the runner uses transactional DDL so this should ' + 'be rare).'; diff --git a/src/dotnet/src/HoldFast.Data.Postgres/Migrations/0003_install_extensions.up.sql b/src/dotnet/src/HoldFast.Data.Postgres/Migrations/0003_install_extensions.up.sql new file mode 100644 index 00000000..b1493f4b --- /dev/null +++ b/src/dotnet/src/HoldFast.Data.Postgres/Migrations/0003_install_extensions.up.sql @@ -0,0 +1,42 @@ +-- HOL-26: install Postgres extensions used by the analytics schema. +-- +-- Both extensions are conditional — CREATE EXTENSION IF NOT EXISTS is a +-- no-op when already installed, and the surrounding DO blocks let us +-- gracefully no-op when the extension isn't available in the running PG +-- image (e.g. vanilla `postgres:16` doesn't ship TimescaleDB; the hobby +-- compose uses `ankane/pgvector` which doesn't either). +-- +-- When the extensions ARE present (e.g. `timescale/timescaledb-ha`), HOL-29+ +-- migrations will use them for hypertable partitioning + retention. +-- When absent, HOL-29+ falls back to native PG partitioning via pg_partman +-- (also conditional) or per-month declarative partitions managed in-app. +-- +-- Reference: docs.timescale.com/self-hosted/latest/install/installation-docker + +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_available_extensions WHERE name = 'timescaledb') THEN + CREATE EXTENSION IF NOT EXISTS timescaledb; + RAISE NOTICE 'HOL-26: TimescaleDB extension enabled'; + ELSE + RAISE NOTICE + 'HOL-26: TimescaleDB extension not available in this PG image. ' + 'Analytics tables will use native partitioning. ' + 'For larger deployments switch the postgres image to ' + 'timescale/timescaledb-ha and re-run migrations.'; + END IF; +END +$$; + +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_available_extensions WHERE name = 'pg_partman') THEN + CREATE EXTENSION IF NOT EXISTS pg_partman; + RAISE NOTICE 'HOL-26: pg_partman extension enabled'; + ELSE + RAISE NOTICE + 'HOL-26: pg_partman not available. Retention will fall back to ' + 'in-app DELETE WHERE timestamp < … instead of partition drops.'; + END IF; +END +$$; diff --git a/src/dotnet/src/HoldFast.Data.Postgres/PostgresAnalyticsOptions.cs b/src/dotnet/src/HoldFast.Data.Postgres/PostgresAnalyticsOptions.cs new file mode 100644 index 00000000..dee8249a --- /dev/null +++ b/src/dotnet/src/HoldFast.Data.Postgres/PostgresAnalyticsOptions.cs @@ -0,0 +1,51 @@ +namespace HoldFast.Data.Postgres; + +/// +/// Configuration for the Postgres analytics backend. +/// +/// HOL-26: HoldFast's hobby deployment runs a single Postgres container that +/// holds both the relational data (users, projects, workspaces — owned by +/// HoldFast.Data via EF Core) AND the analytics data (logs, traces, sessions, +/// errors, metrics — owned by HoldFast.Data.Postgres via direct Npgsql). +/// The two share a host but live in separate schemas (relational uses +/// `public`; analytics uses `analytics`). +/// +/// Production deployments can point AnalyticsConnectionString at a separate +/// Postgres instance for capacity isolation; HoldFast doesn't care. +/// +public class PostgresAnalyticsOptions +{ + /// + /// Npgsql connection string for the analytics database. If unset, falls + /// back to ConnectionStrings:PostgreSQL (the relational connection), + /// which is the right default for the hobby/lean stack where one PG + /// container hosts both schemas. + /// + public string? ConnectionString { get; set; } + + /// + /// Schema name for analytics tables. Default `analytics` keeps the + /// relational `public` schema clean. + /// + public string Schema { get; set; } = "analytics"; +} + +/// +/// Configuration for the Postgres analytics migrations runner. +/// +public class PostgresAnalyticsMigrationOptions +{ + /// + /// Filesystem path to the directory containing *.up.sql migration files. + /// In the Docker image these are copied to /app/postgres-analytics-migrations. + /// Locally (e.g. during dev runs) point this at + /// src/dotnet/src/HoldFast.Data.Postgres/Migrations. + /// + public string Path { get; set; } = "/app/postgres-analytics-migrations"; + + /// + /// Skip running migrations. Useful when the analytics schema is managed + /// externally (Helm pre-jobs, external golang-migrate runs, etc). + /// + public bool Disabled { get; set; } +} diff --git a/src/dotnet/src/HoldFast.Data.Postgres/PostgresMigrationService.cs b/src/dotnet/src/HoldFast.Data.Postgres/PostgresMigrationService.cs new file mode 100644 index 00000000..d8c02838 --- /dev/null +++ b/src/dotnet/src/HoldFast.Data.Postgres/PostgresMigrationService.cs @@ -0,0 +1,238 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Npgsql; + +namespace HoldFast.Data.Postgres; + +/// +/// Runs Postgres analytics-schema migrations from a directory at startup, idempotently. +/// +/// Mirrors the design of HoldFast.Data.ClickHouse.ClickHouseMigrationService +/// so the operator experience is identical across backends — same file naming +/// convention (NNNN_description.up.sql), same logging shape, same disable knob. +/// +/// Differences from the CH runner: +/// - Postgres supports transactional DDL, so each migration file runs in its +/// own transaction. A failed migration leaves the analytics schema unchanged +/// instead of needing a separate dirty flag. (We still record dirty/clean +/// in schema_migrations for parity with golang-migrate's tracking schema, +/// though it's effectively informational here.) +/// - The schema_migrations table is in `analytics` schema (vs CH's `default`), +/// keeping all analytics state in one namespace. +/// - This service ensures the `analytics` schema itself exists before applying +/// any migration. The first migration assumes the schema is present. +/// +public class PostgresMigrationService : IHostedService +{ + private readonly PostgresAnalyticsOptions _pgOptions; + private readonly PostgresAnalyticsMigrationOptions _options; + private readonly IConfiguration _configuration; + private readonly ILogger _logger; + + public PostgresMigrationService( + IOptions pgOptions, + IOptions options, + IConfiguration configuration, + ILogger logger) + { + _pgOptions = pgOptions.Value; + _options = options.Value; + _configuration = configuration; + _logger = logger; + } + + public async Task StartAsync(CancellationToken cancellationToken) + { + if (_options.Disabled) + { + _logger.LogInformation("Analytics PG migrations: disabled by configuration, skipping"); + return; + } + + if (string.IsNullOrEmpty(_options.Path) || !Directory.Exists(_options.Path)) + { + _logger.LogWarning( + "Analytics PG migrations: directory {Path} not found, skipping. " + + "Set PostgresAnalytics:Migrations:Path or mount the migrations folder.", + _options.Path); + return; + } + + var connStr = ResolveConnectionString(); + if (string.IsNullOrEmpty(connStr)) + { + _logger.LogWarning( + "Analytics PG migrations: no connection string configured " + + "(neither PostgresAnalytics:ConnectionString nor ConnectionStrings:PostgreSQL). Skipping."); + return; + } + + await using var conn = new NpgsqlConnection(connStr); + await conn.OpenAsync(cancellationToken); + + await EnsureSchemaAsync(conn, cancellationToken); + await EnsureMigrationsTableAsync(conn, cancellationToken); + var applied = await GetAppliedVersionsAsync(conn, cancellationToken); + + var files = Directory.GetFiles(_options.Path, "*.up.sql") + .OrderBy(f => Path.GetFileName(f), StringComparer.Ordinal) + .ToList(); + + var appliedCount = 0; + foreach (var file in files) + { + var version = ParseVersion(Path.GetFileName(file)); + if (version is null) continue; // not a migration we recognize + if (applied.Contains(version.Value)) continue; + + await ApplyMigrationAsync(conn, version.Value, file, cancellationToken); + appliedCount++; + } + + _logger.LogInformation( + "Analytics PG migrations: {Applied} applied, {Total} total, {AlreadyApplied} already-applied", + appliedCount, files.Count, applied.Count); + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + private string? ResolveConnectionString() => + !string.IsNullOrEmpty(_pgOptions.ConnectionString) + ? _pgOptions.ConnectionString + : _configuration.GetConnectionString("PostgreSQL"); + + private async Task EnsureSchemaAsync(NpgsqlConnection conn, CancellationToken ct) + { + // CREATE SCHEMA IF NOT EXISTS is idempotent and safe to run on every start. + // The schema name is config-driven but defaults to `analytics`; we quote-escape + // the identifier rather than parameterize it because PG doesn't accept bound + // parameters for DDL identifiers. + var schema = SanitizeIdentifier(_pgOptions.Schema); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = $"CREATE SCHEMA IF NOT EXISTS \"{schema}\""; + await cmd.ExecuteNonQueryAsync(ct); + } + + private async Task EnsureMigrationsTableAsync(NpgsqlConnection conn, CancellationToken ct) + { + // Schema parity with golang-migrate's postgres driver: version + dirty. + // We add applied_at for operator convenience; golang-migrate doesn't, but + // since this table isn't shared with a Go runner the extra column is fine. + var schema = SanitizeIdentifier(_pgOptions.Schema); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = $@" + CREATE TABLE IF NOT EXISTS ""{schema}"".schema_migrations + ( + version BIGINT PRIMARY KEY, + dirty BOOLEAN NOT NULL DEFAULT false, + applied_at TIMESTAMPTZ NOT NULL DEFAULT now() + )"; + await cmd.ExecuteNonQueryAsync(ct); + } + + private async Task> GetAppliedVersionsAsync( + NpgsqlConnection conn, CancellationToken ct) + { + var schema = SanitizeIdentifier(_pgOptions.Schema); + await using var cmd = conn.CreateCommand(); + cmd.CommandText = $@" + SELECT version + FROM ""{schema}"".schema_migrations + WHERE dirty = false"; + await using var reader = await cmd.ExecuteReaderAsync(ct); + var set = new HashSet(); + while (await reader.ReadAsync(ct)) + set.Add(reader.GetInt64(0)); + return set; + } + + private async Task ApplyMigrationAsync( + NpgsqlConnection conn, long version, string file, CancellationToken ct) + { + var name = Path.GetFileName(file); + _logger.LogInformation("Analytics PG migration: applying {Version} {Name}", version, name); + + var sql = await File.ReadAllTextAsync(file, ct); + var schema = SanitizeIdentifier(_pgOptions.Schema); + + // Run the whole migration in one transaction. PG handles DDL transactionally, + // so a failure rolls back the schema changes AND the dirty-row insert below + // (rather than leaving the analytics schema half-built). + // + // Edge case: migrations that need CREATE INDEX CONCURRENTLY can't run inside + // a transaction. We don't have any of those in HOL-26's foundational set, but + // when we hit one in HOL-29+ we'll add a `-- migrate:no-tx` marker and split + // the path. Out of scope for this PR. + await using var tx = await conn.BeginTransactionAsync(ct); + try + { + // Insert dirty marker first so a crash mid-migration leaves a row + // we can detect and fix manually. + await using (var ins = conn.CreateCommand()) + { + ins.Transaction = tx; + ins.CommandText = $@" + INSERT INTO ""{schema}"".schema_migrations (version, dirty) + VALUES (@v, true) + ON CONFLICT (version) DO UPDATE SET dirty = true, applied_at = now()"; + ins.Parameters.AddWithValue("v", version); + await ins.ExecuteNonQueryAsync(ct); + } + + // Apply the migration body. Npgsql can run a multi-statement batch + // as a single command — semicolon-separated DDL works directly. + await using (var migrate = conn.CreateCommand()) + { + migrate.Transaction = tx; + migrate.CommandText = sql; + await migrate.ExecuteNonQueryAsync(ct); + } + + // Mark clean. + await using (var clean = conn.CreateCommand()) + { + clean.Transaction = tx; + clean.CommandText = $@" + UPDATE ""{schema}"".schema_migrations + SET dirty = false, applied_at = now() + WHERE version = @v"; + clean.Parameters.AddWithValue("v", version); + await clean.ExecuteNonQueryAsync(ct); + } + + await tx.CommitAsync(ct); + } + catch + { + await tx.RollbackAsync(ct); + throw; + } + } + + internal static long? ParseVersion(string filename) + { + var idx = filename.IndexOf('_'); + if (idx <= 0) return null; + return long.TryParse(filename[..idx], out var n) ? n : null; + } + + /// + /// Defensive identifier sanitization for the schema name. The schema is + /// operator-controlled (config), not user input, but we still strip + /// anything that isn't an alphanumeric or underscore so a typo can't + /// produce SQL injection. + /// + internal static string SanitizeIdentifier(string raw) + { + if (string.IsNullOrWhiteSpace(raw)) + throw new InvalidOperationException("PostgresAnalytics:Schema cannot be empty"); + + var clean = new string(raw.Where(c => char.IsLetterOrDigit(c) || c == '_').ToArray()); + if (string.IsNullOrEmpty(clean)) + throw new InvalidOperationException( + $"PostgresAnalytics:Schema '{raw}' has no valid identifier characters"); + return clean; + } +} diff --git a/src/dotnet/tests/HoldFast.Data.Tests/HoldFast.Data.Tests.csproj b/src/dotnet/tests/HoldFast.Data.Tests/HoldFast.Data.Tests.csproj index e23c4407..1d29ab7f 100644 --- a/src/dotnet/tests/HoldFast.Data.Tests/HoldFast.Data.Tests.csproj +++ b/src/dotnet/tests/HoldFast.Data.Tests/HoldFast.Data.Tests.csproj @@ -21,6 +21,7 @@ + diff --git a/src/dotnet/tests/HoldFast.Data.Tests/Postgres/PostgresMigrationServiceTests.cs b/src/dotnet/tests/HoldFast.Data.Tests/Postgres/PostgresMigrationServiceTests.cs new file mode 100644 index 00000000..aa19510e --- /dev/null +++ b/src/dotnet/tests/HoldFast.Data.Tests/Postgres/PostgresMigrationServiceTests.cs @@ -0,0 +1,104 @@ +using HoldFast.Data.Postgres; + +namespace HoldFast.Data.Tests.Postgres; + +/// +/// HOL-26 unit tests for the PostgresMigrationService pure helpers. +/// +/// Note: there's no live-Postgres test here — the runner's actual SQL +/// execution is exercised end-to-end by the smoke test (compose up, +/// observe "Analytics PG migrations: 3 applied" in logs). Future PRs +/// may add a Testcontainers-based integration test once we have a +/// reason to hit migration edge cases (e.g. non-transactional DDL). +/// +/// Per the project's "OVER TEST" .NET testing rule, this file covers +/// happy paths AND deliberate failure cases for both pure helpers. +/// +public class PostgresMigrationServiceTests +{ + // ── ParseVersion ───────────────────────────────────────────────── + + [Theory] + [InlineData("0001_create_analytics_schema.up.sql", 1)] + [InlineData("0042_add_logs_table.up.sql", 42)] + [InlineData("999999_huge_version.up.sql", 999999)] + [InlineData("0_zero_version.up.sql", 0)] + public void ParseVersion_extracts_numeric_prefix(string filename, long expected) + { + Assert.Equal(expected, PostgresMigrationService.ParseVersion(filename)); + } + + [Theory] + [InlineData("no_underscore.sql")] // no _ at all + [InlineData("abc_text_prefix.up.sql")] // non-numeric prefix + [InlineData("_starts_with_underscore.up.sql")] // _ at index 0 + [InlineData("12.5_decimal_prefix.up.sql")] // floats not allowed (long.TryParse rejects) + [InlineData("")] // empty filename + public void ParseVersion_returns_null_when_prefix_invalid(string filename) + { + Assert.Null(PostgresMigrationService.ParseVersion(filename)); + } + + [Fact] + public void ParseVersion_handles_filename_with_extra_underscores_in_description() + { + // Real migrations often have multiple underscores in their description + // ("0042_add_user_table_for_oauth.up.sql"). Parser only cares about + // the prefix before the FIRST underscore. + Assert.Equal(42, PostgresMigrationService.ParseVersion("0042_add_user_table_for_oauth.up.sql")); + } + + // ── SanitizeIdentifier ─────────────────────────────────────────── + + [Theory] + [InlineData("analytics", "analytics")] + [InlineData("Analytics_v2", "Analytics_v2")] + [InlineData("snake_case_name", "snake_case_name")] + [InlineData("PascalCase", "PascalCase")] + [InlineData("digits123", "digits123")] + [InlineData("0_starts_with_digit", "0_starts_with_digit")] + public void SanitizeIdentifier_passes_safe_identifiers_through(string raw, string expected) + { + Assert.Equal(expected, PostgresMigrationService.SanitizeIdentifier(raw)); + } + + [Theory] + [InlineData("ana lytics", "analytics")] // strips spaces + [InlineData("ana-lytics", "analytics")] // strips hyphens + [InlineData("ana.lytics", "analytics")] // strips dots + [InlineData("ana\"lytics", "analytics")] // strips quotes + [InlineData("ana'lytics", "analytics")] // strips apostrophes + [InlineData("ana;DROP TABLE foo;--", "anaDROPTABLEfoo")] // strips SQLi attempt + [InlineData("ana/lytics\\bar", "analyticsbar")] // strips slashes + public void SanitizeIdentifier_strips_unsafe_characters(string raw, string expected) + { + // The schema name comes from operator config, not user input, but stripping + // anything that isn't [A-Za-z0-9_] guarantees we can't produce SQL injection + // even on a typo or misconfiguration. + Assert.Equal(expected, PostgresMigrationService.SanitizeIdentifier(raw)); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + [InlineData("\t\n")] + public void SanitizeIdentifier_throws_on_empty_or_whitespace(string raw) + { + Assert.Throws( + () => PostgresMigrationService.SanitizeIdentifier(raw)); + } + + [Theory] + [InlineData(";--")] // pure SQL injection + [InlineData("()")] // pure punctuation + [InlineData("[]<>")] + [InlineData(@"\\\\")] + public void SanitizeIdentifier_throws_when_no_valid_chars_remain(string raw) + { + // Edge case: a config value that's entirely punctuation strips down to + // empty string — we throw rather than silently use "" as the schema name + // (which would make CREATE SCHEMA "" fail with a confusing PG error). + Assert.Throws( + () => PostgresMigrationService.SanitizeIdentifier(raw)); + } +}