Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 33 additions & 37 deletions src/GenWave.Host/Configuration/StationSettingsStore.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
using System.Text.Json;
using System.Data.Common;
using GenWave.Core.Abstractions;
using GenWave.Core.Events;
using GenWave.MediaLibrary.Station;
using Microsoft.Extensions.Logging.Abstractions;
using Npgsql;

namespace GenWave.Host.Configuration;

Expand All @@ -11,11 +11,21 @@ namespace GenWave.Host.Configuration;
/// <see cref="StationSettingsConfigurationProvider"/> to reload so
/// <see cref="Microsoft.Extensions.Options.IOptionsMonitor{T}"/> re-binds without restart.
///
/// Registered as a singleton in DI. Thread-safe (Npgsql connections are created per-operation).
/// Registered as a singleton in DI. Thread-safe (each <see cref="StationSettingsRepository"/> call
/// opens its own connection per-operation).
///
/// gh-#406 slice 3: the raw <c>station.settings</c> row I/O lives in
/// <see cref="StationSettingsRepository"/> (<c>GenWave.MediaLibrary.Station</c>) now — this class
/// builds that repository internally from the same <see cref="connectionString"/> it always took
/// (no DI wiring change; <c>StationSettingsHostingExtensions</c> still constructs this type exactly
/// as before) and keeps only the concerns that are genuinely this store's own: the write-side
/// allowlist guard, the live-reload signal, the change event, and the read-side degrade posture
/// below.
/// </summary>
public sealed class StationSettingsStore : IStationSettingsStore
{
readonly string connectionString;
readonly StationSettingsRepository repository;
readonly StationSettingsConfigurationSource source;
readonly IStationEventSink events;
readonly ILogger<StationSettingsStore> logger;
Expand All @@ -27,6 +37,7 @@ public StationSettingsStore(
ILogger<StationSettingsStore>? logger = null)
{
this.connectionString = connectionString;
repository = new StationSettingsRepository(connectionString);
this.source = source;
this.events = events ?? NoOpStationEventSink.Instance;
this.logger = logger ?? NullLogger<StationSettingsStore>.Instance;
Expand All @@ -38,23 +49,7 @@ public async Task WriteAsync(string key, object value, CancellationToken cancell
if (!StationSettingsAllowlist.ByKey.ContainsKey(key))
throw new ArgumentException($"Key '{key}' is not on the station settings allowlist.", nameof(key));

var json = JsonSerializer.Serialize(value);

await using var conn = new NpgsqlConnection(connectionString);
await conn.OpenAsync(cancellationToken);

await using var cmd = conn.CreateCommand();
cmd.CommandText =
"""
INSERT INTO station.settings (key, value, updated_at)
VALUES (@key, @value::jsonb, now())
ON CONFLICT (key) DO UPDATE
SET value = EXCLUDED.value,
updated_at = EXCLUDED.updated_at
""";
cmd.Parameters.AddWithValue("key", key);
cmd.Parameters.AddWithValue("value", json);
await cmd.ExecuteNonQueryAsync(cancellationToken);
await repository.WriteAsync(key, value, cancellationToken);

// Signal the provider; IOptionsMonitor listeners will see the new value.
source.BuiltProvider?.Reload();
Expand All @@ -70,45 +65,46 @@ ON CONFLICT (key) DO UPDATE
/// unconfigured — the settings page must still render with defaults while Postgres is briefly
/// down, mirroring <see cref="StationSettingsConfigurationProvider.Load"/>'s identical
/// degrade-to-empty-overlay behavior at boot. An empty <see cref="connectionString"/> throws
/// <see cref="InvalidOperationException"/> before a <see cref="NpgsqlException"/> is even
/// reachable (same guard the provider's <c>Load()</c> documents), so both cases are covered.
/// <see cref="InvalidOperationException"/> before a <see cref="DbException"/> is even reachable
/// (same guard the provider's <c>Load()</c> documents), so both cases are covered.
///
/// Catches <see cref="DbException"/> — the provider-neutral ADO.NET base type
/// <see cref="Npgsql.NpgsqlException"/> itself derives from — rather than
/// <c>Npgsql.NpgsqlException</c> directly: this class carries no Npgsql reference at all now that
/// <see cref="StationSettingsRepository"/> owns the Postgres specifics (gh-#406 slice 3), and
/// <see cref="DbException"/> catches every failure the original catch did (and nothing broader —
/// it still lets an <see cref="OperationCanceledException"/> from <paramref name="cancellationToken"/>
/// propagate untouched, same as before).
/// </remarks>
public async Task<IReadOnlyDictionary<string, string>> ReadAllAsync(CancellationToken cancellationToken = default)
{
var result = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);

if (string.IsNullOrWhiteSpace(connectionString))
{
logger.LogWarning("No Station connection string; overlay reads as empty");
return result;
return new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
}

try
{
await using var conn = new NpgsqlConnection(connectionString);
await conn.OpenAsync(cancellationToken);

await using var cmd = conn.CreateCommand();
cmd.CommandText = "SELECT key, value FROM station.settings";
var rows = await repository.ReadAllAsync(cancellationToken);

await using var reader = await cmd.ExecuteReaderAsync(cancellationToken);
while (await reader.ReadAsync(cancellationToken))
var result = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
foreach (var (key, value) in rows)
{
var key = reader.GetString(0);
if (!StationSettingsAllowlist.ByKey.ContainsKey(key))
continue; // never surface a key that slipped through write-path guards

result[key] = reader.GetString(1);
result[key] = value;
}

return result;
}
catch (NpgsqlException ex)
catch (DbException ex)
{
// DB down, wrong password, no station schema yet — none of these should turn
// GET /api/settings into a 500; the overlay is empty until the DB is reachable again.
logger.LogWarning(ex, "Overlay read failed; treating as empty");
return new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
}

return result;
}
}
84 changes: 84 additions & 0 deletions src/GenWave.MediaLibrary/Station/StationSettingsRepository.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
using System.Text.Json;
using Npgsql;

namespace GenWave.MediaLibrary.Station;

/// <summary>
/// The <c>station.settings</c> key-value row I/O (gh-#406 slice 3): <c>WriteAsync</c>/
/// <c>ReadAllAsync</c> moved here byte-identical from
/// <c>GenWave.Host.Configuration.StationSettingsStore</c>, which now delegates to this class instead
/// of opening <see cref="NpgsqlConnection"/>s itself (STORY-042's original write side).
///
/// <paramref name="connectionString"/> arrives as a PLAIN string, not the
/// <see cref="Lazy{T}"/>&lt;<see cref="NpgsqlDataSource"/>&gt; every sibling repository in this
/// namespace (<see cref="PersonaRepository"/>, <see cref="RequestRepository"/>, <see cref="ShowRepository"/>,
/// ...) is built from — a deliberate deviation, not an oversight. gh-#406's remaining slices (4:
/// <c>SafeLoopSeedMarkerStore</c>, 5: <c>StationSettingsConfigurationProvider</c>) both need this
/// class constructible OUTSIDE the DI container, on the pre-DI boot path — most pointedly,
/// <c>StationSettingsConfigurationProvider.Load()</c> runs as part of building
/// <see cref="Microsoft.Extensions.Configuration.IConfigurationBuilder"/> itself, before
/// <c>WebApplicationBuilder.Build()</c> ever creates a container capable of constructing (let alone
/// injecting) a built <see cref="NpgsqlDataSource"/>. A plain connection string needs no container
/// and no builder step to exist, so slices 4/5 can construct this repository directly from the raw
/// <c>ConnectionStrings:Station</c> value the same way <c>StationSettingsStore</c> already does today.
///
/// Connection-per-call against a short-lived <see cref="NpgsqlConnection"/>, exactly as the code
/// this class was extracted from — no data-source pooling, matching the original's "thread-safe,
/// Npgsql connections are created per-operation" contract verbatim.
///
/// <see cref="ReadAllAsync"/> returns EVERY row, unfiltered: the settings allowlist
/// (<c>GenWave.Host.Configuration.StationSettingsAllowlist</c>) is a Host-only concern this project
/// has no reference to and must not gain one (L2/L1 confinement) — filtering by allowlist stays the
/// caller's job, same as it always has been for the write-side allowlist check.
/// </summary>
public sealed class StationSettingsRepository(string connectionString)
{
/// <summary>
/// Upserts <paramref name="value"/> (JSON-serialized) under <paramref name="key"/>. No allowlist
/// check here — that guard lives at the caller (<c>StationSettingsStore.WriteAsync</c>), the same
/// separation every other MediaLibrary repository keeps from its own callers' business rules.
/// </summary>
public async Task WriteAsync(string key, object value, CancellationToken ct)
{
var json = JsonSerializer.Serialize(value);

await using var conn = new NpgsqlConnection(connectionString);
await conn.OpenAsync(ct);

await using var cmd = conn.CreateCommand();
cmd.CommandText =
"""
INSERT INTO station.settings (key, value, updated_at)
VALUES (@key, @value::jsonb, now())
ON CONFLICT (key) DO UPDATE
SET value = EXCLUDED.value,
updated_at = EXCLUDED.updated_at
""";
cmd.Parameters.AddWithValue("key", key);
cmd.Parameters.AddWithValue("value", json);
await cmd.ExecuteNonQueryAsync(ct);
}

/// <summary>
/// Every row in <c>station.settings</c>, keyed by <c>key</c>, value verbatim as the stored JSONB
/// text. Lets any failure (including a <see cref="Npgsql.NpgsqlException"/>) propagate to the
/// caller — the degrade-to-empty-on-DB-down posture is a caller policy
/// (<c>StationSettingsStore.ReadAllAsync</c>), not something this repository decides on its own.
/// </summary>
public async Task<IReadOnlyDictionary<string, string>> ReadAllAsync(CancellationToken ct)
{
var result = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);

await using var conn = new NpgsqlConnection(connectionString);
await conn.OpenAsync(ct);

await using var cmd = conn.CreateCommand();
cmd.CommandText = "SELECT key, value FROM station.settings";

await using var reader = await cmd.ExecuteReaderAsync(ct);
while (await reader.ReadAsync(ct))
result[reader.GetString(0)] = reader.GetString(1);

return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,6 @@ internal static class ExemptionBaseline
"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."),
new ArchitectureExemption(
LawId.L2,
"GenWave.Host.Configuration.StationSettingsStore",
"2026-08-07",
"Reads/writes station.settings directly via NpgsqlConnection (the write side of the " +
"settings overlay). Pre-existing (STORY-042); not trivial to fix in this diff — follow-up gh-#406."),
new ArchitectureExemption(
LawId.L2,
"GenWave.Host.Seeding.SafeLoopSeedMarkerStore",
Expand Down
14 changes: 14 additions & 0 deletions tests/GenWave.MediaLibrary.Tests/DatabaseFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,20 @@ public async Task ResetSpecialsAsync()
await cmd.ExecuteNonQueryAsync();
}

/// <summary>
/// Truncate <c>station.settings</c> (gh-#406 slice 3, STORY-042's original table). No identity
/// to restart — <c>key</c> is a bare <c>text</c> primary key, no <c>serial</c>/sequence backs
/// it — and no FK references this table, the same "no CASCADE required" reasoning
/// <see cref="ResetRequestAsync"/>'s own remarks give.
/// </summary>
public async Task ResetSettingsAsync()
{
await using var conn = await StationDataSource.OpenConnectionAsync();
await using var cmd = conn.CreateCommand();
cmd.CommandText = "truncate table station.settings";
await cmd.ExecuteNonQueryAsync();
}

async Task WaitForSchemaAsync()
{
for (var attempt = 0; attempt < 30; attempt++)
Expand Down
Loading
Loading