From 9846210fd654ce447a34225b0f34cc5e0f2a701f Mon Sep 17 00:00:00 2001 From: Paul Schneider Date: Sun, 12 Jul 2026 02:39:13 +0100 Subject: [PATCH] ApplicationDbContext: drop redundant HasOne on 3 Client navs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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().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. --- .../ClientControllerCollectionTests.cs | 150 +----------------- .../ClientController.Collections.cs | 1 - .../Models/ApplicationDbContext.cs | 3 - 3 files changed, 6 insertions(+), 148 deletions(-) diff --git a/src/Yavsc.Org.Tests/Controllers/ClientControllerCollectionTests.cs b/src/Yavsc.Org.Tests/Controllers/ClientControllerCollectionTests.cs index cf18970e..8883c75d 100644 --- a/src/Yavsc.Org.Tests/Controllers/ClientControllerCollectionTests.cs +++ b/src/Yavsc.Org.Tests/Controllers/ClientControllerCollectionTests.cs @@ -290,150 +290,12 @@ public class ClientControllerCollectionTests : IClassFixture BisectClientDbId() - { - await using var scope = _factory.Services.CreateAsyncScope(); - var db = scope.ServiceProvider.GetRequiredService(); - 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(); - 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(); - 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(); - 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(); - 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(); - 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(); - 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(); - 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(); - 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(); - 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(); - var id = await BisectClientDbId(); - var client = await db.Clients - .SingleOrDefaultAsync(c => c.Id == id, TestContext.Current.CancellationToken); - Assert.NotNull(client); - } + // 2026-07-11 fix/issue-3-splitquery: tests retirés car leur hypothèse + // ("bug de provider InMemory sur ces 3 entités") n'est pas la cause + // racine — c'est une redondance de mapping HasOne dans + // ApplicationDbContext.OnModelCreating. Le test gardien + // EditRedirectUris_GET_after_add_lists_both_uris reste en place + // comme sentinelle de régression sur le fix. private static string? ExtractAntiforgeryToken(string html) { diff --git a/src/Yavsc.Org/Controllers/Administration/ClientController.Collections.cs b/src/Yavsc.Org/Controllers/Administration/ClientController.Collections.cs index 5569bf77..b31ec0c3 100644 --- a/src/Yavsc.Org/Controllers/Administration/ClientController.Collections.cs +++ b/src/Yavsc.Org/Controllers/Administration/ClientController.Collections.cs @@ -287,7 +287,6 @@ public partial class ClientController private async Task LoadClientAsync(int id) => await dbContext.Clients - .AsSplitQuery() .Include(c => c.RedirectUris) .Include(c => c.PostLogoutRedirectUris) .Include(c => c.AllowedScopes) diff --git a/src/Yavsc.Server/Models/ApplicationDbContext.cs b/src/Yavsc.Server/Models/ApplicationDbContext.cs index c3404aed..fd924944 100644 --- a/src/Yavsc.Server/Models/ApplicationDbContext.cs +++ b/src/Yavsc.Server/Models/ApplicationDbContext.cs @@ -131,13 +131,10 @@ namespace Yavsc.Models // builder.Entity>().HasKey(i=> new { i.LoginProvider, i.UserId, i.ProviderKey }); builder.Entity().HasOne().WithMany(e => e.ClientSecrets).HasForeignKey(e => e.ClientId); - builder.Entity().HasOne().WithMany(e => e.AllowedScopes).HasForeignKey(e => e.ClientId); builder.Entity().HasOne().WithMany(e => e.IdentityProviderRestrictions).HasForeignKey(e => e.ClientId); builder.Entity().HasOne().WithMany(e => e.Properties).HasForeignKey(e => e.ClientId); builder.Entity().HasOne().WithMany(e => e.PostLogoutRedirectUris).HasForeignKey(e => e.ClientId); - builder.Entity().HasOne().WithMany(e => e.RedirectUris).HasForeignKey(e => e.ClientId); builder.Entity().HasOne().WithMany(e => e.AllowedCorsOrigins).HasForeignKey(e => e.ClientId); - builder.Entity().HasOne().WithMany(e => e.AllowedGrantTypes).HasForeignKey(e => e.ClientId); builder.Entity().HasOne().WithMany(e => e.Secrets).HasForeignKey(e => e.ApiResourceId); builder.Entity().HasOne().WithMany(e => e.Scopes).HasForeignKey(e => e.ApiResourceId); builder.Entity().HasOne().WithMany(e => e.UserClaims).HasForeignKey(e => e.ApiResourceId);