fix/issue-3-splitquery #5

Merged
notazof merged 8 commits from fix/issue-3-splitquery into main 2026-07-12 02:47:16 +01:00
Owner

Corrige le chargement du contexte de donnée des clients d'API

Corrige le chargement du contexte de donnée des clients d'API
Tout billet a un auteur, tout commentaire a un auteur. On aligne la
base sur ce contrat (Postgres) en droppant les orphelins existants
puis en remplaçant les FK en cascade par des FK Restrict.

- ApplicationDbContext: fluent pour BlogPost.Author et Comment.Author
  en DeleteBehavior.Restrict.
- ApplicationUser: ajoute la nav inverse BlogComments (manquait,
  EF aurait sinon créé une shadow FK).
- Migration 20260711173717_EnforceBlogAuthorFKs: Up purge les
  Comment/BlogSpot dont l'AuthorId n'existe plus, log le volume,
  puis drop+add des FK. Down laisse la cascade (état pré-migration).

Le code applicatif (BlogSpotService.Details) s'appuiera sur cette
contrainte dans un commit séparé.
GET /BlogSpot/Details/1 retournait 500 (avec un 500-sur-500 sur
la page d'erreur elle-même) parce que DisplayTemplates
/ApplicationUser.cshtml faisait `var avuri = "/Avatars/" +
Model.UserName + ".s.png"` : avec <Nullable>enable</Nullable>,
Razor émet un null-check implicite sur Model.UserName et lève
NullReferenceException quand l'auteur n'a pas de UserName
posé (donnée héritée, user partiellement initialisé).

- UserDisplayHelpers.AvatarSrc : helper statique pur dans
  Yavsc.Abstract.Identity qui retourne
  YavscConstants.DefaultAvatar pour user null / UserName vide
  ou whitespace, et un path /avatars/<name>.s.png sinon.
- ApplicationUser.cshtml : utilise le helper.
- Tests : 4 cas (null, vide, whitespace, valide) dans
  Yavsc.Org.Tests/NonRegression.

Bonus : le path d'avatar passe de "/Avatars/" (S majuscule,
ne résolvait pas dans le middleware de fichiers statiques) à
YavscConstants.AvatarsPath ("/avatars" minuscule), pour fermer
l'autre trou que centraliser le calcul permettait de fixer
proprement.
Le commit 2 a fixé la NPE du /BlogSpot/Details/{id} en passant
le display template par UserDisplayHelpers.AvatarSrc, qui
défend contre un UserName null. Ce commit complète le filet
de non-régression et pose la doc d'architecture des tests.

- ApplicationUserDisplayTemplateTests : assert que le cshtml ne
  concatène plus directement Model.UserName (ancien code fautif)
  et qu'il utilise bien le helper. Si quelqu'un revert la ligne
  4 du cshtml, les tests cassent. Les autres usages de
  Model.UserName (alt, title, asp-route-id) sont autorisés : ils
  ne sont pas la cause du 500, juste laids si null.
- doc/testing.md : vue d'ensemble de la stratégie de test
  (conventions NonRegression/Mandatory/Smoke/Controllers, EF
  in-memory via InMemoryDatabaseRoot partagé, auth stubs,
  quand ne pas écrire de test).
- src/Yavsc.Tests.Shared/README.md : détails du scaffold partagé
  (WebHostFixture + son cycle de vie et ses hooks,
  TestAuthPolicyProvider, TestTokenIssuer) et des deux
  spécialisations dans le repo
  (Yavsc.Org.Tests.WebServerFixture et
  Yavsc.Blogs.Tests.BlogsWebServerFixture).
- doc/README.md : entrée vers testing.md dans l'index.
TestWebApplicationFactory used ASPNETCORE_ENVIRONMENT=Development, which
caused Program.Main's AddConfiguration("org") to load the tracked
appsettings-org.json (the reference file with the
'*** via dotnet user-secrets ou variable d'environnement ***'
placeholder connection string). Npgsql then failed to parse that
placeholder during host startup, failing six integration tests
(observed 2026-07-11: System.ArgumentException on
NpgsqlConnectionStringBuilder.set_Item).

Switching the test host to a dedicated Testing environment makes
AddConfiguration("org") pick up the new optional
appsettings-org.Testing.json file as the last source in the chain
(JSON → env vars), which overrides YavscConnection with the
InMemory marker and the Smtp section with the test stub values.

The .gitignore exception whitelists this file explicitly: it is a
configuration source for the test host, not a secrets file.

The WebServerFixture path is unchanged — it owns its
WebApplicationBuilder and adds the same in-memory override via its
BuildApp hook.
This reverts commit fa7794b7a0.
LoadClientAsync chains 9 .Include() calls on dbContext.Clients.
On Postgres (and InMemory for some IdentityServer8 nav types), the
resulting cartesian product trips the query shaper with
IndexOutOfRangeException at IncludeCollection materialisation time.
Bug reproduces in production on the Blog admin pages that load a
Client by id.

AsSplitQuery() rewrites the load as 9 separate SELECTs joined by
client id, which sidesteps the cartesian explosion and any shaper
ambiguity between Claims/Properties/ClientSecrets (which share
Type/Value column names across some IdentityServer8 versions).

Tests:
- EditRedirectUris_GET_after_add_lists_both_uris: end-to-end
  reproducer that adds a second RedirectUri via POST then re-GETs
  the editor. Guards the fix on the integration path.
- Bisect_*_alone: nine unit tests that exercise the same
  .SingleOrDefaultAsync(c => c.Id == id).Include(nav) on the
  InMemory provider, one nav at a time. Pinpointed three
  problematic navs (RedirectUris, AllowedScopes, AllowedGrantTypes)
  on InMemory; kept as a regression net for any future shaper
  regressions on the InMemory provider (not the Postgres path).
ApplicationDbContext: drop redundant HasOne on 3 Client navs
Some checks failed
Dotnet build and test / log-the-inputs (pull_request) Has been cancelled
Dotnet build and test / build (pull_request) Has been cancelled
9846210fd6
LoadClientAsync(id) on ClientController used to trip an
IndexOutOfRangeException at the InMemory shaper for any
.Include() of one of three Client navs: RedirectUris,
AllowedScopes, AllowedGrantTypes. Five other Client navs (with
the same EF shape and the same application-level config) worked
fine.

Bisection pointed at the InMemory provider; that hypothesis was
wrong. The real cause is in ApplicationDbContext.OnModelCreating:
yavsc was redeclaring the HasOne<Client>().WithMany(...).
HasForeignKey(e => e.ClientId) for all eight Client* navs. The
same relation is already declared (more completely, with
.IsRequired().OnDelete(DeleteBehavior.Cascade)) by
IdentityServer8's ConfigureClientStore via ModelBuilderExtensions.

The redundant mapping on three specific entities — ClientScope,
ClientRedirectUri, ClientGrantType — interacts with the InMemory
provider's shaper in a way that throws IndexOutOfRange. Removing
the redundancy fixes it.

This commit also walks back b12c272d:
- Drops .AsSplitQuery() from LoadClientAsync (no longer needed
  for the InMemory shaper, and the Postgres path it was a hedge
  against was a false alarm — there is no Postgres production
  bug here, only an InMemory shaper quirk surfaced by the
  redundant mapping).
- Removes the 9 Bisect_*_alone tests that were the artefact of
  the provider-hypothesis phase. They pointed at the right
  entities but for the wrong reason.
- Keeps EditRedirectUris_GET_after_add_lists_both_uris as the
  end-to-end regression sentinel: with the fix in place, it
  loads a Client with two RedirectUris and asserts both are
  rendered. Without the fix, it fails with IndexOutOfRange.
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
notazof/yavsc!5
No description provided.