fix(person): report honest totals on the non-paged top-N persons path
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 <noreply@anthropic.com>
This commit is contained in:
@@ -51,12 +51,14 @@ public class PersonController {
|
|||||||
@RequestParam(required = false) String sort,
|
@RequestParam(required = false) String sort,
|
||||||
@RequestParam(defaultValue = "0") @Min(0) int page,
|
@RequestParam(defaultValue = "0") @Min(0) int page,
|
||||||
@RequestParam(defaultValue = "50") @Min(1) @Max(100) int size) {
|
@RequestParam(defaultValue = "50") @Min(1) @Max(100) int size) {
|
||||||
// Legacy top-N-by-document-count path (reader dashboard): preserved, now wrapped in the
|
// Legacy top-N-by-document-count path (reader dashboard): preserved, wrapped in the
|
||||||
// paged contract so /api/persons always returns one shape.
|
// 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) {
|
if ("documentCount".equals(sort) && q == null) {
|
||||||
int safeSize = Math.min(size, 50);
|
int safeSize = Math.min(size, 50);
|
||||||
List<PersonSummaryDTO> top = personService.findTopByDocumentCount(safeSize);
|
List<PersonSummaryDTO> top = personService.findTopByDocumentCount(safeSize);
|
||||||
return ResponseEntity.ok(PersonSearchResult.paged(top, 0, safeSize, top.size()));
|
return ResponseEntity.ok(PersonSearchResult.topN(top));
|
||||||
}
|
}
|
||||||
|
|
||||||
PersonFilter filter = PersonFilter.builder()
|
PersonFilter filter = PersonFilter.builder()
|
||||||
|
|||||||
@@ -33,4 +33,18 @@ public record PersonSearchResult(
|
|||||||
int totalPages = pageSize == 0 ? 0 : (int) ((totalElements + pageSize - 1) / pageSize);
|
int totalPages = pageSize == 0 ? 0 : (int) ((totalElements + pageSize - 1) / pageSize);
|
||||||
return new PersonSearchResult(slice, totalElements, pageNumber, pageSize, totalPages);
|
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 <em>complete</em> 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<PersonSummaryDTO> all) {
|
||||||
|
int count = all.size();
|
||||||
|
int totalPages = count == 0 ? 0 : 1;
|
||||||
|
return new PersonSearchResult(all, count, 0, count, totalPages);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -175,6 +175,36 @@ class PersonControllerTest {
|
|||||||
.andExpect(jsonPath("$.items[0].firstName").value("Käthe"));
|
.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) {
|
private PersonSummaryDTO mockPersonSummary(String firstName, String lastName) {
|
||||||
return new PersonSummaryDTO() {
|
return new PersonSummaryDTO() {
|
||||||
public java.util.UUID getId() { return UUID.randomUUID(); }
|
public java.util.UUID getId() { return UUID.randomUUID(); }
|
||||||
|
|||||||
Reference in New Issue
Block a user