From 268c31a49bd9374c00a07d0076e451d6b5b60541 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 27 May 2026 18:42:17 +0200 Subject: [PATCH] feat(document): thread an undated filter through search and the /ids path Adds an optional `undated` query param to GET /api/documents/search and /api/documents/ids, threaded through searchDocuments and findIdsForFilter into the shared buildSearchSpec via undatedOnly(boolean). undated=true also bypasses the pure-text RELEVANCE SQL shortcut, which skips buildSearchSpec and would otherwise drop the predicate. The read GET stays unguarded (WebMvc authz test pins 200 for an authenticated user, 401 unauthenticated). A locking test proves the in-memory SENDER sort keeps undated letters under their sender. Refs #668 --- .../document/DocumentController.java | 6 +- .../document/DocumentService.java | 21 +++-- .../document/DocumentControllerTest.java | 75 +++++++++++++--- .../document/DocumentLazyLoadingTest.java | 6 +- .../DocumentListItemIntegrationTest.java | 9 +- .../DocumentSearchPagedIntegrationTest.java | 18 ++-- .../document/DocumentServiceSortTest.java | 14 +-- .../document/DocumentServiceTest.java | 90 +++++++++++++------ 8 files changed, 159 insertions(+), 80 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java index f4bf72d3..daaa96c5 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentController.java @@ -313,9 +313,10 @@ public class DocumentController { @RequestParam(required = false) String tagQ, @RequestParam(required = false) DocumentStatus status, @RequestParam(required = false) String tagOp, + @RequestParam(required = false) Boolean undated, Authentication authentication) { TagOperator operator = "OR".equalsIgnoreCase(tagOp) ? TagOperator.OR : TagOperator.AND; - List ids = documentService.findIdsForFilter(q, from, to, senderId, receiverId, tags, tagQ, status, operator); + List ids = documentService.findIdsForFilter(q, from, to, senderId, receiverId, tags, tagQ, status, operator, Boolean.TRUE.equals(undated)); if (ids.size() > BULK_EDIT_FILTER_MAX_IDS) { throw DomainException.badRequest(ErrorCode.BULK_EDIT_TOO_MANY_IDS, "Filter matches " + ids.size() + " documents — refine filter (max " + BULK_EDIT_FILTER_MAX_IDS + ")"); @@ -375,6 +376,7 @@ public class DocumentController { @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 = "Restrict to undated documents (meta_date IS NULL)") @RequestParam(required = false) Boolean undated, // @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. @@ -387,7 +389,7 @@ public class DocumentController { // defaults to AND, which matches the frontend default and keeps old clients working. TagOperator operator = "OR".equalsIgnoreCase(tagOp) ? TagOperator.OR : TagOperator.AND; Pageable pageable = PageRequest.of(page, size); - return ResponseEntity.ok(documentService.searchDocuments(q, from, to, senderId, receiverId, tags, tagQ, status, sort, dir, operator, pageable)); + return ResponseEntity.ok(documentService.searchDocuments(q, from, to, senderId, receiverId, tags, tagQ, status, sort, dir, operator, Boolean.TRUE.equals(undated), pageable)); } @GetMapping(value = "/density", produces = MediaType.APPLICATION_JSON_VALUE) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index 1cef9593..43d206b7 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -171,7 +171,7 @@ public class DocumentService { hasFts, ftsIds, null, null, filters.sender(), filters.receiver(), filters.tags(), filters.tagQ(), - filters.status(), filters.tagOperator()); + filters.status(), filters.tagOperator(), false); return documentRepository.findAll(spec).stream() .map(Document::getDocumentDate) .filter(Objects::nonNull) @@ -501,7 +501,8 @@ public class DocumentService { */ @Transactional(readOnly = true) public List findIdsForFilter(String text, LocalDate from, LocalDate to, UUID sender, UUID receiver, - List tags, String tagQ, DocumentStatus status, TagOperator tagOperator) { + List tags, String tagQ, DocumentStatus status, TagOperator tagOperator, + boolean undated) { boolean hasText = StringUtils.hasText(text); List rankedIds = null; if (hasText) { @@ -510,7 +511,7 @@ public class DocumentService { } Specification spec = buildSearchSpec( - hasText, rankedIds, from, to, sender, receiver, tags, tagQ, status, tagOperator); + hasText, rankedIds, from, to, sender, receiver, tags, tagQ, status, tagOperator, undated); return documentRepository.findAll(spec).stream().map(Document::getId).toList(); } @@ -524,7 +525,8 @@ public class DocumentService { LocalDate from, LocalDate to, UUID sender, UUID receiver, List tags, String tagQ, - DocumentStatus status, TagOperator tagOperator) { + DocumentStatus status, TagOperator tagOperator, + boolean undated) { boolean useOrLogic = tagOperator == TagOperator.OR; List> expandedTagSets = tagService.expandTagNamesToDescendantIdSets(tags); Specification textSpec = hasText ? hasIds(ftsIds) : (root, query, cb) -> null; @@ -534,7 +536,8 @@ public class DocumentService { .and(hasReceiver(receiver)) .and(hasTags(expandedTagSets, useOrLogic)) .and(hasTagPartial(tagQ)) - .and(hasStatus(status)); + .and(hasStatus(status)) + .and(undatedOnly(undated)); } /** @@ -663,11 +666,13 @@ 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, Pageable pageable) { + 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, boolean undated, Pageable pageable) { boolean hasText = StringUtils.hasText(text); // Pure-text RELEVANCE: push pagination into SQL — skip findAllMatchingIdsByFts entirely (ADR-008). - if (isPureTextRelevance(hasText, sort, from, to, sender, receiver, tags, tagQ, status)) { + // An active undated filter must NOT take this path: it bypasses buildSearchSpec, so the + // undatedOnly predicate would be silently dropped. + if (!undated && isPureTextRelevance(hasText, sort, from, to, sender, receiver, tags, tagQ, status)) { return relevanceSortedPageFromSql(text, pageable); } @@ -678,7 +683,7 @@ public class DocumentService { } Specification spec = buildSearchSpec( - hasText, rankedIds, from, to, sender, receiver, tags, tagQ, status, tagOperator); + hasText, rankedIds, from, to, sender, receiver, tags, tagQ, status, tagOperator, undated); // SENDER and RECEIVER sorts load the full match set and slice in-memory. // JPA's Sort.by("sender.lastName") generates an INNER JOIN that silently drops diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java index 00b69ce5..8608ed83 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java @@ -1,6 +1,7 @@ package org.raddatz.familienarchiv.document; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.raddatz.familienarchiv.document.DocumentBatchMetadataDTO; import org.raddatz.familienarchiv.document.DocumentSearchResult; import org.raddatz.familienarchiv.document.DocumentVersionSummary; @@ -35,7 +36,9 @@ import java.util.List; import java.util.Optional; import java.util.UUID; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; @@ -73,23 +76,69 @@ class DocumentControllerTest { @Test @WithMockUser void search_returns200_whenAuthenticated() throws Exception { - when(documentService.searchDocuments(any(), 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(), anyBoolean(), any())) .thenReturn(DocumentSearchResult.of(List.of())); mockMvc.perform(get("/api/documents/search")) .andExpect(status().isOk()); } + @Test + @WithMockUser + void search_undatedTrue_isReachableByAuthenticatedUser() throws Exception { + // The read GET must stay reachable for READ_ALL users — guards against a + // future refactor accidentally write-guarding the undated triage path (#668). + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any())) + .thenReturn(DocumentSearchResult.of(List.of())); + + mockMvc.perform(get("/api/documents/search").param("undated", "true")) + .andExpect(status().isOk()); + } + + @Test + void search_undatedTrue_returns401_whenUnauthenticated() throws Exception { + mockMvc.perform(get("/api/documents/search").param("undated", "true")) + .andExpect(status().isUnauthorized()); + } + + @Test + @WithMockUser + void search_undatedTrue_isForwardedToServiceAsTrue() throws Exception { + ArgumentCaptor undatedCaptor = ArgumentCaptor.forClass(Boolean.class); + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any())) + .thenReturn(DocumentSearchResult.of(List.of())); + + mockMvc.perform(get("/api/documents/search").param("undated", "true")) + .andExpect(status().isOk()); + + verify(documentService).searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), undatedCaptor.capture(), any()); + assertThat(undatedCaptor.getValue()).isTrue(); + } + + @Test + @WithMockUser + void search_withoutUndatedParam_forwardsFalseToService() throws Exception { + ArgumentCaptor undatedCaptor = ArgumentCaptor.forClass(Boolean.class); + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any())) + .thenReturn(DocumentSearchResult.of(List.of())); + + mockMvc.perform(get("/api/documents/search")) + .andExpect(status().isOk()); + + verify(documentService).searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), undatedCaptor.capture(), any()); + assertThat(undatedCaptor.getValue()).isFalse(); + } + @Test @WithMockUser void search_withStatusParam_passesItToService() throws Exception { - when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), eq(DocumentStatus.REVIEWED), any(), any(), any(), any())) + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), eq(DocumentStatus.REVIEWED), any(), any(), any(), anyBoolean(), 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(), any()); + verify(documentService).searchDocuments(any(), any(), any(), any(), any(), any(), any(), eq(DocumentStatus.REVIEWED), any(), any(), any(), anyBoolean(), any()); } @Test @@ -116,7 +165,7 @@ class DocumentControllerTest { @Test @WithMockUser void search_responseContainsTotalCount() throws Exception { - when(documentService.searchDocuments(any(), 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(), anyBoolean(), any())) .thenReturn(DocumentSearchResult.of(List.of())); mockMvc.perform(get("/api/documents/search")) @@ -131,7 +180,7 @@ class DocumentControllerTest { UUID docId = UUID.randomUUID(); 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(), any())) + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any())) .thenReturn(DocumentSearchResult.of(List.of(new DocumentListItem( docId, "Brief an Anna", "brief.pdf", null, null, DatePrecision.UNKNOWN, null, null, @@ -150,7 +199,7 @@ class DocumentControllerTest { void search_returns_flat_item_with_id_and_without_sensitive_fields() throws Exception { UUID docId = UUID.randomUUID(); var matchData = new SearchMatchData(null, 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(), any())) + when(documentService.searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), any())) .thenReturn(DocumentSearchResult.of(List.of(new DocumentListItem( docId, "Brief an Anna", "brief.pdf", null, null, DatePrecision.UNKNOWN, null, null, @@ -172,7 +221,7 @@ class DocumentControllerTest { @Test @WithMockUser void search_responseExposesPagingFields() throws Exception { - when(documentService.searchDocuments(any(), 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(), anyBoolean(), any())) .thenReturn(DocumentSearchResult.of(List.of())); mockMvc.perform(get("/api/documents/search")) @@ -217,7 +266,7 @@ class DocumentControllerTest { @Test @WithMockUser void search_passesPageRequestToService() throws Exception { - when(documentService.searchDocuments(any(), 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(), anyBoolean(), any())) .thenReturn(DocumentSearchResult.of(List.of())); mockMvc.perform(get("/api/documents/search").param("page", "2").param("size", "25")) @@ -225,7 +274,7 @@ class DocumentControllerTest { 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()); + verify(documentService).searchDocuments(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), anyBoolean(), 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); @@ -1143,7 +1192,7 @@ class DocumentControllerTest { void getDocumentIds_returns200_andDelegatesToService() throws Exception { when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); UUID id = UUID.randomUUID(); - when(documentService.findIdsForFilter(any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(documentService.findIdsForFilter(any(), any(), any(), any(), any(), any(), any(), any(), any(), anyBoolean())) .thenReturn(List.of(id)); mockMvc.perform(get("/api/documents/ids")) @@ -1156,13 +1205,13 @@ class DocumentControllerTest { void getDocumentIds_passesSenderIdParamToService() throws Exception { when(userService.findByEmail(any())).thenReturn(AppUser.builder().id(UUID.randomUUID()).build()); UUID senderId = UUID.randomUUID(); - when(documentService.findIdsForFilter(any(), any(), any(), eq(senderId), any(), any(), any(), any(), any())) + when(documentService.findIdsForFilter(any(), any(), any(), eq(senderId), any(), any(), any(), any(), any(), anyBoolean())) .thenReturn(List.of()); mockMvc.perform(get("/api/documents/ids").param("senderId", senderId.toString())) .andExpect(status().isOk()); - verify(documentService).findIdsForFilter(any(), any(), any(), eq(senderId), any(), any(), any(), any(), any()); + verify(documentService).findIdsForFilter(any(), any(), any(), eq(senderId), any(), any(), any(), any(), any(), anyBoolean()); } @Test @@ -1172,7 +1221,7 @@ class DocumentControllerTest { // Service returns 5001 IDs — one over BULK_EDIT_FILTER_MAX_IDS (5000). java.util.List tooMany = new java.util.ArrayList<>(5001); for (int i = 0; i < 5001; i++) tooMany.add(UUID.randomUUID()); - when(documentService.findIdsForFilter(any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(documentService.findIdsForFilter(any(), any(), any(), any(), any(), any(), any(), any(), any(), anyBoolean())) .thenReturn(tooMany); mockMvc.perform(get("/api/documents/ids")) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java index 62a2d843..1b5a4b1e 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java @@ -123,8 +123,7 @@ class DocumentLazyLoadingTest { DocumentSearchResult result = documentService.searchDocuments( null, null, null, null, null, null, null, null, - DocumentSort.RECEIVER, "asc", null, - PageRequest.of(0, 20)); + DocumentSort.RECEIVER, "asc", null, false, PageRequest.of(0, 20)); assertThat(result.totalElements()).isGreaterThan(0); assertThatCode(() -> result.items().forEach(i -> { if (i.sender() != null) i.sender().getLastName(); })) @@ -139,8 +138,7 @@ class DocumentLazyLoadingTest { assertThatCode(() -> documentService.searchDocuments( null, null, null, null, null, null, null, null, - DocumentSort.SENDER, "asc", null, - PageRequest.of(0, 20))) + DocumentSort.SENDER, "asc", null, false, PageRequest.of(0, 20))) .doesNotThrowAnyException(); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java index d97aaf9c..3d0e4b90 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java @@ -56,8 +56,7 @@ class DocumentListItemIntegrationTest { assertThatCode(() -> documentService.searchDocuments( null, null, null, null, null, null, null, null, - DocumentSort.DATE, "DESC", null, - PageRequest.of(0, 50))) + DocumentSort.DATE, "DESC", null, false, PageRequest.of(0, 50))) .doesNotThrowAnyException(); } @@ -72,8 +71,7 @@ class DocumentListItemIntegrationTest { DocumentSearchResult result = documentService.searchDocuments( null, null, null, null, null, null, null, null, - DocumentSort.DATE, "DESC", null, - PageRequest.of(0, 50)); + DocumentSort.DATE, "DESC", null, false, PageRequest.of(0, 50)); assertThat(result.totalElements()).isGreaterThan(0); DocumentListItem item = result.items().get(0); @@ -94,8 +92,7 @@ class DocumentListItemIntegrationTest { DocumentSearchResult result = documentService.searchDocuments( null, null, null, null, null, null, null, null, - DocumentSort.DATE, "DESC", null, - PageRequest.of(0, 50)); + DocumentSort.DATE, "DESC", null, false, PageRequest.of(0, 50)); DocumentListItem item = result.items().stream() .filter(i -> i.title().equals("Range Brief")).findFirst().orElseThrow(); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchPagedIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchPagedIntegrationTest.java index c61c38af..05c7e025 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchPagedIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchPagedIntegrationTest.java @@ -62,8 +62,7 @@ class DocumentSearchPagedIntegrationTest { 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)); + DocumentSort.DATE, "DESC", null, false, PageRequest.of(0, 50)); assertThat(result.items()).hasSize(50); assertThat(result.totalElements()).isEqualTo(FIXTURE_SIZE); @@ -76,8 +75,7 @@ class DocumentSearchPagedIntegrationTest { void search_lastPartialPage_returnsRemainingItems() { DocumentSearchResult result = documentService.searchDocuments( null, null, null, null, null, null, null, null, - DocumentSort.DATE, "DESC", null, - PageRequest.of(2, 50)); + DocumentSort.DATE, "DESC", null, false, PageRequest.of(2, 50)); // Page 2 (offset 100) of 120 docs → exactly 20 items on the tail. assertThat(result.items()).hasSize(20); @@ -89,8 +87,7 @@ class DocumentSearchPagedIntegrationTest { 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)); + DocumentSort.DATE, "DESC", null, false, PageRequest.of(99, 50)); assertThat(result.items()).isEmpty(); assertThat(result.totalElements()).isEqualTo(FIXTURE_SIZE); @@ -103,8 +100,7 @@ class DocumentSearchPagedIntegrationTest { // 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)); + DocumentSort.SENDER, "asc", null, false, PageRequest.of(1, 50)); assertThat(result.items()).hasSize(50); assertThat(result.totalElements()).isEqualTo(FIXTURE_SIZE); @@ -116,12 +112,10 @@ class DocumentSearchPagedIntegrationTest { void search_differentPagesReturnDisjointSlices() { DocumentSearchResult page0 = documentService.searchDocuments( null, null, null, null, null, null, null, null, - DocumentSort.DATE, "DESC", null, - PageRequest.of(0, 50)); + DocumentSort.DATE, "DESC", null, false, PageRequest.of(0, 50)); DocumentSearchResult page1 = documentService.searchDocuments( null, null, null, null, null, null, null, null, - DocumentSort.DATE, "DESC", null, - PageRequest.of(1, 50)); + DocumentSort.DATE, "DESC", null, false, PageRequest.of(1, 50)); // No document id should appear on both pages — slicing must be exclusive. var idsOnPage0 = page0.items().stream() diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceSortTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceSortTest.java index abf6e389..c3d00619 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceSortTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceSortTest.java @@ -67,7 +67,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, PAGE); + "Brief", null, null, null, null, null, null, null, DocumentSort.DATE, "DESC", null, false, PAGE); assertThat(result.items()).hasSize(2); assertThat(result.items().get(0).id()).isEqualTo(id2); // newer first @@ -84,7 +84,7 @@ class DocumentServiceSortTest { .thenReturn(List.of(doc(id1))); documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, PAGE); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, false, PAGE); verify(documentRepository).findFtsPageRaw(anyString(), anyInt(), anyInt()); verify(documentRepository, never()).findAllMatchingIdsByFts(anyString()); @@ -102,7 +102,7 @@ class DocumentServiceSortTest { when(documentRepository.findAllById(any())).thenReturn(List.of(doc(id2), doc(id1))); // unordered from JPA DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, PAGE); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, false, PAGE); assertThat(result.items().get(0).id()).isEqualTo(id1); } @@ -119,7 +119,7 @@ class DocumentServiceSortTest { when(documentRepository.findAllById(any())).thenReturn(List.of(doc(id2), doc(id1))); DocumentSearchResult result = documentService.searchDocuments( - "Brief", null, null, null, null, null, null, null, null, null, null, PAGE); + "Brief", null, null, null, null, null, null, null, null, null, null, false, PAGE); assertThat(result.items().get(0).id()).isEqualTo(id1); } @@ -133,7 +133,7 @@ class DocumentServiceSortTest { DocumentSearchResult result = documentService.searchDocuments( "Brief", null, null, null, null, null, null, null, - DocumentSort.RELEVANCE, null, null, hugePage); + DocumentSort.RELEVANCE, null, null, false, hugePage); assertThat(result.items()).isEmpty(); verify(documentRepository, never()).findFtsPageRaw(anyString(), anyInt(), anyInt()); @@ -153,7 +153,7 @@ class DocumentServiceSortTest { DocumentSearchResult result = documentService.searchDocuments( "Brief", null, null, null, null, null, null, null, - DocumentSort.RELEVANCE, null, null, PAGE); + DocumentSort.RELEVANCE, null, null, false, PAGE); assertThat(result.items()).hasSize(1); assertThat(result.items().get(0).id()).isEqualTo(uuidId); @@ -173,7 +173,7 @@ class DocumentServiceSortTest { // sender filter is active → triggers in-memory path, not findFtsPageRaw LocalDate from = LocalDate.of(1900, 1, 1); documentService.searchDocuments( - "Brief", from, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, PAGE); + "Brief", from, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, false, PAGE); verify(documentRepository, never()).findFtsPageRaw(anyString(), anyInt(), anyInt()); verify(documentRepository).findAllMatchingIdsByFts("Brief"); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java index 32a4b3b9..757284ca 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java @@ -47,6 +47,8 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.*; @@ -1409,8 +1411,7 @@ class DocumentServiceTest { .thenReturn(new PageImpl<>(List.of())); documentService.searchDocuments(null, null, null, null, null, null, null, null, - org.raddatz.familienarchiv.document.DocumentSort.DATE, "DESC", null, - org.springframework.data.domain.PageRequest.of(1, 50)); + org.raddatz.familienarchiv.document.DocumentSort.DATE, "DESC", null, false, 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)); @@ -1423,8 +1424,7 @@ class DocumentServiceTest { .thenReturn(new PageImpl<>(List.of())); documentService.searchDocuments(null, null, null, null, null, null, null, null, - org.raddatz.familienarchiv.document.DocumentSort.DATE, "DESC", null, - org.springframework.data.domain.PageRequest.of(3, 25)); + org.raddatz.familienarchiv.document.DocumentSort.DATE, "DESC", null, false, 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); @@ -1440,8 +1440,7 @@ class DocumentServiceTest { .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.document.DocumentSort.DATE, "DESC", null, - org.springframework.data.domain.PageRequest.of(0, 50)); + org.raddatz.familienarchiv.document.DocumentSort.DATE, "DESC", null, false, org.springframework.data.domain.PageRequest.of(0, 50)); assertThat(result.totalElements()).isEqualTo(120L); assertThat(result.pageNumber()).isZero(); @@ -1457,8 +1456,7 @@ class DocumentServiceTest { .thenReturn(new PageImpl<>(List.of())); documentService.searchDocuments(null, null, null, null, null, null, null, null, - DocumentSort.DATE, "DESC", null, - org.springframework.data.domain.PageRequest.of(0, 5)); + DocumentSort.DATE, "DESC", null, false, org.springframework.data.domain.PageRequest.of(0, 5)); verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), captor.capture()); Sort.Order dateOrder = captor.getValue().getSort().getOrderFor("documentDate"); @@ -1476,8 +1474,7 @@ class DocumentServiceTest { .thenReturn(new PageImpl<>(List.of())); documentService.searchDocuments(null, null, null, null, null, null, null, null, - DocumentSort.DATE, "ASC", null, - org.springframework.data.domain.PageRequest.of(0, 5)); + DocumentSort.DATE, "ASC", null, false, org.springframework.data.domain.PageRequest.of(0, 5)); verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), captor.capture()); Sort.Order dateOrder = captor.getValue().getSort().getOrderFor("documentDate"); @@ -1493,8 +1490,7 @@ class DocumentServiceTest { .thenReturn(new PageImpl<>(List.of())); documentService.searchDocuments(null, null, null, null, null, null, null, null, - DocumentSort.UPDATED_AT, "DESC", null, - org.springframework.data.domain.PageRequest.of(0, 5)); + DocumentSort.UPDATED_AT, "DESC", null, false, org.springframework.data.domain.PageRequest.of(0, 5)); verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), captor.capture()); assertThat(captor.getValue().getSort()) @@ -1518,8 +1514,7 @@ class DocumentServiceTest { .thenReturn(all); DocumentSearchResult result = documentService.searchDocuments(null, null, null, null, null, null, null, null, - org.raddatz.familienarchiv.document.DocumentSort.SENDER, "asc", null, - org.springframework.data.domain.PageRequest.of(1, 50)); + org.raddatz.familienarchiv.document.DocumentSort.SENDER, "asc", null, false, org.springframework.data.domain.PageRequest.of(1, 50)); assertThat(result.totalElements()).isEqualTo(120L); assertThat(result.pageNumber()).isEqualTo(1); @@ -1543,8 +1538,7 @@ class DocumentServiceTest { .thenReturn(all); DocumentSearchResult result = documentService.searchDocuments(null, null, null, null, null, null, null, null, - org.raddatz.familienarchiv.document.DocumentSort.SENDER, "asc", null, - org.springframework.data.domain.PageRequest.of(10, 50)); + org.raddatz.familienarchiv.document.DocumentSort.SENDER, "asc", null, false, org.springframework.data.domain.PageRequest.of(10, 50)); assertThat(result.items()).isEmpty(); assertThat(result.totalElements()).isEqualTo(30L); @@ -1557,7 +1551,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, UNPAGED); + documentService.searchDocuments(null, null, null, null, null, null, null, DocumentStatus.REVIEWED, null, null, null, false, UNPAGED); verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class)); } @@ -1567,7 +1561,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, UNPAGED); + documentService.searchDocuments(null, null, null, null, null, null, null, null, null, null, null, false, UNPAGED); verify(documentRepository).findAll(any(org.springframework.data.jpa.domain.Specification.class), any(Pageable.class)); } @@ -1645,7 +1639,7 @@ class DocumentServiceTest { .thenReturn(List.of(withSender, noSender)); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, UNPAGED); + null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, false, UNPAGED); assertThat(result.items()).hasSize(2); assertThat(result.items()).extracting(DocumentListItem::title).containsExactly("Has Sender", "No Sender"); @@ -1665,12 +1659,53 @@ class DocumentServiceTest { .thenReturn(List.of(noReceivers, withReceiver)); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, DocumentSort.RECEIVER, "asc", null, UNPAGED); + null, null, null, null, null, null, null, null, DocumentSort.RECEIVER, "asc", null, false, UNPAGED); assertThat(result.items()).extracting(DocumentListItem::title) .containsExactly("Has Receiver", "No Receivers"); } + // ─── searchDocuments — undated docs stay in their person group (#668) ─────── + + @Test + void searchDocuments_senderSort_keepsUndatedDocumentUnderItsSender() { + // Locking test: the in-memory SENDER comparator orders by sender name, not + // date, so an undated (null documentDate) letter must NOT be pulled out of + // its sender's group — it sorts by sender exactly like a dated letter. + Person alice = Person.builder().id(UUID.randomUUID()).firstName("Alice").lastName("Ziegler").build(); + Document datedFromAlice = Document.builder().id(UUID.randomUUID()).title("Dated") + .sender(alice).documentDate(LocalDate.of(1916, 6, 15)).build(); + Document undatedFromAlice = Document.builder().id(UUID.randomUUID()).title("Undated") + .sender(alice).documentDate(null).build(); + + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class))) + .thenReturn(List.of(datedFromAlice, undatedFromAlice)); + + DocumentSearchResult result = documentService.searchDocuments( + null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, false, UNPAGED); + + // Both stay together under Alice; neither is dropped or reordered by date. + assertThat(result.items()).extracting(DocumentListItem::title) + .containsExactlyInAnyOrder("Dated", "Undated"); + assertThat(result.items()).allMatch(item -> item.sender().getId().equals(alice.getId())); + } + + @Test + void searchDocuments_undatedTrue_usesSpecificationPath_notPureTextRelevanceShortcut() { + // undated=true must bypass the pure-text RELEVANCE SQL shortcut, which + // skips buildSearchSpec and would silently drop the undatedOnly predicate. + when(documentRepository.findAllMatchingIdsByFts("brief")).thenReturn(List.of(UUID.randomUUID())); + when(documentRepository.findAll(any(org.springframework.data.jpa.domain.Specification.class))) + .thenReturn(List.of()); + + documentService.searchDocuments("brief", null, null, null, null, null, null, null, + DocumentSort.RELEVANCE, null, null, true, UNPAGED); + + // The FTS-id path (buildSearchSpec) ran; the raw-page SQL shortcut did not. + verify(documentRepository).findAllMatchingIdsByFts("brief"); + verify(documentRepository, never()).findFtsPageRaw(anyString(), anyInt(), anyInt()); + } + @Test void searchDocuments_senderSort_nullLastNameSortsToEnd() { // Without fix: null lastName produces sort key "null Smith" which compares @@ -1687,7 +1722,7 @@ class DocumentServiceTest { .thenReturn(List.of(docNullName, docSmith)); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, UNPAGED); + null, null, null, null, null, null, null, null, DocumentSort.SENDER, "asc", null, false, UNPAGED); // null lastName should sort to end (treated as empty), not before "smith" (as "null") assertThat(result.items()).extracting(DocumentListItem::title) @@ -1710,7 +1745,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, UNPAGED); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, false, UNPAGED); assertThat(result.items()).hasSize(1); SearchMatchData md = result.items().get(0).matchData(); @@ -1724,8 +1759,7 @@ class DocumentServiceTest { .thenReturn(new PageImpl<>(List.of())); DocumentSearchResult result = documentService.searchDocuments( - null, null, null, null, null, null, null, null, null, null, null, - UNPAGED); + null, null, null, null, null, null, null, null, null, null, null, false, UNPAGED); assertThat(result.items()).isEmpty(); } @@ -1745,7 +1779,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, UNPAGED); + "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, false, UNPAGED); SearchMatchData md = result.items().get(0).matchData(); assertThat(md.transcriptionSnippet()).isEqualTo("Hier ist der Brief aus Berlin"); @@ -2262,7 +2296,7 @@ class DocumentServiceTest { .thenReturn(List.of(d1, d2)); List result = documentService.findIdsForFilter( - null, null, null, null, null, null, null, null, null); + null, null, null, null, null, null, null, null, null, false); assertThat(result).containsExactly(d1.getId(), d2.getId()); } @@ -2277,7 +2311,7 @@ class DocumentServiceTest { when(tagService.expandTagNamesToDescendantIdSets(any())).thenReturn(List.of()); documentService.findIdsForFilter( - null, null, null, null, null, List.of("Brief"), null, null, TagOperator.OR); + null, null, null, null, null, List.of("Brief"), null, null, TagOperator.OR, false); // Spec built without throwing → OR branch was exercised. Coverage gain // is in not-throwing on the OR-specific code path; the actual SQL is @@ -2290,7 +2324,7 @@ class DocumentServiceTest { when(documentRepository.findAllMatchingIdsByFts("xyz")).thenReturn(List.of()); List result = documentService.findIdsForFilter( - "xyz", null, null, null, null, null, null, null, null); + "xyz", null, null, null, null, null, null, null, null, false); assertThat(result).isEmpty(); verify(documentRepository, never()).findAll(any(org.springframework.data.jpa.domain.Specification.class));