From b12c272df7fa91bee2ab27dc6df4438ac64660d1 Mon Sep 17 00:00:00 2001
From: Paul Schneider <paul@pschneider.fr>
Date: Sun, 12 Jul 2026 01:15:02 +0100
Subject: [PATCH] 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).
---
 .../ClientControllerCollectionTests.cs        | 229 ++++++++++++++++++
 .../ClientController.Collections.cs           |   1 +
 2 files changed, 230 insertions(+)

diff --git a/src/Yavsc.Org.Tests/Controllers/ClientControllerCollectionTests.cs b/src/Yavsc.Org.Tests/Controllers/ClientControllerCollectionTests.cs
index fe6bc2fc..cf18970e 100644
--- a/src/Yavsc.Org.Tests/Controllers/ClientControllerCollectionTests.cs
+++ b/src/Yavsc.Org.Tests/Controllers/ClientControllerCollectionTests.cs
@@ -110,6 +110,77 @@ public class ClientControllerCollectionTests : IClassFixture<TestWebApplicationF
         Assert.Contains("https://app.example.com/cb", body);
     }
 
+    // Repro: GET /Client/EditRedirectUris/{id} après qu'une seconde
+    // RedirectUri a été ajoutée via POST. LoadClientAsync(id) réhydrate
+    // le Client avec 9 Includes (RedirectUris, AllowedScopes, ClientSecrets,
+    // etc.) via l'InMemory provider. Sur l'Id=2 seedé, la matérialisation
+    // des nav properties sur les entités IdentityServer8 lève
+    // IndexOutOfRangeException — l'action renvoie un 500 et la page
+    // d'erreur masque le diagnostic.
+    //
+    // Ce test reproduit le chemin qui plante : GET initial (lit l'AF
+    // token) → POST AddRedirectUri → GET final qui exerce LoadClientAsync
+    // avec une collection RedirectUris non triviale (2 entrées).
+    // Il doit ÉCHOUER tant que le bug n'est pas traité.
+    [Fact]
+    public async Task EditRedirectUris_GET_after_add_lists_both_uris()
+    {
+        var http = CreateAdminClient();
+        var id = TargetClientDbId();
+        const string newUri = "https://app.example.com/cb-repro";
+
+        // Premier GET pour récupérer l'antiforgery token du form Add.
+        var pageResp = await http.GetAsync(
+            $"/Client/EditRedirectUris/{id}", TestContext.Current.CancellationToken);
+        pageResp.EnsureSuccessStatusCode();
+        var token = ExtractAntiforgeryToken(
+            await pageResp.Content.ReadAsStringAsync(TestContext.Current.CancellationToken));
+        Assert.False(string.IsNullOrEmpty(token));
+
+        // Ajout d'une seconde RedirectUri — c'est l'état non-trivial
+        // qui déclenche la matérialisation problématique côté
+        // InMemory provider.
+        var form = new MultipartFormDataContent
+        {
+            { new StringContent(newUri), "redirectUri" },
+            { new StringContent(token!), "__RequestVerificationToken" },
+        };
+        var addResp = await http.PostAsync(
+            $"/Client/AddRedirectUri/{id}", form, TestContext.Current.CancellationToken);
+        Assert.Equal(HttpStatusCode.OK, addResp.StatusCode);
+
+        try
+        {
+            // GET final : exerce LoadClientAsync(id) sur le client
+            // avec 2 RedirectUris, 1 AllowedScope, 1 GrantType, etc.
+            // Si la matérialisation échoue (IndexOutOfRange), ce GET
+            // renvoie 500 et EnsureSuccessStatusCode fait échouer le test.
+            var finalResp = await http.GetAsync(
+                $"/Client/EditRedirectUris/{id}", TestContext.Current.CancellationToken);
+            finalResp.EnsureSuccessStatusCode();
+            var finalBody = await finalResp.Content.ReadAsStringAsync(
+                TestContext.Current.CancellationToken);
+
+            // La page doit lister les DEUX URIs.
+            Assert.Contains("https://app.example.com/cb", finalBody);
+            Assert.Contains(newUri, finalBody);
+        }
+        finally
+        {
+            // Cleanup idempotent : on retire l'URI ajoutée pour ne
+            // pas polluer les autres tests de la collection.
+            using var scope = _factory.Services.CreateScope();
+            var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+            var row = db.ClientRedirectUris
+                .FirstOrDefault(r => r.RedirectUri == newUri);
+            if (row is not null)
+            {
+                db.ClientRedirectUris.Remove(row);
+                db.SaveChanges();
+            }
+        }
+    }
+
     [Fact]
     public async Task AddRedirectUri_POST_appends_to_database()
     {
@@ -206,6 +277,164 @@ public class ClientControllerCollectionTests : IClassFixture<TestWebApplicationF
         Assert.Equal(HttpStatusCode.NotFound, resp.StatusCode);
     }
 
+    // ---------- Bisection de l'Include qui plante ----------
+    //
+    // LoadClientAsync charge le Client avec 9 Includes :
+    //   RedirectUris, PostLogoutRedirectUris, AllowedScopes,
+    //   AllowedGrantTypes, AllowedCorsOrigins,
+    //   IdentityProviderRestrictions, Claims, Properties, ClientSecrets.
+    //
+    // Le test EditRedirectUris_GET_after_add_lists_both_uris échoue
+    // avec IndexOutOfRangeException au shaper de l'InMemory provider,
+    // sans préciser lequel des Includes pose problème. La stack
+    // indique IncludeCollection (donc un Include de collection, pas
+    // de référence).
+    //
+    // Ces tests exécutent la même query EF qu'un Include à la fois,
+    // sur le client cible déjà seedé par EnsureTargetClient, pour
+    // isoler le coupable. Tant que l'InMemory provider est
+    // incapable de matérialiser la nav, le test correspondant est
+    // rouge. Quand on a l'Include, on peut soit le charger autrement
+    // (AsSplitQuery, projection, etc.), soit basculer la fixture
+    // vers SQLite in-memory (cf. la discussion laissée ouverte
+    // 2026-07-11 sur fix/issue-3-splitquery).
+    private async Task<int> BisectClientDbId()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        return db.Clients.Single(c => c.ClientId == TargetClientId).Id;
+    }
+
+    [Fact]
+    public async Task Bisect_RedirectUris_alone()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .Include(c => c.RedirectUris)
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+        Assert.NotNull(client!.RedirectUris);
+    }
+
+    [Fact]
+    public async Task Bisect_PostLogoutRedirectUris_alone()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .Include(c => c.PostLogoutRedirectUris)
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+        Assert.NotNull(client!.PostLogoutRedirectUris);
+    }
+
+    [Fact]
+    public async Task Bisect_AllowedScopes_alone()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .Include(c => c.AllowedScopes)
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+        Assert.NotNull(client!.AllowedScopes);
+    }
+
+    [Fact]
+    public async Task Bisect_AllowedGrantTypes_alone()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .Include(c => c.AllowedGrantTypes)
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+        Assert.NotNull(client!.AllowedGrantTypes);
+    }
+
+    [Fact]
+    public async Task Bisect_AllowedCorsOrigins_alone()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .Include(c => c.AllowedCorsOrigins)
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+        Assert.NotNull(client!.AllowedCorsOrigins);
+    }
+
+    [Fact]
+    public async Task Bisect_IdentityProviderRestrictions_alone()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .Include(c => c.IdentityProviderRestrictions)
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+        Assert.NotNull(client!.IdentityProviderRestrictions);
+    }
+
+    [Fact]
+    public async Task Bisect_Claims_alone()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .Include(c => c.Claims)
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+        Assert.NotNull(client!.Claims);
+    }
+
+    [Fact]
+    public async Task Bisect_Properties_alone()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .Include(c => c.Properties)
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+        Assert.NotNull(client!.Properties);
+    }
+
+    [Fact]
+    public async Task Bisect_ClientSecrets_alone()
+    {
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .Include(c => c.ClientSecrets)
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+        Assert.NotNull(client!.ClientSecrets);
+    }
+
+    [Fact]
+    public async Task Bisect_baseline_no_include()
+    {
+        // Sanity check : la query de base (sans Include) doit passer.
+        // Si celle-ci échoue, le problème n'est pas un Include.
+        await using var scope = _factory.Services.CreateAsyncScope();
+        var db = scope.ServiceProvider.GetRequiredService<ApplicationDbContext>();
+        var id = await BisectClientDbId();
+        var client = await db.Clients
+            .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken);
+        Assert.NotNull(client);
+    }
+
     private static string? ExtractAntiforgeryToken(string html)
     {
         const string marker = "name=\"__RequestVerificationToken\"";
diff --git a/src/Yavsc.Org/Controllers/Administration/ClientController.Collections.cs b/src/Yavsc.Org/Controllers/Administration/ClientController.Collections.cs
index b31ec0c3..5569bf77 100644
--- a/src/Yavsc.Org/Controllers/Administration/ClientController.Collections.cs
+++ b/src/Yavsc.Org/Controllers/Administration/ClientController.Collections.cs
@@ -287,6 +287,7 @@ public partial class ClientController
 
     private async Task<Client?> LoadClientAsync(int id)
         => await dbContext.Clients
+            .AsSplitQuery()
             .Include(c => c.RedirectUris)
             .Include(c => c.PostLogoutRedirectUris)
             .Include(c => c.AllowedScopes)