From 2e79f6afba0f8050d9429e3abbf9e617a8e86c09 Mon Sep 17 00:00:00 2001 From: Sadeq Abu-Hattem Date: Fri, 6 Nov 2026 22:32:17 +0200 Subject: [PATCH] Upgrade Settings and MultiLingual to 1.0.0-preview.3 MultiLingual: TranslatedText writes and reads every language through SetAll and GetAllAsync(includeBase: true), so it no longer switches the request into the base language. Creates go through a new CreateAsync, which gives "" to base-language properties the body left out. HiLo keys stay, now only for Auditing's nested save. Settings: SettingGroupAuditObserver becomes SettingGroupAuditParticipant, which enlists the audit entry and its change rows in the settings save, so the entry commits with the change or not at all. It uses SettingWrite.GroupType instead of scanning the assembly. Tests cover the entry committing in the caller's save, a failed settings write leaving nothing behind, and a create under another language without every base property. Docs and documented counts updated. Co-Authored-By: Claude Opus 5.5 --- ARCHITECTURE-SLICES.md | 2 +- Directory.Packages.props | 23 +- README.md | 88 ++++--- src/DotNetBoostSample.Api/Program.cs | 2 +- .../Catalog/Categories/CreateCategory.cs | 7 +- .../Catalog/Categories/GetCategory.cs | 3 +- .../Catalog/Products/CreateProduct.cs | 5 +- .../Catalog/Shared/TranslatedText.cs | 224 ++++++------------ .../Settings/AuditingSettings.cs | 6 +- .../DependencyInjection.cs | 20 +- .../Persistence/AppDbContext.cs | 3 +- .../Configurations/CategoryConfiguration.cs | 8 +- .../Configurations/ProductConfiguration.cs | 3 +- .../Persistence/SettingGroupAuditObserver.cs | 103 -------- .../SettingGroupAuditParticipant.cs | 90 +++++++ .../Integration/AuditRetentionTests.cs | 2 +- .../Integration/AuditTrailTests.cs | 67 ++++-- .../Integration/BaseLanguageTests.cs | 74 +++++- .../Integration/SampleApp.cs | 46 +++- 19 files changed, 425 insertions(+), 351 deletions(-) delete mode 100644 src/DotNetBoostSample.Infrastructure/Persistence/SettingGroupAuditObserver.cs create mode 100644 src/DotNetBoostSample.Infrastructure/Persistence/SettingGroupAuditParticipant.cs diff --git a/ARCHITECTURE-SLICES.md b/ARCHITECTURE-SLICES.md index f36fcf7..be0bd59 100644 --- a/ARCHITECTURE-SLICES.md +++ b/ARCHITECTURE-SLICES.md @@ -5,7 +5,7 @@ the two disagreed the code won — §10 lists what changed on contact with the c written while the work sat on `feat/vertical-slices-cqrs` and still argues with itself about whether to merge it; §11.3 says how that ended. -The solution builds clean on `net8.0` and `net10.0` with `TreatWarningsAsErrors`, and all 160 tests +The solution builds clean on `net8.0` and `net10.0` with `TreatWarningsAsErrors`, and all 163 tests pass, including the integration suite that boots the real `Program.cs` against a throwaway PostgreSQL and drives every route over HTTP. No endpoint's URL, status code or JSON shape changed. diff --git a/Directory.Packages.props b/Directory.Packages.props index 51fc66f..5d62771 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -11,19 +11,20 @@ integration package pairing two of them. Each library is installed on its own and knows nothing about the others. tests/ asserts that stays true. - All four happen to be on 1.0.0-preview.2 today, and nothing aligns those numbers: - Auditing reached preview.2 a release after the other three, for a fix of its own - (IgnoreEntity(), below). Four independently versioned libraries are four release + Nothing aligns their numbers. Auditing reached preview.2 a release after the other + three, for a fix of its own (IgnoreEntity(), below); Settings and MultiLingual then + moved to preview.3 for fixes this sample's composition turned up, while Media and + Auditing stayed on preview.2. Four independently versioned libraries are four release cadences, which is the other half of what "no shared kernel" means. ────────────────────────────────────────────────────────────────────────── --> - - - + + + - + @@ -34,12 +35,12 @@ - - - + + + - + diff --git a/README.md b/README.md index f95e9cc..9dbe083 100644 --- a/README.md +++ b/README.md @@ -203,7 +203,7 @@ last three, where one library's data has no place in another's in the shape it i | 5 | `Application/Settings/BrandingSettings.cs` | Settings × Media × MultiLingual | branding's name, its other languages and its logo are one thing | | 6 | `Infrastructure/DependencyInjection.cs` — one line | Auditing → MultiLingual | MultiLingual's translation row is not in the trail: `.IgnoreEntity()` | | 7 | `Domain/Catalog/Category.cs` and `Product.cs` | MultiLingual → Auditing | a translated property is not in the trail: `[AuditIgnore]` beside `[MultiLingual]` | -| 8 | `Infrastructure/Persistence/SettingGroupAuditObserver.cs` | Settings → Auditing | a settings save is one entry for the group, not one per row | +| 8 | `Infrastructure/Persistence/SettingGroupAuditParticipant.cs` | Settings → Auditing | a settings save is one entry for the group, not one per row, committed with it | Nine files in eight entries, because the seventh is one sentence said once per entity. It was five before Auditing arrived. Three of these exist because two libraries **disagree**, which is a worse @@ -219,7 +219,7 @@ string, an attribute or a type both sides must agree on, which is why `BrandingO **Six of the nine name only `DotNetBoost.*.Core`**, so they live in the application and domain layers beside the code that uses them. Only `AppDbContext`, the `IgnoreEntity` line and -`SettingGroupAuditObserver` are infrastructure, and all three are there because four libraries share +`SettingGroupAuditParticipant` are infrastructure, and all three are there because four libraries share **one `DbContext`**. Joining two libraries is not a persistence concern; joining them in one database is. @@ -424,10 +424,11 @@ Neither library learned the other's name: the fix was an ignore-list on one buil pairing the two. **Keys are assigned before the save.** For a database-generated key, the interceptor writes the -`Created` entry in a second, nested `SaveChanges` — which on this context also runs through -MultiLingual's interceptor mid-save, and a second save of a just-created entity once wrote an empty -product name over the stored one. Rather than depend on the order of two libraries' interceptors, -the catalogue's entities take HiLo keys: +`Created` entry in a second, nested `SaveChanges`. Up to MultiLingual `1.0.0-preview.2` that nested +save also restarted MultiLingual's interceptor mid-save, dropping the outer save's pending work and +committing its transaction from the inside. `1.0.0-preview.3` fixed that: a nested save now joins +the outer one, and only the outermost writes, commits or rolls back. The nested save itself is still +Auditing's, though, so rather than depend on it the catalogue's entities keep their HiLo keys: ```csharp category.Property(c => c.Id).UseHiLo("CategoryIds"); @@ -440,7 +441,7 @@ ids have gaps, and an older database needs the sequences created past its existi **What the trail holds:** everything this `DbContext` saves except `Translation` and `Setting` — `MediaFile`, `MediaAttachment` and `Language` as well as the catalogue, and the seed's first run with -no actor. Settings saves are recorded by [the observer](#8-a-settings-save-is-one-entry-not-one-per-row--srcinfrastructurepersistencesettinggroupauditobservercs). +no actor. Settings saves are recorded by [the participant](#8-a-settings-save-is-one-entry-not-one-per-row--srcinfrastructurepersistencesettinggroupauditparticipantcs). `GET /api/audit/entities` lists only what is **declared** — the application's own two — so the dashboard shows the libraries' entries under *Any*. `AuditTrailTests` asserts each of these. @@ -469,31 +470,40 @@ way of recording honours it, and `GET /api/audit/entities/Product` reports it as each library's metadata — because a translated property added without the second attribute would produce a trail that confidently says the wrong thing. -### 8. A settings save is one entry, not one per row — `src/.../Infrastructure/Persistence/SettingGroupAuditObserver.cs` +### 8. A settings save is one entry, not one per row — `src/.../Infrastructure/Persistence/SettingGroupAuditParticipant.cs` DotNetBoost.Settings stores a group one row per property, so the interceptor alone recorded a branding save as one entry per value, each addressed by a row id nobody knows. The settings library -already knows where an edit begins and ends, and reports it through `ISettingWriteObserver`: +already knows where an edit begins and ends, and since `1.0.0-preview.3` lets code take part in it +through `ISettingWriteParticipant`, which runs inside the write, before the store saves: ```csharp // Settings block -.UseWriteObserver() +.UseWriteParticipant() // Auditing block .IgnoreEntity() .IgnoreEntity(); ``` -The observer records the group's whole diff as one `Setting` entry addressed by the group's name. -The write carries the group's class as its type, so the ignore — checked by type — does not drop it, -and `[AuditIgnore]` and `[AuditRedact]` on a group's properties apply. A `[Sensitive]` property is -marked redacted, so the trail does not read "changed from nothing to nothing". The entry is saved -through a context of its own, so a failure leaves nothing behind on the request's. - -**The cost:** this is the one entry that does not commit with its change. The observer runs after -the settings save, and DotNetBoost.Settings logs and swallows its failures, so a failed record leaves -the change without an entry. For dashboard-edited configuration, "catalog: page size 17 → 18" in -one line is worth that. +The participant records the group's whole diff as one `Setting` entry addressed by the group's +name. The write carries the group's class as `SettingWrite.GroupType`, so the ignore — checked by +type — does not drop it, and `[AuditIgnore]` and `[AuditRedact]` on a group's properties apply. A +`[Sensitive]` property is marked redacted, so the trail does not read "changed from nothing to +nothing". + +**It commits with the change.** In the request's scope, Settings' EF Core store, Auditing's EF Core +store and the participant all resolve the same `AppDbContext`, so `IAuditManager.RecordAsync` tracks +the entry on the very context the settings store is about to save. The participant enlists the +entry and each of its change rows with `EfCoreSettingWriteScope.Add`, and never saves. The entry goes +out in the settings store's own save, or not at all: if anything fails, the write is aborted and +everything enlisted is detached again, so nothing is left for the context's next save to insert. A +failure to record now fails the request rather than passing silently. `AuditTrailTests` checks both +halves, the second with a test-only participant that fails the write after the entry is enlisted. + +Up to Settings `1.0.0-preview.2` this was an `ISettingWriteObserver`, which runs after the commit +and whose failures the library logs and swallows. This was then the one entry that did not commit +with its change. --- @@ -529,14 +539,22 @@ One class, `Catalog/Shared/TranslatedText.cs`, through which the four writes and | `TranslatedText` | In the default mode | With a base language | |---|---|---| -| `SaveAsync` | queues every language with `Set` and saves | assigns the base language's values to the entity, queues the rest, and saves **in** the base language | -| `LoadAsync` | the query, then `GetAllAsync()` | reads the entity in the base language and adds its columns under that language's code | - -Each half answers a refusal the library makes on purpose: `Set("en", …)` throws, because the base -language is not a translation, and a create under `Accept-Language: ar` throws, because the base -value must exist first. Saving in the base language settles both, and the request's language is put -back afterwards so the response sentence stays in Arabic. The read half matters as much: without -it, the editor would open with empty English boxes and save that emptiness over the real text. +| `SaveAsync` (an update) | `SetAll`, which is `Set` per language, and saves | `SetAll`: the base language to the entity's columns, the rest as rows, and saves | +| `CreateAsync` (a create) | as `SaveAsync` | as `SaveAsync`, with `""` for every base-language property the body left out | +| `LoadAsync` | the query, then `GetAllAsync()` | the query, then `GetAllAsync(includeBase: true)`, which reads the base language from its columns | + +All of it stays in the request's language. Since MultiLingual `1.0.0-preview.3`, `SetAll` writes the +base language to the columns whatever language is current, and `GetAllAsync(includeBase: true)` +reads it back from the database by key, so the response sentence stays in Arabic without a detour +through English. The read half matters as much as the write: without the base language, the editor +would open with empty English boxes and save that emptiness over the real text. + +One rule is still the application's. The library accepts a create under `Accept-Language: ar` only +when every translatable value came through `SetAll`, and it cannot tell a non-nullable property's +leftover `""` from a value. So `CreateAsync` names every base-language property, giving `""` to any +the body left out, which is what the column would hold anyway. An update does not fill anything: an +unsent property there holds the request's language, and filling it would write that language over +the base column. The seed authors content in `BaseLanguage ?? "en"`, and `Persistence/MultiLingualModelCacheKeyFactory.cs` puts the option in EF's model cache key so two hosts with different modes can share a process. @@ -554,7 +572,7 @@ as `""` where the default mode answers `null`, because the column is a non-nulla `dotnet test` boots the sample twice, in two hosts against two schemas. -**The whole integration suite runs in both modes**: fifty-seven tests against each, the same +**The whole integration suite runs in both modes**: fifty-eight tests against each, the same assertions, unchanged — and between them they call **every one of the API's 48 published routes**. `OpenApiShapeTests` asserts that inventory by name, so a route added by this application or a new library version fails the suite rather than going uncalled. @@ -562,8 +580,8 @@ library version fails the suite rather than going uncalled. `BaseLanguageTests` then asks what only this mode can: that the `Name` column holds English (read with raw SQL), that the editor's round trip keeps it, that writes in Arabic leave it alone, that refusals keep their status and language, and that cascades and the other libraries behave the same. -Deleting the one line that puts base-language values on the entity fails six tests, two of them -duplicated default-mode classes running in this mode, while every default-mode copy still passes. +`A_create_in_another_language_without_every_base_property_still_answers_201` pins the one rule +`TranslatedText` still adds to the library's. Measured as well: across two freshly seeded APIs, one per mode, the OpenAPI document is byte-identical apart from its port, and so is every `GET` response across nine endpoints in all @@ -639,15 +657,15 @@ abstract class holds the assertions and two sealed subclasses bind it to each fi - **`RemainingRouteTests`** — the six routes nothing else called. - **`AuditTrailTests`** — a product's life in the trail and the history that outlives it; a change made straight through `AppDbContext` recorded all the same; no `Translation` entry; settings saves - as one entry; the right actor on each entry; and **a refused write records nothing**. + as one entry, committed in the settings save, and none at all when that save fails; the right actor on each entry; and **a refused write records nothing**. - **`AuditRetentionTests`** — retention is off by default, and once switched on, `AuditRetentionService` deletes old entries and keeps new ones. - **`SchemaUpgradeTests`** — a database from before the HiLo keys still boots and creates rows. - **`BaseLanguageTests`** — what only base-language mode can be asked. - Fifteen tests, beside the fifty-seven the suite runs in each mode. + Sixteen tests, beside the fifty-eight the suite runs in each mode. **Without a container runtime these skip rather than fail** — `dotnet test` still runs the 30 fast -tests and reports the other 130 as skipped, with the reason. +tests and reports the other 133 as skipped, with the reason. ### And the dashboard's own @@ -792,7 +810,7 @@ src/DotNetBoostSample.Infrastructure/ Persistence/AppDbContext.cs one database, four Apply* calls Persistence/Configurations/ one per entity: unique indexes and HiLo keys Persistence/DatabaseInitialiser.cs schema and seed on first run, and upgrades for old volumes - Persistence/SettingGroupAuditObserver.cs + Persistence/SettingGroupAuditParticipant.cs Persistence/PostgresErrorTranslator.cs DependencyInjection.cs AddInfrastructure(): four independent blocks src/DotNetBoostSample.Api/ diff --git a/src/DotNetBoostSample.Api/Program.cs b/src/DotNetBoostSample.Api/Program.cs index c2b83c4..7d292b1 100644 --- a/src/DotNetBoostSample.Api/Program.cs +++ b/src/DotNetBoostSample.Api/Program.cs @@ -28,7 +28,7 @@ // // Infrastructure/Persistence/AppDbContext.cs — all four: one Apply* call each // Infrastructure/DependencyInjection.cs — .IgnoreEntity() -// Infrastructure/Persistence/SettingGroupAuditObserver.cs — one audit entry per settings save +// Infrastructure/Persistence/SettingGroupAuditParticipant.cs — one audit entry per settings save // Application/Composition/MediaText.cs — a file's per-language alt text // Application/Composition/MediaTextCleanup.cs — deleted with the file // Application/Composition/MultiLingualSchemaContributor.cs — `translated` in settings schemas diff --git a/src/DotNetBoostSample.Application/Catalog/Categories/CreateCategory.cs b/src/DotNetBoostSample.Application/Catalog/Categories/CreateCategory.cs index 6ae0439..b18365b 100644 --- a/src/DotNetBoostSample.Application/Catalog/Categories/CreateCategory.cs +++ b/src/DotNetBoostSample.Application/Catalog/Categories/CreateCategory.cs @@ -48,10 +48,9 @@ public async Task HandleAsync(Command request, CancellationToke // AddAsync, because adding is what assigns the HiLo id — see CategoryConfiguration. await db.Categories.AddAsync(category, ct); - // One SaveChanges for the row, every language's text and its audit entry. TranslatedText - // saves, because with a base language configured the language it runs in decides where - // each value lands. - await translatedText.SaveAsync(db, category, request.Translations, ct); + // One SaveChanges for the row, every language's text and its audit entry. CreateAsync + // rather than SaveAsync, so a create under any language names every base-language value. + await translatedText.CreateAsync(db, category, request.Translations, ct); return new CategoryCreated(category.Id, category.Slug); } diff --git a/src/DotNetBoostSample.Application/Catalog/Categories/GetCategory.cs b/src/DotNetBoostSample.Application/Catalog/Categories/GetCategory.cs index a3cc7a3..4e279ce 100644 --- a/src/DotNetBoostSample.Application/Catalog/Categories/GetCategory.cs +++ b/src/DotNetBoostSample.Application/Catalog/Categories/GetCategory.cs @@ -18,7 +18,8 @@ namespace DotNetBoostSample.Application.Catalog.Categories; /// /// This is the exception to "the interceptors already translated it": an editor needs every /// language at once, which is what TranslatedText.LoadAsync answers with — the stored -/// translations, plus the base language's own columns where a base language is configured. +/// translations, plus the base language read from its own columns where a base language is +/// configured. /// public static class GetCategory { diff --git a/src/DotNetBoostSample.Application/Catalog/Products/CreateProduct.cs b/src/DotNetBoostSample.Application/Catalog/Products/CreateProduct.cs index ad89de2..c1ff211 100644 --- a/src/DotNetBoostSample.Application/Catalog/Products/CreateProduct.cs +++ b/src/DotNetBoostSample.Application/Catalog/Products/CreateProduct.cs @@ -65,8 +65,9 @@ public async Task> HandleAsync(Command request, Cancellat // AddAsync, because adding is what assigns the HiLo id — see ProductConfiguration. await db.Products.AddAsync(product, ct); - // One SaveChanges for the row, every language's text and its audit entry. - await translatedText.SaveAsync(db, product, request.Translations, ct); + // One SaveChanges for the row, every language's text and its audit entry. CreateAsync + // rather than SaveAsync, so a create under any language names every base-language value. + await translatedText.CreateAsync(db, product, request.Translations, ct); return new ProductCreated(product.Id, product.Sku); } diff --git a/src/DotNetBoostSample.Application/Catalog/Shared/TranslatedText.cs b/src/DotNetBoostSample.Application/Catalog/Shared/TranslatedText.cs index 324e42c..a115b1b 100644 --- a/src/DotNetBoostSample.Application/Catalog/Shared/TranslatedText.cs +++ b/src/DotNetBoostSample.Application/Catalog/Shared/TranslatedText.cs @@ -11,24 +11,21 @@ namespace DotNetBoostSample.Application.Catalog.Shared; /// Category or a Product. /// /// Why the read and the write are one class. They are one decision, asked twice: -/// where does a language live? Without -/// MultiLingualOptions.BaseLanguage the answer is "a translation row, always", and both -/// sides are a single library call. With one configured, one language is the entity's own columns -/// instead — and a reader has to know that on the way out as surely as a writer does on the way -/// in. Split across two classes, the second half of that was written twice and could be changed -/// once. +/// where does a language live? Without MultiLingualOptions.BaseLanguage the answer +/// is "a translation row, always". With one configured, one language is the entity's own columns +/// instead. Since DotNetBoost.MultiLingual 1.0.0-preview.3 the library answers both halves itself — +/// SetAll writes every language wherever it lives, and GetAllAsync(includeBase: true) +/// reads every language back the same way — so neither half has to change the request's language +/// any more. What is left here is the one rule a create needs (see ). /// /// It is also the whole of what this application knows about the mode. Four handlers /// hand it the translations map the request arrived with, two hand it a query, and none of -/// them asks which mode is on — which is what keeps the slices identical in both and is the reason -/// this is a class rather than a few lines in each of them. +/// them asks which mode is on — which is what keeps the slices identical in both. /// /// And one word of the name is deliberately loose. In base-language mode the base -/// language's values are not a translation at all — they are the entity's own columns, and -/// ITranslationAccessor.Set refuses to be told otherwise. "Translated text" is what this -/// application calls a property whose value depends on the language, wherever the library keeps -/// it; the two stores are an implementation detail of that, and keeping them behind one name is -/// the whole reason a slice can ignore which mode is on. +/// language's values are not a translation at all — they are the entity's own columns. "Translated +/// text" is what this application calls a property whose value depends on the language, wherever +/// the library keeps it. /// /// Scoped, like MediaText: it holds the request's engine and nothing else. /// @@ -40,49 +37,43 @@ public sealed class TranslatedText(IMultiLingualManager multiLingual) /// /// The single entity selects, and every language of text it has, as - /// languageCode → (propertyName → value). + /// languageCode → (propertyName → value) — the base language included, read from its columns. /// /// This is the exception to "the interceptors already translated it". Everywhere else a /// plain ToListAsync() comes back in the request's language and that is the whole story; /// an editor draws a box per language, so it needs all of them at once. /// - /// The query is taken rather than its result because, with a base language configured, - /// WHICH language it is read in is part of the answer — see . - /// Nothing else in either detail response is affected by that: neither carries a localized - /// name, and the alt text beside their images is read by the slice afterwards, in the request's - /// own language. + /// The entity is read in the request's own language. That puts the request's language in + /// its translatable properties, which does not matter: neither detail response carries a + /// localized name, and GetAllAsync(includeBase: true) reads the base language's columns + /// from the database by key rather than off the properties. /// /// The entity, or null when the query matched nothing — then the slice answers 404. public async Task<(T? Entity, IReadOnlyDictionary> Text)> LoadAsync(IQueryable one, CancellationToken ct) where T : class { - var entity = await InBaseLanguageAsync(() => one.FirstOrDefaultAsync(ct), ct); + var entity = await one.FirstOrDefaultAsync(ct); - if (entity is null) return (null, None); - - var stored = await multiLingual.For(entity).GetAllAsync(ct); - - // `stored` is every stored TRANSLATION. In base-language mode that is every language but - // one, so the base language is put back beside them — otherwise the editor would open with - // its boxes empty and save that emptiness over the canonical text on the first write. - return (entity, BaseLanguage is { } code ? WithBaseValues(stored, code, entity) : stored); + return entity is null + ? (null, None) + : (entity, await multiLingual.For(entity).GetAllAsync(includeBase: true, ct)); } /// - /// Writes and every language in in - /// one SaveChanges — the row and its text in the same transaction, which is why - /// Category and Product are deliberately absent from ExposeOverHttp: a - /// second way in would be a second answer. + /// Writes a change to and the languages in + /// in one SaveChanges — the row and its text in the same + /// transaction, which is why Category and Product are deliberately absent from + /// ExposeOverHttp: a second way in would be a second answer. /// - /// Every language is queued with Set, by name because the property names arrive in - /// a request body — except the base language, where there is nothing to queue: that language is - /// the entity's own columns, and Set refuses to be told otherwise - /// (BaseLanguageNotTranslatableException). It is assigned instead, and the save then runs - /// in the language those values belong to. + /// An update: only the languages and properties the request sent are written. Every one + /// goes through SetAll, which puts the base language in the entity's columns and every + /// other language in a translation row, whatever language the request is in. A property name the + /// entity does not declare translatable is refused before anything is queued, in both modes + /// alike, so a caller cannot tell from a refusal which mode the server is in. /// /// The unit of work the row and its text are written in. - /// A new or tracked entity carrying [MultiLingual] properties. + /// A tracked entity carrying [MultiLingual] properties. /// languageCode → (propertyName → value), as the request sent it. /// The request's token. public Task SaveAsync( @@ -92,131 +83,72 @@ public Task SaveAsync( CancellationToken ct) where T : class { - var accessor = multiLingual.For(entity); - - foreach (var (language, values) in translations ?? []) - { - if (IsBase(language)) Assign(entity, values); - else accessor.Set(language, values); - } + if (translations is { Count: > 0 }) + multiLingual.For(entity).SetAll(ReadOnly(translations)); - return InBaseLanguageAsync(() => db.SaveChangesAsync(ct), ct); + return db.SaveChangesAsync(ct); } - // ── Base-language mode ─────────────────────────────────────────────────── - // Everything below is inert without MultiLingualOptions.BaseLanguage: `BaseLanguage` is null, - // `IsBase` is false for every code, and `InBaseLanguageAsync` runs the work as it is. - - /// The language kept in each entity's own columns, or null when every one is a row. - private string? BaseLanguage => multiLingual.Languages.BaseLanguage; - - /// Whether is that language. False in the other mode, always. - private bool IsBase(string code) => multiLingual.Languages.IsBaseLanguage(code); - /// - /// Runs with this scope reading and writing the BASE language, then - /// puts the request's own language back. + /// As , for a new : every base-language property + /// the request left out is given "", which is what its column would hold anyway. /// - /// It is the same detour for a read and for a write, because the base language is where - /// the columns are read from and written to alike: - /// - /// Reading, a tracked entity's translatable properties hold whatever language the - /// query resolved — so one read under Accept-Language: ar has the Arabic name sitting - /// where the base value lives in the database. Reading in the base language is what makes - /// entity.Name the base value again. - /// Writing, an entity may only be CREATED in the base language (the base value is - /// what every translation hangs off), and an assigned property goes to the column only while - /// that language is the one being authored. Otherwise it becomes a translation in whatever - /// language the request happened to be read in. - /// + /// Why a create fills the gaps. Product and Category declare their + /// translatable properties as non-nullable strings defaulting to string.Empty. In + /// base-language mode the library accepts a create under another language only when every + /// translatable value the new entity holds came from SetAll, and it cannot tell a leftover + /// "" from a value. So a body with an English Name but no English + /// Description, sent under Accept-Language: ar, would be refused + /// (BaseLanguageRequiredException). Naming every property settles it — including a body + /// with no base-language entry at all, which reads back as "". /// - /// And why it is put back. The endpoint answers with a sentence from - /// IStringLocalizer<ApiMessages>, which reads CurrentLanguage, and so does - /// the problem detail for a refused constraint — in a finally, so a failed save answers - /// in the caller's language too. Leaving the scope in the base language would answer an Arabic - /// request in English, which is exactly the kind of change to a response this mode is not - /// allowed to make. + /// Why an update does not. There, an unsent property holds whatever language the + /// request read it in, and filling it would write that language into the base column. /// - /// A scope with no language resolved at all — a seeder, a background job — is left alone: - /// the library exempts it from the create rule and its property values go straight to the - /// columns. + /// The request's own base-language values are kept as sent, unknown names included, so + /// SetAll refuses a misspelled property here exactly as it does on an update. /// - private async Task InBaseLanguageAsync(Func> work, CancellationToken ct) + /// The unit of work the row and its text are written in. + /// A new entity, already added to . + /// languageCode → (propertyName → value), as the request sent it. + /// The request's token. + public Task CreateAsync( + IAppDbContext db, + T entity, + Dictionary>? translations, + CancellationToken ct) + where T : class { - var requested = multiLingual.IsLanguageResolved ? multiLingual.CurrentLanguage.Code : null; - - if (BaseLanguage is not { } baseLanguage || requested is null || IsBase(requested)) - return await work(); - - await multiLingual.UseLanguageAsync(baseLanguage, ct); + var text = ReadOnly(translations ?? []); - try - { - return await work(); - } - finally + // Inert without a base language: every language is a row, and an unset one is just absent. + if (multiLingual.Languages.BaseLanguage is { } baseLanguage) { - await multiLingual.UseLanguageAsync(requested, ct); - } - } + var values = text.TryGetValue(baseLanguage, out var given) + ? new Dictionary(given, StringComparer.Ordinal) + : new Dictionary(StringComparer.Ordinal); - /// - /// The stored translations plus the base language, taken off the entity's own properties — - /// which is where that language lives, and which has just - /// made sure is what they hold. - /// - /// Read after that method put the request's language back, because what it restores is a - /// SCOPE: it does not touch an entity already in memory. - /// - /// The map is case-insensitive on the way in, because its two halves name the language - /// independently — the stored keys come from the translation rows and this one from the - /// configured option. Both are canonical today, and a map carrying en beside EN - /// would be a shape no client should have to read. - /// - private static IReadOnlyDictionary> WithBaseValues( - IReadOnlyDictionary> stored, - string baseLanguage, - T entity) - where T : class - { - var text = new Dictionary>( - StringComparer.OrdinalIgnoreCase); + foreach (var property in MultiLingualMetadata.For(typeof(T))) + values.TryAdd(property.Name, string.Empty); - foreach (var (code, inLanguage) in stored) text[code] = inLanguage; + // The map ignores case, so this replaces the request's entry however it spelled the code. + text[baseLanguage] = values; + } - text[baseLanguage] = MultiLingualMetadata.For(typeof(T)) - .ToDictionary(property => property.Name, property => property.GetValue(entity), - StringComparer.Ordinal); + if (text.Count > 0) + multiLingual.For(entity).SetAll(text); - return text; + return db.SaveChangesAsync(ct); } /// - /// Assigns the base language's values to those same properties. - /// - /// Matched against MultiLingualMetadata rather than assigned to whatever the body - /// happens to name, so a property the entity does not declare as translatable is refused here - /// exactly as Set refuses it in the other mode: a caller cannot tell from a refusal which - /// mode the server is in. - /// - /// null becomes the empty string, because these properties are non-nullable - /// strings and their columns are NOT NULL. Clearing the base language's text leaves an - /// empty value rather than removing a row — the honest difference between a column and a - /// translation is that there is nothing to delete. + /// The request's map in the shape SetAll takes. Case-insensitive on the language code, + /// because a code is the same language however a client spells it. /// - private static void Assign(T entity, Dictionary values) - where T : class - { - var translatable = MultiLingualMetadata.For(typeof(T)); - - foreach (var (name, value) in values) - { - var property = - translatable.FirstOrDefault(p => p.Name.Equals(name, StringComparison.OrdinalIgnoreCase)) - ?? throw new ArgumentException( - $"'{name}' is not a translatable property of {typeof(T).Name}.", nameof(values)); - - property.SetValue(entity, value ?? string.Empty); - } - } + private static Dictionary> ReadOnly( + Dictionary> translations) => + translations.ToDictionary( + kv => kv.Key, + kv => (IReadOnlyDictionary)kv.Value, + StringComparer.OrdinalIgnoreCase); } diff --git a/src/DotNetBoostSample.Application/Settings/AuditingSettings.cs b/src/DotNetBoostSample.Application/Settings/AuditingSettings.cs index ae6f63e..d7fb9a9 100644 --- a/src/DotNetBoostSample.Application/Settings/AuditingSettings.cs +++ b/src/DotNetBoostSample.Application/Settings/AuditingSettings.cs @@ -19,9 +19,9 @@ namespace DotNetBoostSample.Application.Settings; /// sample that started deleting its own trail because someone ran it for a year would be the /// wrong kind of helpful. Turned on, the trail older than goes. /// -/// Editing this group is itself recorded — SettingGroupAuditObserver writes one entry -/// per save, like any other group's — so who shortened the retention, and from what, is in the -/// trail the change applies to. A prune never removes that entry sooner than any other. +/// Editing this group is itself recorded — SettingGroupAuditParticipant writes one +/// entry per save, like any other group's, in the same transaction as the save — so who shortened +/// the retention, and from what, is in the trail the change applies to. A prune never removes that entry sooner than any other. /// [SettingGroup("auditing")] public class AuditingSettings diff --git a/src/DotNetBoostSample.Infrastructure/DependencyInjection.cs b/src/DotNetBoostSample.Infrastructure/DependencyInjection.cs index 6d03719..ff81e8a 100644 --- a/src/DotNetBoostSample.Infrastructure/DependencyInjection.cs +++ b/src/DotNetBoostSample.Infrastructure/DependencyInjection.cs @@ -98,8 +98,9 @@ public static IServiceCollection AddInfrastructure( .UseSchemaContributor() // GET /api/settings/branding with its text in the request's language and its logo. .UseProjector() - // One audit entry per group saved, rather than one per Settings row. - .UseWriteObserver() + // One audit entry per group saved, rather than one per Settings row — enlisted in the + // settings save itself, so the entry commits with the change or not at all. + .UseWriteParticipant() .Build(); // ── DotNetBoost.Media ──────────────────────────────────────────────── @@ -154,15 +155,16 @@ public static IServiceCollection AddInfrastructure( // is that attribute applied from outside. Translated text is out of the trail anyway. // // 2. Keys. For a database-generated key, the interceptor writes a Created entry in a - // second, nested SaveChanges — which on this context also runs through MultiLingual's - // interceptor mid-save, and once wrote an empty product name over the stored one. - // Category and Product take HiLo keys instead (see their configurations), so the id - // exists when the entity is added and every entry goes into the caller's own save. + // second, nested SaveChanges. Since MultiLingual 1.0.0-preview.3 a nested save joins + // MultiLingual's outer one rather than restarting it, so that is no longer a problem on + // MultiLingual's side — but the nested save is still Auditing's, and Category and + // Product keep HiLo keys (see their configurations) so the id exists when the entity is + // added and every entry goes into the caller's own save. // // 3. Settings. The library stores a group one row per property. IgnoreEntity() - // keeps those rows out, and SettingGroupAuditObserver records the save as one entry for - // the group instead — after the settings save commits, the one entry not written - // atomically with its change. + // keeps those rows out, and SettingGroupAuditParticipant records the save as one entry + // for the group instead — enlisted in the settings store's own save, so it commits + // atomically with its change like every other entry. // // Every other mapped entity is captured, because that is the library's default — MediaFile, // MediaAttachment and Language as well as the catalogue. diff --git a/src/DotNetBoostSample.Infrastructure/Persistence/AppDbContext.cs b/src/DotNetBoostSample.Infrastructure/Persistence/AppDbContext.cs index 36d8468..48ee613 100644 --- a/src/DotNetBoostSample.Infrastructure/Persistence/AppDbContext.cs +++ b/src/DotNetBoostSample.Infrastructure/Persistence/AppDbContext.cs @@ -32,7 +32,8 @@ namespace DotNetBoostSample.Infrastructure.Persistence; /// /// It is also one of only three of the nine composition files that are infrastructure; the /// others are that one line in DependencyInjection.cs and -/// SettingGroupAuditObserver, which saves its entry through this context. The remaining six — +/// SettingGroupAuditParticipant, which enlists its entry in the settings store's save on this +/// context. The remaining six — /// MediaText, MediaTextCleanup, MultiLingualSchemaContributor, /// BrandingProjector and the two entities' attributes — depend on DotNetBoost.*.Core /// and nothing else, so they live in the application and domain layers beside the code that uses diff --git a/src/DotNetBoostSample.Infrastructure/Persistence/Configurations/CategoryConfiguration.cs b/src/DotNetBoostSample.Infrastructure/Persistence/Configurations/CategoryConfiguration.cs index 280d26c..f7c5193 100644 --- a/src/DotNetBoostSample.Infrastructure/Persistence/Configurations/CategoryConfiguration.cs +++ b/src/DotNetBoostSample.Infrastructure/Persistence/Configurations/CategoryConfiguration.cs @@ -36,10 +36,10 @@ public void Configure(EntityTypeBuilder category) // ids from "CategoryIds" and hands one out when the entity is ADDED, before anything is saved. // That is what keeps a creation's audit entry in the same SaveChanges as the row: // DotNetBoost.Auditing defers the entry for a key the INSERT has yet to generate, to a - // second save nested inside the first — on a context MultiLingual is writing translations - // through at the same moment. Infrastructure/DependencyInjection.cs has why that is worth - // avoiding, and DatabaseInitialiser creates the sequence on a database made before it - // existed. + // second save nested inside the first. MultiLingual (since 1.0.0-preview.3) lets a nested + // save join its own, so the reason is now Auditing's nested save alone — see + // Infrastructure/DependencyInjection.cs. DatabaseInitialiser creates the sequence on a + // database made before it existed. // // Add with AddAsync for the same reason: a block that has run out is refilled from the // database, and AddAsync is the overload that does that without blocking. diff --git a/src/DotNetBoostSample.Infrastructure/Persistence/Configurations/ProductConfiguration.cs b/src/DotNetBoostSample.Infrastructure/Persistence/Configurations/ProductConfiguration.cs index 85c641a..12469c1 100644 --- a/src/DotNetBoostSample.Infrastructure/Persistence/Configurations/ProductConfiguration.cs +++ b/src/DotNetBoostSample.Infrastructure/Persistence/Configurations/ProductConfiguration.cs @@ -28,7 +28,8 @@ public void Configure(EntityTypeBuilder product) product.HasKey(p => p.Id); // HiLo, from a sequence of its own — see CategoryConfiguration for why a key assigned - // before the save is what keeps a creation's audit entry in that save. + // before the save is what keeps a creation's audit entry in that save, without Auditing's + // nested second save. product.Property(p => p.Id).UseHiLo("ProductIds"); product.Property(p => p.Sku).HasMaxLength(64).IsRequired(); product.Property(p => p.Price).HasPrecision(18, 2); diff --git a/src/DotNetBoostSample.Infrastructure/Persistence/SettingGroupAuditObserver.cs b/src/DotNetBoostSample.Infrastructure/Persistence/SettingGroupAuditObserver.cs deleted file mode 100644 index 2413db1..0000000 --- a/src/DotNetBoostSample.Infrastructure/Persistence/SettingGroupAuditObserver.cs +++ /dev/null @@ -1,103 +0,0 @@ -using DotNetBoost.Auditing.Core.Interfaces; -using DotNetBoost.Auditing.Core.Models; -using DotNetBoost.Settings.Core.Attributes; -using DotNetBoost.Settings.Core.Interfaces; -using DotNetBoostSample.Application.Settings; -using Microsoft.Extensions.DependencyInjection; -using System.Reflection; - -namespace DotNetBoostSample.Infrastructure.Persistence; - -/// -/// Records one audit entry per save of a settings group, rather than one per row of the -/// Settings table. -/// -/// Why not the interceptor. DotNetBoost.Settings stores a group as one row per -/// property, so the interceptor — which sees rows — wrote a separate Setting entry for every -/// value that moved, addressed by a row id nobody reading the trail knows. The settings library -/// already knows where an edit begins and ends: is called once -/// per save, with the whole group's diff. So the Setting rows are ignored by the capture, -/// and this writes the entry instead, still under but addressed by the -/// group's name. -/// -/// Why the entry carries the group's type. IgnoreEntity<Setting>() also -/// suppresses a write addressed by the name "Setting" alone, so an entry with no type would -/// be dropped as ignored. Auditing checks the ignore by type when the write has one, and the type -/// this entry is about is the group's class — , say — which is not -/// ignored. It is also where the model's rules come from: an [AuditIgnore] or -/// [AuditRedact] on a group's property is honoured, exactly as on an entity. -/// -/// What it costs. The interceptor's guarantee — a change and its entry in one -/// SaveChanges — does not hold here. The observer runs after the settings write has -/// committed, and DotNetBoost.Settings logs and swallows anything it throws, so a failure to record -/// leaves the change standing without an entry. For configuration edited through a dashboard that -/// is the right trade for a readable trail; for anything stricter, it is not. Closing that gap -/// needs a hook inside the settings store's own save, which DotNetBoost.Settings does not have. -/// -/// Why a scope of its own. The settings rows were saved through the request's -/// context. Recording on that one would leave the entry tracked there — and, if its save failed, -/// still pending for whatever saved next. A fresh scope's context holds the entry and nothing else, -/// and a failure discards it with the scope. -/// -/// Secrets. DotNetBoost.Settings has already withheld the values of every -/// [Sensitive] property, and this marks those changes redacted on the entry, so the trail -/// says "changed, value withheld" rather than "changed from nothing to nothing". -/// -public sealed class SettingGroupAuditObserver(IServiceScopeFactory scopes) : ISettingWriteObserver -{ - /// The entity name a settings group's entries are stored under. - public const string EntityName = "Setting"; - - /// - /// Every settings group in the application assembly, by the name its rows are stored under — - /// the same assembly UseFluentValidation scans, resolved the way DotNetBoost.Settings - /// resolves it. - /// - private static readonly Dictionary Groups = typeof(BrandingSettings).Assembly - .GetTypes() - .Where(type => type.GetCustomAttribute() is not null) - .ToDictionary(SettingGroupAttribute.ResolveName, StringComparer.Ordinal); - - /// - public async Task OnWrittenAsync(SettingWrite write, CancellationToken cancellationToken) - { - ArgumentNullException.ThrowIfNull(write); - - // Thrown rather than recorded without a type, which would be dropped as ignored and say - // nothing. DotNetBoost.Settings logs it with the group's name. - if (!Groups.TryGetValue(write.GroupName, out var groupType)) - throw new InvalidOperationException( - $"Settings group '{write.GroupName}' has no [SettingGroup] class in " - + $"{typeof(BrandingSettings).Assembly.GetName().Name}, so its save cannot be audited."); - - // The manager and the context from one scope, so the EF Core store tracks the entry on the - // very context saved below. - await using var scope = scopes.CreateAsyncScope(); - var audit = scope.ServiceProvider.GetRequiredService(); - var db = scope.ServiceProvider.GetRequiredService(); - - var entry = await audit.RecordAsync(new AuditWrite - { - EntityName = EntityName, - EntityId = write.GroupName, - EntityType = groupType, - Action = write.Kind == SettingWriteKind.Cleared ? AuditAction.Deleted : AuditAction.Updated, - Changes = [.. write.Changes.Select(c => new AuditChange(c.PropertyName, c.OldValue, c.NewValue))], - // Who and which edit, from the write itself rather than from whatever the new scope can - // reach. A null actor still falls back to the request's caller, as it did before. - Actor = write.Actor, - CorrelationId = write.CorrelationId, - }, cancellationToken); - - if (entry is null) return; - - var redacted = write.Changes.Where(c => c.IsRedacted).Select(c => c.PropertyName).ToHashSet(); - foreach (var change in entry.Changes.Where(c => redacted.Contains(c.PropertyName))) - change.IsRedacted = true; - - // The EF Core store tracks the entry rather than saving it, and the save it would normally - // ride along with has already happened — so this one is the entry's own, and it is all - // this context holds. - await db.SaveChangesAsync(cancellationToken); - } -} diff --git a/src/DotNetBoostSample.Infrastructure/Persistence/SettingGroupAuditParticipant.cs b/src/DotNetBoostSample.Infrastructure/Persistence/SettingGroupAuditParticipant.cs new file mode 100644 index 0000000..f0201fe --- /dev/null +++ b/src/DotNetBoostSample.Infrastructure/Persistence/SettingGroupAuditParticipant.cs @@ -0,0 +1,90 @@ +using DotNetBoost.Auditing.Core.Interfaces; +using DotNetBoost.Auditing.Core.Models; +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.EntityFrameworkCore; + +namespace DotNetBoostSample.Infrastructure.Persistence; + +/// +/// Records one audit entry per save of a settings group, rather than one per row of the +/// Settings table — in the same transaction as the save. +/// +/// Why not the interceptor. DotNetBoost.Settings stores a group as one row per +/// property, so the interceptor — which sees rows — wrote a separate Setting entry for every +/// value that moved, addressed by a row id nobody reading the trail knows. The settings library +/// already knows where an edit begins and ends: is called +/// once per write, with the whole group's diff. So the Setting rows are ignored by the +/// capture, and this writes the entry instead, still under but addressed by +/// the group's name. +/// +/// Why it commits with the change. A participant runs inside the settings write, +/// before the store saves. In the request's scope, Settings' EF Core store, Auditing's EF Core store +/// and this class all resolve the same scoped , so +/// IAuditManager.RecordAsync tracks the entry on the very context the settings store is +/// about to save. The entry goes out in that save, or not at all — and anything this throws aborts +/// the settings write, so a failure to record fails the request rather than passing silently. +/// Nothing here saves, and nothing here reaches outside the context: the write can still fail +/// after this has run. +/// +/// Why the change rows are enlisted too. If the write does not commit, +/// detaches only the entities passed to Add, not ones +/// reached through navigations. Auditing tracks an entry together with its change rows, so each is +/// enlisted, or a failed write would leave them Added on the request's context for its next +/// SaveChangesAsync to insert. +/// +/// Why the entry carries the group's type. IgnoreEntity<Setting>() also +/// suppresses a write addressed by the name "Setting" alone, so an entry with no type would +/// be dropped as ignored. Auditing checks the ignore by type when the write has one, and the type +/// this entry is about is the group's class — CatalogSettings, say — which is not ignored. +/// It is also where the model's rules come from: an [AuditIgnore] or [AuditRedact] +/// on a group's property is honoured, exactly as on an entity. The settings library hands that type +/// over as . +/// +/// Secrets. DotNetBoost.Settings has already withheld the values of every +/// [Sensitive] property, and this marks those changes redacted on the entry, so the trail +/// says "changed, value withheld" rather than "changed from nothing to nothing". +/// +public sealed class SettingGroupAuditParticipant(IAuditManager audit, EfCoreSettingWriteScope settingsWrite) + : ISettingWriteParticipant +{ + /// The entity name a settings group's entries are stored under. + public const string EntityName = "Setting"; + + /// + public async Task OnWritingAsync(SettingWrite write, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(write); + + // Always set by DotNetBoost.Settings; null only on a SettingWrite built by hand. Thrown + // rather than recorded without a type, which would be dropped as ignored and say nothing. + var groupType = write.GroupType + ?? throw new InvalidOperationException( + $"Settings group '{write.GroupName}' arrived without its type, so its save cannot be audited."); + + // Tracks the entry and its change rows on the request's AppDbContext — the one the settings + // store is about to save. Nothing is saved here. + var entry = await audit.RecordAsync(new AuditWrite + { + EntityName = EntityName, + EntityId = write.GroupName, + EntityType = groupType, + Action = write.Kind == SettingWriteKind.Cleared ? AuditAction.Deleted : AuditAction.Updated, + Changes = [.. write.Changes.Select(c => new AuditChange(c.PropertyName, c.OldValue, c.NewValue))], + // Who and which edit, from the write itself. A null actor still falls back to the + // request's caller. + Actor = write.Actor, + CorrelationId = write.CorrelationId, + }, cancellationToken); + + if (entry is null) return; + + var redacted = write.Changes.Where(c => c.IsRedacted).Select(c => c.PropertyName).ToHashSet(); + foreach (var change in entry.Changes.Where(c => redacted.Contains(c.PropertyName))) + change.IsRedacted = true; + + // The whole graph, so a write that does not commit takes all of it back. + settingsWrite.Add(entry); + foreach (var change in entry.Changes) + settingsWrite.Add(change); + } +} diff --git a/tests/DotNetBoostSample.Tests/Integration/AuditRetentionTests.cs b/tests/DotNetBoostSample.Tests/Integration/AuditRetentionTests.cs index e23a6f2..a6ae614 100644 --- a/tests/DotNetBoostSample.Tests/Integration/AuditRetentionTests.cs +++ b/tests/DotNetBoostSample.Tests/Integration/AuditRetentionTests.cs @@ -101,7 +101,7 @@ public async Task Turned_on_it_deletes_what_is_past_retention_and_keeps_the_rest // Who changed the retention is recorded like any other group's save — and, being new, it // is not among what that retention deletes. var turnedOn = (await client.GetJsonAsync( - $"/api/audit?entityName={SettingGroupAuditObserver.EntityName}&entityId={nameof(AuditingSettings)}")) + $"/api/audit?entityName={SettingGroupAuditParticipant.EntityName}&entityId={nameof(AuditingSettings)}")) .GetProperty("entries").EnumerateArray().First(); Assert.Equal(Http.DefaultActor, turnedOn.Str("actor")); diff --git a/tests/DotNetBoostSample.Tests/Integration/AuditTrailTests.cs b/tests/DotNetBoostSample.Tests/Integration/AuditTrailTests.cs index 348cb5e..24a5ab1 100644 --- a/tests/DotNetBoostSample.Tests/Integration/AuditTrailTests.cs +++ b/tests/DotNetBoostSample.Tests/Integration/AuditTrailTests.cs @@ -4,6 +4,7 @@ using DotNetBoostSample.Application.Settings; using DotNetBoostSample.Domain.Catalog; using DotNetBoostSample.Infrastructure.Persistence; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using System.Net; using System.Text.Json; @@ -217,8 +218,8 @@ public async Task A_creation_keeps_its_text_in_every_language() /// /// The group is stored one row per property, so the interceptor alone would record a /// save as an entry per row. IgnoreEntity<Setting>() keeps the rows out and - /// records the save, addressed as - /// ("Setting", <group name>) — under the ignored name, which is why the observer + /// records the save, addressed as + /// ("Setting", <group name>) — under the ignored name, which is why the participant /// gives the write the group's type. Two values are changed at once here, because one entry /// holding both is the whole point. /// @@ -262,16 +263,15 @@ public async Task A_settings_save_is_one_entry_for_the_group() } /// - /// A settings group's entry is saved through a context of its own, not the caller's. + /// A settings group's entry commits in the caller's own save, on the caller's own context. /// - /// The settings rows are saved through the caller's , and the - /// observer runs once they have committed. Recording on that same context left the entry tracked - /// there afterwards — and, had its save failed, still pending for whatever saved next. Asked of - /// the caller's change tracker, because a save that fails is not one a test can cause on - /// demand. + /// runs inside the settings write, before the + /// store saves, and records on the scope's — the one the settings + /// store saves. So the entry is tracked there afterwards, already saved, with nothing left + /// pending: one save carried the rows and the entry. /// [RequiresDockerFact] - public async Task A_settings_entry_is_saved_on_a_context_of_its_own() + public async Task A_settings_entry_commits_in_the_callers_own_save() { try { @@ -283,11 +283,14 @@ await services.GetRequiredService() .For() .SetAsync(new CatalogSettings { PageSize = 19, CurrencySymbol = "¥" }); - // The caller's context saved the rows, and holds nothing of the trail. - Assert.Empty(db.ChangeTracker.Entries()); + // The entry went out through the caller's context, in its save, and nothing waits. + Assert.Equal( + EntityState.Unchanged, + Assert.Single(db.ChangeTracker.Entries()).State); + Assert.False(db.ChangeTracker.HasChanges()); }); - // And the entry is in the trail all the same. + // And the entry is in the trail. var newest = (await GroupTrail(app.Client.AsActor()))[0]; Assert.Equal("19", Changes(newest)["PageSize"].Str("newValue")); @@ -298,6 +301,42 @@ await services.GetRequiredService() } } + /// + /// A settings write that fails after its entry was recorded leaves the group unchanged, no + /// entry in the trail, and nothing pending on the caller's context. + /// + /// The guarantee the participant gives and the observer before it could not. The write is + /// failed by , a test-only participant that runs after the + /// application's — so the entry and its change rows have been enlisted when it throws, and the + /// settings library has to take all of them back. The last save is the trap: had a change row + /// been left Added, the context's next unrelated save would insert it. + /// + [RequiresDockerFact] + public async Task A_settings_write_that_fails_leaves_the_group_and_the_trail_as_they_were() + { + var client = app.Client.AsActor(); + var entriesBefore = (await GroupTrail(client)).Count; + var before = await client.GetJsonAsync("/api/settings/catalog"); + + await app.WithScopeAsync(async services => + { + var db = services.GetRequiredService(); + services.GetRequiredService().Armed = true; + + await Assert.ThrowsAsync(() => + services.GetRequiredService() + .For() + .SetAsync(new CatalogSettings { PageSize = 23, CurrencySymbol = "₩" })); + + // Neither the entry nor any of its change rows is left for the next save to insert. + Assert.False(db.ChangeTracker.HasChanges()); + Assert.Equal(0, await db.SaveChangesAsync()); + }); + + Assert.Equal(entriesBefore, (await GroupTrail(client)).Count); + Assert.Equal(before.Int("pageSize"), (await client.GetJsonAsync("/api/settings/catalog")).Int("pageSize")); + } + /// /// GET /api/audit/{id} — one entry, read by the id the page carries, with its changes. /// @@ -469,7 +508,7 @@ public async Task A_caller_who_names_nobody_is_the_default_sample_user() /// Declared is not the same as recorded, and this is where the difference shows. The /// list is what DeclareEntities named, so it is the application's own two; the trail is /// wider — MediaFile and the rest are recorded too, by the interceptor, and a settings - /// group by its observer, as shows. And Translation is + /// group by its participant, as shows. And Translation is /// neither: ignored, so it cannot be declared, which Build() enforces. /// /// It also publishes maxPageSize, so a client can stop asking for more than it @@ -497,7 +536,7 @@ public async Task The_declared_entities_are_this_applications_own_two() /// The catalog settings group's entries, newest first. private static async Task> GroupTrail(HttpClient client) => [.. (await client.GetJsonAsync( - $"/api/audit?entityName={SettingGroupAuditObserver.EntityName}&entityId={nameof(CatalogSettings)}")) + $"/api/audit?entityName={SettingGroupAuditParticipant.EntityName}&entityId={nameof(CatalogSettings)}")) .GetProperty("entries").EnumerateArray()]; /// One entity's entries, newest first. diff --git a/tests/DotNetBoostSample.Tests/Integration/BaseLanguageTests.cs b/tests/DotNetBoostSample.Tests/Integration/BaseLanguageTests.cs index 5a8f498..9281a4b 100644 --- a/tests/DotNetBoostSample.Tests/Integration/BaseLanguageTests.cs +++ b/tests/DotNetBoostSample.Tests/Integration/BaseLanguageTests.cs @@ -10,7 +10,7 @@ namespace DotNetBoostSample.Tests.Integration; /// Why a second container is worth it. Base-language mode is not a flag that changes /// a response: it changes where content is STORED. A translatable property keeps its column, that /// column holds English, every other language stays a translation row, an entity may only be -/// created in the base language, and Set("en", …) is refused outright. Four of those five +/// created with its base-language text, and Set("en", …) is refused outright. Four of those five /// are invisible to every other test in this project, and the fifth is a 500 waiting for the first /// caller who posts under Accept-Language: ar. /// @@ -68,11 +68,11 @@ string NameOf(JsonElement page) => /// is reading in Arabic — still answers 201, with its sentence in Arabic, and stores the /// English text from the body as the base value. /// - /// The library refuses a create outside the base language, for a good reason: the base - /// value is what every translation hangs off and the last fallback for every other language. - /// TranslatedText moves the save into the base language instead of letting that refusal - /// reach the caller — and puts the request's language back, which is why the message below is - /// Arabic. + /// The library refuses a create outside the base language unless every translatable value + /// came through SetAll, for a good reason: the base value is what every translation hangs + /// off and the last fallback for every other language. TranslatedText writes every + /// language through SetAll, which puts the English values in the columns without + /// changing the request's language — which is why the message below is Arabic. /// [RequiresDockerFact] public async Task A_create_read_in_another_language_still_answers_201_in_that_language() @@ -95,8 +95,8 @@ public async Task A_create_read_in_another_language_still_answers_201_in_that_la var created = await response.ReadJsonAsync("POST /api/products"); - // The API's own sentence, in the language the request was read in — not in the language - // the row had to be written in. + // The API's own sentence, in the language the request was read in — not in the base + // language the row's columns were written in. Assert.StartsWith("تم إنشاء المنتج", created.Str("message")); var id = created.Int("id"); @@ -115,6 +115,54 @@ public async Task A_create_read_in_another_language_still_answers_201_in_that_la await client.SendJsonAsync(HttpMethod.Delete, $"/api/products/{id}"); } + /// + /// A create under Accept-Language: ar whose body names an English Name but no + /// English Description still answers 201, with the description's column empty. + /// + /// The case TranslatedText.CreateAsync exists for. Description is a + /// non-nullable string defaulting to "", and the library cannot tell that leftover from + /// a value — so a create in another language that left it unset would be refused as one whose + /// base text had not come through SetAll. Naming every base-language property, with + /// "" for the ones the body left out, keeps the answer it had when the save ran in + /// English. + /// + [RequiresDockerFact] + public async Task A_create_in_another_language_without_every_base_property_still_answers_201() + { + var client = app.Client; + + var response = await client.SendJsonAsync(HttpMethod.Post, "/api/products", new + { + sku = $"BP-{Guid.NewGuid():N}"[..12], + price = 10.0m, + categoryId = app.SeededCategoryId, + translations = new Dictionary> + { + ["en"] = new() { ["Name"] = "Name only" }, + ["ar"] = new() { ["Name"] = "الاسم فقط", ["Description"] = "وصف." } + } + }, language: "ar"); + + Assert.Equal(HttpStatusCode.Created, response.StatusCode); + + var id = (await response.ReadJsonAsync("POST /api/products")).Int("id"); + + Assert.Equal( + "Name only", + await app.ScalarAsync($"""select "Name" from "Products" where "Id" = {id}""")); + Assert.Equal( + "", + await app.ScalarAsync($"""select "Description" from "Products" where "Id" = {id}""")); + + // The Arabic text is a translation like any other. + var detail = (await client.GetJsonAsync($"/api/products/{id}")).GetProperty("translations"); + + Assert.Equal("الاسم فقط", detail.GetProperty("ar").Str("Name")); + Assert.Equal("", detail.GetProperty("en").Str("Description")); + + await client.SendJsonAsync(HttpMethod.Delete, $"/api/products/{id}"); + } + /// /// The detail endpoint still answers with every language in one map, the base language among /// them — which is what the dashboard's editor draws a box per language from. @@ -375,11 +423,11 @@ public async Task Content_created_without_the_base_language_reads_as_empty_rathe /// A refused write is still a refusal — 400, 404 or 409 as before, and in the caller's /// language. /// - /// The one assertion here that is really about this mode is the last. The save runs - /// inside the base language, and the refusal is raised by the database in the middle of that — - /// so the language has to be back before DbUpdateExceptionHandler looks the sentence up. - /// That is what the finally in TranslatedText is for, and a test that only - /// checked the status code would pass with it deleted. + /// The one assertion here that is really about this mode is the last. The save writes the + /// base language's columns, and the refusal is raised by the database in the middle of that — + /// so the sentence DbUpdateExceptionHandler looks up has to be in the caller's language, + /// not the one the columns were written in. SetAll never changes the request's language, + /// and a test that only checked the status code would not notice if something did. /// [RequiresDockerFact] public async Task A_refused_write_keeps_its_status_and_its_language() diff --git a/tests/DotNetBoostSample.Tests/Integration/SampleApp.cs b/tests/DotNetBoostSample.Tests/Integration/SampleApp.cs index a9c54ce..66b89e6 100644 --- a/tests/DotNetBoostSample.Tests/Integration/SampleApp.cs +++ b/tests/DotNetBoostSample.Tests/Integration/SampleApp.cs @@ -1,6 +1,8 @@ using DotNetBoostSample.Infrastructure.Persistence; +using DotNetBoost.Settings.Core.Interfaces; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -27,7 +29,10 @@ namespace DotNetBoostSample.Tests.Integration; /// /// Two things are overridden, both infrastructure rather than composition: the /// connection string, so the run gets a throwaway database, and Media:Root, so uploaded -/// bytes land in a temporary directory instead of the repository's media folder. +/// bytes land in a temporary directory instead of the repository's media folder. One thing is +/// added: , a settings write participant that does nothing +/// until a test arms it in its own scope — the one way to make a settings write fail after the +/// application's own participant has enlisted its audit entry. /// /// Postgres comes from Testcontainers. The README already asks for a container runtime to /// run the sample at all, so this adds no prerequisite — and where there is no Docker the tests @@ -110,6 +115,7 @@ public async Task InitializeAsync() { builder.UseSetting("ConnectionStrings:catalog", _postgres.GetConnectionString()); builder.UseSetting("Media:Root", _mediaRoot); + UseTestHooks(builder); // The third override, and the only one that is not infrastructure: it decides where a // [MultiLingual] property is STORED, so a run with it set exercises a different schema @@ -198,11 +204,24 @@ public WebApplicationFactory Boot(string connectionString) => { builder.UseSetting("ConnectionStrings:catalog", connectionString); builder.UseSetting("Media:Root", _mediaRoot); + UseTestHooks(builder); if (BaseLanguage is not null) builder.UseSetting("MultiLingual:BaseLanguage", BaseLanguage); }); + /// + /// What a test can switch on in a scope of its own, and nothing else can: see + /// . Added after the application's registrations, so the + /// sabotage runs after SettingGroupAuditParticipant has enlisted its entry. + /// + private static void UseTestHooks(IWebHostBuilder builder) => + builder.ConfigureTestServices(services => + { + services.AddScoped(); + services.AddScoped(sp => sp.GetRequiredService()); + }); + /// /// The running instance of a hosted service, for a test that drives one step of it rather than /// waiting for its timer — AuditRetentionService.PruneIfDueAsync, which is what the @@ -241,3 +260,28 @@ public sealed class SampleAppCollection : ICollectionFixture { public const string Name = "sample-app"; } + +/// +/// A settings write participant that fails the write — but only in a scope where a test has set +/// , so every request and every other test is untouched. +/// +/// It is registered after the application's own participant, so when it throws the audit +/// entry has already been recorded and enlisted. That is the case +/// SettingGroupAuditParticipant has to get right: the write does not commit, and neither the +/// entry nor its change rows may be left behind on the request's context. +/// +public sealed class SettingsWriteSabotage : ISettingWriteParticipant +{ + /// Set by a test, in its own scope, to fail that scope's next settings write. + public bool Armed { get; set; } + + /// + public Task OnWritingAsync(SettingWrite write, CancellationToken cancellationToken) => + Armed + ? throw new SettingsWriteSabotagedException(write.GroupName) + : Task.CompletedTask; +} + +/// What throws, so a test can tell it from a real failure. +public sealed class SettingsWriteSabotagedException(string group) + : Exception($"The test failed the write to settings group '{group}' on purpose.");