Adds 2 units and docs for unit 3. nfr-requirements plan

This commit is contained in:
2026-06-29 22:18:37 +02:00
parent 0e01ca1e1c
commit c156107cb1
126 changed files with 15204 additions and 80199 deletions
@@ -0,0 +1,77 @@
# NFR Requirements — Unit 1: master-backend
## Performance
| Requirement | Specification | Source |
|-------------|--------------|--------|
| HTTP timeout for slave calls | Configurable via `MasterModuleOptions.HttpTimeoutSeconds`; default **10 seconds** | Q1 |
| HTTP retry budget | Maximum 3 attempts (initial + 2 retries) with exponential backoff (1s, 2s); per-attempt timeout applies | Q2 |
| Background service interval | Configurable via `MasterModuleOptions.IntegrityCheckIntervalMinutes`; default 60 minutes | NFR-MASTER-04 |
| Controller endpoint latency | No explicit SLA; bounded by HTTP timeout × retry attempts (worst case ~33s for a single unresponsive slave during Add/UpdateStatus) | derived |
---
## Security
| Requirement | Specification | Source |
|-------------|--------------|--------|
| ApiKey at-rest encryption | Encrypted with ASP.NET Core Data Protection before writing to DB; decrypted immediately before HTTP calls | Q3 (+ functional design) |
| Data Protection key storage | **Default file system** (platform default); machine-bound; acceptable for single-instance deployment | Q3 |
| ApiKey exposure | Never included in `CmsInstanceDto` or any API response; `[JsonIgnore]` or explicit DTO mapping | NFR-MASTER-03 |
| Endpoint authorization | All `CmsInstanceController` actions require `[Authorize(Policy = "OwnerOnly")]` | FR-MASTER-10 |
| Internal slave endpoint auth | `POST /api/internal/master/register` validated via `X-Master-Api-Key` header (Unit 2 concern) | FR-MASTER-03 |
---
## Reliability
| Requirement | Specification | Source |
|-------------|--------------|--------|
| Fail-open on slave unreachability | Status push failure returns `SlaveContactSuccess = false` but does not roll back DB change; integrity check sets `LastIntegrityCheckFailedAt` and continues | NFR-MASTER-01 |
| Retry policy | Exponential backoff: attempt 1 (immediate), attempt 2 (+1s delay), attempt 3 (+2s delay); implemented via Polly `ResiliencePipeline` | Q2 |
| Background service isolation | Exceptions per slave instance are caught, logged, and do not abort the full integrity check batch | BR-BG-03 |
| Background service scope | `IServiceScopeFactory` used per tick to resolve scoped `ICmsInstanceService`; scope disposed after each tick | BR-BG-01/02 |
---
## Testability
| Requirement | Specification | Source |
|-------------|--------------|--------|
| Minimum test coverage | ≥ 80% line/branch coverage for `SlpModularCms.Modules.Master` (excluding items below) | NFR-MASTER-05 |
| Coverage exclusions | Apply `[ExcludeFromCodeCoverage]` to: `MasterModule.cs` (IModule boilerplate), EF Core migration files, plain DTO/record classes with no logic | Q4 |
| Test project | `SlpModularCms.Modules.Master.Tests` — separate project; mirrors production project structure | Unit decomposition decision |
| Key test targets | `CmsInstanceService`, `SlaveApiClient`, `IntegrityCheckBackgroundService`, `CmsInstanceController` | NFR-MASTER-05 |
| Interface-driven design | `ICmsInstanceRepository`, `ICmsInstanceService`, `ISlaveApiClient` interfaces required to enable unit test mocking | derived |
---
## Maintainability
| Requirement | Specification | Source |
|-------------|--------------|--------|
| Log level — integrity check failures | **Warning** — slave unreachability during background checks is expected; does not require immediate attention | Q5 |
| Log level — status push failures | **Error** — owner-triggered action failed to reach slave; requires visibility | Q5 |
| Log level — re-registration on mismatch | **Information** — expected recovery action | derived |
| Log level — background service tick | **Debug** — high frequency; only visible when debugging | derived |
| Structured logging | Use `ILogger<T>` with structured message templates; include `slaveUrl` and `instanceId` in log scope | derived |
---
## Updated `MasterModuleOptions` Fields
The following field is added as a result of Q1:
| Property | Type | Default | Notes |
|----------|------|---------|-------|
| `HttpTimeoutSeconds` | `int` | `10` | Timeout applied to each individual HTTP attempt in `SlaveApiClient` |
Full updated options shape:
| Property | Type | Default | Side |
|----------|------|---------|------|
| `IntegrityCheckIntervalMinutes` | `int` | `60` | Master |
| `HttpTimeoutSeconds` | `int` | `10` | Master |
| `MasterUrl` | `string?` | `null` | Master |
| `CacheMinutes` | `int` | `60` | Slave |
| `ApiKey` | `string?` | `null` | Slave |
@@ -0,0 +1,112 @@
# Tech Stack Decisions — Unit 1: master-backend
## HTTP Client & Resilience
### Decision: Typed HTTP Client via `IHttpClientFactory` + Polly
| Aspect | Decision | Rationale |
|--------|----------|-----------|
| HTTP client abstraction | `ISlaveApiClient` / `SlaveApiClient` typed client | Testable via mock injection; clean contract boundary |
| Client registration | `services.AddHttpClient<ISlaveApiClient, SlaveApiClient>()` | Framework manages `HttpClient` lifetime and connection pooling |
| Retry policy | **Polly** `ResiliencePipelineBuilder` with `AddRetry` | Industry standard .NET resilience library; integrates natively with `IHttpClientFactory` via `AddResilienceHandler` |
| Retry configuration | 3 total attempts; delays: 1s → 2s (exponential); jitter optional | Bounded worst-case latency; exponential reduces thundering herd on widespread slave outages |
| Per-attempt timeout | `MasterModuleOptions.HttpTimeoutSeconds` (default 10s) | Configurable per-environment; keeps controller responses bounded |
**NuGet package required**: `Microsoft.Extensions.Http.Resilience` (includes Polly integration)
**Registration pattern**:
```csharp
services.AddHttpClient<ISlaveApiClient, SlaveApiClient>()
.AddResilienceHandler("slave-retry", builder =>
{
builder.AddRetry(new HttpRetryStrategyOptions
{
MaxRetryAttempts = 2,
Delay = TimeSpan.FromSeconds(1),
BackoffType = DelayBackoffType.Exponential,
UseJitter = true,
});
builder.AddTimeout(TimeSpan.FromSeconds(options.HttpTimeoutSeconds));
});
```
---
## Data Protection
### Decision: Default ASP.NET Core Data Protection (file system)
| Aspect | Decision | Rationale |
|--------|----------|-----------|
| Key storage | Default file system (no explicit `PersistKeysTo*` call) | Zero configuration; acceptable for single-instance; owner manages production key persistence |
| Purpose string | `"SlpModularCms.Master.ApiKey"` | Scoped protection; prevents cross-purpose decryption |
| Registration | `services.AddDataProtection()` (already called by framework if not explicitly called) | No extra setup needed beyond injecting `IDataProtectionProvider` |
**Production note** (to be included in Unit 4 README update): For containerized or multi-instance deployments, configure a persistent key ring (e.g., `PersistKeysToDbContext`, `PersistKeysToAzureBlobStorage`). Without it, restarting the container causes all encrypted `ApiKey` values to become unreadable.
---
## Background Service
### Decision: .NET `BackgroundService` + `PeriodicTimer`
| Aspect | Decision | Rationale |
|--------|----------|-----------|
| Base class | `BackgroundService` | Built-in .NET hosted service; lifecycle managed by `IHostApplicationLifetime` |
| Timer mechanism | `PeriodicTimer` | Allocates less than `Timer`; await-friendly; cancellation-aware |
| Scope management | `IServiceScopeFactory.CreateScope()` per tick | Required because `ICmsInstanceService` is Scoped; prevents captive dependency |
| Exception handling | `try/catch` around entire tick body; log `Error` and continue | Prevents background service crash on unexpected errors |
---
## Logging
### Decision: `ILogger<T>` structured logging
| Scenario | Log Level | Structured Fields |
|----------|-----------|------------------|
| Status push failed (slave unreachable) | `Error` | `instanceId`, `slaveUrl`, `exception` |
| Integrity check: slave unreachable | `Warning` | `instanceId`, `slaveUrl` |
| Integrity check: URL mismatch detected | `Warning` | `instanceId`, `slaveUrl`, `expectedUrl`, `registeredUrl` |
| Integrity check: re-registration succeeded | `Information` | `instanceId`, `slaveUrl` |
| Integrity check: re-registration failed | `Warning` | `instanceId`, `slaveUrl` |
| Background service tick started | `Debug` | `instanceCount` |
| ApiKey decryption failed | `Error` | `instanceId` (do NOT log the key itself) |
---
## EF Core / Database
### Decision: Per-module `MasterDbContext` with own migrations
| Aspect | Decision |
|--------|----------|
| DbContext class | `MasterDbContext : DbContext` in `SlpModularCms.Modules.Master` |
| Migration assembly | `SlpModularCms.Modules.Master` (same project) |
| Migration application | `app.ApplicationServices.CreateScope()``MasterDbContext.Database.MigrateAsync()` in `MasterModule.UseModule(IApplicationBuilder)` |
| Tables owned | `CmsInstances`, `DataProtectionKeys` (if needed in future) |
| Connection string | Reuses the same connection string as `ApplicationDbContext` (from `ConnectionStrings:DefaultConnection`) |
---
## Test Framework
### Decision: xUnit + Moq (matching existing test projects)
| Aspect | Decision | Rationale |
|--------|----------|-----------|
| Test framework | xUnit | Matches existing `Availability.Tests` project |
| Mocking | Moq | Matches existing test projects |
| Coverage tool | coverlet (via `.runsettings` or `dotnet test --collect`) | Already in use in existing test projects |
| `[ExcludeFromCodeCoverage]` targets | `MasterModule`, EF Core migration files, DTO records | Q4 decision |
| HTTP testing | Mock `ISlaveApiClient` via Moq | Typed client interface enables clean mocking without `HttpMessageHandler` fakes |
---
## Summary of New Dependencies
| Package | Version | Purpose |
|---------|---------|---------|
| `Microsoft.Extensions.Http.Resilience` | Latest stable | Polly integration for `IHttpClientFactory` retry policies |
All other dependencies (EF Core, ASP.NET Core, xUnit, Moq) are already present in the solution.