From 1a70a3db6d5c0d6acd612312bc6af1cffa6185b9 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 08:05:05 +0200 Subject: [PATCH 1/8] feat(search-result): extend DocumentSearchResult with pageNumber/pageSize/totalPages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename `total` → `totalElements` for Spring-Page parity and add three new required paging fields: pageNumber, pageSize, totalPages. Adds a `paged( slice, pageable, totalElements)` factory alongside the existing single-page `of(list)` shortcut. Enables offset pagination of /documents search (#315). Co-Authored-By: Claude Opus 4.7 --- .../dto/DocumentSearchResult.java | 26 ++++++++- .../dto/DocumentSearchResultTest.java | 53 +++++++++++++++++-- 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/dto/DocumentSearchResult.java b/backend/src/main/java/org/raddatz/familienarchiv/dto/DocumentSearchResult.java index 764d9c12..9374e6c6 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/dto/DocumentSearchResult.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/dto/DocumentSearchResult.java @@ -1,6 +1,7 @@ package org.raddatz.familienarchiv.dto; import io.swagger.v3.oas.annotations.media.Schema; +import org.springframework.data.domain.Pageable; import java.util.List; @@ -8,9 +9,30 @@ public record DocumentSearchResult( @Schema(requiredMode = Schema.RequiredMode.REQUIRED) List items, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) - long total + long totalElements, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + int pageNumber, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + int pageSize, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + int totalPages ) { + /** + * Single-page convenience factory used by empty-result shortcuts and by tests that + * don't care about paging. Treats the whole list as page 0 of itself. + */ public static DocumentSearchResult of(List items) { - return new DocumentSearchResult(items, items.size()); + int size = items.size(); + return new DocumentSearchResult(items, size, 0, size, size == 0 ? 0 : 1); + } + + /** + * Paged factory used by the service when it has a real Pageable + full match count + * (e.g. from Spring's Page or from an in-memory sort-then-slice). + */ + public static DocumentSearchResult paged(List slice, Pageable pageable, long totalElements) { + int pageSize = pageable.getPageSize(); + int totalPages = pageSize == 0 ? 0 : (int) ((totalElements + pageSize - 1) / pageSize); + return new DocumentSearchResult(slice, totalElements, pageable.getPageNumber(), pageSize, totalPages); } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/dto/DocumentSearchResultTest.java b/backend/src/test/java/org/raddatz/familienarchiv/dto/DocumentSearchResultTest.java index 3673459d..c860f9b6 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/dto/DocumentSearchResultTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/dto/DocumentSearchResultTest.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.audit.ActivityActorDTO; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; +import org.springframework.data.domain.PageRequest; import java.util.List; import java.util.UUID; @@ -24,10 +25,43 @@ class DocumentSearchResultTest { } @Test - void of_total_equals_list_size() { - DocumentSearchResult result = DocumentSearchResult.of(List.of(item(UUID.randomUUID()), item(UUID.randomUUID()))); + void of_totalElements_equals_list_size_for_unpaged_shortcut() { + DocumentSearchResult result = DocumentSearchResult.of( + List.of(item(UUID.randomUUID()), item(UUID.randomUUID()))); - assertThat(result.total()).isEqualTo(2L); + assertThat(result.totalElements()).isEqualTo(2L); + assertThat(result.pageNumber()).isZero(); + assertThat(result.pageSize()).isEqualTo(2); + assertThat(result.totalPages()).isEqualTo(1); + } + + @Test + void of_empty_shortcut_has_zero_totalPages() { + DocumentSearchResult result = DocumentSearchResult.of(List.of()); + + assertThat(result.totalElements()).isZero(); + assertThat(result.totalPages()).isZero(); + } + + @Test + void paged_factory_populates_paging_fields_from_pageable_and_total() { + List slice = List.of(item(UUID.randomUUID()), item(UUID.randomUUID())); + + DocumentSearchResult result = DocumentSearchResult.paged(slice, PageRequest.of(1, 50), 120L); + + assertThat(result.items()).hasSize(2); + assertThat(result.totalElements()).isEqualTo(120L); + assertThat(result.pageNumber()).isEqualTo(1); + assertThat(result.pageSize()).isEqualTo(50); + assertThat(result.totalPages()).isEqualTo(3); // ceil(120 / 50) + } + + @Test + void paged_factory_totalPages_rounds_up_on_remainder() { + DocumentSearchResult result = + DocumentSearchResult.paged(List.of(), PageRequest.of(0, 7), 30L); + + assertThat(result.totalPages()).isEqualTo(5); // ceil(30 / 7) } @Test @@ -53,9 +87,18 @@ class DocumentSearchResultTest { } @Test - void total_component_is_annotated_as_required_in_openapi_schema() throws NoSuchFieldException { - Schema schema = DocumentSearchResult.class.getDeclaredField("total").getAnnotation(Schema.class); + void totalElements_component_is_annotated_as_required_in_openapi_schema() throws NoSuchFieldException { + Schema schema = DocumentSearchResult.class.getDeclaredField("totalElements").getAnnotation(Schema.class); assertThat(schema).isNotNull(); assertThat(schema.requiredMode()).isEqualTo(Schema.RequiredMode.REQUIRED); } + + @Test + void paging_components_are_annotated_as_required_in_openapi_schema() throws NoSuchFieldException { + for (String name : List.of("pageNumber", "pageSize", "totalPages")) { + Schema schema = DocumentSearchResult.class.getDeclaredField(name).getAnnotation(Schema.class); + assertThat(schema).as(name + " must have @Schema").isNotNull(); + assertThat(schema.requiredMode()).isEqualTo(Schema.RequiredMode.REQUIRED); + } + } } -- 2.49.1 From 782f845dc97bd18e65f34410ba0a5d3b59d03bfc Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 08:23:15 +0200 Subject: [PATCH 2/8] feat(search): DocumentService.searchDocuments takes Pageable and slices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fast path (DATE/TITLE/UPLOAD_DATE) pushes sort + paging into the DB via findAll(Specification, PageRequest) and enriches only the returned slice — 30× cheaper than enriching all 1500 matches when the user is only going to see 50. In-memory sort paths (SENDER/RECEIVER/RELEVANCE) keep their LEFT JOIN-friendly sort but now slice in-memory too, so enrichment still runs against the page slice only. Controller passes PageRequest.of(page, size) built from @RequestParam values. Plan-level "add @Validated" prerequisite comes in the next commit. All existing tests updated mechanically to pass a pageable argument (PageRequest.of(0, 10_000) as an "effectively unpaged" sentinel). Stubs that previously matched findAll(Specification, Sort) for the fast path now match findAll(Specification, Pageable) with PageImpl<>. Co-Authored-By: Claude Opus 4.7 --- .../controller/DocumentController.java | 16 ++++++- .../service/DocumentService.java | 44 ++++++++++++------- .../controller/DocumentControllerTest.java | 12 ++--- .../service/DocumentServiceSortTest.java | 15 ++++--- .../service/DocumentServiceTest.java | 33 +++++++------- 5 files changed, 74 insertions(+), 46 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java index 1abee3ad..8b17a0ef 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/DocumentController.java @@ -13,6 +13,11 @@ import java.util.UUID; import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.responses.ApiResponse; +import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.Min; +import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; +import org.springframework.validation.annotation.Validated; import org.raddatz.familienarchiv.dto.DocumentSearchResult; import org.raddatz.familienarchiv.dto.DocumentUpdateDTO; import org.raddatz.familienarchiv.dto.TagOperator; @@ -62,6 +67,7 @@ import lombok.extern.slf4j.Slf4j; @RequestMapping("/api/documents") @RequiredArgsConstructor @Slf4j +@Validated public class DocumentController { private final DocumentService documentService; @@ -252,14 +258,20 @@ public class DocumentController { @Parameter(description = "Filter by document status") @RequestParam(required = false) DocumentStatus status, @Parameter(description = "Sort field") @RequestParam(required = false) DocumentSort sort, @Parameter(description = "Sort direction: ASC or DESC") @RequestParam(required = false, defaultValue = "DESC") String dir, - @Parameter(description = "Tag operator: AND (default) or OR") @RequestParam(required = false) String tagOp) { + @Parameter(description = "Tag operator: AND (default) or OR") @RequestParam(required = false) String tagOp, + // @Max on page guards against overflow when pageable.getOffset() is computed + // as page * size — Integer.MAX_VALUE * 50 would wrap to a negative long, which + // Hibernate cheerfully turns into an invalid SQL OFFSET. + @Parameter(description = "Page number (0-indexed)") @RequestParam(defaultValue = "0") @Min(0) @Max(100_000) int page, + @Parameter(description = "Page size (max 100)") @RequestParam(defaultValue = "50") @Min(1) @Max(100) int size) { if (!"ASC".equalsIgnoreCase(dir) && !"DESC".equalsIgnoreCase(dir)) { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "dir must be ASC or DESC"); } // tagOp is a raw String at the HTTP boundary; any value other than "OR" (case-insensitive) // defaults to AND, which matches the frontend default and keeps old clients working. TagOperator operator = "OR".equalsIgnoreCase(tagOp) ? TagOperator.OR : TagOperator.AND; - return ResponseEntity.ok(documentService.searchDocuments(q, from, to, senderId, receiverId, tags, tagQ, status, sort, dir, operator)); + Pageable pageable = PageRequest.of(page, size); + return ResponseEntity.ok(documentService.searchDocuments(q, from, to, senderId, receiverId, tags, tagQ, status, sort, dir, operator, pageable)); } // --- TRAINING LABELS --- diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index 5b563e47..fbdec954 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -22,7 +22,9 @@ import org.raddatz.familienarchiv.model.TrainingLabel; import org.raddatz.familienarchiv.model.Person; import org.raddatz.familienarchiv.model.Tag; import org.raddatz.familienarchiv.repository.DocumentRepository; +import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; +import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.Specification; import org.raddatz.familienarchiv.exception.DomainException; @@ -355,7 +357,7 @@ public class DocumentService { } // 1. Allgemeine Suche (für das Suchfeld im Frontend) - public DocumentSearchResult searchDocuments(String text, LocalDate from, LocalDate to, UUID sender, UUID receiver, List tags, String tagQ, DocumentStatus status, DocumentSort sort, String dir, TagOperator tagOperator) { + public DocumentSearchResult searchDocuments(String text, LocalDate from, LocalDate to, UUID sender, UUID receiver, List tags, String tagQ, DocumentStatus status, DocumentSort sort, String dir, TagOperator tagOperator, Pageable pageable) { boolean hasText = StringUtils.hasText(text); List rankedIds = null; @@ -376,15 +378,18 @@ public class DocumentService { .and(hasTagPartial(tagQ)) .and(hasStatus(status)); - // SENDER and RECEIVER are sorted in-memory because JPA's Sort.by("sender.lastName") - // generates an INNER JOIN that silently drops documents with null sender/receivers. + // SENDER, RECEIVER and RELEVANCE sorts load the full match set and slice in memory. + // JPA's Sort.by("sender.lastName") generates an INNER JOIN that silently drops + // documents with null sender/receivers; RELEVANCE maps a DB order to an external + // rank list. Cost scales linearly with match count — acceptable while documents + // stays under ~10k rows. Past that, replace with SQL-level LEFT JOIN sort. if (sort == DocumentSort.RECEIVER) { - List results = documentRepository.findAll(spec); - return buildResult(sortByFirstReceiver(results, dir), text); + List sorted = sortByFirstReceiver(documentRepository.findAll(spec), dir); + return buildResultPaged(pageSlice(sorted, pageable), text, pageable, sorted.size()); } if (sort == DocumentSort.SENDER) { - List results = documentRepository.findAll(spec); - return buildResult(sortBySender(results, dir), text); + List sorted = sortBySender(documentRepository.findAll(spec), dir); + return buildResultPaged(pageSlice(sorted, pageable), text, pageable, sorted.size()); } // RELEVANCE: default when text present and no explicit sort given @@ -397,15 +402,26 @@ public class DocumentService { .sorted(Comparator.comparingInt( doc -> rankMap.getOrDefault(doc.getId(), Integer.MAX_VALUE))) .toList(); - return buildResult(sorted, text); + return buildResultPaged(pageSlice(sorted, pageable), text, pageable, sorted.size()); } - Sort springSort = resolveSort(sort, dir); - List results = documentRepository.findAll(spec, springSort); - return buildResult(results, text); + // Fast path — push sort + paging into the DB and enrich only the returned slice. + PageRequest pageRequest = PageRequest.of(pageable.getPageNumber(), pageable.getPageSize(), resolveSort(sort, dir)); + Page page = documentRepository.findAll(spec, pageRequest); + return buildResultPaged(page.getContent(), text, pageable, page.getTotalElements()); } - private DocumentSearchResult buildResult(List documents, String text) { + private static List pageSlice(List sorted, Pageable pageable) { + int from = Math.min((int) pageable.getOffset(), sorted.size()); + int to = Math.min(from + pageable.getPageSize(), sorted.size()); + return sorted.subList(from, to); + } + + private DocumentSearchResult buildResultPaged(List slice, String text, Pageable pageable, long totalElements) { + return DocumentSearchResult.paged(enrichItems(slice, text), pageable, totalElements); + } + + private List enrichItems(List documents, String text) { List colorResolved = resolveDocumentTagColors(documents); Map matchData = enrichWithMatchData(colorResolved, text); @@ -413,14 +429,12 @@ public class DocumentService { Map completionByDoc = fetchCompletionPercentages(docIds); Map> contributorsByDoc = auditLogQueryService.findRecentContributorsPerDocument(docIds); - List items = colorResolved.stream().map(doc -> new DocumentSearchItem( + return colorResolved.stream().map(doc -> new DocumentSearchItem( doc, matchData.getOrDefault(doc.getId(), SearchMatchData.empty()), completionByDoc.getOrDefault(doc.getId(), 0), contributorsByDoc.getOrDefault(doc.getId(), List.of()) )).toList(); - - return DocumentSearchResult.of(items); } private Map fetchCompletionPercentages(List docIds) { diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java index e1282989..c8383c34 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java @@ -69,7 +69,7 @@ class DocumentControllerTest { @Test @WithMockUser void search_returns200_whenAuthenticated() throws Exception { - when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(DocumentSearchResult.of(List.of())); mockMvc.perform(get("/api/documents/search")) @@ -79,13 +79,13 @@ class DocumentControllerTest { @Test @WithMockUser void search_withStatusParam_passesItToService() throws Exception { - when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), eq(DocumentStatus.REVIEWED), any(), any(), any())) + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), eq(DocumentStatus.REVIEWED), any(), any(), any(), any())) .thenReturn(DocumentSearchResult.of(List.of())); mockMvc.perform(get("/api/documents/search").param("status", "REVIEWED")) .andExpect(status().isOk()); - verify(documentService).searchDocuments(any(), any(), any(), any(), any(), any(), any(), eq(DocumentStatus.REVIEWED), any(), any(), any()); + verify(documentService).searchDocuments(any(), any(), any(), any(), any(), any(), any(), eq(DocumentStatus.REVIEWED), any(), any(), any(), any()); } @Test @@ -112,12 +112,12 @@ class DocumentControllerTest { @Test @WithMockUser void search_responseContainsTotalCount() throws Exception { - when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(DocumentSearchResult.of(List.of())); mockMvc.perform(get("/api/documents/search")) .andExpect(status().isOk()) - .andExpect(jsonPath("$.total").value(0)) + .andExpect(jsonPath("$.totalElements").value(0)) .andExpect(jsonPath("$.items").isArray()); } @@ -133,7 +133,7 @@ class DocumentControllerTest { .build(); var matchData = new SearchMatchData( "Er schrieb einen langen Brief", List.of(), false, List.of(), List.of(), List.of(), null, List.of()); - when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(DocumentSearchResult.of(List.of(new DocumentSearchItem(doc, matchData, 0, List.of())))); mockMvc.perform(get("/api/documents/search").param("q", "Brief")) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java index b3905d0e..3bdbbc2c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java @@ -11,7 +11,8 @@ import org.raddatz.familienarchiv.dto.DocumentSort; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.repository.DocumentRepository; -import org.springframework.data.domain.Sort; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.domain.Specification; import java.time.LocalDate; @@ -51,12 +52,12 @@ class DocumentServiceSortTest { // FTS returns id1 first (higher rank), id2 second when(documentRepository.findRankedIdsByFts("Brief")).thenReturn(List.of(id1, id2)); - // findAll(spec, sort) — the correct date path — returns date-DESC order - when(documentRepository.findAll(any(Specification.class), any(Sort.class))) - .thenReturn(List.of(newer, older)); + // findAll(spec, pageable) — the correct date path — returns date-DESC order + when(documentRepository.findAll(any(Specification.class), any(Pageable.class))) + .thenReturn(new PageImpl<>(List.of(newer, older))); DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.DATE, "DESC", null); + "Brief", null, null, null, null, null, null, null, DocumentSort.DATE, "DESC", null, org.springframework.data.domain.PageRequest.of(0, 10_000)); // Expect: date order (newer 1960 first), NOT rank order (older 1940 first) assertThat(result.items()).hasSize(2); @@ -78,7 +79,7 @@ class DocumentServiceSortTest { .thenReturn(List.of(doc2, doc1)); // unordered from DB DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); // Expect: rank order restored (id1 first) assertThat(result.items().get(0).document().getId()).isEqualTo(id1); @@ -97,7 +98,7 @@ class DocumentServiceSortTest { .thenReturn(List.of(doc2, doc1)); DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, null, null, null); + "Brief", null, null, null, null, null, null, null, null, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); assertThat(result.items().get(0).document().getId()).isEqualTo(id1); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java index 657c822d..41c0479b 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -1327,22 +1327,22 @@ class DocumentServiceTest { @Test void searchDocuments_passesStatusSpecificationToRepository() { - when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Sort.class))) - .thenReturn(List.of()); + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class))) + .thenReturn(new PageImpl<>(List.of())); - documentService.searchDocuments(null, null, null, null, null, null, null, DocumentStatus.REVIEWED, null, null, null); + documentService.searchDocuments(null, null, null, null, null, null, null, DocumentStatus.REVIEWED, null, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); - verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Sort.class)); + verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class)); } @Test void searchDocuments_withNullStatus_doesNotFilterByStatus() { - when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Sort.class))) - .thenReturn(List.of()); + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class))) + .thenReturn(new PageImpl<>(List.of())); - documentService.searchDocuments(null, null, null, null, null, null, null, null, null, null, null); + documentService.searchDocuments(null, null, null, null, null, null, null, null, null, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); - verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Sort.class)); + verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class)); } // ─── getRecentActivity ──────────────────────────────────────────────────── @@ -1418,7 +1418,7 @@ class DocumentServiceTest { .thenReturn(List.of(withSender, noSender)); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null); + null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, org.springframework.data.domain.PageRequest.of(0, 10_000)); assertThat(result.items()).hasSize(2); assertThat(result.items()).extracting(item -> item.document().getTitle()).containsExactly("Has Sender", "No Sender"); @@ -1438,7 +1438,7 @@ class DocumentServiceTest { .thenReturn(List.of(noReceivers, withReceiver)); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, DocumentSort.RECEIVER, "asc", null); + null, null, null, null, null, null, null, null, DocumentSort.RECEIVER, "asc", null, org.springframework.data.domain.PageRequest.of(0, 10_000)); assertThat(result.items()).extracting(item -> item.document().getTitle()) .containsExactly("Has Receiver", "No Receivers"); @@ -1460,7 +1460,7 @@ class DocumentServiceTest { .thenReturn(List.of(docNullName, docSmith)); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null); + null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, org.springframework.data.domain.PageRequest.of(0, 10_000)); // null lastName should sort to end (treated as empty), not before "smith" (as "null") assertThat(result.items()).extracting(item -> item.document().getTitle()) @@ -1482,7 +1482,7 @@ class DocumentServiceTest { when(documentRepository.findEnrichmentData(any(), eq("Brief"))).thenReturn(rows); DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); assertThat(result.items()).hasSize(1); SearchMatchData md = result.items().get(0).matchData(); @@ -1492,11 +1492,12 @@ class DocumentServiceTest { @Test void searchDocuments_withoutTextQuery_returnsEmptyMatchData() { - when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Sort.class))) - .thenReturn(List.of()); + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class))) + .thenReturn(new PageImpl<>(List.of())); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, null, null, null); + null, null, null, null, null, null, null, null, null, null, null, + org.springframework.data.domain.PageRequest.of(0, 10_000)); assertThat(result.items()).isEmpty(); } @@ -1515,7 +1516,7 @@ class DocumentServiceTest { when(documentRepository.findEnrichmentData(any(), eq("Brief"))).thenReturn(rows); DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); SearchMatchData md = result.items().get(0).matchData(); assertThat(md.transcriptionSnippet()).isEqualTo("Hier ist der Brief aus Berlin"); -- 2.49.1 From 67ae494d18c7e96a4d9ce133ea5423e255b1a113 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 08:26:41 +0200 Subject: [PATCH 3/8] test(search): lock pagination behaviour and @Validated rejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 5 dedicated controller cases — paging fields exposed on the JSON, rejections for size>100 / size<1 / page<0 / page>100000, and a captor assertion that the built PageRequest is forwarded to the service. The size>100 case is the load-bearing guard on @Validated at DocumentController — removing the annotation silently reopens the DoS window this PR is meant to close. Adds 5 service cases — fast path uses findAll(Spec, Pageable) (not Sort), propagates page+size to the DB, carries totalElements/totalPages/ pageNumber/pageSize back on the result, and for SENDER sort slices in memory and reports the pre-slice total. Page-beyond-last returns empty content with a correct totalElements (JPA edge case). (#315) Co-Authored-By: Claude Opus 4.7 --- .../controller/DocumentControllerTest.java | 64 ++++++++++++ .../service/DocumentServiceTest.java | 98 +++++++++++++++++++ 2 files changed, 162 insertions(+) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java index c8383c34..eb4c6873 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/DocumentControllerTest.java @@ -143,6 +143,70 @@ class DocumentControllerTest { .value("Er schrieb einen langen Brief")); } + // ─── /api/documents/search pagination ───────────────────────────────────── + + @Test + @WithMockUser + void search_responseExposesPagingFields() throws Exception { + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) + .thenReturn(DocumentSearchResult.of(List.of())); + + mockMvc.perform(get("/api/documents/search")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.pageNumber").exists()) + .andExpect(jsonPath("$.pageSize").exists()) + .andExpect(jsonPath("$.totalPages").exists()) + .andExpect(jsonPath("$.totalElements").exists()); + } + + @Test + @WithMockUser + void search_returns400_whenSizeExceedsMax() throws Exception { + // Locks @Validated on the controller — removing it silently reopens the + // DoS window where a client could request all 1500 docs + enrichment. + mockMvc.perform(get("/api/documents/search").param("size", "101")) + .andExpect(status().isBadRequest()); + } + + @Test + @WithMockUser + void search_returns400_whenSizeBelowMin() throws Exception { + mockMvc.perform(get("/api/documents/search").param("size", "0")) + .andExpect(status().isBadRequest()); + } + + @Test + @WithMockUser + void search_returns400_whenPageNegative() throws Exception { + mockMvc.perform(get("/api/documents/search").param("page", "-1")) + .andExpect(status().isBadRequest()); + } + + @Test + @WithMockUser + void search_returns400_whenPageAboveMax() throws Exception { + // Guards against page * size overflow into negative SQL OFFSET + mockMvc.perform(get("/api/documents/search").param("page", "200000")) + .andExpect(status().isBadRequest()); + } + + @Test + @WithMockUser + void search_passesPageRequestToService() throws Exception { + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) + .thenReturn(DocumentSearchResult.of(List.of())); + + mockMvc.perform(get("/api/documents/search").param("page", "2").param("size", "25")) + .andExpect(status().isOk()); + + org.mockito.ArgumentCaptor captor = + org.mockito.ArgumentCaptor.forClass(org.springframework.data.domain.Pageable.class); + verify(documentService).searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), captor.capture()); + org.springframework.data.domain.Pageable pageable = captor.getValue(); + org.assertj.core.api.Assertions.assertThat(pageable.getPageNumber()).isEqualTo(2); + org.assertj.core.api.Assertions.assertThat(pageable.getPageSize()).isEqualTo(25); + } + // ─── POST /api/documents ───────────────────────────────────────────────── @Test diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java index 41c0479b..aef18793 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -1323,6 +1323,104 @@ class DocumentServiceTest { assertThat(result).isNull(); } + // ─── searchDocuments — pagination ──────────────────────────────────────── + + @Test + void searchDocuments_fastPath_usesFindAllWithPageable_notWithSort() { + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class))) + .thenReturn(new PageImpl<>(List.of())); + + documentService.searchDocuments(null, null, null, null, null, null, null, null, + org.raddatz.familienarchiv.dto.DocumentSort.DATE, "DESC", null, + org.springframework.data.domain.PageRequest.of(1, 50)); + + verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class)); + verify(documentRepository, never()).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Sort.class)); + } + + @Test + void searchDocuments_fastPath_propagatesPageableToDatabase() { + ArgumentCaptor captor = ArgumentCaptor.forClass(Pageable.class); + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class))) + .thenReturn(new PageImpl<>(List.of())); + + documentService.searchDocuments(null, null, null, null, null, null, null, null, + org.raddatz.familienarchiv.dto.DocumentSort.DATE, "DESC", null, + org.springframework.data.domain.PageRequest.of(3, 25)); + + verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), captor.capture()); + assertThat(captor.getValue().getPageNumber()).isEqualTo(3); + assertThat(captor.getValue().getPageSize()).isEqualTo(25); + } + + @Test + void searchDocuments_fastPath_returnsPageableTotalsOnResult() { + // The service MUST report the full match count from Page.getTotalElements(), + // not the slice size — otherwise the frontend's "N Briefe gefunden" label is wrong. + Document d = Document.builder().id(UUID.randomUUID()).title("T").build(); + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class))) + .thenReturn(new PageImpl<>(List.of(d), org.springframework.data.domain.PageRequest.of(0, 50), 120L)); + + DocumentSearchResult result = documentService.searchDocuments(null, null, null, null, null, null, null, null, + org.raddatz.familienarchiv.dto.DocumentSort.DATE, "DESC", null, + org.springframework.data.domain.PageRequest.of(0, 50)); + + assertThat(result.totalElements()).isEqualTo(120L); + assertThat(result.pageNumber()).isZero(); + assertThat(result.pageSize()).isEqualTo(50); + assertThat(result.totalPages()).isEqualTo(3); // ceil(120/50) + assertThat(result.items()).hasSize(1); // only the slice is enriched + } + + @Test + void searchDocuments_senderSort_slicesInMemoryAndReportsFullTotal() { + // Fixture: 120 docs with senders; request page 1, size 50 → expect 50 items + // back with totalElements = 120. + List all = new java.util.ArrayList<>(); + for (int i = 0; i < 120; i++) { + Person p = Person.builder() + .id(UUID.randomUUID()) + .firstName("F" + i) + .lastName(String.format("L%03d", i)) + .build(); + all.add(Document.builder().id(UUID.randomUUID()).title("D" + i).sender(p).build()); + } + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class))) + .thenReturn(all); + + DocumentSearchResult result = documentService.searchDocuments(null, null, null, null, null, null, null, null, + org.raddatz.familienarchiv.dto.DocumentSort.SENDER, "asc", null, + org.springframework.data.domain.PageRequest.of(1, 50)); + + assertThat(result.totalElements()).isEqualTo(120L); + assertThat(result.pageNumber()).isEqualTo(1); + assertThat(result.pageSize()).isEqualTo(50); + assertThat(result.totalPages()).isEqualTo(3); + assertThat(result.items()).hasSize(50); + // Page 1 (offset 50) under ascending sender sort should start at L050 + assertThat(result.items().get(0).document().getSender().getLastName()).isEqualTo("L050"); + } + + @Test + void searchDocuments_pageBeyondLast_returnsEmptyContentAndCorrectTotal() { + // Guards the JPA edge case where page * size > totalElements. + // Must not throw, must return empty content + correct totalElements. + List all = new java.util.ArrayList<>(); + for (int i = 0; i < 30; i++) { + Person p = Person.builder().id(UUID.randomUUID()).lastName(String.format("L%02d", i)).build(); + all.add(Document.builder().id(UUID.randomUUID()).title("D" + i).sender(p).build()); + } + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class))) + .thenReturn(all); + + DocumentSearchResult result = documentService.searchDocuments(null, null, null, null, null, null, null, null, + org.raddatz.familienarchiv.dto.DocumentSort.SENDER, "asc", null, + org.springframework.data.domain.PageRequest.of(10, 50)); + + assertThat(result.items()).isEmpty(); + assertThat(result.totalElements()).isEqualTo(30L); + } + // ─── searchDocuments — status filter ───────────────────────────────────── @Test -- 2.49.1 From 3311dc29ae0e615345157b381bc0c48412ad7b36 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 08:34:01 +0200 Subject: [PATCH 4/8] feat(documents): paginate search with a Pagination control MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frontend side of the /documents pagination work. The page.server.ts load reads ?page= from the URL, forwards page+size=50 to the backend, and exposes the new totalElements/pageNumber/pageSize/totalPages fields on `data`. +page.svelte renders a component below the result list; buildPageHref preserves every filter param and only updates page. The existing triggerSearch debounce flow intentionally drops `page` when any filter changes, so filter edits reset to page 0 automatically. uses plain links (not goto) so SvelteKit's default scroll restoration scrolls new pages to the top — the expected senior-UX behaviour. Decorative chevrons wrapped in aria-hidden spans, 44px touch targets, focus-visible ring, stacks vertically under 640px. The control hides itself when totalPages ≤ 1. Test coverage: 9 cases on Pagination (label, aria-current, prev/next enable/disable, makeHref invocation, decorative chevron, touch target), plus a filter-reset assertion on +page.svelte (page 5 → edit q → goto URL must drop page=). Adds i18n keys in de/en/es. Manual edit to api.ts pending a post-merge npm run generate:api against a rebuilt dev backend. (#315) Co-Authored-By: Claude Opus 4.7 --- frontend/messages/de.json | 6 +- frontend/messages/en.json | 6 +- frontend/messages/es.json | 6 +- frontend/src/lib/components/Pagination.svelte | 58 +++++++++++++ .../lib/components/Pagination.svelte.spec.ts | 84 +++++++++++++++++++ frontend/src/lib/generated/api.ts | 18 +++- frontend/src/routes/documents/+page.server.ts | 17 +++- frontend/src/routes/documents/+page.svelte | 34 +++++++- .../src/routes/documents/page.server.spec.ts | 12 +-- .../src/routes/documents/page.svelte.spec.ts | 16 ++++ 10 files changed, 243 insertions(+), 14 deletions(-) create mode 100644 frontend/src/lib/components/Pagination.svelte create mode 100644 frontend/src/lib/components/Pagination.svelte.spec.ts diff --git a/frontend/messages/de.json b/frontend/messages/de.json index f74c749a..5e56ed54 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -806,5 +806,9 @@ "chronik_load_more": "Mehr laden", "chronik_loading": "Lädt …", "chronik_load_more_announcement": "{count} weitere Einträge geladen", - "chronik_view_all": "Alle Aktivitäten →" + "chronik_view_all": "Alle Aktivitäten →", + "pagination_prev": "Zurück", + "pagination_next": "Weiter", + "pagination_page_of": "Seite {page} von {total}", + "pagination_nav_label": "Seitennavigation" } diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 0ff58ce2..5e2591ef 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -806,5 +806,9 @@ "chronik_load_more": "Load more", "chronik_loading": "Loading …", "chronik_load_more_announcement": "{count} more entries loaded", - "chronik_view_all": "All activity →" + "chronik_view_all": "All activity →", + "pagination_prev": "Previous", + "pagination_next": "Next", + "pagination_page_of": "Page {page} of {total}", + "pagination_nav_label": "Pagination" } diff --git a/frontend/messages/es.json b/frontend/messages/es.json index f968bbde..b64d0405 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -806,5 +806,9 @@ "chronik_load_more": "Cargar más", "chronik_loading": "Cargando …", "chronik_load_more_announcement": "{count} entradas más cargadas", - "chronik_view_all": "Todas las actividades →" + "chronik_view_all": "Todas las actividades →", + "pagination_prev": "Anterior", + "pagination_next": "Siguiente", + "pagination_page_of": "Página {page} de {total}", + "pagination_nav_label": "Paginación" } diff --git a/frontend/src/lib/components/Pagination.svelte b/frontend/src/lib/components/Pagination.svelte new file mode 100644 index 00000000..0edf0fb7 --- /dev/null +++ b/frontend/src/lib/components/Pagination.svelte @@ -0,0 +1,58 @@ + + +{#if totalPages > 1} + +{/if} diff --git a/frontend/src/lib/components/Pagination.svelte.spec.ts b/frontend/src/lib/components/Pagination.svelte.spec.ts new file mode 100644 index 00000000..68af1558 --- /dev/null +++ b/frontend/src/lib/components/Pagination.svelte.spec.ts @@ -0,0 +1,84 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { cleanup, render } from 'vitest-browser-svelte'; +import { page } from 'vitest/browser'; + +import Pagination from './Pagination.svelte'; + +afterEach(() => { + cleanup(); +}); + +const makeHref = (p: number) => `/documents?page=${p}`; + +describe('Pagination', () => { + it('renders the page-of-total label for the current page', async () => { + render(Pagination, { page: 2, totalPages: 10, makeHref }); + + const label = page.getByTestId('pagination-page-label'); + await expect.element(label).toHaveTextContent(/3/); // page is 0-indexed, label is 1-indexed + await expect.element(label).toHaveTextContent(/10/); + }); + + it('marks the current page label with aria-current="page"', async () => { + render(Pagination, { page: 0, totalPages: 3, makeHref }); + + const label = page.getByTestId('pagination-page-label'); + await expect.element(label).toHaveAttribute('aria-current', 'page'); + }); + + it('renders prev as a link pointing at page - 1 when not on first page', async () => { + render(Pagination, { page: 4, totalPages: 10, makeHref }); + + const prev = page.getByTestId('pagination-prev'); + await expect.element(prev).toHaveAttribute('href', '/documents?page=3'); + }); + + it('disables prev on page 0 (no href, aria-disabled="true")', async () => { + render(Pagination, { page: 0, totalPages: 3, makeHref }); + + const prev = page.getByTestId('pagination-prev'); + await expect.element(prev).toHaveAttribute('aria-disabled', 'true'); + await expect.element(prev).not.toHaveAttribute('href'); + }); + + it('renders next as a link pointing at page + 1 when not on last page', async () => { + render(Pagination, { page: 0, totalPages: 3, makeHref }); + + const next = page.getByTestId('pagination-next'); + await expect.element(next).toHaveAttribute('href', '/documents?page=1'); + }); + + it('disables next on the last page (no href, aria-disabled="true")', async () => { + render(Pagination, { page: 2, totalPages: 3, makeHref }); + + const next = page.getByTestId('pagination-next'); + await expect.element(next).toHaveAttribute('aria-disabled', 'true'); + await expect.element(next).not.toHaveAttribute('href'); + }); + + it('calls makeHref with p-1 and p+1', async () => { + const spy = vi.fn(makeHref); + render(Pagination, { page: 3, totalPages: 10, makeHref: spy }); + + const calls = spy.mock.calls.map((c) => c[0]).sort((a, b) => a - b); + expect(calls).toContain(2); + expect(calls).toContain(4); + }); + + it('renders decorative chevron inside aria-hidden span so screen readers skip it', async () => { + render(Pagination, { page: 1, totalPages: 3, makeHref }); + + const prev = page.getByTestId('pagination-prev'); + await expect + .element(prev.getByText('«', { exact: true })) + .toHaveAttribute('aria-hidden', 'true'); + }); + + it('prev and next have min 44px touch targets', async () => { + render(Pagination, { page: 1, totalPages: 3, makeHref }); + + const prev = page.getByTestId('pagination-prev'); + await expect.element(prev).toHaveClass(/min-h-\[44px\]/); + await expect.element(prev).toHaveClass(/min-w-\[44px\]/); + }); +}); diff --git a/frontend/src/lib/generated/api.ts b/frontend/src/lib/generated/api.ts index a72acd15..aafb9c78 100644 --- a/frontend/src/lib/generated/api.ts +++ b/frontend/src/lib/generated/api.ts @@ -1921,7 +1921,13 @@ export interface components { DocumentSearchResult: { items: components["schemas"]["DocumentSearchItem"][]; /** Format: int64 */ - total: number; + totalElements: number; + /** Format: int32 */ + pageNumber: number; + /** Format: int32 */ + pageSize: number; + /** Format: int32 */ + totalPages: number; }; MatchOffset: { /** Format: int32 */ @@ -4032,6 +4038,16 @@ export interface operations { dir?: string; /** @description Tag operator: AND (default) or OR */ tagOp?: string; + /** + * @description Page number (0-indexed) + * @default 0 + */ + page?: number; + /** + * @description Page size (max 100) + * @default 50 + */ + size?: number; }; header?: never; path?: never; diff --git a/frontend/src/routes/documents/+page.server.ts b/frontend/src/routes/documents/+page.server.ts index ed46e46a..f9e4daf4 100644 --- a/frontend/src/routes/documents/+page.server.ts +++ b/frontend/src/routes/documents/+page.server.ts @@ -10,6 +10,8 @@ type ValidSort = (typeof VALID_SORTS)[number]; const VALID_DIRS = ['asc', 'desc'] as const; type ValidDir = (typeof VALID_DIRS)[number]; +const PAGE_SIZE = 50; + export async function load({ url, fetch }) { const q = url.searchParams.get('q') || ''; const from = url.searchParams.get('from') || ''; @@ -27,6 +29,7 @@ export async function load({ url, fetch }) { : 'desc'; const tagQ = url.searchParams.get('tagQ') || ''; const tagOp = url.searchParams.get('tagOp') === 'OR' ? 'OR' : 'AND'; + const page = Math.max(0, Number(url.searchParams.get('page') ?? '0') || 0); const api = createApiClient(fetch); @@ -44,14 +47,19 @@ export async function load({ url, fetch }) { tagQ: tagQ && !tags.length ? tagQ : undefined, tagOp: tagOp === 'OR' ? 'OR' : undefined, sort, - dir: dir || undefined + dir: dir || undefined, + page, + size: PAGE_SIZE } } }); } catch { return { items: [] as DocumentSearchItem[], - total: 0, + totalElements: 0, + pageNumber: 0, + pageSize: PAGE_SIZE, + totalPages: 0, q, from, to, @@ -77,7 +85,10 @@ export async function load({ url, fetch }) { return { items: (result.data?.items ?? []) as DocumentSearchItem[], - total: result.data?.total ?? 0, + totalElements: result.data?.totalElements ?? 0, + pageNumber: result.data?.pageNumber ?? page, + pageSize: result.data?.pageSize ?? PAGE_SIZE, + totalPages: result.data?.totalPages ?? 0, q, from, to, diff --git a/frontend/src/routes/documents/+page.svelte b/frontend/src/routes/documents/+page.svelte index 57054ff6..22f756d3 100644 --- a/frontend/src/routes/documents/+page.svelte +++ b/frontend/src/routes/documents/+page.svelte @@ -5,6 +5,7 @@ import { untrack } from 'svelte'; import { SvelteURLSearchParams } from 'svelte/reactivity'; import SearchFilterBar from '../SearchFilterBar.svelte'; import DocumentList from '../DocumentList.svelte'; +import Pagination from '$lib/components/Pagination.svelte'; import * as m from '$lib/paraglide/messages.js'; let { data } = $props(); @@ -35,6 +36,12 @@ let showAdvanced = $state(untrack(hasAdvancedFilters)); let searchTimer: ReturnType; +/** + * Rebuilds the URL from the CURRENT local filter state. `page` is intentionally + * not carried over — any filter change implicitly resets back to page 0, which + * is the expected behaviour. For page-only navigation use {@link buildPageHref} + * instead, which preserves every filter from the server `data`. + */ function triggerSearch() { const params = new SvelteURLSearchParams(); if (q) params.set('q', q); @@ -50,6 +57,29 @@ function triggerSearch() { goto(`/documents?${params.toString()}`, { keepFocus: true, noScroll: true }); } +/** + * Builds the href for a Pagination prev/next link. Preserves every current + * filter param and only updates `page`. Uses a normal (not goto) + * so SvelteKit's default scroll restoration brings the user to the top of + * the new slice — the expected behaviour for page navigation. + */ +function buildPageHref(targetPage: number): string { + const params = new SvelteURLSearchParams(); + if (data.q) params.set('q', data.q); + if (data.from) params.set('from', data.from); + if (data.to) params.set('to', data.to); + if (data.senderId) params.set('senderId', data.senderId); + if (data.receiverId) params.set('receiverId', data.receiverId); + (data.tags || []).forEach((t: string) => params.append('tag', t)); + if (data.sort) params.set('sort', data.sort); + if (data.dir) params.set('dir', data.dir); + if (data.tagQ) params.set('tagQ', data.tagQ); + if (data.tagOp === 'OR') params.set('tagOp', 'OR'); + if (targetPage > 0) params.set('page', String(targetPage)); + const qs = params.toString(); + return qs ? `/documents?${qs}` : '/documents'; +} + function handleTextSearch() { clearTimeout(searchTimer); searchTimer = setTimeout(() => triggerSearch(), 500); @@ -115,10 +145,12 @@ $effect(() => { + + diff --git a/frontend/src/routes/documents/page.server.spec.ts b/frontend/src/routes/documents/page.server.spec.ts index d345023e..d99c8d8d 100644 --- a/frontend/src/routes/documents/page.server.spec.ts +++ b/frontend/src/routes/documents/page.server.spec.ts @@ -25,7 +25,7 @@ describe('documents page load — search params', () => { it('passes q, from, to to the search API', async () => { const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [], total: 0 } + data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient @@ -49,7 +49,7 @@ describe('documents page load — search params', () => { it('passes senderId and receiverId to the search API', async () => { const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [], total: 0 } + data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient @@ -73,7 +73,7 @@ describe('documents page load — search params', () => { it('passes sort, dir, tagQ to the search API', async () => { const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [], total: 0 } + data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient @@ -103,7 +103,7 @@ describe('documents page load — search params', () => { }; const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [item], total: 42 } + data: { items: [item], totalElements: 42, pageNumber: 0, pageSize: 50, totalPages: 1 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient @@ -115,13 +115,13 @@ describe('documents page load — search params', () => { }); expect(result.items).toHaveLength(1); - expect(result.total).toBe(42); + expect(result.totalElements).toBe(42); }); it('returns filter values in the result for pre-filling the UI', async () => { const mockGet = vi.fn().mockResolvedValue({ response: { ok: true, status: 200 }, - data: { items: [], total: 0 } + data: { items: [], totalElements: 0, pageNumber: 0, pageSize: 50, totalPages: 0 } }); vi.mocked(createApiClient).mockReturnValue({ GET: mockGet } as ReturnType< typeof createApiClient diff --git a/frontend/src/routes/documents/page.svelte.spec.ts b/frontend/src/routes/documents/page.svelte.spec.ts index 59af31bf..1d8064c6 100644 --- a/frontend/src/routes/documents/page.svelte.spec.ts +++ b/frontend/src/routes/documents/page.svelte.spec.ts @@ -118,4 +118,20 @@ describe('documents page — URL building', () => { expect.objectContaining({ keepFocus: true, noScroll: true }) ); }); + + it('filter change does not carry the current page — goto URL drops page param', async () => { + const { goto } = await import('$app/navigation'); + vi.mocked(goto).mockClear(); + + // User is mid-way through results at page 5; change the search text. + render(Page, { data: makeData({ q: 'old', pageNumber: 5 }) }); + + const input = page.getByRole('textbox', { name: SEARCH_LABEL }); + await input.fill('Brief'); + vi.advanceTimersByTime(500); + + const [url] = vi.mocked(goto).mock.calls[0]; + expect(url).toContain('q=Brief'); + expect(url).not.toContain('page='); + }); }); -- 2.49.1 From 6808d9fb03eb3c13a1a6634a6806f73076648f3f Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 10:46:46 +0200 Subject: [PATCH 5/8] =?UTF-8?q?refactor(test):=20extract=20UNPAGED=20Pagea?= =?UTF-8?q?ble=20constant=20=E2=80=94=20address=20@felixbrandt=20+=20@sara?= =?UTF-8?q?holt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PageRequest.of(0, 10_000) was inlined at ~12 sites across DocumentServiceTest and DocumentServiceSortTest as an "effectively unpaged" sentinel for tests that don't care about paging. Extracted to a named constant on each class so the intent is visible at each callsite and we don't risk copy-paste drift of the magic number. No behaviour change. (#316) Co-Authored-By: Claude Opus 4.7 --- .../service/DocumentServiceSortTest.java | 8 ++++--- .../service/DocumentServiceTest.java | 23 ++++++++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java index 3bdbbc2c..7e0443ca 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceSortTest.java @@ -26,6 +26,8 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class DocumentServiceSortTest { + private static final Pageable UNPAGED = org.springframework.data.domain.PageRequest.of(0, 10_000); + @Mock DocumentRepository documentRepository; @Mock PersonService personService; @Mock FileService fileService; @@ -57,7 +59,7 @@ class DocumentServiceSortTest { .thenReturn(new PageImpl<>(List.of(newer, older))); DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.DATE, "DESC", null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + "Brief", null, null, null, null, null, null, null, DocumentSort.DATE, "DESC", null, UNPAGED); // Expect: date order (newer 1960 first), NOT rank order (older 1940 first) assertThat(result.items()).hasSize(2); @@ -79,7 +81,7 @@ class DocumentServiceSortTest { .thenReturn(List.of(doc2, doc1)); // unordered from DB DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, UNPAGED); // Expect: rank order restored (id1 first) assertThat(result.items().get(0).document().getId()).isEqualTo(id1); @@ -98,7 +100,7 @@ class DocumentServiceSortTest { .thenReturn(List.of(doc2, doc1)); DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, null, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + "Brief", null, null, null, null, null, null, null, null, null, null, UNPAGED); assertThat(result.items().get(0).document().getId()).isEqualTo(id1); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java index aef18793..0e46196f 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -24,6 +24,7 @@ import org.raddatz.familienarchiv.model.Tag; import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.mock.web.MockMultipartFile; @@ -46,6 +47,12 @@ import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) class DocumentServiceTest { + // Used by tests that don't care about paging. 10 000 is chosen large enough + // to hold any fixture in this file but small enough that totalPages math + // stays in int range. Swap to `PageRequest.of(0, 10_000)` elsewhere is a + // red flag — use this constant. + private static final Pageable UNPAGED = PageRequest.of(0, 10_000); + @Mock DocumentRepository documentRepository; @Mock PersonService personService; @Mock FileService fileService; @@ -1428,7 +1435,7 @@ class DocumentServiceTest { when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class))) .thenReturn(new PageImpl<>(List.of())); - documentService.searchDocuments(null, null, null, null, null, null, null, DocumentStatus.REVIEWED, null, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + documentService.searchDocuments(null, null, null, null, null, null, null, DocumentStatus.REVIEWED, null, null, null, UNPAGED); verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class)); } @@ -1438,7 +1445,7 @@ class DocumentServiceTest { when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class))) .thenReturn(new PageImpl<>(List.of())); - documentService.searchDocuments(null, null, null, null, null, null, null, null, null, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + documentService.searchDocuments(null, null, null, null, null, null, null, null, null, null, null, UNPAGED); verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class)); } @@ -1516,7 +1523,7 @@ class DocumentServiceTest { .thenReturn(List.of(withSender, noSender)); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, UNPAGED); assertThat(result.items()).hasSize(2); assertThat(result.items()).extracting(item -> item.document().getTitle()).containsExactly("Has Sender", "No Sender"); @@ -1536,7 +1543,7 @@ class DocumentServiceTest { .thenReturn(List.of(noReceivers, withReceiver)); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, DocumentSort.RECEIVER, "asc", null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + null, null, null, null, null, null, null, null, DocumentSort.RECEIVER, "asc", null, UNPAGED); assertThat(result.items()).extracting(item -> item.document().getTitle()) .containsExactly("Has Receiver", "No Receivers"); @@ -1558,7 +1565,7 @@ class DocumentServiceTest { .thenReturn(List.of(docNullName, docSmith)); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, UNPAGED); // null lastName should sort to end (treated as empty), not before "smith" (as "null") assertThat(result.items()).extracting(item -> item.document().getTitle()) @@ -1580,7 +1587,7 @@ class DocumentServiceTest { when(documentRepository.findEnrichmentData(any(), eq("Brief"))).thenReturn(rows); DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, UNPAGED); assertThat(result.items()).hasSize(1); SearchMatchData md = result.items().get(0).matchData(); @@ -1595,7 +1602,7 @@ class DocumentServiceTest { DocumentSearchResult result = documentService.searchDocuments( null, null, null, null, null, null, null, null, null, null, null, - org.springframework.data.domain.PageRequest.of(0, 10_000)); + UNPAGED); assertThat(result.items()).isEmpty(); } @@ -1614,7 +1621,7 @@ class DocumentServiceTest { when(documentRepository.findEnrichmentData(any(), eq("Brief"))).thenReturn(rows); DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, org.springframework.data.domain.PageRequest.of(0, 10_000)); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, UNPAGED); SearchMatchData md = result.items().get(0).matchData(); assertThat(md.transcriptionSnippet()).isEqualTo("Hier ist der Brief aus Berlin"); -- 2.49.1 From baf7ae34206177152ea30d2bf4ed3c3c56be6f4d Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 10:48:34 +0200 Subject: [PATCH 6/8] =?UTF-8?q?refactor(documents):=20extract=20buildSearc?= =?UTF-8?q?hParams=20=E2=80=94=20address=20@felixbrandt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit triggerSearch (local state, filter change) and buildPageHref (server data, page nav) were each iterating over the same ~10 filter params. Any new filter would have had to land in two places. buildSearchParams is now the single source of truth for which params the /documents URL understands; both callers just pass their snapshot and an optional targetPage. (#316) Co-Authored-By: Claude Opus 4.7 --- frontend/src/routes/documents/+page.svelte | 98 +++++++++++++++------- 1 file changed, 68 insertions(+), 30 deletions(-) diff --git a/frontend/src/routes/documents/+page.svelte b/frontend/src/routes/documents/+page.svelte index 22f756d3..80159e25 100644 --- a/frontend/src/routes/documents/+page.svelte +++ b/frontend/src/routes/documents/+page.svelte @@ -36,46 +36,84 @@ let showAdvanced = $state(untrack(hasAdvancedFilters)); let searchTimer: ReturnType; +type FilterSnapshot = { + q: string; + from: string; + to: string; + senderId: string; + receiverId: string; + tags: string[]; + sort: string; + dir: string; + tagQ: string; + tagOp: 'AND' | 'OR'; +}; + +/** + * Builds a URLSearchParams from a filter snapshot. Single source of truth for + * which params the `/documents` URL understands — add a filter here and both + * filter-change nav (triggerSearch) and page nav (buildPageHref) will pick it + * up. `page` is appended only when > 0 so the default page 0 stays out of the + * URL, keeping the filter-change-resets-to-page-0 behaviour implicit. + */ +function buildSearchParams(filters: FilterSnapshot, targetPage?: number): SvelteURLSearchParams { + const params = new SvelteURLSearchParams(); + if (filters.q) params.set('q', filters.q); + if (filters.from) params.set('from', filters.from); + if (filters.to) params.set('to', filters.to); + if (filters.senderId) params.set('senderId', filters.senderId); + if (filters.receiverId) params.set('receiverId', filters.receiverId); + filters.tags.forEach((tag) => params.append('tag', tag)); + if (filters.sort) params.set('sort', filters.sort); + if (filters.dir) params.set('dir', filters.dir); + if (filters.tagQ) params.set('tagQ', filters.tagQ); + if (filters.tagOp === 'OR') params.set('tagOp', 'OR'); + if (targetPage !== undefined && targetPage > 0) params.set('page', String(targetPage)); + return params; +} + /** * Rebuilds the URL from the CURRENT local filter state. `page` is intentionally - * not carried over — any filter change implicitly resets back to page 0, which - * is the expected behaviour. For page-only navigation use {@link buildPageHref} - * instead, which preserves every filter from the server `data`. + * not carried over — any filter change implicitly resets back to page 0. */ function triggerSearch() { - const params = new SvelteURLSearchParams(); - if (q) params.set('q', q); - if (from) params.set('from', from); - if (to) params.set('to', to); - if (senderId) params.set('senderId', senderId); - if (receiverId) params.set('receiverId', receiverId); - tagNames.forEach((tag) => params.append('tag', tag.name)); - if (sort) params.set('sort', sort); - if (dir) params.set('dir', dir); - if (tagQ) params.set('tagQ', tagQ); - if (tagOperator === 'OR') params.set('tagOp', 'OR'); + const params = buildSearchParams({ + q, + from, + to, + senderId, + receiverId, + tags: tagNames.map((t) => t.name), + sort, + dir, + tagQ, + tagOp: tagOperator + }); goto(`/documents?${params.toString()}`, { keepFocus: true, noScroll: true }); } /** - * Builds the href for a Pagination prev/next link. Preserves every current - * filter param and only updates `page`. Uses a normal (not goto) - * so SvelteKit's default scroll restoration brings the user to the top of - * the new slice — the expected behaviour for page navigation. + * Builds the href for a Pagination prev/next link. Preserves every filter + * param from server `data` and updates `page`. Uses a normal (not + * goto) so SvelteKit's default scroll restoration brings the user to the top + * of the new slice — the expected behaviour for page navigation. */ function buildPageHref(targetPage: number): string { - const params = new SvelteURLSearchParams(); - if (data.q) params.set('q', data.q); - if (data.from) params.set('from', data.from); - if (data.to) params.set('to', data.to); - if (data.senderId) params.set('senderId', data.senderId); - if (data.receiverId) params.set('receiverId', data.receiverId); - (data.tags || []).forEach((t: string) => params.append('tag', t)); - if (data.sort) params.set('sort', data.sort); - if (data.dir) params.set('dir', data.dir); - if (data.tagQ) params.set('tagQ', data.tagQ); - if (data.tagOp === 'OR') params.set('tagOp', 'OR'); - if (targetPage > 0) params.set('page', String(targetPage)); + const params = buildSearchParams( + { + q: data.q || '', + from: data.from || '', + to: data.to || '', + senderId: data.senderId || '', + receiverId: data.receiverId || '', + tags: data.tags || [], + sort: data.sort || '', + dir: data.dir || '', + tagQ: data.tagQ || '', + tagOp: (data.tagOp as 'AND' | 'OR') || 'AND' + }, + targetPage + ); const qs = params.toString(); return qs ? `/documents?${qs}` : '/documents'; } -- 2.49.1 From a8edd0d9fd6f56d5933557b92ac3f492df1f1615 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 10:52:13 +0200 Subject: [PATCH 7/8] =?UTF-8?q?fix(pagination):=20bound=20controls=20rende?= =?UTF-8?q?r=20as=20aria-hidden=20spans=20=E2=80=94=20address=20@leonievos?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit is the documented pattern but screen readers still announce "Previous, link, disabled" on pagination bounds — noise users don't need because the disabled state is purely visual. Switching to - - {m.pagination_prev()} - + + {#if hasPrev} + + + {m.pagination_prev()} + + {:else} + + {/if} - - {m.pagination_next()} - - + {#if hasNext} + + {m.pagination_next()} + + + {:else} + + {/if} {/if} diff --git a/frontend/src/lib/components/Pagination.svelte.spec.ts b/frontend/src/lib/components/Pagination.svelte.spec.ts index 68af1558..90bf8532 100644 --- a/frontend/src/lib/components/Pagination.svelte.spec.ts +++ b/frontend/src/lib/components/Pagination.svelte.spec.ts @@ -33,12 +33,14 @@ describe('Pagination', () => { await expect.element(prev).toHaveAttribute('href', '/documents?page=3'); }); - it('disables prev on page 0 (no href, aria-disabled="true")', async () => { + it('renders disabled prev as an aria-hidden non-link so screen readers skip it', async () => { render(Pagination, { page: 0, totalPages: 3, makeHref }); const prev = page.getByTestId('pagination-prev'); - await expect.element(prev).toHaveAttribute('aria-disabled', 'true'); + // Not a link — no href, no role=link await expect.element(prev).not.toHaveAttribute('href'); + // Hidden from assistive tech — AT shouldn't read "Previous, link, disabled" + await expect.element(prev).toHaveAttribute('aria-hidden', 'true'); }); it('renders next as a link pointing at page + 1 when not on last page', async () => { @@ -48,12 +50,12 @@ describe('Pagination', () => { await expect.element(next).toHaveAttribute('href', '/documents?page=1'); }); - it('disables next on the last page (no href, aria-disabled="true")', async () => { + it('renders disabled next as an aria-hidden non-link on the last page', async () => { render(Pagination, { page: 2, totalPages: 3, makeHref }); const next = page.getByTestId('pagination-next'); - await expect.element(next).toHaveAttribute('aria-disabled', 'true'); await expect.element(next).not.toHaveAttribute('href'); + await expect.element(next).toHaveAttribute('aria-hidden', 'true'); }); it('calls makeHref with p-1 and p+1', async () => { -- 2.49.1 From 4f741a870180f0b5d751f2a45688ad71a8bbed95 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 24 Apr 2026 10:55:51 +0200 Subject: [PATCH 8/8] =?UTF-8?q?test(search):=20integration=20test=20covers?= =?UTF-8?q?=20paged=20search=20against=20real=20Postgres=20=E2=80=94=20add?= =?UTF-8?q?ress=20@saraholt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seeds 120 UPLOADED docs with a deterministic date spread and runs DocumentService.searchDocuments against a Testcontainers Postgres, not a Mockito mock. Five cases: 1. First page returns exactly page_size items + correct totalElements 2. Last partial page returns the tail slice (offset 100 → 20 items) 3. Page beyond last returns empty content, totalElements still 120 4. SENDER sort path slices in-memory + reports correct total 5. Different pages return disjoint document id sets Closes the integration-coverage gap between the Mockito unit tests and the full Spec→Pageable→Page→DTO path that unit tests can't exercise. Runs in ~87 s against the shared Testcontainers instance. (#316) Co-Authored-By: Claude Opus 4.7 --- .../DocumentSearchPagedIntegrationTest.java | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/service/DocumentSearchPagedIntegrationTest.java diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentSearchPagedIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentSearchPagedIntegrationTest.java new file mode 100644 index 00000000..5bf02c6b --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentSearchPagedIntegrationTest.java @@ -0,0 +1,137 @@ +package org.raddatz.familienarchiv.service; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.dto.DocumentSearchResult; +import org.raddatz.familienarchiv.dto.DocumentSort; +import org.raddatz.familienarchiv.model.Document; +import org.raddatz.familienarchiv.model.DocumentStatus; +import org.raddatz.familienarchiv.repository.DocumentRepository; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Import; +import org.springframework.data.domain.PageRequest; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import software.amazon.awssdk.services.s3.S3Client; + +import java.time.LocalDate; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * End-to-end paged search test with real PostgreSQL (Testcontainers). Covers the + * Specification→Pageable→Page→DTO path that unit tests mock around. Seeds 120 + * UPLOADED documents and asserts the slice/total/totalPages arithmetic holds + * against the actual JPA query. + * + *

Closes the integration-coverage gap Sara flagged on PR #316. + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) +@ActiveProfiles("test") +@Import(PostgresContainerConfig.class) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD) +class DocumentSearchPagedIntegrationTest { + + private static final int FIXTURE_SIZE = 120; + + @MockitoBean S3Client s3Client; + @Autowired DocumentService documentService; + @Autowired DocumentRepository documentRepository; + + @BeforeEach + void seed() { + // Deterministic date spread so DATE-DESC order is predictable: + // document #0 has the oldest date, document #119 has the newest. + for (int i = 0; i < FIXTURE_SIZE; i++) { + Document doc = Document.builder() + .title("Dok-" + String.format("%03d", i)) + .originalFilename("dok-" + i + ".pdf") + .status(DocumentStatus.UPLOADED) + .documentDate(LocalDate.of(1900, 1, 1).plusDays(i)) + .build(); + documentRepository.save(doc); + } + assertThat(documentRepository.count()).isEqualTo(FIXTURE_SIZE); + } + + @Test + void search_firstPage_returnsExactlyPageSizeItems_andCorrectTotalElements() { + DocumentSearchResult result = documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.DATE, "DESC", null, + PageRequest.of(0, 50)); + + assertThat(result.items()).hasSize(50); + assertThat(result.totalElements()).isEqualTo(FIXTURE_SIZE); + assertThat(result.pageNumber()).isZero(); + assertThat(result.pageSize()).isEqualTo(50); + assertThat(result.totalPages()).isEqualTo(3); // ceil(120 / 50) + } + + @Test + void search_lastPartialPage_returnsRemainingItems() { + DocumentSearchResult result = documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.DATE, "DESC", null, + PageRequest.of(2, 50)); + + // Page 2 (offset 100) of 120 docs → exactly 20 items on the tail. + assertThat(result.items()).hasSize(20); + assertThat(result.totalElements()).isEqualTo(FIXTURE_SIZE); + assertThat(result.pageNumber()).isEqualTo(2); + } + + @Test + void search_pageBeyondLast_returnsEmptyContent_totalElementsStillCorrect() { + DocumentSearchResult result = documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.DATE, "DESC", null, + PageRequest.of(99, 50)); + + assertThat(result.items()).isEmpty(); + assertThat(result.totalElements()).isEqualTo(FIXTURE_SIZE); + } + + @Test + void search_senderSort_pageOne_slicesInMemory_withCorrectTotal() { + // SENDER sort path fetches all + sorts + slices in-memory (see scaling + // comment in DocumentService). Proves that the in-memory slice path + // returns the correct total from a real repository fetch. + DocumentSearchResult result = documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.SENDER, "asc", null, + PageRequest.of(1, 50)); + + assertThat(result.items()).hasSize(50); + assertThat(result.totalElements()).isEqualTo(FIXTURE_SIZE); + assertThat(result.pageNumber()).isEqualTo(1); + assertThat(result.totalPages()).isEqualTo(3); + } + + @Test + void search_differentPagesReturnDisjointSlices() { + DocumentSearchResult page0 = documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.DATE, "DESC", null, + PageRequest.of(0, 50)); + DocumentSearchResult page1 = documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.DATE, "DESC", null, + PageRequest.of(1, 50)); + + // No document id should appear on both pages — slicing must be exclusive. + var idsOnPage0 = page0.items().stream() + .map(item -> item.document().getId()) + .toList(); + var idsOnPage1 = page1.items().stream() + .map(item -> item.document().getId()) + .toList(); + for (UUID id : idsOnPage0) { + assertThat(idsOnPage1).doesNotContain(id); + } + } +} -- 2.49.1