Skip to content

fix(api): a request that names a page or a size gets one - #1156

Merged
ExtraToast merged 1 commit into
mainfrom
fix/a-size-is-a-size
Sep 5, 2026
Merged

fix(api): a request that names a page or a size gets one#1156
ExtraToast merged 1 commit into
mainfrom
fix/a-size-is-a-size

Conversation

@ExtraToast

@ExtraToast ExtraToast commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Closes #1145.

Why

GET /users?size=500 answered with every user. Not 500 — every one.

Spring's PageableHandlerMethodArgumentResolver reads its fallback unless the request carries both page and size, and PagingConfig sets that fallback to unpaged. So naming one half of a page bought nothing, silently: the response is well formed and merely larger than the caller believed possible. A caller that names a size is one that cannot hold an unbounded answer.

Measured against four users, before this change:

request rows page.size
GET /users 4 4
GET /users?size=1 4 4
GET /users?page=0 4 4
GET /users?page=0&size=1 1 1

page alone was ignored too, which the issue did not know.

What this achieves

Either half means paged. ?size=1 is one row; ?page=0 is the documented default of 20. Naming neither still answers unpaged, because a listing that nobody paged is what several readers want and what unpagedByDefault is for.

How

PagingConfig now registers a resolver ahead of Spring Data's, overriding the one decision that was wrong: with neither parameter present it answers unpaged as before, and with either present it fills in 0 or 20 for the half that was not named and pages as asked.

20 is not invented — it is what the OpenAPI document already claims size defaults to, so the spec and the api now agree where before the spec described a parameter that was not honoured.

The callers, which had to move with it

Both remaining callers that named a size wanted the whole listing and were getting it by accident. Left alone, this change would have truncated them at 500 — which is exactly the bug #1139 fixed a day ago, re-created from the other side.

loadMemberAccounts and UserPicker both hold the list and filter it in the browser, so they now ask for the listing and name no size. Their behaviour on the wire is unchanged; what changed is that they say what they mean.

Worth a reviewer's attention

That both of them fetch an unbounded listing at all is a real problem — the committee picker stopped doing it in #1139 by asking the api as the reader types. These two are rarer paths (attaching a roster entry, a board membership), which is the reasoning recorded in users.ts, but the same argument that beat 500 will eventually beat "everybody". Out of scope here: this change is about the api keeping the promise its parameters make.

Verification

./gradlew :services:api:test :services:api:integrationTest — both suites pass in full.

Three new UserControllerIT cases pin all the shapes, and two of them were red first, for the right reasons: expected:<1> but was:<4> for a size without a page, and page.size expected:<20> but was:<4> for a page without a size.

yarn vitest run — 179 files, 1540 tests. One existing test asserted {query: {size: 500}} and now states the intent instead: it asks for the whole listing rather than a page whose size it would have to guess. yarn typecheck and yarn lint clean, and the generated spec does not drift.


Diff breakdown added removed, scaled to the largest row.

api                                                +65    -10    2
  production         ████████████████████░░░░░░    +33    -10    1
  integration tests  ███████████████████           +32     -0    1

frontend                                            +8     -4    3
  production         ████░                          +6     -2    2
  unit tests         █░                             +2     -2    1

──────────────────────────────────────────────────────────────────
production                                         +39    -12
tests                                              +34     -2  0.87 test lines per prod line
total (hand-written)                               +73    -14  5 files

GET /users?size=500 answered with every user. Spring's resolver reads its
fallback unless the request carries both `page` and `size`, and this api's
fallback is unpaged, so naming one half of a page silently bought nothing. A
caller that names a size is one that cannot hold an unbounded answer, and it was
handed exactly that.

Measured on four users, before: no parameters, `?size=1` and `?page=0` all
answered with four rows; only `?page=0&size=1` paged. After: the two halves each
page, with 0 and the documented 20 filling in for whichever was not named, and a
request naming neither still answers unpaged, which is what the listing is for.

The two callers that named a size wanted the whole listing and were getting it
by accident: loadMemberAccounts and UserPicker both filter what they hold, so
they now ask for the listing and name no size. Left as they were, this change
would have truncated them at 500 — the bug #1139 has just been fixed.

Closes #1145
@ExtraToast ExtraToast added the bug Something isn't working label Sep 5, 2026
@ExtraToast ExtraToast self-assigned this Sep 5, 2026
@ExtraToast
ExtraToast merged commit 38c2a4b into main Sep 5, 2026
25 checks passed
@ExtraToast
ExtraToast deleted the fix/a-size-is-a-size branch September 5, 2026 22:10
@github-actions github-actions Bot mentioned this pull request Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A size without a page is ignored, so a bounded request answers with everything

1 participant