From 08cd3a002d8b25ab60790cc575bdb84b8a99a47a Mon Sep 17 00:00:00 2001 From: Timothy Date: Wed, 26 Aug 2026 07:39:23 +0000 Subject: [PATCH] fix(690,758): count the same query a paged handler pages (#833) Co-authored-by: Timothy --- .../Queries/GetPagedFillerPresetsHandler.cs | 13 +- .../Queries/GetPagedCollectionsHandler.cs | 7 +- .../GetPagedMultiCollectionsHandler.cs | 6 +- .../GetPagedRerunCollectionsHandler.cs | 9 +- .../GetPagedSmartCollectionsHandler.cs | 6 +- .../Queries/GetPagedTraktListsHandler.cs | 13 +- .../Queries/GetPagedPlayoutsHandler.cs | 15 +- .../GetPagedProgramSchedulesHandler.cs | 7 +- .../Data/Repositories/MusicVideoRepository.cs | 11 +- .../Data/Repositories/TelevisionRepository.cs | 32 +- .../Paging/MediaCardsCountMatchesPageTests.cs | 166 ++++++++++ .../Paging/PagedQueryTotalCountTests.cs | 286 ++++++++++++++++++ docs/README.md | 1 + docs/api-conventions.md | 10 + docs/decisions/README.md | 1 + .../api/paged-count-matches-page-query.md | 83 +++++ 16 files changed, 629 insertions(+), 37 deletions(-) create mode 100644 ErsatzTV.Tests/Application/Paging/MediaCardsCountMatchesPageTests.cs create mode 100644 ErsatzTV.Tests/Application/Paging/PagedQueryTotalCountTests.cs create mode 100644 docs/decisions/records/api/paged-count-matches-page-query.md diff --git a/ErsatzTV.Application/Filler/Queries/GetPagedFillerPresetsHandler.cs b/ErsatzTV.Application/Filler/Queries/GetPagedFillerPresetsHandler.cs index b668e6d06..00e4bcc60 100644 --- a/ErsatzTV.Application/Filler/Queries/GetPagedFillerPresetsHandler.cs +++ b/ErsatzTV.Application/Filler/Queries/GetPagedFillerPresetsHandler.cs @@ -1,4 +1,5 @@ -using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Core.Domain.Filler; +using ErsatzTV.Infrastructure.Data; using Microsoft.EntityFrameworkCore; using static ErsatzTV.Application.Filler.Mapper; @@ -12,9 +13,13 @@ public class GetPagedFillerPresetsHandler(IDbContextFactory dbContext CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - int count = await dbContext.FillerPresets.CountAsync(cancellationToken); - List page = await dbContext.FillerPresets - .AsNoTracking() + // no filter today, but count and page are still derived from ONE query so that adding one + // cannot leave the count behind (api.paged-count-matches-page-query) + IQueryable query = dbContext.FillerPresets.AsNoTracking(); + + int count = await query.CountAsync(cancellationToken); + + List page = await query .OrderBy(f => f.Name) .Skip(request.PageNum * request.PageSize) .Take(request.PageSize) diff --git a/ErsatzTV.Application/MediaCollections/Queries/GetPagedCollectionsHandler.cs b/ErsatzTV.Application/MediaCollections/Queries/GetPagedCollectionsHandler.cs index dbecd0789..850716ba3 100644 --- a/ErsatzTV.Application/MediaCollections/Queries/GetPagedCollectionsHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Queries/GetPagedCollectionsHandler.cs @@ -1,4 +1,4 @@ -using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Domain; using ErsatzTV.Infrastructure.Data; using Microsoft.EntityFrameworkCore; using static ErsatzTV.Application.MediaCollections.Mapper; @@ -13,8 +13,6 @@ public class GetPagedCollectionsHandler(IDbContextFactory dbContextFa CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - int count = await dbContext.Collections.CountAsync(cancellationToken); - IQueryable query = dbContext.Collections.AsNoTracking(); if (!string.IsNullOrWhiteSpace(request.Query)) @@ -22,6 +20,9 @@ public class GetPagedCollectionsHandler(IDbContextFactory dbContextFa query = query.Where(c => EF.Functions.Like(c.Name, $"%{request.Query}%")); } + // count the SAME query the page is taken from, so the two cannot drift (issues #690, #758) + int count = await query.CountAsync(cancellationToken); + List page = await query .OrderBy(c => c.Name) .Skip(request.PageNum * request.PageSize) diff --git a/ErsatzTV.Application/MediaCollections/Queries/GetPagedMultiCollectionsHandler.cs b/ErsatzTV.Application/MediaCollections/Queries/GetPagedMultiCollectionsHandler.cs index 68f3b81b2..5ee029f36 100644 --- a/ErsatzTV.Application/MediaCollections/Queries/GetPagedMultiCollectionsHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Queries/GetPagedMultiCollectionsHandler.cs @@ -13,9 +13,6 @@ public class GetPagedMultiCollectionsHandler(IDbContextFactory dbCont CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - int count = await dbContext.MultiCollections - .CountAsync(mc => mc.OwnedByChannelId == null, cancellationToken); - IQueryable query = dbContext.MultiCollections .AsNoTracking() .Where(mc => mc.OwnedByChannelId == null); @@ -25,6 +22,9 @@ public class GetPagedMultiCollectionsHandler(IDbContextFactory dbCont query = query.Where(mc => EF.Functions.Like(mc.Name, $"%{request.Query}%")); } + // count the SAME query the page is taken from, so the two cannot drift (issues #690, #758) + int count = await query.CountAsync(cancellationToken); + List page = await query .OrderBy(mc => mc.Name) .Skip(request.PageNum * request.PageSize) diff --git a/ErsatzTV.Application/MediaCollections/Queries/GetPagedRerunCollectionsHandler.cs b/ErsatzTV.Application/MediaCollections/Queries/GetPagedRerunCollectionsHandler.cs index cfdf08378..7cb2ef67e 100644 --- a/ErsatzTV.Application/MediaCollections/Queries/GetPagedRerunCollectionsHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Queries/GetPagedRerunCollectionsHandler.cs @@ -13,18 +13,21 @@ public class GetPagedRerunCollectionsHandler(IDbContextFactory dbCont CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - int count = await dbContext.RerunCollections.CountAsync(cancellationToken); - - IQueryable query = dbContext.RerunCollections.AsNoTracking().IncludeSelectionDetails(); + IQueryable query = dbContext.RerunCollections.AsNoTracking(); if (!string.IsNullOrWhiteSpace(request.Query)) { query = query.Where(rc => EF.Functions.Like(rc.Name, $"%{request.Query}%")); } + // count the SAME query the page is taken from, so the two cannot drift (issues #690, #758). + // The includes belong to the page chain only — a COUNT does not materialize the graph. + int count = await query.CountAsync(cancellationToken); + // EF applies the includes to the paged subquery, so the selection graph is loaded for at most // PageSize rows — the per-request cost is bounded by the page, not by the table (issue #671). List page = await query + .IncludeSelectionDetails() .OrderBy(rc => rc.Name) .Skip(request.PageNum * request.PageSize) .Take(request.PageSize) diff --git a/ErsatzTV.Application/MediaCollections/Queries/GetPagedSmartCollectionsHandler.cs b/ErsatzTV.Application/MediaCollections/Queries/GetPagedSmartCollectionsHandler.cs index ccb2b1f31..059861f6e 100644 --- a/ErsatzTV.Application/MediaCollections/Queries/GetPagedSmartCollectionsHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Queries/GetPagedSmartCollectionsHandler.cs @@ -13,9 +13,6 @@ public class GetPagedSmartCollectionsHandler(IDbContextFactory dbCont CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - int count = await dbContext.SmartCollections - .CountAsync(sc => sc.OwnedByChannelId == null, cancellationToken); - IQueryable query = dbContext.SmartCollections .AsNoTracking() .Where(sc => sc.OwnedByChannelId == null); @@ -25,6 +22,9 @@ public class GetPagedSmartCollectionsHandler(IDbContextFactory dbCont query = query.Where(sc => EF.Functions.Like(sc.Name, $"%{request.Query}%")); } + // count the SAME query the page is taken from, so the two cannot drift (issues #690, #758) + int count = await query.CountAsync(cancellationToken); + List page = await query .OrderBy(s => s.Name) .Skip(request.PageNum * request.PageSize) diff --git a/ErsatzTV.Application/MediaCollections/Queries/GetPagedTraktListsHandler.cs b/ErsatzTV.Application/MediaCollections/Queries/GetPagedTraktListsHandler.cs index 2ae039daa..2f95e4615 100644 --- a/ErsatzTV.Application/MediaCollections/Queries/GetPagedTraktListsHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Queries/GetPagedTraktListsHandler.cs @@ -1,4 +1,5 @@ -using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; using Microsoft.EntityFrameworkCore; using static ErsatzTV.Application.MediaCollections.Mapper; @@ -12,9 +13,13 @@ public class GetPagedTraktListsHandler(IDbContextFactory dbContextFac CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - int count = await dbContext.TraktLists.CountAsync(cancellationToken); - List page = await dbContext.TraktLists - .AsNoTracking() + // no filter today, but count and page are still derived from ONE query so that adding one + // cannot leave the count behind (api.paged-count-matches-page-query) + IQueryable query = dbContext.TraktLists.AsNoTracking(); + + int count = await query.CountAsync(cancellationToken); + + List page = await query .OrderBy(l => l.Name) .Skip(request.PageNum * request.PageSize) .Take(request.PageSize) diff --git a/ErsatzTV.Application/Playouts/Queries/GetPagedPlayoutsHandler.cs b/ErsatzTV.Application/Playouts/Queries/GetPagedPlayoutsHandler.cs index a45972cee..8664fcbb0 100644 --- a/ErsatzTV.Application/Playouts/Queries/GetPagedPlayoutsHandler.cs +++ b/ErsatzTV.Application/Playouts/Queries/GetPagedPlayoutsHandler.cs @@ -1,4 +1,4 @@ -using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Domain; using ErsatzTV.Infrastructure.Data; using Microsoft.EntityFrameworkCore; using static ErsatzTV.Application.Playouts.Mapper; @@ -13,13 +13,8 @@ public class GetPagedPlayoutsHandler(IDbContextFactory dbContextFacto CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - int count = await dbContext.Playouts.CountAsync(cancellationToken); - IQueryable query = dbContext.Playouts .AsNoTracking() - .Include(p => p.Channel) - .Include(p => p.ProgramSchedule) - .Include(p => p.BuildStatus) .Filter(p => p.Channel != null); if (!string.IsNullOrWhiteSpace(request.Query)) @@ -27,7 +22,15 @@ public class GetPagedPlayoutsHandler(IDbContextFactory dbContextFacto query = query.Where(p => EF.Functions.Like(p.Channel.Name, $"%{request.Query}%")); } + // count the SAME query the page is taken from, so the two cannot drift (issues #690, #758). + // This is also what makes the `Channel != null` filter count, which the old unfiltered + // CountAsync over the whole DbSet did not. + int count = await query.CountAsync(cancellationToken); + List page = await query + .Include(p => p.Channel) + .Include(p => p.ProgramSchedule) + .Include(p => p.BuildStatus) .OrderBy(p => p.Channel.SortNumber) .Skip(request.PageNum * request.PageSize) .Take(request.PageSize) diff --git a/ErsatzTV.Application/ProgramSchedules/Queries/GetPagedProgramSchedulesHandler.cs b/ErsatzTV.Application/ProgramSchedules/Queries/GetPagedProgramSchedulesHandler.cs index 28d45f704..b09d74752 100644 --- a/ErsatzTV.Application/ProgramSchedules/Queries/GetPagedProgramSchedulesHandler.cs +++ b/ErsatzTV.Application/ProgramSchedules/Queries/GetPagedProgramSchedulesHandler.cs @@ -1,4 +1,4 @@ -using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Domain; using ErsatzTV.Infrastructure.Data; using Microsoft.EntityFrameworkCore; using static ErsatzTV.Application.ProgramSchedules.Mapper; @@ -13,8 +13,6 @@ public class GetPagedProgramSchedulesHandler(IDbContextFactory dbCont CancellationToken cancellationToken) { await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); - int count = await dbContext.ProgramSchedules.CountAsync(cancellationToken); - IQueryable query = dbContext.ProgramSchedules.AsNoTracking(); if (!string.IsNullOrWhiteSpace(request.Query)) @@ -22,6 +20,9 @@ public class GetPagedProgramSchedulesHandler(IDbContextFactory dbCont query = query.Where(ps => EF.Functions.Like(ps.Name, $"%{request.Query}%")); } + // count the SAME query the page is taken from, so the two cannot drift (issues #690, #758) + int count = await query.CountAsync(cancellationToken); + List page = await query .OrderBy(ps => ps.Name) .Skip(request.PageNum * request.PageSize) diff --git a/ErsatzTV.Infrastructure/Data/Repositories/MusicVideoRepository.cs b/ErsatzTV.Infrastructure/Data/Repositories/MusicVideoRepository.cs index 26aaf59d2..5f198ac5d 100644 --- a/ErsatzTV.Infrastructure/Data/Repositories/MusicVideoRepository.cs +++ b/ErsatzTV.Infrastructure/Data/Repositories/MusicVideoRepository.cs @@ -1,4 +1,4 @@ -using Dapper; +using Dapper; using ErsatzTV.Core; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Interfaces.Repositories; @@ -171,8 +171,15 @@ public class MusicVideoRepository : IMusicVideoRepository public async Task GetMusicVideoCount(int artistId) { await using TvContext dbContext = await _dbContextFactory.CreateDbContextAsync(); + // count the same population GetPagedMusicVideos pages — MusicVideoMetadata, not MusicVideo. + // A music video whose metadata row is missing (a scanner failure; FindOrphanPaths models + // exactly that state) is not pageable, so counting the item table over-reports + // (api.paged-count-matches-page-query, #832). return await dbContext.Connection.QuerySingleAsync( - @"SELECT COUNT(*) FROM MusicVideo WHERE ArtistId = @ArtistId", + @"SELECT COUNT(*) + FROM MusicVideoMetadata MVM + INNER JOIN MusicVideo M on MVM.MusicVideoId = M.Id + WHERE M.ArtistId = @ArtistId", new { ArtistId = artistId }); } diff --git a/ErsatzTV.Infrastructure/Data/Repositories/TelevisionRepository.cs b/ErsatzTV.Infrastructure/Data/Repositories/TelevisionRepository.cs index 4909612a7..7c2897889 100644 --- a/ErsatzTV.Infrastructure/Data/Repositories/TelevisionRepository.cs +++ b/ErsatzTV.Infrastructure/Data/Repositories/TelevisionRepository.cs @@ -1,4 +1,4 @@ -using Dapper; +using Dapper; using ErsatzTV.Core; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; @@ -134,9 +134,26 @@ public class TelevisionRepository : ITelevisionRepository public async Task GetSeasonCount(int showId) { await using TvContext dbContext = await _dbContextFactory.CreateDbContextAsync(); - return await dbContext.Seasons - .AsNoTracking() - .CountAsync(s => s.ShowId == showId); + // GetPagedSeasons expands the requested show to EVERY show sharing its Title+Year (the same + // show present in two libraries) and pages the union, so the count must expand identically + // or it under-reports (api.paged-count-matches-page-query, #832). + Option maybeShowMetadata = await dbContext.ShowMetadata + .SelectOneAsync(sm => sm.Id, sm => sm.ShowId == showId, CancellationToken.None); + + foreach (ShowMetadata showMetadata in maybeShowMetadata) + { + List showIds = await dbContext.ShowMetadata + .Filter(sm => sm.Title == showMetadata.Title && sm.Year == showMetadata.Year) + .Map(sm => sm.ShowId) + .ToListAsync(); + + return await dbContext.Seasons + .AsNoTracking() + .CountAsync(s => showIds.Contains(s.ShowId)); + } + + // no metadata for the requested show: GetPagedSeasons returns nothing, so neither does this + return 0; } public async Task> GetPagedSeasons( @@ -179,9 +196,12 @@ public class TelevisionRepository : ITelevisionRepository public async Task GetEpisodeCount(int seasonId) { await using TvContext dbContext = await _dbContextFactory.CreateDbContextAsync(); - return await dbContext.Episodes + // count the same population GetPagedEpisodes pages — EpisodeMetadata, not Episode. An + // episode whose metadata row is missing is not pageable, so counting the item table + // over-reports (api.paged-count-matches-page-query, #832). + return await dbContext.EpisodeMetadata .AsNoTracking() - .CountAsync(e => e.SeasonId == seasonId); + .CountAsync(em => em.Episode.SeasonId == seasonId); } public async Task> GetPagedEpisodes(int seasonId, int pageNumber, int pageSize) diff --git a/ErsatzTV.Tests/Application/Paging/MediaCardsCountMatchesPageTests.cs b/ErsatzTV.Tests/Application/Paging/MediaCardsCountMatchesPageTests.cs new file mode 100644 index 000000000..6358d2937 --- /dev/null +++ b/ErsatzTV.Tests/Application/Paging/MediaCardsCountMatchesPageTests.cs @@ -0,0 +1,166 @@ +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Infrastructure.Data.Repositories; +using ErsatzTV.Tests.Support; +using Microsoft.Extensions.Logging.Abstractions; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Application.Paging; + +/// +/// The MediaCards count/page pairs live in two separate repository methods rather than in one +/// handler, so `api.paged-count-matches-page-query` cannot be satisfied structurally there — the +/// two must be kept in agreement and pinned by a test instead. Each case below constructs the +/// divergence the count used to miss and asserts count == pageable rows. +/// Expected values are pinned literals, never re-derived from the method's own predicate. +/// +[TestFixture] +public class MediaCardsCountMatchesPageTests +{ + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private TelevisionRepository TelevisionRepo => + new(_db.Factory, NullLogger.Instance); + + [Test] + public async Task GetSeasonCount_Should_Expand_To_The_Same_Shows_GetPagedSeasons_Pages() + { + // the same show present in two libraries: same Title+Year, different Show rows. + // GetPagedSeasons pages the union (2 + 3), so the count must be 5, not 2. + await using (TvContext context = _db.CreateContext()) + { + context.Shows.Add(new Show { Id = 1 }); + context.Shows.Add(new Show { Id = 2 }); + context.ShowMetadata.Add(new ShowMetadata { Id = 201, ShowId = 1, Title = "Star Trek", Year = 1966 }); + context.ShowMetadata.Add(new ShowMetadata { Id = 202, ShowId = 2, Title = "Star Trek", Year = 1966 }); + + // an unrelated show that must NOT be swept in + context.Shows.Add(new Show { Id = 3 }); + context.ShowMetadata.Add(new ShowMetadata { Id = 203, ShowId = 3, Title = "Star Trek", Year = 1987 }); + context.Seasons.Add(new Season { Id = 19, ShowId = 3, SeasonNumber = 1 }); + + context.Seasons.Add(new Season { Id = 11, ShowId = 1, SeasonNumber = 1 }); + context.Seasons.Add(new Season { Id = 12, ShowId = 1, SeasonNumber = 2 }); + context.Seasons.Add(new Season { Id = 13, ShowId = 2, SeasonNumber = 1 }); + context.Seasons.Add(new Season { Id = 14, ShowId = 2, SeasonNumber = 2 }); + context.Seasons.Add(new Season { Id = 15, ShowId = 2, SeasonNumber = 3 }); + await context.SaveChangesAsync(); + } + + int count = await TelevisionRepo.GetSeasonCount(1); + List page = await TelevisionRepo.GetPagedSeasons(1, 1, 50, CancellationToken.None); + + count.ShouldBe(5); + page.Count.ShouldBe(5); + count.ShouldBe(page.Count); + + // pin WHICH rows, not just how many — a count and a page can agree on the wrong set + page.Select(s => s.Id).OrderBy(id => id).ShouldBe([11, 12, 13, 14, 15]); + + } + + [Test] + public async Task GetSeasonCount_Should_Be_Zero_When_The_Show_Has_No_Metadata() + { + // GetPagedSeasons returns nothing without a ShowMetadata row to expand from, so the count + // must agree rather than reporting the show's seasons + await using (TvContext context = _db.CreateContext()) + { + context.Shows.Add(new Show { Id = 1 }); + context.Seasons.Add(new Season { Id = 11, ShowId = 1, SeasonNumber = 1 }); + await context.SaveChangesAsync(); + } + + int count = await TelevisionRepo.GetSeasonCount(1); + List page = await TelevisionRepo.GetPagedSeasons(1, 1, 50, CancellationToken.None); + + count.ShouldBe(0); + page.ShouldBeEmpty(); + } + + [Test] + public async Task GetEpisodeCount_Should_Count_Episodes_That_Have_Metadata() + { + // 3 episodes, one of which lost its metadata row to a scanner failure. GetPagedEpisodes + // pages EpisodeMetadata, so only 2 are reachable and the count must say 2. + await using (TvContext context = _db.CreateContext()) + { + // GetPagedEpisodes's include chain reaches Episode -> Season -> Show through REQUIRED + // reference navs, which EF emits as INNER JOINs, so a missing Season or Show row drops + // every row and would make the page 0 for a reason unrelated to the count under test. + // The ShowMetadata leg is a COLLECTION nav (LEFT JOIN) and drops nothing — the row below + // is incidental, seeded only to keep the graph realistic. + context.Shows.Add(new Show { Id = 1 }); + context.ShowMetadata.Add(new ShowMetadata { Id = 201, ShowId = 1, Title = "Show", Year = 2000 }); + context.Seasons.Add(new Season { Id = 11, ShowId = 1, SeasonNumber = 1 }); + for (var i = 21; i <= 23; i++) + { + context.Episodes.Add(new Episode { Id = i, SeasonId = 11 }); + } + + context.EpisodeMetadata.Add(new EpisodeMetadata { Id = 221, EpisodeId = 21, EpisodeNumber = 1 }); + context.EpisodeMetadata.Add(new EpisodeMetadata { Id = 222, EpisodeId = 22, EpisodeNumber = 2 }); + + // an episode in a different season must not be swept in + context.Seasons.Add(new Season { Id = 12, ShowId = 1, SeasonNumber = 2 }); + context.Episodes.Add(new Episode { Id = 29, SeasonId = 12 }); + context.EpisodeMetadata.Add(new EpisodeMetadata { Id = 229, EpisodeId = 29, EpisodeNumber = 1 }); + + await context.SaveChangesAsync(); + } + + int count = await TelevisionRepo.GetEpisodeCount(11); + List page = await TelevisionRepo.GetPagedEpisodes(11, 1, 50); + + count.ShouldBe(2); + page.Count.ShouldBe(2); + count.ShouldBe(page.Count); + + // the two episodes WITH metadata, and not the other season's + page.Select(em => em.EpisodeId).OrderBy(id => id).ShouldBe([21, 22]); + } + + [Test] + public async Task GetMusicVideoCount_Should_Count_Music_Videos_That_Have_Metadata() + { + // 3 music videos for the artist, one without a metadata row; GetPagedMusicVideos pages + // MusicVideoMetadata, so the count must be 2 + await using (TvContext context = _db.CreateContext()) + { + context.Artists.Add(new Artist { Id = 41 }); + for (var i = 31; i <= 33; i++) + { + context.MusicVideos.Add(new MusicVideo { Id = i, ArtistId = 41 }); + } + + context.MusicVideoMetadata.Add(new MusicVideoMetadata { Id = 231, MusicVideoId = 31, Title = "A" }); + context.MusicVideoMetadata.Add(new MusicVideoMetadata { Id = 232, MusicVideoId = 32, Title = "B" }); + + // another artist's video must not be swept in + context.Artists.Add(new Artist { Id = 42 }); + context.MusicVideos.Add(new MusicVideo { Id = 39, ArtistId = 42 }); + context.MusicVideoMetadata.Add(new MusicVideoMetadata { Id = 239, MusicVideoId = 39, Title = "C" }); + + await context.SaveChangesAsync(); + } + + var repo = new MusicVideoRepository(_db.Factory); + + int count = await repo.GetMusicVideoCount(41); + List page = await repo.GetPagedMusicVideos(41, 1, 50); + + count.ShouldBe(2); + page.Count.ShouldBe(2); + count.ShouldBe(page.Count); + + // this artist's two videos with metadata, and not the other artist's + page.Select(m => m.Title).OrderBy(x => x).ShouldBe(["A", "B"]); + } +} diff --git a/ErsatzTV.Tests/Application/Paging/PagedQueryTotalCountTests.cs b/ErsatzTV.Tests/Application/Paging/PagedQueryTotalCountTests.cs new file mode 100644 index 000000000..3f6f72c0a --- /dev/null +++ b/ErsatzTV.Tests/Application/Paging/PagedQueryTotalCountTests.cs @@ -0,0 +1,286 @@ +using ErsatzTV.Application.MediaCollections; +using ErsatzTV.Application.Playouts; +using ErsatzTV.Application.ProgramSchedules; +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using NUnit.Framework; +using Shouldly; +using DomainChannel = ErsatzTV.Core.Domain.Channel; + +namespace ErsatzTV.Tests.Application.Paging; + +/// +/// Every paged handler whose page query applies a filter must compute its TotalCount from the SAME +/// query, or a filtered page reports the unfiltered total and the SPA paginates to pages that can +/// never contain anything (issues #690, #758). +/// Expected counts here are PINNED LITERALS derived from the seeded set by hand — never recomputed +/// by re-applying the handler's own predicate, which would pass whatever the handler happens to do. +/// +[TestFixture] +public class PagedQueryTotalCountTests +{ + // 5 seeded rows, of which exactly these 2 contain "Alpha" + private const int SeededRows = 5; + private const int MatchingAlpha = 2; + + private InMemoryTvContext _db = null!; + + [SetUp] + public async Task SetUp() => _db = await InMemoryTvContext.CreateAsync(); + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private static readonly string[] Names = + ["Alpha One", "Beta", "Alpha Two", "Gamma", "Delta"]; + + [Test] + public async Task GetPagedCollections_Filtered_Count_Should_Match_Filter() + { + await using (TvContext context = _db.CreateContext()) + { + for (var i = 0; i < Names.Length; i++) + { + context.Collections.Add(new Collection { Id = i + 1, Name = Names[i], MediaItems = [] }); + } + + await context.SaveChangesAsync(); + } + + var handler = new GetPagedCollectionsHandler(_db.Factory); + + PagedMediaCollectionsViewModel unfiltered = + await handler.Handle(new GetPagedCollections(string.Empty, 0, 10), CancellationToken.None); + unfiltered.TotalCount.ShouldBe(SeededRows); + + PagedMediaCollectionsViewModel filtered = + await handler.Handle(new GetPagedCollections("Alpha", 0, 10), CancellationToken.None); + + filtered.TotalCount.ShouldBe(MatchingAlpha); + filtered.Page.Select(c => c.Name).ShouldBe(["Alpha One", "Alpha Two"]); + } + + [Test] + public async Task GetPagedMultiCollections_Filtered_Count_Should_Match_Filter() + { + await using (TvContext context = _db.CreateContext()) + { + for (var i = 0; i < Names.Length; i++) + { + context.MultiCollections.Add(new MultiCollection { Id = i + 1, Name = Names[i] }); + } + + // channel-owned rows are excluded from BOTH the page and the count, filter or no filter + context.MultiCollections.Add( + new MultiCollection { Id = 99, Name = "Alpha Owned", OwnedByChannelId = 7 }); + await context.SaveChangesAsync(); + } + + var handler = new GetPagedMultiCollectionsHandler(_db.Factory); + + PagedMultiCollectionsViewModel unfiltered = + await handler.Handle(new GetPagedMultiCollections(string.Empty, 0, 10), CancellationToken.None); + unfiltered.TotalCount.ShouldBe(SeededRows); + + PagedMultiCollectionsViewModel filtered = + await handler.Handle(new GetPagedMultiCollections("Alpha", 0, 10), CancellationToken.None); + + filtered.TotalCount.ShouldBe(MatchingAlpha); + filtered.Page.Select(mc => mc.Name).ShouldBe(["Alpha One", "Alpha Two"]); + } + + [Test] + public async Task GetPagedSmartCollections_Filtered_Count_Should_Match_Filter() + { + await using (TvContext context = _db.CreateContext()) + { + for (var i = 0; i < Names.Length; i++) + { + context.SmartCollections.Add( + new SmartCollection { Id = i + 1, Name = Names[i], Query = "tag:family" }); + } + + context.SmartCollections.Add( + new SmartCollection + { + Id = 99, + Name = "Alpha Owned", + Query = "tag:family", + OwnedByChannelId = 7 + }); + await context.SaveChangesAsync(); + } + + var handler = new GetPagedSmartCollectionsHandler(_db.Factory); + + PagedSmartCollectionsViewModel unfiltered = + await handler.Handle(new GetPagedSmartCollections(string.Empty, 0, 10), CancellationToken.None); + unfiltered.TotalCount.ShouldBe(SeededRows); + + PagedSmartCollectionsViewModel filtered = + await handler.Handle(new GetPagedSmartCollections("Alpha", 0, 10), CancellationToken.None); + + filtered.TotalCount.ShouldBe(MatchingAlpha); + filtered.Page.Select(sc => sc.Name).ShouldBe(["Alpha One", "Alpha Two"]); + } + + [Test] + public async Task GetPagedRerunCollections_Filtered_Count_Should_Match_Filter() + { + await using (TvContext context = _db.CreateContext()) + { + for (var i = 0; i < Names.Length; i++) + { + context.RerunCollections.Add( + new RerunCollection + { + Id = i + 1, + Name = Names[i], + CollectionType = CollectionType.Collection, + CollectionId = i + 1 + }); + context.Collections.Add(new Collection { Id = i + 1, Name = Names[i], MediaItems = [] }); + } + + await context.SaveChangesAsync(); + } + + var handler = new GetPagedRerunCollectionsHandler(_db.Factory); + + PagedRerunCollectionsViewModel unfiltered = + await handler.Handle(new GetPagedRerunCollections(string.Empty, 0, 10), CancellationToken.None); + unfiltered.TotalCount.ShouldBe(SeededRows); + + PagedRerunCollectionsViewModel filtered = + await handler.Handle(new GetPagedRerunCollections("Alpha", 0, 10), CancellationToken.None); + + filtered.TotalCount.ShouldBe(MatchingAlpha); + filtered.Page.Select(rc => rc.Name).ShouldBe(["Alpha One", "Alpha Two"]); + + // the selection graph still loads for the page — moving IncludeSelectionDetails off the + // counted query must not stop the page from projecting it (issue #671) + filtered.Page.Select(rc => rc.Collection?.Name).ShouldBe(["Alpha One", "Alpha Two"]); + } + + [Test] + public async Task GetPagedProgramSchedules_Filtered_Count_Should_Match_Filter() + { + await using (TvContext context = _db.CreateContext()) + { + for (var i = 0; i < Names.Length; i++) + { + context.ProgramSchedules.Add(new ProgramSchedule { Id = i + 1, Name = Names[i] }); + } + + await context.SaveChangesAsync(); + } + + var handler = new GetPagedProgramSchedulesHandler(_db.Factory); + + PagedProgramSchedulesViewModel unfiltered = + await handler.Handle(new GetPagedProgramSchedules(string.Empty, 0, 10), CancellationToken.None); + unfiltered.TotalCount.ShouldBe(SeededRows); + + PagedProgramSchedulesViewModel filtered = + await handler.Handle(new GetPagedProgramSchedules("Alpha", 0, 10), CancellationToken.None); + + filtered.TotalCount.ShouldBe(MatchingAlpha); + filtered.Page.Select(ps => ps.Name).ShouldBe(["Alpha One", "Alpha Two"]); + } + + [Test] + public async Task GetPagedPlayouts_Filtered_Count_Should_Match_Filter() + { + await using (TvContext context = _db.CreateContext()) + { + for (var i = 0; i < Names.Length; i++) + { + context.Channels.Add(NewChannel(i + 1, $"{i + 1}", Names[i])); + context.Playouts.Add( + new Playout + { + Id = i + 1, + ChannelId = i + 1, + ScheduleKind = PlayoutScheduleKind.Classic + }); + } + + // a playout whose channel row does not exist: excluded from the page by the + // `Channel != null` filter, so it must be excluded from the count too + context.Playouts.Add( + new Playout { Id = 99, ChannelId = 4242, ScheduleKind = PlayoutScheduleKind.Classic }); + await context.SaveChangesAsync(); + } + + var handler = new GetPagedPlayoutsHandler(_db.Factory); + + PagedPlayoutsViewModel unfiltered = + await handler.Handle(new GetPagedPlayouts(string.Empty, 0, 10), CancellationToken.None); + + // 5, NOT 6 — the orphaned playout is filtered out of the page, so it is not part of the total + unfiltered.TotalCount.ShouldBe(SeededRows); + unfiltered.Page.Count.ShouldBe(SeededRows); + + PagedPlayoutsViewModel filtered = + await handler.Handle(new GetPagedPlayouts("Alpha", 0, 10), CancellationToken.None); + + filtered.TotalCount.ShouldBe(MatchingAlpha); + filtered.Page.Select(p => p.ChannelName).ShouldBe(["Alpha One", "Alpha Two"]); + } + + [Test] + public async Task Filtered_Count_Should_Drive_A_Second_Page() + { + await using (TvContext context = _db.CreateContext()) + { + for (var i = 0; i < Names.Length; i++) + { + context.Collections.Add(new Collection { Id = i + 1, Name = Names[i], MediaItems = [] }); + } + + await context.SaveChangesAsync(); + } + + var handler = new GetPagedCollectionsHandler(_db.Factory); + + // pageSize 1 over the 2 matching rows: this is the property the issues are about — the count + // is what tells the client a SECOND page exists, and each page holds exactly its own row + PagedMediaCollectionsViewModel first = + await handler.Handle(new GetPagedCollections("Alpha", 0, 1), CancellationToken.None); + first.TotalCount.ShouldBe(MatchingAlpha); + first.Page.Select(c => c.Name).ShouldBe(["Alpha One"]); + + PagedMediaCollectionsViewModel second = + await handler.Handle(new GetPagedCollections("Alpha", 1, 1), CancellationToken.None); + second.TotalCount.ShouldBe(MatchingAlpha); + second.Page.Select(c => c.Name).ShouldBe(["Alpha Two"]); + + // and the page AFTER the last matching row is empty — with the pre-fix count of 5 the client + // would have been told to fetch three more pages that can never contain anything + PagedMediaCollectionsViewModel past = + await handler.Handle(new GetPagedCollections("Alpha", 2, 1), CancellationToken.None); + past.TotalCount.ShouldBe(MatchingAlpha); + past.Page.ShouldBeEmpty(); + } + + private static DomainChannel NewChannel(int id, string number, string name) => + new(Guid.NewGuid()) + { + Id = id, + Number = number, + SortNumber = id, + Name = name, + Group = "ErsatzTV", + Categories = string.Empty, + FFmpegProfileId = 1, + StreamSelector = string.Empty, + PreferredAudioLanguageCode = string.Empty, + PreferredAudioTitle = string.Empty, + PreferredSubtitleLanguageCode = string.Empty, + MusicVideoCreditsTemplate = string.Empty, + StreamingMode = StreamingMode.TransportStreamHybrid, + PlayoutSource = ChannelPlayoutSource.Generated, + PlayoutMode = ChannelPlayoutMode.Continuous + }; +} diff --git a/docs/README.md b/docs/README.md index 2ee6233c2..a8f851f56 100644 --- a/docs/README.md +++ b/docs/README.md @@ -21,6 +21,7 @@ doc below, or that changes which sections a task signal points to.** | Adding/changing a `/api/*` endpoint | `docs/api-conventions.md` checklist + `docs/endpoint-index.md` | | Adding a ChicoryTV SPA screen | `docs/spa-conventions.md` | | Scheduling / playout engine work | `docs/domain-model.md` + decisions catalog rows keyed `sched.*` (`docs/decisions/README.md`) | +| Adding or changing a paged list handler (a page plus a `TotalCount`) | Resolve `api.paged-count-matches-page-query` via `docs/decisions/README.md` — for an EF-backed filtered list, count the SAME query you page, with includes appended to the page chain only; where the count and the page are separate methods, a test pins their agreement. Then `api.paging-zero-based` for the `pageNum`/`pageSize` contract | | Concurrency / optimistic-locking work | `docs/api-conventions.md` §7a/b/c + `docs/decisions/optimistic-concurrency.md` | | Auth / security-surface work | `docs/decisions/api-auth-security.md` | | CI / release pipeline work | `docs/ci-cd.md` + `docs/decisions/release-ci-governance.md` | diff --git a/docs/api-conventions.md b/docs/api-conventions.md index a02be0b11..3d582adb9 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -54,6 +54,16 @@ Exemplars: members, 1000 `search/all-items`). `OpenApiPagingContractTests` pins this and names the expected set of paged operations, so a new paged endpoint fails until it is added there **with** descriptions. See `api.paging-zero-based`. + **The handler behind it must compute its total from the SAME query it pages.** Build one + `IQueryable`, apply every filter to it, then take both `CountAsync` and the page from that object — + never `dbContext..CountAsync(ct)` beside a separate page query, and never a second + `CountAsync(pred, ct)` restating the predicate. Take the shape even before the first filter exists — + a handler with no filter today is where the drift is introduced tomorrow. The drifted state is silent and shaped like working + software: the page is right, the total is wrong, and the client believes the total, so the SPA + paginates to pages that can never fill and an MCP caller pages toward a completeness target it + cannot reach. Eager-loading `.Include(...)` belongs on the page chain only, appended after the + count — a `COUNT` does not materialize the graph. See `api.paged-count-matches-page-query` + (ersatztv#690, #758). - **Sortable GET with allow-listed sort params**: same file — `sortField`/`sortDirection` are normalized against a fixed allow-list (`AllowedSortFields`) rather than trusted or rejected with a 422: an unrecognized `sortField` silently falls back to the default field, an unrecognized diff --git a/docs/decisions/README.md b/docs/decisions/README.md index b20c68e00..a3456ad35 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -19,6 +19,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `api.logs-sort-params` | `GET /api/logs` takes allow-listed `sortField` (`timestamp`\|`level`) and `sortDirection` (`asc`\|`desc`) query params, normalized (not rejected) on an unrecognized value. | 2026-07-11 | [link](records/api/logs-sort-params.md) | | `api.mediatr-passthrough` | The REST API is thin controllers over existing MediatR handlers, with no new service/business-logic layer. | 2026-06 | [link](records/api/mediatr-passthrough.md) | | `api.openapi-mirrors-runtime` | The generated OpenAPI spec is made to match the runtime Newtonsoft wire contract (via `NewtonsoftSchemaNamingTransformer`), not the reverse. | 2026-07-09 | [link](records/api/openapi-mirrors-runtime.md) | +| `api.paged-count-matches-page-query` | A handler that returns a page plus a total count builds ONE `IQueryable`, applies every filter to it, and then derives BOTH the count and the page from that single object — `int count = await query.CountAsync(ct)` followed by `query.Include(...).OrderBy(...).Skip(...).Take(...)`. Counting the `DbSet` directly, or re-stating the predicate in a second `CountAsync(pred, ct)`, is the defect: the two expressions are then free to drift and nothing reports it. This is not a style preference — the drifted state is SILENT and shaped like working software. The page is correct, the count is wrong, and the client trusts the count: the SPA paginates on `TotalCount`, so 40 rows with 3 matching a search renders 4 pages of which 3 are permanently empty (#690), and an MCP agent paging to a completeness target reads a `totalCount` its own page can never reach (#758). Scope that harm honestly — of the six, only `GetPagedRerunCollections`, `GetPagedMultiCollections` and `GetPagedPlayouts` reach a controller today; `GetPagedCollections`, `GetPagedSmartCollections` and `GetPagedProgramSchedules` have no production caller (their REST routes use unpaged `GetAll*` queries), so they were latent, not live. Both named issues were ONE mechanism at six sites, of which the issues named two: `GetPagedCollections`, `GetPagedMultiCollections`, `GetPagedRerunCollections`, `GetPagedSmartCollections`, `GetPagedPlayouts`, `GetPagedProgramSchedules`. THE FILTER IS NOT ONLY THE SEARCH STRING — `GetPagedPlayouts` also applies `Filter(p => p.Channel != null)` to the page, and counting the DbSet missed that too; that clause is DEFENSIVE rather than a live defect, because `Playout.ChannelId` is non-nullable with `DeleteBehavior.Cascade` and both production connection strings set `foreign keys=true`, so the orphan state is unreachable while the FK holds. Three corollaries. (1) INCLUDES BELONG TO THE PAGE CHAIN, not to the shared filtered query: a COUNT does not materialize the graph, so `.Include(...)`/`IncludeSelectionDetails()` are appended after the count is taken, which keeps `api.selection-projection-include-chain` intact while leaving one predicate source. (2) A HANDLER WITH NO FILTER STILL TAKES THE SHAPE — `GetPagedFillerPresets` and `GetPagedTraktLists` take no `Query` parameter, so their `DbSet` counts were not WRONG, but leaving them counting one expression while paging another preserves exactly the drift this record is about for whoever adds the first filter. They derive both from one query too. (3) THE POPULATION IS DERIVED FROM THE SHAPE, NOT FROM THE `GetPaged*` NAME — three further count+page producers in `ErsatzTV.Application/MediaCards` (`GetTelevisionSeasonCards`, `GetTelevisionEpisodeCards`, `GetMusicVideoCards`) carry the same drift across a REPOSITORY boundary, where the count and the page are two interface methods rather than two expressions, so the structural fix cannot apply and they are pinned by a test instead (`MediaCardsCountMatchesPageTests`). `GetSeasonCount` now expands to the same Title+Year show set `GetPagedSeasons` pages; `GetEpisodeCount` and `GetMusicVideoCount` now count the METADATA table their pages are taken from, so a media item whose metadata row is missing no longer inflates the total. Their 1-based `pageNumber` is a separate defect against `api.paging-zero-based` and stays open in #832. | 2026-08-26 | [link](records/api/paged-count-matches-page-query.md) | | `api.paging-zero-based` | `pageNum` is 0-based across the entire `/api/v1` surface and every wrapper of it (MCP tool catalog, SPA hooks, docs); the page offset is always derived from the EFFECTIVE (bounded) `pageSize`, never the requested one, so a `pageSize` above an endpoint's cap narrows the page without widening the offset. The cap itself is per-endpoint (100 typical, 200 auto-tune members, 1000 search/all-items) and must not be documented as one number. A paging parameter description that omits or contradicts "0-based" is a defect. | 2026-07-25 | [link](records/api/paging-zero-based.md) | | `api.parentid-drillin` | Media drill-in (season/episode/artist/music-video) is served by an optional `parentId` query param on library-browse, not dedicated per-kind child-listing endpoints. | 2026-07-07 | [link](records/api/parentid-drillin.md) | | `api.playout-build-lock-409` | Every id-keyed playout/channel mutation endpoint checks `IEntityLocker.IsPlayoutLocked(id)` and returns 409 Conflict while a build is in-flight, mirroring Blazor's disabled-buttons behavior; reset-all stays 202 and silently skips locked playouts. | 2026-07-10 | [link](records/api/playout-build-lock-409.md) | diff --git a/docs/decisions/records/api/paged-count-matches-page-query.md b/docs/decisions/records/api/paged-count-matches-page-query.md new file mode 100644 index 000000000..b4973ba79 --- /dev/null +++ b/docs/decisions/records/api/paged-count-matches-page-query.md @@ -0,0 +1,83 @@ +--- +key: api.paged-count-matches-page-query +title: '2026-08-26 — A paged total is computed from the SAME query it pages — one IQueryable in a handler, a test-pinned pair where the count and page are separate methods (#690, #758)' +status: active +since: '2026-08-26' +supersedes: none +superseded-by: none +rule: 'A handler that returns a page plus a total count builds ONE `IQueryable`, applies every filter to it, and then derives BOTH the count and the page from that single object — `int count = await query.CountAsync(ct)` followed by `query.Include(...).OrderBy(...).Skip(...).Take(...)`. Counting the `DbSet` directly, or re-stating the predicate in a second `CountAsync(pred, ct)`, is the defect: the two expressions are then free to drift and nothing reports it. This is not a style preference — the drifted state is SILENT and shaped like working software. The page is correct, the count is wrong, and the client trusts the count: the SPA paginates on `TotalCount`, so 40 rows with 3 matching a search renders 4 pages of which 3 are permanently empty (#690), and an MCP agent paging to a completeness target reads a `totalCount` its own page can never reach (#758). Scope that harm honestly — of the six, only `GetPagedRerunCollections`, `GetPagedMultiCollections` and `GetPagedPlayouts` reach a controller today; `GetPagedCollections`, `GetPagedSmartCollections` and `GetPagedProgramSchedules` have no production caller (their REST routes use unpaged `GetAll*` queries), so they were latent, not live. Both named issues were ONE mechanism at six sites, of which the issues named two: `GetPagedCollections`, `GetPagedMultiCollections`, `GetPagedRerunCollections`, `GetPagedSmartCollections`, `GetPagedPlayouts`, `GetPagedProgramSchedules`. THE FILTER IS NOT ONLY THE SEARCH STRING — `GetPagedPlayouts` also applies `Filter(p => p.Channel != null)` to the page, and counting the DbSet missed that too; that clause is DEFENSIVE rather than a live defect, because `Playout.ChannelId` is non-nullable with `DeleteBehavior.Cascade` and both production connection strings set `foreign keys=true`, so the orphan state is unreachable while the FK holds. Three corollaries. (1) INCLUDES BELONG TO THE PAGE CHAIN, not to the shared filtered query: a COUNT does not materialize the graph, so `.Include(...)`/`IncludeSelectionDetails()` are appended after the count is taken, which keeps `api.selection-projection-include-chain` intact while leaving one predicate source. (2) A HANDLER WITH NO FILTER STILL TAKES THE SHAPE — `GetPagedFillerPresets` and `GetPagedTraktLists` take no `Query` parameter, so their `DbSet` counts were not WRONG, but leaving them counting one expression while paging another preserves exactly the drift this record is about for whoever adds the first filter. They derive both from one query too. (3) THE POPULATION IS DERIVED FROM THE SHAPE, NOT FROM THE `GetPaged*` NAME — three further count+page producers in `ErsatzTV.Application/MediaCards` (`GetTelevisionSeasonCards`, `GetTelevisionEpisodeCards`, `GetMusicVideoCards`) carry the same drift across a REPOSITORY boundary, where the count and the page are two interface methods rather than two expressions, so the structural fix cannot apply and they are pinned by a test instead (`MediaCardsCountMatchesPageTests`). `GetSeasonCount` now expands to the same Title+Year show set `GetPagedSeasons` pages; `GetEpisodeCount` and `GetMusicVideoCount` now count the METADATA table their pages are taken from, so a media item whose metadata row is missing no longer inflates the total. Their 1-based `pageNumber` is a separate defect against `api.paging-zero-based` and stays open in #832.' +signals: 'TotalCount ignores the search query · filtered page reports the unfiltered total · SPA renders empty pages after a search · agent pages to a completeness target it can never reach · count the same query you page · one predicate applied to both so they cannot drift · CountAsync on the DbSet · Channel != null missing from the count · includes belong to the page chain not the counted query · an unfiltered paged handler takes the shape too · a repository count and its page are two methods that must be pinned by a test · paths: `ErsatzTV.Application/MediaCollections/Queries/GetPagedCollectionsHandler.cs`, `ErsatzTV.Application/MediaCollections/Queries/GetPagedMultiCollectionsHandler.cs`, `ErsatzTV.Application/MediaCollections/Queries/GetPagedRerunCollectionsHandler.cs`, `ErsatzTV.Application/MediaCollections/Queries/GetPagedSmartCollectionsHandler.cs`, `ErsatzTV.Application/Playouts/Queries/GetPagedPlayoutsHandler.cs`, `ErsatzTV.Application/ProgramSchedules/Queries/GetPagedProgramSchedulesHandler.cs`, `ErsatzTV.Application/Filler/Queries/GetPagedFillerPresetsHandler.cs`, `ErsatzTV.Application/MediaCollections/Queries/GetPagedTraktListsHandler.cs`, `ErsatzTV.Infrastructure/Data/Repositories/TelevisionRepository.cs`, `ErsatzTV.Infrastructure/Data/Repositories/MusicVideoRepository.cs`, `ErsatzTV.Tests/Application/Paging/PagedQueryTotalCountTests.cs`, `ErsatzTV.Tests/Application/Paging/MediaCardsCountMatchesPageTests.cs` · issues: #758, #690, #671, #757' +mechanics: 'TWO enforcement modes, because the rule has two shapes. Where the count and the page are expressions in one handler, the STRUCTURE carries it (one `IQueryable`) and `PagedQueryTotalCountTests` pins at least one test per filtering paged handler. Where they are separate repository methods (`MediaCards`), nothing structural is available and `MediaCardsCountMatchesPageTests` pins their agreement instead. Every case asserts LITERAL expected counts AND the literal identities on the page — counts alone would let a count and a page agree on the WRONG SET and stay green. No repo-wide detector is proposed — see the record body for why the obvious one is not reliable.' +--- + +**The two issues were filed as separate bugs on separate entities, and they are one mechanism.** +#690 (rerun collections) and #758 (playouts) each describe a handler that counts +`dbContext..CountAsync(ct)` and then pages a differently-filtered `IQueryable`. Auditing only +the two named handlers would have fixed two of six sites and left the same defect live on +collections, multi-collections, smart collections and program schedules — which is +`an issue's file list is not the population` in its most ordinary form: the reporter found the +instance that bit them, not the class. + +**Why the fix is structural rather than "add the missing `Where` to the count".** The natural repair +is to give `CountAsync` a predicate matching the page's. That restores today's correctness and +preserves the defect: two expressions stating one intent, which the next person to add a filter has +to remember to update in both places. `GetPagedMultiCollections` and `GetPagedSmartCollections` were +already in exactly that half-state — their counts carried the `OwnedByChannelId == null` clause, +faithfully, and silently omitted the `Query` clause added later. The predicate that drifts is the one +added after the count was written, so no amount of care in the existing line reaches it. Deriving +both from one object removes the possibility rather than asserting its absence. + +**The three `MediaCards` sites are the reason the population is stated by shape**, and the reason the +rule needs a second enforcement mode. Their count and their page are two different *repository +methods*, not two expressions in one handler, so "derive both from one `IQueryable`" has nothing to +attach to: `GetSeasonCount` counted `ShowId == showId` while `GetPagedSeasons` pages every show +sharing a Title+Year, and the episode and music-video pairs counted the item table while paging the +*metadata* table, so a media item whose metadata row was lost to a scanner failure inflated the +total. Where the structure cannot carry the invariant, a test does: `MediaCardsCountMatchesPageTests` +constructs each divergence and asserts `count == pageable rows`. + +**Two things that surfaced only by writing those tests, and are the reason they are worth keeping.** +The seasons count has a THIRD answer nobody would guess from the count alone — with no `ShowMetadata` +row there is nothing to expand from, so `GetPagedSeasons` returns nothing and the count must be 0 +rather than the show's season total; that case is pinned separately. And an include chain can filter more narrowly than +the count, in ALL THREE pairs rather than the one it was first noticed in: a REQUIRED reference +`Include` is emitted as an INNER JOIN, so `GetPagedEpisodes` (`Episode -> Season -> Show`), +`GetPagedSeasons` (`Include(s => s.Show)`) and `GetPagedMusicVideos` (`ThenInclude(mv => mv.Artist)`) +each return nothing when the principal row is absent, while the corrected count still counts. A +COLLECTION `Include` such as `Show.ShowMetadata` is a LEFT JOIN and drops nothing — the distinction +is the whole mechanism, so do not read "an include filters" as a blanket claim. Scope the consequence +honestly, the same way the `Channel != null` clause above is scoped: `Episode.SeasonId`, `Season.ShowId` and +`MusicVideo.ArtistId` are all non-nullable with `DeleteBehavior.Cascade` and production enforces the FK, +so count-N / page-0 is a corruption-only state no user can reach. It is stated because it makes +"count the table the page reads" necessary and NOT sufficient as a general rule, not because a live +defect is being left open; #832 carries it. + +**`GetBlockPlayoutHistory`, `GetFuturePlayoutItemsById` and `GetLibraryBrowseItems` already did +this** — they build the filtered query, count it, then page it. The idiom was in the repo; the six +defective handlers predate it or were written beside it. That is the reason this is written down as a +convention: the correct shape existing somewhere did not stop six handlers from taking the other one. + +**Why no repo-wide detector.** The plausible check is "a handler containing both `CountAsync` and a +conditional `Where` must count a variable, not a `DbSet`". It cannot distinguish the legitimate cases: +`GetPlayoutWarningsCount` is a bare count with a predicate that pages nothing, +`DeleteFFmpegProfileHandler` counts rows to decide whether a delete is allowed, and +`GetLibraryBrowseItems` sums five independently-filtered counts across entity types in a way no +single-query rule describes. It would also miss the three `MediaCards` sites entirely, since there +the count and the page are not in the same file at all. A detector that flags those reads +as noise and gets suppressed. The population is instead enumerated by SHAPE — every handler +returning a page plus a count, found by reading the git index for `CountAsync`/`.Skip(`/`TotalCount` +rather than for the `GetPaged*` name — and pinned by at least one test per filtering instance. That +distinction is not pedantic: the name-derived population is eight handlers and misses the three +`MediaCards` sites entirely — they were found, and fixed, only because the population was re-derived +by shape. That is the failure this record's own first paragraph names, committed once inside the fix +for it. (#832 carries what is deliberately left there: 1-based paging and delete-or-keep.) + +**The tests assert pinned literals, not filter-derived expectations.** Each seeds five matchable rows +of which exactly two contain `"Alpha"` — plus, where the handler carries a non-search clause, one row +that clause must exclude from BOTH sides (a channel-owned collection, an orphaned playout) — then +asserts `TotalCount.ShouldBe(2)` and the page's names against the literal pair. Recomputing the expectation by re-applying the handler's own predicate would pass +whatever the handler does. The mutation proof is recorded in the PR, in both modes: restoring the +pre-fix `CountAsync` clause at all six handler sites turns the six per-handler +`PagedQueryTotalCountTests` cases red, and each of those constructs exactly one handler, so every +test is shown to detect its own site rather than a neighbour's; restoring all three pre-fix +repository counts turns all four `MediaCardsCountMatchesPageTests` red.