feat(localization): request localization, localized errors and user locale (1/4) - #1381
marcelo-maciel wants to merge 11 commits into
Conversation
…ocale Framework slice of the i18n work (split of fullstackhero#1344 as requested in review). - `SharedResources` catalog (en + pt-BR) and `SupportedCultures` as the single source of supported tags. - `CustomException` carries `MessageKey`, `MessageArgs` and `ResourceSource`; `Message` stays English so logs remain culture-independent. `ILocalizableMessage` subclasses keep `UnauthorizedAccessException` / `KeyNotFoundException` as base types so audit severity classification is unaffected. - `GlobalExceptionHandler` localizes `title`/`detail` and surfaces the message key as a stable `code` extension on ProblemDetails. - `UseHeroLocalization` request-localization chain, UI-culture-only: `CurrentCulture` stays invariant, only `CurrentUICulture` is negotiated. `UserLocaleRequestCultureProvider` reads the `locale` claim, so the middleware sits between `UseAuthentication` and `UseAuthorization`. - `User.Locale` (`varchar(10)`, nullable, no database default; `en-US` is a code-level fallback) plus the `AddUserLocale` migration, the `locale` claim emission and the write-boundary validator rejecting tags outside `SupportedCultures.Tags`. - `LogContext.PushProperty` scoped in `using` blocks, fixing a pre-existing AsyncLocal leak that contaminated later log entries in the same request. - `SSH.NET` pin (`2026.0.0`), byte-identical to fullstackhero#1333, so `dotnet restore` passes while that PR is open.
…est culture No exception message this PR localizes was actually translated at runtime. Every detail resolved from a MessageKey and every title mapped from a status code came back from the neutral resx, whatever the client asked for. UseExceptionHandler is registered above UseHeroLocalization, and RequestLocalizationMiddleware assigns CultureInfo.CurrentUICulture inside its own async frame. That assignment belongs to the ExecutionContext of that frame and is gone by the time an exception unwinds up to the handler, so every localizer there resolved under the culture of the host process -- the invariant one in a container with no LANG, hence the neutral resx. GlobalExceptionHandler now reads the culture from HttpContext.Features.Get<IRequestCultureFeature>(), which the middleware sets on the request itself and therefore survives the unwind. Reading the negotiated culture rather than re-reading Accept-Language keeps the whole provider chain, including the user locale claim. Only CurrentUICulture is touched: AddHeroLocalization pins CurrentCulture to invariant on purpose. The previous value is restored in a finally so no request culture leaks onto the thread. When no feature is present -- an exception escaping before localization runs -- the ambient culture stands and no Content-Language is claimed. Also restores Content-Language on the problem body. ExceptionHandlerMiddleware clears the response before re-executing, which drops the header the localization middleware had already written, leaving the culture of the prose undeclared. GlobalExceptionHandlerLocalizationTests could not catch this: they assign CurrentUICulture by hand and call the handler directly, never through a pipeline. ExceptionLocalizationPipelineTests build the real pipeline and pin the ambient culture to invariant, which is what a container gives the API -- without that pin a developer machine whose own culture is the tested one reports a false pass. Against the handler as it stood before this commit, five of those cases fail; the negotiation baseline and the no-localization case pass either way.
…advisories `dotnet restore` fails for the whole solution under `TreatWarningsAsErrors`, on `main` and on every open PR alike. Advisory-database drift, not a regression from any change: a commit green on 2026-08-10 is red today with no edits. - `Testcontainers.PostgreSql` / `.Redis` / `.Minio` 4.11.0 -> 4.14.0 (NU1903, GHSA-q939-rpr3-3284). 4.11.0 depends on `SSH.NET` 2025.1.0; 4.14.0 already depends on the patched 2026.0.0, so the advisory clears with no transitive pin to remember to remove later. Same fix as fullstackhero#1369, so the two do not conflict. - `Microsoft.SourceLink.GitHub` 8.0.0 -> 10.0.401 (NU1902, GHSA-23fw-v26w-5fgq). 8.0.0 drags in `Microsoft.Build.Tasks.Git` 8.0.0 and the 8.x line has no patched release, so a transitive pin cannot fix it; the package itself has to move. 10.0.401 depends on `Microsoft.Build.Tasks.Git` 10.0.401, past the patched 10.0.303. Build-time only (`PrivateAssets="all"`), referenced only where `IsPackable == true`, which is the CLI alone - and `src/Tools/**` is excluded from the template, so the scaffold never sees it. Verified: `dotnet restore src/FSH.Starter.slnx` exits 0 with no NU19xx, and `dotnet build src/FSH.Starter.slnx -c Release -warnaserror` reports 0 warnings and 0 errors.
MinIO withdrew `minio/minio` from Docker Hub. Docker Hub's API now answers
`object not found` for the repository, and a pull fails with:
pull access denied for minio/minio, repository does not exist or may
require 'docker login'
That takes down every Testcontainers-backed integration test (the harness boots
a MinIO container per fixture, so all 724 tests in `Integration.Tests` fail at
container start), the Aspire AppHost, and the Docker Compose deployment. The
image is still published at `quay.io/minio/minio`:
- `Integration.Tests` and `Integration.Middleware.Tests` harnesses
- `AppHost.cs`, via Aspire's `WithImageRegistry` / `WithImageTag`
- `deploy/docker/docker-compose.yml` and the image table in its README
The tag is pinned to `RELEASE.2025-09-07T16-13-09Z` rather than `:latest`. quay
has not moved `:latest` since 2025-09-07, so the two resolve to the same digest
today; pinning only removes the surprise of a silent move later, and keeps the
test harness off a floating tag. Whether to track a newer release, or a different
S3-compatible image, is a separate call.
While in the README's image table: `postgres` and `redis` rows had drifted from
what compose actually ships (`postgres:18-alpine`, `valkey/valkey:9.1.0-alpine`).
Verified: `docker pull minio/minio:latest` fails with the error above;
`docker pull quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z` succeeds
(`sha256:14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936e`, the
same digest `:latest` resolves to). `dotnet test Integration.Tests -c Release`
passes against the pinned image, and the Aspire manifest renders the container
as `quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z`.
UpdateUserCommandValidator guards its allow-list rule with .When(!IsNullOrWhiteSpace), so "" never reaches the allow-list. The service disagreed: `if (locale is not null)` wrote the empty string straight onto the user, so a body carrying locale: "" silently wiped a language the user had chosen. A form that serialises an untouched locale field as "" clears the preference on every unrelated save, and the allow-list never sees the value that got stored. Aligns the service with the validator's reading. The integration-test UserDto mirror gains Locale, which is why no existing test caught this. Gates: the new integration test fails on the previous code (Shouldly: dto.Locale) and passes after; UserProfileTests 8/8, Identity.Tests 330/330.
…main The migration was generated as 20260720062947, which sorts *before* 20260807063015_DropIdentityOutbox that has since landed on main. The history was therefore inconsistent with itself: DropIdentityOutbox's designer snapshot, the later of the two by name, knows nothing about the Locale column the earlier one adds, so anything generated on top of that snapshot would try to add it again. Regenerated against main's snapshot rather than hand-edited: the two files were deleted, the snapshot restored from origin/main, and `dotnet ef migrations add AddUserLocale` re-run. The Up/Down bodies are unchanged (AddColumn Locale, varchar(10), nullable) and the resulting model snapshot is byte-identical to the one already committed (`git diff HEAD` empty), so this is purely a reordering.
Matching key sets were the only gate, and they do not catch a translation that
drops {0} or renumbers it: the argument is either swallowed or the message
throws FormatException at the point it is built, and neither shows up as a
missing key. The new assertion compares the placeholder index set per key,
ignoring alignment and format specifier ({0,-10}, {0:N2} are the same argument).
Verified by mutation: collapsing "{0}/{1} bytes" to "{0} bytes" in the pt-BR
catalog turns it red.
SupportedCultures.Tags was a public static string[], so the whitelist that a validator, a culture provider and the request-localization setup all trust was writable by any caller holding a reference. FrozenSet with an ordinal comparer keeps the exact matching semantics (a wrong-case tag is still rejected) and makes the set immutable.
The 401 that JwtBearer's OnChallenge writes is the one error response the global exception handler never sees, so it stayed English while every other error was negotiated: a pt-BR reader got "Authentication is required to access this resource." in the middle of an otherwise translated app. It resolves IStringLocalizer<SharedResources> per request. That works because of the pipeline order this PR already relies on: UseRequestLocalization sits ahead of UseAuthorization, which is where the challenge is emitted, so the negotiated UI culture is in place by then. Error.AuthenticationRequired is new; the title reuses Error.Unauthorized. ChallengeLocalizationTests pins both the body and that ordering, hitting a protected endpoint with and without Accept-Language.
The MinIO carve-out this branch carries only moved `minio/minio`. `minio/mc` is gone from Docker Hub as well (`hub.docker.com/v2/repositories/minio/mc/` answers 404), and it is what `minio-init` runs: without it `dotnet run --project src/Host/FSH.Starter.AppHost` and `docker compose up` both die on the image pull, and the `fsh` bucket is never created, so the first upload fails with NoSuchBucket. Same pinned tag as fullstackhero#1388, which owns the fix, so the copy stays byte-identical to it and can be dropped once that lands.
The pin's own comment says "Testcontainers 4.11.0 and 4.13.0 both depend on 2025.1.0, so bumping Testcontainers does not help", but the branch also bumps Testcontainers to 4.14.0, whose nuspec declares `SSH.NET >= 2026.0.0`. The two statements cannot both be true, and the bump is the one that is: with the pin removed, `dotnet restore src/FSH.Starter.slnx --force` reports zero NU1902/NU1903 and exits 0. It was carrying a transitive pin that no longer pins anything. The MessagePack pin above it stays: that one is still load-bearing (removing it brings GHSA-hv8m-jj95-wg3x straight back, verified in the same probe).
|
Merge order for the three PRs that share the Identity profile path. #1381, #1382 and #1387 all touch the same eight files. None of the three declared an order against the others, so this is what the merges actually do, measured rather than guessed. #1381 + #1382 is clean. Same series, and #1382 carries #1381's changes to those files verbatim ( #1387 conflicts with both, on all eight: Twelve hunks, and every one is additive: one PR adds The part worth knowing about is not in the conflicts. Recommended order: #1387 → #1381 → #1382 → #1383 → #1384. Three reasons, in order of weight. #1387 is the fix for a silent lost update, and it should not queue behind a four-part feature. #1384 already declares it depends on #1387 landing first, so any other order serializes the same way with an extra step. And the side that rebases re-applies its own changes: #1381's footprint in the shared files is Verified end to end on a scratch worktree off One follow-up that is not a merge problem. |
Framework slice of the i18n work, split out of #1344 as you asked. This is the one that needs real scrutiny; 56 files.
The split is four PRs rather than three. Your three were framework /
clients/admin/clients/dashboard, but the ~220 files of module-level localization fit none of them, and folding them into the framework PR would put it back at ~285 files and defeat the point. So the module catalogs and their handler/validator wiring live in their own PR.clients/adminclients/dashboardThe union of the four is byte-identical to
#1344's tree, with empty pairwise intersection apart from the one sharedsrc/Directory.Packages.propshunk. That is asserted by a script, not by eye: for each slice,git diff --quiet feat/i18n <slice> -- <its paths>andgit diff --name-only main <slice>equal to its declared path set.The two front-end PRs depend on nothing here and can be reviewed in parallel. The module PR does not compile without this one — verified, not assumed:
src/Modules/**applied alone onmainfails with 862 compile errors, all rooted inFSH.Framework.Core.Localizationnot existing.src/BuildingBlocks(Golden Rule #4)Seventeen files here, needing maintainer sign-off:
Core—Core.csproj;Exceptions/(CustomException,ForbiddenException,UnauthorizedException, and the newILocalizableMessage,LocalizedKeyNotFoundException,LocalizedUnauthorizedAccessException);Localization/(newSharedResourcesmarker +SupportedCultures+ the two shared catalogs).Web—Extensions.cs(registers and orders the localization middleware, +6 lines);Exceptions/GlobalExceptionHandler.cs; newLocalization/(LocalizationExtensions,UserLocaleRequestCultureProvider).Jobs—Extensions.cs, one exception message.Storage—QuotaMeteredStorageService.cs, one exception message.One eighteenth
BuildingBlocksfile is in the module PR instead, and I want to be upfront about it:Web/Validation/PagedQueryValidator.cs. Its constructor now takesIStringLocalizer<SharedResources>, and its three callers (GetAuditsQueryValidator,GetTenantsQueryValidator,SearchUsersQueryValidator) live in modules. (They are callers, not subclasses: the class is sealed and is pulled in withInclude(...). The earlier wording here and on #1382 was wrong.) Keeping the base class here would either break this PR's build or drag the Auditing catalog and handler in with it; sending it with its three callers keeps both PRs compiling on their own and puts the change in front of the code it affects. It is declared under Golden Rule #4 there too.No existing behaviour of other building blocks is altered.
src/Directory.Packages.propscarries one addition, theSSH.NETpin discussed at the end.UseRequestLocalizationsets the UI culture onlyYou asked whether UI-culture-only was considered. It is what ships.
mainhas no request localization at all, so pinning the formatting culture is less change than negotiating it:CultureInfo.CurrentCulturebehaves exactly as it does onmaintoday, and only resource lookup follows the request. For an API whose output is JSON that is the safer default, and it makes the CA1305 question moot rather than merely bounded.It is not one switch.
RequestLocalizationMiddleware.SetCurrentThreadCultureassigns both cultures unconditionally, so the culture half has to be pinned:DefaultRequestCulturecarries(InvariantCulture, configured default). The middleware resolves the culture half ascultureInfo ??= DefaultRequestCulture.Culture, making invariant the only reachable value.SupportedCulturesisnull, so the middleware skips culture filtering entirely. A one-element[InvariantCulture]list behaves identically but logsUnsupportedCultureson every request — the middleware's parent-culture walk bails at the empty culture name, so invariant is unmatchable by design.With formatting out of the negotiation, the neutral
pt/enentries inRequestMatchbought nothing and are gone;SupportedCultures.Tagsis the single, specific-only list. A request asking for a bareptor an unsupported variant resolves to the configured default. Both React apps canonicalise variants onto supported tags before calling the API, so app traffic is unaffected; a hand-rolled client sending bareptgets the default.Message arguments are culture-insensitive too. The localizer formats with
string.FormatunderCurrentCulture, so adoubleorDateTimein a message would render with an invariant separator. EveryMessageArgssite and everylocalizer["…", …]call site was enumerated: allint,long,stringor enum, exceptMaxWindow.TotalDaysin the two audit-window validators, which is now anintat the source (that change travels with the module PR).Catalogs are named for specific cultures
SharedResources.pt-BR.resxhere, and the same convention for the ten module catalogs in the module PR. Renames only, no string changed.The asymmetry with the front-end is gone, and so is the trap behind it: a future
pt-PTis no longer served Brazilian strings by parent fallback. The documented consequence is that a bareptor an unsupported variant lands on the neutral English catalog rather than on Portuguese. Adding a language is: add the specific tag toSupportedCultures.Tags, add a*.{tag}.resxper catalog, add the JSON catalogs to both apps, and drop it from the front-endCANONmap if it was being folded into another tag..agents/rules/localization.mdrecords all of this.The
LocalecolumnThe original summary was wrong: there is no DB default. The column is nullable with
en-USas a code-level fallback, and it ischaracter varying(10)rather than unboundedtext— 10 covers language-script-region (zh-Hant-TW).AddUserLocalewas written as a singleAddColumnrather than stacked with anALTER, since it has never shipped in a release. Review caught that its timestamp (20260720…) sorted before20260807063015_DropIdentityOutbox, which has since landed onmain: the later migration by name carried a designer snapshot with noLocalecolumn, so anything generated on top of it would try to add the column a second time. It is regenerated at20260918053951, againstmain's snapshot, with the Up/Down bodies unchanged and a model snapshot byte-identical to the one already committed.Confirmed as you asked:
Validation.UnsupportedLocaleis wired at the write boundary.UpdateUserCommandValidatorrestrictsLocaletoSupportedCultures.Tags, onPUT /identity/profile, via the MediatorValidationBehavior. The column constraint is the storage-level backstop, not the validation.Because whole files cannot be split across PRs, three Identity files carry both the
Localeplumbing and theirIdentityResourceswiring in the same diff (IdentityService,UserProfileService,StartImpersonationCommandHandler). They ship here, which is whyIdentityResourcesand its two catalogs ride along in this PR rather than in the module one.Known behaviour (documented, not bugs)
localeclaim lags a language switch by one token. The provider reads the JWT claim, so a switch reaches the API at the next token issue. The front-end persists to the profile and re-mints, so it converges; in between, the shell can be in the new language while an API error is still in the old one. The alternative is a per-request DB read on every authenticated call.apiFetch, soAccept-Languageon the negotiate is the browser's. Applies to every session, not just impersonation. Named explicitly in the front-endhandoff-locale.spec.tsso any other channel that stops carrying the locale fails the test.UseExceptionHandler()sits ahead ofUseHeroLocalization(), which in turn has to sit afterUseAuthentication()because the culture provider reads thelocaleclaim offHttpContext.User. An exception thrown by anything in between — HTTPS redirection, CORS, static files, routing — is therefore rendered in the configured default culture rather than the caller's. Endpoint handlers, where every localized exception in this codebase is actually thrown, are unaffected. Moving the exception handler below localization would leave those middlewares with noProblemDetailsat all, which is the worse trade, so this stays as documented behaviour rather than being papered over.Also in this slice, from the last review round
LogContext.PushPropertyis scoped inusingblocks. Pre-existing AsyncLocal leak that contaminated every subsequent log entry in the request; unrelated to i18n, fixed here because the same lines were being touched.TitleKeyForsent everything outside four statuses toError.Unexpected; the type-name fallback beside it only fires onResourceNotFound, and that key resolves, so it never fired.#1344regressed this — before it,Titlewas the exception type name. Unmapped statuses fall back to the type name again, andConflictgets a real localized title. The 41Conflictthrow sites this affected are in Billing and Catalog, so the visible half of that fix lands with the module PR.ExceptionSeverityClassifieris now exercised with theLocalized*subclasses. They subclass the BCL types precisely so audit severity classification keeps working; changing a base type would have silently reclassified every unauthorized access with the suite green.The dependency bumps that
restoreneedsdotnet restore src/FSH.Starter.slnxfails onmainunderTreatWarningsAsErrors— not because ofthis PR — so the branch carries the Testcontainers 4.14.0 and SourceLink bumps that clear it, in the
same shape as the PR that owns them.
It used to carry an explicit
SSH.NETpin too, on the stated grounds that bumping Testcontainerswould not help. That was wrong: 4.14.0 declares
SSH.NET >= 2026.0.0, and with the pin removeddotnet restore --forcereports zero NU1902/NU1903 and exits 0. The pin is gone.Testing
Every number below is this slice on its own, at
mainplus these 56 files.dotnet restore src/FSH.Starter.slnxwith the audit on: exit 0, noNU1903.dotnet build -warnaserror: exit 0.#1344reported 15 assemblies; the missing one isTickets.Tests, which the module PR adds to the solution, and the remaining difference is that project plus the module-catalog tests. Nothing was dropped — the per-slice sums add back up.Chat.TypingIndicatorTests.Typing_Should_Throttle_To_OneEventPer3Seconds(a wall-clock throttle window) andMultitenancy.TenantHeaderOverrideTests.RootOperator_Should_TargetOtherTenant_When_HeaderProvided. Re-run on their own against the same build: 7 passed / 0 failed. Neither touches anything this PR changes, and both passed in the same run of the module slice, which contains this slice in full.The verdict above is aggregated per assembly rather than taken from the process exit code:
dotnet teston this solution has been observed exiting 0 while reporting failures, and zero assemblies reporting is itself treated as red.Docs (Golden Rule #10)
fullstackhero/docs#238, kept as a single PR covering all four slices —
internationalization.mdxis one page whose sections map across the split, so cutting it into four would put four PRs on the same file and leave three describing half a feature. From this slice it documents the culture resolution chain, per-user language, the new config section andcodeonProblemDetails, all of which are public contract.It should merge after the last of the four, not with this one: landing it here alone would publish the module-catalog and front-end sections before that code is on
main.Notes
en-USandpt-BRare held at strict key and placeholder parity bySharedResourcesKeyParityTests, so a missing or mis-arged translation fails the build instead of shipping English. The placeholder half was added in review: until then only the key sets were compared, and a translation that dropped{0}passed. The generic reflection-drivenCatalogParityTeststhat covers every module catalog travels with the module PR.PUT /identity/profileand is tracked separately in #1359.Review follow-ups
An independent review of this slice produced the following. All are in the branch:
that drops
{0}or renumbers it — the argument is swallowed, or the message throwsFormatExceptionwhere it is built. Verified by mutation: collapsing
"{0}/{1} bytes"to"{0} bytes"in the pt-BRcatalog turns the new assertion red.
OnChallenge, which the globalexception handler never sees, so it was the one error response that stayed English while everything
around it was negotiated. It resolves the localizer per request, which works because
UseRequestLocalizationsits ahead ofUseAuthorization, where the challenge is emitted.ChallengeLocalizationTestspins the body and that ordering.SupportedCultures.Tagsis aFrozenSet, not a public static array: the whitelist a validatorand a culture provider both trust should not be writable by any caller. Ordinal comparer, so the
matching semantics are unchanged.
AddUserLocalewas regenerated so it sorts after the migrations already onmain(detail above).Two things review raised that are deliberately not changed
mainloggedexception_title(the ProblemDetailsTitle) andexception_detail(the ProblemDetailsDetail);this PR logs
exception_type(the CLR type name) andexception_detail(the exception's ownMessage). Both halves are on purpose:TitleandDetailare now localized, so logging themwould make the log text follow the caller's
Accept-Languageand stop being groupable, while theCLR type name and the raw message do not move with culture. Two consequences for operators: a
dashboard or alert keyed on
exception_titlehas to be repointed atexception_type, and on a 500the logged
exception_detailis now the exception's own message rather than the generic "Anunexpected error occurred" — i.e. it can carry whatever the thrower put in the message, which is the
same exposure the stack trace already has, but in a field that used to be safe to index.
locale.PUT /identity/profiletreats a blank locale as"leave it alone", so a user who has picked a language cannot go back to "follow the browser". Adding
that is an API contract change (an explicit null, or a dedicated reset), not a fix to this slice, so
it is left as a known limitation rather than smuggled in here.
Infra carve-outs, corrected after review. Two things in the out-of-topic hunks were wrong and
are fixed on the branch:
minio/minioto quay.io.minio/mcis gone from Docker Hub too(
hub.docker.com/v2/repositories/minio/mc/answers 404) and it is whatminio-initruns, so bothdotnet run --project src/Host/FSH.Starter.AppHostanddocker compose updied on the pull and thefshbucket was never created. Now pinned to the same quay tag #1388 uses.SSH.NETpin is gone: it pinned nothing. Its own comment claimed bumping Testcontainersdoes not help, but 4.14.0 — which this branch also carries — declares
SSH.NET >= 2026.0.0.Measured rather than argued: with the pin removed,
dotnet restore src/FSH.Starter.slnx --forcereports zero NU1902/NU1903 and exits 0. (The MessagePack pin next to it stays; removing that one
does bring its advisory straight back.)
With both applied,
deploy/docker/docker-compose.ymlandsrc/Directory.Packages.propsare nowgenuinely byte-identical to #1388 (
git diff --exit-code, checked today), which the earlier claimwas not.