From 1e3e4208609ac31036bd488128f65bb0198b641d Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 27 May 2026 14:19:00 +0200 Subject: [PATCH] fix(person): report honest totals on the non-paged top-N persons path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy sort=documentCount path wrapped its result with paged(top, 0, safeSize, top.size()), so totalElements/pageSize looked like a paged slice of a larger set when in fact the top-N query returns the complete result. Add a dedicated PersonSearchResult.topN factory that reports reality — totalElements = returned count, pageSize = that count, totalPages = 1 (0 when empty) — and pin both the populated and empty semantics with controller tests. Refs #667 Co-Authored-By: Claude Opus 4.7 --- .../person/PersonController.java | 8 +++-- .../person/PersonSearchResult.java | 14 +++++++++ .../person/PersonControllerTest.java | 30 +++++++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonController.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonController.java index 81c35825..dad52b5a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonController.java @@ -51,12 +51,14 @@ public class PersonController { @RequestParam(required = false) String sort, @RequestParam(defaultValue = "0") @Min(0) int page, @RequestParam(defaultValue = "50") @Min(1) @Max(100) int size) { - // Legacy top-N-by-document-count path (reader dashboard): preserved, now wrapped in the - // paged contract so /api/persons always returns one shape. + // Legacy top-N-by-document-count path (reader dashboard): preserved, wrapped in the + // same envelope so /api/persons always returns one shape. It is explicitly NON-paged — + // the top-N query returns the complete result, so PersonSearchResult.topN reports an + // honest totalElements (= returned count) instead of pretending to be a page slice. if ("documentCount".equals(sort) && q == null) { int safeSize = Math.min(size, 50); List top = personService.findTopByDocumentCount(safeSize); - return ResponseEntity.ok(PersonSearchResult.paged(top, 0, safeSize, top.size())); + return ResponseEntity.ok(PersonSearchResult.topN(top)); } PersonFilter filter = PersonFilter.builder() diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonSearchResult.java b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonSearchResult.java index 7f8c8e93..ff605770 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/PersonSearchResult.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/PersonSearchResult.java @@ -33,4 +33,18 @@ public record PersonSearchResult( int totalPages = pageSize == 0 ? 0 : (int) ((totalElements + pageSize - 1) / pageSize); return new PersonSearchResult(slice, totalElements, pageNumber, pageSize, totalPages); } + + /** + * Non-paged factory for the legacy {@code sort=documentCount} top-N dashboard path. + * That query returns the complete result in one shot — there is no further page + * to fetch — so the envelope reports reality rather than pretending to be a slice of a + * larger set: {@code totalElements} equals the number of rows actually returned, + * {@code pageSize} equals that same count, and {@code totalPages} is 1 (or 0 when empty). + * This avoids the earlier ambiguity where {@code totalElements} looked like a paged total. + */ + public static PersonSearchResult topN(List all) { + int count = all.size(); + int totalPages = count == 0 ? 0 : 1; + return new PersonSearchResult(all, count, 0, count, totalPages); + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonControllerTest.java index f421f86e..d43e9a9a 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/person/PersonControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/person/PersonControllerTest.java @@ -175,6 +175,36 @@ class PersonControllerTest { .andExpect(jsonPath("$.items[0].firstName").value("Käthe")); } + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_topByDocumentCount_isNonPaged_totalElementsEqualsReturnedCount() throws Exception { + // The top-N dashboard path is deliberately NON-paged: it returns the complete result + // (no further page exists), so totalElements equals the number of rows returned and + // totalPages is 1. Pinned so nobody "fixes" it into a misleading paged total. + when(personService.findTopByDocumentCount(50)) + .thenReturn(List.of(mockPersonSummary("Käthe", "Raddatz"), + mockPersonSummary("Hans", "Müller"))); + + mockMvc.perform(get("/api/persons").param("sort", "documentCount")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.items.length()").value(2)) + .andExpect(jsonPath("$.totalElements").value(2)) + .andExpect(jsonPath("$.pageNumber").value(0)) + .andExpect(jsonPath("$.pageSize").value(2)) + .andExpect(jsonPath("$.totalPages").value(1)); + } + + @Test + @WithMockUser(authorities = "READ_ALL") + void getPersons_topByDocumentCount_emptyResult_reportsZeroPages() throws Exception { + when(personService.findTopByDocumentCount(50)).thenReturn(Collections.emptyList()); + + mockMvc.perform(get("/api/persons").param("sort", "documentCount")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.totalElements").value(0)) + .andExpect(jsonPath("$.totalPages").value(0)); + } + private PersonSummaryDTO mockPersonSummary(String firstName, String lastName) { return new PersonSummaryDTO() { public java.util.UUID getId() { return UUID.randomUUID(); }