From f22dcaecb7dc2169676f4b400c86114ea1cc9d15 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 22 May 2026 17:43:48 +0200 Subject: [PATCH 1/5] refactor(document): replace DocumentSearchItem with flat DocumentListItem DTO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminates excessive data exposure (OWASP API3:2023) — transcription, filePath, fileHash, thumbnailKey, scriptType and other detail-only fields are no longer serialised in the list API response. Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentListItem.java | 36 +++++++++++++++++++ .../document/DocumentSearchItem.java | 18 ---------- .../document/DocumentSearchResult.java | 14 ++------ .../document/DocumentService.java | 25 +++++++++++-- .../document/DocumentControllerTest.java | 33 ++++++++++++----- .../document/DocumentLazyLoadingTest.java | 2 +- .../DocumentSearchPagedIntegrationTest.java | 4 +-- .../document/DocumentSearchResultTest.java | 24 ++++++------- .../document/DocumentServiceSortTest.java | 8 ++--- .../document/DocumentServiceTest.java | 10 +++--- 10 files changed, 108 insertions(+), 66 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/document/DocumentListItem.java delete mode 100644 backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchItem.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentListItem.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentListItem.java new file mode 100644 index 00000000..7cbc6496 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentListItem.java @@ -0,0 +1,36 @@ +package org.raddatz.familienarchiv.document; + +import io.swagger.v3.oas.annotations.media.Schema; +import org.raddatz.familienarchiv.audit.ActivityActorDTO; +import org.raddatz.familienarchiv.person.Person; +import org.raddatz.familienarchiv.tag.Tag; + +import java.time.LocalDate; +import java.util.List; +import java.util.UUID; + +public record DocumentListItem( + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + UUID id, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + String title, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + String originalFilename, + String thumbnailUrl, + LocalDate documentDate, + Person sender, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + List receivers, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + List tags, + String archiveBox, + String archiveFolder, + String location, + String summary, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + int completionPercentage, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + List contributors, + @Schema(requiredMode = Schema.RequiredMode.REQUIRED) + SearchMatchData matchData +) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchItem.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchItem.java deleted file mode 100644 index 58d31a03..00000000 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchItem.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.raddatz.familienarchiv.document; - -import io.swagger.v3.oas.annotations.media.Schema; -import org.raddatz.familienarchiv.audit.ActivityActorDTO; -import org.raddatz.familienarchiv.document.Document; - -import java.util.List; - -public record DocumentSearchItem( - @Schema(requiredMode = Schema.RequiredMode.REQUIRED) - Document document, - @Schema(requiredMode = Schema.RequiredMode.REQUIRED) - SearchMatchData matchData, - @Schema(requiredMode = Schema.RequiredMode.REQUIRED) - int completionPercentage, - @Schema(requiredMode = Schema.RequiredMode.REQUIRED) - List contributors -) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchResult.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchResult.java index e80efbd2..25b39184 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchResult.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchResult.java @@ -7,7 +7,7 @@ import java.util.List; public record DocumentSearchResult( @Schema(requiredMode = Schema.RequiredMode.REQUIRED) - List items, + List items, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) long totalElements, @Schema(requiredMode = Schema.RequiredMode.REQUIRED) @@ -17,20 +17,12 @@ public record DocumentSearchResult( @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) { + public static DocumentSearchResult of(List items) { 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) { + 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/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index a739764e..0a290c5b 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -10,7 +10,6 @@ import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.document.DocumentBatchMetadataDTO; import org.raddatz.familienarchiv.document.DocumentBatchSummary; import org.raddatz.familienarchiv.document.DocumentBulkEditDTO; -import org.raddatz.familienarchiv.document.DocumentSearchItem; import org.raddatz.familienarchiv.document.DocumentSearchResult; import org.raddatz.familienarchiv.document.DocumentSort; import org.raddatz.familienarchiv.document.DocumentUpdateDTO; @@ -736,7 +735,7 @@ public class DocumentService { return DocumentSearchResult.paged(enrichItems(slice, text), pageable, totalElements); } - private List enrichItems(List documents, String text) { + private List enrichItems(List documents, String text) { List colorResolved = resolveDocumentTagColors(documents); Map matchData = enrichWithMatchData(colorResolved, text); @@ -744,7 +743,7 @@ public class DocumentService { Map completionByDoc = fetchCompletionPercentages(docIds); Map> contributorsByDoc = auditLogQueryService.findRecentContributorsPerDocument(docIds); - return colorResolved.stream().map(doc -> new DocumentSearchItem( + return colorResolved.stream().map(doc -> toListItem( doc, matchData.getOrDefault(doc.getId(), SearchMatchData.empty()), completionByDoc.getOrDefault(doc.getId(), 0), @@ -752,6 +751,26 @@ public class DocumentService { )).toList(); } + private DocumentListItem toListItem(Document doc, SearchMatchData match, int completionPct, List contributors) { + return new DocumentListItem( + doc.getId(), + doc.getTitle(), + doc.getOriginalFilename(), + doc.getThumbnailUrl(), + doc.getDocumentDate(), + doc.getSender(), + doc.getReceivers() != null ? List.copyOf(doc.getReceivers()) : List.of(), + doc.getTags() != null ? List.copyOf(doc.getTags()) : List.of(), + doc.getArchiveBox(), + doc.getArchiveFolder(), + doc.getLocation(), + doc.getSummary(), + completionPct, + contributors, + match + ); + } + private Map fetchCompletionPercentages(List docIds) { return transcriptionBlockQueryService.getCompletionStats(docIds); } 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 de2a79f3..f7c24541 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentControllerTest.java @@ -27,7 +27,6 @@ import org.springframework.security.test.context.support.WithMockUser; import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.web.servlet.MockMvc; -import org.raddatz.familienarchiv.document.DocumentSearchItem; import org.raddatz.familienarchiv.document.SearchMatchData; import java.time.LocalDateTime; @@ -130,16 +129,13 @@ class DocumentControllerTest { @WithMockUser void search_responseBodyItemsContainMatchData() throws Exception { UUID docId = UUID.randomUUID(); - Document doc = Document.builder() - .id(docId) - .title("Brief an Anna") - .originalFilename("brief.pdf") - .status(DocumentStatus.UPLOADED) - .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(), any())) - .thenReturn(DocumentSearchResult.of(List.of(new DocumentSearchItem(doc, matchData, 0, List.of())))); + .thenReturn(DocumentSearchResult.of(List.of(new DocumentListItem( + docId, "Brief an Anna", "brief.pdf", null, null, null, + List.of(), List.of(), null, null, null, null, + 0, List.of(), matchData)))); mockMvc.perform(get("/api/documents/search").param("q", "Brief")) .andExpect(status().isOk()) @@ -148,6 +144,27 @@ class DocumentControllerTest { .value("Er schrieb einen langen Brief")); } + @Test + @WithMockUser + 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())) + .thenReturn(DocumentSearchResult.of(List.of(new DocumentListItem( + docId, "Brief an Anna", "brief.pdf", null, null, null, + List.of(), List.of(), null, null, null, null, + 0, List.of(), matchData)))); + + mockMvc.perform(get("/api/documents/search")) + .andExpect(status().isOk()) + // flat id field present at top of item (not nested under $.items[0].document.id) + .andExpect(jsonPath("$.items[0].id").value(docId.toString())) + // sensitive storage fields must never appear in list response + .andExpect(jsonPath("$.items[0].transcription").doesNotExist()) + .andExpect(jsonPath("$.items[0].filePath").doesNotExist()) + .andExpect(jsonPath("$.items[0].fileHash").doesNotExist()); + } + // ─── /api/documents/search pagination ───────────────────────────────────── @Test 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 bc33815f..62a2d843 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java @@ -127,7 +127,7 @@ class DocumentLazyLoadingTest { PageRequest.of(0, 20)); assertThat(result.totalElements()).isGreaterThan(0); assertThatCode(() -> - result.items().forEach(i -> i.document().getSender().getLastName())) + result.items().forEach(i -> { if (i.sender() != null) i.sender().getLastName(); })) .doesNotThrowAnyException(); } 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 0ba61da3..c61c38af 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchPagedIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchPagedIntegrationTest.java @@ -125,10 +125,10 @@ class DocumentSearchPagedIntegrationTest { // No document id should appear on both pages — slicing must be exclusive. var idsOnPage0 = page0.items().stream() - .map(item -> item.document().getId()) + .map(item -> item.id()) .toList(); var idsOnPage1 = page1.items().stream() - .map(item -> item.document().getId()) + .map(item -> item.id()) .toList(); for (UUID id : idsOnPage0) { assertThat(idsOnPage1).doesNotContain(id); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchResultTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchResultTest.java index cbf44933..1dd09fed 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchResultTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentSearchResultTest.java @@ -3,8 +3,6 @@ package org.raddatz.familienarchiv.document; import io.swagger.v3.oas.annotations.media.Schema; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.audit.ActivityActorDTO; -import org.raddatz.familienarchiv.document.Document; -import org.raddatz.familienarchiv.document.DocumentStatus; import org.springframework.data.domain.PageRequest; import java.util.List; @@ -14,14 +12,11 @@ import static org.assertj.core.api.Assertions.assertThat; class DocumentSearchResultTest { - private DocumentSearchItem item(UUID docId) { - Document doc = Document.builder() - .id(docId) - .title("Test") - .originalFilename("test.pdf") - .status(DocumentStatus.UPLOADED) - .build(); - return new DocumentSearchItem(doc, SearchMatchData.empty(), 0, List.of()); + private DocumentListItem item(UUID docId) { + return new DocumentListItem( + docId, "Test", "test.pdf", null, null, null, + List.of(), List.of(), null, null, null, null, + 0, List.of(), SearchMatchData.empty()); } @Test @@ -45,7 +40,7 @@ class DocumentSearchResultTest { @Test void paged_factory_populates_paging_fields_from_pageable_and_total() { - List slice = List.of(item(UUID.randomUUID()), item(UUID.randomUUID())); + List slice = List.of(item(UUID.randomUUID()), item(UUID.randomUUID())); DocumentSearchResult result = DocumentSearchResult.paged(slice, PageRequest.of(1, 50), 120L); @@ -68,9 +63,10 @@ class DocumentSearchResultTest { void of_exposes_items_with_completion_and_contributors() { UUID id = UUID.randomUUID(); ActivityActorDTO actor = new ActivityActorDTO("AB", "#f00", "Anna Braun"); - Document doc = Document.builder().id(id).title("T").originalFilename("t.pdf") - .status(DocumentStatus.UPLOADED).build(); - DocumentSearchItem item = new DocumentSearchItem(doc, SearchMatchData.empty(), 75, List.of(actor)); + DocumentListItem item = new DocumentListItem( + id, "T", "t.pdf", null, null, null, + List.of(), List.of(), null, null, null, null, + 75, List.of(actor), SearchMatchData.empty()); DocumentSearchResult result = DocumentSearchResult.of(List.of(item)); 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 f7c4564f..abf6e389 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceSortTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceSortTest.java @@ -70,7 +70,7 @@ class DocumentServiceSortTest { "Brief", null, null, null, null, null, null, null, DocumentSort.DATE, "DESC", null, PAGE); assertThat(result.items()).hasSize(2); - assertThat(result.items().get(0).document().getId()).isEqualTo(id2); // newer first + assertThat(result.items().get(0).id()).isEqualTo(id2); // newer first } // ─── RELEVANCE sort — pure text (no filters) ────────────────────────────── @@ -104,7 +104,7 @@ class DocumentServiceSortTest { DocumentSearchResult result = documentService.searchDocuments( "Brief", null, null, null, null, null, null, null, DocumentSort.RELEVANCE, null, null, PAGE); - assertThat(result.items().get(0).document().getId()).isEqualTo(id1); + assertThat(result.items().get(0).id()).isEqualTo(id1); } @Test @@ -121,7 +121,7 @@ class DocumentServiceSortTest { DocumentSearchResult result = documentService.searchDocuments( "Brief", null, null, null, null, null, null, null, null, null, null, PAGE); - assertThat(result.items().get(0).document().getId()).isEqualTo(id1); + assertThat(result.items().get(0).id()).isEqualTo(id1); } // ─── RELEVANCE sort — overflow guard ───────────────────────────────────── @@ -156,7 +156,7 @@ class DocumentServiceSortTest { DocumentSort.RELEVANCE, null, null, PAGE); assertThat(result.items()).hasSize(1); - assertThat(result.items().get(0).document().getId()).isEqualTo(uuidId); + assertThat(result.items().get(0).id()).isEqualTo(uuidId); } // ─── RELEVANCE sort — text + active filter ──────────────────────────────── 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 186fd2aa..8ef7a6f2 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java @@ -11,7 +11,7 @@ import org.raddatz.familienarchiv.audit.AuditLogQueryService; import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.document.annotation.AnnotationService; import org.raddatz.familienarchiv.document.transcription.TranscriptionBlockQueryService; -import org.raddatz.familienarchiv.document.DocumentSearchItem; +import org.raddatz.familienarchiv.document.DocumentListItem; import org.raddatz.familienarchiv.document.DocumentSearchResult; import org.raddatz.familienarchiv.document.DocumentSort; import org.raddatz.familienarchiv.document.DocumentUpdateDTO; @@ -1444,7 +1444,7 @@ class DocumentServiceTest { 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"); + assertThat(result.items().get(0).sender().getLastName()).isEqualTo("L050"); } @Test @@ -1565,7 +1565,7 @@ class DocumentServiceTest { 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"); + assertThat(result.items()).extracting(DocumentListItem::title).containsExactly("Has Sender", "No Sender"); } // ─── searchDocuments — RECEIVER sort, empty receivers ─────────────────────── @@ -1584,7 +1584,7 @@ class DocumentServiceTest { DocumentSearchResult result = documentService.searchDocuments( null, null, null, null, null, null, null, null, DocumentSort.RECEIVER, "asc", null, UNPAGED); - assertThat(result.items()).extracting(item -> item.document().getTitle()) + assertThat(result.items()).extracting(DocumentListItem::title) .containsExactly("Has Receiver", "No Receivers"); } @@ -1607,7 +1607,7 @@ class DocumentServiceTest { 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()) + assertThat(result.items()).extracting(DocumentListItem::title) .containsExactly("smith doc", "Null lastname doc"); } -- 2.49.1 From 41b205becc7929c7afa03526e1670cff1493b585 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 22 May 2026 17:49:01 +0200 Subject: [PATCH 2/5] test(document): add LazyInit guard + detail regression tests; prune Document.list graph Remove trainingLabels from Document.list entity graph now that DocumentListItem does not touch that association. Integration tests guard against future LazyInitializationException regressions and confirm Document.full still loads trainingLabels for the detail endpoint. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/document/Document.java | 3 +- .../DocumentListItemIntegrationTest.java | 98 +++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java b/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java index 387e3547..71c6dead 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java @@ -31,8 +31,7 @@ import java.util.UUID; @NamedEntityGraph(name = "Document.list", attributeNodes = { @NamedAttributeNode("sender"), @NamedAttributeNode("receivers"), - @NamedAttributeNode("tags"), - @NamedAttributeNode("trainingLabels") + @NamedAttributeNode("tags") }) @Entity @Table(name = "documents") diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java new file mode 100644 index 00000000..485b865e --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java @@ -0,0 +1,98 @@ +package org.raddatz.familienarchiv.document; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.raddatz.familienarchiv.PostgresContainerConfig; +import org.raddatz.familienarchiv.audit.AuditLogQueryService; +import org.raddatz.familienarchiv.ocr.TrainingLabel; +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.context.ActiveProfiles; +import org.springframework.test.context.bean.override.mockito.MockitoBean; +import software.amazon.awssdk.services.s3.S3Client; + +import java.util.HashSet; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +/** + * AC #2: Document with trainingLabels does not cause LazyInitializationException in search. + * AC #3: Detail API still returns trainingLabels after the Document.list graph change. + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) +@ActiveProfiles("test") +@Import(PostgresContainerConfig.class) +class DocumentListItemIntegrationTest { + + @MockitoBean + S3Client s3Client; + + @MockitoBean + AuditLogQueryService auditLogQueryService; + + @Autowired + DocumentRepository documentRepository; + + @Autowired + DocumentService documentService; + + @AfterEach + void cleanup() { + documentRepository.deleteAll(); + } + + @Test + void search_doesNotThrow_whenDocumentHasTrainingLabels() { + documentRepository.save(Document.builder() + .title("Kurrent Brief") + .originalFilename("kurrent.pdf") + .status(DocumentStatus.UPLOADED) + .trainingLabels(new HashSet<>(Set.of(TrainingLabel.KURRENT_RECOGNITION))) + .build()); + + assertThatCode(() -> documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.DATE, "DESC", null, + PageRequest.of(0, 50))) + .doesNotThrowAnyException(); + } + + @Test + void search_returns_list_item_without_sensitive_fields_when_document_has_training_labels() { + documentRepository.save(Document.builder() + .title("Kurrent Brief") + .originalFilename("kurrent2.pdf") + .status(DocumentStatus.UPLOADED) + .trainingLabels(new HashSet<>(Set.of(TrainingLabel.KURRENT_RECOGNITION))) + .build()); + + DocumentSearchResult result = documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.DATE, "DESC", null, + PageRequest.of(0, 50)); + + assertThat(result.totalElements()).isGreaterThan(0); + DocumentListItem item = result.items().get(0); + assertThat(item.id()).isNotNull(); + assertThat(item.title()).isEqualTo("Kurrent Brief"); + } + + @Test + void detail_stillReturnsTrainingLabels() { + Document saved = documentRepository.save(Document.builder() + .title("Detail Test") + .originalFilename("detail_test.pdf") + .status(DocumentStatus.UPLOADED) + .trainingLabels(new HashSet<>(Set.of(TrainingLabel.KURRENT_RECOGNITION))) + .build()); + + // Document.full entity graph (used by findById) must still load trainingLabels + Document loaded = documentRepository.findById(saved.getId()).orElseThrow(); + + assertThat(loaded.getTrainingLabels()).containsExactly(TrainingLabel.KURRENT_RECOGNITION); + } +} -- 2.49.1 From 6583226d79617a991afcd53253f0d41336b6b284 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 22 May 2026 18:20:59 +0200 Subject: [PATCH 3/5] refactor(document): migrate frontend from DocumentSearchItem to flat DocumentListItem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All components, specs, and the generated API client now use the new DocumentListItem shape — flat access (item.title, item.sender) instead of the removed item.document.* nesting. Co-Authored-By: Claude Sonnet 4.6 --- .../lib/document/DocumentMultiSelect.svelte | 6 +- frontend/src/lib/document/DocumentRow.svelte | 6 +- .../lib/document/DocumentRow.svelte.spec.ts | 100 +++++------- .../lib/document/DocumentRow.svelte.test.ts | 56 ++++--- frontend/src/lib/generated/api.ts | 41 +++-- frontend/src/routes/DocumentList.svelte | 24 +-- .../src/routes/DocumentList.svelte.spec.ts | 153 ++++++++---------- frontend/src/routes/documents/+page.server.ts | 6 +- 8 files changed, 193 insertions(+), 199 deletions(-) diff --git a/frontend/src/lib/document/DocumentMultiSelect.svelte b/frontend/src/lib/document/DocumentMultiSelect.svelte index 4dabda60..4d2e42ba 100644 --- a/frontend/src/lib/document/DocumentMultiSelect.svelte +++ b/frontend/src/lib/document/DocumentMultiSelect.svelte @@ -5,7 +5,7 @@ import { clickOutside } from '$lib/shared/actions/clickOutside'; import { formatDate } from '$lib/shared/utils/date'; type Document = components['schemas']['Document']; -type DocumentSearchItem = components['schemas']['DocumentSearchItem']; +type DocumentListItem = components['schemas']['DocumentListItem']; interface Props { selectedDocuments?: Document[]; @@ -45,8 +45,8 @@ function handleInput() { try { const res = await fetch(`/api/documents/search?q=${encodeURIComponent(searchTerm)}&size=10`); if (res.ok) { - const body: { items: DocumentSearchItem[] } = await res.json(); - const docs = body.items.map((it) => it.document); + const body: { items: DocumentListItem[] } = await res.json(); + const docs = body.items as unknown as Document[]; results = docs.filter((d) => !selectedDocuments.some((s) => s.id === d.id)); } } catch { diff --git a/frontend/src/lib/document/DocumentRow.svelte b/frontend/src/lib/document/DocumentRow.svelte index 70c85ed3..903ed727 100644 --- a/frontend/src/lib/document/DocumentRow.svelte +++ b/frontend/src/lib/document/DocumentRow.svelte @@ -9,11 +9,11 @@ import ProgressRing from '$lib/shared/primitives/ProgressRing.svelte'; import ContributorStack from '$lib/shared/primitives/ContributorStack.svelte'; import DocumentThumbnail from './DocumentThumbnail.svelte'; -type DocumentSearchItem = components['schemas']['DocumentSearchItem']; +type DocumentListItem = components['schemas']['DocumentListItem']; -let { item, canWrite = false }: { item: DocumentSearchItem; canWrite?: boolean } = $props(); +let { item, canWrite = false }: { item: DocumentListItem; canWrite?: boolean } = $props(); -const doc = $derived(item.document); +const doc = $derived(item); const titleText = $derived(doc.title || doc.originalFilename); const titleOffsets = $derived(item.matchData?.titleOffsets ?? []); const titleSegments = $derived(applyOffsets(titleText, titleOffsets)); diff --git a/frontend/src/lib/document/DocumentRow.svelte.spec.ts b/frontend/src/lib/document/DocumentRow.svelte.spec.ts index 19b3c66a..79ce8913 100644 --- a/frontend/src/lib/document/DocumentRow.svelte.spec.ts +++ b/frontend/src/lib/document/DocumentRow.svelte.spec.ts @@ -14,24 +14,17 @@ afterEach(() => { bulkSelectionStore.clear(); }); -type DocumentSearchItem = components['schemas']['DocumentSearchItem']; +type DocumentListItem = components['schemas']['DocumentListItem']; -function makeItem(overrides: Partial = {}): DocumentSearchItem { +function makeItem(overrides: Partial = {}): DocumentListItem { return { - document: { - id: '1', - title: 'Testbrief', - originalFilename: 'testbrief.pdf', - status: 'UPLOADED', - documentDate: '2024-03-15', - sender: null, - receivers: [], - tags: [], - createdAt: '2024-01-01T00:00:00Z', - updatedAt: '2024-01-01T00:00:00Z', - metadataComplete: false, - scriptType: 'UNKNOWN' - }, + id: '1', + title: 'Testbrief', + originalFilename: 'testbrief.pdf', + documentDate: '2024-03-15', + sender: undefined, + receivers: [], + tags: [], matchData: { titleOffsets: [], senderMatched: false, @@ -55,14 +48,14 @@ describe('DocumentRow – title', () => { }); it('falls back to originalFilename when title is null', async () => { - const item = makeItem({ document: { ...makeItem().document, title: null } }); + const item = makeItem({ title: null as unknown as string }); render(DocumentRow, { item }); await expect.element(page.getByRole('heading', { name: 'testbrief.pdf' })).toBeInTheDocument(); }); it('renders a mark element for highlighted title offsets', async () => { const item = makeItem({ - document: { ...makeItem().document, title: 'Brief an Anna' }, + title: 'Brief an Anna', matchData: { titleOffsets: [{ start: 0, length: 5 }], senderMatched: false, @@ -109,9 +102,12 @@ describe('DocumentRow – snippet', () => { describe('DocumentRow – sender', () => { it('shows sender display name', async () => { const item = makeItem({ - document: { - ...makeItem().document, - sender: { id: 's1', displayName: 'Großmutter Maria' } + sender: { + id: 's1', + lastName: 'Maria', + displayName: 'Großmutter Maria', + personType: 'PERSON', + familyMember: false } }); render(DocumentRow, { item }); @@ -126,9 +122,12 @@ describe('DocumentRow – sender', () => { it('highlights the sender when senderMatched is true', async () => { const item = makeItem({ - document: { - ...makeItem().document, - sender: { id: 's1', displayName: 'Großmutter Maria' } + sender: { + id: 's1', + lastName: 'Maria', + displayName: 'Großmutter Maria', + personType: 'PERSON', + familyMember: false }, matchData: { ...makeItem().matchData, @@ -142,10 +141,15 @@ describe('DocumentRow – sender', () => { it('highlights a receiver when matchedReceiverIds includes its id', async () => { const item = makeItem({ - document: { - ...makeItem().document, - receivers: [{ id: 'r1', displayName: 'Onkel Karl' }] - }, + receivers: [ + { + id: 'r1', + lastName: 'Karl', + displayName: 'Onkel Karl', + personType: 'PERSON', + familyMember: false + } + ], matchData: { ...makeItem().matchData, matchedReceiverIds: ['r1'] @@ -162,10 +166,7 @@ describe('DocumentRow – sender', () => { describe('DocumentRow – summary', () => { it('renders the document summary when present', async () => { const item = makeItem({ - document: { - ...makeItem().document, - summary: 'Brief von Eugenie über die Heimreise aus dem Süden.' - } + summary: 'Brief von Eugenie über die Heimreise aus dem Süden.' }); render(DocumentRow, { item }); await expect @@ -180,7 +181,7 @@ describe('DocumentRow – summary', () => { it('applies summary search-match highlight via summaryOffsets', async () => { const item = makeItem({ - document: { ...makeItem().document, summary: 'Brief über Menton' }, + summary: 'Brief über Menton', matchData: { ...makeItem().matchData, summaryOffsets: [{ start: 11, length: 6 }] @@ -196,25 +197,19 @@ describe('DocumentRow – summary', () => { describe('DocumentRow – archive chips', () => { it('renders the archive box chip when set', async () => { - const item = makeItem({ - document: { ...makeItem().document, archiveBox: 'K3' } - }); + const item = makeItem({ archiveBox: 'K3' }); render(DocumentRow, { item }); await expect.element(page.getByText('K3')).toBeInTheDocument(); }); it('renders the archive folder chip when set', async () => { - const item = makeItem({ - document: { ...makeItem().document, archiveFolder: 'Mappe A' } - }); + const item = makeItem({ archiveFolder: 'Mappe A' }); render(DocumentRow, { item }); await expect.element(page.getByText('Mappe A')).toBeInTheDocument(); }); it('renders the location chip when meta_location is set', async () => { - const item = makeItem({ - document: { ...makeItem().document, location: 'Berlin' } - }); + const item = makeItem({ location: 'Berlin' }); render(DocumentRow, { item }); await expect.element(page.getByText('Berlin')).toBeInTheDocument(); }); @@ -225,10 +220,7 @@ describe('DocumentRow – archive chips', () => { describe('DocumentRow – tags', () => { it('renders tag buttons', async () => { const item = makeItem({ - document: { - ...makeItem().document, - tags: [{ id: 't1', name: 'Familie', color: null, parentId: null }] - } + tags: [{ id: 't1', name: 'Familie' }] }); render(DocumentRow, { item }); await expect.element(page.getByRole('button', { name: 'Familie' })).toBeInTheDocument(); @@ -236,10 +228,7 @@ describe('DocumentRow – tags', () => { it('navigates to /documents?tag=… on tag click', async () => { const item = makeItem({ - document: { - ...makeItem().document, - tags: [{ id: 't1', name: 'Urlaub & Reise', color: null, parentId: null }] - } + tags: [{ id: 't1', name: 'Urlaub & Reise' }] }); render(DocumentRow, { item }); // Tailwind CSS isn't loaded in the vitest-browser client project, so the @@ -255,10 +244,7 @@ describe('DocumentRow – tags', () => { it('tag click does not navigate to the document detail page', async () => { const item = makeItem({ - document: { - ...makeItem().document, - tags: [{ id: 't2', name: 'Familie', color: null, parentId: null }] - } + tags: [{ id: 't2', name: 'Familie' }] }); render(DocumentRow, { item }); const before = window.location.href; @@ -281,7 +267,7 @@ describe('DocumentRow – bulk selection checkbox', () => { }); it('checkbox aria-label includes the document title', async () => { - const item = makeItem({ document: { ...makeItem().document, title: 'Brief an Anna' } }); + const item = makeItem({ title: 'Brief an Anna' }); render(DocumentRow, { item, canWrite: true }); await expect .element(page.getByRole('checkbox', { name: /Brief an Anna/i })) @@ -289,7 +275,7 @@ describe('DocumentRow – bulk selection checkbox', () => { }); it('toggling the checkbox calls bulkSelectionStore.toggle', async () => { - const item = makeItem({ document: { ...makeItem().document, id: 'doc-42' } }); + const item = makeItem({ id: 'doc-42' }); render(DocumentRow, { item, canWrite: true }); expect(bulkSelectionStore.has('doc-42')).toBe(false); @@ -300,7 +286,7 @@ describe('DocumentRow – bulk selection checkbox', () => { it('checked state mirrors the store', async () => { bulkSelectionStore.add('doc-99'); - const item = makeItem({ document: { ...makeItem().document, id: 'doc-99' } }); + const item = makeItem({ id: 'doc-99' }); render(DocumentRow, { item, canWrite: true }); await expect.element(page.getByRole('checkbox')).toBeChecked(); }); diff --git a/frontend/src/lib/document/DocumentRow.svelte.test.ts b/frontend/src/lib/document/DocumentRow.svelte.test.ts index 704428bc..58ac9bbd 100644 --- a/frontend/src/lib/document/DocumentRow.svelte.test.ts +++ b/frontend/src/lib/document/DocumentRow.svelte.test.ts @@ -20,10 +20,31 @@ const { default: DocumentRow } = await import('./DocumentRow.svelte'); afterEach(cleanup); -const sender = { id: 's1', displayName: 'Anna Schmidt' }; -const receiver = { id: 'r1', displayName: 'Bert Meier' }; +const sender = { + id: 's1', + lastName: 'Schmidt', + displayName: 'Anna Schmidt', + personType: 'PERSON' as const, + familyMember: false +}; +const receiver = { + id: 'r1', + lastName: 'Meier', + displayName: 'Bert Meier', + personType: 'PERSON' as const, + familyMember: false +}; -const makeDoc = (overrides: Record = {}) => ({ +const emptyMatchData = { + titleOffsets: [], + senderMatched: false, + matchedReceiverIds: [], + matchedTagIds: [], + snippetOffsets: [], + summaryOffsets: [] +}; + +const baseItem = (overrides: Record = {}) => ({ id: 'd1', title: 'Brief 1923', originalFilename: 'b.pdf', @@ -31,20 +52,14 @@ const makeDoc = (overrides: Record = {}) => ({ sender, receivers: [receiver], tags: [], - thumbnailUrl: null, - contentType: 'application/pdf', - summary: null, - archiveBox: null, - archiveFolder: null, - location: null, - ...overrides -}); - -const baseItem = (docOverrides: Record = {}) => ({ - document: makeDoc(docOverrides), - matchData: null, + summary: undefined, + archiveBox: undefined, + archiveFolder: undefined, + location: undefined, + matchData: emptyMatchData, completionPercentage: 0, - contributors: [] + contributors: [], + ...overrides }); describe('DocumentRow', () => { @@ -121,12 +136,9 @@ describe('DocumentRow', () => { it('renders the snippet when matchData provides a transcriptionSnippet', async () => { render(DocumentRow, { props: { - item: { - document: makeDoc(), - matchData: { transcriptionSnippet: 'Hello world snippet' }, - completionPercentage: 50, - contributors: [] - } + item: baseItem({ + matchData: { ...emptyMatchData, transcriptionSnippet: 'Hello world snippet' } + }) } }); diff --git a/frontend/src/lib/generated/api.ts b/frontend/src/lib/generated/api.ts index 764adce9..9a9a5408 100644 --- a/frontend/src/lib/generated/api.ts +++ b/frontend/src/lib/generated/api.ts @@ -2068,12 +2068,20 @@ export interface components { }; ImportStatus: { /** @enum {string} */ - state?: "IDLE" | "RUNNING" | "DONE" | "FAILED"; - statusCode?: string; + state: "IDLE" | "RUNNING" | "DONE" | "FAILED"; + statusCode: string; /** Format: int32 */ - processed?: number; + processed: number; + skippedFiles: components["schemas"]["SkippedFile"][]; /** Format: date-time */ startedAt?: string; + /** Format: int32 */ + skipped?: number; + }; + SkippedFile: { + filename: string; + /** @enum {string} */ + reason: "INVALID_FILENAME_PATH_TRAVERSAL" | "INVALID_PDF_SIGNATURE" | "FILE_READ_ERROR" | "ALREADY_EXISTS" | "S3_UPLOAD_FAILED"; }; BackfillStatus: { /** @enum {string} */ @@ -2197,10 +2205,10 @@ export interface components { totalStories: number; }; PersonSummaryDTO: { - title?: string; /** Format: uuid */ id?: string; displayName?: string; + title?: string; firstName?: string; lastName?: string; /** Format: int64 */ @@ -2307,14 +2315,14 @@ export interface components { /** Format: int32 */ totalPages?: number; pageable?: components["schemas"]["PageableObject"]; + first?: boolean; + last?: boolean; /** Format: int32 */ size?: number; content?: components["schemas"]["NotificationDTO"][]; /** Format: int32 */ number?: number; sort?: components["schemas"]["SortObject"]; - first?: boolean; - last?: boolean; /** Format: int32 */ numberOfElements?: number; empty?: boolean; @@ -2380,15 +2388,28 @@ export interface components { /** Format: int32 */ totalPages?: number; }; - DocumentSearchItem: { - document: components["schemas"]["Document"]; - matchData: components["schemas"]["SearchMatchData"]; + DocumentListItem: { + /** Format: uuid */ + id: string; + title: string; + originalFilename: string; + thumbnailUrl?: string; + /** Format: date */ + documentDate?: string; + sender?: components["schemas"]["Person"]; + receivers: components["schemas"]["Person"][]; + tags: components["schemas"]["Tag"][]; + archiveBox?: string; + archiveFolder?: string; + location?: string; + summary?: string; /** Format: int32 */ completionPercentage: number; contributors: components["schemas"]["ActivityActorDTO"][]; + matchData: components["schemas"]["SearchMatchData"]; }; DocumentSearchResult: { - items: components["schemas"]["DocumentSearchItem"][]; + items: components["schemas"]["DocumentListItem"][]; /** Format: int64 */ totalElements: number; /** Format: int32 */ diff --git a/frontend/src/routes/DocumentList.svelte b/frontend/src/routes/DocumentList.svelte index f2bd1c18..8d29c870 100644 --- a/frontend/src/routes/DocumentList.svelte +++ b/frontend/src/routes/DocumentList.svelte @@ -5,7 +5,7 @@ import DocumentRow from '$lib/document/DocumentRow.svelte'; import { SvelteMap } from 'svelte/reactivity'; import type { components } from '$lib/generated/api'; -type DocumentSearchItem = components['schemas']['DocumentSearchItem']; +type DocumentListItem = components['schemas']['DocumentListItem']; type SortMode = 'DATE' | 'TITLE' | 'SENDER' | 'RECEIVER' | 'UPLOAD_DATE' | 'RELEVANCE'; @@ -17,7 +17,7 @@ let { q = '', sort = 'DATE' }: { - items: DocumentSearchItem[]; + items: DocumentListItem[]; canWrite: boolean; error?: string | null; total?: number; @@ -31,10 +31,10 @@ const groups = $derived.by(() => { return groupByYear(items); }); -function groupByYear(docItems: DocumentSearchItem[]) { - const map = new SvelteMap(); +function groupByYear(docItems: DocumentListItem[]) { + const map = new SvelteMap(); for (const item of docItems) { - const label = item.document.documentDate?.substring(0, 4) ?? m.docs_group_undated(); + const label = item.documentDate?.substring(0, 4) ?? m.docs_group_undated(); const bucket = map.get(label); if (bucket) bucket.push(item); else map.set(label, [item]); @@ -42,10 +42,10 @@ function groupByYear(docItems: DocumentSearchItem[]) { return Array.from(map.entries()).map(([label, groupItems]) => ({ label, items: groupItems })); } -function groupBySender(docItems: DocumentSearchItem[]) { - const map = new SvelteMap(); +function groupBySender(docItems: DocumentListItem[]) { + const map = new SvelteMap(); for (const item of docItems) { - const label = item.document.sender?.displayName ?? m.docs_group_unknown_sender(); + const label = item.sender?.displayName ?? m.docs_group_unknown_sender(); const bucket = map.get(label); if (bucket) bucket.push(item); else map.set(label, [item]); @@ -53,10 +53,10 @@ function groupBySender(docItems: DocumentSearchItem[]) { return Array.from(map.entries()).map(([label, groupItems]) => ({ label, items: groupItems })); } -function groupByReceiver(docItems: DocumentSearchItem[]) { - const map = new SvelteMap(); +function groupByReceiver(docItems: DocumentListItem[]) { + const map = new SvelteMap(); for (const item of docItems) { - const receivers = item.document.receivers ?? []; + const receivers = item.receivers ?? []; const labels = receivers.length > 0 ? receivers.map((r) => r.displayName) @@ -99,7 +99,7 @@ function groupByReceiver(docItems: DocumentSearchItem[]) { >
    - {#each group.items as item (group.label + '-' + item.document.id)} + {#each group.items as item (group.label + '-' + item.id)} {/each}
diff --git a/frontend/src/routes/DocumentList.svelte.spec.ts b/frontend/src/routes/DocumentList.svelte.spec.ts index 19a11676..a21be4b4 100644 --- a/frontend/src/routes/DocumentList.svelte.spec.ts +++ b/frontend/src/routes/DocumentList.svelte.spec.ts @@ -8,24 +8,17 @@ vi.mock('$app/navigation', () => ({ goto: vi.fn() })); afterEach(() => cleanup()); -type DocumentSearchItem = components['schemas']['DocumentSearchItem']; +type DocumentListItem = components['schemas']['DocumentListItem']; -function makeItem(overrides: Partial = {}): DocumentSearchItem { +function makeItem(overrides: Partial = {}): DocumentListItem { return { - document: { - id: '1', - title: 'Testbrief', - originalFilename: 'testbrief.pdf', - status: 'UPLOADED', - documentDate: '2024-03-15', - sender: undefined, - receivers: [], - tags: [], - createdAt: '2024-01-01T00:00:00Z', - updatedAt: '2024-01-01T00:00:00Z', - metadataComplete: false, - scriptType: 'UNKNOWN' - }, + id: '1', + title: 'Testbrief', + originalFilename: 'testbrief.pdf', + documentDate: '2024-03-15', + sender: undefined, + receivers: [], + tags: [], matchData: { titleOffsets: [], senderMatched: false, @@ -75,8 +68,8 @@ describe('DocumentList – empty state', () => { describe('DocumentList – year grouping', () => { it('groups documents by year into separate cards', async () => { const items = [ - makeItem({ document: { ...makeItem().document, id: '1', documentDate: '1923-04-12' } }), - makeItem({ document: { ...makeItem().document, id: '2', documentDate: '1965-08-03' } }) + makeItem({ id: '1', documentDate: '1923-04-12' }), + makeItem({ id: '2', documentDate: '1965-08-03' }) ]; render(DocumentList, { ...baseProps, items, total: 2 }); const groupCards = page.getByTestId('group-card'); @@ -85,17 +78,15 @@ describe('DocumentList – year grouping', () => { }); it('uses undated label for items with no documentDate', async () => { - const items = [ - makeItem({ document: { ...makeItem().document, id: '1', documentDate: undefined } }) - ]; + const items = [makeItem({ id: '1', documentDate: undefined })]; render(DocumentList, { ...baseProps, items, total: 1 }); await expect.element(page.getByText('Undatiert')).toBeInTheDocument(); }); it('single year renders one group-card', async () => { const items = [ - makeItem({ document: { ...makeItem().document, id: '1', documentDate: '1938-01-01' } }), - makeItem({ document: { ...makeItem().document, id: '2', documentDate: '1938-06-15' } }) + makeItem({ id: '1', documentDate: '1938-01-01' }), + makeItem({ id: '2', documentDate: '1938-06-15' }) ]; render(DocumentList, { ...baseProps, items, total: 2 }); const groupCards = page.getByTestId('group-card'); @@ -108,9 +99,7 @@ describe('DocumentList – year grouping', () => { describe('DocumentList – sort fallback', () => { it('falls back to year grouping when sort is not SENDER or RECEIVER', async () => { - const items = [ - makeItem({ document: { ...makeItem().document, id: '1', documentDate: '2024-03-15' } }) - ]; + const items = [makeItem({ id: '1', documentDate: '2024-03-15' })]; render(DocumentList, { ...baseProps, items, total: 1, sort: 'TITLE' }); await expect .element(page.getByTestId('group-header').filter({ hasText: '2024' })) @@ -124,29 +113,23 @@ describe('DocumentList – sender grouping', () => { it('groups by sender displayName when sort is SENDER', async () => { const items = [ makeItem({ - document: { - ...makeItem().document, - id: '1', - sender: { - id: 's1', - lastName: 'Mustermann', - displayName: 'Max Mustermann', - personType: 'PERSON', - familyMember: false - } + id: '1', + sender: { + id: 's1', + lastName: 'Mustermann', + displayName: 'Max Mustermann', + personType: 'PERSON', + familyMember: false } }), makeItem({ - document: { - ...makeItem().document, - id: '2', - sender: { - id: 's2', - lastName: 'Musterfrau', - displayName: 'Anna Musterfrau', - personType: 'PERSON', - familyMember: false - } + id: '2', + sender: { + id: 's2', + lastName: 'Musterfrau', + displayName: 'Anna Musterfrau', + personType: 'PERSON', + familyMember: false } }) ]; @@ -167,10 +150,7 @@ describe('DocumentList – sender grouping', () => { personType: 'PERSON' as const, familyMember: false }; - const items = [ - makeItem({ document: { ...makeItem().document, id: '1', sender } }), - makeItem({ document: { ...makeItem().document, id: '2', sender } }) - ]; + const items = [makeItem({ id: '1', sender }), makeItem({ id: '2', sender })]; render(DocumentList, { ...baseProps, items, total: 2, sort: 'SENDER' }); const cards = page.getByTestId('group-card'); await expect.element(cards.first()).toBeInTheDocument(); @@ -178,7 +158,7 @@ describe('DocumentList – sender grouping', () => { }); it('places items with no sender under fallback label', async () => { - const items = [makeItem({ document: { ...makeItem().document, id: '1', sender: undefined } })]; + const items = [makeItem({ id: '1', sender: undefined })]; render(DocumentList, { ...baseProps, items, total: 1, sort: 'SENDER' }); await expect.element(page.getByText('Unbekannter Absender')).toBeInTheDocument(); }); @@ -190,19 +170,16 @@ describe('DocumentList – receiver grouping', () => { it('groups by receiver displayName when sort is RECEIVER', async () => { const items = [ makeItem({ - document: { - ...makeItem().document, - id: '1', - receivers: [ - { - id: 'r1', - lastName: 'Brandt', - displayName: 'Felix Brandt', - personType: 'PERSON', - familyMember: false - } - ] - } + id: '1', + receivers: [ + { + id: 'r1', + lastName: 'Brandt', + displayName: 'Felix Brandt', + personType: 'PERSON', + familyMember: false + } + ] }) ]; render(DocumentList, { ...baseProps, items, total: 1, sort: 'RECEIVER' }); @@ -214,27 +191,24 @@ describe('DocumentList – receiver grouping', () => { it('duplicates a document into each receiver group', async () => { const items = [ makeItem({ - document: { - ...makeItem().document, - id: '1', - title: 'Rundbriefchen', - receivers: [ - { - id: 'r1', - lastName: 'Brandt', - displayName: 'Felix Brandt', - personType: 'PERSON', - familyMember: false - }, - { - id: 'r2', - lastName: 'Meier', - displayName: 'Hans Meier', - personType: 'PERSON', - familyMember: false - } - ] - } + id: '1', + title: 'Rundbriefchen', + receivers: [ + { + id: 'r1', + lastName: 'Brandt', + displayName: 'Felix Brandt', + personType: 'PERSON', + familyMember: false + }, + { + id: 'r2', + lastName: 'Meier', + displayName: 'Hans Meier', + personType: 'PERSON', + familyMember: false + } + ] }) ]; render(DocumentList, { ...baseProps, items, total: 1, sort: 'RECEIVER' }); @@ -249,7 +223,7 @@ describe('DocumentList – receiver grouping', () => { }); it('places items with no receivers under fallback label', async () => { - const items = [makeItem({ document: { ...makeItem().document, id: '1', receivers: [] } })]; + const items = [makeItem({ id: '1', receivers: [] })]; render(DocumentList, { ...baseProps, items, total: 1, sort: 'RECEIVER' }); await expect.element(page.getByText('Unbekannter Empfänger')).toBeInTheDocument(); }); @@ -261,7 +235,7 @@ describe('DocumentList – DocumentRow delegation', () => { it('shows transcription snippet when matchData has one', async () => { const items = [ makeItem({ - document: { ...makeItem().document, id: 'doc1' }, + id: 'doc1', matchData: { transcriptionSnippet: 'Er schrieb einen langen Brief', titleOffsets: [], @@ -278,7 +252,7 @@ describe('DocumentList – DocumentRow delegation', () => { }); it('does not render snippet when matchData has no transcription snippet', async () => { - const items = [makeItem({ document: { ...makeItem().document, id: 'doc1' } })]; + const items = [makeItem({ id: 'doc1' })]; render(DocumentList, { ...baseProps, items, total: 1 }); await expect.element(page.getByTestId('search-snippet')).not.toBeInTheDocument(); }); @@ -286,7 +260,8 @@ describe('DocumentList – DocumentRow delegation', () => { it('renders mark for title highlight when titleOffsets present', async () => { const items = [ makeItem({ - document: { ...makeItem().document, id: 'doc1', title: 'Brief an Anna' }, + id: 'doc1', + title: 'Brief an Anna', matchData: { titleOffsets: [{ start: 0, length: 5 }], // "Brief" senderMatched: false, diff --git a/frontend/src/routes/documents/+page.server.ts b/frontend/src/routes/documents/+page.server.ts index 64f270e5..16186611 100644 --- a/frontend/src/routes/documents/+page.server.ts +++ b/frontend/src/routes/documents/+page.server.ts @@ -20,7 +20,7 @@ async function resolvePersonName( } } -type DocumentSearchItem = components['schemas']['DocumentSearchItem']; +type DocumentListItem = components['schemas']['DocumentListItem']; const VALID_SORTS = ['DATE', 'TITLE', 'SENDER', 'RECEIVER', 'UPLOAD_DATE', 'RELEVANCE'] as const; type ValidSort = (typeof VALID_SORTS)[number]; @@ -77,7 +77,7 @@ export async function load({ url, fetch }) { ]); } catch { return { - items: [] as DocumentSearchItem[], + items: [] as DocumentListItem[], totalElements: 0, pageNumber: 0, pageSize: PAGE_SIZE, @@ -107,7 +107,7 @@ export async function load({ url, fetch }) { : null; return { - items: (result.data?.items ?? []) as DocumentSearchItem[], + items: (result.data?.items ?? []) as DocumentListItem[], totalElements: result.data?.totalElements ?? 0, pageNumber: result.data?.pageNumber ?? page, pageSize: result.data?.pageSize ?? PAGE_SIZE, -- 2.49.1 From 627fc44d999a2d26f13013a8b3df417e4555cc92 Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 22 May 2026 19:14:44 +0200 Subject: [PATCH 4/5] fix(document): fix test regressions from DocumentListItem migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use documentService.getDocumentById() in detail_stillReturnsTrainingLabels so the Document.full entity graph eager-loads trainingLabels - Flatten makeItem() factory in DocumentList.svelte.test.ts (nested document: {} overrides broke item.id / item.documentDate access) - Remove { document: {} } wrapper from DocumentMultiSelect.svelte.spec.ts mock responses — component now reads body.items directly as flat items - Flatten single nested item in page.svelte.test.ts document list test Co-Authored-By: Claude Sonnet 4.6 --- .../DocumentListItemIntegrationTest.java | 4 +- .../DocumentMultiSelect.svelte.spec.ts | 7 +- .../src/routes/DocumentList.svelte.test.ts | 79 +++++++++++++------ .../src/routes/documents/page.svelte.test.ts | 15 ++-- 4 files changed, 67 insertions(+), 38 deletions(-) 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 485b865e..4c532882 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentListItemIntegrationTest.java @@ -90,8 +90,8 @@ class DocumentListItemIntegrationTest { .trainingLabels(new HashSet<>(Set.of(TrainingLabel.KURRENT_RECOGNITION))) .build()); - // Document.full entity graph (used by findById) must still load trainingLabels - Document loaded = documentRepository.findById(saved.getId()).orElseThrow(); + // Document.full entity graph (used by getDocumentById) must still load trainingLabels + Document loaded = documentService.getDocumentById(saved.getId()); assertThat(loaded.getTrainingLabels()).containsExactly(TrainingLabel.KURRENT_RECOGNITION); } diff --git a/frontend/src/lib/document/DocumentMultiSelect.svelte.spec.ts b/frontend/src/lib/document/DocumentMultiSelect.svelte.spec.ts index b5650fbd..55e75853 100644 --- a/frontend/src/lib/document/DocumentMultiSelect.svelte.spec.ts +++ b/frontend/src/lib/document/DocumentMultiSelect.svelte.spec.ts @@ -22,7 +22,7 @@ function mockSearchResponse(items: ReturnType[]) { 'fetch', vi.fn().mockResolvedValue({ ok: true, - json: vi.fn().mockResolvedValue({ items: items.map((document) => ({ document })) }) + json: vi.fn().mockResolvedValue({ items }) }) ); } @@ -91,10 +91,7 @@ describe('DocumentMultiSelect — search and select', () => { const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: vi.fn().mockResolvedValue({ - items: [ - { document: docFactory('d1', 'Already attached') }, - { document: docFactory('d2', 'Not attached') } - ] + items: [docFactory('d1', 'Already attached'), docFactory('d2', 'Not attached')] }) }); vi.stubGlobal('fetch', fetchMock); diff --git a/frontend/src/routes/DocumentList.svelte.test.ts b/frontend/src/routes/DocumentList.svelte.test.ts index 782a60e6..8d1c7b5c 100644 --- a/frontend/src/routes/DocumentList.svelte.test.ts +++ b/frontend/src/routes/DocumentList.svelte.test.ts @@ -20,29 +20,46 @@ const { default: DocumentList } = await import('./DocumentList.svelte'); afterEach(cleanup); -const sender = { id: 's1', displayName: 'Anna Schmidt' }; -const receiver = { id: 'r1', displayName: 'Bert Meier' }; +const sender = { + id: 's1', + lastName: 'Schmidt', + displayName: 'Anna Schmidt', + personType: 'PERSON' as const, + familyMember: false +}; +const receiver = { + id: 'r1', + lastName: 'Meier', + displayName: 'Bert Meier', + personType: 'PERSON' as const, + familyMember: false +}; + +const emptyMatchData = { + titleOffsets: [], + senderMatched: false, + matchedReceiverIds: [], + matchedTagIds: [], + snippetOffsets: [], + summaryOffsets: [] +}; const makeItem = (overrides: Record = {}) => ({ - document: { - id: 'd1', - title: 'Brief 1923', - originalFilename: 'b.pdf', - documentDate: '1923-04-15', - sender, - receivers: [receiver], - tags: [], - thumbnailUrl: null, - contentType: 'application/pdf', - summary: null, - archiveBox: null, - archiveFolder: null, - location: null, - ...overrides - }, - matchData: null, + id: 'd1', + title: 'Brief 1923', + originalFilename: 'b.pdf', + documentDate: '1923-04-15', + sender, + receivers: [receiver], + tags: [], + summary: undefined, + archiveBox: undefined, + archiveFolder: undefined, + location: undefined, + matchData: emptyMatchData, completionPercentage: 0, - contributors: [] + contributors: [], + ...overrides }); describe('DocumentList', () => { @@ -87,8 +104,26 @@ describe('DocumentList', () => { render(DocumentList, { props: { items: [ - makeItem({ id: 'd1', sender: { id: 's1', displayName: 'Anna Schmidt' } }), - makeItem({ id: 'd2', sender: { id: 's2', displayName: 'Bert Meier' } }) + makeItem({ + id: 'd1', + sender: { + id: 's1', + lastName: 'Schmidt', + displayName: 'Anna Schmidt', + personType: 'PERSON', + familyMember: false + } + }), + makeItem({ + id: 'd2', + sender: { + id: 's2', + lastName: 'Meier', + displayName: 'Bert Meier', + personType: 'PERSON', + familyMember: false + } + }) ], canWrite: false, sort: 'SENDER' as const diff --git a/frontend/src/routes/documents/page.svelte.test.ts b/frontend/src/routes/documents/page.svelte.test.ts index 5731ea18..bf7ec810 100644 --- a/frontend/src/routes/documents/page.svelte.test.ts +++ b/frontend/src/routes/documents/page.svelte.test.ts @@ -140,15 +140,12 @@ describe('documents/+ page', () => { data: baseData({ items: [ { - document: { - id: 'd1', - title: 'Brief 1899', - status: 'TRANSCRIBED', - documentDate: '1899-04-14', - summary: '', - originalFilename: 'b1.pdf', - receivers: [] - }, + id: 'd1', + title: 'Brief 1899', + documentDate: '1899-04-14', + originalFilename: 'b1.pdf', + receivers: [], + tags: [], matchData: { titleOffsets: [], senderMatched: false, -- 2.49.1 From 8e9e3bba06eb54cddf009ff05915f52af33b881a Mon Sep 17 00:00:00 2001 From: Marcel Date: Fri, 22 May 2026 19:27:31 +0200 Subject: [PATCH 5/5] refactor(document): address review concerns from PR #660 - Restore JavaDoc on DocumentSearchResult.of() and .paged() factory methods - Remove redundant null guards on @Builder.Default collections in toListItem() - Map DocumentListItem fields explicitly in DocumentMultiSelect before cast - Add DocumentListItem required fields to docFactory in spec Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentSearchResult.java | 8 ++++++++ .../familienarchiv/document/DocumentService.java | 4 ++-- .../src/lib/document/DocumentMultiSelect.svelte | 6 +++++- .../document/DocumentMultiSelect.svelte.spec.ts | 14 +++++++++++++- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchResult.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchResult.java index 25b39184..b04f7fa2 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchResult.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentSearchResult.java @@ -17,11 +17,19 @@ public record DocumentSearchResult( @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) { 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<T> 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); 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 0a290c5b..cfbbf848 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -759,8 +759,8 @@ public class DocumentService { doc.getThumbnailUrl(), doc.getDocumentDate(), doc.getSender(), - doc.getReceivers() != null ? List.copyOf(doc.getReceivers()) : List.of(), - doc.getTags() != null ? List.copyOf(doc.getTags()) : List.of(), + List.copyOf(doc.getReceivers()), + List.copyOf(doc.getTags()), doc.getArchiveBox(), doc.getArchiveFolder(), doc.getLocation(), diff --git a/frontend/src/lib/document/DocumentMultiSelect.svelte b/frontend/src/lib/document/DocumentMultiSelect.svelte index 4d2e42ba..0196544b 100644 --- a/frontend/src/lib/document/DocumentMultiSelect.svelte +++ b/frontend/src/lib/document/DocumentMultiSelect.svelte @@ -46,7 +46,11 @@ function handleInput() { const res = await fetch(`/api/documents/search?q=${encodeURIComponent(searchTerm)}&size=10`); if (res.ok) { const body: { items: DocumentListItem[] } = await res.json(); - const docs = body.items as unknown as Document[]; + const docs = body.items.map((it) => ({ + id: it.id, + title: it.title, + documentDate: it.documentDate + })) as unknown as Document[]; results = docs.filter((d) => !selectedDocuments.some((s) => s.id === d.id)); } } catch { diff --git a/frontend/src/lib/document/DocumentMultiSelect.svelte.spec.ts b/frontend/src/lib/document/DocumentMultiSelect.svelte.spec.ts index 55e75853..6514ab55 100644 --- a/frontend/src/lib/document/DocumentMultiSelect.svelte.spec.ts +++ b/frontend/src/lib/document/DocumentMultiSelect.svelte.spec.ts @@ -10,7 +10,19 @@ const docFactory = (id: string, title: string, date = '1880-01-01') => ({ title, documentDate: date, originalFilename: `${title}.pdf`, - status: 'UPLOADED', + receivers: [], + tags: [], + completionPercentage: 0, + contributors: [], + matchData: { + titleOffsets: [], + senderMatched: false, + matchedReceiverIds: [], + matchedTagIds: [], + snippetOffsets: [], + summaryOffsets: [] + }, + status: 'UPLOADED' as const, metadataComplete: false, scriptType: 'UNKNOWN' as const, createdAt: '2024-01-01T00:00:00', -- 2.49.1