diff --git a/src/GenWave.Host/Configuration/StationSettingsConfigurationProvider.cs b/src/GenWave.Host/Configuration/StationSettingsConfigurationProvider.cs index f8a69fc3..021104f5 100644 --- a/src/GenWave.Host/Configuration/StationSettingsConfigurationProvider.cs +++ b/src/GenWave.Host/Configuration/StationSettingsConfigurationProvider.cs @@ -1,7 +1,8 @@ +using System.Data.Common; using System.Text.Json; +using GenWave.MediaLibrary.Station; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Primitives; -using Npgsql; namespace GenWave.Host.Configuration; @@ -16,10 +17,18 @@ namespace GenWave.Host.Configuration; /// Call (or expose via ) to /// raise the change token and trigger /// re-binding without an API restart. +/// +/// gh-#406 slice 5: the raw station.settings row I/O this class used to open directly via +/// NpgsqlConnection now lives in +/// ( documents why that method — not +/// — is the one this class calls). Every other +/// behavior below (allowlist filtering, array expansion, the degrade-to-empty-overlay posture, +/// stderr diagnostics) stays exactly as it was. /// public class StationSettingsConfigurationProvider : IConfigurationProvider, IDisposable { readonly string connectionString; + readonly StationSettingsRepository repository; // The mutable data bag surfaced to IConfiguration. IDictionary data = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -30,6 +39,14 @@ public class StationSettingsConfigurationProvider : IConfigurationProvider, IDis public StationSettingsConfigurationProvider(string connectionString) { this.connectionString = connectionString; + + // Constructed directly here, not resolved through DI: Load() (below) is called by the + // configuration system while Microsoft.Extensions.Configuration.IConfigurationBuilder itself + // is being built, before WebApplicationBuilder.Build() ever creates a container capable of + // constructing — let alone injecting — a repository. StationSettingsRepository's + // plain-connection-string ctor exists precisely for this pre-DI boot path (see that class's + // own remarks, gh-#406 slice 3/4/5). + repository = new StationSettingsRepository(connectionString); } // ── IConfigurationProvider ───────────────────────────────────────────── @@ -72,10 +89,10 @@ public virtual void Load() // Treat the same as a DB that is temporarily unreachable: the overlay is empty and // env/appsettings defaults apply. This guard prevents the Npgsql library from throwing // InvalidOperationException ("ConnectionString not initialized") before attempting a - // TCP connection — an exception type that the NpgsqlException catch below does not cover. + // TCP connection — an exception type that the DbException catch below does not cover. if (string.IsNullOrWhiteSpace(connectionString)) { - // Match the NpgsqlException catch below: surface the degradation so an accidentally + // Match the DbException catch below: surface the degradation so an accidentally // empty Station connection string in a real deploy is observable, not silent. Console.Error.WriteLine("[station-settings] no Station connection string; using config defaults"); data = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -85,18 +102,12 @@ public virtual void Load() var loaded = new Dictionary(StringComparer.OrdinalIgnoreCase); try { - using var conn = new NpgsqlConnection(connectionString); - conn.Open(); - using var cmd = conn.CreateCommand(); - cmd.CommandText = "SELECT key, value FROM station.settings"; - using var reader = cmd.ExecuteReader(); - while (reader.Read()) + var rows = repository.ReadAllForBoot(); + foreach (var (key, jsonValue) in rows) { - var key = reader.GetString(0); if (!StationSettingsAllowlist.ByKey.ContainsKey(key)) continue; // paranoid guard: skip anything not on the allowlist - var jsonValue = reader.GetString(1); var scalar = ExtractScalar(jsonValue); if (scalar is not null) loaded[key] = scalar; @@ -104,13 +115,19 @@ public virtual void Load() ExtractArrayItems(loaded, key, jsonValue); } } - catch (NpgsqlException ex) + catch (DbException ex) { // No station schema yet, wrong password, DB down — none of these should prevent boot. // Defaults from env/appsettings continue to apply; the overlay is empty until the // DB is reachable and station.settings is populated. // A logger is not injectable here (provider builds before DI), so we surface the // failure via stderr so operators can diagnose without exposing connection secrets. + // + // Catches System.Data.Common.DbException — the provider-neutral ADO.NET base type + // Npgsql.NpgsqlException itself derives from — rather than NpgsqlException directly: + // this class carries no Npgsql reference at all now that StationSettingsRepository owns + // the Postgres specifics (gh-#406 slice 5), and DbException catches every failure the + // original catch did (and nothing broader). Console.Error.WriteLine($"[station-settings] overlay load failed; using config defaults: {ex.Message}"); } diff --git a/src/GenWave.MediaLibrary/Station/StationSettingsRepository.cs b/src/GenWave.MediaLibrary/Station/StationSettingsRepository.cs index 0daf6561..dc52143c 100644 --- a/src/GenWave.MediaLibrary/Station/StationSettingsRepository.cs +++ b/src/GenWave.MediaLibrary/Station/StationSettingsRepository.cs @@ -102,4 +102,35 @@ public async Task ExistsAsync(string key, CancellationToken ct) await using var reader = await cmd.ExecuteReaderAsync(ct); return await reader.ReadAsync(ct); } + + /// + /// SYNCHRONOUS read of every row in station.settings — the one deliberate sync exception + /// in this otherwise async-only repository, added for gh-#406 slice 5. + /// GenWave.Host.Configuration.StationSettingsConfigurationProvider.Load() implements + /// , a synchronous + /// contract member the configuration system calls while + /// itself is still being + /// built — the same pre-DI boot path this class's plain-connection-string ctor exists for — and + /// has no async entry point available there to await from. SQL is + /// byte-identical to 's; only the sync/async shape differs. Any failure + /// (including a ) propagates to the caller — same posture as + /// /, degrade policy is a caller concern, not + /// this repository's. + /// + public IReadOnlyDictionary ReadAllForBoot() + { + var result = new Dictionary(StringComparer.OrdinalIgnoreCase); + + using var conn = new NpgsqlConnection(connectionString); + conn.Open(); + + using var cmd = conn.CreateCommand(); + cmd.CommandText = "SELECT key, value FROM station.settings"; + + using var reader = cmd.ExecuteReader(); + while (reader.Read()) + result[reader.GetString(0)] = reader.GetString(1); + + return result; + } } diff --git a/tests/GenWave.Architecture.Tests/Support/ExemptionBaseline.cs b/tests/GenWave.Architecture.Tests/Support/ExemptionBaseline.cs index cae99b65..06cb6390 100644 --- a/tests/GenWave.Architecture.Tests/Support/ExemptionBaseline.cs +++ b/tests/GenWave.Architecture.Tests/Support/ExemptionBaseline.cs @@ -2,13 +2,13 @@ namespace GenWave.Architecture.Tests.Support; /// /// The F105.2 adoption baseline (PLAN T211): every pre-existing law violation found when this -/// suite went live, named, dated, and reasoned. Two of the seven L2 entries are the law's own -/// designed exemption (constructing NpgsqlDataSource is composition-root wiring, not -/// querying — ARCHITECTURE.md "Architecture governance"); the other five are pre-existing debt that -/// was not trivial to fix in this diff (moving working, already-tested Host code into -/// MediaLibrary's repository layer is a real refactor, not a using-swap) — tracked as gh-#406. -/// A violation whose member is not on this list still fails (STORY-290 AC6) — see -/// . +/// suite went live, named, dated, and reasoned. The two entries here are the law's own designed +/// exemption (constructing NpgsqlDataSource is composition-root wiring, not querying — +/// ARCHITECTURE.md "Architecture governance"); the five 2026-08-07 debt rows that once sat +/// alongside them were burned down via gh-#406 (2026-08-13) — every one of those types now reaches +/// Postgres through a GenWave.MediaLibrary.Station repository instead of opening a raw +/// NpgsqlConnection itself. A violation whose member is not on this list still fails +/// (STORY-290 AC6) — see . /// internal static class ExemptionBaseline { @@ -30,15 +30,5 @@ internal static class ExemptionBaseline "MediaLibrary's own module composition root (AddMediaLibrary): builds the library_svc " + "NpgsqlDataSource and sets Dapper's static DefaultTypeMap — wiring/global config, never a " + "query, same wiring-not-querying exemption as the Host composition root."), - - // ── Pre-existing debt: genuine querying/exception-coupling outside the repository layer, - // found at T211 adoption, not trivial to fix in this diff (follow-up filed as gh-#406). - new ArchitectureExemption( - LawId.L2, - "GenWave.Host.Configuration.StationSettingsConfigurationProvider", - "2026-08-07", - "IConfigurationProvider.Load() queries station.settings directly via NpgsqlConnection at " + - "boot, before the DI container (and any MediaLibrary repository) exists to inject. " + - "Pre-existing (STORY-042); not trivial to fix in this diff — follow-up gh-#406."), }; } diff --git a/tests/GenWave.MediaLibrary.Tests/Specs/Story042_StationSettingsRepository.cs b/tests/GenWave.MediaLibrary.Tests/Specs/Story042_StationSettingsRepository.cs index c6d5a09a..a5afe3bb 100644 --- a/tests/GenWave.MediaLibrary.Tests/Specs/Story042_StationSettingsRepository.cs +++ b/tests/GenWave.MediaLibrary.Tests/Specs/Story042_StationSettingsRepository.cs @@ -12,6 +12,11 @@ // gh-#406 slice 4 added ExistsAsync (single-key existence probe) for // GenWave.Host.Seeding.SafeLoopSeedMarkerStore's boot-seed marker check (F27.10) — its own // coverage is the FeatureExistsAsync section below. +// +// gh-#406 slice 5 added ReadAllForBoot (the sync exception, SQL byte-identical to ReadAllAsync) +// for GenWave.Host.Configuration.StationSettingsConfigurationProvider.Load(), the synchronous +// IConfigurationProvider contract member — its own coverage is the ScenarioReadAllForBoot section +// below. using System.Text.Json; using Dapper; @@ -303,4 +308,58 @@ public async Task ItOnlyReportsTheExactKeyProbedNotOtherRows() Assert.False(exists); } } + + // --------------------------------------------------------------------- + // HAPPY PATH — ReadAllForBoot, the sync exception (gh-#406 slice 5) + // --------------------------------------------------------------------- + + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioReadAllForBoot(DatabaseFixture db) + { + [Fact] + public async Task AnEmptyTableReadsAsAnEmptyDictionary() + { + // Given no rows at all... + await db.ResetSettingsAsync(); + var repo = Repo(db); + + // When every row is read synchronously... + var rows = repo.ReadAllForBoot(); + + // Then nothing comes back. + Assert.Empty(rows); + } + + [Fact] + public async Task AWrittenValueIsReadBackSynchronously() + { + // Given a value written through the async side... + await db.ResetSettingsAsync(); + var repo = Repo(db); + await repo.WriteAsync("Loudness:TargetLufs", -14.0, CancellationToken.None); + + // When every row is read synchronously... + var rows = repo.ReadAllForBoot(); + + // Then the written value comes back exactly, same shape ReadAllAsync would return. + Assert.Equal(JsonSerializer.Serialize(-14.0), rows["Loudness:TargetLufs"]); + } + + [Fact] + public async Task KeysAreLookedUpCaseInsensitivelyJustLikeReadAllAsync() + { + // Given a row stored under its canonical casing... + await db.ResetSettingsAsync(); + await InsertRawRowAsync(db, "Station:Theme", "\"midnight\""); + var repo = Repo(db); + + // When every row is read synchronously... + var rows = repo.ReadAllForBoot(); + + // Then a differently-cased lookup still finds it. + Assert.True(rows.TryGetValue("station:theme", out var value)); + Assert.Equal("\"midnight\"", value); + } + } }