refactor(site-runtime): excise vestigial site-side notification-list surface — repo, DI registration, dead write paths (arch-review 08 §1.3/#23)
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -424,7 +424,8 @@ public class SiteCompositionRootTests : IDisposable
|
||||
public static IEnumerable<object[]> SiteScopedServices => new[]
|
||||
{
|
||||
new object[] { typeof(IExternalSystemRepository) },
|
||||
new object[] { typeof(INotificationRepository) },
|
||||
// INotificationRepository is intentionally absent — notification config is
|
||||
// central-only (arch-review 08 §1.3/#23); the site-side variant was removed.
|
||||
new object[] { typeof(ExternalSystemClient) },
|
||||
new object[] { typeof(IExternalSystemClient) },
|
||||
new object[] { typeof(DatabaseGateway) },
|
||||
@@ -442,11 +443,15 @@ public class SiteCompositionRootTests : IDisposable
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Site_NotificationRepository_IsSiteImplementation()
|
||||
public void Site_NotificationRepository_IsNotRegistered()
|
||||
{
|
||||
// Notification delivery is central-only (CLAUDE.md "Notification delivery is
|
||||
// central-only"); the site-local notification_lists/smtp_configurations tables
|
||||
// are purged on every artifact apply, so a site-side INotificationRepository
|
||||
// could only ever serve empty results. It must not be registered at all.
|
||||
using var scope = _host.Services.CreateScope();
|
||||
var repo = scope.ServiceProvider.GetRequiredService<INotificationRepository>();
|
||||
Assert.IsType<SiteNotificationRepository>(repo);
|
||||
var repo = scope.ServiceProvider.GetService<INotificationRepository>();
|
||||
Assert.Null(repo);
|
||||
}
|
||||
|
||||
// --- Options ---
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Persistence;
|
||||
using ZB.MOM.WW.ScadaBridge.SiteRuntime.Repositories;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests.Persistence;
|
||||
|
||||
@@ -123,46 +122,67 @@ public class ArtifactStorageTests : IAsyncLifetime, IDisposable
|
||||
// Upsert should not throw
|
||||
}
|
||||
|
||||
// ── Notification List Storage ──
|
||||
|
||||
[Fact]
|
||||
public async Task StoreNotificationList_DoesNotThrow()
|
||||
{
|
||||
await _storage.StoreNotificationListAsync(
|
||||
"Ops Team", ["ops@example.com", "admin@example.com"]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task StoreNotificationList_Upserts()
|
||||
{
|
||||
await _storage.StoreNotificationListAsync("Team1", ["a@b.com"]);
|
||||
await _storage.StoreNotificationListAsync("Team1", ["x@y.com", "z@w.com"]);
|
||||
|
||||
// Upsert should not throw
|
||||
}
|
||||
|
||||
// ── DeploymentManager-025 / SiteRuntime-031: central-only notif/SMTP purge ──
|
||||
//
|
||||
// Notification config is central-only. The site-side write paths and
|
||||
// SiteNotificationRepository were removed 2026-07-10 (arch-review 08 §1.3/#23);
|
||||
// PurgeCentralOnlyNotificationConfigAsync is retained as the security cleanup for
|
||||
// DBs written by older builds. These tests seed the (still-present) tables via raw
|
||||
// SQL — the only way rows can now exist — and assert the purge empties them.
|
||||
|
||||
private async Task SeedNotificationRowAsync(string name, string emailsJson)
|
||||
{
|
||||
await using var connection = new Microsoft.Data.Sqlite.SqliteConnection($"Data Source={_dbFile}");
|
||||
await connection.OpenAsync();
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText =
|
||||
"INSERT INTO notification_lists (name, recipient_emails, updated_at) VALUES (@n, @e, @u)";
|
||||
command.Parameters.AddWithValue("@n", name);
|
||||
command.Parameters.AddWithValue("@e", emailsJson);
|
||||
command.Parameters.AddWithValue("@u", DateTimeOffset.UtcNow.ToString("O"));
|
||||
await command.ExecuteNonQueryAsync();
|
||||
}
|
||||
|
||||
private async Task SeedSmtpRowAsync(string name, string password)
|
||||
{
|
||||
await using var connection = new Microsoft.Data.Sqlite.SqliteConnection($"Data Source={_dbFile}");
|
||||
await connection.OpenAsync();
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText =
|
||||
@"INSERT INTO smtp_configurations (name, server, port, auth_mode, from_address, username, password, oauth_config, updated_at)
|
||||
VALUES (@n, 'smtp.example.com', 587, 'BasicAuth', 'noreply@example.com', 'smtpuser', @p, NULL, @u)";
|
||||
command.Parameters.AddWithValue("@n", name);
|
||||
command.Parameters.AddWithValue("@p", password);
|
||||
command.Parameters.AddWithValue("@u", DateTimeOffset.UtcNow.ToString("O"));
|
||||
await command.ExecuteNonQueryAsync();
|
||||
}
|
||||
|
||||
private async Task<long> RowCountAsync(string table)
|
||||
{
|
||||
await using var connection = new Microsoft.Data.Sqlite.SqliteConnection($"Data Source={_dbFile}");
|
||||
await connection.OpenAsync();
|
||||
await using var command = connection.CreateCommand();
|
||||
command.CommandText = $"SELECT COUNT(*) FROM {table}";
|
||||
return (long)(await command.ExecuteScalarAsync())!;
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PurgeCentralOnlyNotificationConfig_RemovesPersistedNotificationListsAndSmtpRows()
|
||||
{
|
||||
// Simulate a pre-fix build that already shipped a notification list and an
|
||||
// SMTP config (with a plaintext password) to the site.
|
||||
await _storage.StoreNotificationListAsync("Ops Team", ["ops@example.com"]);
|
||||
await _storage.StoreSmtpConfigurationAsync(
|
||||
"smtp.example.com:587", "smtp.example.com", 587, "BasicAuth",
|
||||
"noreply@example.com", "smtpuser", "PLAINTEXT-SECRET", null);
|
||||
await SeedNotificationRowAsync("Ops Team", "[\"ops@example.com\"]");
|
||||
await SeedSmtpRowAsync("smtp.example.com:587", "PLAINTEXT-SECRET");
|
||||
|
||||
var repo = new SiteNotificationRepository(_storage);
|
||||
Assert.NotEmpty(await repo.GetAllNotificationListsAsync());
|
||||
Assert.NotEmpty(await repo.GetAllSmtpConfigurationsAsync());
|
||||
Assert.Equal(1, await RowCountAsync("notification_lists"));
|
||||
Assert.Equal(1, await RowCountAsync("smtp_configurations"));
|
||||
|
||||
// The fix: every artifact apply/deploy purges these central-only rows.
|
||||
await _storage.PurgeCentralOnlyNotificationConfigAsync();
|
||||
|
||||
// Both tables are now empty — the plaintext SMTP credential is gone.
|
||||
Assert.Empty(await repo.GetAllNotificationListsAsync());
|
||||
Assert.Empty(await repo.GetAllSmtpConfigurationsAsync());
|
||||
Assert.Equal(0, await RowCountAsync("notification_lists"));
|
||||
Assert.Equal(0, await RowCountAsync("smtp_configurations"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -172,9 +192,8 @@ public class ArtifactStorageTests : IAsyncLifetime, IDisposable
|
||||
await _storage.PurgeCentralOnlyNotificationConfigAsync();
|
||||
await _storage.PurgeCentralOnlyNotificationConfigAsync();
|
||||
|
||||
var repo = new SiteNotificationRepository(_storage);
|
||||
Assert.Empty(await repo.GetAllNotificationListsAsync());
|
||||
Assert.Empty(await repo.GetAllSmtpConfigurationsAsync());
|
||||
Assert.Equal(0, await RowCountAsync("notification_lists"));
|
||||
Assert.Equal(0, await RowCountAsync("smtp_configurations"));
|
||||
}
|
||||
|
||||
// ── Schema includes all WP-33 tables ──
|
||||
@@ -186,7 +205,9 @@ public class ArtifactStorageTests : IAsyncLifetime, IDisposable
|
||||
await _storage.StoreSharedScriptAsync("s", "code", null, null);
|
||||
await _storage.StoreExternalSystemAsync("e", "url", "None", null, null);
|
||||
await _storage.StoreDatabaseConnectionAsync("d", "connstr", 1, TimeSpan.Zero);
|
||||
await _storage.StoreNotificationListAsync("n", ["email@test.com"]);
|
||||
|
||||
// notification_lists / smtp_configurations remain in the schema (kept for the
|
||||
// security purge) — their presence is exercised by the purge tests above.
|
||||
|
||||
// All succeeded without exceptions = tables exist
|
||||
}
|
||||
|
||||
@@ -199,25 +199,8 @@ public class SiteRepositoryTests : IDisposable
|
||||
Assert.Null(await repo.GetDatabaseConnectionByNameAsync("DoesNotExist"));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// SiteRuntime-007: the same stability guarantee for notification lists.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task NotificationRepository_SyntheticId_IsStableAcrossRestart()
|
||||
{
|
||||
var storage1 = NewStorage();
|
||||
await storage1.InitializeAsync();
|
||||
await storage1.StoreNotificationListAsync(
|
||||
"OnCall", new[] { "a@example.com", "b@example.com" });
|
||||
|
||||
var repo1 = new SiteNotificationRepository(storage1);
|
||||
var idBeforeRestart = (await repo1.GetAllNotificationListsAsync())[0].Id;
|
||||
|
||||
var storage2 = NewStorage();
|
||||
var repo2 = new SiteNotificationRepository(storage2);
|
||||
var found = await repo2.GetNotificationListByIdAsync(idBeforeRestart);
|
||||
|
||||
Assert.NotNull(found);
|
||||
Assert.Equal("OnCall", found.Name);
|
||||
}
|
||||
// NotificationRepository_SyntheticId_IsStableAcrossRestart was removed 2026-07-10
|
||||
// (arch-review 08 §1.3/#23) along with the vestigial SiteNotificationRepository —
|
||||
// notification config is central-only and never lives on a site. The synthetic-ID
|
||||
// stability guarantee is still exercised by ExternalSystemRepository_SyntheticId_IsStableAcrossRestart.
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user