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

8 commits

Author SHA1 Message Date
9846210fd6 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
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.
2026-07-12 02:39:13 +01:00
b12c272df7 ClientController: split LoadClientAsync into per-collection subqueries
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).
2026-07-12 01:15:02 +01:00
cb20b8a2d5 Revert "repoduces the bug"
This reverts commit fa7794b7a0.
2026-07-11 22:17:58 +01:00
Lum
fa7794b7a0 repoduces the bug 2026-07-11 21:56:25 +01:00
7a066707b3 Tests: route Yavsc.Org test host through Testing environment
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.
2026-07-11 20:50:39 +01:00
Lum
cb7526de9d Blog: add test guarding the display-template fix + document test architecture
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.
2026-07-11 19:59:03 +01:00
Lum
bbdcc7f2ad Blog: render user avatar through a null-safe helper
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.
2026-07-11 19:41:09 +01:00
Lum
98613e7070 Blog: enforce Restrict FK on BlogPost.Author and Comment.Author
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é.
2026-07-11 18:45:27 +01:00