diff --git a/ErsatzTV.Tests/Controllers/OpenApiPagingContractTests.cs b/ErsatzTV.Tests/Controllers/OpenApiPagingContractTests.cs new file mode 100644 index 000000000..bb7e9ebf2 --- /dev/null +++ b/ErsatzTV.Tests/Controllers/OpenApiPagingContractTests.cs @@ -0,0 +1,183 @@ +using System.Text.RegularExpressions; +using ErsatzTV.Tests.Support; +using Microsoft.OpenApi; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Controllers; + +/// +/// Pins the api.paging-zero-based contract onto the generated OpenAPI document (ersatztv#633). +/// The spec is the contract REST consumers read — and what generated clients surface to their users — +/// so a paging parameter that documents nothing forces every consumer to infer the base from +/// default: 0. That is exactly the inference that cost ersatztv#487 a verification pass on the +/// MCP side, where the description was present but wrong. The MCP wrapper is pinned the same way in +/// ErsatzTV.Mcp.Tests.ToolCatalogTests; this is the API-side half. +/// +[TestFixture] +public class OpenApiPagingContractTests +{ + /// + /// Every operation that pages. Named explicitly rather than discovered, because a test that only + /// FILTERS on "declares pageNum" cannot see the endpoint that should page and does not — the + /// defect escapes the filter and the test still passes green over a shrinking scope. That is not + /// hypothetical: ersatztv#616 found two MCP tools doing precisely that. So the expected set is + /// pinned here, and asserts the + /// discovered set equals it in BOTH directions — a new paged endpoint fails until it is added + /// (with descriptions), and an endpoint that silently drops paging fails too. + /// + private static readonly string[] PagedOperations = + [ + "GET /api/v1/channels/auto-tune/members", + "GET /api/v1/collections/{id}/items", + "GET /api/v1/library/browse", + "GET /api/v1/logs", + "GET /api/v1/multi-collections", + "GET /api/v1/playouts", + "GET /api/v1/playouts/{id}/blocks/{blockId}/history", + "GET /api/v1/playouts/{id}/items", + "GET /api/v1/rerun-collections", + "GET /api/v1/search", + "GET /api/v1/search/all-items", + "GET /api/v1/trakt/lists" + ]; + + private static OpenApiDocument _document = null!; + + [OneTimeSetUp] + public async Task BuildDocument() => _document = await GeneratedOpenApiDocument.BuildV1Async(); + + [Test] + public void Paged_Operations_Should_Be_Exactly_The_Pinned_Set() + { + List discovered = EnumerateOperations() + .Where(op => ParameterNames(op.Operation).Overlaps(new[] { "pageNum", "pageSize" })) + .Select(op => $"{op.Method} {op.Path}") + .OrderBy(s => s, StringComparer.Ordinal) + .ToList(); + + discovered.ShouldBe(PagedOperations.OrderBy(s => s, StringComparer.Ordinal).ToList()); + } + + [Test] + public void Every_Paged_Operation_Should_Declare_Both_Paging_Parameters() + { + foreach (string key in PagedOperations) + { + HashSet names = ParameterNames(Find(key)); + + names.ShouldContain("pageNum", $"{key} should declare pageNum"); + names.ShouldContain("pageSize", $"{key} should declare pageSize"); + } + } + + [Test] + public void Every_PageNum_Parameter_Should_Document_The_ZeroBased_Contract() + { + foreach (string key in PagedOperations) + { + string description = Description(key, "pageNum"); + + // The whole point of the record: a consumer must not have to infer the base from `default: 0`. + description.ShouldContain("0-based", Case.Insensitive, $"{key} pageNum should say it is 0-based"); + description.ShouldNotContain("1-based", Case.Insensitive, $"{key} pageNum must not claim 1-based"); + } + } + + [Test] + public void Every_PageSize_Parameter_Should_Document_The_Cap_And_The_Effective_Offset() + { + foreach (string key in PagedOperations) + { + string description = Description(key, "pageSize"); + + // `api.paging-zero-based` is explicit that the cap is PER-ENDPOINT and must not be documented + // as one number, and that the offset derives from the effective (capped) size — so an + // over-large pageSize narrows the page without widening the offset. + description.ShouldContain("capped at", Case.Insensitive, $"{key} pageSize should state its cap"); + description.ShouldContain("this endpoint", Case.Insensitive, $"{key} pageSize cap should be scoped to the endpoint"); + description.ShouldContain("effective", Case.Insensitive, $"{key} pageSize should explain the effective-size offset"); + } + } + + [Test] + public void PageSize_Caps_Should_Match_The_Values_The_Controllers_Actually_Clamp_To() + { + // The caps genuinely differ per endpoint, which is why the record forbids documenting one number. + // A description naming the wrong cap is worse than none — a wrong justification outlives a wrong + // line — so pin each against the value its controller clamps to. + var expectedCaps = new Dictionary(StringComparer.Ordinal) + { + ["GET /api/v1/channels/auto-tune/members"] = 200, + ["GET /api/v1/collections/{id}/items"] = 100, + ["GET /api/v1/library/browse"] = 100, + ["GET /api/v1/logs"] = 100, + ["GET /api/v1/multi-collections"] = 100, + ["GET /api/v1/playouts"] = 100, + ["GET /api/v1/playouts/{id}/blocks/{blockId}/history"] = 100, + ["GET /api/v1/playouts/{id}/items"] = 100, + ["GET /api/v1/rerun-collections"] = 100, + ["GET /api/v1/search"] = 100, + ["GET /api/v1/search/all-items"] = 1000, + ["GET /api/v1/trakt/lists"] = 100 + }; + + // Guard the guard: every pinned operation must carry an expected cap, so adding one above + // without its cap here cannot quietly skip this assertion. + expectedCaps.Keys.OrderBy(k => k, StringComparer.Ordinal) + .ShouldBe(PagedOperations.OrderBy(k => k, StringComparer.Ordinal)); + + foreach ((string key, int cap) in expectedCaps) + { + // Enumerate EVERY cap claim in the description and require the set to be exactly one + // number, the right one. Two weaker forms were rejected on the way here: + // - ShouldContain("capped at 100") is satisfied by the string "capped at 1000", so a + // cap-100 endpoint claiming 1000 passed — the very defect this test exists to catch. + // - Matching one occurrence as a whole token ("capped at 100(?!\d)") fixes that, but + // still passes a description that names a wrong cap somewhere ELSE in the sentence + // and the right one later. Presence of a true claim is not absence of a false one. + List claimedCaps = Regex + .Matches(Description(key, "pageSize"), @"capped at (\d+)", RegexOptions.IgnoreCase) + .Select(match => int.Parse(match.Groups[1].Value)) + .ToList(); + + claimedCaps.ShouldBe([cap], $"{key} pageSize should make exactly one cap claim, of {cap}"); + } + } + + private static string Description(string key, string parameterName) + { + // Not `First(...)`: a missing parameter would throw "Sequence contains no matching element", + // which names neither the endpoint nor the parameter and reads as a broken test rather than + // the contract violation it is. + IOpenApiParameter parameter = (Find(key).Parameters ?? []) + .FirstOrDefault(p => string.Equals(p.Name, parameterName, StringComparison.Ordinal)) + .ShouldNotBeNull($"{key} should declare a {parameterName} parameter"); + + string? description = parameter.Description; + description.ShouldNotBeNullOrWhiteSpace($"{key} {parameterName} should carry a description"); + + return description!; + } + + private static OpenApiOperation Find(string key) => + EnumerateOperations() + .Where(op => string.Equals($"{op.Method} {op.Path}", key, StringComparison.Ordinal)) + .Select(op => op.Operation) + .FirstOrDefault() + .ShouldNotBeNull($"{key} should exist in the generated document"); + + private static HashSet ParameterNames(OpenApiOperation operation) => + (operation.Parameters ?? []).Select(p => p.Name ?? string.Empty).ToHashSet(StringComparer.Ordinal); + + private static IEnumerable<(string Method, string Path, OpenApiOperation Operation)> EnumerateOperations() + { + foreach ((string path, IOpenApiPathItem item) in _document.Paths) + { + foreach ((HttpMethod method, OpenApiOperation operation) in item.Operations!) + { + yield return (method.Method.ToUpperInvariant(), path, operation); + } + } + } +} diff --git a/ErsatzTV/Controllers/Api/ChannelController.cs b/ErsatzTV/Controllers/Api/ChannelController.cs index 496e3c244..a688295e5 100644 --- a/ErsatzTV/Controllers/Api/ChannelController.cs +++ b/ErsatzTV/Controllers/Api/ChannelController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using System.ComponentModel.DataAnnotations; using System.Threading.Channels; using ErsatzTV.Application; @@ -264,8 +265,12 @@ public class ChannelController( public async Task GetAutoTuneChannelMembers( [FromQuery] AutoTuneAxis axis, [FromQuery] string value, - [FromQuery] int pageNum, - [FromQuery] int pageSize, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum, + [FromQuery] + [Description("Rows per page; capped at 200 for this endpoint. A value of 0 or less falls back to 100 rather than being clamped to 1. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize, CancellationToken cancellationToken) { pageNum = Math.Max(0, pageNum); diff --git a/ErsatzTV/Controllers/Api/CollectionController.cs b/ErsatzTV/Controllers/Api/CollectionController.cs index 2a7e81e5a..4e64d8ece 100644 --- a/ErsatzTV/Controllers/Api/CollectionController.cs +++ b/ErsatzTV/Controllers/Api/CollectionController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using System.ComponentModel.DataAnnotations; using ErsatzTV.Application.MediaCollections; using ErsatzTV.Controllers.Api.Requests; @@ -46,8 +47,12 @@ public class CollectionController(IMediator mediator) : ControllerBase [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] public async Task GetItems( int id, - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 100, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 100, CancellationToken cancellationToken = default) { int clampedPageNum = Math.Max(0, pageNum); diff --git a/ErsatzTV/Controllers/Api/LibraryBrowseController.cs b/ErsatzTV/Controllers/Api/LibraryBrowseController.cs index 0a5997550..8d05baae2 100644 --- a/ErsatzTV/Controllers/Api/LibraryBrowseController.cs +++ b/ErsatzTV/Controllers/Api/LibraryBrowseController.cs @@ -21,8 +21,12 @@ public class LibraryBrowseController(IMediator mediator) : ControllerBase [FromQuery] string query = "", [FromQuery] int? libraryId = null, [FromQuery] LibraryBrowseMediaType? mediaType = null, - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 100, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 100, [FromQuery] [Description("Parent id for a drill-in listing; only used with mediaType=TelevisionSeason (that show's seasons), mediaType=Episode (that season's episodes) or mediaType=MusicVideo (that artist's music videos), ignored otherwise")] int? parentId = null, diff --git a/ErsatzTV/Controllers/Api/LogsController.cs b/ErsatzTV/Controllers/Api/LogsController.cs index a079129ca..2b8276e70 100644 --- a/ErsatzTV/Controllers/Api/LogsController.cs +++ b/ErsatzTV/Controllers/Api/LogsController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using System.Linq.Expressions; using ErsatzTV.Application.Logs; using ErsatzTV.Core.Api.Logs; @@ -29,8 +30,12 @@ public class LogsController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(typeof(PagedLogEntriesResponseModel), StatusCodes.Status200OK)] public async Task GetLogs( - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 100, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 100, [FromQuery] string filter = "", [FromQuery] string sortField = "timestamp", [FromQuery] string sortDirection = "desc", diff --git a/ErsatzTV/Controllers/Api/MultiCollectionController.cs b/ErsatzTV/Controllers/Api/MultiCollectionController.cs index 21796e795..e8f6a2f01 100644 --- a/ErsatzTV/Controllers/Api/MultiCollectionController.cs +++ b/ErsatzTV/Controllers/Api/MultiCollectionController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using System.ComponentModel.DataAnnotations; using ErsatzTV.Application.MediaCollections; using ErsatzTV.Controllers.Api.Requests; @@ -22,8 +23,12 @@ public class MultiCollectionController(IMediator mediator) : ControllerBase [ProducesResponseType(typeof(PagedMultiCollectionsResponseModel), StatusCodes.Status200OK)] public async Task GetAll( [FromQuery] string query = "", - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 100, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 100, CancellationToken cancellationToken = default) { int clampedPageNum = Math.Max(0, pageNum); diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index 4fa94da83..1d0846b80 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using System.ComponentModel.DataAnnotations; using ErsatzTV.Application.Playouts; using ErsatzTV.Application.ProgramSchedules; @@ -41,8 +42,12 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : [ProducesResponseType(typeof(PagedPlayoutsResponseModel), StatusCodes.Status200OK)] public async Task GetAll( [FromQuery] string query = "", - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 100, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 100, CancellationToken cancellationToken = default) { pageNum = Math.Max(0, pageNum); @@ -83,8 +88,12 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : public async Task GetItems( int id, [FromQuery] bool showFiller = false, - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 100, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 100, CancellationToken cancellationToken = default) { Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); @@ -543,8 +552,12 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : public async Task GetBlockHistory( int id, int blockId, - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 100, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 100, CancellationToken cancellationToken = default) { Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); diff --git a/ErsatzTV/Controllers/Api/RerunCollectionController.cs b/ErsatzTV/Controllers/Api/RerunCollectionController.cs index 8262afe24..4426c3ce6 100644 --- a/ErsatzTV/Controllers/Api/RerunCollectionController.cs +++ b/ErsatzTV/Controllers/Api/RerunCollectionController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using System.ComponentModel.DataAnnotations; using ErsatzTV.Application.MediaCollections; using ErsatzTV.Controllers.Api.Requests; @@ -22,8 +23,12 @@ public class RerunCollectionController(IMediator mediator) : ControllerBase [ProducesResponseType(typeof(PagedRerunCollectionsResponseModel), StatusCodes.Status200OK)] public async Task GetAll( [FromQuery] string query = "", - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 100, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 100, CancellationToken cancellationToken = default) { int clampedPageNum = Math.Max(0, pageNum); diff --git a/ErsatzTV/Controllers/Api/SearchController.cs b/ErsatzTV/Controllers/Api/SearchController.cs index b2153e374..1f9c39d86 100644 --- a/ErsatzTV/Controllers/Api/SearchController.cs +++ b/ErsatzTV/Controllers/Api/SearchController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using ErsatzTV.Application.MediaCollections; using ErsatzTV.Application.MediaItems; using ErsatzTV.Application.Search; @@ -35,8 +36,12 @@ public class SearchController(IMediator mediator) : ControllerBase [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Search( [FromQuery] string query = "", - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 50, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 50); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 50, CancellationToken cancellationToken = default) { if (string.IsNullOrWhiteSpace(query)) @@ -65,8 +70,12 @@ public class SearchController(IMediator mediator) : ControllerBase [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task SearchAllItems( [FromQuery] string query = "", - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = DefaultAllItemsPageSize, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0. Unlike the other paged endpoints this one is also bounded ABOVE, at 2000000, so that pageNum * pageSize cannot overflow; a larger value is clamped down to that maximum rather than rejected.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 500); capped at 1000 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = DefaultAllItemsPageSize, CancellationToken cancellationToken = default) { if (string.IsNullOrWhiteSpace(query)) diff --git a/ErsatzTV/Controllers/Api/TraktController.cs b/ErsatzTV/Controllers/Api/TraktController.cs index 838004695..c597ab263 100644 --- a/ErsatzTV/Controllers/Api/TraktController.cs +++ b/ErsatzTV/Controllers/Api/TraktController.cs @@ -1,3 +1,4 @@ +using System.ComponentModel; using System.ComponentModel.DataAnnotations; using System.Text.RegularExpressions; using System.Threading.Channels; @@ -28,8 +29,12 @@ public partial class TraktController( [EndpointGroupName("general")] [ProducesResponseType(typeof(PagedTraktListsResponseModel), StatusCodes.Status200OK)] public async Task GetAll( - [FromQuery] int pageNum = 0, - [FromQuery] int pageSize = 100, + [FromQuery] + [Description("0-based page index: the first page is 0, not 1. A negative value is clamped to 0.")] + int pageNum = 0, + [FromQuery] + [Description("Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.")] + int pageSize = 100, CancellationToken cancellationToken = default) { int clampedPageNum = Math.Max(0, pageNum); diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 5d384d488..383fd5bb5 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -2578,6 +2578,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32" @@ -2586,6 +2587,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page; capped at 200 for this endpoint. A value of 0 or less falls back to 100 rather than being clamped to 1. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32" @@ -3863,6 +3865,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -3872,6 +3875,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -9385,6 +9389,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -9394,6 +9399,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -10154,6 +10160,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -10163,6 +10170,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -10760,6 +10768,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -10769,6 +10778,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -12479,6 +12489,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -12488,6 +12499,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -13075,6 +13087,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -13084,6 +13097,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -14040,6 +14054,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -14049,6 +14064,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -15533,6 +15549,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -15542,6 +15559,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -17351,6 +17369,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -17360,6 +17379,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 50); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -17454,6 +17474,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0. Unlike the other paged endpoints this one is also bounded ABOVE, at 2000000, so that pageNum * pageSize cannot overflow; a larger value is clamped down to that maximum rather than rejected.", "schema": { "type": "integer", "format": "int32", @@ -17463,6 +17484,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 500); capped at 1000 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", @@ -21042,6 +21064,7 @@ { "name": "pageNum", "in": "query", + "description": "0-based page index: the first page is 0, not 1. A negative value is clamped to 0.", "schema": { "type": "integer", "format": "int32", @@ -21051,6 +21074,7 @@ { "name": "pageSize", "in": "query", + "description": "Rows per page (default 100); capped at 100 for this endpoint. The page offset is derived from the effective (capped) size, so a larger value narrows the page instead of widening the offset.", "schema": { "type": "integer", "format": "int32", diff --git a/docs/api-conventions.md b/docs/api-conventions.md index db3546bd5..260686024 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -46,7 +46,14 @@ Exemplars: narrower pages — it never widens the offset. Say "0-based" in the description of any paging parameter you expose, including on wrapper surfaces like the MCP tool catalog: describing it as 1-based makes a caller skip the first page silently, which reads as data loss rather than as an - off-by-one (ersatztv#616). See `api.paging-zero-based`. + off-by-one (ersatztv#616). Put that description on the parameter itself with + `[Description("...")]` (`System.ComponentModel`, on the `[FromQuery]` parameter) so it reaches the + generated OpenAPI document — an attribute-free paging parameter is emitted with no description at + all, leaving a REST consumer to infer the base from `default: 0` (ersatztv#633). State the + endpoint's **own** cap, never one global number: the caps differ (100 typical, 200 auto-tune + 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`. - **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/records/api/paging-zero-based.md b/docs/decisions/records/api/paging-zero-based.md index 4b5d8f930..9c1ec058c 100644 --- a/docs/decisions/records/api/paging-zero-based.md +++ b/docs/decisions/records/api/paging-zero-based.md @@ -1,6 +1,6 @@ --- key: api.paging-zero-based -title: 2026-07-25 — Paging is 0-based everywhere; every wrapper must say so (OpenAPI still doesn't) (#616) +title: 2026-07-25 — Paging is 0-based everywhere; every wrapper must say so (#616, #633) status: active since: '2026-07-25' supersedes: none @@ -42,11 +42,22 @@ dozen controllers and the SPA, and a 1-based wrapper over a 0-based API would ma parameter name* mean different things on two surfaces a reader routinely reads together — trading a documented off-by-one for an undocumented one. Accuracy in the description is the cheaper contract. -**Where "0-based" is stated, and where it still isn't.** The MCP tool catalog and these docs say it -explicitly. The generated OpenAPI `pageNum` parameters carry **no description at all** (12 of them), -so a REST consumer reading only `v1.json` still has to infer the base from the default — a real -remaining gap, tracked separately rather than fixed here. Treat "every wrapper says 0-based" as the -target this record sets, not a property already true of the OpenAPI surface. +**Where "0-based" is stated.** The MCP tool catalog, these docs, and — since #633 — the generated +OpenAPI document all say it explicitly. All 24 paging parameters across the 12 paged operations carry +a `[Description]` (`System.ComponentModel`, on the `[FromQuery]` parameter, the same mechanism +`parentId` already used), so a REST consumer reading only `v1.json` no longer has to infer the base +from `default: 0` — which is the inference that cost #487 a verification pass on the MCP side, where +the description was present but wrong. `pageSize` descriptions state the endpoint's own cap and that +the offset derives from the effective size, never a single global number. + +That sweep is pinned by `OpenApiPagingContractTests` against the in-process generated document. The +test names the expected set of 12 operations rather than only filtering for parameters called +`pageNum`: a filter cannot see an endpoint that *should* page and doesn't, so set-equality is +asserted in both directions — a new paged endpoint fails until it is added with descriptions, and an +endpoint that quietly drops paging fails too. Both directions are mutation-verified. The residual +gap this cannot close is a brand-new endpoint that returns a page while declaring no paging +parameters at all under any name; nothing in the document distinguishes that from an unpaged +endpoint, so it stays a review concern. **Corollary — ids in paged rows.** A row that names a related entity should expose that entity's id, not only its display fields, wherever a caller is expected to act on that entity. This is a rule about