diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a378aaf..e1fa294 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,13 +76,17 @@ jobs: name: unit-test-results path: ./test-results - - name: Coverage gate + # Cobertura is in the list so the threshold step below has a single merged report to + # read. The unit tests run on both TFMs, so the artifact holds two coverage files + # describing the same source: summing them would double-count, and taking either one + # alone misses what only the other compiles. ReportGenerator's merge is the honest number + # (81.55% line, against 81.95% for net10.0 by itself). + - name: Generate coverage report uses: danielpalme/ReportGenerator-GitHub-Action@5 with: reports: './test-results/**/coverage.cobertura.xml' targetdir: './coverage-html' - reporttypes: 'HtmlInline;Badges;MarkdownSummaryGithub' - minimumCoverage: ${{ env.COVERAGE_THRESHOLD }} + reporttypes: 'HtmlInline;Badges;MarkdownSummaryGithub;Cobertura' tag: ${{ github.run_number }} - name: Publish coverage summary @@ -90,9 +94,67 @@ jobs: run: | [ -f "./coverage-html/SummaryGithub.md" ] && cat ./coverage-html/SummaryGithub.md >> $GITHUB_STEP_SUMMARY || true + # Uploaded before the gate runs, and on failure too: a build that fails on coverage is + # exactly when someone wants the line-by-line report. - uses: actions/upload-artifact@v4 + if: always() with: { name: coverage-html, path: ./coverage-html/ } + # The actual gate. + # + # This job used to pass `minimumCoverage` to the action above, which has no such input — + # GitHub logged "Unexpected input(s) 'minimumCoverage'" and ignored it, so the job + # generated a report and exited 0 whatever the coverage was. A required check named + # "Coverage Gate" that cannot fail is worse than no check: the ruleset demands it and + # everyone reads the green tick as proof. + # + # Line coverage only. Branch coverage is several points lower (75.73% merged) and gating + # on it today would fail the build over code nobody is touching; it is printed here so the + # number stays visible, and raising it is a deliberate follow-up rather than an ambush. + - name: Enforce coverage threshold + run: | + set -euo pipefail + + REPORT=./coverage-html/Cobertura.xml + if [[ ! -f "${REPORT}" ]]; then + echo "::error::No merged Cobertura report at ${REPORT} — coverage cannot be verified" + exit 1 + fi + + python3 - "${REPORT}" "${COVERAGE_THRESHOLD}" <<'PY' + import os + import sys + import xml.etree.ElementTree as ET + + report, threshold = sys.argv[1], float(sys.argv[2]) + root = ET.parse(report).getroot() + + # Abort on a missing or malformed rate rather than treating it as 0 or as "fine". Passing + # silently is the precise failure this step exists to replace, so it must not be reachable. + try: + line = float(root.get("line-rate")) * 100 + branch = float(root.get("branch-rate")) * 100 + except (TypeError, ValueError) as exc: + print(f"::error::Cannot read line-rate/branch-rate from {report}: {exc}") + sys.exit(1) + + print(f"Line coverage: {line:.2f}% (threshold {threshold:.0f}%)") + print(f"Branch coverage: {branch:.2f}% (reported, not gated)") + + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a", encoding="utf-8") as handle: + handle.write("\n| Metric | Coverage | Threshold |\n|---|---|---|\n") + handle.write(f"| Line | {line:.2f}% | {threshold:.0f}% |\n") + handle.write(f"| Branch | {branch:.2f}% | not gated |\n") + + if line < threshold: + print(f"::error::Line coverage {line:.2f}% is below the {threshold:.0f}% threshold") + sys.exit(1) + + print(f"Line coverage {line:.2f}% meets the {threshold:.0f}% threshold") + PY + # ─────────────────────────────────────────────────────────────────────── provider-tests: name: Provider Tests diff --git a/CHANGELOG.md b/CHANGELOG.md index 785ae48..afe530b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,243 @@ This project follows [Semantic Versioning](https://semver.org/). _Nothing yet._ +## [1.0.0-preview.2] — 2026-10-10 + +The second prerelease. **One breaking change from `1.0.0-preview.1`:** the built-in audit trail +is gone, replaced by the `ISettingWriteObserver` hook. Its entry below carries the migration +step. + +Two deprecations rather than breaks: `ISettingDbContext` is obsolete and does nothing, and +`ApplySettingsConfiguration` is renamed `ApplySettings`. Both still compile, with a warning, and +go in the next release. Still not frozen: pin an exact version rather than a floating range. + +### Removed +- **The built-in audit trail, as BREAKING in prerelease.** Gone: `ISettingAuditStore`, + `SettingAuditEntry`, `EfCoreAuditStore`, `.UseAuditStore()`, + `GET /api/settings/{route}/audit`, and the `SettingAudits` table in both the EF Core model and + the Dapper schema bootstrap. + It could not be frozen into 1.0. `ChangedBy` was the literal string `"system"` on every entry + and never anything else, though the column was required in all four schemas — it carried no + information. `ClearAsync` deleted every row in a group and recorded nothing, so the most + destructive operation in the library was the only one with no trail. Entries were written one + round trip at a time *after* the write had already committed, each with its own + `SaveChangesAsync`, outside the write's transaction — and the loop had no `try`/`catch`, so an + audit store failure threw on a write that had succeeded, thirty lines away from change handlers + whose exceptions are deliberately swallowed. `GetHistoryAsync` was unbounded and the endpoint + offered no paging, over a table that only grows. And it existed only for EF Core: the Dapper + bootstrap created a `SettingAudits` table that nothing ever wrote to, so a Dapper user got a + table, concluded auditing worked, and got silence. + Auditing returns as a general `DotNetBoost.Auditing` package over the new write hook — port in + Core, implementation in a satellite package, the way `ISettingValidator` and + `DotNetBoost.Settings.FluentValidation` already work. + **Migration:** implement `ISettingWriteObserver`, or call `.UseLoggingWriteObserver()` in the + meantime. Nothing drops an existing `SettingAudits` table for you; your rows stay where they + are. + +### Added +- **`ISettingWriteObserver`** — called once after every completed write to any settings group, + with `SettingWrite`: the group name and route, the kind of write, one `SettingChange` per + property that actually changed, the actor, a correlation id and a timestamp. Register with + `.UseWriteObserver()`; several may be registered and all run. This is the hook an + auditing package, a webhook publisher or a search indexer implements. + One notification per write rather than one per property, so the several changes of a single + save share a correlation id and are identifiable as one edit. **`ClearAsync` now reports too**, + as `SettingWriteKind.Cleared`. An observer's exception is logged and swallowed, the same policy + as `ISettingChangedHandler` — a write that has already committed must not fail because a + downstream observer did — and each observer is wrapped on its own so one failure does not skip + the rest. + A `[Sensitive]` property arrives with `IsRedacted` set and both values `null`. The library + decides what is sensitive and an observer only learns that it changed; redaction follows the + attribute rather than whether the value happened to be encrypted at rest, so it holds with no + encryptor configured. +- **`GET /api/settings` and `GET /api/settings/{route}/schema`** — nothing enumerated the + `[SettingGroup]` classes and nothing described a group's properties, so a "form generated from + the class" was really generated by guessing at the shape of a JSON response, with the bounds + hardcoded in the client and enforced nowhere on the server. + The list gives each group's route, persistence key and whether it carries `[Authorize]`. The + schema gives each property's name (spelled as the `GET` body spells it), type, default and + whether it is `[Sensitive]`. **Neither returns a stored value**, and the `default` is withheld + for a `[Sensitive]` property — a compile-time default on a secret is a value like any other, + and these endpoints are meant to be safe to expose more widely than values are. The schema + sits inside its group, so it is covered by the group's `[Authorize]` exactly as `GET` and + `POST` are. Both are on by default; pass `includeDiscovery: false` to `MapSettingsEndpoints` + to leave them out. +- **`ISettingProjector`** — a projector that names the type it returns, so the + generated `GET` advertises the projection instead of the stored group. The untyped interface + returns `object`, and the library could not do better on its own: a projector is resolved from + the request scope, long after the route was mapped, so `.Produces(200, …)` had no choice but + to name the group. The docs admitted it in prose and told you to patch the document yourself. + The type sits on the projector rather than as a third argument to `UseProjector`, so the + compiler checks it — a third generic argument would be a promise the author could get wrong, + and catching that would need a runtime check. + Opt-in and not a break: an existing `ISettingProjector` keeps compiling, keeps returning + the same body, and keeps declaring the group. The typed interface adapts to the + object-returning member through a default interface member, so implementing it is all there + is to do. **This documents the response; it does not change it** — the body is byte for byte + what the untyped interface produced, and `POST` still accepts the stored group, because + advertising the projection as a request body would promise a write the API refuses. + Prerelease-cheap, and no change to the net8.0/net10.0 support matrix. +- **`.UseSchemaContributor()`**, with a factory overload — `ISettingSchemaContributor` + was the only port on `SettingBuilder` without a builder method, which nobody noticed because + `DotNetBoost.Settings.FluentValidation` registers its own internally. A consumer publishing a + constraint of its own had to drop out of the `AddSettings()` chain and call + `AddSingleton()` by hand. + It **composes rather than replaces**: registering one adds to whatever `UseFluentValidation` + already publishes, which is only true as of the merge change in this same release. One type + registers once however often it is called — a contributor only describes, so a duplicate would + describe a group twice and read as a collision with itself — unlike `UseWriteObserver()`, + where two registrations are two observers. Registration order decides which value survives a + collision, and the XML doc says so. +- **`ISettingSchemaContributor`**, implemented over FluentValidation's own descriptor and + registered by the existing `UseFluentValidation(assembly)` call, so the schema publishes the + bounds your validators already enforce. `ISettingValidator` could not supply these: it answers + only whether a value is valid, never what would be. A rule the contributor cannot read — a + `Must(...)`, a comparison against another property — is skipped rather than guessed at, since + a wrong published bound is worse than a missing one. With no contributor registered the schema + simply reports no constraints. +- `ISettingProjector` now states its failure semantics: a projector that throws **fails the + request**, deliberately unlike `ISettingChangedHandler` and `ISettingWriteObserver`, whose + exceptions are logged and swallowed. A failed notification must not break a write that has + already committed; a failed projection would otherwise hide a wrong response. Behaviour + unchanged — it was simply never written down, and is now covered by a test. +- **`SettingSchema.Describe(Type)`** in Core, which is also now the single place the rule for + *which* properties the engine stores is written, shared with `SettingManager`. +- **`ISettingChangeSignal`, and `.SynchronizeThroughDistributedCache()`** — settings now + propagate between application instances. A write evicted the local `IMemoryCache` entry and + nothing else, so every other instance went on serving the old values for up to + `CacheDuration` (10 minutes by default): "change it, reload, done, no redeploy" silently + stopped being true at two replicas, in a repo that ships an Aspire AppHost. + Each write publishes a fresh token for that group, and a cache hit re-checks the token at + most once per `ChangeCheckInterval` — **5 seconds by default, and that interval is the + staleness bound, not zero**. What this buys is a bound of seconds rather than minutes, not + instant propagation. Publishing is wired up as an `ISettingWriteObserver`, so `ClearAsync` + propagates without a second call site. An unreachable signalling store is logged and treated + as unchanged — a signal is an optimisation over the cache duration, so a Redis outage costs a + slower reload, not an error. Register your own transport with `UseChangeSignal()`. + The default signals nothing, so a single-instance application needs no configuration and + behaves exactly as before. Note that this is cache coherence only: `ISettingChangedHandler` + and `ISettingWriteObserver` stay in-process by design. +- **`ISettingActorAccessor`**, and `.UseHttpContextActor()` in the API package, which reads + `ClaimTypes.NameIdentifier` then `Identity.Name` off the current request. `Actor` is nullable + and `null` means *not captured* — never a placeholder, which is what made the old `ChangedBy` + worthless. +- **`LoggingSettingWriteObserver`**, opt in with `.UseLoggingWriteObserver()`: one log line per + write naming the group, actor, correlation id and the **names** of the changed properties. + Never a value, redacted or not — a settings value is exactly the kind of thing that should not + reach a log aggregator because a default turned it on. +- **`ISettingProjector`** — a read-path hook for serving a group as something other than its + stored values: a display name in the request's language, an image id resolved to a URL, a + field masked for display. Register with `.UseProjector()`; the + generated `GET /api/settings/{route}` applies it. Three constraints are what make it safe to + have, and all three are covered by tests: it is **read path only**, so `POST` never consults + it and a projector cannot influence what is stored; it is **never applied to the `ETag`**, + which keeps describing the stored revision, so conditional writes go on protecting the stored + values; and with no projector registered the response is byte for byte what it was. Without + this hook, composing settings with anything else meant standing up a parallel endpoint and + leaving the generated one returning a value the UI never shows. +- `MapSettingsEndpoints` now tags its routes `Settings`, matching the sibling DotNetBoost + packages. The `app.MapGroup("").WithTags("Settings").MapSettingsEndpoints()` dance — an empty + prefix whose only purpose was to carry a tag — becomes `app.MapSettingsEndpoints()`. +- `ApplySettings(DbContext)`: an overload that reads the engine off the context, so + `OnModelCreating` is `modelBuilder.ApplySettings(this)` and switching provider needs no + matching edit. The explicit `ApplySettings(DatabaseProvider)` overload stays for overriding + it — a model built against one engine and migrated onto another. An unrecognised EF Core + provider throws naming the overload to use rather than guessing a column type, which would + surface much later as truncated settings. + +### Changed +- **`ApplySettingsConfiguration` is renamed `ApplySettings`.** Media exposes + `modelBuilder.ApplyMedia()` and MultiLingual exposes `modelBuilder.ApplyMultiLingual(this)`; + Settings was the only one of the three naming the mechanism rather than the library. In a + consuming `AppDbContext` the three calls sit on adjacent lines, so the odd shape read as if it + meant something, and it did not. Done now because `1.0.0-preview.2` is the last cheap moment. + `ApplySettingsConfiguration(DatabaseProvider)` shipped in `1.0.0-preview.1`, so **it remains as + an `[Obsolete]` forwarder**: a context written against it keeps compiling with a deprecation + warning rather than breaking, and the alias goes in the next release. The context-inferring + overload is listed under Added above, under its new name — it has never been released under + the old one. +- **Two projectors for one settings group is now a startup error.** `UseProjector` used + `AddScoped`, so a second registration for the same group was accepted and the last one + silently won — the other was never even constructed, and the response was quietly missing + half its shape. That is never what anyone means: projectors do not compose, because + `ProjectAsync` takes the *group*, so a second has nowhere to receive the first's output. A + group needing both translations and media wants one projector doing the join. + `Build()` now rejects it, beside the checks for duplicate group names and routes, naming the + group and both implementations. The same implementation type registered twice is collapsed + rather than reported — an idempotent mistake, not an ambiguity. **Migration:** an app that + relied on the implicit last-wins to override a projector set by a shared + `AddMyCompanySettings()` should call the new **`.ReplaceProjector()`**, + which the error message names. Registrations added *after* `Build()` are not seen and still + win by last-wins, which is what keeps the test-override seam open. Prerelease-cheap: nothing + that was working correctly changes. +- **The schema endpoint merges every applicable `ISettingSchemaContributor` instead of using the + first.** Constraints are additive facts about a property rather than a decision, so two + contributors describing different aspects of one group is the normal case — and first-match + meant registering a second one silently deleted the first's constraints, with the survivor + decided by registration order, which nothing documented or controlled. (The + `ISettingValidator` precedent the old behaviour cited does not transfer: first-match is right + for a validator, because running two over one property risks rejecting or reporting a value + twice.) + This is what lets a consumer publish a constraint this library cannot know about — a rule + carried by an attribute belonging to another package — *alongside* the validation rules, + rather than instead of them. Merging is two levels deep, property then constraint name. On a + collision the first registered wins and the clash is logged at `Warning`, naming both + contributors, the group, the property and the constraint; silently taking either would make + the schema depend on registration order. A contributor that throws still costs only its own + constraints, now also meaning it cannot cost the others theirs. + **Not breaking for anyone registering a single contributor**, which is every current caller — + including `UseFluentValidation`, which registers exactly one. +- **Nothing is registered against `DbContext` any more.** `UseEntityFrameworkCore` had + been registering `TContext` as the scoped `DbContext` so `EfCoreAuditStore` could resolve; with + the audit store gone nothing needs it, and the settings store is constructed from `TContext` + explicitly. A library should not claim a framework type it does not own — in an application + with two contexts it silently handed a bare `DbContext` dependency whichever one Settings was + pointed at. +- **The EF Core store takes a plain `DbContext`.** `EfCoreSettingStore` reaches its entities + through `Set()`, and `UseEntityFrameworkCore` no longer constrains + `TContext` beyond `DbContext`. A consuming context's whole settings registration is now one + `ApplySettings` call in `OnModelCreating` — no interface, no `DbSet` properties + the library never read. `ISettingDbContext` is **obsolete** and does nothing; delete + `: ISettingDbContext` and the two `DbSet` properties from your context and nothing else + changes. It will be removed in the next release. + +### Fixed +- **A write silently replaced an undecryptable secret with its default.** `SetAsync` mapped the + stored rows inside a bare `catch { previous = new T(); }`, which swallowed the + `SettingDecryptionException` that `MapToModel` raises on purpose and compared against + compile-time defaults instead. Every `[Sensitive]` property then read as changed, so a write to + one unrelated property rewrote them all under the current key, told `ISettingWriteObserver` + they had changed, and discarded the stranded values — the exact "application running on default + credentials" outcome the throw was added to prevent, reachable through the write path and + bypassing the `IgnoreDecryptionFailures()` opt-out. Nothing was logged, so the only symptom was + a secret quietly becoming its default. + A decryption failure now propagates from a write as it already did from a read, and no row is + written. Other mapping faults still fall back to defaults so a write cannot be blocked by one, + but are logged rather than swallowed. `IgnoreDecryptionFailures()` is unaffected: opting out + still falls back and still writes. +- **`SetAsync(selector, value)` could revert another writer's change to a different property.** + It rebuilt the whole model *from the cache* and put it through the group write, which compares + every property against fresh store rows and writes whatever differs. A property another + instance had changed while this one's cache was warm therefore read back stale, counted as a + difference, and was written back — so a call that only meant to touch property A undid + someone's edit to property B, over a window as wide as `CacheDuration` (default 10 minutes) + rather than a round trip. It now builds and writes a single row for the named property; + nothing else is part of the write. Validators still see the whole group, built from a fresh + read, since a validator takes a model rather than a property. The XML doc understated this + twice — it said "reads the current group" when the read was cached, and framed the damage as + landing on the property being written — and has been corrected. +- **`GetAsync` no longer hands every caller the same mutable object.** The cache held the mapped + settings instance, so assigning to a property of what you read wrote into the entry every + later reader of that group was served from — including `GET /api/settings/{group}`, which then + returned the mutated value as though it were stored. The cache now holds the stored rows and + each read materialises its own model; the round trip to the store is what the cache was ever + worth, and the mapping is reflection over a handful of properties. `SetAsync(selector, value)` + was mutating the cached instance for the same reason, and is fixed by the same change. + Two consequences: a `[Sensitive]` value is decrypted per read rather than once per cache + entry, and a distributed `ISettingCache` is now handed `Setting[]` — so what lands in Redis is + the stored ciphertext rather than a decrypted model. A custom cache that serialises needs no + change beyond the new type. + ## [1.0.0-preview.1] — 2026-08-29 First public release, published to nuget.org as a prerelease. diff --git a/README.md b/README.md index be9071d..851860c 100644 --- a/README.md +++ b/README.md @@ -61,14 +61,14 @@ builder.Services.AddSettings() var app = builder.Build(); -// Create the database and the settings tables on first run. +// Create the database and the settings table on first run. // In a real app, use EF Core migrations instead. using (var scope = app.Services.CreateScope()) { await scope.ServiceProvider.GetRequiredService().Database.EnsureCreatedAsync(); } -app.MapSettingsEndpoints(); // adds GET/POST /api/settings/mail-server +app.MapSettingsEndpoints(); // GET/POST /api/settings/mail-server, plus the group list and schema app.MapGet("/", async (ISettingManager settings) => { @@ -86,15 +86,12 @@ public class MailSettings public int Port { get; set; } = 587; } -// Your EF Core context: add the two settings tables to it. -public class AppDbContext(DbContextOptions options) - : DbContext(options), ISettingDbContext +// Your EF Core context: add the settings table to it. One line, no interface +// and no DbSet properties — the engine is read off the context. +public class AppDbContext(DbContextOptions options) : DbContext(options) { - public DbSet Settings => Set(); - public DbSet SettingAudits => Set(); - protected override void OnModelCreating(ModelBuilder modelBuilder) - => modelBuilder.ApplySettingsConfiguration(DatabaseProvider.Sqlite); + => modelBuilder.ApplySettings(this); } ``` @@ -153,7 +150,9 @@ public class EmailService(ISettingManager settings) ``` Reads are cached (10 minutes by default), so reading settings on every request is cheap. A write -clears the cache on that server straight away. If you run several servers, see [Caching](https://github.com/dotnetboost/settings/blob/main/docs/caching.md). +clears the cache on the instance that made it. If you run more than one instance, add +`.SynchronizeThroughDistributedCache()` so the others pick the change up too — see +[Caching](https://github.com/dotnetboost/settings/blob/main/docs/caching.md#running-on-more-than-one-instance). --- @@ -168,7 +167,7 @@ already have: | Dapper / plain ADO.NET | `DotNetBoost.Settings.Dapper` | `.UseDapper(sp => new SqlConnection(cs), migrateSchema: true)` | | MongoDB | `DotNetBoost.Settings.MongoDb` | `.UseMongoDb("mongodb://localhost:27017", "my_app_db")` | -EF Core needs two extra `DbSet`s on your context. See the +EF Core needs one line in `OnModelCreating` — no interface and no `DbSet` properties. See the [storage providers guide](https://github.com/dotnetboost/settings/blob/main/docs/storage-providers.md) for the full setup of each provider. @@ -183,8 +182,8 @@ builder.Services.AddSettings() .UseEntityFrameworkCore() .UseAesEncryption(key) // encrypt [Sensitive] properties .UseFluentValidation(typeof(Program).Assembly) // reject invalid values - .UseAuditStore() // keep a change history (EF Core) .OnChanged() // react when a value changes + .UseWriteObserver() // record or forward every write .Build(); ``` @@ -192,13 +191,15 @@ builder.Services.AddSettings() |---|---| | Use SQL Server, PostgreSQL or MongoDB instead of SQLite | [Storage providers](https://github.com/dotnetboost/settings/blob/main/docs/storage-providers.md) | | Expose and secure the REST API | [REST API](https://github.com/dotnetboost/settings/blob/main/docs/rest-api.md) | +| Change what `GET` returns without changing what is stored | [Projectors](https://github.com/dotnetboost/settings/blob/main/docs/rest-api.md#reshaping-what-get-returns) | | Reject invalid values (`[Range]`, FluentValidation) | [Validation](https://github.com/dotnetboost/settings/blob/main/docs/validation.md) | | Encrypt passwords and API keys in the database | [Encryption](https://github.com/dotnetboost/settings/blob/main/docs/encryption.md) | | Rename a settings class safely, set defaults | [Defining settings](https://github.com/dotnetboost/settings/blob/main/docs/defining-settings.md) | -| Run on several servers (Redis cache) | [Caching](https://github.com/dotnetboost/settings/blob/main/docs/caching.md) | +| Run on several servers and keep their caches in step | [Caching](https://github.com/dotnetboost/settings/blob/main/docs/caching.md#running-on-more-than-one-instance) | | Run code when a setting changes | [Change notifications](https://github.com/dotnetboost/settings/blob/main/docs/change-notifications.md) | | Stop two people overwriting each other's edits | [Reading & writing](https://github.com/dotnetboost/settings/blob/main/docs/reading-and-writing.md#concurrent-writes) | -| See who changed what, and when | [Audit trail](https://github.com/dotnetboost/settings/blob/main/docs/audit-trail.md) | +| Record or forward every write (and who made it) | [Write notifications](https://github.com/dotnetboost/settings/blob/main/docs/write-notifications.md) | +| Generate a form from the class (list groups, read the schema) | [Discovery endpoints](https://github.com/dotnetboost/settings/blob/main/docs/discovery-endpoints.md) | | Give admins a web UI to edit settings | [Dashboard](https://github.com/dotnetboost/settings/blob/main/docs/dashboard.md) | | See every builder option | [Configuration reference](https://github.com/dotnetboost/settings/blob/main/docs/configuration-reference.md) | | Run the full demo stack (API, dashboard, PostgreSQL, Redis) | [.NET Aspire](https://github.com/dotnetboost/settings/blob/main/docs/aspire.md) | @@ -223,7 +224,7 @@ See [Running everything with .NET Aspire](https://github.com/dotnetboost/setting | Package | What it's for | |---|---| -| `DotNetBoost.Settings.Core` | Required. The engine: `ISettingManager`, caching, encryption, audit, notifications | +| `DotNetBoost.Settings.Core` | Required. The engine: `ISettingManager`, caching, encryption, notifications | | `DotNetBoost.Settings.EntityFrameworkCore` | Store settings through EF Core (SQL Server, PostgreSQL, SQLite) | | `DotNetBoost.Settings.Dapper` | Store settings through Dapper (SQL Server, PostgreSQL, SQLite) | | `DotNetBoost.Settings.MongoDb` | Store settings in MongoDB | diff --git a/docs/README.md b/docs/README.md index 986f44b..b742e0b 100644 --- a/docs/README.md +++ b/docs/README.md @@ -11,9 +11,10 @@ New here? Start with the [5-minute quick start](../README.md#get-started-in-5-mi - [Encrypting sensitive values](encryption.md): `[Sensitive]`, AES-256-GCM, key rotation - [Validation](validation.md): Data Annotations and FluentValidation - [Change notifications](change-notifications.md): run code when a setting changes -- [Audit trail](audit-trail.md): who changed what, and when +- [Write notifications](write-notifications.md): observe every write — what changed, by whom - [Caching](caching.md): cache duration, Redis for multi-server deployments - [REST API endpoints](rest-api.md): generated endpoints and **how to secure them** +- [Discovery endpoints](discovery-endpoints.md): list the groups, describe one's properties and constraints **Tooling** - [Dashboard (SPA client)](dashboard.md): a web UI for editing settings diff --git a/docs/architecture.md b/docs/architecture.md index 1ec0906..5669108 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -9,13 +9,15 @@ SettingManager ┌───────────┬────────┼────────┬─────────────┐ │ │ │ │ │ - ISettingCache ISettingStore ISettingEncryptor ISettingAuditStore - (IMemoryCache (EF Core / (AES-256-GCM (EfCoreAuditStore - or Redis) Dapper / or custom) or custom) + ISettingCache ISettingStore ISettingEncryptor ISettingValidator + (IMemoryCache (EF Core / (AES-256-GCM (DataAnnotations + or Redis) Dapper / or custom) or FluentValidation) MongoDB) │ - ISettingChangedHandler - (your app's runtime reactions) + ┌───────────┴────────────┐ + ISettingChangedHandler ISettingWriteObserver + (typed, one group, (untyped, every group: the + in-process reactions) property diff, actor, correlation id) ``` ## Repository layout diff --git a/docs/aspire.md b/docs/aspire.md index d3f45a5..eba85ed 100644 --- a/docs/aspire.md +++ b/docs/aspire.md @@ -31,7 +31,7 @@ What comes up: | Resource | What it is | |---|---| | `postgres` / `settingsdb` | PostgreSQL 18 container, named volume, persistent across runs | -| `pgweb` | Browser SQL client for inspecting `Settings` and `SettingAudits` | +| `pgweb` | Browser SQL client for inspecting the `Settings` table | | `cache` | Redis container backing `RedisSettingCache` | | `redisinsight` | Browser UI for watching the cache fill and expire | | `api` | `samples/SampleApp` — the REST endpoints and the Scalar reference at `/scalar` | @@ -61,7 +61,7 @@ written out in full and commented, so switching is four edits and no new code: The available blocks are PostgreSQL (EF Core, active), SQL Server (EF Core), SQLite (EF Core, no container), MongoDB, and Dapper on PostgreSQL. For the EF Core ones, also flip the -`DatabaseProvider` passed to `ApplySettingsConfiguration` in `samples/SampleApp/AppDbContext.cs` — +`DatabaseProvider` passed to `ApplySettings` in `samples/SampleApp/AppDbContext.cs` — it decides the column type used for `Value` and how `RowVersion` is mapped. ## The Redis cache diff --git a/docs/audit-trail.md b/docs/audit-trail.md deleted file mode 100644 index 6398b99..0000000 --- a/docs/audit-trail.md +++ /dev/null @@ -1,24 +0,0 @@ -# Audit trail - -When an audit store is registered, `SetAsync` records the before/after value, property key, and timestamp for **each property whose value actually changed**. Properties identical to what is already stored are not written to the trail, so saving a model with one edited field produces one entry rather than one per property. - -```csharp -builder.Services.AddSettings() - .UseEntityFrameworkCore() - .UseAuditStore() // ships with the EF Core package - .Build(); -``` - -Query history directly: - -```csharp -IReadOnlyList history = - await auditStore.GetHistoryAsync("MailSettings", key: "Host"); -``` - -Or via the REST API — every settings group automatically gets a `GET /api/settings/{route}/audit` endpoint. The audit store is optional: without one the endpoint returns `404` with an explanatory body, and the rest of the settings API is unaffected. - -Values marked `[Sensitive]` are recorded as `[encrypted]` on both sides of the entry rather than in cleartext. Change detection compares them as plaintext, not as stored ciphertext: AES-GCM draws a fresh nonce on every call, so an unchanged secret is re-encrypted to different bytes each save and a ciphertext comparison would log a spurious change every time. - -Write your own store (SQL table, Elasticsearch, whatever) by implementing `ISettingAuditStore`. - diff --git a/docs/caching.md b/docs/caching.md index 60b1052..d16b203 100644 --- a/docs/caching.md +++ b/docs/caching.md @@ -2,6 +2,20 @@ Reads go through `ISettingCache` (default: `IMemoryCache`, 10-minute absolute expiration). A per-group `SemaphoreSlim` prevents cache stampedes under concurrent load. +**What is cached is the stored rows, not the settings object.** Every `GetAsync` materialises a +fresh instance from them, so two callers never share one: assigning to a property of what you +read changes nothing for the next reader, and nothing for `GET /api/settings/{route}`. The cache +is there to skip the round trip to the store; the mapping is reflection over a handful of +properties and is cheap to repeat. + +Two consequences worth knowing: + +- A `[Sensitive]` value is decrypted per read rather than once per cache entry. For a + distributed cache this is the better trade anyway — what is written to Redis is the stored + ciphertext, not the decrypted model. +- A custom `ISettingCache` is handed `Setting[]`. Implementations that serialise, like the Redis + one below, need nothing special for this; the type is public and plain. + ```csharp builder.Services.AddSettings() .UseEntityFrameworkCore() @@ -10,7 +24,50 @@ builder.Services.AddSettings() .Build(); ``` -> **Multi-node note:** the default cache is per-instance in-memory. In a multi-node deployment, use `UseCustomCache()` with a distributed cache (Redis, etc.) so a write on one node invalidates the cache on all nodes. +## Running on more than one instance + +A write evicts the cache entry on the instance that made it. Nothing else happens by itself, so +every other instance goes on serving the old values until its own entry expires — up to +`CacheDuration`, ten minutes by default. "Change it, reload, done" stops being true at two +replicas. + +One line fixes it, using whatever `IDistributedCache` the application already has: + +```csharp +builder.Services.AddStackExchangeRedisCache(o => o.Configuration = "localhost:6379"); + +builder.Services.AddSettings() + .UseEntityFrameworkCore() + .SynchronizeThroughDistributedCache() + .Build(); +``` + +Every write publishes a fresh token for that group, and a cache hit compares its token against +the published one at most once per **`ChangeCheckInterval`, 5 seconds by default**. + +**That interval is the staleness bound, and it is not zero.** An instance can serve a value +another instance has already replaced for up to five seconds. What the signal buys is that the +bound is five seconds rather than ten minutes — not that propagation is instant. Pass a +different interval if you want a different trade: `SynchronizeThroughDistributedCache(checkInterval: TimeSpan.FromSeconds(1))` +costs more round trips to Redis for a tighter bound. + +The token is per group, so a write to one group does not make other groups reload. An +unreachable signalling store is logged and treated as "unchanged": a signal is an optimisation +over the cache duration, so a Redis outage should cost a slower reload, not an error. And with +no signal registered the check never fires, so a single-instance application behaves exactly as +it did before and needs no configuration. + +Register your own transport — a bus, a database table, a pub/sub topic — with +`UseChangeSignal()` over `ISettingChangeSignal`. + +> **This is about cache coherence, and nothing else.** `ISettingChangedHandler` and +> `ISettingWriteObserver` stay in-process by design: the signal makes other instances notice a +> new *value*, it does not run their handlers. If you need every node to react, publish from an +> observer onto something every node subscribes to. + +### Or share one cache + +The alternative is to give every instance the same cache, so there is only one entry to evict: ```csharp public class RedisSettingCache(IConnectionMultiplexer redis) : ISettingCache diff --git a/docs/change-notifications.md b/docs/change-notifications.md index 16b9a10..43726f4 100644 --- a/docs/change-notifications.md +++ b/docs/change-notifications.md @@ -25,3 +25,20 @@ builder.Services.AddSettings() Multiple handlers per settings type are supported and run in registration order. A handler that throws is logged and does not roll back the write or block other handlers. +## Which of the two hooks you want + +The library notifies you of a write in two shapes. They serve different consumers, and picking +the wrong one is mostly a matter of fighting the data you are handed. + +| | `ISettingChangedHandler` | [`ISettingWriteObserver`](write-notifications.md) | +|---|---|---| +| Scope | One settings group, typed | Every group, untyped | +| Payload | The `previous` and `current` models | Per-property diff, plus actor and correlation id | +| Good for | Reacting to a new value: *if `MailSettings.Host` changed, reconnect* | Recording or forwarding what happened: auditing, webhooks, indexing | +| Reports `ClearAsync` | No | Yes, as `SettingWriteKind.Cleared` | + +Both are **in-process only**, and both have the same failure policy: **an exception is logged and +swallowed**, so neither can fail a write that has already committed, and one failing handler +never skips the ones after it. + +Registering both is normal — they are not alternatives. diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index 3d95905..1570e68 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -10,11 +10,26 @@ | `.UseMongoDb(databaseFactory, createIndexes)` | MongoDb | Backing store, reusing your own `IMongoClient` | | `.UseCustomCache()` | Core | Replace the default cache | | `.WithCacheDuration(TimeSpan)` | Core | Override the 10-minute default | +| `.SynchronizeThroughDistributedCache(prefix, interval)` | Core | Propagate writes between instances over `IDistributedCache` | +| `.UseChangeSignal(interval)` | Core | Propagate writes over your own transport | | `.UseAesEncryption(key, retiredKeys...)` | Core | Enable built-in AES-256-GCM encryption; retired keys decrypt only | | `.IgnoreDecryptionFailures()` | Core | Fall back to defaults instead of throwing when a value will not decrypt | | `.UseCustomEncryption()` | Core | Plug in a custom encryptor | -| `.UseAuditStore()` | Core (+ EF Core impl) | Enable change history | +| `.UseWriteObserver()` | Core | Observe every completed write; several may be registered | +| `.UseLoggingWriteObserver()` | Core | Log one line per write — property names, never values | +| `.UseActorAccessor()` | Core | Supply who is making the change | +| `.UseHttpContextActor()` | API | Take the actor from the signed-in user of the request | | `.OnChanged()` | Core | Register a runtime change handler | -| `.UseFluentValidation(assembly)` | FluentValidation | Register validators | +| `.UseProjector()` | Core (applied by API) | Reshape a group for `GET` only; never touches the `ETag`. One per group, and declares `GET`'s response type when the projector is typed | +| `.ReplaceProjector()` | Core | Override a projector a shared registration already set | +| `.UseFluentValidation(assembly)` | FluentValidation | Register validators, and publish their rules in the schema | +| `.UseSchemaContributor()` | Core | Publish extra schema constraints; composes with any already registered | | `.Build()` | Core | Validate configuration, return `IServiceCollection` | +## `MapSettingsEndpoints()` parameters + +| Parameter | Default | Description | +|---|---|---| +| `requireIfMatch` | `false` | Reject a POST without `If-Match` with `428 Precondition Required` | +| `includeDiscovery` | `true` | Also map `GET /api/settings` and `GET /api/settings/{route}/schema` | + diff --git a/docs/dashboard.md b/docs/dashboard.md index 1d09b43..351b7c1 100644 --- a/docs/dashboard.md +++ b/docs/dashboard.md @@ -54,3 +54,12 @@ The browser never calls the API directly: Nitro proxies `/api/settings/**` to `NUXT_SETTINGS_API_URL`, so the API needs no CORS configuration. See [`clients/dashboard/README.md`](../clients/dashboard/README.md) for the full setup. +## Generating the form instead of hardcoding it + +A client does not have to know your settings classes up front. `GET /api/settings` lists the +groups and `GET /api/settings/{route}/schema` describes one — property names as the `GET` body +spells them, types, defaults, which properties are `[Sensitive]`, and the validation bounds your +validators enforce. See [Discovery endpoints](discovery-endpoints.md). + +That is the alternative to a hardcoded field list with its own copy of the bounds — a copy which +is, by construction, enforced nowhere. diff --git a/docs/defining-settings.md b/docs/defining-settings.md index 30d21ec..50da86c 100644 --- a/docs/defining-settings.md +++ b/docs/defining-settings.md @@ -4,15 +4,15 @@ [SettingGroup("payment", Name = "PaymentSettings")] public class PaymentSettings { - public string GatewayUrl { get; set; } = "https://gateway.example.com"; + public string GatewayUrl { get; set; } = "https://gateway.example.com"; [Sensitive] // encrypted at rest - public string ApiKey { get; set; } = string.Empty; + public string ApiKey { get; set; } = string.Empty; [SettingDefault(10_000)] // used when no row exists yet - public decimal MaxAmount { get; set; } = 10_000m; + public decimal MaxAmount { get; set; } = 10_000m; - public bool SandboxMode { get; set; } = true; + public bool SandboxMode { get; set; } = true; } ``` @@ -47,7 +47,6 @@ Adding `Name` to a class that **already has rows** requires renaming those rows, ```sql UPDATE Settings SET SettingGroup = 'new-name' WHERE SettingGroup = 'OldClassName'; -UPDATE SettingAudits SET SettingGroup = 'new-name' WHERE SettingGroup = 'OldClassName'; ``` ```js diff --git a/docs/discovery-endpoints.md b/docs/discovery-endpoints.md new file mode 100644 index 0000000..edc794d --- /dev/null +++ b/docs/discovery-endpoints.md @@ -0,0 +1,162 @@ +# Discovery: listing groups and describing them + +Two endpoints let a client find the settings groups and learn what is in one, so a form can be +generated from the class rather than from guesswork about the shape of a JSON response. + +| Method | Route | Returns | +|---|---|---| +| `GET` | `/api/settings` | The registered groups | +| `GET` | `/api/settings/{route}/schema` | One group's properties | + +**Neither returns a stored value.** That is what `GET /api/settings/{route}` is for, and keeping +these two free of values is what makes them safe to expose more widely than values are. + +Both are registered by `MapSettingsEndpoints()` by default. Turn them off with: + +```csharp +app.MapSettingsEndpoints(requireIfMatch: false, includeDiscovery: false); +``` + +## `GET /api/settings` + +```json +[ + { "route": "mail-server", "name": "MailSettings", "requiresAuthorization": false }, + { "route": "payment", "name": "PaymentSettings", "requiresAuthorization": true } +] +``` + +`name` is the persistence key — the `Name` on `[SettingGroup]`, or the class name — which is not +necessarily the route. `requiresAuthorization` says whether the class carries `[Authorize]`. + +This endpoint is anonymous. What it reveals is the inventory of route segments and persistence +keys, which the per-group endpoints — anonymous by default, and named after your classes — +already make guessable. If you would rather not publish the list anyway, turn discovery off. + +## `GET /api/settings/{route}/schema` + +```json +{ + "route": "mail-server", + "name": "MailSettings", + "properties": [ + { "name": "host", "type": "String", "nullable": false, "default": "smtp.example.com", + "sensitive": false, "constraints": { "required": true, "minLength": 3, "maxLength": 255 } }, + { "name": "port", "type": "Int32", "nullable": false, "default": 587, + "sensitive": false, "constraints": { "min": 1, "max": 65535 } }, + { "name": "password", "type": "String", "nullable": false, "default": null, + "sensitive": true, "constraints": null } + ] +} +``` + +- **`name`** is spelled the way the `GET` body spells it, so a client can match the two up + without guessing at a casing convention. +- **`default`** is what a read falls back to when nothing is stored: the `[SettingDefault]` value + when there is one, otherwise whatever the class initialises the property to. It is a + compile-time fact about the class, never anything read from the store. +- **`default` is omitted for a `[Sensitive]` property.** A compile-time default on a secret is a + value like any other, and this endpoint is meant to be safe to expose more widely than values + are. You still get `sensitive: true`, which is what a form needs in order to render a password + field. +- The properties listed are exactly the ones the engine stores — public, with a getter and a + setter. The schema and the engine read that rule from the same place, so a property cannot + appear here that a write would ignore. + +The schema endpoint sits inside its group, so it is covered by the group's `[Authorize]` exactly +as `GET` and `POST` are. A group's endpoints should not disagree about who may look at it. + +## Publishing validation constraints + +`constraints` is the valuable part for a generated form, and `ISettingValidator` cannot supply +it: it answers `ValidateAsync` and `CanValidate`, so it can reject a value but has no way to say +what it would accept. Without somewhere to publish rules, every client hardcodes its own — and a +bound that lives only in a dashboard is enforced nowhere. + +**With `UseFluentValidation(assembly)` you get this for free.** The same registration that wires +up your validators also registers a contributor that reads their rules out of FluentValidation's +own descriptor: + +| Rule | Published as | +|---|---| +| `NotEmpty()` / `NotNull()` | `required: true` | +| `InclusiveBetween(a, b)` | `min`, `max` | +| `GreaterThanOrEqualTo(n)` / `LessThanOrEqualTo(n)` | `min` / `max` | +| `GreaterThan(n)` / `LessThan(n)` | `exclusiveMin` / `exclusiveMax` | +| `Length(a, b)`, `MinimumLength`, `MaximumLength` | `minLength`, `maxLength` | +| `Matches(pattern)` | `pattern` | + +A rule it cannot read — a `Must(...)`, a custom validator, a comparison against another property +— is **skipped rather than guessed at**. A wrong published bound is worse than a missing one, +because a client would believe it. + +For anything else, implement `ISettingSchemaContributor`: + +```csharp +public sealed class MySchemaContributor : ISettingSchemaContributor +{ + public bool CanDescribe(Type type) => type == typeof(BrandingSettings); + + public IReadOnlyDictionary> Describe(Type type) + => new Dictionary> + { + ["HeaderLogoSize"] = new Dictionary { ["min"] = 16, ["max"] = 64 } + }; +} +``` + +Keys are property names as spelled on the class. Constraint names are conventional rather than +closed — `required`, `min`, `max`, `exclusiveMin`, `exclusiveMax`, `minLength`, `maxLength`, +`pattern` — so you can describe a rule this library has never heard of. + +### Several contributors compose + +**Every** contributor whose `CanDescribe` returns `true` is consulted, and their constraints are +merged per property. Constraints are additive facts about a property, not a decision, so +describing different aspects of one group is the normal case — one contributor publishing the +FluentValidation rules while another publishes something this library cannot know about, such as +a rule carried by an attribute from a different package: + +```csharp +builder.Services.AddSettings() + .UseFluentValidation(assembly) // min, max, pattern, required + .UseSchemaContributor() // translated + .Build(); +``` + +Both survive; neither deletes the other. + +`UseSchemaContributor()` registers one type once however often it is called — a contributor +only describes, so a duplicate would describe the same group twice and read as a collision with +itself. That is unlike `UseWriteObserver()`, where two registrations are two observers, +because an observer *does* something. There is also a factory overload, +`UseSchemaContributor(sp => new MyContributor(...))`, for a contributor needing something DI +cannot construct it with — which is common here, since the whole point is describing rules this +library does not own. + +(This is deliberately unlike `ISettingValidator`, where the first match wins. Running two +validators over one property risks rejecting or reporting a value twice — but there is nothing +wrong with two contributors each naming a different fact about it.) + +On a collision — the same constraint name on the same property from two contributors — **the +first registered wins** and the clash is logged at `Warning`, naming both contributors, the +group, the property and the constraint. Resolving it quietly would make the published schema +depend on registration order. + +Registration order is the order of the builder calls, so putting `UseSchemaContributor()` +before or after `UseFluentValidation()` decides which one's value survives a clash. Above, +`UseFluentValidation` comes first and so wins any constraint both describe. + +With none registered, every property reports `constraints: null` and the endpoint is otherwise +unchanged. A contributor that throws costs its own constraints — not the response, and not the +other contributors' constraints. + +> **Publishing a constraint changes nothing about enforcement.** The validator is still what +> rejects a bad write. The schema exists so a client can show the same bounds the server will +> apply, instead of inventing its own. + +## See also + +- [REST API endpoints](rest-api.md) — the read and write endpoints, and how to secure them +- [Dashboard](dashboard.md) — the SPA client these endpoints are for +- [Validation](validation.md) — defining the rules in the first place diff --git a/docs/reading-and-writing.md b/docs/reading-and-writing.md index 1272b2b..d2c725f 100644 --- a/docs/reading-and-writing.md +++ b/docs/reading-and-writing.md @@ -24,18 +24,43 @@ public class EmailService(ISettingManager settings) |---|---| | `GetAsync(refreshCache, ct)` | Returns the full settings object | | `GetAsync(selector, refreshCache, ct)` | Returns one property | -| `SetAsync(model, ct)` | Persists the full object — validates, encrypts, audits, notifies | -| `SetAsync(selector, value, ct)` | Updates a single property | +| `SetAsync(model, ct)` | Persists the full object — validates, encrypts, notifies | +| `SetAsync(selector, value, ct)` | Updates a single property — and writes only that property | | `ExistsAsync(allProperties, ct)` | Checks row existence | | `ClearAsync(ct)` | Deletes all settings for the group | | `GetVersionAsync(ct)` | Current revision, for conditional writes | | `SetAsync(model, expectedVersion, ct)` | Persists only if the group is still at that revision | +> **Each read returns its own object.** `GetAsync` materialises a fresh instance every time, +> even on a cache hit — the cache holds the stored rows, not the model. Assigning to a property +> of what you read changes nothing anyone else sees; persist it with `SetAsync`. See +> [Caching](caching.md). + > **There is no synchronous read.** A blocking `Get()` would park a thread-pool thread on > database I/O, and under load that starves the pool for the whole application — not just for > settings. Where a value is needed inside a synchronous lambda, read it once with `await` > beforehand and capture it; that is both correct and cheaper than resolving it per element. +## Updating one property + +`SetAsync(x => x.Port, 587)` writes one row. Properties you did not name are not part of the +write at all, so a concurrent edit to a different property of the same group cannot be undone +by it. + +This was not always true. The call used to rebuild the whole model **from the cache** and put it +through the group write, which compares every property against fresh store rows and writes +whatever differs — so a property another instance had changed while this one's cache was warm +read back stale, counted as a difference, and was written back. A call that meant to touch one +property reverted someone else's edit to another, over a window as wide as `CacheDuration` +rather than a round trip. + +Registered validators still see the whole group, built from a fresh read, because a validator +takes a model rather than a property. A `null` value writes nothing: properties are skipped +rather than stored as null, so this overload cannot clear one — use `ClearAsync` for that. + +Two callers racing on the *same* property are still resolved rather than merged: the write is +conditional on that property's stored revision, and the loser gets `SettingConcurrencyException`. + ## Concurrent writes `SetAsync` writes **only the properties whose values differ from what is stored**, and each of diff --git a/docs/rest-api.md b/docs/rest-api.md index c7753cc..7fcb0f2 100644 --- a/docs/rest-api.md +++ b/docs/rest-api.md @@ -14,7 +14,93 @@ Registers, per `[SettingGroup]` class: |---|---|---| | `GET` | `/api/settings/{route}` | Current values, with an `ETag` for the revision | | `POST` | `/api/settings/{route}` | Validate + persist; honours `If-Match`, `412` on a lost race | -| `GET` | `/api/settings/{route}/audit` | Change history (`404` if no audit store configured) | +| `GET` | `/api/settings` | The registered groups — see [Discovery](discovery-endpoints.md) | +| `GET` | `/api/settings/{route}/schema` | One group's properties, types, defaults and constraints | + +All of them carry the OpenAPI tag `Settings`, so they group themselves in Swagger or Scalar +without being wrapped in a `MapGroup("")` purely to hang a tag on. + +The last two are the discovery pair, and neither returns a stored value. They are on by default; +`MapSettingsEndpoints(requireIfMatch: false, includeDiscovery: false)` leaves them out. + +> There is no `/audit` endpoint. Auditing is not built in — see +> [Write notifications](write-notifications.md) for the hook that replaced it. + +## Reshaping what GET returns + +`GET` serves the stored values. When the representation your UI needs is not the representation +you store — a display name in the request's language, an image id resolved to a URL, a field +masked for display — register an `ISettingProjector`: + +```csharp +public sealed class BrandingProjector(ITranslator translator) : ISettingProjector +{ + public async Task ProjectAsync(BrandingSettings group, CancellationToken ct = default) + => new + { + StoreName = await translator.ForCurrentRequestAsync(group.StoreName, ct), + group.PrimaryColor + }; +} +``` + +```csharp +builder.Services.AddSettings() + .UseEntityFrameworkCore() + .UseProjector() + .Build(); +``` + +**One projector per group.** They do not compose: `ProjectAsync` takes the *group*, so a second +projector has nowhere to receive the first's output. Registering two is rejected by `Build()` +rather than silently letting the last one win — a group that needs both translations and media +wants one projector doing the join. To override a projector a shared registration already set, +call `.ReplaceProjector()`. + +Projectors are registered scoped, so they can depend on anything else in the request scope. +Four rules make this safe to have: + +- **Read path only.** `POST` never consults a projector, so it cannot influence what is stored. + The projected shape is an output shape; it is not something you can post back. +- **Never applied to the `ETag`.** The tag keeps describing the stored revision. A tag derived + from projected output would move with the request's language, and `If-Match` would stop + protecting anything. +- **No projector registered is the previous behaviour, byte for byte.** +- **A projector that throws fails the request** — deliberately unlike + [`ISettingChangedHandler` and `ISettingWriteObserver`](write-notifications.md#failure-policy), + whose exceptions are logged and swallowed. A failed notification must not break a write that + has already committed; a failed projection means the response would be wrong, and a wrong + response must not be hidden. + +Programmatic reads are unaffected: `For().GetAsync()` always returns the stored group. + +### Letting the document describe the projection + +`ISettingProjector` returns `object`, so `GET` can only declare the stored group as its 200 +response — wrong for every projected group. Name the type instead, and the generated document +describes it: + +```csharp +public sealed record BrandingView(string StoreName, string PrimaryColor); + +public sealed class BrandingProjector(ITranslator translator) + : ISettingProjector +{ + public async Task ProjectAsync(BrandingSettings group, CancellationToken ct = default) + => new(await translator.ForCurrentRequestAsync(group.StoreName, ct), group.PrimaryColor); +} +``` + +Registration is unchanged — `.UseProjector()` — and so is +the response body. This documents what you already return; it does not change it. + +The type goes on the projector rather than as a third argument to `UseProjector` so the compiler +checks it. A `UseProjector()` would be a promise the author +could get wrong, and catching that would need a runtime check. + +`ISettingProjector` keeps working and keeps declaring the group, so nothing has to change. +`POST` still accepts the stored group either way: advertising the projection as a request body +would promise a write the API refuses. ## Securing the endpoints diff --git a/docs/storage-providers.md b/docs/storage-providers.md index 84ff42b..18df374 100644 --- a/docs/storage-providers.md +++ b/docs/storage-providers.md @@ -14,25 +14,36 @@ Pick one provider per application. Each is a separate package on top of `DotNetB dotnet add package DotNetBoost.Settings.EntityFrameworkCore --prerelease ``` -Add the two settings tables to your `DbContext`: +Add the settings table to your `DbContext`. That one call is the whole registration — +your context needs no interface and no `DbSet` properties, because the store reaches its +entities through `Set()`: ```csharp -using DotNetBoost.Settings.Core.Models; using DotNetBoost.Settings.EntityFrameworkCore; using Microsoft.EntityFrameworkCore; -public class AppDbContext(DbContextOptions options) - : DbContext(options), ISettingDbContext +public class AppDbContext(DbContextOptions options) : DbContext(options) { - public DbSet Settings => Set(); - public DbSet SettingAudits => Set(); - protected override void OnModelCreating(ModelBuilder mb) - => mb.ApplySettingsConfiguration(DatabaseProvider.Sqlite); - // Options: SqlServer | PostgreSql | Sqlite + => mb.ApplySettings(this); } ``` +The engine — which decides the `Value` column type and how `RowVersion` is mapped — is read +off the context, so switching provider needs no matching edit here. Name it explicitly when +you need to override that, for a model built against one engine and migrated onto another: + +```csharp +protected override void OnModelCreating(ModelBuilder mb) + => mb.ApplySettings(DatabaseProvider.SqlServer); + // Options: SqlServer | PostgreSql | Sqlite +``` + +> **Upgrading from `1.0.0-preview.1`?** `ISettingDbContext` is obsolete and does nothing. +> Delete `: ISettingDbContext` and the two `DbSet` properties; nothing else changes. +> `UseEntityFrameworkCore()` registers nothing against `DbContext` itself, so the +> library never claims a framework type it does not own. + ```csharp builder.Services.AddDbContext(o => o.UseSqlite("Data Source=app.db")); builder.Services.AddSettings() @@ -40,7 +51,7 @@ builder.Services.AddSettings() .Build(); ``` -Run `dotnet ef migrations add Init && dotnet ef database update` — this creates both the `Settings` and `SettingAudits` tables. +Run `dotnet ef migrations add Init && dotnet ef database update` — this creates the `Settings` table. ## Dapper @@ -55,7 +66,7 @@ builder.Services.AddSettings() .Build(); ``` -Supports `SqlConnection`, `NpgsqlConnection`, and `SqliteConnection`. `migrateSchema: true` auto-creates `Settings` and `SettingAudits` tables on startup. +Supports `SqlConnection`, `NpgsqlConnection`, and `SqliteConnection`. `migrateSchema: true` auto-creates the `Settings` table on startup. ## MongoDB diff --git a/docs/write-notifications.md b/docs/write-notifications.md new file mode 100644 index 0000000..c268810 --- /dev/null +++ b/docs/write-notifications.md @@ -0,0 +1,113 @@ +# Write notifications + +**Auditing is not built into this library.** It used to be, and the implementation could not be +frozen into 1.0: it recorded no actor, recorded nothing at all for a group clear, wrote outside +the write's own transaction, could fail a write that had already committed, exposed an unbounded +history endpoint, and existed only for Entity Framework Core — the Dapper bootstrap created a +`SettingAudits` table that nothing ever wrote to. + +What the library does instead is report every write, and let you decide what to do with it. +A general `DotNetBoost.Auditing` package will implement recording over this hook, the same way +`DotNetBoost.Settings.FluentValidation` implements `ISettingValidator` — port here, +implementation in a package of its own. + +## `ISettingWriteObserver` + +Called once after every completed write to any settings group. + +```csharp +public sealed class WriteForwarder(IMessageBus bus) : ISettingWriteObserver +{ + public Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default) + => bus.PublishAsync(write, ct); +} +``` + +```csharp +builder.Services.AddSettings() + .UseEntityFrameworkCore() + .UseWriteObserver() + .Build(); +``` + +Several observers can be registered; all of them run, in registration order. + +### What a `SettingWrite` carries + +| Member | | +|---|---| +| `GroupName` | The persistence key the rows are stored under | +| `Route` | The group's API route segment | +| `Kind` | `Updated`, or `Cleared` for `ClearAsync` | +| `Changes` | One `SettingChange` per property that actually changed; empty when `Cleared` | +| `Actor` | Who made the change, or `null` for "not captured" | +| `CorrelationId` | One value per write, so the several changes of one save are identifiable as one edit | +| `OccurredAt` | UTC timestamp | + +Each `SettingChange` is `(PropertyName, OldValue, NewValue, IsRedacted)`. `OldValue` is `null` +when the property had never been stored. + +A write that changes nothing raises no notification at all. Change detection compares +`[Sensitive]` values as plaintext rather than as stored ciphertext: AES-GCM draws a fresh nonce +on every call, so an unchanged secret re-encrypts to different bytes each save and a ciphertext +comparison would report a change every time. + +### `[Sensitive]` properties are redacted, not reported + +A `[Sensitive]` property arrives with `IsRedacted` set and **both values `null`**. The library +decides what is sensitive; an observer only learns that it changed. This is not configurable, +and that is the point — an observer is precisely the component that forwards writes somewhere +else, so it should not be the place a secret escapes. + +Redaction follows the `[Sensitive]` attribute, not whether a value happened to be encrypted at +rest, so it holds in an application with no encryptor configured. + +### Capturing the actor + +`Actor` is `null` until you register an accessor, and `null` means *not captured* — never read +it as a claim about who acted. ASP.NET applications want the one from the API package: + +```csharp +settings.UseHttpContextActor(); // DotNetBoost.Settings.API +``` + +It reads `ClaimTypes.NameIdentifier`, then `Identity.Name`, and returns `null` outside a request +or for an unauthenticated one. Implement `ISettingActorAccessor` for anything else — a background +job's name, a CLI user, a tenant id. + +### Failure policy + +**An observer's exception is logged and swallowed.** An observer runs after the store write has +committed and the cache entry has been evicted, so failing the call would report a write that in +fact succeeded. Each observer is wrapped on its own, so one failure does not skip the rest. + +This is the same policy as [`ISettingChangedHandler`](change-notifications.md) and +deliberately not the one for [`ISettingProjector`](rest-api.md#reshaping-what-get-returns), +which *can* fail a request — a failed notification must not break a committed write, while a +failed projection would otherwise return a wrong response. + +## The stopgap: logging writes + +`LoggingSettingWriteObserver` ships in Core so the hook is not an empty port: + +```csharp +settings.UseLoggingWriteObserver(); +``` + +One log line per write, at `Information`, naming the group, the kind, the actor, the correlation +id and **the properties that changed — never their values**, redacted or not. A settings value +is exactly the kind of thing that should not reach a log aggregator because a default turned it +on. It is off unless you ask for it. + +This is a stopgap, not a trail: there is nothing to query, and nothing enforces retention. For a +real history, implement `ISettingWriteObserver` over your own store. + +## Upgrading from `1.0.0-preview.1` + +`ISettingAuditStore`, `SettingAuditEntry`, `EfCoreAuditStore`, `UseAuditStore()` and +`GET /api/settings/{route}/audit` are gone, along with the `SettingAudits` table in the EF model +and the Dapper bootstrap. Implement `ISettingWriteObserver`, or call `.UseLoggingWriteObserver()` +in the meantime. + +Nothing drops an existing `SettingAudits` table for you — your rows are still there. Drop it +yourself once you no longer want them. diff --git a/samples/SampleApp/AppDbContext.cs b/samples/SampleApp/AppDbContext.cs index ed7e3c6..0bc57fd 100644 --- a/samples/SampleApp/AppDbContext.cs +++ b/samples/SampleApp/AppDbContext.cs @@ -1,23 +1,21 @@ using DotNetBoost.Settings.Core.Interfaces; -using DotNetBoost.Settings.Core.Models; using DotNetBoost.Settings.EntityFrameworkCore; using Microsoft.EntityFrameworkCore; using SampleApp.Settings; namespace SampleApp; -public sealed class AppDbContext(DbContextOptions options) - : DbContext(options), ISettingDbContext +public sealed class AppDbContext(DbContextOptions options) : DbContext(options) { - public DbSet Settings => Set(); - public DbSet SettingAudits => Set(); - - // Must match the provider block active in Program.cs: it decides the column type - // chosen for Value (text / nvarchar(max) / longtext) and how RowVersion is mapped. + // The entire settings registration. No interface and no DbSet properties: the store + // reaches its entities through Set(), and the engine — which decides the column type + // chosen for Value and how RowVersion is mapped — is read off the context, so switching + // the provider block in Program.cs needs no matching edit here. + // + // To pin the engine instead (a model built against one and migrated onto another): + // => modelBuilder.ApplySettings(DatabaseProvider.PostgreSql); protected override void OnModelCreating(ModelBuilder modelBuilder) - => modelBuilder.ApplySettingsConfiguration(DatabaseProvider.PostgreSql); - // => modelBuilder.ApplySettingsConfiguration(DatabaseProvider.SqlServer); - // => modelBuilder.ApplySettingsConfiguration(DatabaseProvider.Sqlite); + => modelBuilder.ApplySettings(this); } /// diff --git a/samples/SampleApp/Program.cs b/samples/SampleApp/Program.cs index 60246cc..7d5ce8c 100644 --- a/samples/SampleApp/Program.cs +++ b/samples/SampleApp/Program.cs @@ -103,12 +103,7 @@ var settings = builder.Services.AddSettings(); // ── Store registration, one line per provider ──────────────────────────────── -// EfCoreAuditStore is what backs GET /api/settings/{route}/audit. ISettingAuditStore is -// optional, so a provider without one simply records no history — MongoDB has no -// implementation yet, and the Dapper store writes its SettingAudits table only when its -// own schema migration has run. -settings.UseEntityFrameworkCore() - .UseAuditStore(); +settings.UseEntityFrameworkCore(); // MongoDB: settings.UseMongoDb(mongoConnectionString, databaseName: "settingsdb"); // Dapper: (add `using Npgsql;` at the top) @@ -123,12 +118,22 @@ .UseAesEncryption(encryptionKey) .UseFluentValidation(Assembly.GetExecutingAssembly()) .OnChanged() + + // Every completed write is reported through ISettingWriteObserver. This one logs the + // group, the actor and the names of the properties that changed — never their values. + // Swap it for your own observer to record, forward or index writes. + .UseLoggingWriteObserver() + + // Fills SettingWrite.Actor from the signed-in user. The sample has no identity provider, + // so it stays null here; it is the line a real application needs. + .UseHttpContextActor() .Build(); builder.Services.AddScoped(); // Generates the OpenAPI document that the Scalar UI reads. The settings endpoints -// already carry WithName/WithSummary/Produces metadata, so they document themselves. +// already carry WithName/WithSummary/Produces metadata and a "Settings" tag, so they +// document and group themselves. builder.Services.AddOpenApi(options => options.AddDocumentTransformer((document, _, _) => { @@ -136,8 +141,9 @@ document.Info.Version = "v1"; document.Info.Description = "Runtime settings served from PostgreSQL. Each [SettingGroup] class is exposed as " - + "GET (read), POST (update) and GET /audit (change history). Properties marked " - + "[Sensitive] are encrypted at rest and shown as \"[encrypted]\" in the audit trail."; + + "GET (read) and POST (update). Properties marked [Sensitive] are encrypted at " + + "rest, and their values are withheld from the write notifications the library " + + "raises."; return Task.CompletedTask; })); diff --git a/src/DotNetBoost.Settings.API/ApiServiceCollectionExtensions.cs b/src/DotNetBoost.Settings.API/ApiServiceCollectionExtensions.cs new file mode 100644 index 0000000..35f2601 --- /dev/null +++ b/src/DotNetBoost.Settings.API/ApiServiceCollectionExtensions.cs @@ -0,0 +1,21 @@ +using DotNetBoost.Settings.API; +using DotNetBoost.Settings.Core; +using DotNetBoost.Settings.Core.Interfaces; + +namespace Microsoft.Extensions.DependencyInjection; + +/// Builder methods that need ASP.NET Core. +public static class SettingsApiBuilderExtensions +{ + /// + /// Captures the signed-in user of the current request as , + /// so a write observer can report who made a change instead of leaving it unknown. + /// Also registers IHttpContextAccessor, which it needs. + /// + public static SettingBuilder UseHttpContextActor(this SettingBuilder builder) + { + ArgumentNullException.ThrowIfNull(builder); + builder.Services.AddHttpContextAccessor(); + return builder.UseActorAccessor(); + } +} diff --git a/src/DotNetBoost.Settings.API/HttpContextSettingActorAccessor.cs b/src/DotNetBoost.Settings.API/HttpContextSettingActorAccessor.cs new file mode 100644 index 0000000..9acb8ad --- /dev/null +++ b/src/DotNetBoost.Settings.API/HttpContextSettingActorAccessor.cs @@ -0,0 +1,38 @@ +using DotNetBoost.Settings.Core.Interfaces; +using Microsoft.AspNetCore.Http; +using System.Security.Claims; + +namespace DotNetBoost.Settings.API; + +/// +/// Reports the signed-in user of the current request as +/// . Register with .UseHttpContextActor(). +/// +/// +/// Prefers over Identity.Name: the subject +/// identifier is stable, while a display name can be changed by the person it names, which +/// is the wrong property for something a record of who-did-what is keyed on. +/// +/// Returns null outside a request, for an unauthenticated one, and when neither value +/// is present — the generated endpoints are anonymous unless the settings class carries +/// [Authorize], so that last case is a real one rather than a defensive branch. Null +/// means "not captured" and never stands in for a user. +/// +/// +/// Supplies the ambient request, if there is one. +public sealed class HttpContextSettingActorAccessor(IHttpContextAccessor httpContextAccessor) + : ISettingActorAccessor +{ + /// + public string? GetActor() + { + var user = httpContextAccessor.HttpContext?.User; + if (user?.Identity is not { IsAuthenticated: true }) return null; + + var subject = user.FindFirstValue(ClaimTypes.NameIdentifier); + return string.IsNullOrWhiteSpace(subject) ? NullIfBlank(user.Identity.Name) : subject; + } + + private static string? NullIfBlank(string? value) + => string.IsNullOrWhiteSpace(value) ? null : value; +} diff --git a/src/DotNetBoost.Settings.API/SettingsDiscoveryContracts.cs b/src/DotNetBoost.Settings.API/SettingsDiscoveryContracts.cs new file mode 100644 index 0000000..adbc2ae --- /dev/null +++ b/src/DotNetBoost.Settings.API/SettingsDiscoveryContracts.cs @@ -0,0 +1,44 @@ +namespace DotNetBoost.Settings.API; + +/// One registered settings group, as GET /api/settings lists it. +/// The route segment its endpoints live under. +/// The persistence key its rows are stored under. +/// +/// Whether the group's class carries [Authorize]. false means its endpoints are +/// anonymous, which is the default for a settings class. +/// +public sealed record SettingGroupDescriptor(string Route, string Name, bool RequiresAuthorization); + +/// One property, as GET /api/settings/{route}/schema describes it. +/// +/// The property name as the GET body spells it, so a client can match the two up +/// without guessing at a casing convention. +/// +/// The CLR type name — the underlying type for a nullable. +/// Whether the property is a nullable value type. +/// +/// The value a read falls back to when nothing is stored. Omitted for a [Sensitive] +/// property: a compile-time default on a secret is exactly the kind of thing this endpoint +/// must not hand out, given it is meant to be safe to expose more widely than values are. +/// +/// Whether the property carries [Sensitive]. +/// +/// Validation rules from the registered ISettingSchemaContributor, or null when +/// none is registered or it has nothing to say about this property. +/// +public sealed record SettingPropertyDescriptor( + string Name, + string Type, + bool Nullable, + object? Default, + bool Sensitive, + IReadOnlyDictionary? Constraints); + +/// The shape of one settings group. Never its values. +/// The route segment the group's endpoints live under. +/// The persistence key its rows are stored under. +/// The properties the engine reads and writes. +public sealed record SettingGroupSchema( + string Route, + string Name, + IReadOnlyList Properties); diff --git a/src/DotNetBoost.Settings.API/SettingsMinimalApiExtensions.cs b/src/DotNetBoost.Settings.API/SettingsMinimalApiExtensions.cs index 66b309a..7ab6197 100644 --- a/src/DotNetBoost.Settings.API/SettingsMinimalApiExtensions.cs +++ b/src/DotNetBoost.Settings.API/SettingsMinimalApiExtensions.cs @@ -1,5 +1,9 @@ +using DotNetBoost.Settings.API; +using DotNetBoost.Settings.Core; using DotNetBoost.Settings.Core.Attributes; using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.Core.Models; +using Microsoft.Extensions.Logging; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; @@ -31,12 +35,23 @@ private static readonly ConcurrentDictionary private static readonly ConcurrentDictionary>> VersionDelegates = new(); + private static readonly ConcurrentDictionary ProjectorTypes = new(); + + private static readonly ConcurrentDictionary>> + ProjectorDelegates = new(); + + /// + /// OpenAPI tag the generated endpoints are grouped under, matching how the sibling + /// DotNetBoost packages tag theirs. Without it every consumer has to wrap the call in + /// MapGroup("") purely to carry a tag. + /// + private const string EndpointTag = "Settings"; + private static readonly JsonSerializerOptions JsonOpts = new() { PropertyNameCaseInsensitive = true }; /// /// Registers GET and POST endpoints for every [SettingGroup]-decorated class. - /// Optionally registers GET /audit for classes that have an audit store registered. /// public static void MapSettingsEndpoints(this IEndpointRouteBuilder endpoints) => endpoints.MapSettingsEndpoints(requireIfMatch: false); @@ -50,7 +65,19 @@ public static void MapSettingsEndpoints(this IEndpointRouteBuilder endpoints) /// 428 Precondition Required instead of writing unconditionally. Turn this on once /// every client round-trips the ETag returned by GET. /// - public static void MapSettingsEndpoints(this IEndpointRouteBuilder endpoints, bool requireIfMatch) + /// + /// When true (the default), also registers GET /api/settings listing the registered + /// groups and GET /api/settings/{route}/schema describing one group's properties. + /// + /// Neither returns a stored value. The schema endpoint is covered by its group's + /// [Authorize] like the group's other endpoints; the list is anonymous, and what it + /// reveals is the set of route segments and persistence keys — which the per-group + /// endpoints, anonymous by default and named after the class, already make guessable. Turn + /// it off if you would rather not publish that inventory. + /// + /// + public static void MapSettingsEndpoints( + this IEndpointRouteBuilder endpoints, bool requireIfMatch, bool includeDiscovery = true) { var settingTypes = AppDomain.CurrentDomain .GetAssemblies() @@ -62,18 +89,228 @@ public static void MapSettingsEndpoints(this IEndpointRouteBuilder endpoints, bo { var attr = type.GetCustomAttribute()!; var auth = type.GetCustomAttribute(); - var group = endpoints.MapGroup($"api/settings/{attr.Route}"); + var group = endpoints.MapGroup($"api/settings/{attr.Route}").WithTags(EndpointTag); if (auth is not null) group.RequireAuthorization(auth); - RegisterGet(group, type); + // One resolve per group at map time. A projector itself is scoped and cannot be + // resolved here, which is exactly why the response type has to be declared + // separately rather than discovered from the projector. + var declared = endpoints.ServiceProvider + .GetKeyedService(type)?.Projection; + + RegisterGet(group, type, declared); RegisterPost(group, type, requireIfMatch); - RegisterAuditGet(group, type); + + // Inside the same group, so it inherits the RequireAuthorization above: a group's + // endpoints should not disagree about who may look at it. + if (includeDiscovery) RegisterSchemaGet(group, type); + } + + if (includeDiscovery) RegisterGroupList(endpoints, settingTypes); + } + + /// + /// GET /api/settings — the registered groups. Routes and persistence keys only; + /// nothing here reads the store. + /// + private static void RegisterGroupList(IEndpointRouteBuilder endpoints, List settingTypes) + { + // Resolved once at startup rather than per request: the set of [SettingGroup] classes + // cannot change while the application runs. + var groups = settingTypes + .Select(t => new SettingGroupDescriptor( + Route: t.GetCustomAttribute()!.Route, + Name: SettingGroupAttribute.ResolveName(t), + RequiresAuthorization: t.GetCustomAttribute() is not null)) + .OrderBy(g => g.Route, StringComparer.Ordinal) + .ToList(); + + endpoints.MapGet("api/settings", () => Results.Ok(groups)) + .WithTags(EndpointTag) + .WithName("ListSettingGroups") + .WithSummary("Lists the registered settings groups. Returns no values.") + .Produces>(); + } + + /// + /// GET /api/settings/{route}/schema — what the group's properties are, so a client + /// can generate a form from the class instead of guessing at the shape of a response. + /// + private static void RegisterSchemaGet(RouteGroupBuilder group, Type type) + { + var route = type.GetCustomAttribute()!.Route; + var groupName = SettingGroupAttribute.ResolveName(type); + + // The property list is a fact about the class, so it is built once. Constraints are + // resolved per request, because a contributor is a registered service. + var properties = SettingSchema.Describe(type); + + group.MapGet("/schema", (IServiceProvider sp) => + { + var constraints = DescribeConstraints(sp, type); + + var described = properties + .Select(p => new SettingPropertyDescriptor( + Name: JsonNamingPolicy.CamelCase.ConvertName(p.Name), + Type: p.ClrType, + Nullable: p.IsNullable, + + // Withheld for a [Sensitive] property. This endpoint is meant to be safe + // to expose more widely than values are, and a compile-time default on a + // secret property is a value like any other. + Default: p.IsSensitive ? null : p.DefaultValue, + Sensitive: p.IsSensitive, + Constraints: constraints.GetValueOrDefault(p.Name))) + .ToList(); + + return Results.Ok(new SettingGroupSchema(route, groupName, described)); + }) + .WithName($"Schema{type.Name}") + .WithSummary($"Describes the {type.Name} properties. Returns no values.") + .Produces(); + } + + /// + /// Constraints for a group, merged across every registered contributor that can describe it. + /// + /// Every applicable contributor is consulted, not just the first. Constraints are additive + /// facts about a property rather than a decision, so two contributors describing different + /// aspects of one group is the normal case: one publishing the validation rules, another + /// publishing something this library cannot know about — an attribute belonging to a + /// different package, say. First-match-wins would make registering the second silently + /// delete the first's constraints, and which one survived would depend on registration + /// order. That is deliberately unlike ISettingValidator, where first-match is right + /// because running two validators over one property risks rejecting or reporting it twice. + /// + /// + /// Merging is two levels deep: property name, then constraint name. On a collision — the + /// same constraint on the same property from two contributors — the first registered wins + /// and the clash is logged, because the alternative is an answer that depends on + /// registration order. + /// + /// + private static IReadOnlyDictionary> DescribeConstraints( + IServiceProvider sp, Type type) + { + // Split into first and rest so the common case — exactly one contributor — hands back + // its own dictionary with nothing merged and nothing allocated. + ISettingSchemaContributor? first = null; + List? rest = null; + + foreach (var candidate in sp.GetServices()) + { + if (!candidate.CanDescribe(type)) continue; + + if (first is null) first = candidate; + else (rest ??= []).Add(candidate); + } + + if (first is null) return EmptyConstraints; + if (rest is null) return SafeDescribe(first, type, sp) ?? EmptyConstraints; + + var merged = new Dictionary>(StringComparer.Ordinal); + + Merge(first, type, sp, merged); + foreach (var contributor in rest) Merge(contributor, type, sp, merged); + + return merged.ToDictionary( + entry => entry.Key, + entry => (IReadOnlyDictionary)entry.Value.ToDictionary( + constraint => constraint.Key, + constraint => constraint.Value.Value, + StringComparer.Ordinal), + StringComparer.Ordinal); + } + + /// + /// Folds one contributor's constraints into , keeping the first + /// claim on any property/constraint pair and logging the loser. + /// + private static void Merge( + ISettingSchemaContributor contributor, + Type type, + IServiceProvider sp, + Dictionary> merged) + { + var described = SafeDescribe(contributor, type, sp); + if (described is null) return; + + var owner = contributor.GetType().Name; + + foreach (var (propertyName, constraints) in described) + { + if (constraints is null) continue; + + if (!merged.TryGetValue(propertyName, out var target)) + merged[propertyName] = target = new Dictionary(StringComparer.Ordinal); + + foreach (var (constraintName, value) in constraints) + { + if (target.TryGetValue(constraintName, out var existing)) + { + // Logged rather than resolved quietly: two contributors disagreeing about + // the same constraint is a configuration problem, and silently taking + // either one makes the published schema depend on registration order. + if (Logger(sp) is { } logger) + LogConstraintClash(logger, existing.Owner, owner, type.Name, propertyName, constraintName, null); + + continue; + } + + target[constraintName] = new Claim(value, owner); + } + } + } + + /// + /// One contributor's constraints, or null when it failed. + /// + /// A contributor is third-party code reflecting over validation rules, and a schema is a + /// convenience, so one that cannot describe itself costs its own constraints and not the + /// endpoint. Wrapped per contributor rather than around the whole loop: with several + /// registered, one failing must not cost the others theirs. + /// + /// + private static IReadOnlyDictionary>? SafeDescribe( + ISettingSchemaContributor contributor, Type type, IServiceProvider sp) + { + try { return contributor.Describe(type); } + catch (Exception ex) when (ex is not OperationCanceledException) + { + if (Logger(sp) is { } logger) + LogContributorFailed(logger, contributor.GetType().Name, type.Name, ex); + + return null; } } - private static void RegisterGet(RouteGroupBuilder group, Type type) + /// A constraint value and the contributor that claimed it, for the clash message. + private readonly record struct Claim(object? Value, string Owner); + + private static ILogger? Logger(IServiceProvider sp) + => sp.GetService()?.CreateLogger(LoggerCategory); + + private static readonly IReadOnlyDictionary> EmptyConstraints + = new Dictionary>(); + + private const string LoggerCategory = "DotNetBoost.Settings.API.Schema"; + + private static readonly Action LogContributorFailed = + LoggerMessage.Define( + LogLevel.Warning, new EventId(1200, nameof(LogContributorFailed)), + "Schema contributor {Contributor} failed for settings group {Group}; reporting no constraints."); + + private static readonly Action LogConstraintClash = + LoggerMessage.Define( + LogLevel.Warning, new EventId(1201, nameof(LogConstraintClash)), + // Each placeholder appears once and in the order the arguments are passed: + // LoggerMessage.Define binds them positionally, not by name. + "Schema contributors {Winner} and {Loser} both describe {Group}.{Property}'s " + + "'{Constraint}' constraint; keeping the first registered."); + + private static void RegisterGet(RouteGroupBuilder group, Type type, Type? declared) { group.MapGet("/", async (HttpContext ctx, ISettingManager manager, CancellationToken ct) => { @@ -87,12 +324,31 @@ private static void RegisterGet(RouteGroupBuilder group, Type type) var getter = GetterDelegates.GetOrAdd(type, CreateGetterDelegate); var result = await getter(accessor, false, ct).ConfigureAwait(false); + // The tag describes the STORED revision and is set from the version read above — + // deliberately before projection, and never derived from it. A tag covering + // projected output would move with the request's language, or with data owned by + // some other component, and If-Match would stop protecting the stored values. ctx.Response.Headers.ETag = $"\"{version}\""; + + // Resolved from the request scope: a projector is registered scoped and is + // expected to depend on per-request state. Nothing registered means the group is + // serialised exactly as before. + var projectorType = ProjectorTypes.GetOrAdd(type, MakeProjectorType); + if (ctx.RequestServices.GetService(projectorType) is { } projector && result is not null) + { + result = await ProjectorDelegates.GetOrAdd(type, CreateProjectorDelegate)(projector, result, ct) + .ConfigureAwait(false); + } + return Results.Ok(result); }) .WithName($"Get{type.Name}") .WithSummary($"Returns the current {type.Name} settings.") - .Produces(200, type); + + // The projection when a typed projector declared one, otherwise the stored group. + // POST below keeps accepting the group either way: advertising the projection as a + // request body would promise a write the API refuses. + .Produces(200, declared ?? type); } private static void RegisterPost(RouteGroupBuilder group, Type type, bool requireIfMatch) @@ -144,30 +400,6 @@ private static void RegisterPost(RouteGroupBuilder group, Type type, bool requir .ProducesValidationProblem(); } - private static void RegisterAuditGet(RouteGroupBuilder group, Type type) - { - // The audit trail is keyed by the group's persistence name, which is not necessarily - // the class name. Resolved once here rather than per request. - var groupName = SettingGroupAttribute.ResolveName(type); - - group.MapGet("/audit", async (IServiceProvider sp, string? key, CancellationToken ct) => - { - // Resolved from the request scope rather than bound as a handler parameter. - // ISettingAuditStore is optional, and minimal APIs infer an unregistered complex - // parameter as a *body* parameter — which throws while building the endpoint, - // taking the whole application down at startup rather than at request time. - var auditStore = sp.GetService(); - - if (auditStore is null) - return Results.NotFound("Audit store is not configured."); - - var history = await auditStore.GetHistoryAsync(groupName, key, ct).ConfigureAwait(false); - return Results.Ok(history); - }) - .WithName($"Audit{type.Name}") - .WithSummary($"Returns the change history for {type.Name} settings."); - } - private static object GetAccessor(ISettingManager manager, Type type) => AccessorFactories.GetOrAdd(type, t => { @@ -200,6 +432,21 @@ private static object GetAccessor(ISettingManager manager, Type type) }; } + private static Type MakeProjectorType(Type type) + => typeof(ISettingProjector<>).MakeGenericType(type); + + private static Func> CreateProjectorDelegate(Type type) + { + var method = MakeProjectorType(type).GetMethod( + nameof(ISettingProjector.ProjectAsync), [type, typeof(CancellationToken)])!; + + return async (projector, group, ct) => + { + var task = (Task)method.Invoke(projector, [group, ct])!; + return await task.ConfigureAwait(false); + }; + } + private static Func> CreateVersionDelegate(Type type) { var accessorType = typeof(ISettingAccessor<>).MakeGenericType(type); diff --git a/src/DotNetBoost.Settings.Core/Interfaces/ISettingChangeSignal.cs b/src/DotNetBoost.Settings.Core/Interfaces/ISettingChangeSignal.cs new file mode 100644 index 0000000..cc82656 --- /dev/null +++ b/src/DotNetBoost.Settings.Core/Interfaces/ISettingChangeSignal.cs @@ -0,0 +1,38 @@ +namespace DotNetBoost.Settings.Core.Interfaces; + +/// +/// Tells every instance of the application that a settings group changed, so each one reloads +/// its cached copy within seconds instead of waiting out +/// . +/// +/// A version token per group, in some store every instance can reach. Every write publishes a +/// new token through , and a read compares +/// against the token its cached copy was loaded under, at most +/// once per . The +/// built-in implementation uses IDistributedCache +/// (SynchronizeThroughDistributedCache()); register your own with +/// UseChangeSignal<T>(). +/// +/// +/// The default does nothing, and a single-instance application needs no other. Without a real +/// one, an instance only sees another's write when its own cache entry expires. Failures are +/// logged and tolerated on both sides: a signal is an optimisation over the cache duration, +/// never the thing that makes a write durable. +/// +/// +/// +/// This is about cache coherence, and nothing else. and +/// stay in-process by design: a signal makes other +/// instances notice a new value, it does not run their handlers. +/// +public interface ISettingChangeSignal +{ + /// + /// The current token for , or null when no change has + /// been signalled for it (or the entry was evicted). + /// + Task GetVersionAsync(string groupName, CancellationToken ct = default); + + /// Publishes a new token for . + Task SignalChangeAsync(string groupName, CancellationToken ct = default); +} diff --git a/src/DotNetBoost.Settings.Core/Interfaces/ISettingProjector.cs b/src/DotNetBoost.Settings.Core/Interfaces/ISettingProjector.cs new file mode 100644 index 0000000..16aebf5 --- /dev/null +++ b/src/DotNetBoost.Settings.Core/Interfaces/ISettingProjector.cs @@ -0,0 +1,109 @@ +namespace DotNetBoost.Settings.Core.Interfaces; + +/// +/// Shapes a settings group for the READ path only. Registered in DI with +/// .UseProjector<TSettings, TProjector>(); applied by MapSettingsEndpoints +/// on GET. +/// +/// Implement instead to name the type you +/// return, and GET's OpenAPI response will describe the projection rather than the stored +/// group. This interface cannot: the projector is resolved per request, long after the route +/// was mapped, so at map time nothing knows what a projection looks like. +/// +/// +/// This is the hook for serving a group as something other than its stored values — +/// translating a display name into the request's language, resolving an image id to a URL, +/// masking a field for the UI — without the settings engine needing to know what any of +/// those things are. +/// +/// +/// +/// Four properties hold, and the generated endpoints are what hold them: +/// +/// +/// +/// Read path only. A projector is never consulted on POST/PUT, so it cannot influence +/// what is stored. Round-tripping a projected body back through POST is the caller's problem, +/// not something the library pretends to support. +/// +/// +/// +/// +/// Never applied to the ETag. GetVersionAsync keeps covering the stored values. +/// A tag derived from projected output would change with the request's language or with data +/// owned by another component, and conditional writes would stop protecting anything. +/// +/// +/// +/// +/// No projector registered is today's behaviour, byte for byte. The group is serialised +/// exactly as it was before. +/// +/// +/// +/// +/// A projector that throws fails the request. This is deliberately unlike +/// and , whose +/// exceptions are logged and swallowed: a failed notification must not break a write that has +/// already committed, whereas a failed projection means the response would be wrong, and a +/// wrong response must not be hidden. +/// +/// +/// +/// +/// Programmatic reads through ISettingManager are unaffected: For<T>().GetAsync() +/// always returns the stored group. Projection belongs to the HTTP representation. +/// +/// +/// The settings group this projector shapes. +public interface ISettingProjector where T : new() +{ + /// + /// Returns what GET should serialise in place of . Returning + /// itself is valid and means "no change". + /// + /// The settings group as stored. Treat it as read-only. + /// Cancels the projection. + Task ProjectAsync(T group, CancellationToken ct = default); +} + +/// +/// A projector that names the type it returns, so the generated GET can advertise it. +/// +/// +/// +/// returns object, which leaves the endpoint +/// declaring the stored group as its 200 response — wrong for every projected group, and not +/// something the library could fix on its own: a projector is resolved from the request scope, +/// long after the route was mapped. Declaring here puts +/// the answer somewhere the compiler checks it, which a third generic argument on +/// UseProjector would not. +/// +/// +/// Everything in still holds — read path only, never +/// applied to the ETag, and a projector that throws fails the request. This changes the +/// documentation of the response, not the response: the body is byte for byte what the +/// untyped interface produces, and POST still accepts the stored group, because +/// advertising the projection as a request body would promise a write the API refuses. +/// +/// +/// The settings group this projector shapes. +/// What GET returns in its place. +public interface ISettingProjector : ISettingProjector + where T : new() + where TProjection : class +{ + /// + /// Returns what GET should serialise in place of . + /// + /// The settings group as stored. Treat it as read-only. + /// Cancels the projection. + new Task ProjectAsync(T group, CancellationToken ct = default); + + /// + /// Adapts to the object-returning member the generated endpoint calls, so implementing + /// the typed interface is all an author has to do. + /// + async Task ISettingProjector.ProjectAsync(T group, CancellationToken ct) + => await ProjectAsync(group, ct).ConfigureAwait(false); +} diff --git a/src/DotNetBoost.Settings.Core/Interfaces/ISettingSchemaContributor.cs b/src/DotNetBoost.Settings.Core/Interfaces/ISettingSchemaContributor.cs new file mode 100644 index 0000000..c879fc0 --- /dev/null +++ b/src/DotNetBoost.Settings.Core/Interfaces/ISettingSchemaContributor.cs @@ -0,0 +1,51 @@ +namespace DotNetBoost.Settings.Core.Interfaces; + +/// +/// Describes the validation constraints on a settings group's properties, for the schema +/// endpoint to publish. +/// +/// +/// +/// A separate port because cannot answer this: it only offers +/// ValidateAsync and CanValidate, so it can reject a value but has no way to say +/// what it would accept. Without a way to publish rules, every client ends up hardcoding them +/// — and a bound that lives only in a dashboard is not enforced anywhere. +/// +/// +/// Constraints are property name → constraint name → value. Names are conventional +/// rather than closed, so a contributor can describe a rule this library has never heard of; +/// prefer the familiar ones where they fit: required, min, max, +/// exclusiveMin, exclusiveMax, minLength, maxLength, +/// pattern. +/// +/// +/// Several compose. Every registered contributor whose returns +/// true is consulted, and their constraints are merged per property — constraints are additive +/// facts about a property, not a decision, so two contributors describing different aspects of +/// one group is the normal case rather than a conflict. Register a second one to publish +/// something this library cannot know about: a rule carried by an attribute from another +/// package, for instance, alongside the validation rules the first contributor reads. +/// +/// +/// On a collision — the same constraint name on the same property from two contributors — the +/// first registered wins and the clash is logged at Warning. Resolving it quietly would +/// make the published schema depend on registration order. +/// +/// +/// Entirely optional. With no contributor registered the schema endpoint simply reports no +/// constraints, and publishing a constraint changes nothing about enforcement — the validator +/// is still what rejects a bad write. A contributor that throws costs its own constraints and +/// not the endpoint, nor the other contributors' constraints. +/// +/// +public interface ISettingSchemaContributor +{ + /// Whether this contributor has anything to say about . + bool CanDescribe(Type type); + + /// + /// The constraints on 's properties, keyed by property name as it + /// is spelled on the class. + /// + IReadOnlyDictionary> Describe(Type type); +} diff --git a/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs b/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs new file mode 100644 index 0000000..05efceb --- /dev/null +++ b/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs @@ -0,0 +1,119 @@ +namespace DotNetBoost.Settings.Core.Interfaces; + +/// What a did to the group. +public enum SettingWriteKind +{ + /// Properties were written. lists them. + Updated, + + /// + /// Every row in the group was deleted by ClearAsync. + /// is empty — the operation is group-wide, and nothing + /// is read back to enumerate what was there. + /// + Cleared +} + +/// One property's change within a write. +/// The property that changed, as named on the settings class. +/// +/// The value before the write, or null when the property had never been stored. +/// Always null when is set. +/// +/// +/// The value as written. Always null when is set. +/// +/// +/// Whether the property is [Sensitive]. Its values are withheld rather than reported: +/// the library decides what is sensitive, and an observer only learns that it changed. An +/// observer cannot opt out of this, which is the point — a recording or forwarding observer +/// should not be the place a secret escapes. +/// +public sealed record SettingChange( + string PropertyName, + string? OldValue, + string? NewValue, + bool IsRedacted); + +/// A completed write to one settings group. +/// +/// The persistence key the group's rows are stored under — the Name on +/// [SettingGroup], or the class name when none is set. +/// +/// +/// The group's API route segment, from [SettingGroup]. Falls back to +/// for a settings class that carries no attribute. +/// +/// Whether properties were written or the whole group was cleared. +/// +/// One entry per property that actually changed; empty when is +/// . A write that changed nothing raises no +/// at all. +/// +/// +/// Who made the change, from the registered . null +/// means not captured — read it as "unknown", never as a claim about who acted. +/// +/// +/// One value per write, so the several of a single save are +/// identifiable as one edit. The ambient Activity id when there is one, otherwise a +/// fresh value. +/// +/// UTC timestamp of the write. +public sealed record SettingWrite( + string GroupName, + string Route, + SettingWriteKind Kind, + IReadOnlyList Changes, + string? Actor, + string CorrelationId, + DateTime OccurredAt); + +/// +/// Observes every completed write to every settings group. This is the hook an auditing +/// package, a webhook publisher, a search indexer or a distributed cache signal implements. +/// +/// +/// +/// Failure policy: an exception is logged and swallowed. An observer runs after the +/// store write has committed and the cache entry has been evicted, so failing the call would +/// report a write that in fact succeeded. Each observer is wrapped individually, so one +/// failure does not skip the rest. The same policy applies to +/// , and deliberately not to +/// , which can still fail a request because a failed +/// projection would otherwise return a wrong response. +/// +/// +/// In-process only. Several observers may be registered; all are invoked in registration +/// order. +/// +/// +/// This is the untyped half of the notification pair. Use +/// instead when you want the typed before/after models +/// of one group — "if MailSettings.Host changed, reconnect" — rather than a +/// per-property diff of every group. +/// +/// +public interface ISettingWriteObserver +{ + /// Called once per completed write. Exceptions are logged and swallowed. + Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default); +} + +/// +/// Supplies who is making the current change, for . +/// +/// +/// The default registered by AddSettings() returns null. ASP.NET applications +/// can register the HttpContext-backed one from DotNetBoost.Settings.API with +/// .UseHttpContextActor(). +/// +public interface ISettingActorAccessor +{ + /// + /// The current actor, or null when it cannot be determined. Returning a + /// placeholder instead of null would turn "nobody captured this" into a claim + /// about who acted, so do not. + /// + string? GetActor(); +} diff --git a/src/DotNetBoost.Settings.Core/Interfaces/Interfaces.cs b/src/DotNetBoost.Settings.Core/Interfaces/Interfaces.cs index 52921e9..b0c92eb 100644 --- a/src/DotNetBoost.Settings.Core/Interfaces/Interfaces.cs +++ b/src/DotNetBoost.Settings.Core/Interfaces/Interfaces.cs @@ -42,12 +42,29 @@ Task GetAsync(Expression> selector, Task SetAsync(T model, CancellationToken cancellationToken = default); /// - /// Updates one property. Reads the current group, applies the value, and writes the whole - /// group back — so concurrent single-property writes can overwrite one another. + /// Updates one property, and writes only that property. + /// + /// Properties you did not name are never part of the write, so a concurrent edit to a + /// different property of the same group cannot be reverted by this call. The write is + /// still conditional on the property's own stored revision, so two callers racing on the + /// same property are resolved rather than silently merged — the loser gets + /// . + /// + /// + /// Registered validators still see the whole group, built from a fresh read of the store, + /// because a validator takes a model rather than a property. + /// /// /// Must point directly at a property of . - /// The value to store. + /// + /// The value to store. A null writes nothing — a property is skipped rather than + /// stored as null, so this cannot be used to clear one. Use + /// to remove the group's rows. + /// /// Cancels the read and the write. + /// + /// A registered validator rejected the group with the new value applied; nothing is written. + /// Task SetAsync(Expression> selector, TProp value, CancellationToken cancellationToken = default); @@ -119,24 +136,15 @@ public interface ISettingEncryptor string Decrypt(string ciphertext); } -/// -/// Persists a full history of setting changes. -/// Register an implementation to enable auditing. -/// -public interface ISettingAuditStore -{ - /// Appends one entry to the change history. - Task RecordAsync(SettingAuditEntry entry, CancellationToken ct = default); - - /// Reads the change history for a group, optionally narrowed to a single property. - Task> GetHistoryAsync( - string group, string? key = null, CancellationToken ct = default); -} - /// /// Receives a notification whenever the settings group is written. /// Register one or more implementations in DI; all will be invoked in registration order. /// +/// +/// The typed half of the notification pair. For the per-property diff of every group, with +/// the actor and a correlation id, implement instead. +/// Both are in-process, and both have their exceptions logged and swallowed. +/// public interface ISettingChangedHandler where T : new() { /// diff --git a/src/DotNetBoost.Settings.Core/Models/Setting.cs b/src/DotNetBoost.Settings.Core/Models/Setting.cs index 9401eb5..84d7eec 100644 --- a/src/DotNetBoost.Settings.Core/Models/Setting.cs +++ b/src/DotNetBoost.Settings.Core/Models/Setting.cs @@ -48,28 +48,3 @@ public sealed class Setting /// Generates a fresh concurrency token. Stores call this when writing a row. public static byte[] NewRowVersion() => Guid.NewGuid().ToByteArray(); } - -/// Immutable record of a historical change to a setting value. -public sealed class SettingAuditEntry -{ - /// Surrogate primary key for the audit row. - public Guid Id { get; set; } = Guid.NewGuid(); - - /// Persistence key of the settings group the change belongs to. - public required string Group { get; set; } - - /// Name of the property that changed. - public required string Key { get; set; } - - /// Value before the change, or empty on first write. Encrypted values read [encrypted]. - public required string OldValue { get; set; } - - /// Value after the change. Encrypted values read [encrypted]. - public required string NewValue { get; set; } - - /// Who made the change. - public required string ChangedBy { get; set; } - - /// UTC timestamp of the change. - public DateTime ChangedAt { get; set; } = DateTime.UtcNow; -} diff --git a/src/DotNetBoost.Settings.Core/Models/SettingProjectionDescriptor.cs b/src/DotNetBoost.Settings.Core/Models/SettingProjectionDescriptor.cs new file mode 100644 index 0000000..a6ed944 --- /dev/null +++ b/src/DotNetBoost.Settings.Core/Models/SettingProjectionDescriptor.cs @@ -0,0 +1,19 @@ +namespace DotNetBoost.Settings.Core.Models; + +/// +/// The type a settings group's GET returns — the projection when a typed projector is +/// registered, otherwise the group itself. +/// +/// +/// Registered as a keyed singleton under the group type by UseProjector, and read once +/// per group by MapSettingsEndpoints to declare the endpoint's 200 response. A keyed +/// entry means there is one per group by construction, with no ordering rule to define: the +/// last registration to write the key wins, exactly as the projector registration it +/// accompanies does. +/// +/// A declaration about the response, nothing more. Nothing resolves a projector from it, and +/// it is read at map time rather than per request. +/// +/// +/// The CLR type GET serialises. +public sealed record SettingProjectionDescriptor(Type Projection); diff --git a/src/DotNetBoost.Settings.Core/Services/SettingChangeSignals.cs b/src/DotNetBoost.Settings.Core/Services/SettingChangeSignals.cs new file mode 100644 index 0000000..9a1a70a --- /dev/null +++ b/src/DotNetBoost.Settings.Core/Services/SettingChangeSignals.cs @@ -0,0 +1,83 @@ +using DotNetBoost.Settings.Core.Interfaces; +using Microsoft.Extensions.Caching.Distributed; +using System.Text; + +namespace DotNetBoost.Settings.Core.Services; + +/// +/// Default : signals nothing. Registered by +/// AddSettings() so the service always resolves, and so a single-instance application +/// needs no configuration at all. +/// +/// +/// It always reports null, which always matches the token a cached copy was loaded +/// under, so every cache hit is served exactly as it was before change signalling existed. +/// +internal sealed class NullSettingChangeSignal : ISettingChangeSignal +{ + private static readonly Task NoVersion = Task.FromResult(null); + + public Task GetVersionAsync(string groupName, CancellationToken ct = default) => NoVersion; + + public Task SignalChangeAsync(string groupName, CancellationToken ct = default) => Task.CompletedTask; +} + +/// +/// over — Redis, SQL Server, +/// or whatever the application already shares between instances. One small entry per settings +/// group, with no expiry; if the cache evicts one anyway, every instance reloads that group +/// once and carries on. +/// +/// A cache every instance of the application shares. +/// +/// Prefixes the entry holding each group's token. Make it distinct per application if several +/// share a cache. +/// +public sealed class DistributedCacheSettingChangeSignal( + IDistributedCache cache, + string keyPrefix = DistributedCacheSettingChangeSignal.DefaultKeyPrefix) + : ISettingChangeSignal +{ + /// The key prefix used when none is given. + public const string DefaultKeyPrefix = "DotNetBoost.Settings:version:"; + + /// + public async Task GetVersionAsync(string groupName, CancellationToken ct = default) + => Decode(await cache.GetAsync(Key(groupName), ct).ConfigureAwait(false)); + + /// + public Task SignalChangeAsync(string groupName, CancellationToken ct = default) + => cache.SetAsync( + Key(groupName), + Encoding.UTF8.GetBytes(Guid.NewGuid().ToString("N")), + new DistributedCacheEntryOptions(), + ct); + + private string Key(string groupName) + { + ArgumentException.ThrowIfNullOrWhiteSpace(groupName); + return keyPrefix + groupName; + } + + private static string? Decode(byte[]? value) => value is null ? null : Encoding.UTF8.GetString(value); +} + +/// +/// Publishes a change signal for every completed write. Registered as an +/// rather than called from inside the manager, so there is +/// one notification path rather than two — which is also what makes ClearAsync signal +/// without a second call site. +/// +/// +/// An observer's exception is logged and swallowed, which is the right policy here: the write +/// has already committed, and a signal is an optimisation over the cache duration. A failed +/// signal costs other instances a slower reload, not a lost write. +/// +internal sealed class ChangeSignalPublisher(ISettingChangeSignal signal) : ISettingWriteObserver +{ + public Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default) + { + ArgumentNullException.ThrowIfNull(write); + return signal.SignalChangeAsync(write.GroupName, ct); + } +} diff --git a/src/DotNetBoost.Settings.Core/Services/SettingManager.cs b/src/DotNetBoost.Settings.Core/Services/SettingManager.cs index 83fd8e2..cea2ff9 100644 --- a/src/DotNetBoost.Settings.Core/Services/SettingManager.cs +++ b/src/DotNetBoost.Settings.Core/Services/SettingManager.cs @@ -4,6 +4,8 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using System.Collections.Concurrent; +using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Linq.Expressions; using System.Reflection; @@ -17,14 +19,12 @@ public sealed partial class SettingManager : ISettingManager private readonly ISettingStore _store; private readonly ISettingCache _cache; private readonly ISettingEncryptor? _encryptor; - private readonly ISettingAuditStore? _auditStore; + private readonly ISettingActorAccessor? _actorAccessor; + private readonly ISettingChangeSignal _signal; private readonly IServiceProvider _sp; private readonly ILogger _logger; private readonly SettingOptions _options; - /// Placeholder written to the audit trail in place of an encrypted value. - private const string EncryptedPlaceholder = "[encrypted]"; - internal TimeSpan CacheDuration => _options.CacheDuration; private static readonly ConcurrentDictionary TypeMaps = new(); @@ -39,52 +39,90 @@ public sealed partial class SettingManager : ISettingManager private static readonly ConcurrentDictionary Locks = new(); /// - /// Creates a manager. , and - /// are optional: omitting them disables encryption and - /// auditing, and applies the default options. + /// What is known about each cached group: the change-signal token the entry was loaded + /// under, and when that token was last re-checked. + /// + /// Kept beside the cache rather than inside it. When a token was last checked is a fact + /// about this process, so it must not travel through a distributed ISettingCache to + /// other instances. It is keyed by the cache instance because a probe describes one + /// cache's entry: two managers sharing a cache must share probes, and two holding separate + /// caches must not. The table holds the cache weakly, so nothing is kept alive by it. + /// + /// + private static readonly ConditionalWeakTable> + ProbesByCache = new(); + + private ConcurrentDictionary Probes => ProbesByCache.GetOrCreateValue(_cache); + + /// + /// Creates a manager. , , + /// and are optional: omitting them + /// disables encryption, leaves unset, keeps cache + /// invalidation local to this instance, and applies the default options. /// public SettingManager( ISettingStore store, ISettingCache cache, IServiceProvider sp, ILogger logger, - ISettingEncryptor? encryptor = null, - ISettingAuditStore? auditStore = null, - SettingOptions? options = null) + ISettingEncryptor? encryptor = null, + ISettingActorAccessor? actorAccessor = null, + ISettingChangeSignal? signal = null, + SettingOptions? options = null) { - _store = store ?? throw new ArgumentNullException(nameof(store)); - _cache = cache ?? throw new ArgumentNullException(nameof(cache)); - _sp = sp ?? throw new ArgumentNullException(nameof(sp)); - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - _encryptor = encryptor; - _auditStore = auditStore; - _options = options ?? new SettingOptions(); + _store = store ?? throw new ArgumentNullException(nameof(store)); + _cache = cache ?? throw new ArgumentNullException(nameof(cache)); + _sp = sp ?? throw new ArgumentNullException(nameof(sp)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _encryptor = encryptor; + _actorAccessor = actorAccessor; + _signal = signal ?? new NullSettingChangeSignal(); + _options = options ?? new SettingOptions(); } /// public ISettingAccessor For() where T : new() => new SettingAccessor(this); + /// + /// Reads a group, serving the stored rows from the cache when they are there. + /// + /// What is cached is the rows, never the model: every caller gets a model of its own, so + /// assigning to a property of one read cannot change what the next read returns. Caching + /// the model instead would hand the same mutable object to every reader of the group — + /// including GET /api/settings/{group}, which would then serve a value nobody + /// stored. The round trip to the store is what the cache is for; + /// is reflection over a handful of properties and is cheap to repeat. + /// + /// internal async Task GetAsync(bool refreshCache, CancellationToken ct) where T : new() { var map = GetTypeMap(typeof(T)); var key = CacheKey(map); - if (!refreshCache && TryFromCache(key, out var fast)) - return fast!; + if (!refreshCache && await TryServeFromCacheAsync(key, map, ct).ConfigureAwait(false) is { } fast) + return MapToModel(fast); var locker = Locks.GetOrAdd(key, _ => new SemaphoreSlim(1, 1)); await locker.WaitAsync(ct).ConfigureAwait(false); try { - if (!refreshCache && TryFromCache(key, out var hit)) - return hit!; + if (!refreshCache && await TryServeFromCacheAsync(key, map, ct).ConfigureAwait(false) is { } hit) + return MapToModel(hit); + + // Read the token BEFORE the rows, so a write landing mid-read leaves a mismatch + // for the next check to catch rather than being recorded as already seen. + var version = await ReadSignalVersionAsync(map.GroupName, ct).ConfigureAwait(false); + + var rows = await _store.GetGroupAsync(map.GroupName, ct).ConfigureAwait(false); - var rows = await _store.GetGroupAsync(map.GroupName, ct).ConfigureAwait(false); - var model = MapToModel(rows); + // A concrete array rather than the store's own list: a distributed ISettingCache + // has to serialise this, and an interface is not something every serialiser can + // round-trip. + _cache.Set(key, rows as Setting[] ?? [.. rows], CacheDuration); + Probes[key] = new CacheProbe(version, DateTimeOffset.UtcNow); - _cache.Set(key, model, CacheDuration); - return model; + return MapToModel(rows); } finally { @@ -92,6 +130,58 @@ public SettingManager( } } + /// + /// The cached rows for a group, or null to reload. + /// + /// With the default no-op signal the token is always null on both sides, so this is + /// a cache lookup and nothing else — a single-instance application behaves exactly as it + /// did before change signalling existed. With a real signal, a cache hit is re-validated + /// at most once per , which is what bounds + /// how long one instance can serve values another instance has already replaced. + /// + /// + private async Task TryServeFromCacheAsync(string key, TypeMap map, CancellationToken ct) + { + if (!TryRowsFromCache(key, out var rows)) return null; + + // No probe means this instance did not load the entry — a shared ISettingCache was + // populated by another one. MinValue makes the check due immediately, so the token is + // compared now rather than trusted. + var probe = Probes.GetValueOrDefault(key) ?? new CacheProbe(null, DateTimeOffset.MinValue); + + var now = DateTimeOffset.UtcNow; + if (now - probe.CheckedAt < _options.ChangeCheckInterval) return rows; + + var current = await ReadSignalVersionAsync(map.GroupName, ct).ConfigureAwait(false); + + if (!string.Equals(current, probe.Version, StringComparison.Ordinal)) + return null; + + Probes[key] = probe with { CheckedAt = now }; + return rows; + } + + /// + /// The group's current change-signal token, or null. + /// + /// An unreachable signal is logged and treated as "unchanged": it is an optimisation over + /// the cache duration, so failing a read because the signalling store is down would turn a + /// slower reload into an outage. The cache entry still expires on its own. + /// + /// + private async Task ReadSignalVersionAsync(string groupName, CancellationToken ct) + { + try + { + return await _signal.GetVersionAsync(groupName, ct).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + LogChangeSignalUnreachable(_logger, groupName, ex); + return null; + } + } + internal async Task GetAsync( Expression> selector, bool refreshCache, CancellationToken ct) where T : new() { @@ -116,8 +206,8 @@ internal async Task GetAsync( // One authoritative read of the stored state, serving four purposes: deciding what // actually changed, supplying the concurrency token each write is conditional on, - // giving the audit trail its before-values, and the model handed to change handlers. - // It deliberately bypasses the cache — a stale token would fail every write. + // giving the write notification its before-values, and the model handed to change + // handlers. It deliberately bypasses the cache — a stale token would fail every write. var prevRows = await _store.GetGroupAsync(map.GroupName, ct).ConfigureAwait(false); var prevMap = prevRows.ToDictionary(r => r.Key, StringComparer.OrdinalIgnoreCase); @@ -130,47 +220,30 @@ internal async Task GetAsync( throw new SettingConcurrencyException(map.GroupName, "*"); } + // A decryption failure is fatal here, exactly as it is on the read path. Substituting + // defaults would read every encrypted property as changed, write them all back and tell + // an observer they changed — the silent-default behaviour that IgnoreDecryptionFailures() + // exists to make a deliberate choice rather than an accident. Anything else still falls + // back to defaults so a mapping fault cannot block a write, but is logged, not swallowed. T previous; - try { previous = MapToModel(prevRows); } - catch { previous = new T(); } + try + { + previous = MapToModel(prevRows); + } + catch (Exception ex) when (ex is not SettingDecryptionException) + { + LogPreviousModelFailed(_logger, map.GroupName, ex); + previous = new T(); + } var rows = new List(map.Properties.Count); foreach (var prop in map.Properties) { - var raw = prop.Getter(model!); - if (raw is null) continue; - - var plaintext = Extensions.ConvertFrom(prop.PropertyType, raw); - var strValue = plaintext; - - var isEncrypted = false; - if (prop.IsSensitive && _encryptor is not null) - { - strValue = _encryptor.Encrypt(strValue); - isEncrypted = true; - } - - prevMap.TryGetValue(prop.Name, out var prev); - // Writing only what changed is what keeps concurrent edits to *different* // properties of the same group from overwriting one another. - if (IsUnchanged(prev, isEncrypted, plaintext, map.GroupName, prop.Name)) continue; - - rows.Add(new Setting - { - Id = prev?.Id ?? Guid.NewGuid(), - Group = map.GroupName, - Key = prop.Name, - Type = prop.TypeName, - Value = strValue, - IsEncrypted = isEncrypted, - UpdatedAt = DateTime.UtcNow, - - // The token this write is conditional on. Null for a property that has never - // been stored, which is a plain insert. - RowVersion = prev?.RowVersion - }); + if (BuildRowIfChanged(map, prop, prop.Getter(model!), prevMap) is { } row) + rows.Add(row); } if (rows.Count == 0) @@ -181,37 +254,132 @@ internal async Task GetAsync( await _store.UpsertManyAsync(rows, ct).ConfigureAwait(false); - _cache.Remove(CacheKey()); + EvictLocally(); - if (_auditStore is not null) - { - // rows already holds only what changed, so every one of them earns an entry. - foreach (var row in rows) - { - prevMap.TryGetValue(row.Key, out var prev); - await _auditStore.RecordAsync(new SettingAuditEntry - { - Group = row.Group, - Key = row.Key, - OldValue = prev is null ? string.Empty - : prev.IsEncrypted ? EncryptedPlaceholder : prev.Value, - NewValue = row.IsEncrypted ? EncryptedPlaceholder : row.Value, - ChangedBy = "system" - }, ct).ConfigureAwait(false); - } - } + // One notification for the whole write, not one per row: the N properties of a single + // save are one edit, and the shared correlation id is what says so. + await NotifyWrittenAsync( + map, SettingWriteKind.Updated, BuildChanges(map, rows, prevMap), ct).ConfigureAwait(false); await FireChangedHandlersAsync(previous, model, map.GroupName, ct).ConfigureAwait(false); LogGroupUpdated(_logger, map.GroupName, rows.Count); } + /// + /// Writes one property, and only that property. + /// + /// It used to rebuild the whole model from the cache and put it through the group + /// write, which compares every property against fresh store rows and writes whatever + /// differs. So a property another writer had changed while this instance's cache was warm + /// read back stale, counted as a difference, and was written back — reverting their edit, + /// from a call that only ever meant to touch one property, over a window as wide as + /// CacheDuration rather than a round trip. + /// + /// + /// Now a single row is built and written. Properties the caller did not name are never + /// part of the write, so there is nothing to revert. Validators still see the whole group + /// — they take a model, not a property — built from a fresh read so the rest of it is what + /// is actually stored. + /// + /// internal async Task SetAsync( Expression> selector, TProp value, CancellationToken ct) where T : new() { - var model = await GetAsync(false, ct).ConfigureAwait(false); - GetPropertyInfo(selector).SetValue(model, value); - await SetAsync(model, null, ct).ConfigureAwait(false); + var map = GetTypeMap(typeof(T)); + var info = GetPropertyInfo(selector); + + var prop = map.Properties.FirstOrDefault(p => string.Equals(p.Name, info.Name, StringComparison.Ordinal)) + ?? throw new InvalidOperationException( + $"'{info.Name}' is not a readable and writable property of {typeof(T).Name}."); + + // Straight from the store, never the cache. A stale read is the whole bug, and the + // concurrency token has to be the current one or the conditional write fails. + var prevRows = await _store.GetGroupAsync(map.GroupName, ct).ConfigureAwait(false); + var prevMap = prevRows.ToDictionary(r => r.Key, StringComparer.OrdinalIgnoreCase); + + // Both map the same rows on purpose: `previous` is the before-image and `candidate` is + // the instance mutated below, so they cannot be the same object. A decryption failure is + // fatal for the reason given in the group overload above. + T previous, candidate; + try + { + previous = MapToModel(prevRows); + candidate = MapToModel(prevRows); + } + catch (Exception ex) when (ex is not SettingDecryptionException) + { + LogPreviousModelFailed(_logger, map.GroupName, ex); + previous = new T(); + candidate = new T(); + } + + prop.Setter(candidate!, value); + + await ValidateOrThrowAsync(candidate, ct).ConfigureAwait(false); + + var row = BuildRowIfChanged(map, prop, prop.Getter(candidate!), prevMap); + if (row is null) + { + LogGroupUnchanged(_logger, map.GroupName); + return; + } + + await _store.UpsertManyAsync([row], ct).ConfigureAwait(false); + + EvictLocally(); + + await NotifyWrittenAsync( + map, SettingWriteKind.Updated, BuildChanges(map, [row], prevMap), ct).ConfigureAwait(false); + + await FireChangedHandlersAsync(previous, candidate, map.GroupName, ct).ConfigureAwait(false); + + LogGroupUpdated(_logger, map.GroupName, 1); + } + + /// + /// Builds the row for one property, or null when nothing needs writing. Shared by + /// the group write and the single-property write so the encryption and change-detection + /// rules cannot drift apart between them. + /// + /// + /// null when the value is unchanged, and also when it is null — a null + /// property is skipped rather than stored, so a property cannot be cleared by assigning + /// null to it. Use ClearAsync to remove a group's rows. + /// + private Setting? BuildRowIfChanged( + TypeMap map, PropertyMap prop, object? raw, Dictionary prevMap) + { + if (raw is null) return null; + + var plaintext = Extensions.ConvertFrom(prop.PropertyType, raw); + var strValue = plaintext; + + var isEncrypted = false; + if (prop.IsSensitive && _encryptor is not null) + { + strValue = _encryptor.Encrypt(strValue); + isEncrypted = true; + } + + prevMap.TryGetValue(prop.Name, out var prev); + + if (IsUnchanged(prev, isEncrypted, plaintext, map.GroupName, prop.Name)) return null; + + return new Setting + { + Id = prev?.Id ?? Guid.NewGuid(), + Group = map.GroupName, + Key = prop.Name, + Type = prop.TypeName, + Value = strValue, + IsEncrypted = isEncrypted, + UpdatedAt = DateTime.UtcNow, + + // The token this write is conditional on. Null for a property that has never + // been stored, which is a plain insert. + RowVersion = prev?.RowVersion + }; } internal async Task ExistsAsync(bool allProperties, CancellationToken ct) where T : new() @@ -228,13 +396,92 @@ internal async Task SetAsync( { var map = GetTypeMap(typeof(T)); await _store.DeleteGroupAsync(map.GroupName, ct).ConfigureAwait(false); - _cache.Remove(CacheKey()); + EvictLocally(); + + // The most destructive operation in the library, and the one most worth reporting. + // Changes is empty: the deletion is group-wide, and nothing is read back to enumerate + // what was there — SettingWriteKind.Cleared is what carries the meaning. + await NotifyWrittenAsync(map, SettingWriteKind.Cleared, [], ct).ConfigureAwait(false); + LogGroupCleared(_logger, map.GroupName); } /// - /// Decides whether a written row is identical to what was already stored, so the audit - /// trail can skip it. Encrypted values are compared as plaintext: AES-GCM draws a fresh + /// Turns the rows about to be written, and the state they replaced, into the per-property + /// diff an observer is handed. already holds only what changed. + /// + private static List BuildChanges( + TypeMap map, List rows, Dictionary prevMap) + { + var sensitive = map.Properties + .Where(p => p.IsSensitive) + .Select(p => p.Name) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + var changes = new List(rows.Count); + + foreach (var row in rows) + { + // Redaction is decided here rather than by the observer, so a [Sensitive] value + // cannot escape through a recording or forwarding observer by omission. + if (sensitive.Contains(row.Key)) + { + changes.Add(new SettingChange(row.Key, null, null, IsRedacted: true)); + continue; + } + + prevMap.TryGetValue(row.Key, out var prev); + changes.Add(new SettingChange(row.Key, prev?.Value, row.Value, IsRedacted: false)); + } + + return changes; + } + + /// + /// Reports a completed write to every registered . + /// + /// Each observer is wrapped on its own: the write has already committed and the cache + /// entry is already gone, so an observer's exception must neither fail the caller nor + /// skip the observers after it. Same policy as + /// , and the two are documented as a pair. + /// + /// + private async Task NotifyWrittenAsync( + TypeMap map, SettingWriteKind kind, IReadOnlyList changes, CancellationToken ct) + { + var observers = _sp.GetServices() as IList + ?? [.. _sp.GetServices()]; + + if (observers.Count == 0) return; + + var write = new SettingWrite( + GroupName: map.GroupName, + Route: map.Route, + Kind: kind, + Changes: changes, + Actor: _actorAccessor?.GetActor(), + + // One id for the whole write. The ambient Activity when there is one, so the edit + // joins the surrounding trace rather than being correlatable only with itself. + CorrelationId: Activity.Current?.Id ?? Guid.NewGuid().ToString("n"), + OccurredAt: DateTime.UtcNow); + + foreach (var observer in observers) + { + try + { + await observer.OnWrittenAsync(write, ct).ConfigureAwait(false); + } + catch (Exception ex) + { + LogWriteObserverFailed(_logger, observer.GetType().Name, map.GroupName, ex); + } + } + } + + /// + /// Decides whether a written row is identical to what was already stored, so neither the + /// store nor an observer sees it. Encrypted values are compared as plaintext: AES-GCM draws a fresh /// nonce per call, so re-encrypting an unchanged secret always produces different /// ciphertext and would otherwise look like a change on every single save. /// Anything that cannot be compared with confidence is reported as changed. @@ -258,7 +505,7 @@ private bool IsUnchanged(Setting? previous, bool willEncrypt, string plaintext, } catch (Exception ex) { - LogAuditCompareFailed(_logger, group, key, ex); + LogChangeCompareFailed(_logger, group, key, ex); return false; } } @@ -349,13 +596,18 @@ private async Task FireChangedHandlersAsync(T previous, T current, string gro private static TypeMap GetTypeMap(Type type) => TypeMaps.GetOrAdd(type, t => { - var props = t - .GetProperties(BindingFlags.Public | BindingFlags.Instance) - .Where(p => p.CanRead && p.CanWrite) + // Same rule as SettingSchema.Describe, from the same place: a schema that listed a + // property the engine ignores would be worse than no schema at all. + var props = SettingSchema.StorableProperties(t) .Select(BuildPropertyMap) .ToList(); - return new TypeMap(SettingGroupAttribute.ResolveName(t), props); + // Route falls back to the persistence key for a settings class with no + // [SettingGroup] — it has no endpoint, but a write to it is still addressable. + var route = t.GetCustomAttribute()?.Route; + var name = SettingGroupAttribute.ResolveName(t); + + return new TypeMap(name, string.IsNullOrWhiteSpace(route) ? name : route, props); }); private static PropertyMap BuildPropertyMap(PropertyInfo p) @@ -404,8 +656,19 @@ private static PropertyInfo GetPropertyInfo(Expression> throw new InvalidOperationException($"Selector must point directly at a property on {typeof(T).Name}."); } - private bool TryFromCache(string key, out T? value) - => _cache.TryGetValue(key, out value) && value is not null; + /// + /// Drops the cache entry and the probe that describes it together. Keeping the probe + /// would let a stale "checked recently" outlive the entry it was about. + /// + private void EvictLocally() where T : new() + { + var key = CacheKey(); + _cache.Remove(key); + Probes.TryRemove(key, out _); + } + + private bool TryRowsFromCache(string key, out Setting[]? rows) + => _cache.TryGetValue(key, out rows) && rows is not null; private static string CacheKey() => CacheKey(GetTypeMap(typeof(T))); @@ -423,6 +686,14 @@ private bool TryFromCache(string key, out T? value) Message = "Change handler {handler} threw for settings group '{group}'.")] private static partial void LogChangeHandlerFailed(ILogger logger, string handler, string group, Exception ex); + [LoggerMessage(EventId = 1007, Level = LogLevel.Error, + Message = "Write observer {observer} threw for settings group '{group}'; the write itself succeeded.")] + private static partial void LogWriteObserverFailed(ILogger logger, string observer, string group, Exception ex); + + [LoggerMessage(EventId = 1008, Level = LogLevel.Warning, + Message = "Change signal unreachable for settings group '{group}'; serving the cached copy until it expires.")] + private static partial void LogChangeSignalUnreachable(ILogger logger, string group, Exception ex); + [LoggerMessage(EventId = 1003, Level = LogLevel.Error, Message = "Failed to decrypt setting '{group}.{key}'; using default.")] private static partial void LogDecryptFailed(ILogger logger, string group, string key, Exception ex); @@ -436,10 +707,17 @@ private bool TryFromCache(string key, out T? value) private static partial void LogGroupUnchanged(ILogger logger, string group); [LoggerMessage(EventId = 1005, Level = LogLevel.Warning, - Message = "Cannot decrypt the stored value of '{group}.{key}' to compare it; recording an audit entry anyway.")] - private static partial void LogAuditCompareFailed(ILogger logger, string group, string key, Exception ex); + Message = "Cannot decrypt the stored value of '{group}.{key}' to compare it; treating it as changed.")] + private static partial void LogChangeCompareFailed(ILogger logger, string group, string key, Exception ex); + + [LoggerMessage(EventId = 1009, Level = LogLevel.Error, + Message = "Cannot map the stored state of settings group '{group}'; comparing against defaults instead.")] + private static partial void LogPreviousModelFailed(ILogger logger, string group, Exception ex); + + /// What this instance's cached copy of a group was loaded under, and when it was last re-checked. + private sealed record CacheProbe(string? Version, DateTimeOffset CheckedAt); - private sealed record TypeMap(string GroupName, IReadOnlyList Properties); + private sealed record TypeMap(string GroupName, string Route, IReadOnlyList Properties); private sealed record PropertyMap( string Name, diff --git a/src/DotNetBoost.Settings.Core/Services/SettingWriteObservers.cs b/src/DotNetBoost.Settings.Core/Services/SettingWriteObservers.cs new file mode 100644 index 0000000..7c67c0e --- /dev/null +++ b/src/DotNetBoost.Settings.Core/Services/SettingWriteObservers.cs @@ -0,0 +1,67 @@ +using DotNetBoost.Settings.Core.Interfaces; +using Microsoft.Extensions.Logging; +using System.Globalization; + +namespace DotNetBoost.Settings.Core.Services; + +/// +/// Default : reports no actor. Registered by +/// AddSettings() so the service always resolves. +/// +internal sealed class NullSettingActorAccessor : ISettingActorAccessor +{ + public string? GetActor() => null; +} + +/// +/// An that writes one log line per settings write. +/// Opt in with .UseLoggingWriteObserver(); nothing logs writes by default. +/// +/// +/// Property names only — never values, redacted or not. A settings value is exactly +/// the kind of thing that should not reach a log aggregator because a default turned it on: +/// a connection string, a webhook URL, an account identifier. Which properties changed, who +/// changed them and under what correlation id is the part worth having by default, and it is +/// also the part that is safe to keep. +/// +/// This exists so the write hook is not an empty port. For a real trail — queryable, with +/// retention — implement over your own store. +/// +/// +public sealed partial class LoggingSettingWriteObserver : ISettingWriteObserver +{ + private readonly ILogger _logger; + + /// Creates the observer. + public LoggingSettingWriteObserver(ILogger logger) + => _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + + /// + public Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default) + { + ArgumentNullException.ThrowIfNull(write); + + // Joining the property names allocates, so do not do it for a disabled level. + if (_logger.IsEnabled(LogLevel.Information)) + { + var actor = write.Actor ?? "(not captured)"; + var properties = write.Changes.Count == 0 + ? "(none)" + : string.Join(", ", write.Changes.Select(Describe)); + + LogWrite(_logger, write.GroupName, write.Kind, actor, write.CorrelationId, properties); + } + + return Task.CompletedTask; + } + + private static string Describe(SettingChange change) + => change.IsRedacted + ? string.Create(CultureInfo.InvariantCulture, $"{change.PropertyName} [redacted]") + : change.PropertyName; + + [LoggerMessage(EventId = 1100, Level = LogLevel.Information, + Message = "Settings group '{group}' {kind} by {actor} (correlation {correlationId}); properties: {properties}.")] + private static partial void LogWrite( + ILogger logger, string group, SettingWriteKind kind, string actor, string correlationId, string properties); +} diff --git a/src/DotNetBoost.Settings.Core/SettingInfrastructure.cs b/src/DotNetBoost.Settings.Core/SettingInfrastructure.cs index 2248105..b38952a 100644 --- a/src/DotNetBoost.Settings.Core/SettingInfrastructure.cs +++ b/src/DotNetBoost.Settings.Core/SettingInfrastructure.cs @@ -43,6 +43,18 @@ public SettingBuilder(IServiceCollection services) /// Validates the builder configuration and returns the service collection. /// Call once at the end of the configuration chain. /// + /// + /// Checks the [SettingGroup] classes and the projector registrations made on this + /// builder. It is a guard, not a guarantee: registrations added to the service collection + /// after this call — a test host's ConfigureServices, a plain + /// AddScoped<ISettingProjector<T>, Fake>() later in Program.cs — + /// are not seen here and still win by last-wins, which is what keeps the test-override + /// seam open. + /// + /// + /// No provider is configured, a [SettingGroup] class breaks a naming rule, or a + /// settings group has two different projectors registered. + /// public IServiceCollection Build() { if (!ProviderConfigured) @@ -53,6 +65,7 @@ public IServiceCollection Build() } SettingBuilderValidator.Validate(AppDomain.CurrentDomain.GetAssemblies()); + SettingBuilderValidator.ValidateProjectors(Services); return Services; } } @@ -82,8 +95,9 @@ public static void EnsureProviderNotConfigured(SettingBuilder builder, string pr } /// -/// Startup validation for [SettingGroup] classes: blank routes, blank explicit names, -/// and duplicate group names or routes. Run by . +/// Startup validation for [SettingGroup] classes — blank routes, blank explicit names, +/// duplicate group names or routes — and for projector registrations. Run by +/// . /// public static class SettingBuilderValidator { @@ -109,6 +123,85 @@ public static void Validate(IEnumerable types) Run(tagged!); } + /// + /// Rejects a settings group with two different projectors registered. + /// + /// Projectors do not compose: ProjectAsync takes the group, so a second + /// projector has nowhere to receive the first's output. Two registrations therefore mean + /// the last one silently wins and the other is never even constructed — a response quietly + /// missing half its shape. A group needing both translations and media wants one projector + /// doing the join, not two registrations. + /// + /// + /// The same implementation type registered twice is an idempotent mistake, not an + /// ambiguity, and is collapsed rather than reported — matching the mercy + /// UseSchemaContributor already extends. + /// + /// + /// + /// A group has two different projectors; the message names the group and both. + /// + public static void ValidateProjectors(IServiceCollection services) + { + ArgumentNullException.ThrowIfNull(services); + + var clashes = services + .Where(d => d.ServiceType.IsGenericType + && d.ServiceType.GetGenericTypeDefinition() == typeof(ISettingProjector<>)) + .GroupBy(d => d.ServiceType) + .Select(g => (Group: g.Key.GetGenericArguments()[0], Implementations: Describe(g))) + .Where(x => x.Implementations.Count > 1) + .ToList(); + + if (clashes.Count == 0) return; + + var sb = new StringBuilder( + "More than one ISettingProjector registered for a settings group. A projector takes the " + + "group, so a second one has nowhere to receive the first's output: the last registration " + + "would silently win and the other would never run. Register one projector that does the " + + "whole shaping, or call ReplaceProjector() to override one " + + "deliberately."); + + foreach (var clash in clashes) + { + sb.AppendLine().Append(CultureInfo.InvariantCulture, + $" {clash.Group.FullName}: {string.Join(", ", clash.Implementations)}"); + } + + throw new InvalidOperationException(sb.ToString()); + } + + /// + /// The distinct implementations registered for one group. + /// + /// A descriptor registered by factory or instance has no ImplementationType, so it + /// cannot be compared with anything. Those are counted individually rather than folded + /// together — treating two unknowns as one would silently pass the case this guard most + /// wants to catch. + /// + /// + private static List Describe(IEnumerable descriptors) + { + var named = new HashSet(); + var unknown = new List(); + + foreach (var descriptor in descriptors) + { + if (descriptor.ImplementationType is { } type) + { + named.Add(type); + } + else + { + unknown.Add(descriptor.ImplementationInstance is { } instance + ? $"{instance.GetType().Name} (registered as an instance)" + : "(registered by factory)"); + } + } + + return [.. named.Select(t => t.Name), .. unknown]; + } + private static void Run(List<(Type Type, SettingGroupAttribute Attr)> list) { CheckEmptyRoutes(list); diff --git a/src/DotNetBoost.Settings.Core/SettingOptions.cs b/src/DotNetBoost.Settings.Core/SettingOptions.cs index e42b09d..439cd7d 100644 --- a/src/DotNetBoost.Settings.Core/SettingOptions.cs +++ b/src/DotNetBoost.Settings.Core/SettingOptions.cs @@ -12,6 +12,18 @@ public sealed class SettingOptions /// public TimeSpan CacheDuration { get; set; } = TimeSpan.FromMinutes(10); + /// + /// How often, at most, a cache hit re-checks the ISettingChangeSignal to see + /// whether another instance has written the group. Default: 5 seconds. + /// Set via UseChangeSignal() or SynchronizeThroughDistributedCache(). + /// + /// This is the staleness bound a change signal leaves: an instance can serve values + /// another instance replaced for up to this long. Without a signal registered the check + /// costs nothing and never fires, and is the bound instead. + /// + /// + public TimeSpan ChangeCheckInterval { get; set; } = TimeSpan.FromSeconds(5); + /// /// Whether a [Sensitive] value that cannot be decrypted aborts the read with a /// . Default: true. diff --git a/src/DotNetBoost.Settings.Core/SettingSchema.cs b/src/DotNetBoost.Settings.Core/SettingSchema.cs new file mode 100644 index 0000000..bb57d7c --- /dev/null +++ b/src/DotNetBoost.Settings.Core/SettingSchema.cs @@ -0,0 +1,96 @@ +using DotNetBoost.Settings.Core.Attributes; +using System.Collections.Concurrent; +using System.Reflection; + +namespace DotNetBoost.Settings.Core; + +/// One property of a settings group, as the engine sees it. +/// The property name on the settings class. +/// +/// The property's type name — the underlying type for a . +/// +/// Whether the property is a . +/// +/// The value used when the store holds no row: the [SettingDefault] value when there is +/// one, otherwise whatever a fresh instance of the class initialises the property to. This is +/// a compile-time fact about the class, never anything read from the store. +/// +/// Whether the property carries [Sensitive]. +public sealed record SettingPropertyInfo( + string Name, + string ClrType, + bool IsNullable, + object? DefaultValue, + bool IsSensitive); + +/// +/// Describes the shape of a settings group — the properties the engine will store, their +/// types and their defaults. +/// +/// +/// This is also where the rule for which properties count lives, so the description +/// and the engine cannot drift apart: SettingManager builds its own map from the same +/// . A description that listed a property the engine ignores +/// would be worse than no description. +/// +public static class SettingSchema +{ + private static readonly ConcurrentDictionary> Descriptions = new(); + + /// + /// The properties of that the engine reads and writes: public + /// instance properties with both a getter and a setter. + /// + public static IEnumerable StorableProperties(Type groupType) + { + ArgumentNullException.ThrowIfNull(groupType); + return groupType + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.CanRead && p.CanWrite); + } + + /// + /// Describes every storable property of . Cached per type — + /// it is a fact about the class, not about what is stored. + /// + public static IReadOnlyList Describe(Type groupType) + { + ArgumentNullException.ThrowIfNull(groupType); + return Descriptions.GetOrAdd(groupType, BuildDescription); + } + + private static IReadOnlyList BuildDescription(Type groupType) + { + // One throwaway instance, so a property with no [SettingDefault] can report whatever + // the class initialises it to — which is the value a read actually falls back to. + object? blank = null; + try { blank = Activator.CreateInstance(groupType); } + catch (MissingMethodException) { /* no parameterless ctor: initialiser defaults are unknowable */ } + + return + [ + .. StorableProperties(groupType).Select(p => + { + var underlying = Nullable.GetUnderlyingType(p.PropertyType); + + return new SettingPropertyInfo( + Name: p.Name, + ClrType: (underlying ?? p.PropertyType).Name, + IsNullable: underlying is not null, + DefaultValue: ResolveDefault(p, blank), + IsSensitive: p.GetCustomAttribute() is not null); + }) + ]; + } + + private static object? ResolveDefault(PropertyInfo property, object? blank) + { + if (property.GetCustomAttribute() is { } attribute) + return attribute.Value; + + if (blank is null) return null; + + try { return property.GetValue(blank); } + catch (TargetInvocationException) { return null; } + } +} diff --git a/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs b/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs index 35305bb..cab21cf 100644 --- a/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs +++ b/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs @@ -1,6 +1,8 @@ using DotNetBoost.Settings.Core; using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.Core.Models; using DotNetBoost.Settings.Core.Services; +using Microsoft.Extensions.Caching.Distributed; using Microsoft.Extensions.DependencyInjection.Extensions; namespace Microsoft.Extensions.DependencyInjection @@ -17,6 +19,15 @@ public static SettingBuilder AddSettings(this IServiceCollection services) services.TryAddSingleton(); services.TryAddScoped(); + // Always resolvable, so nothing has to null-check it. Reports no actor until an + // application registers one — null means "not captured", never a claim about who + // acted. ASP.NET applications want .UseHttpContextActor() from the API package. + services.TryAddScoped(); + + // Signals nothing until an application opts in, so a single-instance deployment + // needs no configuration and behaves exactly as it did before signalling existed. + services.TryAddSingleton(); + return new SettingBuilder(services); } } @@ -92,17 +103,289 @@ public static SettingBuilder UseCustomEncryption(this SettingBuilder } } - /// Builder methods for the change-history trail. - public static class AuditBuilderExtensions + /// Builder methods for the write-notification hook. + public static class WriteObserverBuilderExtensions { - /// Plugs in a custom audit store to record full change history. - public static SettingBuilder UseAuditStore(this SettingBuilder builder) - where TAuditStore : class, ISettingAuditStore + /// + /// Registers an observer called once after every completed write to any settings + /// group, with the per-property diff, the actor and a correlation id. + /// + /// Several may be registered; all run, in registration order. An observer's + /// exception is logged and swallowed — see . + /// + /// + public static SettingBuilder UseWriteObserver(this SettingBuilder builder) + where TObserver : class, ISettingWriteObserver { ArgumentNullException.ThrowIfNull(builder); - builder.Services.AddScoped(); + + // Add, not TryAdd: observers compose. A second registration is a second observer, + // not a replacement for the first. + builder.Services.AddScoped(); return builder; } + + /// + /// Registers , which logs one line per write + /// naming the changed properties — never their values. Off unless you ask for it. + /// + public static SettingBuilder UseLoggingWriteObserver(this SettingBuilder builder) + => builder.UseWriteObserver(); + + /// + /// Replaces the default actor accessor, which reports no actor, so + /// carries who made the change. + /// + public static SettingBuilder UseActorAccessor(this SettingBuilder builder) + where TAccessor : class, ISettingActorAccessor + { + ArgumentNullException.ThrowIfNull(builder); + builder.Services.Replace(ServiceDescriptor.Scoped()); + return builder; + } + } + + /// Builder methods for the read-path projection hook. + public static class ProjectionBuilderExtensions + { + /// + /// Registers a projector that shapes for + /// GET /api/settings/{route}. Registered scoped, so a projector may depend on + /// anything else the request scope offers. + /// + /// Read path only, and never applied to the ETag — see + /// . Programmatic reads through + /// ISettingManager are unaffected. + /// + /// + public static SettingBuilder UseProjector(this SettingBuilder builder) + where TSettings : new() + where TProjector : class, ISettingProjector + { + ArgumentNullException.ThrowIfNull(builder); + builder.Services.AddScoped, TProjector>(); + return builder.DeclareProjection(); + } + + /// + /// Replaces whatever projector is registered for with + /// . + /// + /// The deliberate-override path. A shared AddMyCompanySettings() that registers + /// a default projector needs a legible way to be overridden, and registering a second + /// one no longer works for that: Build() rejects two projectors for one group, + /// because the implicit last-wins it used to rely on was indistinguishable from the + /// mistake of registering two by accident. + /// + /// + public static SettingBuilder ReplaceProjector(this SettingBuilder builder) + where TSettings : new() + where TProjector : class, ISettingProjector + { + ArgumentNullException.ThrowIfNull(builder); + builder.Services.Replace(ServiceDescriptor.Scoped, TProjector>()); + return builder.DeclareProjection(); + } + + /// + /// Records what GET returns for , so the + /// generated endpoint can declare it: the projection when + /// implements + /// , otherwise the group itself. + /// + /// + /// Every registration path writes the key, including the untyped case. That is what + /// stops a later ReplaceProjector leaving a stale projection type behind: the + /// last write wins, exactly as the projector registration it accompanies does. + /// + private static SettingBuilder DeclareProjection(this SettingBuilder builder) + where TSettings : new() + where TProjector : class, ISettingProjector + { + var declared = typeof(TProjector).GetInterfaces() + .Where(i => i.IsGenericType + && i.GetGenericTypeDefinition() == typeof(ISettingProjector<,>) + && i.GetGenericArguments()[0] == typeof(TSettings)) + .Select(i => i.GetGenericArguments()[1]) + .ToList(); + + if (declared.Count > 1) + { + // The same ambiguity Build() guards for two projectors, caught at the call + // that causes it. Reported here rather than left to SingleOrDefault, whose + // "sequence contains more than one element" says nothing about projections. + throw new InvalidOperationException( + $"{typeof(TProjector).Name} declares more than one projection type for " + + $"{typeof(TSettings).Name}: {string.Join(", ", declared.Select(t => t.Name))}. " + + "A group's GET has one response shape, so implement ISettingProjector<,> once."); + } + + // The type argument is required: without it the call is ambiguous between + // AddKeyedSingleton(IServiceCollection, Type, object?) and + // AddKeyedSingleton(IServiceCollection, object?, TService), because a + // Type is also a valid key. Do not simplify it away. + builder.Services.AddKeyedSingleton( + typeof(TSettings), + new SettingProjectionDescriptor(declared.Count == 1 ? declared[0] : typeof(TSettings))); + + return builder; + } + } + + /// Builder methods for the schema-constraint hook. + public static class SchemaContributorBuilderExtensions + { + /// + /// Registers a contributor that publishes validation constraints through + /// GET /api/settings/{route}/schema, so a generated form can show the bounds the + /// server will apply instead of hardcoding its own. + /// + /// Contributors compose: every registered type whose CanDescribe returns + /// true is consulted and their constraints merged, so this adds to whatever + /// UseFluentValidation already publishes rather than replacing it. Register one + /// to describe something this library cannot know about — a rule carried by an + /// attribute belonging to another package, say. + /// + /// + /// Order matters on a collision. When two contributors set the same constraint + /// name on the same property, the first registered wins and the clash is + /// logged at Warning. Putting this call before or after + /// UseFluentValidation() is therefore a choice about which one's value survives. + /// + /// + /// + /// Registering the same twice registers it once: + /// a contributor only describes, so a duplicate would describe the same group twice + /// and read as a collision with itself. That is deliberately unlike + /// , where two + /// registrations of one observer are two observers — because an observer does + /// something. + /// + public static SettingBuilder UseSchemaContributor(this SettingBuilder builder) + where TContributor : class, ISettingSchemaContributor + { + ArgumentNullException.ThrowIfNull(builder); + + // TryAddEnumerable with a type descriptor: several *different* contributor types + // all register, which is the point, while a repeat of one type is deduped. + // Transient to match the contributor DotNetBoost.Settings.FluentValidation + // registers, so the two are indistinguishable to a reader. + builder.Services.TryAddEnumerable( + ServiceDescriptor.Transient()); + + return builder; + } + + /// + /// Registers a contributor built by , for one needing + /// something DI cannot construct it with — which a schema contributor often does, since + /// its whole purpose is describing rules this library does not own. + /// + /// See for how + /// contributors compose and which registration wins a collision. + /// + /// + public static SettingBuilder UseSchemaContributor( + this SettingBuilder builder, Func factory) + where TContributor : class, ISettingSchemaContributor + { + ArgumentNullException.ThrowIfNull(builder); + ArgumentNullException.ThrowIfNull(factory); + + // Plain Add, not TryAddEnumerable: two factories are not comparable by + // implementation type, so deduping would silently drop one of them. + builder.Services.Add(ServiceDescriptor.Transient(factory)); + + return builder; + } + } + + /// Builder methods for propagating writes between application instances. + public static class ChangeSignalBuilderExtensions + { + /// + /// Uses to tell every instance that a group changed, + /// re-checked at most every (default: 5 seconds). + /// Registered as a singleton. See . + /// + public static SettingBuilder UseChangeSignal( + this SettingBuilder builder, TimeSpan? checkInterval = null) + where TSignal : class, ISettingChangeSignal + { + ArgumentNullException.ThrowIfNull(builder); + SetCheckInterval(builder, checkInterval); + builder.Services.Replace(ServiceDescriptor.Singleton()); + return builder.PublishChangeSignals(); + } + + /// + /// Uses the signal creates. + /// See . + /// + public static SettingBuilder UseChangeSignal( + this SettingBuilder builder, + Func factory, + TimeSpan? checkInterval = null) + { + ArgumentNullException.ThrowIfNull(builder); + ArgumentNullException.ThrowIfNull(factory); + SetCheckInterval(builder, checkInterval); + builder.Services.Replace(ServiceDescriptor.Singleton(factory)); + return builder.PublishChangeSignals(); + } + + /// + /// Propagates writes between instances through the application's + /// IDistributedCache — Redis, SQL Server, or whatever is already shared. + /// + /// After this, an instance notices another's write within + /// (default: 5 seconds) rather than waiting out its + /// own CacheDuration. + /// + /// + /// The settings builder. + /// + /// Prefixes the entry holding each group's token. Make it distinct per application if + /// several share a cache. + /// + /// How often, at most, a cache hit re-checks the token. + public static SettingBuilder SynchronizeThroughDistributedCache( + this SettingBuilder builder, + string keyPrefix = DistributedCacheSettingChangeSignal.DefaultKeyPrefix, + TimeSpan? checkInterval = null) + { + ArgumentNullException.ThrowIfNull(builder); + ArgumentException.ThrowIfNullOrWhiteSpace(keyPrefix); + + return builder.UseChangeSignal( + sp => new DistributedCacheSettingChangeSignal( + sp.GetService() ?? throw new InvalidOperationException( + "SynchronizeThroughDistributedCache() needs an IDistributedCache that every " + + "instance shares. Register one, e.g. services.AddStackExchangeRedisCache(...)."), + keyPrefix), + checkInterval); + } + + /// + /// Publishes a signal on every write, as an — one + /// notification path rather than two, which is also how ClearAsync comes to + /// signal without a second call site. Idempotent; the UseChangeSignal overloads + /// call it for you. + /// + private static SettingBuilder PublishChangeSignals(this SettingBuilder builder) + { + builder.Services.TryAddEnumerable( + ServiceDescriptor.Scoped()); + return builder; + } + + private static void SetCheckInterval(SettingBuilder builder, TimeSpan? checkInterval) + { + if (checkInterval is not { } interval) return; + + ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(interval, TimeSpan.Zero, nameof(checkInterval)); + builder.Options.ChangeCheckInterval = interval; + } } /// Builder methods for runtime change notifications. diff --git a/src/DotNetBoost.Settings.Dapper/Dapper.cs b/src/DotNetBoost.Settings.Dapper/Dapper.cs index 74c1c44..3b835fd 100644 --- a/src/DotNetBoost.Settings.Dapper/Dapper.cs +++ b/src/DotNetBoost.Settings.Dapper/Dapper.cs @@ -66,19 +66,6 @@ RowVersion VARBINARY(16) NULL ); CREATE UNIQUE INDEX UX_Settings_Group_Key ON Settings(SettingGroup, SettingKey); END; - IF OBJECT_ID('SettingAudits','U') IS NULL - BEGIN - CREATE TABLE SettingAudits ( - Id UNIQUEIDENTIFIER PRIMARY KEY, - SettingGroup NVARCHAR(191) NOT NULL, - SettingKey NVARCHAR(191) NOT NULL, - OldValue NVARCHAR(MAX) NOT NULL, - NewValue NVARCHAR(MAX) NOT NULL, - ChangedBy NVARCHAR(256) NOT NULL, - ChangedAt DATETIME2 NOT NULL - ); - CREATE INDEX IX_SettingAudits_Group_Key ON SettingAudits(SettingGroup, SettingKey); - END; """; private const string PostgreSqlScript = """ @@ -94,15 +81,6 @@ UpdatedBy varchar(256), RowVersion bytea ); CREATE UNIQUE INDEX IF NOT EXISTS UX_Settings_Group_Key ON Settings(SettingGroup,SettingKey); - CREATE TABLE IF NOT EXISTS SettingAudits ( - Id uuid PRIMARY KEY, - SettingGroup varchar(191) NOT NULL, - SettingKey varchar(191) NOT NULL, - OldValue text NOT NULL, - NewValue text NOT NULL, - ChangedBy varchar(256) NOT NULL, - ChangedAt timestamp NOT NULL - ); """; private const string SqliteScript = """ @@ -118,15 +96,6 @@ CREATE TABLE IF NOT EXISTS Settings ( RowVersion BLOB ); CREATE UNIQUE INDEX IF NOT EXISTS UX_Settings_Group_Key ON Settings(SettingGroup, SettingKey); - CREATE TABLE IF NOT EXISTS SettingAudits ( - Id TEXT NOT NULL PRIMARY KEY, - SettingGroup TEXT NOT NULL, - SettingKey TEXT NOT NULL, - OldValue TEXT NOT NULL, - NewValue TEXT NOT NULL, - ChangedBy TEXT NOT NULL, - ChangedAt TEXT NOT NULL - ); """; } diff --git a/src/DotNetBoost.Settings.Dapper/DapperServiceCollectionExtensions.cs b/src/DotNetBoost.Settings.Dapper/DapperServiceCollectionExtensions.cs index 9b52257..5da9619 100644 --- a/src/DotNetBoost.Settings.Dapper/DapperServiceCollectionExtensions.cs +++ b/src/DotNetBoost.Settings.Dapper/DapperServiceCollectionExtensions.cs @@ -15,8 +15,8 @@ public static class DapperBuilderExtensions /// The settings builder. /// Creates the connection for each scope. /// - /// When true, a hosted service creates the Settings and SettingAudits - /// tables at startup if they are absent. Leave it off if schema is managed elsewhere. + /// When true, a hosted service creates the Settings table at startup if it is + /// absent. Leave it off if schema is managed elsewhere. /// public static SettingBuilder UseDapper( this SettingBuilder builder, diff --git a/src/DotNetBoost.Settings.EntityFrameworkCore/EfCore.cs b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCore.cs index 418264d..647e2ba 100644 --- a/src/DotNetBoost.Settings.EntityFrameworkCore/EfCore.cs +++ b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCore.cs @@ -22,15 +22,22 @@ public enum DatabaseProvider } /// -/// Implement this on your to let the EF Core store use it. +/// No longer required. The EF Core store reads its entities through +/// , so a context needs neither this interface nor the +/// DbSet properties it declares — ApplySettings in OnModelCreating is the +/// whole registration. /// +/// +/// Kept for one release so existing contexts keep compiling. Delete the +/// : ISettingDbContext and the DbSet property; nothing else changes. +/// +[Obsolete("ISettingDbContext is no longer used. Remove it and the DbSet properties from your " + + "DbContext; UseEntityFrameworkCore() only needs a DbContext. " + + "This interface will be removed in the next release.")] public interface ISettingDbContext { /// The persisted settings rows. - DbSet Settings { get; } - - /// The change-history rows. - DbSet SettingAudits { get; } + DbSet Settings { get; } /// Persists pending changes. Task SaveChangesAsync(CancellationToken cancellationToken = default); @@ -76,49 +83,97 @@ public void Configure(EntityTypeBuilder builder) } } -/// Entity mapping for . -public sealed class SettingAuditConfiguration : IEntityTypeConfiguration -{ - /// - public void Configure(EntityTypeBuilder builder) - { - builder.ToTable("SettingAudits"); - builder.HasKey(x => x.Id); - builder.Property(x => x.Id).ValueGeneratedNever(); - builder.Property(x => x.Group).HasColumnName("SettingGroup").HasMaxLength(191).IsRequired(); - builder.Property(x => x.Key).HasColumnName("SettingKey").HasMaxLength(191).IsRequired(); - builder.Property(x => x.ChangedBy).HasMaxLength(256).IsRequired(); - builder.Property(x => x.ChangedAt).IsRequired(); - builder.HasIndex(x => new { x.Group, x.Key }); - } -} - /// Model-building helpers for wiring the settings entities into a DbContext. public static class ModelBuilderExtensions { /// - /// Applies both settings entity configurations. Call from OnModelCreating. + /// Applies the settings entity configuration. Call from OnModelCreating. /// /// The model builder. /// The engine being targeted. - public static ModelBuilder ApplySettingsConfiguration(this ModelBuilder mb, DatabaseProvider provider) + public static ModelBuilder ApplySettings(this ModelBuilder mb, DatabaseProvider provider) { ArgumentNullException.ThrowIfNull(mb); mb.ApplyConfiguration(new SettingConfiguration(provider)); - mb.ApplyConfiguration(new SettingAuditConfiguration()); return mb; } + + /// + /// Applies the settings entity configuration, taking the engine from + /// rather than being told it. Call from + /// OnModelCreating as modelBuilder.ApplySettings(this). + /// + /// The explicit overload stays for anyone who needs to override the inference — a + /// provider this package has no mapping for, or a model built against one engine and + /// migrated onto another. + /// + /// + /// The model builder. + /// The context being configured; usually this. + /// + /// The context's EF Core provider is not one this package ships column mappings for. + /// Use the overload and name + /// the closest engine. + /// + public static ModelBuilder ApplySettings(this ModelBuilder mb, DbContext context) + { + ArgumentNullException.ThrowIfNull(context); + return mb.ApplySettings(ResolveProvider(context.Database.ProviderName)); + } + + /// + /// Renamed to . + /// + /// + /// Kept for one release so existing contexts keep compiling, the same way + /// is. The old name described the mechanism rather than + /// the library, which read as meaningful next to the ApplyMedia() and + /// ApplyMultiLingual() calls it usually sits beside. It was not. + /// + [Obsolete(RenamedMessage)] + public static ModelBuilder ApplySettingsConfiguration(this ModelBuilder mb, DatabaseProvider provider) + => mb.ApplySettings(provider); + + private const string RenamedMessage = + "Renamed to ApplySettings, to match ApplyMedia and ApplyMultiLingual. " + + "This alias will be removed in the next release."; + + /// + /// Maps an EF Core provider assembly name onto the engine whose column types and + /// concurrency-token style the settings tables need. + /// + private static DatabaseProvider ResolveProvider(string? providerName) => providerName switch + { + "Microsoft.EntityFrameworkCore.SqlServer" => DatabaseProvider.SqlServer, + "Npgsql.EntityFrameworkCore.PostgreSQL" => DatabaseProvider.PostgreSql, + "Microsoft.EntityFrameworkCore.Sqlite" => DatabaseProvider.Sqlite, + + // Named deliberately rather than guessed at: picking the wrong one silently gives the + // Value column a type the engine cannot hold, which shows up as truncated settings + // long after the migration ran. + _ => throw new InvalidOperationException( + $"Cannot infer a settings DatabaseProvider from EF Core provider '{providerName ?? "(none)"}'. " + + "Supported: SQL Server, PostgreSQL and SQLite. Call " + + "ApplySettings(DatabaseProvider) with the engine to target instead.") + }; } /// EF Core-backed . -/// The context holding the settings entities. -public sealed class EfCoreSettingStore(ISettingDbContext db) : ISettingStore +/// +/// Takes a plain and reaches its entities through +/// , so a consuming context needs no interface and no +/// DbSet properties — only ApplySettings in OnModelCreating. +/// +/// The context the settings entities are mapped on. +public sealed class EfCoreSettingStore(DbContext db) : ISettingStore { + private DbSet Settings => db.Set(); + /// public async Task> GetGroupAsync(string group, CancellationToken ct = default) { ArgumentException.ThrowIfNullOrWhiteSpace(group); - return await db.Settings.AsNoTracking().Where(x => x.Group == group).ToListAsync(ct).ConfigureAwait(false); + return await Settings.AsNoTracking().Where(x => x.Group == group).ToListAsync(ct).ConfigureAwait(false); } /// @@ -126,7 +181,7 @@ public async Task> GetGroupAsync(string group, Cancellati { ArgumentException.ThrowIfNullOrWhiteSpace(group); ArgumentException.ThrowIfNullOrWhiteSpace(key); - return db.Settings.AsNoTracking().FirstOrDefaultAsync(x => x.Group == group && x.Key == key, ct); + return Settings.AsNoTracking().FirstOrDefaultAsync(x => x.Group == group && x.Key == key, ct); } /// @@ -145,7 +200,7 @@ public async Task UpsertManyAsync(IEnumerable settings, CancellationTok var group = list[0].Group; var keys = list.Select(x => x.Key).ToHashSet(StringComparer.Ordinal); - var existing = await db.Settings.Where(x => x.Group == group && keys.Contains(x.Key)).ToListAsync(ct).ConfigureAwait(false); + var existing = await Settings.Where(x => x.Group == group && keys.Contains(x.Key)).ToListAsync(ct).ConfigureAwait(false); var byKey = existing.ToDictionary(x => x.Key, StringComparer.Ordinal); foreach (var item in list) @@ -168,7 +223,7 @@ public async Task UpsertManyAsync(IEnumerable settings, CancellationTok else { item.RowVersion = Setting.NewRowVersion(); - await db.Settings.AddAsync(item, ct).ConfigureAwait(false); + await Settings.AddAsync(item, ct).ConfigureAwait(false); } } @@ -188,9 +243,9 @@ public async Task DeleteAsync(string group, string key, CancellationToken ct = d { ArgumentException.ThrowIfNullOrWhiteSpace(group); ArgumentException.ThrowIfNullOrWhiteSpace(key); - var row = await db.Settings.FirstOrDefaultAsync(x => x.Group == group && x.Key == key, ct).ConfigureAwait(false); + var row = await Settings.FirstOrDefaultAsync(x => x.Group == group && x.Key == key, ct).ConfigureAwait(false); if (row is null) return; - db.Settings.Remove(row); + Settings.Remove(row); await db.SaveChangesAsync(ct).ConfigureAwait(false); } @@ -198,7 +253,7 @@ public async Task DeleteAsync(string group, string key, CancellationToken ct = d public Task DeleteGroupAsync(string group, CancellationToken ct = default) { ArgumentException.ThrowIfNullOrWhiteSpace(group); - return db.Settings.Where(x => x.Group == group).ExecuteDeleteAsync(ct); + return Settings.Where(x => x.Group == group).ExecuteDeleteAsync(ct); } @@ -206,27 +261,6 @@ public Task DeleteGroupAsync(string group, CancellationToken ct = default) public Task CountAsync(string group, CancellationToken ct = default) { ArgumentException.ThrowIfNullOrWhiteSpace(group); - return db.Settings.CountAsync(x => x.Group == group, ct); - } -} - -/// EF Core-backed audit store. Registered via .UseAuditStore<EfCoreAuditStore>(). -public sealed class EfCoreAuditStore(ISettingDbContext db) : ISettingAuditStore -{ - /// - public async Task RecordAsync(SettingAuditEntry entry, CancellationToken ct = default) - { - ArgumentNullException.ThrowIfNull(entry); - await db.SettingAudits.AddAsync(entry, ct).ConfigureAwait(false); - await db.SaveChangesAsync(ct).ConfigureAwait(false); - } - - /// - public async Task> GetHistoryAsync(string group, string? key = null, CancellationToken ct = default) - { - ArgumentException.ThrowIfNullOrWhiteSpace(group); - var q = db.SettingAudits.AsNoTracking().Where(x => x.Group == group); - if (key is not null) q = q.Where(x => x.Key == key); - return await q.OrderByDescending(x => x.ChangedAt).ToListAsync(ct).ConfigureAwait(false); + return Settings.CountAsync(x => x.Group == group, ct); } } diff --git a/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreServiceCollectionExtensions.cs b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreServiceCollectionExtensions.cs index a59df2e..dd46910 100644 --- a/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreServiceCollectionExtensions.cs +++ b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreServiceCollectionExtensions.cs @@ -8,13 +8,28 @@ namespace Microsoft.Extensions.DependencyInjection /// Builder methods for the EF Core storage provider. public static class EntityFrameworkBuilderExtensions { - /// Configures Entity Framework Core as the settings provider. + /// + /// Configures Entity Framework Core as the settings provider. + /// + /// only has to be a that calls + /// ApplySettings in OnModelCreating. It needs no interface + /// and no DbSet properties: the store reads its entities through + /// . + /// + /// + /// + /// Registers nothing against itself. The store is constructed + /// from explicitly, so the library never claims a + /// framework type it does not own — which in an application with two contexts would + /// silently hand a bare DbContext dependency whichever one Settings was + /// pointed at. + /// public static SettingBuilder UseEntityFrameworkCore(this SettingBuilder builder) - where TContext : DbContext, ISettingDbContext + where TContext : DbContext { SettingBuilderGuard.EnsureProviderNotConfigured(builder, "EntityFrameworkCore"); - builder.Services.AddScoped(sp => sp.GetRequiredService()); - builder.Services.AddScoped(); + builder.Services.AddScoped( + sp => new EfCoreSettingStore(sp.GetRequiredService())); return builder; } } diff --git a/src/DotNetBoost.Settings.FluentValidation/FluentValidationSchemaContributor.cs b/src/DotNetBoost.Settings.FluentValidation/FluentValidationSchemaContributor.cs new file mode 100644 index 0000000..d4b86f0 --- /dev/null +++ b/src/DotNetBoost.Settings.FluentValidation/FluentValidationSchemaContributor.cs @@ -0,0 +1,115 @@ +using DotNetBoost.Settings.Core.Interfaces; +using FluentValidation; +using FluentValidation.Validators; + +namespace DotNetBoost.Settings.FluentValidation; + +/// +/// Publishes the rules of a registered FluentValidation validator through +/// , so a generated form can enforce the same bounds +/// the server does instead of hardcoding its own. +/// +/// +/// Reads FluentValidation's own IValidatorDescriptor. Rules it does not recognise are +/// skipped rather than guessed at: a wrong published bound is worse than a missing one, +/// because a client would believe it. A rule expressed as Must(...) is opaque by +/// nature and will never appear here. +/// +/// Resolves the IValidator<T> for a settings type. +public sealed class FluentValidationSchemaContributor(IServiceProvider serviceProvider) + : ISettingSchemaContributor +{ + /// + public bool CanDescribe(Type type) => Resolve(type) is not null; + + /// + public IReadOnlyDictionary> Describe(Type type) + { + var validator = Resolve(type); + if (validator is null) return new Dictionary>(); + + var result = new Dictionary>(StringComparer.Ordinal); + + foreach (var member in validator.CreateDescriptor().GetMembersWithValidators()) + { + var constraints = new Dictionary(StringComparer.Ordinal); + + foreach (var (propertyValidator, _) in member) + Collect(propertyValidator, constraints); + + if (constraints.Count > 0) result[member.Key] = constraints; + } + + return result; + } + + private IValidator? Resolve(Type type) + { + ArgumentNullException.ThrowIfNull(type); + return serviceProvider.GetService(typeof(IValidator<>).MakeGenericType(type)) as IValidator; + } + + private static void Collect(IPropertyValidator validator, Dictionary into) + { + switch (validator) + { + case IBetweenValidator between: + into["min"] = between.From; + into["max"] = between.To; + break; + + // Order matters: the exact/minimum/maximum validators all implement + // ILengthValidator, and an exact length is both bounds at once. + case IExactLengthValidator exact: + into["minLength"] = exact.Min; + into["maxLength"] = exact.Max; + break; + + case IMaximumLengthValidator maximum: + into["maxLength"] = maximum.Max; + break; + + case IMinimumLengthValidator minimum: + into["minLength"] = minimum.Min; + break; + + case ILengthValidator length: + into["minLength"] = length.Min; + if (length.Max > 0) into["maxLength"] = length.Max; + break; + + case IRegularExpressionValidator regex: + into["pattern"] = regex.Expression; + break; + + case INotEmptyValidator or INotNullValidator: + into["required"] = true; + break; + + case IComparisonValidator comparison: + CollectComparison(comparison, into); + break; + + default: + // Deliberately silent. A Must(...) or a custom validator cannot be described, + // and inventing a bound for it would publish a lie. + break; + } + } + + private static void CollectComparison(IComparisonValidator comparison, Dictionary into) + { + // Only a literal comparison can be published: ValueToCompare is null when the rule + // compares against another property or a lambda, and there is nothing to report then. + if (comparison.ValueToCompare is not { } value) return; + + switch (comparison.Comparison) + { + case Comparison.GreaterThanOrEqual: into["min"] = value; break; + case Comparison.LessThanOrEqual: into["max"] = value; break; + case Comparison.GreaterThan: into["exclusiveMin"] = value; break; + case Comparison.LessThan: into["exclusiveMax"] = value; break; + default: break; // Equal / NotEqual are not bounds. + } + } +} diff --git a/src/DotNetBoost.Settings.FluentValidation/FluentValidationServiceCollectionExtensions.cs b/src/DotNetBoost.Settings.FluentValidation/FluentValidationServiceCollectionExtensions.cs index 782dbb0..b68f3f6 100644 --- a/src/DotNetBoost.Settings.FluentValidation/FluentValidationServiceCollectionExtensions.cs +++ b/src/DotNetBoost.Settings.FluentValidation/FluentValidationServiceCollectionExtensions.cs @@ -2,6 +2,7 @@ using DotNetBoost.Settings.Core; using DotNetBoost.Settings.FluentValidation; using FluentValidation; +using Microsoft.Extensions.DependencyInjection.Extensions; using System.Reflection; namespace Microsoft.Extensions.DependencyInjection @@ -12,6 +13,11 @@ public static class ValidationBuilderExtensions /// /// Scans for all AbstractValidator<T> /// implementations and registers them as . + /// + /// Also registers an , so the rules those + /// validators enforce are published by GET /api/settings/{route}/schema rather + /// than being reinvented by every client. + /// /// public static SettingBuilder UseFluentValidation(this SettingBuilder builder, Assembly assembly) { @@ -44,6 +50,11 @@ private static void RegisterValidators(IServiceCollection services, Assembly ass .Where(x => x.ServiceType is not null) .ToList(); + // One contributor covers every settings type: it resolves the validator per type + // when asked, so it is registered once rather than per validator found. + services.TryAddEnumerable( + ServiceDescriptor.Transient()); + foreach (var item in pairs) { var settingsType = item.ServiceType!.GetGenericArguments()[0]; diff --git a/tests/DotNetBoost.Settings.ApiTests/AuditEndpointTests.cs b/tests/DotNetBoost.Settings.ApiTests/AuditEndpointTests.cs deleted file mode 100644 index 0005a2f..0000000 --- a/tests/DotNetBoost.Settings.ApiTests/AuditEndpointTests.cs +++ /dev/null @@ -1,91 +0,0 @@ -using System.Net; -using DotNetBoost.Settings.Core.Attributes; -using DotNetBoost.Settings.Core.Interfaces; -using Microsoft.Extensions.DependencyInjection; - -namespace DotNetBoost.Settings.ApiTests; - -public class AuditEndpointTests -{ - /// - /// ISettingAuditStore is optional. Bound as a handler parameter it was inferred as a - /// *body* parameter when unregistered, which throws while the endpoint is being built - /// and takes the whole application down at startup — long before any request arrives. - /// - [Fact] - public async Task Application_Starts_WhenNoAuditStoreIsRegistered() - { - await using var app = await TestApp.StartAsync(); - - var response = await app.Client.GetAsync("/api/settings/api-test"); - - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - } - - [Fact] - public async Task AuditEndpoint_Returns404_WhenNoAuditStoreIsRegistered() - { - await using var app = await TestApp.StartAsync(); - - var response = await app.Client.GetAsync("/api/settings/api-test/audit"); - - Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); - Assert.Contains("Audit store is not configured", await response.Content.ReadAsStringAsync(), StringComparison.Ordinal); - } - - [Fact] - public async Task AuditEndpoint_ReturnsHistory_WhenAuditStoreIsRegistered() - { - var audit = new RecordingAuditStore(); - await using var app = await TestApp.StartAsync(s => s.AddScoped(_ => audit)); - - var response = await app.Client.GetAsync("/api/settings/api-test/audit"); - - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.Contains("after", await response.Content.ReadAsStringAsync(), StringComparison.Ordinal); - } - - /// - /// UseAuditStore<T>() registers the store as scoped, so the endpoint must resolve it - /// from the request scope. Resolving from the root provider would throw at runtime. - /// - [Fact] - public async Task AuditEndpoint_ResolvesScopedAuditStore() - { - await using var app = await TestApp.StartAsync(s => s.AddScoped()); - - var response = await app.Client.GetAsync("/api/settings/api-test/audit"); - - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - } - - [Fact] - public async Task AuditEndpoint_QueriesTheResolvedGroupName_NotTheClassName() - { - var audit = new RecordingAuditStore(); - await using var app = await TestApp.StartAsync(s => s.AddScoped(_ => audit)); - - await app.Client.GetAsync("/api/settings/api-test/audit"); - - var (group, _) = Assert.Single(audit.Queries); - Assert.Equal("api-test-group", group); - Assert.NotEqual(nameof(ApiTestSettings), group); - } - - [Fact] - public async Task AuditEndpoint_ForwardsTheKeyFilter() - { - var audit = new RecordingAuditStore(); - await using var app = await TestApp.StartAsync(s => s.AddScoped(_ => audit)); - - await app.Client.GetAsync("/api/settings/api-test/audit?key=Host"); - - Assert.Equal("Host", Assert.Single(audit.Queries).Key); - } -} - -[SettingGroup("api-test", Name = "api-test-group")] -public class ApiTestSettings -{ - public string Host { get; set; } = string.Empty; -} diff --git a/tests/DotNetBoost.Settings.ApiTests/DiscoveryEndpointTests.cs b/tests/DotNetBoost.Settings.ApiTests/DiscoveryEndpointTests.cs new file mode 100644 index 0000000..f33d763 --- /dev/null +++ b/tests/DotNetBoost.Settings.ApiTests/DiscoveryEndpointTests.cs @@ -0,0 +1,546 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using DotNetBoost.Settings.Core.Attributes; +using DotNetBoost.Settings.Core.Interfaces; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Routing; +using FluentValidation; +using Microsoft.Extensions.DependencyInjection; + +namespace DotNetBoost.Settings.ApiTests; + +/// +/// Nothing used to enumerate the [SettingGroup] classes or describe a group's +/// properties, so a "form generated from the class" was really generated by guessing at the +/// shape of a JSON response — and the bounds it enforced lived in the client, unenforced on +/// the server and reinvented by every new client. +/// +/// Neither endpoint returns a stored value; several of these tests exist to keep it that way. +/// +/// +public class DiscoveryEndpointTests +{ + [Fact] + public async Task GroupList_ContainsEveryRegisteredGroup() + { + await using var app = await TestApp.StartAsync(); + + var groups = await app.Client.GetFromJsonAsync>("/api/settings"); + + var routes = groups!.Select(g => g.GetProperty("route").GetString()).ToList(); + Assert.Contains("api-test", routes); + Assert.Contains("api-projected", routes); + Assert.Contains("api-secured", routes); + } + + [Fact] + public async Task GroupList_ReportsThePersistenceKey_NotTheClassName() + { + await using var app = await TestApp.StartAsync(); + + var groups = await app.Client.GetFromJsonAsync>("/api/settings"); + + var apiTest = groups!.Single(g => g.GetProperty("route").GetString() == "api-test"); + Assert.Equal("api-test-group", apiTest.GetProperty("name").GetString()); + Assert.NotEqual(nameof(ApiTestSettings), apiTest.GetProperty("name").GetString()); + } + + [Fact] + public async Task GroupList_SaysWhichGroupsRequireAuthorization() + { + await using var app = await TestApp.StartAsync(); + + var groups = await app.Client.GetFromJsonAsync>("/api/settings"); + + Assert.True(Find(groups!, "api-secured").GetProperty("requiresAuthorization").GetBoolean()); + Assert.False(Find(groups!, "api-test").GetProperty("requiresAuthorization").GetBoolean()); + } + + [Fact] + public async Task GroupList_ReturnsNoValues() + { + await using var app = await TestApp.StartAsync(); + + // The stub store seeds api-test-group's Host. It must not appear anywhere in a listing. + var body = await app.Client.GetStringAsync("/api/settings"); + + Assert.DoesNotContain("stored.example.com", body, StringComparison.Ordinal); + Assert.DoesNotContain("host", body, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task GroupList_CanBeTurnedOff() + { + await using var app = await TestApp.StartAsync(includeDiscovery: false); + + var response = await app.Client.GetAsync("/api/settings"); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task Schema_NamesPropertiesTheWayTheGetBodyDoes() + { + await using var app = await TestApp.StartAsync(); + + var schema = await app.Client.GetFromJsonAsync("/api/settings/api-described/schema"); + var body = await app.Client.GetFromJsonAsync("/api/settings/api-described"); + + var schemaNames = schema.GetProperty("properties") + .EnumerateArray() + .Select(p => p.GetProperty("name").GetString()!) + .OrderBy(n => n, StringComparer.Ordinal) + .ToList(); + + var bodyNames = body.EnumerateObject() + .Select(p => p.Name) + .OrderBy(n => n, StringComparer.Ordinal) + .ToList(); + + Assert.Equal(bodyNames, schemaNames); + } + + [Fact] + public async Task Schema_ReportsTypes() + { + await using var app = await TestApp.StartAsync(); + + var properties = await PropertiesOf(app, "api-described"); + + Assert.Equal("String", properties["host"].GetProperty("type").GetString()); + Assert.Equal("Int32", properties["port"].GetProperty("type").GetString()); + Assert.Equal("Boolean", properties["useSsl"].GetProperty("type").GetString()); + } + + [Fact] + public async Task Schema_ReportsTheSettingDefault_WhenThereIsOne() + { + await using var app = await TestApp.StartAsync(); + + var properties = await PropertiesOf(app, "api-described"); + + Assert.Equal(2525, properties["port"].GetProperty("default").GetInt32()); + } + + [Fact] + public async Task Schema_FallsBackToTheClassInitialiser_WithoutASettingDefault() + { + await using var app = await TestApp.StartAsync(); + + var properties = await PropertiesOf(app, "api-described"); + + Assert.Equal("smtp.example.com", properties["host"].GetProperty("default").GetString()); + Assert.True(properties["useSsl"].GetProperty("default").GetBoolean()); + } + + /// + /// A compile-time default on a secret property is a value like any other, and this + /// endpoint is meant to be safe to expose more widely than values are. + /// + [Fact] + public async Task Schema_MarksSensitiveProperties_AndWithholdsTheirDefault() + { + await using var app = await TestApp.StartAsync(); + + var properties = await PropertiesOf(app, "api-described"); + + Assert.True(properties["password"].GetProperty("sensitive").GetBoolean()); + Assert.Equal(JsonValueKind.Null, properties["password"].GetProperty("default").ValueKind); + Assert.False(properties["host"].GetProperty("sensitive").GetBoolean()); + } + + [Fact] + public async Task Schema_ReturnsNoStoredValues() + { + await using var app = await TestApp.StartAsync(); + + // api-test-group has a stored Host of "stored.example.com". + var body = await app.Client.GetStringAsync("/api/settings/api-test/schema"); + + Assert.DoesNotContain("stored.example.com", body, StringComparison.Ordinal); + } + + /// + /// Asserted on the endpoint metadata rather than on a status code: the test host has no + /// authentication middleware, so an authorized request throws instead of answering 401. + /// The metadata is the thing that matters anyway — a group's endpoints must not disagree + /// about who may look at it. + /// + [Fact] + public async Task Schema_CarriesTheSameAuthorization_AsTheGroupsOtherEndpoints() + { + await using var app = await TestApp.StartAsync(); + + Assert.True(IsAuthorized(app, "api/settings/api-secured/")); + Assert.True(IsAuthorized(app, "api/settings/api-secured/schema")); + + // And an anonymous group's schema stays anonymous. + Assert.False(IsAuthorized(app, "api/settings/api-described/schema")); + } + + /// + /// Whether every endpoint on is authorized. A pattern can + /// carry more than one method — GET and POST share the group's root — and they must agree. + /// + private static bool IsAuthorized(TestApp app, string routePattern) + { + var endpoints = app.Services.GetRequiredService() + .Endpoints + .OfType() + .Where(e => e.RoutePattern.RawText == routePattern) + .ToList(); + + Assert.NotEmpty(endpoints); + return endpoints.TrueForAll(e => e.Metadata.GetMetadata() is not null); + } + + [Fact] + public async Task Schema_CanBeTurnedOff() + { + await using var app = await TestApp.StartAsync(includeDiscovery: false); + + var response = await app.Client.GetAsync("/api/settings/api-described/schema"); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task Schema_WithNoContributor_IsValidAndCarriesNoConstraints() + { + await using var app = await TestApp.StartAsync(); + + var properties = await PropertiesOf(app, "api-described"); + + Assert.Equal(JsonValueKind.Null, properties["port"].GetProperty("constraints").ValueKind); + } + + [Fact] + public async Task Schema_WithAContributor_PublishesItsConstraints() + { + await using var app = await TestApp.StartAsync( + s => s.AddSingleton()); + + var properties = await PropertiesOf(app, "api-described"); + + var port = properties["port"].GetProperty("constraints"); + Assert.Equal(1, port.GetProperty("min").GetInt32()); + Assert.Equal(65_535, port.GetProperty("max").GetInt32()); + + // A property the contributor said nothing about still reports none. + Assert.Equal(JsonValueKind.Null, properties["useSsl"].GetProperty("constraints").ValueKind); + } + + /// + /// A schema is a convenience. A contributor that cannot describe itself should cost the + /// constraints, not the endpoint. + /// + [Fact] + public async Task Schema_SurvivesAContributorThatThrows() + { + await using var app = await TestApp.StartAsync( + s => s.AddSingleton()); + + var response = await app.Client.GetAsync("/api/settings/api-described/schema"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var properties = await PropertiesOf(app, "api-described"); + Assert.Equal(JsonValueKind.Null, properties["port"].GetProperty("constraints").ValueKind); + } + + /// + /// The case this merge exists for. A consumer publishing something this library cannot + /// know about — a rule carried by an attribute from another package — must not silently + /// delete the validation rules by registering alongside them. + /// + [Fact] + public async Task Schema_MergesContributorsDescribingDifferentProperties() + { + await using var app = await TestApp.StartAsync(s => + { + s.AddSingleton(); + s.AddSingleton(_ => new TranslatedContributor(nameof(DescribedSettings.Host))); + }); + + var properties = await PropertiesOf(app, "api-described"); + + Assert.Equal(1, properties["port"].GetProperty("constraints").GetProperty("min").GetInt32()); + Assert.True(properties["host"].GetProperty("constraints").GetProperty("translated").GetBoolean()); + } + + [Fact] + public async Task Schema_MergesContributorsDescribingTheSameProperty() + { + await using var app = await TestApp.StartAsync(s => + { + s.AddSingleton(); + s.AddSingleton(_ => new TranslatedContributor(nameof(DescribedSettings.Port))); + }); + + var port = (await PropertiesOf(app, "api-described"))["port"].GetProperty("constraints"); + + // Both contributors' constraints survive on the one property. + Assert.Equal(1, port.GetProperty("min").GetInt32()); + Assert.Equal(65_535, port.GetProperty("max").GetInt32()); + Assert.True(port.GetProperty("translated").GetBoolean()); + } + + /// + /// Last-wins would make the published schema depend on registration order, which is the + /// defect being fixed — so the value is asserted, not merely that one of them landed. + /// + [Fact] + public async Task Schema_OnACollidingConstraint_KeepsTheFirstRegistered() + { + await using var app = await TestApp.StartAsync(s => + { + s.AddSingleton(); // min = 1 + s.AddSingleton(_ => new MinOverridingContributor(99)); + }); + + var port = (await PropertiesOf(app, "api-described"))["port"].GetProperty("constraints"); + + Assert.Equal(1, port.GetProperty("min").GetInt32()); + + // The loser's other constraints still land — only the colliding key is dropped. + Assert.Equal(7, port.GetProperty("maxLength").GetInt32()); + } + + /// + /// The case that justifies wrapping each contributor rather than the loop: it is what + /// silently regresses if someone later hoists the try/catch outward. + /// + [Fact] + public async Task Schema_AContributorThatThrows_DoesNotCostTheOthersTheirConstraints() + { + await using var app = await TestApp.StartAsync(s => + { + s.AddSingleton(); + s.AddSingleton(); + }); + + var response = await app.Client.GetAsync("/api/settings/api-described/schema"); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var port = (await PropertiesOf(app, "api-described"))["port"].GetProperty("constraints"); + Assert.Equal(1, port.GetProperty("min").GetInt32()); + Assert.Equal(65_535, port.GetProperty("max").GetInt32()); + } + + [Fact] + public async Task Schema_DoesNotConsultAContributorThatCannotDescribeTheType() + { + var unrelated = new RecordingContributor(canDescribe: false); + var applies = new RecordingContributor(canDescribe: true); + + await using var app = await TestApp.StartAsync(s => + { + s.AddSingleton(_ => unrelated); + s.AddSingleton(_ => applies); + }); + + await app.Client.GetAsync("/api/settings/api-described/schema"); + + Assert.Equal(0, unrelated.DescribeCalls); + Assert.True(applies.DescribeCalls > 0); + } + + // ---------------------------------------------------------------- UseSchemaContributor + + [Fact] + public async Task UseSchemaContributor_PublishesItsConstraints() + { + await using var app = await TestApp.StartAsync( + configureSettings: b => b.UseSchemaContributor()); + + var port = (await PropertiesOf(app, "api-described"))["port"].GetProperty("constraints"); + + Assert.Equal(1, port.GetProperty("min").GetInt32()); + Assert.Equal(65_535, port.GetProperty("max").GetInt32()); + } + + /// + /// The case that breaks if someone reaches for TryAddSingleton instead of + /// TryAddEnumerable: the second registration would be dropped and its constraints + /// would vanish. + /// + [Fact] + public async Task UseSchemaContributor_RegistersEveryDistinctType() + { + await using var app = await TestApp.StartAsync(configureSettings: b => b + .UseSchemaContributor() + .UseSchemaContributor()); + + var properties = await PropertiesOf(app, "api-described"); + + Assert.Equal(1, properties["port"].GetProperty("constraints").GetProperty("min").GetInt32()); + Assert.True(properties["host"].GetProperty("constraints").GetProperty("translated").GetBoolean()); + } + + /// + /// A contributor only describes, so registering one twice would describe the group twice — + /// and the merge would read the second pass as a collision with itself, logging a clash + /// that names the same contributor on both sides. + /// + [Fact] + public async Task UseSchemaContributor_RegisteringOneTypeTwice_RegistersItOnce() + { + await using var app = await TestApp.StartAsync(configureSettings: b => b + .UseSchemaContributor() + .UseSchemaContributor()); + + var port = (await PropertiesOf(app, "api-described"))["port"].GetProperty("constraints"); + + Assert.Equal(1, port.GetProperty("min").GetInt32()); + Assert.DoesNotContain(app.Warnings, w => w.Contains("both describe", StringComparison.Ordinal)); + } + + [Fact] + public async Task UseSchemaContributor_ComposesWithUseFluentValidation() + { + await using var app = await TestApp.StartAsync(configureSettings: b => b + .UseFluentValidation(typeof(DescribedSettingsValidator).Assembly) + .UseSchemaContributor()); + + var properties = await PropertiesOf(app, "api-described"); + var host = properties["host"].GetProperty("constraints"); + + // The validator's bounds... + Assert.Equal(3, host.GetProperty("minLength").GetInt32()); + Assert.Equal(255, host.GetProperty("maxLength").GetInt32()); + + // ...and the extra contributor's, on the same property. + Assert.True(host.GetProperty("translated").GetBoolean()); + } + + [Fact] + public async Task UseSchemaContributor_FactoryOverload_RegistersWhatTheFactoryReturns() + { + await using var app = await TestApp.StartAsync(configureSettings: b => b + .UseSchemaContributor(_ => new TranslatedContributor(nameof(DescribedSettings.UseSsl)))); + + var properties = await PropertiesOf(app, "api-described"); + + Assert.True(properties["useSsl"].GetProperty("constraints").GetProperty("translated").GetBoolean()); + } + + private static JsonElement Find(List groups, string route) + => groups.Single(g => g.GetProperty("route").GetString() == route); + + private static async Task> PropertiesOf(TestApp app, string route) + { + var schema = await app.Client.GetFromJsonAsync($"/api/settings/{route}/schema"); + return schema.GetProperty("properties") + .EnumerateArray() + .ToDictionary(p => p.GetProperty("name").GetString()!, p => p); + } + + private sealed class StubSchemaContributor : ISettingSchemaContributor + { + public bool CanDescribe(Type type) => type == typeof(DescribedSettings); + + public IReadOnlyDictionary> Describe(Type type) + => new Dictionary> + { + [nameof(DescribedSettings.Port)] = new Dictionary + { + ["min"] = 1, + ["max"] = 65_535 + } + }; + } + + /// + /// Stands in for a contributor from another package: it publishes a constraint + /// DotNetBoost.Settings has never heard of, which is what this port exists for. + /// + private sealed class TranslatedContributor(string propertyName) : ISettingSchemaContributor + { + public bool CanDescribe(Type type) => type == typeof(DescribedSettings); + + public IReadOnlyDictionary> Describe(Type type) + => new Dictionary> + { + [propertyName] = new Dictionary { ["translated"] = true } + }; + } + + /// A over Host, for the DI-constructed overload. + private sealed class HostTranslatedContributor : ISettingSchemaContributor + { + private readonly TranslatedContributor _inner = new(nameof(DescribedSettings.Host)); + + public bool CanDescribe(Type type) => _inner.CanDescribe(type); + + public IReadOnlyDictionary> Describe(Type type) + => _inner.Describe(type); + } + + /// Collides with on min, and only on that. + private sealed class MinOverridingContributor(int min) : ISettingSchemaContributor + { + public bool CanDescribe(Type type) => type == typeof(DescribedSettings); + + public IReadOnlyDictionary> Describe(Type type) + => new Dictionary> + { + [nameof(DescribedSettings.Port)] = new Dictionary + { + ["min"] = min, + ["maxLength"] = 7 + } + }; + } + + private sealed class RecordingContributor(bool canDescribe) : ISettingSchemaContributor + { + public int DescribeCalls { get; private set; } + + public bool CanDescribe(Type type) => canDescribe; + + public IReadOnlyDictionary> Describe(Type type) + { + DescribeCalls++; + return new Dictionary>(); + } + } + + private sealed class ThrowingSchemaContributor : ISettingSchemaContributor + { + public bool CanDescribe(Type type) => true; + + public IReadOnlyDictionary> Describe(Type type) + => throw new InvalidOperationException("cannot read the rules"); + } +} + +/// A group with one of each thing the schema has to report. +[SettingGroup("api-described")] +public class DescribedSettings +{ + public string Host { get; set; } = "smtp.example.com"; + + [SettingDefault(2525)] + public int Port { get; set; } = 587; + + public bool UseSsl { get; set; } = true; + + [Sensitive] + public string Password { get; set; } = "compile-time-default"; +} + +[SettingGroup("api-secured")] +[Authorize] +public class SecuredSettings +{ + public string Host { get; set; } = string.Empty; +} + +/// +/// Gives UseFluentValidation rules to publish for , so the +/// composition test is about two real contributors rather than one and a stub. +/// +public sealed class DescribedSettingsValidator : AbstractValidator +{ + public DescribedSettingsValidator() => RuleFor(x => x.Host).Length(3, 255); +} diff --git a/tests/DotNetBoost.Settings.ApiTests/DotNetBoost.Settings.ApiTests.csproj b/tests/DotNetBoost.Settings.ApiTests/DotNetBoost.Settings.ApiTests.csproj index 29545bf..671e0a1 100644 --- a/tests/DotNetBoost.Settings.ApiTests/DotNetBoost.Settings.ApiTests.csproj +++ b/tests/DotNetBoost.Settings.ApiTests/DotNetBoost.Settings.ApiTests.csproj @@ -9,8 +9,11 @@ + + + diff --git a/tests/DotNetBoost.Settings.ApiTests/EndpointMetadataTests.cs b/tests/DotNetBoost.Settings.ApiTests/EndpointMetadataTests.cs new file mode 100644 index 0000000..eabae2c --- /dev/null +++ b/tests/DotNetBoost.Settings.ApiTests/EndpointMetadataTests.cs @@ -0,0 +1,66 @@ +using Microsoft.AspNetCore.Http.Metadata; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; + +namespace DotNetBoost.Settings.ApiTests; + +/// +/// The generated routes carry their own OpenAPI tag, as the sibling DotNetBoost packages do. +/// Without it every consumer wraps the call in MapGroup("") — an empty prefix whose +/// only purpose is to hang a tag on. +/// +public class EndpointMetadataTests +{ + [Fact] + public async Task GeneratedEndpoints_AreTagged() + { + await using var app = await TestApp.StartAsync(); + + var tagged = SettingsEndpoints(app) + .Where(e => e.Metadata.GetMetadata()?.Tags.Contains("Settings") == true) + .ToList(); + + Assert.NotEmpty(tagged); + Assert.Equal(SettingsEndpoints(app).Count, tagged.Count); + } + + [Fact] + public async Task EveryGeneratedEndpoint_IsTagged() + { + await using var app = await TestApp.StartAsync(); + + var untagged = SettingsEndpoints(app) + .Where(e => e.Metadata.GetMetadata()?.Tags.Contains("Settings") != true) + .Select(e => e.DisplayName) + .ToList(); + + Assert.Empty(untagged); + } + + /// + /// Two endpoints per group, GET and POST. Pinned so that adding a third — or leaving the + /// removed /audit route behind — is a test failure rather than something to notice later. + /// + [Fact] + public async Task EachGroup_GetsExactlyGetAndPost() + { + await using var app = await TestApp.StartAsync(); + + var methods = SettingsEndpoints(app) + .Where(e => e.RoutePattern.RawText == "api/settings/api-test/") + .SelectMany(e => e.Metadata.GetMetadata()!.HttpMethods) + .OrderBy(m => m, StringComparer.Ordinal) + .ToList(); + + Assert.Equal(["GET", "POST"], methods); + Assert.DoesNotContain(SettingsEndpoints(app), + e => e.RoutePattern.RawText!.EndsWith("/audit", StringComparison.Ordinal)); + } + + private static List SettingsEndpoints(TestApp app) + => app.Services.GetRequiredService() + .Endpoints + .OfType() + .Where(e => e.RoutePattern.RawText!.StartsWith("api/settings/", StringComparison.Ordinal)) + .ToList(); +} diff --git a/tests/DotNetBoost.Settings.ApiTests/ProjectionEndpointTests.cs b/tests/DotNetBoost.Settings.ApiTests/ProjectionEndpointTests.cs new file mode 100644 index 0000000..9f2d314 --- /dev/null +++ b/tests/DotNetBoost.Settings.ApiTests/ProjectionEndpointTests.cs @@ -0,0 +1,318 @@ +using System.Net; +using System.Net.Http.Json; +using Microsoft.AspNetCore.Http.Metadata; +using Microsoft.AspNetCore.Routing; +using System.Text.Json; +using DotNetBoost.Settings.Core.Attributes; +using DotNetBoost.Settings.Core.Interfaces; +using Microsoft.Extensions.DependencyInjection; + +namespace DotNetBoost.Settings.ApiTests; + +/// +/// exists so a consumer can serve a group as something +/// other than its stored values — in the request's language, with an id resolved to a URL — +/// without the settings engine learning what any of that is. The constraints are what make +/// it safe to have: read path only, never near the entity tag, and inert when unregistered. +/// +public class ProjectionEndpointTests +{ + private const string Url = "/api/settings/api-projected"; + + [Fact] + public async Task Get_WithNoProjectorRegistered_ReturnsTheStoredGroup() + { + await using var app = await TestApp.StartAsync(); + + var body = await app.Client.GetFromJsonAsync(Url); + + Assert.Equal("stored", body.GetProperty("displayName").GetString()); + Assert.False(body.TryGetProperty("language", out _)); + } + + [Fact] + public async Task Get_WithAProjectorRegistered_ReturnsTheProjection() + { + await using var app = await TestApp.StartAsync( + s => s.AddScoped, UppercasingProjector>()); + + var body = await app.Client.GetFromJsonAsync(Url); + + Assert.Equal("STORED", body.GetProperty("displayName").GetString()); + Assert.Equal("xx-test", body.GetProperty("language").GetString()); + } + + /// + /// The easy way to get this feature wrong. A tag covering projected output would move + /// with whatever the projector depends on, and If-Match would stop describing the + /// stored revision it is supposed to protect. + /// + [Fact] + public async Task Get_ETag_DescribesTheStoredRevision_NotTheProjection() + { + await using var app = await TestApp.StartAsync( + s => s.AddScoped, UppercasingProjector>()); + + await app.Client.PostAsJsonAsync(Url, new { DisplayName = "shared" }); + + var response = await app.Client.GetAsync(Url); + + // The body is projected... + var body = await response.Content.ReadFromJsonAsync(); + Assert.Equal("SHARED", body.GetProperty("displayName").GetString()); + + // ...and the tag is still exactly what GetVersionAsync computes from the stored rows. + using var scope = app.Services.CreateScope(); + var stored = await scope.ServiceProvider.GetRequiredService() + .For().GetVersionAsync(); + + Assert.Equal($"\"{stored}\"", response.Headers.ETag!.Tag); + } + + /// + /// A projector must not be able to influence what is stored. POST neither consults it nor + /// accepts its shape back. + /// + [Fact] + public async Task Post_DoesNotConsultTheProjector() + { + var projector = new CountingProjector(); + await using var app = await TestApp.StartAsync( + s => s.AddScoped>(_ => projector)); + + await app.Client.PostAsJsonAsync(Url, new { DisplayName = "written" }); + + Assert.Equal(0, projector.Calls); + } + + /// + /// Deliberately unlike a notification, whose exception is logged and swallowed. A failed + /// notification must not break a write that already committed; a failed projection means + /// the response would be wrong, and a wrong response must not be hidden. + /// + [Fact] + public async Task Get_WithAProjectorThatThrows_FailsTheRequest() + { + await using var app = await TestApp.StartAsync( + s => s.AddScoped, ThrowingProjector>()); + + // The test host rethrows an unhandled exception rather than turning it into a 500, so + // either outcome proves the point: the failure is not swallowed. + var failure = await Record.ExceptionAsync(() => app.Client.GetAsync(Url)); + + if (failure is null) + { + var response = await app.Client.GetAsync(Url); + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + } + } + + // ------------------------------------------------- the declared OpenAPI response type + + /// + /// Asserted on endpoint metadata rather than a generated OpenAPI document: AddOpenApi is + /// .NET 9+, and this suite also runs on net8.0. .Produces is metadata either way. + /// + [Fact] + public async Task Get_WithATypedProjector_DeclaresTheProjectionAsIts200() + { + await using var app = await TestApp.StartAsync( + configureSettings: b => b.UseProjector()); + + Assert.Equal(typeof(BrandingView), DeclaredResponseType(app, Url)); + } + + /// + /// The declared response type follows the projector that actually runs, in both + /// directions. These are two separate registrations, so replacing one without the other + /// would publish a shape nothing returns — the first case below is what catches it. + /// + [Fact] + public async Task Get_ReplacingATypedProjectorWithAnUntypedOne_DeclaresTheGroup() + { + await using var app = await TestApp.StartAsync( + configureSettings: b => b + .UseProjector() + .ReplaceProjector()); + + Assert.Equal(typeof(ProjectedSettings), DeclaredResponseType(app, Url)); + } + + /// + [Fact] + public async Task Get_ReplacingAnUntypedProjectorWithATypedOne_DeclaresTheProjection() + { + await using var app = await TestApp.StartAsync( + configureSettings: b => b + .UseProjector() + .ReplaceProjector()); + + Assert.Equal(typeof(BrandingView), DeclaredResponseType(app, Url)); + } + + /// + /// The regression that matters most: an untyped projector is what every existing consumer + /// has, and its endpoint must go on declaring the stored group. + /// + [Fact] + public async Task Get_WithAnUntypedProjector_StillDeclaresTheGroup() + { + await using var app = await TestApp.StartAsync( + configureSettings: b => b.UseProjector()); + + Assert.Equal(typeof(ProjectedSettings), DeclaredResponseType(app, Url)); + } + + [Fact] + public async Task Get_WithNoProjector_DeclaresTheGroup() + { + await using var app = await TestApp.StartAsync(); + + Assert.Equal(typeof(ProjectedSettings), DeclaredResponseType(app, Url)); + } + + /// + /// Proves the default interface member really dispatches through the endpoint's reflective + /// call path — the body is the typed projector's output, not the stored group. + /// + [Fact] + public async Task Get_WithATypedProjector_StillReturnsTheProjectedBody() + { + await using var app = await TestApp.StartAsync( + configureSettings: b => b.UseProjector()); + + var body = await app.Client.GetFromJsonAsync(Url); + + Assert.Equal("STORED", body.GetProperty("displayName").GetString()); + Assert.Equal("xx-test", body.GetProperty("language").GetString()); + } + + /// + /// POST keeps accepting the stored group. Advertising the projection as a request body + /// would promise a write the API refuses. + /// + [Fact] + public async Task Post_StillAcceptsTheGroup_NotTheProjection() + { + await using var app = await TestApp.StartAsync( + configureSettings: b => b.UseProjector()); + + var accepted = app.Services.GetRequiredService() + .Endpoints + .OfType() + .Where(e => e.RoutePattern.RawText == "api/settings/api-projected/") + .Select(e => e.Metadata.GetMetadata()) + .FirstOrDefault(m => m is not null); + + Assert.Equal(typeof(ProjectedSettings), accepted!.RequestType); + } + + [Fact] + public void AProjectorDeclaringTwoProjections_ForOneGroup_ThrowsNamingBoth() + { + var ex = Assert.Throws(() => + new ServiceCollection().AddSettings() + .UseProjector()); + + Assert.Contains(nameof(BrandingView), ex.Message, StringComparison.Ordinal); + Assert.Contains(nameof(OtherView), ex.Message, StringComparison.Ordinal); + } + + private static Type? DeclaredResponseType(TestApp app, string url) + { + var pattern = url.TrimStart('/') + "/"; + + return app.Services.GetRequiredService() + .Endpoints + .OfType() + .Where(e => e.RoutePattern.RawText == pattern) + .Where(e => e.Metadata.GetMetadata()!.HttpMethods.Contains("GET")) + .SelectMany(e => e.Metadata.OfType()) + .First(m => m.StatusCode == 200) + .Type; + } + + [Fact] + public async Task Projector_DoesNotAffectProgrammaticReads() + { + await using var app = await TestApp.StartAsync( + s => s.AddScoped, UppercasingProjector>()); + + using var scope = app.Services.CreateScope(); + var manager = scope.ServiceProvider.GetRequiredService(); + + var group = await manager.For().GetAsync(); + + Assert.Equal("stored", group.DisplayName); + } +} + +[SettingGroup("api-projected", Name = "api-projected-group")] +public class ProjectedSettings +{ + public string DisplayName { get; set; } = "stored"; +} + +/// Stands in for the real case: a group reshaped with per-request context. +internal sealed class UppercasingProjector : ISettingProjector +{ + public Task ProjectAsync(ProjectedSettings group, CancellationToken ct = default) + { + ArgumentNullException.ThrowIfNull(group); + return Task.FromResult(new + { + DisplayName = group.DisplayName.ToUpperInvariant(), + Language = "xx-test" + }); + } +} + +/// What a typed projector returns, and what GET must then advertise. +public sealed record BrandingView(string DisplayName, string Language); + +/// A second view, only so one projector can ambiguously declare two. +public sealed record OtherView(string DisplayName); + +/// +/// The same projection as , but named — so the endpoint can +/// declare it without the library having to resolve a scoped projector at map time. +/// +internal sealed class TypedProjector : ISettingProjector +{ + public Task ProjectAsync(ProjectedSettings group, CancellationToken ct = default) + { + ArgumentNullException.ThrowIfNull(group); + return Task.FromResult(new BrandingView(group.DisplayName.ToUpperInvariant(), "xx-test")); + } +} + +/// Declares two projections for one group, which has no single answer. +internal sealed class AmbiguousProjector + : ISettingProjector, ISettingProjector +{ + Task ISettingProjector.ProjectAsync( + ProjectedSettings group, CancellationToken ct) => Task.FromResult(new BrandingView("", "")); + + Task ISettingProjector.ProjectAsync( + ProjectedSettings group, CancellationToken ct) => Task.FromResult(new OtherView("")); + + public Task ProjectAsync(ProjectedSettings group, CancellationToken ct = default) + => Task.FromResult(group); +} + +internal sealed class ThrowingProjector : ISettingProjector +{ + public Task ProjectAsync(ProjectedSettings group, CancellationToken ct = default) + => throw new InvalidOperationException("the translator is unavailable"); +} + +internal sealed class CountingProjector : ISettingProjector +{ + public int Calls { get; private set; } + + public Task ProjectAsync(ProjectedSettings group, CancellationToken ct = default) + { + Calls++; + return Task.FromResult(group); + } +} diff --git a/tests/DotNetBoost.Settings.ApiTests/TestApp.cs b/tests/DotNetBoost.Settings.ApiTests/TestApp.cs index 68693f2..afe5e53 100644 --- a/tests/DotNetBoost.Settings.ApiTests/TestApp.cs +++ b/tests/DotNetBoost.Settings.ApiTests/TestApp.cs @@ -1,4 +1,5 @@ using DotNetBoost.Settings.Core; +using DotNetBoost.Settings.Core.Attributes; using DotNetBoost.Settings.Core.Interfaces; using DotNetBoost.Settings.Core.Models; using Microsoft.AspNetCore.Builder; @@ -18,30 +19,53 @@ namespace DotNetBoost.Settings.ApiTests; internal sealed class TestApp : IAsyncDisposable { private readonly WebApplication _app; + private readonly CapturingLoggerProvider _log; public HttpClient Client { get; } - private TestApp(WebApplication app) + /// Warning-and-above messages logged by the application under test. + public IReadOnlyList Warnings => _log.Messages; + + /// The running host's root provider, for tests that need to resolve a service. + public IServiceProvider Services => _app.Services; + + private TestApp(WebApplication app, CapturingLoggerProvider log) { - _app = app; - Client = app.GetTestClient(); + _app = app; + _log = log; + Client = app.GetTestClient(); } - public static async Task StartAsync(Action? configure = null, bool requireIfMatch = false) + public static async Task StartAsync( + Action? configure = null, + bool requireIfMatch = false, + bool includeDiscovery = true, + Action? configureSettings = null) { var builder = WebApplication.CreateSlimBuilder(); builder.WebHost.UseTestServer(); builder.Logging.ClearProviders(); builder.Services.AddSingleton(); - builder.Services.AddSettings(); + + // Warnings are captured so a test can assert that something was *not* logged. + var log = new CapturingLoggerProvider(); + builder.Logging.AddProvider(log); + builder.Logging.SetMinimumLevel(LogLevel.Warning); + + // configureSettings runs against the real SettingBuilder, so builder methods are + // exercised through the chain an application actually writes. AddSettings is called + // into a local first: inside a ?.Invoke argument it would not run at all when no + // configureSettings was passed. + var settings = builder.Services.AddSettings(); + configureSettings?.Invoke(settings); configure?.Invoke(builder.Services); var app = builder.Build(); - app.MapSettingsEndpoints(requireIfMatch); + app.MapSettingsEndpoints(requireIfMatch, includeDiscovery); await app.StartAsync(); - return new TestApp(app); + return new TestApp(app, log); } public async ValueTask DisposeAsync() @@ -111,24 +135,43 @@ public Task CountAsync(string g, CancellationToken ct = default) } } -/// Records what it is asked for so tests can assert the group key used. -internal sealed class RecordingAuditStore : ISettingAuditStore +/// Collects log messages so a test can assert on what was, or was not, logged. +internal sealed class CapturingLoggerProvider : ILoggerProvider { - public List<(string Group, string? Key)> Queries { get; } = []; + private readonly List _messages = []; - public Task RecordAsync(SettingAuditEntry entry, CancellationToken ct = default) => Task.CompletedTask; + public IReadOnlyList Messages + { + get { lock (_messages) return [.. _messages]; } + } - public Task> GetHistoryAsync( - string group, string? key = null, CancellationToken ct = default) + public ILogger CreateLogger(string categoryName) => new Capturing(this); + + public void Dispose() { } + + private void Add(string message) { - Queries.Add((group, key)); - return Task.FromResult>( - [ - new SettingAuditEntry - { - Group = group, Key = key ?? "Host", - OldValue = "before", NewValue = "after", ChangedBy = "tester" - } - ]); + lock (_messages) _messages.Add(message); } + + private sealed class Capturing(CapturingLoggerProvider owner) : ILogger + { + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => owner.Add(formatter(state, exception)); + } +} + +/// +/// The group the endpoint tests exercise. Its Name differs from the class name on purpose, +/// so anything keyed on the persistence key cannot pass by using the class name instead. +/// seeds it. +/// +[SettingGroup("api-test", Name = "api-test-group")] +public class ApiTestSettings +{ + public string Host { get; set; } = string.Empty; } diff --git a/tests/DotNetBoost.Settings.IntegrationTests/Stores/DapperPostgresStoreTests.cs b/tests/DotNetBoost.Settings.IntegrationTests/Stores/DapperPostgresStoreTests.cs index 2148beb..eb22b46 100644 --- a/tests/DotNetBoost.Settings.IntegrationTests/Stores/DapperPostgresStoreTests.cs +++ b/tests/DotNetBoost.Settings.IntegrationTests/Stores/DapperPostgresStoreTests.cs @@ -37,7 +37,7 @@ private async Task OpenAsync(string connectionString) } [Fact] - public async Task SchemaInitializer_CreatesBothTables_AndIsIdempotent() + public async Task SchemaInitializer_CreatesTheSettingsTable_AndIsIdempotent() { var connection = await NewConnectionAsync(); @@ -48,7 +48,10 @@ public async Task SchemaInitializer_CreatesBothTables_AndIsIdempotent() "SELECT table_name FROM information_schema.tables WHERE table_schema = 'public'"); Assert.Contains("settings", tables, StringComparer.OrdinalIgnoreCase); - Assert.Contains("settingaudits", tables, StringComparer.OrdinalIgnoreCase); + + // The bootstrap used to create a SettingAudits table that nothing ever wrote to, so a + // Dapper user concluded auditing worked and got silence. It must stay gone. + Assert.DoesNotContain("settingaudits", tables, StringComparer.OrdinalIgnoreCase); } [Fact] diff --git a/tests/DotNetBoost.Settings.IntegrationTests/Stores/DapperSqlServerStoreTests.cs b/tests/DotNetBoost.Settings.IntegrationTests/Stores/DapperSqlServerStoreTests.cs index d7b35fe..c01d9be 100644 --- a/tests/DotNetBoost.Settings.IntegrationTests/Stores/DapperSqlServerStoreTests.cs +++ b/tests/DotNetBoost.Settings.IntegrationTests/Stores/DapperSqlServerStoreTests.cs @@ -38,7 +38,7 @@ private async Task OpenAsync(string connectionString) } [Fact] - public async Task SchemaInitializer_CreatesBothTables_AndIsIdempotent() + public async Task SchemaInitializer_CreatesTheSettingsTable_AndIsIdempotent() { var connection = await NewConnectionAsync(); @@ -50,7 +50,10 @@ public async Task SchemaInitializer_CreatesBothTables_AndIsIdempotent() "SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE = 'BASE TABLE'"); Assert.Contains("Settings", tables, StringComparer.OrdinalIgnoreCase); - Assert.Contains("SettingAudits", tables, StringComparer.OrdinalIgnoreCase); + + // The bootstrap used to create a SettingAudits table that nothing ever wrote to, so a + // Dapper user concluded auditing worked and got silence. It must stay gone. + Assert.DoesNotContain("SettingAudits", tables, StringComparer.OrdinalIgnoreCase); } /// diff --git a/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCorePostgresStoreTests.cs b/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCorePostgresStoreTests.cs index 0dea5b0..1d02cfc 100644 --- a/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCorePostgresStoreTests.cs +++ b/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCorePostgresStoreTests.cs @@ -6,14 +6,14 @@ namespace DotNetBoost.Settings.IntegrationTests.Stores; -public sealed class PostgresDbContext(DbContextOptions options) - : DbContext(options), ISettingDbContext +/// +/// Configured through the inferring overload, so the Npgsql provider name really does map to +/// DatabaseProvider.PostgreSql against a live server rather than only in a unit test. +/// +public sealed class PostgresDbContext(DbContextOptions options) : DbContext(options) { - public DbSet Settings => Set(); - public DbSet SettingAudits => Set(); - protected override void OnModelCreating(ModelBuilder modelBuilder) - => modelBuilder.ApplySettingsConfiguration(DatabaseProvider.PostgreSql); + => modelBuilder.ApplySettings(this); } /// @@ -65,30 +65,13 @@ public async Task UniqueIndex_RejectsDuplicateGroupKeyPair() { var ctx = await NewContextAsync(); - ctx.Settings.Add(new Setting { Group = "Mail", Key = "Host", Value = "a", Type = "System.String" }); - ctx.Settings.Add(new Setting { Group = "Mail", Key = "Host", Value = "b", Type = "System.String" }); + ctx.Set().Add(new Setting { Group = "Mail", Key = "Host", Value = "a", Type = "System.String" }); + ctx.Set().Add(new Setting { Group = "Mail", Key = "Host", Value = "b", Type = "System.String" }); // UX_Settings_Group_Key must be enforced by the database, not just by application code. await Assert.ThrowsAsync(() => ctx.SaveChangesAsync()); } - [Fact] - public async Task AuditStore_RecordsAndReadsBackHistory() - { - var ctx = await NewContextAsync(); - var audit = new EfCoreAuditStore(ctx); - - await audit.RecordAsync(new SettingAuditEntry - { - Group = "Mail", Key = "Port", OldValue = "25", NewValue = "587", ChangedBy = "tester" - }); - - var history = await audit.GetHistoryAsync("Mail", "Port"); - Assert.Single(history); - Assert.Equal("587", history[0].NewValue); - Assert.Equal("tester", history[0].ChangedBy); - } - public Task InitializeAsync() => Task.CompletedTask; public async Task DisposeAsync() diff --git a/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCoreSqlServerStoreTests.cs b/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCoreSqlServerStoreTests.cs index e8669c6..358e25f 100644 --- a/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCoreSqlServerStoreTests.cs +++ b/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCoreSqlServerStoreTests.cs @@ -6,14 +6,15 @@ namespace DotNetBoost.Settings.IntegrationTests.Stores; -public sealed class SqlServerDbContext(DbContextOptions options) - : DbContext(options), ISettingDbContext +/// +/// Configured through the inferring overload, so the SQL Server provider name really does +/// map to DatabaseProvider.SqlServer — nvarchar(max) and a genuine rowversion — against +/// a live server rather than only in a unit test. +/// +public sealed class SqlServerDbContext(DbContextOptions options) : DbContext(options) { - public DbSet Settings => Set(); - public DbSet SettingAudits => Set(); - protected override void OnModelCreating(ModelBuilder modelBuilder) - => modelBuilder.ApplySettingsConfiguration(DatabaseProvider.SqlServer); + => modelBuilder.ApplySettings(this); } /// @@ -73,21 +74,6 @@ public async Task ValueColumn_AcceptsPayloadsBeyondNVarCharLimit() Assert.Equal(big, (await store.GetAsync("Mail", "Blob"))!.Value); } - [Fact] - public async Task AuditStore_RecordsAndReadsBackHistory() - { - var ctx = await NewContextAsync(); - var audit = new EfCoreAuditStore(ctx); - - await audit.RecordAsync(new SettingAuditEntry - { - Group = "Mail", Key = "Host", OldValue = "old", NewValue = "new", ChangedBy = "tester" - }); - - var history = await audit.GetHistoryAsync("Mail", "Host"); - Assert.Equal("old", Assert.Single(history).OldValue); - } - public Task InitializeAsync() => Task.CompletedTask; public async Task DisposeAsync() diff --git a/tests/DotNetBoost.Settings.ProviderTests/DotNetBoost.Settings.ProviderTests.csproj b/tests/DotNetBoost.Settings.ProviderTests/DotNetBoost.Settings.ProviderTests.csproj index 1805dae..98f8560 100644 --- a/tests/DotNetBoost.Settings.ProviderTests/DotNetBoost.Settings.ProviderTests.csproj +++ b/tests/DotNetBoost.Settings.ProviderTests/DotNetBoost.Settings.ProviderTests.csproj @@ -14,6 +14,9 @@ + + diff --git a/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreModelConfigurationTests.cs b/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreModelConfigurationTests.cs new file mode 100644 index 0000000..5e16e20 --- /dev/null +++ b/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreModelConfigurationTests.cs @@ -0,0 +1,156 @@ +using DotNetBoost.Settings.Core.Models; +using DotNetBoost.Settings.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore; + +namespace DotNetBoost.Settings.ProviderTests.Stores; + +/// +/// ApplySettings picks the Value column type and the concurrency-token +/// style per engine, and getting it wrong is the kind of mistake that only shows up as +/// truncated settings long after the migration ran. These tests read the type back off the +/// built model, so the mapping is checked without needing any of these servers to exist. +/// +public class EfCoreModelConfigurationTests +{ + [Fact] + public void InferringOverload_ReadsTheEngineOffTheContext() + { + // TestDbContext is on SQLite and calls ApplySettings(this). + Assert.Equal("TEXT", ValueColumnTypeOf(new InferredSqliteContext())); + } + + [Theory] + [InlineData(DatabaseProvider.SqlServer, "nvarchar(max)")] + [InlineData(DatabaseProvider.PostgreSql, "text")] + [InlineData(DatabaseProvider.Sqlite, "TEXT")] + public void ExplicitOverload_WinsOverWhateverTheContextIsConnectedTo( + DatabaseProvider provider, string expectedColumnType) + { + // Every context here runs on SQLite. Naming the engine explicitly is the documented + // escape hatch for a model built against one engine and migrated onto another, so it + // has to override the inference rather than be overridden by it. + Assert.Equal(expectedColumnType, ValueColumnTypeOf(ContextFor(provider))); + } + + [Fact] + public void InferringOverload_OnAnUnmappedProvider_SaysWhatToDoInstead() + { + using var ctx = new InMemoryContext(); + + var ex = Assert.ThrowsAny(() => ctx.Model); + + Assert.Contains("ApplySettings(DatabaseProvider)", Flatten(ex), StringComparison.Ordinal); + Assert.Contains("Microsoft.EntityFrameworkCore.InMemory", Flatten(ex), StringComparison.Ordinal); + } + + /// + /// The whole purpose of the obsolete alias: a context written against the old name keeps + /// compiling and keeps building the same model. Deprecation warning, not a break. + /// + [Theory] + [InlineData(DatabaseProvider.SqlServer, "nvarchar(max)")] + [InlineData(DatabaseProvider.PostgreSql, "text")] + [InlineData(DatabaseProvider.Sqlite, "TEXT")] + public void TheObsoleteName_StillBuildsTheSameModel(DatabaseProvider provider, string expectedColumnType) + { + Assert.Equal(expectedColumnType, ValueColumnTypeOf(ObsoleteContextFor(provider))); + + // And the same as the new name produces, rather than merely something plausible. + Assert.Equal(ValueColumnTypeOf(ContextFor(provider)), ValueColumnTypeOf(ObsoleteContextFor(provider))); + } + + /// EF wraps a throw from OnModelCreating, so assert over the whole chain. + private static string Flatten(Exception ex) + { + var text = ex.Message; + for (var inner = ex.InnerException; inner is not null; inner = inner.InnerException) + text += " | " + inner.Message; + return text; + } + + private static string? ValueColumnTypeOf(DbContext ctx) + { + using (ctx) + { + return ctx.Model + .FindEntityType(typeof(Setting))! + .FindProperty(nameof(Setting.Value))! + .GetColumnType(); + } + } + + // One context type per provider: EF caches the built model per context type, so a single + // parameterised type would hand every case back the first model it built. + private static DbContext ContextFor(DatabaseProvider provider) => provider switch + { + DatabaseProvider.SqlServer => new ExplicitSqlServerContext(), + DatabaseProvider.PostgreSql => new ExplicitPostgresContext(), + _ => new ExplicitSqliteContext() + }; + + private abstract class SqliteBackedContext : DbContext + { + protected override void OnConfiguring(DbContextOptionsBuilder options) + => options.UseSqlite("DataSource=:memory:"); + } + + private sealed class InferredSqliteContext : SqliteBackedContext + { + protected override void OnModelCreating(ModelBuilder mb) => mb.ApplySettings(this); + } + + private sealed class ExplicitSqlServerContext : SqliteBackedContext + { + protected override void OnModelCreating(ModelBuilder mb) + => mb.ApplySettings(DatabaseProvider.SqlServer); + } + + private sealed class ExplicitPostgresContext : SqliteBackedContext + { + protected override void OnModelCreating(ModelBuilder mb) + => mb.ApplySettings(DatabaseProvider.PostgreSql); + } + + private sealed class ExplicitSqliteContext : SqliteBackedContext + { + protected override void OnModelCreating(ModelBuilder mb) + => mb.ApplySettings(DatabaseProvider.Sqlite); + } + + // One context type per provider, as above, each calling the obsolete alias. CS0618 is + // suppressed here and nowhere else: these exist precisely to exercise the deprecated name. + private static DbContext ObsoleteContextFor(DatabaseProvider provider) => provider switch + { + DatabaseProvider.SqlServer => new ObsoleteSqlServerContext(), + DatabaseProvider.PostgreSql => new ObsoletePostgresContext(), + _ => new ObsoleteSqliteContext() + }; + +#pragma warning disable CS0618 // Deliberately calling the obsolete alias. + private sealed class ObsoleteSqlServerContext : SqliteBackedContext + { + protected override void OnModelCreating(ModelBuilder mb) + => mb.ApplySettingsConfiguration(DatabaseProvider.SqlServer); + } + + private sealed class ObsoletePostgresContext : SqliteBackedContext + { + protected override void OnModelCreating(ModelBuilder mb) + => mb.ApplySettingsConfiguration(DatabaseProvider.PostgreSql); + } + + private sealed class ObsoleteSqliteContext : SqliteBackedContext + { + protected override void OnModelCreating(ModelBuilder mb) + => mb.ApplySettingsConfiguration(DatabaseProvider.Sqlite); + } +#pragma warning restore CS0618 + + private sealed class InMemoryContext : DbContext + { + protected override void OnConfiguring(DbContextOptionsBuilder options) + => options.UseInMemoryDatabase(nameof(InMemoryContext)); + + protected override void OnModelCreating(ModelBuilder mb) => mb.ApplySettings(this); + } +} diff --git a/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreRegistrationTests.cs b/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreRegistrationTests.cs new file mode 100644 index 0000000..82dd49f --- /dev/null +++ b/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreRegistrationTests.cs @@ -0,0 +1,58 @@ +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; + +namespace DotNetBoost.Settings.ProviderTests.Stores; + +/// +/// UseEntityFrameworkCore<TContext> asks nothing of the context beyond being a +/// . implements no interface and declares +/// no DbSet properties, so these tests only compile and pass if the constraint and the +/// marker interface are genuinely gone. +/// +public class EfCoreRegistrationTests +{ + [Fact] + public void UseEntityFrameworkCore_ResolvesTheStore_FromAPlainDbContext() + { + using var provider = BuildProvider(b => b.UseEntityFrameworkCore()); + using var scope = provider.CreateScope(); + + Assert.IsType(scope.ServiceProvider.GetRequiredService()); + } + + /// + /// The library must not claim a framework type it does not own. Registering + /// would, in an application with two contexts, silently hand a + /// bare DbContext dependency whichever one Settings happened to be pointed at — + /// and nothing here needs it, because the store is constructed from + /// TestDbContext explicitly. + /// + [Fact] + public void UseEntityFrameworkCore_RegistersNothingAgainstDbContext() + { + using var provider = BuildProvider(b => b.UseEntityFrameworkCore()); + using var scope = provider.CreateScope(); + + Assert.Null(scope.ServiceProvider.GetService()); + Assert.NotNull(scope.ServiceProvider.GetRequiredService()); + } + + private static ServiceProvider BuildProvider(Action configure) + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddDbContext(o => o.UseSqlite("DataSource=:memory:")); + + configure(services.AddSettings()); + + // Validate on build so a lifetime mistake in the registrations fails here rather than + // on some later request. + return services.BuildServiceProvider(new ServiceProviderOptions + { + ValidateScopes = true, + ValidateOnBuild = true + }); + } +} diff --git a/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreSettingStoreTests.cs b/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreSettingStoreTests.cs index 341eba5..c0c0232 100644 --- a/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreSettingStoreTests.cs +++ b/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreSettingStoreTests.cs @@ -1,17 +1,16 @@ -using DotNetBoost.Settings.Core.Models; using DotNetBoost.Settings.EntityFrameworkCore; using Microsoft.EntityFrameworkCore; namespace DotNetBoost.Settings.ProviderTests.Stores; -public sealed class TestDbContext(DbContextOptions options) - : DbContext(options), ISettingDbContext +/// +/// The whole registration a consuming context needs: no interface, no DbSet properties, and +/// the engine taken from the context rather than restated. +/// +public sealed class TestDbContext(DbContextOptions options) : DbContext(options) { - public DbSet Settings => Set(); - public DbSet SettingAudits => Set(); - protected override void OnModelCreating(ModelBuilder modelBuilder) - => modelBuilder.ApplySettingsConfiguration(DatabaseProvider.Sqlite); + => modelBuilder.ApplySettings(this); } public sealed class EfCoreSettingStoreTests : SettingStoreContractTests, IDisposable diff --git a/tests/DotNetBoost.Settings.UnitTests/AuditAndOptionsTests.cs b/tests/DotNetBoost.Settings.UnitTests/AuditAndOptionsTests.cs deleted file mode 100644 index 19ffcbf..0000000 --- a/tests/DotNetBoost.Settings.UnitTests/AuditAndOptionsTests.cs +++ /dev/null @@ -1,313 +0,0 @@ -using DotNetBoost.Settings.Core; -using DotNetBoost.Settings.Core.Attributes; -using DotNetBoost.Settings.Core.Interfaces; -using DotNetBoost.Settings.Core.Models; -using DotNetBoost.Settings.Core.Services; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging.Abstractions; -using Moq; -using System.Security.Cryptography; - -namespace DotNetBoost.Settings.UnitTests; - -/// -/// Regression tests for the audit trail. These use a stateful fake store rather than a mock: -/// the bug they cover (OldValue read back *after* the write) is invisible to a store whose -/// reads are not affected by its writes. -/// -public class AuditTrailTests -{ - [Fact] - public async Task Audit_RecordsPreviousValue_AsOldValue() - { - var (mgr, _, audit) = Build(); - - await mgr.For().SetAsync(new AuditSettings { Host = "old.example.com" }); - audit.Entries.Clear(); - await mgr.For().SetAsync(new AuditSettings { Host = "new.example.com" }); - - var entry = audit.Entries.Single(e => e.Key == nameof(AuditSettings.Host)); - Assert.Equal("old.example.com", entry.OldValue); - Assert.Equal("new.example.com", entry.NewValue); - } - - [Fact] - public async Task Audit_FirstWrite_RecordsEmptyOldValue() - { - var (mgr, _, audit) = Build(); - - await mgr.For().SetAsync(new AuditSettings { Host = "first.example.com" }); - - var entry = audit.Entries.Single(e => e.Key == nameof(AuditSettings.Host)); - Assert.Equal(string.Empty, entry.OldValue); - Assert.Equal("first.example.com", entry.NewValue); - } - - [Fact] - public async Task Audit_RedactsEncryptedValues_OnBothSides() - { - var (mgr, _, audit) = Build(new ReversibleEncryptor()); - - await mgr.For().SetAsync(new AuditSettings { Host = "h", Secret = "s3cret" }); - audit.Entries.Clear(); - await mgr.For().SetAsync(new AuditSettings { Host = "h", Secret = "n3wer" }); - - var entry = audit.Entries.Single(e => e.Key == nameof(AuditSettings.Secret)); - Assert.Equal("[encrypted]", entry.OldValue); - Assert.Equal("[encrypted]", entry.NewValue); - Assert.DoesNotContain("s3cret", entry.OldValue, StringComparison.Ordinal); - } - - [Fact] - public async Task Audit_SkipsUnchangedProperties() - { - var (mgr, _, audit) = Build(); - - await mgr.For().SetAsync(new AuditSettings { Host = "same.example.com" }); - audit.Entries.Clear(); - await mgr.For().SetAsync(new AuditSettings { Host = "same.example.com" }); - - Assert.Empty(audit.Entries); - } - - [Fact] - public async Task Audit_RecordsOnlyTheChangedProperty() - { - var (mgr, _, audit) = Build(); - - await mgr.For().SetAsync(new MultiSettings { Host = "a", Port = 25 }); - audit.Entries.Clear(); - await mgr.For().SetAsync(new MultiSettings { Host = "a", Port = 587 }); - - var entry = Assert.Single(audit.Entries); - Assert.Equal(nameof(MultiSettings.Port), entry.Key); - Assert.Equal("25", entry.OldValue); - Assert.Equal("587", entry.NewValue); - } - - [Fact] - public async Task Audit_SkipsUnchangedSensitiveProperty_DespiteFreshCiphertext() - { - // The real AES-GCM encryptor, not a stub: it draws a new nonce per call, so an - // unchanged secret serialises to different ciphertext on every save. Comparing - // ciphertext would report a change here; comparing plaintext must not. - var key = Convert.ToBase64String(RandomNumberGenerator.GetBytes(32)); - var encryptor = new AesSettingEncryptor(key); - - // Guards the premise: re-encrypting the same plaintext really does produce different - // bytes, so comparing stored ciphertext would report a change on every save. - Assert.NotEqual(encryptor.Encrypt("s3cret"), encryptor.Encrypt("s3cret")); - - var (mgr, store, audit) = Build(encryptor); - - await mgr.For().SetAsync(new AuditSettings { Host = "h", Secret = "s3cret" }); - var firstCiphertext = (await store.GetAsync("AuditSettings", nameof(AuditSettings.Secret)))!.Value; - audit.Entries.Clear(); - - await mgr.For().SetAsync(new AuditSettings { Host = "h", Secret = "s3cret" }); - - Assert.Empty(audit.Entries); - // Unchanged properties are not rewritten at all, so the stored bytes stay put. - Assert.Equal(firstCiphertext, (await store.GetAsync("AuditSettings", nameof(AuditSettings.Secret)))!.Value); - } - - [Fact] - public async Task Audit_RecordsChangedSensitiveProperty_Redacted() - { - var key = Convert.ToBase64String(RandomNumberGenerator.GetBytes(32)); - var (mgr, _, audit) = Build(new AesSettingEncryptor(key)); - - await mgr.For().SetAsync(new AuditSettings { Host = "h", Secret = "s3cret" }); - audit.Entries.Clear(); - await mgr.For().SetAsync(new AuditSettings { Host = "h", Secret = "n3wer" }); - - var entry = Assert.Single(audit.Entries); - Assert.Equal(nameof(AuditSettings.Secret), entry.Key); - Assert.Equal("[encrypted]", entry.OldValue); - Assert.Equal("[encrypted]", entry.NewValue); - } - - [Fact] - public async Task Audit_NotQueried_WhenNoAuditStoreRegistered() - { - var store = new Mock(); - store.Setup(x => x.GetGroupAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync([]); - var mgr = new SettingManager(store.Object, new NoCache(), - new ServiceCollection().BuildServiceProvider(), NullLogger.Instance); - - await mgr.For().SetAsync(new AuditSettings { Host = "h" }); - - // Exactly one read. Change detection, concurrency tokens, the audit before-values and - // the change-handler model all come from that single snapshot. - store.Verify(x => x.GetGroupAsync("AuditSettings", It.IsAny()), Times.Once); - } - - private static (ISettingManager, FakeStore, FakeAuditStore) Build(ISettingEncryptor? encryptor = null) - { - var store = new FakeStore(); - var audit = new FakeAuditStore(); - var mgr = new SettingManager(store, new NoCache(), - new ServiceCollection().BuildServiceProvider(), - NullLogger.Instance, encryptor, audit); - return (mgr, store, audit); - } - - public class AuditSettings - { - public string? Host { get; set; } - [Sensitive] public string? Secret { get; set; } - } - - public class MultiSettings - { - public string? Host { get; set; } - public int Port { get; set; } - } - - private sealed class ReversibleEncryptor : ISettingEncryptor - { - public string Encrypt(string plaintext) => "enc:" + plaintext; - public string Decrypt(string ciphertext) => ciphertext["enc:".Length..]; - } - - /// A store whose reads reflect its writes, like a real one. - private sealed class FakeStore : ISettingStore - { - private readonly Dictionary _data = new(StringComparer.Ordinal); - private static string K(string g, string k) => g + "|" + k; - - public Task> GetGroupAsync(string group, CancellationToken ct = default) - => Task.FromResult>(_data.Values.Where(x => x.Group == group).ToList()); - - public Task GetAsync(string group, string key, CancellationToken ct = default) - => Task.FromResult(_data.GetValueOrDefault(K(group, key))); - - public Task UpsertAsync(Setting setting, CancellationToken ct = default) - { - _data[K(setting.Group, setting.Key)] = Copy(setting); - return Task.CompletedTask; - } - - public Task UpsertManyAsync(IEnumerable settings, CancellationToken ct = default) - { - foreach (var s in settings) _data[K(s.Group, s.Key)] = Copy(s); - return Task.CompletedTask; - } - - public Task DeleteAsync(string group, string key, CancellationToken ct = default) - { - _data.Remove(K(group, key)); - return Task.CompletedTask; - } - - public Task DeleteGroupAsync(string group, CancellationToken ct = default) - { - foreach (var k in _data.Where(x => x.Value.Group == group).Select(x => x.Key).ToList()) - _data.Remove(k); - return Task.CompletedTask; - } - - - public Task CountAsync(string group, CancellationToken ct = default) - => Task.FromResult(_data.Values.Count(x => x.Group == group)); - - // Persist a copy so later mutations of the caller's instance cannot alter stored state. - private static Setting Copy(Setting s) => new() - { - Id = s.Id, Group = s.Group, Key = s.Key, Value = s.Value, Type = s.Type, - IsEncrypted = s.IsEncrypted, UpdatedAt = s.UpdatedAt, UpdatedBy = s.UpdatedBy - }; - } - - private sealed class FakeAuditStore : ISettingAuditStore - { - public List Entries { get; } = []; - - public Task RecordAsync(SettingAuditEntry entry, CancellationToken ct = default) - { - Entries.Add(entry); - return Task.CompletedTask; - } - - public Task> GetHistoryAsync( - string group, string? key = null, CancellationToken ct = default) - => Task.FromResult>(Entries); - } - - private sealed class NoCache : ISettingCache - { - public bool TryGetValue(string key, out T? value) { value = default; return false; } - public void Set(string key, T value, TimeSpan duration) { } - public void Remove(string key) { } - } -} - -/// Regression tests for WithCacheDuration() reaching the manager. -public class SettingOptionsTests -{ - [Fact] - public async Task WithCacheDuration_IsAppliedToCachedEntries() - { - var (mgr, cache) = BuildWith(b => b.WithCacheDuration(TimeSpan.FromMinutes(5))); - - await mgr.For().GetAsync(); - - cache.Verify(x => x.Set("dnb:setting:OptionsSettings", - It.IsAny(), TimeSpan.FromMinutes(5)), Times.Once); - } - - [Fact] - public async Task CacheDuration_DefaultsToTenMinutes() - { - var (mgr, cache) = BuildWith(_ => { }); - - await mgr.For().GetAsync(); - - cache.Verify(x => x.Set("dnb:setting:OptionsSettings", - It.IsAny(), TimeSpan.FromMinutes(10)), Times.Once); - } - - [Fact] - public void WithCacheDuration_RejectsNonPositiveDuration() - { - var builder = new ServiceCollection().AddSettings(); - Assert.Throws(() => builder.WithCacheDuration(TimeSpan.Zero)); - Assert.Throws(() => builder.WithCacheDuration(TimeSpan.FromSeconds(-1))); - } - - [Fact] - public void AddSettings_RegistersOptions() - { - var services = new ServiceCollection(); - services.AddSettings().WithCacheDuration(TimeSpan.FromHours(2)); - - var options = services.BuildServiceProvider().GetRequiredService(); - - Assert.Equal(TimeSpan.FromHours(2), options.CacheDuration); - } - - // Resolves the manager through DI so the test covers the real registration path, - // which is where the builder-to-manager wiring was previously lost. - private static (ISettingManager, Mock) BuildWith(Action configure) - { - var store = new Mock(); - store.Setup(x => x.GetGroupAsync("OptionsSettings", It.IsAny())) - .ReturnsAsync([]); - - var cache = new Mock(); - OptionsSettings? miss = null; - cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); - - var services = new ServiceCollection(); - services.AddSingleton(store.Object); - services.AddSingleton(cache.Object); // registered first so AddSettings' TryAdd defers - services.AddLogging(); - - configure(services.AddSettings()); - - return (services.BuildServiceProvider().GetRequiredService(), cache); - } - - public class OptionsSettings { public int Port { get; set; } } -} diff --git a/tests/DotNetBoost.Settings.UnitTests/BuilderAndAccessorTests.cs b/tests/DotNetBoost.Settings.UnitTests/BuilderAndAccessorTests.cs index 09cd81a..cc14d1e 100644 --- a/tests/DotNetBoost.Settings.UnitTests/BuilderAndAccessorTests.cs +++ b/tests/DotNetBoost.Settings.UnitTests/BuilderAndAccessorTests.cs @@ -94,8 +94,8 @@ public async Task ExistsAsync_ReturnsTrue_WhenRowsExist() var store = new Mock(); store.Setup(x => x.CountAsync("AccessorMailSettings", default)).ReturnsAsync(1); var cache = new Mock(); - AccessorMailSettings? miss = null; - cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); + Setting[]? miss = null; + cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); var mgr = new SettingManager(store.Object, cache.Object, new ServiceCollection().BuildServiceProvider(), NullLogger.Instance); @@ -106,8 +106,8 @@ private static (Mock store, ISettingManager manager) BuildManager { var store = new Mock(); var cache = new Mock(); - AccessorMailSettings? miss = null; - cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); + Setting[]? miss = null; + cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); store.Setup(x => x.GetGroupAsync("AccessorMailSettings", default)).ReturnsAsync(rows.ToList()); var mgr = new SettingManager(store.Object, cache.Object, new ServiceCollection().BuildServiceProvider(), NullLogger.Instance); diff --git a/tests/DotNetBoost.Settings.UnitTests/ChangeSignalTests.cs b/tests/DotNetBoost.Settings.UnitTests/ChangeSignalTests.cs new file mode 100644 index 0000000..2868301 --- /dev/null +++ b/tests/DotNetBoost.Settings.UnitTests/ChangeSignalTests.cs @@ -0,0 +1,340 @@ +using DotNetBoost.Settings.Core; +using DotNetBoost.Settings.Core.Attributes; +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.Core.Models; +using DotNetBoost.Settings.Core.Services; +using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; + +namespace DotNetBoost.Settings.UnitTests; + +/// +/// "Change it, reload, done, no redeploy" is this library's central claim, and it silently +/// stopped being true at two replicas: a write on one instance evicted that instance's +/// IMemoryCache and nothing else, so every other instance went on serving the old +/// values for up to CacheDuration. +/// +/// Each manager here gets its own , which is what two processes +/// have, and they share the signal — exactly the arrangement a Redis-backed deployment has. +/// +/// +public class ChangeSignalTests +{ + [Fact] + public async Task WithoutASignal_AnInstanceServesStaleValuesUntilItsCacheExpires() + { + // The behaviour being fixed, pinned so the test below is known to be about the signal + // rather than about something else keeping the caches in step. + var store = new FakeStore(); + + var first = NewManager(store, NewCache(), signal: null); + var second = NewManager(store, NewCache(), signal: null); + + await first.For().SetAsync(new SignalSettings { Host = "original" }); + await second.For().GetAsync(); // second warms its cache + + await first.For().SetAsync(new SignalSettings { Host = "updated" }); + + Assert.Equal("original", (await second.For().GetAsync()).Host); + } + + /// + /// ChangeCheckInterval is zero here so the token is compared on every read, which + /// makes the test deterministic rather than dependent on wall-clock delay. In a real + /// deployment it is 5 seconds, and that interval is exactly the staleness bound — see + /// . + /// + [Fact] + public async Task WithASharedSignal_AWriteOnOneInstanceInvalidatesTheOther() + { + var store = new FakeStore(); + var signal = NewSignal(); + + var first = NewManager(store, NewCache(), signal, CheckAlways()); + var second = NewManager(store, NewCache(), signal, CheckAlways()); + + await first.For().SetAsync(new SignalSettings { Host = "original" }); + await second.For().GetAsync(); // second warms its cache + + await first.For().SetAsync(new SignalSettings { Host = "updated" }); + + Assert.Equal("updated", (await second.For().GetAsync()).Host); + } + + [Fact] + public async Task ClearOnOneInstance_InvalidatesTheOther() + { + var store = new FakeStore(); + var signal = NewSignal(); + + var first = NewManager(store, NewCache(), signal, CheckAlways()); + var second = NewManager(store, NewCache(), signal, CheckAlways()); + + await first.For().SetAsync(new SignalSettings { Host = "original" }); + await second.For().GetAsync(); + + await first.For().ClearAsync(); + + Assert.Null((await second.For().GetAsync()).Host); + } + + /// + /// A write to one group must not make every other group reload. The token is per group, + /// so this is what proves the key carries the group name. + /// + [Fact] + public async Task AWriteToOneGroup_DoesNotInvalidateAnother() + { + var store = new FakeStore(); + var signal = NewSignal(); + + var first = NewManager(store, NewCache(), signal, CheckAlways()); + var second = NewManager(store, NewCache(), signal, CheckAlways()); + + await first.For().SetAsync(new OtherSignalSettings { Host = "other" }); + await second.For().GetAsync(); + var readsAfterWarmup = store.GroupReads; + + await first.For().SetAsync(new SignalSettings { Host = "unrelated" }); + var readsAfterUnrelatedWrite = store.GroupReads; + + await second.For().GetAsync(); + + // The unrelated write cost the second instance no reload of its own group. + Assert.Equal(readsAfterUnrelatedWrite, store.GroupReads); + Assert.True(readsAfterUnrelatedWrite > readsAfterWarmup); // the write itself did read + } + + [Fact] + public async Task TheTokenIsOnlyRechecked_OncePerCheckInterval() + { + var store = new FakeStore(); + var signal = new CountingSignal(NewSignal()); + + // A long interval, so the second read inside it must not reach the signal. + var mgr = NewManager(store, NewCache(), signal, + new SettingOptions { ChangeCheckInterval = TimeSpan.FromMinutes(5) }); + + await mgr.For().SetAsync(new SignalSettings { Host = "a" }); + await mgr.For().GetAsync(); // loads, records the token + var readsAfterLoad = signal.VersionReads; + + await mgr.For().GetAsync(); + await mgr.For().GetAsync(); + + Assert.Equal(readsAfterLoad, signal.VersionReads); + } + + /// + /// A signal is an optimisation over the cache duration. Failing a read because the + /// signalling store is down would turn a slower reload into an outage. + /// + [Fact] + public async Task AnUnreachableSignal_DoesNotFailReadsOrWrites() + { + var store = new FakeStore(); + var mgr = NewManager(store, NewCache(), new BrokenSignal()); + + var writeError = await Record.ExceptionAsync( + () => mgr.For().SetAsync(new SignalSettings { Host = "a" })); + + var read = await mgr.For().GetAsync(); + + Assert.Null(writeError); + Assert.Equal("a", read.Host); + } + + [Fact] + public async Task TheDefaultSignal_LeavesSingleInstanceBehaviourUnchanged() + { + // No signal registered: a cache hit is a cache hit, with no extra store round trip. + var store = new FakeStore(); + var mgr = NewManager(store, NewCache(), signal: null); + + await mgr.For().SetAsync(new SignalSettings { Host = "a" }); + await mgr.For().GetAsync(); + var readsAfterLoad = store.GroupReads; + + await mgr.For().GetAsync(); + await mgr.For().GetAsync(); + + Assert.Equal(readsAfterLoad, store.GroupReads); + } + + [Fact] + public async Task DistributedCacheSignal_PublishesAFreshTokenPerWrite() + { + var signal = NewSignal(); + + Assert.Null(await signal.GetVersionAsync("Group")); + + await signal.SignalChangeAsync("Group"); + var first = await signal.GetVersionAsync("Group"); + + await signal.SignalChangeAsync("Group"); + var second = await signal.GetVersionAsync("Group"); + + Assert.NotNull(first); + Assert.NotEqual(first, second); + Assert.Null(await signal.GetVersionAsync("AnotherGroup")); + } + + [Fact] + public void SynchronizeThroughDistributedCache_WithoutACache_SaysWhatToRegister() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddSingleton(new Mock.NoopStore()); + services.AddSingleton(sp => sp.GetRequiredService()); + services.AddSettings().SynchronizeThroughDistributedCache(); + + var provider = services.BuildServiceProvider(); + + var ex = Assert.Throws( + () => provider.GetRequiredService()); + + Assert.Contains("IDistributedCache", ex.Message, StringComparison.Ordinal); + Assert.Contains("AddStackExchangeRedisCache", ex.Message, StringComparison.Ordinal); + } + + /// + /// Compare the token on every read. Only reachable by building options directly — the + /// builder rejects a non-positive interval, because in production it would mean a round + /// trip to the signalling store on every settings read. + /// + private static SettingOptions CheckAlways() => new() { ChangeCheckInterval = TimeSpan.Zero }; + + private static SettingCache NewCache() => new(new MemoryCache(new MemoryCacheOptions())); + + private static DistributedCacheSettingChangeSignal NewSignal() + => new(new MemoryDistributedCache( + Microsoft.Extensions.Options.Options.Create(new MemoryDistributedCacheOptions()))); + + private static SettingManager NewManager( + ISettingStore store, ISettingCache cache, ISettingChangeSignal? signal, SettingOptions? options = null) + { + var services = new ServiceCollection(); + if (signal is not null) + services.AddScoped(_ => new PublishingObserver(signal)); + + return new SettingManager(store, cache, services.BuildServiceProvider(), + NullLogger.Instance, encryptor: null, actorAccessor: null, + signal: signal, options: options); + } + + /// + /// Stands in for the ChangeSignalPublisher the builder registers, which is internal + /// to Core. Same one line, so this exercises the same arrangement. + /// + private sealed class PublishingObserver(ISettingChangeSignal signal) : ISettingWriteObserver + { + public Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default) + => signal.SignalChangeAsync(write.GroupName, ct); + } + + private sealed class CountingSignal(ISettingChangeSignal inner) : ISettingChangeSignal + { + private int _versionReads; + + public int VersionReads => Volatile.Read(ref _versionReads); + + public Task GetVersionAsync(string groupName, CancellationToken ct = default) + { + Interlocked.Increment(ref _versionReads); + return inner.GetVersionAsync(groupName, ct); + } + + public Task SignalChangeAsync(string groupName, CancellationToken ct = default) + => inner.SignalChangeAsync(groupName, ct); + } + + private sealed class BrokenSignal : ISettingChangeSignal + { + public Task GetVersionAsync(string groupName, CancellationToken ct = default) + => throw new InvalidOperationException("signalling store is down"); + + public Task SignalChangeAsync(string groupName, CancellationToken ct = default) + => throw new InvalidOperationException("signalling store is down"); + } + + [SettingGroup("signal-settings")] + public class SignalSettings { public string? Host { get; set; } } + + [SettingGroup("other-signal-settings")] + public class OtherSignalSettings { public string? Host { get; set; } } + + internal static class Mock + { + internal sealed class NoopStore : ISettingStore + { + public Task> GetGroupAsync(string g, CancellationToken ct = default) + => Task.FromResult>([]); + public Task GetAsync(string g, string k, CancellationToken ct = default) + => Task.FromResult(null); + public Task UpsertAsync(Setting s, CancellationToken ct = default) => Task.CompletedTask; + public Task UpsertManyAsync(IEnumerable s, CancellationToken ct = default) => Task.CompletedTask; + public Task DeleteAsync(string g, string k, CancellationToken ct = default) => Task.CompletedTask; + public Task DeleteGroupAsync(string g, CancellationToken ct = default) => Task.CompletedTask; + public Task CountAsync(string g, CancellationToken ct = default) => Task.FromResult(0); + } + } + + /// A store whose reads reflect its writes, counting group reads. + private sealed class FakeStore : ISettingStore + { + private readonly Dictionary _data = new(StringComparer.Ordinal); + private int _groupReads; + private static string K(string g, string k) => g + "|" + k; + + public int GroupReads => Volatile.Read(ref _groupReads); + + public Task> GetGroupAsync(string group, CancellationToken ct = default) + { + Interlocked.Increment(ref _groupReads); + return Task.FromResult>( + _data.Values.Where(x => x.Group == group).Select(Copy).ToList()); + } + + public Task GetAsync(string group, string key, CancellationToken ct = default) + => Task.FromResult(_data.TryGetValue(K(group, key), out var s) ? Copy(s) : null); + + public Task UpsertAsync(Setting setting, CancellationToken ct = default) + => UpsertManyAsync([setting], ct); + + public Task UpsertManyAsync(IEnumerable settings, CancellationToken ct = default) + { + foreach (var s in settings) + { + var stored = Copy(s); + stored.RowVersion = Setting.NewRowVersion(); + _data[K(s.Group, s.Key)] = stored; + } + return Task.CompletedTask; + } + + public Task DeleteAsync(string group, string key, CancellationToken ct = default) + { + _data.Remove(K(group, key)); + return Task.CompletedTask; + } + + public Task DeleteGroupAsync(string group, CancellationToken ct = default) + { + foreach (var k in _data.Where(x => x.Value.Group == group).Select(x => x.Key).ToList()) + _data.Remove(k); + return Task.CompletedTask; + } + + public Task CountAsync(string group, CancellationToken ct = default) + => Task.FromResult(_data.Values.Count(x => x.Group == group)); + + private static Setting Copy(Setting s) => new() + { + Id = s.Id, Group = s.Group, Key = s.Key, Value = s.Value, Type = s.Type, + IsEncrypted = s.IsEncrypted, UpdatedAt = s.UpdatedAt, UpdatedBy = s.UpdatedBy, + RowVersion = s.RowVersion + }; + } +} diff --git a/tests/DotNetBoost.Settings.UnitTests/DotNetBoost.Settings.UnitTests.csproj b/tests/DotNetBoost.Settings.UnitTests/DotNetBoost.Settings.UnitTests.csproj index 57dbe40..4547a1a 100644 --- a/tests/DotNetBoost.Settings.UnitTests/DotNetBoost.Settings.UnitTests.csproj +++ b/tests/DotNetBoost.Settings.UnitTests/DotNetBoost.Settings.UnitTests.csproj @@ -8,6 +8,7 @@ + @@ -15,6 +16,7 @@ + diff --git a/tests/DotNetBoost.Settings.UnitTests/FluentSettingValidatorTests.cs b/tests/DotNetBoost.Settings.UnitTests/FluentSettingValidatorTests.cs new file mode 100644 index 0000000..3e37c29 --- /dev/null +++ b/tests/DotNetBoost.Settings.UnitTests/FluentSettingValidatorTests.cs @@ -0,0 +1,125 @@ +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.FluentValidation; +using FluentValidation; +using Microsoft.Extensions.DependencyInjection; + +namespace DotNetBoost.Settings.UnitTests; + +/// +/// is the whole reason a FluentValidation rule reaches +/// SetAsync and the REST API: Core knows only , which is +/// untyped, and this adapts one to the other. The shape it produces — per-property keys, every +/// message for a property in one array — is what the API serialises into a 400 body, so it is +/// part of the contract rather than an implementation detail. +/// +public class FluentSettingValidatorTests +{ + [Fact] + public void CanValidate_IsTrue_OnlyForTheValidatedType() + { + var validator = Bridge(); + + Assert.True(validator.CanValidate(typeof(BridgeSettings))); + Assert.False(validator.CanValidate(typeof(OtherSettings))); + } + + [Fact] + public async Task ValidateAsync_ReportsAValidModelWithNoErrors() + { + var (isValid, errors) = await Bridge().ValidateAsync(new BridgeSettings { Host = "localhost", Port = 25 }); + + Assert.True(isValid); + Assert.Empty(errors); + } + + /// + /// Keys are property names, so a client can attach each message to the field it came from. + /// + [Fact] + public async Task ValidateAsync_KeysErrorsByPropertyName() + { + var (isValid, errors) = await Bridge().ValidateAsync(new BridgeSettings { Host = "localhost", Port = 0 }); + + Assert.False(isValid); + Assert.Equal(nameof(BridgeSettings.Port), Assert.Single(errors).Key); + } + + /// + /// Two rules failing on one property is one key carrying two messages, not the first message + /// or two entries fighting over the same key — the grouping is what makes that true. + /// + [Fact] + public async Task ValidateAsync_GroupsEveryMessageForOnePropertyIntoOneEntry() + { + var (isValid, errors) = await Bridge().ValidateAsync(new BridgeSettings { Host = string.Empty, Port = 25 }); + + Assert.False(isValid); + Assert.Equal(2, errors[nameof(BridgeSettings.Host)].Length); + } + + /// + /// The untyped interface makes this reachable: Core picks a validator by + /// and then hands it an object. Casting + /// blindly would throw from inside FluentValidation with nothing naming the real problem, so + /// the mismatch is reported as a validation failure that says which type was expected. + /// + [Fact] + public async Task ValidateAsync_ReportsATypeMismatchInsteadOfThrowing() + { + var (isValid, errors) = await Bridge().ValidateAsync(new OtherSettings()); + + Assert.False(isValid); + var message = Assert.Single(errors["TypeMismatch"]); + Assert.Contains(nameof(BridgeSettings), message, StringComparison.Ordinal); + Assert.Contains(nameof(OtherSettings), message, StringComparison.Ordinal); + } + + /// + /// The standalone counterpart to UseFluentValidation, for an application wiring + /// services up without the builder. It has to register three things to be useful: the + /// validator itself, the bridge that exposes it to Core, and the schema contributor. + /// + [Fact] + public void AddFluentValidationSettings_RegistersTheValidatorTheBridgeAndTheContributor() + { + var services = new ServiceCollection(); + + services.AddFluentValidationSettings(typeof(BridgeSettingsValidator).Assembly); + + var provider = services.BuildServiceProvider(); + + Assert.NotNull(provider.GetService>()); + Assert.Contains(provider.GetServices(), v => v.CanValidate(typeof(BridgeSettings))); + Assert.IsType( + Assert.Single(provider.GetServices())); + } + + [Fact] + public void AddFluentValidationSettings_RejectsNullArguments() + { + var assembly = typeof(BridgeSettingsValidator).Assembly; + IServiceCollection services = null!; + + Assert.Throws(() => services.AddFluentValidationSettings(assembly)); + Assert.Throws(() => new ServiceCollection().AddFluentValidationSettings(null!)); + } + + private static FluentSettingValidator Bridge() => new(new BridgeSettingsValidator()); + + public class BridgeSettings + { + public string Host { get; set; } = string.Empty; + public int Port { get; set; } + } + + public class OtherSettings; + + public class BridgeSettingsValidator : AbstractValidator + { + public BridgeSettingsValidator() + { + RuleFor(x => x.Host).NotEmpty().MinimumLength(3); + RuleFor(x => x.Port).InclusiveBetween(1, 65_535); + } + } +} diff --git a/tests/DotNetBoost.Settings.UnitTests/FluentValidationSchemaContributorTests.cs b/tests/DotNetBoost.Settings.UnitTests/FluentValidationSchemaContributorTests.cs new file mode 100644 index 0000000..dfeedd3 --- /dev/null +++ b/tests/DotNetBoost.Settings.UnitTests/FluentValidationSchemaContributorTests.cs @@ -0,0 +1,184 @@ +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.FluentValidation; +using FluentValidation; +using Microsoft.Extensions.DependencyInjection; + +namespace DotNetBoost.Settings.UnitTests; + +/// +/// The constraints a generated form needs are the valuable half of a schema, and +/// cannot supply them — it answers only "is this valid", +/// never "what would be". This reads them out of FluentValidation's own descriptor. +/// +public class FluentValidationSchemaContributorTests +{ + [Fact] + public void Describe_PublishesInclusiveBetweenAsMinAndMax() + { + var constraints = Describe()[nameof(FormSettings.Port)]; + + Assert.Equal(1, constraints["min"]); + Assert.Equal(65_535, constraints["max"]); + } + + [Fact] + public void Describe_PublishesLengthBounds() + { + var constraints = Describe()[nameof(FormSettings.Host)]; + + Assert.Equal(3, constraints["minLength"]); + Assert.Equal(255, constraints["maxLength"]); + } + + /// + /// An exact length is both bounds at once. It matters that this is read as exact rather than + /// falling through to the general length rule, which would publish only a minimum and let a + /// client accept a value the server rejects. + /// + [Fact] + public void Describe_PublishesAnExactLengthAsBothBounds() + { + var constraints = Describe()[nameof(FormSettings.Region)]; + + Assert.Equal(2, constraints["minLength"]); + Assert.Equal(2, constraints["maxLength"]); + } + + /// + /// A one-sided length rule publishes the bound it states and stays silent on the other, so a + /// client does not infer a floor of zero as a real constraint. + /// + [Fact] + public void Describe_PublishesAMaximumLengthAlone() + { + var constraints = Describe()[nameof(FormSettings.Label)]; + + Assert.Equal(50, constraints["maxLength"]); + Assert.DoesNotContain("minLength", constraints.Keys, StringComparer.Ordinal); + } + + [Fact] + public void Describe_PublishesAMinimumLengthAlone() + { + var constraints = Describe()[nameof(FormSettings.Slug)]; + + Assert.Equal(4, constraints["minLength"]); + Assert.DoesNotContain("maxLength", constraints.Keys, StringComparer.Ordinal); + } + + /// + /// The four comparisons split along inclusive/exclusive rather than collapsing into min/max: + /// a client that renders exclusiveMax as max would allow the boundary value the + /// server refuses. + /// + [Fact] + public void Describe_DistinguishesInclusiveFromExclusiveComparisons() + { + var schema = Describe(); + + Assert.Equal(32, schema[nameof(FormSettings.Workers)]["max"]); + Assert.Equal(0, schema[nameof(FormSettings.Timeout)]["exclusiveMin"]); + } + + [Fact] + public void Describe_PublishesNotEmptyAsRequired() + => Assert.Equal(true, Describe()[nameof(FormSettings.Host)]["required"]); + + [Fact] + public void Describe_PublishesComparisonsAgainstALiteral() + { + var retries = Describe()[nameof(FormSettings.Retries)]; + + Assert.Equal(0, retries["min"]); + Assert.Equal(10, retries["exclusiveMax"]); + } + + [Fact] + public void Describe_PublishesARegularExpression() + => Assert.Equal("^[a-z]+$", Describe()[nameof(FormSettings.Code)]["pattern"]); + + /// + /// A Must(...) is opaque by nature. Inventing a bound for it would publish a lie + /// that a client would then believe, so it is skipped and the property simply carries no + /// constraints. + /// + [Fact] + public void Describe_SkipsRulesItCannotRead() + => Assert.DoesNotContain(nameof(FormSettings.Opaque), Describe().Keys, StringComparer.Ordinal); + + [Fact] + public void CanDescribe_IsFalse_ForATypeWithNoValidator() + { + var contributor = new FluentValidationSchemaContributor(Provider()); + + Assert.False(contributor.CanDescribe(typeof(UnvalidatedSettings))); + Assert.Empty(contributor.Describe(typeof(UnvalidatedSettings))); + } + + /// + /// Single() is about this one registration being idempotent — one contributor covers + /// every settings type, so UseFluentValidation registers it once however many + /// validators it finds, and twice over is still once. It says nothing about how many + /// contributors an application may have: the schema endpoint merges every applicable one. + /// + [Fact] + public void UseFluentValidation_RegistersExactlyOneContributor_HoweverOftenItIsCalled() + { + var services = new ServiceCollection(); + var assembly = typeof(FormSettingsValidator).Assembly; + + services.AddSettings() + .UseFluentValidation(assembly) + .UseFluentValidation(assembly); + + var contributor = services.BuildServiceProvider().GetServices().Single(); + + Assert.IsType(contributor); + } + + private static IReadOnlyDictionary> Describe() + => new FluentValidationSchemaContributor(Provider()).Describe(typeof(FormSettings)); + + private static ServiceProvider Provider() + { + var services = new ServiceCollection(); + services.AddTransient, FormSettingsValidator>(); + return services.BuildServiceProvider(); + } + + public class FormSettings + { + public string Host { get; set; } = string.Empty; + public int Port { get; set; } + public int Retries { get; set; } + public string Code { get; set; } = string.Empty; + public string Opaque { get; set; } = string.Empty; + public string Region { get; set; } = string.Empty; + public string Label { get; set; } = string.Empty; + public string Slug { get; set; } = string.Empty; + public int Workers { get; set; } + public int Timeout { get; set; } + } + + public class UnvalidatedSettings + { + public string Host { get; set; } = string.Empty; + } + + public class FormSettingsValidator : AbstractValidator + { + public FormSettingsValidator() + { + RuleFor(x => x.Host).NotEmpty().Length(3, 255); + RuleFor(x => x.Port).InclusiveBetween(1, 65_535); + RuleFor(x => x.Retries).GreaterThanOrEqualTo(0).LessThan(10); + RuleFor(x => x.Code).Matches("^[a-z]+$"); + RuleFor(x => x.Opaque).Must(v => v.Length % 2 == 0); + RuleFor(x => x.Region).Length(2); + RuleFor(x => x.Label).MaximumLength(50); + RuleFor(x => x.Slug).MinimumLength(4); + RuleFor(x => x.Workers).LessThanOrEqualTo(32); + RuleFor(x => x.Timeout).GreaterThan(0); + } + } +} diff --git a/tests/DotNetBoost.Settings.UnitTests/KeyRotationTests.cs b/tests/DotNetBoost.Settings.UnitTests/KeyRotationTests.cs index 5843518..ba8534d 100644 --- a/tests/DotNetBoost.Settings.UnitTests/KeyRotationTests.cs +++ b/tests/DotNetBoost.Settings.UnitTests/KeyRotationTests.cs @@ -192,6 +192,59 @@ public async Task UndecryptableValue_FallsBackToDefault_WhenExplicitlyIgnored() Assert.Equal("smtp.example.com", model.Host); // unencrypted properties still load } + /// + /// The write path mapped the stored rows inside a bare catch { previous = new T(); }, + /// so it swallowed this exception and compared against compile-time defaults instead. That + /// read every encrypted property as changed, wrote them all back under the current key and + /// told a write observer they had changed — silently losing the stranded value, which is the + /// failure the throw exists to prevent. Nothing was logged either, so the only symptom was + /// a secret quietly replaced by its default. + /// + [Fact] + public async Task UndecryptableValue_ThrowsOnAGroupWrite_AndWritesNothing() + { + var mgr = Build(out var store); + + var ex = await Assert.ThrowsAsync( + () => mgr.For().SetAsync(new SecretSettings { Host = "smtp.other.com" })); + + Assert.Equal(nameof(SecretSettings.ApiKey), ex.Key); + store.Verify(x => x.UpsertManyAsync(It.IsAny>(), It.IsAny()), Times.Never); + store.Verify(x => x.UpsertAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + /// + /// Same fault on the single-property overload, which mapped the group twice under two bare + /// catches. Writing one unrelated property must not be the operation that discards a secret + /// it never names. + /// + [Fact] + public async Task UndecryptableValue_ThrowsOnASinglePropertyWrite_AndWritesNothing() + { + var mgr = Build(out var store); + + var ex = await Assert.ThrowsAsync( + () => mgr.For().SetAsync(x => x.Host, "smtp.other.com")); + + Assert.Equal(nameof(SecretSettings.ApiKey), ex.Key); + store.Verify(x => x.UpsertManyAsync(It.IsAny>(), It.IsAny()), Times.Never); + store.Verify(x => x.UpsertAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + /// + /// Opting out still opts out: the fallback to defaults is a choice the caller can make, and + /// the fix narrows what is caught rather than removing the fallback. + /// + [Fact] + public async Task UndecryptableValue_StillWrites_WhenExplicitlyIgnored() + { + var mgr = Build(out var store, o => o.ThrowOnDecryptionFailure = false); + + await mgr.For().SetAsync(x => x.Host, "smtp.other.com"); + + store.Verify(x => x.UpsertManyAsync(It.IsAny>(), It.IsAny()), Times.Once); + } + [Fact] public void IgnoreDecryptionFailures_FlowsFromTheBuilder() { @@ -220,8 +273,8 @@ private static SettingManager Build(out Mock store, Action(); - SecretSettings? miss = null; - cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); + Setting[]? miss = null; + cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); var options = new SettingOptions(); configure?.Invoke(options); @@ -229,6 +282,6 @@ private static SettingManager Build(out Mock store, Action.Instance, new AesSettingEncryptor(Convert.ToBase64String(RandomNumberGenerator.GetBytes(32))), - null, options); + actorAccessor: null, signal: null, options: options); } } diff --git a/tests/DotNetBoost.Settings.UnitTests/ProjectorRegistrationTests.cs b/tests/DotNetBoost.Settings.UnitTests/ProjectorRegistrationTests.cs new file mode 100644 index 0000000..1805097 --- /dev/null +++ b/tests/DotNetBoost.Settings.UnitTests/ProjectorRegistrationTests.cs @@ -0,0 +1,152 @@ +using DotNetBoost.Settings.Core; +using DotNetBoost.Settings.Core.Interfaces; +using Microsoft.Extensions.DependencyInjection; + +namespace DotNetBoost.Settings.UnitTests; + +/// +/// Projectors do not compose: ProjectAsync takes the group, so a second projector has +/// nowhere to receive the first's output. Two registrations for one group therefore meant the +/// last silently won and the other was never constructed — a response quietly missing half its +/// shape. Build() rejects it, in the same place that already rejects duplicate group +/// names and routes. +/// +public class ProjectorRegistrationTests +{ + [Fact] + public void TwoDifferentProjectors_ForOneGroup_ThrowsAndNamesBoth() + { + var ex = Assert.Throws(() => + SettingBuilderValidator.ValidateProjectors(Services(s => + { + s.AddScoped, FirstAlphaProjector>(); + s.AddScoped, SecondAlphaProjector>(); + }))); + + Assert.Contains(nameof(FirstAlphaProjector), ex.Message, StringComparison.Ordinal); + Assert.Contains(nameof(SecondAlphaProjector), ex.Message, StringComparison.Ordinal); + Assert.Contains(nameof(AlphaSettings), ex.Message, StringComparison.Ordinal); + } + + /// The error should say what to do instead, not only that something is wrong. + [Fact] + public void TheError_PointsAtReplaceProjector() + { + var ex = Assert.Throws(() => + SettingBuilderValidator.ValidateProjectors(Services(s => + { + s.AddScoped, FirstAlphaProjector>(); + s.AddScoped, SecondAlphaProjector>(); + }))); + + Assert.Contains("ReplaceProjector", ex.Message, StringComparison.Ordinal); + } + + /// + /// An idempotent mistake rather than an ambiguity — both registrations name the same + /// implementation, so there is nothing to be wrong about. + /// + [Fact] + public void TheSameProjectorTwice_DoesNotThrow() + => Assert.Null(Record.Exception(() => + SettingBuilderValidator.ValidateProjectors(Services(s => + { + s.AddScoped, FirstAlphaProjector>(); + s.AddScoped, FirstAlphaProjector>(); + })))); + + [Fact] + public void OneProjectorEach_ForTwoGroups_DoesNotThrow() + => Assert.Null(Record.Exception(() => + SettingBuilderValidator.ValidateProjectors(Services(s => + { + s.AddScoped, FirstAlphaProjector>(); + s.AddScoped, BetaProjector>(); + })))); + + [Fact] + public void NoProjectors_DoesNotThrow() + => Assert.Null(Record.Exception(() => SettingBuilderValidator.ValidateProjectors(Services(_ => { })))); + + /// + /// A factory registration has no ImplementationType, so it cannot be compared with + /// anything. Folding two unknowns together would silently pass the case the guard most + /// wants to catch. + /// + [Fact] + public void TwoFactoryRegistrations_ForOneGroup_StillThrows() + { + var ex = Assert.Throws(() => + SettingBuilderValidator.ValidateProjectors(Services(s => + { + s.AddScoped>(_ => new FirstAlphaProjector()); + s.AddScoped>(_ => new SecondAlphaProjector()); + }))); + + Assert.Contains("factory", ex.Message, StringComparison.Ordinal); + } + + [Fact] + public void AFactoryAlongsideAType_ForOneGroup_StillThrows() + => Assert.Throws(() => + SettingBuilderValidator.ValidateProjectors(Services(s => + { + s.AddScoped, FirstAlphaProjector>(); + s.AddScoped>(_ => new SecondAlphaProjector()); + }))); + + [Fact] + public void ReplaceProjector_AfterUseProjector_LeavesExactlyOne_AndDoesNotThrow() + { + var services = new ServiceCollection(); + services.AddSettings() + .UseProjector() + .ReplaceProjector(); + + var registered = services + .Where(d => d.ServiceType == typeof(ISettingProjector)) + .ToList(); + + Assert.Single(registered); + Assert.Equal(typeof(SecondAlphaProjector), registered[0].ImplementationType); + Assert.Null(Record.Exception(() => SettingBuilderValidator.ValidateProjectors(services))); + } + + // A Build()-level test is not reachable from this assembly: Build() scans every loaded + // assembly for [SettingGroup] classes first, and this one deliberately contains broken + // fixtures (EmptyRouteSettings, DupRouteA/B) for the tests above it, so Build() always + // throws on those before reaching the projector check. The check itself is covered here + // directly; the one line wiring it into Build() is covered by inspection. + + [Fact] + public void ValidateProjectors_NullServices_Throws() + => Assert.Throws(() => SettingBuilderValidator.ValidateProjectors(null!)); + + private static ServiceCollection Services(Action configure) + { + var services = new ServiceCollection(); + configure(services); + return services; + } + + public class AlphaSettings { public string? Host { get; set; } } + public class BetaSettings { public string? Host { get; set; } } + + private sealed class FirstAlphaProjector : ISettingProjector + { + public Task ProjectAsync(AlphaSettings group, CancellationToken ct = default) + => Task.FromResult(group); + } + + private sealed class SecondAlphaProjector : ISettingProjector + { + public Task ProjectAsync(AlphaSettings group, CancellationToken ct = default) + => Task.FromResult(group); + } + + private sealed class BetaProjector : ISettingProjector + { + public Task ProjectAsync(BetaSettings group, CancellationToken ct = default) + => Task.FromResult(group); + } +} diff --git a/tests/DotNetBoost.Settings.UnitTests/SettingGroupNameTests.cs b/tests/DotNetBoost.Settings.UnitTests/SettingGroupNameTests.cs index bd1acc3..867fc3d 100644 --- a/tests/DotNetBoost.Settings.UnitTests/SettingGroupNameTests.cs +++ b/tests/DotNetBoost.Settings.UnitTests/SettingGroupNameTests.cs @@ -100,7 +100,7 @@ public async Task CacheKey_FollowsTheGroupName_NotTheClassName() await mgr.For().GetAsync(); - cache.Verify(x => x.Set("dnb:setting:billing-v1", It.IsAny(), It.IsAny()), Times.Once); + cache.Verify(x => x.Set("dnb:setting:billing-v1", It.IsAny(), It.IsAny()), Times.Once); } [Fact] @@ -111,7 +111,7 @@ public async Task CacheKey_IsUnchanged_ForGroupsWithoutAnExplicitName() await mgr.For().GetAsync(); - cache.Verify(x => x.Set("dnb:setting:RouteOnlySettings", It.IsAny(), It.IsAny()), Times.Once); + cache.Verify(x => x.Set("dnb:setting:RouteOnlySettings", It.IsAny(), It.IsAny()), Times.Once); } private static (Mock, ISettingManager) Build() where T : new() @@ -126,8 +126,8 @@ public async Task CacheKey_IsUnchanged_ForGroupsWithoutAnExplicitName() store.Setup(x => x.GetGroupAsync(It.IsAny(), It.IsAny())).ReturnsAsync([]); var cache = new Mock(); - T? miss = default; - cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); + Setting[]? miss = null; + cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); var mgr = new SettingManager(store.Object, cache.Object, new ServiceCollection().BuildServiceProvider(), NullLogger.Instance); diff --git a/tests/DotNetBoost.Settings.UnitTests/SettingManagerTests.cs b/tests/DotNetBoost.Settings.UnitTests/SettingManagerTests.cs index 028a36a..d50f4d6 100644 --- a/tests/DotNetBoost.Settings.UnitTests/SettingManagerTests.cs +++ b/tests/DotNetBoost.Settings.UnitTests/SettingManagerTests.cs @@ -45,8 +45,7 @@ public async Task GetAsync_ReturnsDefault_WhenStoreEmpty() public async Task GetAsync_UsesCache_WhenHit() { var (store, cache, mgr) = Build(); - var cached = new MailSettings { Port = 9999 }; - cache.Setup(x => x.TryGetValue("dnb:setting:MailSettings", out cached)).Returns(true); + SetCachedRows(cache, Rows("MailSettings", ("Port", "9999", "System.Int32"))); var result = await mgr.For().GetAsync(); @@ -58,8 +57,7 @@ public async Task GetAsync_UsesCache_WhenHit() public async Task GetAsync_BypassesCache_WhenRefreshCacheTrue() { var (store, cache, mgr) = Build(); - var cached = new MailSettings { Port = 9999 }; - cache.Setup(x => x.TryGetValue("dnb:setting:MailSettings", out cached)).Returns(true); + SetCachedRows(cache, Rows("MailSettings", ("Port", "9999", "System.Int32"))); store.Setup(x => x.GetGroupAsync("MailSettings", default)).ReturnsAsync([]); await mgr.For().GetAsync(refreshCache: true); @@ -68,15 +66,56 @@ public async Task GetAsync_BypassesCache_WhenRefreshCacheTrue() } [Fact] - public async Task GetAsync_StoresResultInCache() + public async Task GetAsync_StoresTheStoredRowsInCache_NotTheModel() { + // What goes into the cache decides what every later reader is served. Rows are + // immutable as far as the manager is concerned; a model is not. var (store, cache, mgr) = Build(); - SetCacheMiss(cache); - store.Setup(x => x.GetGroupAsync("MailSettings", default)).ReturnsAsync([]); + SetCacheMiss(cache); + store.Setup(x => x.GetGroupAsync("MailSettings", default)) + .ReturnsAsync(Rows("MailSettings", ("Port", "587", "System.Int32"))); await mgr.For().GetAsync(); - cache.Verify(x => x.Set("dnb:setting:MailSettings", It.IsAny(), It.IsAny()), Times.Once); + cache.Verify(x => x.Set("dnb:setting:MailSettings", + It.Is(r => r.Length == 1 && r[0].Key == "Port"), It.IsAny()), Times.Once); + } + + /// + /// The whole point of caching rows rather than the model: two reads of a group must not + /// share an object, or assigning to a property of one quietly rewrites what every later + /// reader — including GET /api/settings/{group} — is served. + /// + [Fact] + public async Task GetAsync_MutatingTheResult_DoesNotAffectLaterReads() + { + var (store, cache, mgr) = Build(); + UseRealCache(cache); + store.Setup(x => x.GetGroupAsync("MailSettings", default)) + .ReturnsAsync(Rows("MailSettings", ("Host", "smtp.example.com", "System.String"))); + + var first = await mgr.For().GetAsync(); + first.Host = "mutated.example.com"; + + var second = await mgr.For().GetAsync(); + + Assert.Equal("smtp.example.com", second.Host); + Assert.NotSame(first, second); + store.Verify(x => x.GetGroupAsync("MailSettings", default), Times.Once); // still one round trip + } + + [Fact] + public async Task GetAsync_MutatingAPropertyRead_DoesNotAffectLaterReads() + { + var (store, cache, mgr) = Build(); + UseRealCache(cache); + store.Setup(x => x.GetGroupAsync("MailSettings", default)) + .ReturnsAsync(Rows("MailSettings", ("Port", "587", "System.Int32"))); + + var model = await mgr.For().GetAsync(); + model.Port = 2525; + + Assert.Equal(587, await mgr.For().GetAsync(m => m.Port)); } [Fact] @@ -208,6 +247,26 @@ private static void SetCacheMiss(Mock cache) cache.Setup(x => x.TryGetValue(It.IsAny(), out nothing)).Returns(false); } + private static void SetCachedRows(Mock cache, List rows) + { + var cached = rows.ToArray(); + cache.Setup(x => x.TryGetValue("dnb:setting:MailSettings", out cached!)).Returns(true); + } + + /// Backs the mock with a real dictionary, so Set is visible to a later TryGetValue. + private static void UseRealCache(Mock cache) + { + var backing = new Dictionary(StringComparer.Ordinal); + + cache.Setup(x => x.Set(It.IsAny(), It.IsAny(), It.IsAny())) + .Callback((k, v, _) => backing[k] = v); + + cache.Setup(x => x.TryGetValue(It.IsAny(), out It.Ref.IsAny)) + .Returns(new TryGetRows((string k, out Setting[]? v) => backing.TryGetValue(k, out v))); + } + + private delegate bool TryGetRows(string key, out Setting[]? value); + private static List Rows(string group, params (string Key, string Value, string Type)[] data) => data.Select(d => new Setting { Group = group, Key = d.Key, Value = d.Value, Type = d.Type }).ToList(); diff --git a/tests/DotNetBoost.Settings.UnitTests/SettingOptionsTests.cs b/tests/DotNetBoost.Settings.UnitTests/SettingOptionsTests.cs new file mode 100644 index 0000000..526a704 --- /dev/null +++ b/tests/DotNetBoost.Settings.UnitTests/SettingOptionsTests.cs @@ -0,0 +1,76 @@ +using DotNetBoost.Settings.Core; +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.Core.Models; +using Microsoft.Extensions.DependencyInjection; +using Moq; + +namespace DotNetBoost.Settings.UnitTests; + +/// Regression tests for WithCacheDuration() reaching the manager. +public class SettingOptionsTests +{ + [Fact] + public async Task WithCacheDuration_IsAppliedToCachedEntries() + { + var (mgr, cache) = BuildWith(b => b.WithCacheDuration(TimeSpan.FromMinutes(5))); + + await mgr.For().GetAsync(); + + cache.Verify(x => x.Set("dnb:setting:OptionsSettings", + It.IsAny(), TimeSpan.FromMinutes(5)), Times.Once); + } + + [Fact] + public async Task CacheDuration_DefaultsToTenMinutes() + { + var (mgr, cache) = BuildWith(_ => { }); + + await mgr.For().GetAsync(); + + cache.Verify(x => x.Set("dnb:setting:OptionsSettings", + It.IsAny(), TimeSpan.FromMinutes(10)), Times.Once); + } + + [Fact] + public void WithCacheDuration_RejectsNonPositiveDuration() + { + var builder = new ServiceCollection().AddSettings(); + Assert.Throws(() => builder.WithCacheDuration(TimeSpan.Zero)); + Assert.Throws(() => builder.WithCacheDuration(TimeSpan.FromSeconds(-1))); + } + + [Fact] + public void AddSettings_RegistersOptions() + { + var services = new ServiceCollection(); + services.AddSettings().WithCacheDuration(TimeSpan.FromHours(2)); + + var options = services.BuildServiceProvider().GetRequiredService(); + + Assert.Equal(TimeSpan.FromHours(2), options.CacheDuration); + } + + // Resolves the manager through DI so the test covers the real registration path, + // which is where the builder-to-manager wiring was previously lost. + private static (ISettingManager, Mock) BuildWith(Action configure) + { + var store = new Mock(); + store.Setup(x => x.GetGroupAsync("OptionsSettings", It.IsAny())) + .ReturnsAsync([]); + + var cache = new Mock(); + Setting[]? miss = null; + cache.Setup(x => x.TryGetValue(It.IsAny(), out miss)).Returns(false); + + var services = new ServiceCollection(); + services.AddSingleton(store.Object); + services.AddSingleton(cache.Object); // registered first so AddSettings' TryAdd defers + services.AddLogging(); + + configure(services.AddSettings()); + + return (services.BuildServiceProvider().GetRequiredService(), cache); + } + + public class OptionsSettings { public int Port { get; set; } } +} diff --git a/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs b/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs new file mode 100644 index 0000000..658f63c --- /dev/null +++ b/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs @@ -0,0 +1,391 @@ +using DotNetBoost.Settings.Core.Attributes; +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.Core.Models; +using DotNetBoost.Settings.Core.Services; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using System.Security.Cryptography; + +namespace DotNetBoost.Settings.UnitTests; + +/// +/// The write hook that replaced the audit trail. These use a stateful fake store rather than +/// a mock: the before-values an observer is handed come from a read the write itself performs, +/// which is invisible to a store whose reads are unaffected by its writes. +/// +public class SettingWriteObserverTests +{ + [Fact] + public async Task Write_RaisesOneNotification_PerSave() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Host = "a", Port = 25 }); + + var write = Assert.Single(observer.Writes); + Assert.Equal(nameof(ObservedSettings), write.GroupName); + Assert.Equal("observed", write.Route); + Assert.Equal(SettingWriteKind.Updated, write.Kind); + } + + /// + /// The N properties of one save are one edit. Separate notifications per row would make + /// that unrecoverable downstream, which is why the correlation id is shared. + /// + [Fact] + public async Task Write_CarriesEveryChangedProperty_UnderOneCorrelationId() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Host = "a", Port = 25 }); + + var write = Assert.Single(observer.Writes); + Assert.Equal( + [nameof(ObservedSettings.Host), nameof(ObservedSettings.Port)], + write.Changes.Select(c => c.PropertyName).OrderBy(n => n, StringComparer.Ordinal)); + Assert.False(string.IsNullOrWhiteSpace(write.CorrelationId)); + } + + [Fact] + public async Task Write_SeparateSaves_GetDifferentCorrelationIds() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Host = "a" }); + await mgr.For().SetAsync(new ObservedSettings { Host = "b" }); + + Assert.Equal(2, observer.Writes.Count); + Assert.NotEqual(observer.Writes[0].CorrelationId, observer.Writes[1].CorrelationId); + } + + [Fact] + public async Task Change_CarriesBeforeAndAfterValues() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Host = "old.example.com" }); + observer.Writes.Clear(); + await mgr.For().SetAsync(new ObservedSettings { Host = "new.example.com" }); + + var change = Single(observer, nameof(ObservedSettings.Host)); + Assert.Equal("old.example.com", change.OldValue); + Assert.Equal("new.example.com", change.NewValue); + } + + [Fact] + public async Task Change_FirstWrite_HasNullOldValue() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Host = "first.example.com" }); + + var change = Single(observer, nameof(ObservedSettings.Host)); + Assert.Null(change.OldValue); + Assert.Equal("first.example.com", change.NewValue); + } + + /// + /// The library decides what is sensitive; an observer only learns that it changed. An + /// observer is exactly the component that forwards writes somewhere else, so a secret + /// must not be reachable through it even by accident. + /// + [Fact] + public async Task Change_SensitiveProperty_IsRedactedWithNoValues() + { + var key = Convert.ToBase64String(RandomNumberGenerator.GetBytes(32)); + var (mgr, _, observer) = Build(new AesSettingEncryptor(key)); + + await mgr.For().SetAsync(new ObservedSettings { Secret = "s3cret" }); + observer.Writes.Clear(); + await mgr.For().SetAsync(new ObservedSettings { Secret = "n3wer" }); + + var change = Single(observer, nameof(ObservedSettings.Secret)); + Assert.True(change.IsRedacted); + Assert.Null(change.OldValue); + Assert.Null(change.NewValue); + } + + [Fact] + public async Task Change_SensitiveProperty_IsRedacted_EvenWithNoEncryptorConfigured() + { + // Redaction follows [Sensitive], not whether the value happened to be encrypted at + // rest. Tying it to the encryptor would leak every secret in an app without one. + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Secret = "s3cret" }); + + var change = Single(observer, nameof(ObservedSettings.Secret)); + Assert.True(change.IsRedacted); + Assert.Null(change.NewValue); + } + + [Fact] + public async Task Write_ReportsOnlyTheChangedProperty() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Host = "a", Port = 25 }); + observer.Writes.Clear(); + await mgr.For().SetAsync(new ObservedSettings { Host = "a", Port = 587 }); + + var change = Assert.Single(Assert.Single(observer.Writes).Changes); + Assert.Equal(nameof(ObservedSettings.Port), change.PropertyName); + Assert.Equal("25", change.OldValue); + Assert.Equal("587", change.NewValue); + } + + [Fact] + public async Task Write_ThatChangesNothing_RaisesNoNotification() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Host = "same.example.com" }); + observer.Writes.Clear(); + await mgr.For().SetAsync(new ObservedSettings { Host = "same.example.com" }); + + Assert.Empty(observer.Writes); + } + + [Fact] + public async Task Write_UnchangedSensitiveProperty_RaisesNothing_DespiteFreshCiphertext() + { + // The real AES-GCM encryptor, not a stub: it draws a new nonce per call, so an + // unchanged secret serialises to different ciphertext on every save. Comparing + // ciphertext would report a change here; comparing plaintext must not. + var key = Convert.ToBase64String(RandomNumberGenerator.GetBytes(32)); + var encryptor = new AesSettingEncryptor(key); + + // Guards the premise: re-encrypting the same plaintext really does produce different + // bytes, so comparing stored ciphertext would report a change on every save. + Assert.NotEqual(encryptor.Encrypt("s3cret"), encryptor.Encrypt("s3cret")); + + var (mgr, store, observer) = Build(encryptor); + + await mgr.For().SetAsync(new ObservedSettings { Secret = "s3cret" }); + var firstCiphertext = (await store.GetAsync(nameof(ObservedSettings), nameof(ObservedSettings.Secret)))!.Value; + observer.Writes.Clear(); + + await mgr.For().SetAsync(new ObservedSettings { Secret = "s3cret" }); + + Assert.Empty(observer.Writes); + // Unchanged properties are not rewritten at all, so the stored bytes stay put. + Assert.Equal( + firstCiphertext, + (await store.GetAsync(nameof(ObservedSettings), nameof(ObservedSettings.Secret)))!.Value); + } + + /// + /// The reason exists. Deleting every row in a group is the + /// most destructive thing the library does, and under the old audit trail it was the one + /// operation that recorded nothing at all. + /// + [Fact] + public async Task Clear_RaisesAClearedNotification_WithNoChanges() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Host = "a", Port = 25 }); + observer.Writes.Clear(); + + await mgr.For().ClearAsync(); + + var write = Assert.Single(observer.Writes); + Assert.Equal(SettingWriteKind.Cleared, write.Kind); + Assert.Empty(write.Changes); + Assert.Equal(nameof(ObservedSettings), write.GroupName); + } + + /// + /// An observer runs after the store write committed and the cache entry was evicted, so + /// failing the call would report a write that in fact succeeded. + /// + [Fact] + public async Task Observer_ThatThrows_DoesNotFailTheWrite_AndLaterObserversStillRun() + { + var throwing = new ThrowingObserver(); + var after = new RecordingObserver(); + var store = new FakeStore(); + + var services = new ServiceCollection(); + services.AddScoped(_ => throwing); + services.AddScoped(_ => after); + + var mgr = new SettingManager(store, new NoCache(), services.BuildServiceProvider(), + NullLogger.Instance); + + var ex = await Record.ExceptionAsync( + () => mgr.For().SetAsync(new ObservedSettings { Host = "a" })); + + Assert.Null(ex); + Assert.True(throwing.WasCalled); + Assert.Single(after.Writes); + + // And the write really did land, rather than being rolled back with the notification. + Assert.Equal("a", (await store.GetAsync(nameof(ObservedSettings), nameof(ObservedSettings.Host)))!.Value); + } + + [Fact] + public async Task Actor_IsNull_WhenNoAccessorIsRegistered() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Host = "a" }); + + Assert.Null(Assert.Single(observer.Writes).Actor); + } + + [Fact] + public async Task Actor_CarriesTheAccessorsValue_WhenOneIsRegistered() + { + var (mgr, _, observer) = Build(actor: new StubActorAccessor("alex@example.com")); + + await mgr.For().SetAsync(new ObservedSettings { Host = "a" }); + + Assert.Equal("alex@example.com", Assert.Single(observer.Writes).Actor); + } + + [Fact] + public async Task LoggingObserver_LogsPropertyNames_AndNeverValues() + { + var logger = new CapturingLogger(); + var observer = new LoggingSettingWriteObserver(logger); + + await observer.OnWrittenAsync(new SettingWrite( + "ObservedSettings", "observed", SettingWriteKind.Updated, + [ + new SettingChange("Host", "old.example.com", "new.example.com", IsRedacted: false), + new SettingChange("Secret", null, null, IsRedacted: true) + ], + Actor: "alex@example.com", CorrelationId: "corr-1", OccurredAt: DateTime.UtcNow)); + + var line = Assert.Single(logger.Messages); + Assert.Contains("Host", line, StringComparison.Ordinal); + Assert.Contains("Secret", line, StringComparison.Ordinal); + Assert.Contains("alex@example.com", line, StringComparison.Ordinal); + + // The property names, never what they were set to. + Assert.DoesNotContain("old.example.com", line, StringComparison.Ordinal); + Assert.DoesNotContain("new.example.com", line, StringComparison.Ordinal); + } + + private static SettingChange Single(RecordingObserver observer, string propertyName) + => Assert.Single(observer.Writes).Changes.Single(c => c.PropertyName == propertyName); + + private static (ISettingManager, FakeStore, RecordingObserver) Build( + ISettingEncryptor? encryptor = null, ISettingActorAccessor? actor = null) + { + var store = new FakeStore(); + var observer = new RecordingObserver(); + + var services = new ServiceCollection(); + services.AddScoped(_ => observer); + + var mgr = new SettingManager(store, new NoCache(), services.BuildServiceProvider(), + NullLogger.Instance, encryptor, actor); + + return (mgr, store, observer); + } + + [SettingGroup("observed")] + public class ObservedSettings + { + public string? Host { get; set; } + public int Port { get; set; } + [Sensitive] public string? Secret { get; set; } + } + + private sealed class RecordingObserver : ISettingWriteObserver + { + public List Writes { get; } = []; + + public Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default) + { + Writes.Add(write); + return Task.CompletedTask; + } + } + + private sealed class ThrowingObserver : ISettingWriteObserver + { + public bool WasCalled { get; private set; } + + public Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default) + { + WasCalled = true; + throw new InvalidOperationException("observer is down"); + } + } + + private sealed class StubActorAccessor(string? actor) : ISettingActorAccessor + { + public string? GetActor() => actor; + } + + private sealed class CapturingLogger : ILogger + { + public List Messages { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => Messages.Add(formatter(state, exception)); + } + + /// A store whose reads reflect its writes, like a real one. + private sealed class FakeStore : ISettingStore + { + private readonly Dictionary _data = new(StringComparer.Ordinal); + private static string K(string g, string k) => g + "|" + k; + + public Task> GetGroupAsync(string group, CancellationToken ct = default) + => Task.FromResult>(_data.Values.Where(x => x.Group == group).ToList()); + + public Task GetAsync(string group, string key, CancellationToken ct = default) + => Task.FromResult(_data.GetValueOrDefault(K(group, key))); + + public Task UpsertAsync(Setting setting, CancellationToken ct = default) + { + _data[K(setting.Group, setting.Key)] = Copy(setting); + return Task.CompletedTask; + } + + public Task UpsertManyAsync(IEnumerable settings, CancellationToken ct = default) + { + foreach (var s in settings) _data[K(s.Group, s.Key)] = Copy(s); + return Task.CompletedTask; + } + + public Task DeleteAsync(string group, string key, CancellationToken ct = default) + { + _data.Remove(K(group, key)); + return Task.CompletedTask; + } + + public Task DeleteGroupAsync(string group, CancellationToken ct = default) + { + foreach (var k in _data.Where(x => x.Value.Group == group).Select(x => x.Key).ToList()) + _data.Remove(k); + return Task.CompletedTask; + } + + public Task CountAsync(string group, CancellationToken ct = default) + => Task.FromResult(_data.Values.Count(x => x.Group == group)); + + // Persist a copy so later mutations of the caller's instance cannot alter stored state. + private static Setting Copy(Setting s) => new() + { + Id = s.Id, Group = s.Group, Key = s.Key, Value = s.Value, Type = s.Type, + IsEncrypted = s.IsEncrypted, UpdatedAt = s.UpdatedAt, UpdatedBy = s.UpdatedBy + }; + } + + private sealed class NoCache : ISettingCache + { + public bool TryGetValue(string key, out T? value) { value = default; return false; } + public void Set(string key, T value, TimeSpan duration) { } + public void Remove(string key) { } + } +} diff --git a/tests/DotNetBoost.Settings.UnitTests/SinglePropertyWriteTests.cs b/tests/DotNetBoost.Settings.UnitTests/SinglePropertyWriteTests.cs new file mode 100644 index 0000000..d516a3d --- /dev/null +++ b/tests/DotNetBoost.Settings.UnitTests/SinglePropertyWriteTests.cs @@ -0,0 +1,282 @@ +using DotNetBoost.Settings.Core; +using DotNetBoost.Settings.Core.Attributes; +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.Core.Models; +using DotNetBoost.Settings.Core.Services; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; + +namespace DotNetBoost.Settings.UnitTests; + +/// +/// SetAsync(selector, value) used to rebuild the whole model from the cache and +/// put it through the group write. A property another writer had changed while this instance's +/// cache was warm read back stale, counted as a difference, and was written back — so a call +/// that only meant to touch property A reverted someone's edit to property B, over a window as +/// wide as CacheDuration rather than a round trip. +/// +public class SinglePropertyWriteTests +{ + /// + /// The scenario itself: a warm cache on instance one, an out-of-band change to a different + /// property through instance two, then a single-property write through instance one. + /// + /// The two managers get separate caches on purpose. That is what two replicas + /// behind a load balancer have, and it is the only arrangement in which the bug is + /// reachable: sharing one cache would let instance two's eviction clear instance one's + /// stale copy, and the stale read that does the damage would never happen. + /// + /// + [Fact] + public async Task SettingOneProperty_DoesNotRevertAnotherWritersChangeToADifferentProperty() + { + var store = new RecordingStore(); + + var first = NewManager(store, SharedCache()); + var second = NewManager(store, SharedCache()); + + await first.For().SetAsync(new MailSettings { Host = "original.example.com", Port = 25 }); + + // Instance one warms its cache on the group as it is now. + await first.For().GetAsync(); + + // Someone else changes a *different* property, out of band. + await second.For().SetAsync(x => x.Host, "theirs.example.com"); + + // Instance one now writes the property it actually meant to. + await first.For().SetAsync(x => x.Port, 587); + + var current = await first.For().GetAsync(refreshCache: true); + Assert.Equal("theirs.example.com", current.Host); // not reverted + Assert.Equal(587, current.Port); // and the intended write landed + } + + [Fact] + public async Task SettingOneProperty_WritesOnlyThatProperty() + { + var store = new RecordingStore(); + var mgr = NewManager(store, SharedCache()); + + await mgr.For().SetAsync(new MailSettings { Host = "h", Port = 25 }); + store.WrittenKeys.Clear(); + + await mgr.For().SetAsync(x => x.Port, 587); + + Assert.Equal([nameof(MailSettings.Port)], store.WrittenKeys); + } + + [Fact] + public async Task SettingOneProperty_ToItsCurrentValue_WritesNothing() + { + var store = new RecordingStore(); + var mgr = NewManager(store, SharedCache()); + + await mgr.For().SetAsync(new MailSettings { Host = "h", Port = 25 }); + store.WrittenKeys.Clear(); + + await mgr.For().SetAsync(x => x.Port, 25); + + Assert.Empty(store.WrittenKeys); + } + + /// + /// A validator takes a model, not a property, so the single-property path has to build the + /// group as it will be and validate that. Skipping it would make the selector overload a + /// way around every registered validator. + /// + [Fact] + public async Task SettingOneProperty_StillRunsValidators_AndWritesNothingOnFailure() + { + var store = new RecordingStore(); + var services = new ServiceCollection(); + services.AddSingleton(); + + var mgr = new SettingManager(store, new SettingCache(new MemoryCache(new MemoryCacheOptions())), + services.BuildServiceProvider(), NullLogger.Instance); + + await mgr.For().SetAsync(new MailSettings { Host = "h", Port = 25 }); + store.WrittenKeys.Clear(); + + await Assert.ThrowsAsync( + () => mgr.For().SetAsync(x => x.Port, 70_000)); + + Assert.Empty(store.WrittenKeys); + } + + /// + /// The validated model is the group as it will be, so a validator sees the stored values of + /// the properties the caller did not name — not the defaults of a blank instance. + /// + [Fact] + public async Task Validators_SeeTheStoredValuesOfUntouchedProperties() + { + var store = new RecordingStore(); + var validator = new CapturingValidator(); + var services = new ServiceCollection(); + services.AddSingleton(validator); + + var mgr = new SettingManager(store, new SettingCache(new MemoryCache(new MemoryCacheOptions())), + services.BuildServiceProvider(), NullLogger.Instance); + + await mgr.For().SetAsync(new MailSettings { Host = "stored.example.com", Port = 25 }); + validator.Seen.Clear(); + + await mgr.For().SetAsync(x => x.Port, 587); + + var seen = Assert.IsType(Assert.Single(validator.Seen)); + Assert.Equal("stored.example.com", seen.Host); + Assert.Equal(587, seen.Port); + } + + [Fact] + public async Task SettingOneProperty_RaisesAWriteNotification_WithThatOneChange() + { + var store = new RecordingStore(); + var observer = new CollectingObserver(); + var services = new ServiceCollection(); + services.AddScoped(_ => observer); + + var mgr = new SettingManager(store, new SettingCache(new MemoryCache(new MemoryCacheOptions())), + services.BuildServiceProvider(), NullLogger.Instance); + + await mgr.For().SetAsync(new MailSettings { Host = "h", Port = 25 }); + observer.Writes.Clear(); + + await mgr.For().SetAsync(x => x.Port, 587); + + var change = Assert.Single(Assert.Single(observer.Writes).Changes); + Assert.Equal(nameof(MailSettings.Port), change.PropertyName); + Assert.Equal("25", change.OldValue); + Assert.Equal("587", change.NewValue); + } + + [Fact] + public async Task SettingOneProperty_EvictsTheCache() + { + var store = new RecordingStore(); + var mgr = NewManager(store, SharedCache()); + + await mgr.For().SetAsync(new MailSettings { Host = "h", Port = 25 }); + await mgr.For().GetAsync(); // warm + + await mgr.For().SetAsync(x => x.Port, 587); + + Assert.Equal(587, (await mgr.For().GetAsync()).Port); + } + + /// One cache instance, as a single application instance has. + private static SettingCache SharedCache() => new(new MemoryCache(new MemoryCacheOptions())); + + private static SettingManager NewManager(ISettingStore store, ISettingCache cache) + => new(store, cache, new ServiceCollection().BuildServiceProvider(), + NullLogger.Instance); + + [SettingGroup("single-property-mail")] + public class MailSettings + { + public string? Host { get; set; } + public int Port { get; set; } + } + + private sealed class PortRangeValidator : ISettingValidator + { + public bool CanValidate(Type type) => type == typeof(MailSettings); + + public Task<(bool IsValid, IDictionary Errors)> ValidateAsync(object model) + { + var mail = (MailSettings)model; + return Task.FromResult(mail.Port is > 0 and <= 65_535 + ? (true, (IDictionary)new Dictionary()) + : (false, new Dictionary + { + [nameof(MailSettings.Port)] = ["Port must be between 1 and 65535."] + })); + } + } + + private sealed class CapturingValidator : ISettingValidator + { + public List Seen { get; } = []; + + public bool CanValidate(Type type) => type == typeof(MailSettings); + + public Task<(bool IsValid, IDictionary Errors)> ValidateAsync(object model) + { + Seen.Add(model); + return Task.FromResult((true, (IDictionary)new Dictionary())); + } + } + + private sealed class CollectingObserver : ISettingWriteObserver + { + public List Writes { get; } = []; + + public Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default) + { + Writes.Add(write); + return Task.CompletedTask; + } + } + + /// A store whose reads reflect its writes, recording which keys were written. + private sealed class RecordingStore : ISettingStore + { + private readonly Dictionary _data = new(StringComparer.Ordinal); + private static string K(string g, string k) => g + "|" + k; + + public List WrittenKeys { get; } = []; + + public Task> GetGroupAsync(string group, CancellationToken ct = default) + => Task.FromResult>( + _data.Values.Where(x => x.Group == group).Select(Copy).ToList()); + + public Task GetAsync(string group, string key, CancellationToken ct = default) + => Task.FromResult(_data.TryGetValue(K(group, key), out var s) ? Copy(s) : null); + + public Task UpsertAsync(Setting setting, CancellationToken ct = default) + => UpsertManyAsync([setting], ct); + + public Task UpsertManyAsync(IEnumerable settings, CancellationToken ct = default) + { + foreach (var s in settings) + { + if (_data.TryGetValue(K(s.Group, s.Key), out var existing) && + s.RowVersion is not null && + !existing.RowVersion.AsSpan().SequenceEqual(s.RowVersion)) + { + throw new SettingConcurrencyException(s.Group, s.Key); + } + + var stored = Copy(s); + stored.RowVersion = Setting.NewRowVersion(); + _data[K(s.Group, s.Key)] = stored; + WrittenKeys.Add(s.Key); + } + return Task.CompletedTask; + } + + public Task DeleteAsync(string group, string key, CancellationToken ct = default) + { + _data.Remove(K(group, key)); + return Task.CompletedTask; + } + + public Task DeleteGroupAsync(string group, CancellationToken ct = default) + { + foreach (var k in _data.Where(x => x.Value.Group == group).Select(x => x.Key).ToList()) + _data.Remove(k); + return Task.CompletedTask; + } + + public Task CountAsync(string group, CancellationToken ct = default) + => Task.FromResult(_data.Values.Count(x => x.Group == group)); + + private static Setting Copy(Setting s) => new() + { + Id = s.Id, Group = s.Group, Key = s.Key, Value = s.Value, Type = s.Type, + IsEncrypted = s.IsEncrypted, UpdatedAt = s.UpdatedAt, UpdatedBy = s.UpdatedBy, + RowVersion = s.RowVersion + }; + } +}