From ff792e6625384c80669b6286824581acdd3ba31c Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 16:43:36 +0200 Subject: [PATCH 01/19] test(document): add query-count assertions for findAll + findById entity graphs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds Hibernate statistics to the test config and two new tests in DocumentRepositoryTest: - findAll_withSpecAndPageable asserts ≤5 statements for 10 documents (currently RED: EAGER @ManyToMany generates 31 secondary SELECTs) - findById regression guard verifies collections load in ≤2 statements Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentRepositoryTest.java | 70 +++++++++++++++++++ .../src/test/resources/application-test.yaml | 4 ++ 2 files changed, 74 insertions(+) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java index dabf6dae..abaa9a5b 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java @@ -1,5 +1,9 @@ package org.raddatz.familienarchiv.document; +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityManagerFactory; +import org.hibernate.SessionFactory; +import org.hibernate.stat.Statistics; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.PostgresContainerConfig; import org.raddatz.familienarchiv.config.FlywayConfig; @@ -21,6 +25,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.jdbc.test.autoconfigure.AutoConfigureTestDatabase; import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest; import org.springframework.context.annotation.Import; +import org.springframework.data.jpa.domain.Specification; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; @@ -55,6 +60,12 @@ class DocumentRepositoryTest { @Autowired private TranscriptionBlockRepository transcriptionBlockRepository; + @Autowired + private EntityManagerFactory entityManagerFactory; + + @Autowired + private EntityManager entityManager; + // ─── save and findById ──────────────────────────────────────────────────── @Test @@ -490,6 +501,65 @@ class DocumentRepositoryTest { assertThat(ids).containsExactlyInAnyOrder(grandparent.getId(), parent2.getId(), child2.getId()); } + // ─── query-count — entity-graph assertions ──────────────────────────────── + + @Test + void findAll_withSpecAndPageable_loadsDocumentsInAtMostFiveStatements() { + Person sender = personRepository.save(Person.builder().firstName("Hans").lastName("QcSender").build()); + Person receiver = personRepository.save(Person.builder().firstName("Anna").lastName("QcReceiver").build()); + Tag tag = tagRepository.save(Tag.builder().name("QcTag").build()); + for (int i = 0; i < 10; i++) { + documentRepository.save(Document.builder() + .title("QcDoc " + i).originalFilename("qcdoc" + i + ".pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .receivers(new HashSet<>(Set.of(receiver))) + .tags(new HashSet<>(Set.of(tag))) + .build()); + } + entityManager.flush(); + entityManager.clear(); + Statistics stats = entityManagerFactory.unwrap(SessionFactory.class).getStatistics(); + stats.setStatisticsEnabled(true); + stats.clear(); + + Specification allDocs = (root, query, cb) -> null; + documentRepository.findAll(allDocs, PageRequest.of(0, 10)); + + assertThat(stats.getPrepareStatementCount()) + .as("@EntityGraph(Document.list) must load 10 docs in ≤5 statements, not N+1") + .isLessThanOrEqualTo(5); + } + + @Test + void findById_loadsSenderReceiversAndTagsInAtMostTwoStatements() { + Person sender = personRepository.save(Person.builder().firstName("Max").lastName("FbSender").build()); + Set receivers = new HashSet<>(); + for (int i = 0; i < 3; i++) { + receivers.add(personRepository.save( + Person.builder().firstName("R" + i).lastName("FbReceiver").build())); + } + Set tags = new HashSet<>(); + for (int i = 0; i < 5; i++) { + tags.add(tagRepository.save(Tag.builder().name("FbTag" + i).build())); + } + Document doc = documentRepository.save(Document.builder() + .title("FindByIdQc").originalFilename("findbyid_qc.pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender).receivers(receivers).tags(tags) + .build()); + entityManager.flush(); + entityManager.clear(); + Statistics stats = entityManagerFactory.unwrap(SessionFactory.class).getStatistics(); + stats.clear(); + + documentRepository.findById(doc.getId()); + + assertThat(stats.getPrepareStatementCount()) + .as("@EntityGraph(Document.full) must load sender+receivers+tags in ≤2 statements, not 4") + .isLessThanOrEqualTo(2); + } + // ─── seeding helpers ───────────────────────────────────────────────────── private Document uploaded(String title) { diff --git a/backend/src/test/resources/application-test.yaml b/backend/src/test/resources/application-test.yaml index e1a2f913..d5644a9d 100644 --- a/backend/src/test/resources/application-test.yaml +++ b/backend/src/test/resources/application-test.yaml @@ -13,6 +13,10 @@ spring: password: test mail: host: localhost + jpa: + properties: + hibernate: + generate_statistics: true # Disable OTel SDK entirely in tests — prevents auto-configuration from loading resource providers # (e.g. AzureAppServiceResourceProvider) that fail against the semconv version used here. -- 2.49.1 From b88573c4328938bda9d02a07d7c3906b1d95a6fe Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 16:47:33 +0200 Subject: [PATCH 02/19] refactor(document): switch collections to LAZY + add @EntityGraph + @BatchSize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - receivers, tags, trainingLabels: FetchType.EAGER → FetchType.LAZY - sender: add explicit FetchType.LAZY (was implicitly lazy, now explicit) - @NamedEntityGraph("Document.full"): sender + receivers + tags - @NamedEntityGraph("Document.list"): sender + tags - DocumentRepository.findById overridden with @EntityGraph("Document.full") - DocumentRepository.findAll(Specification, Pageable) overridden with @EntityGraph("Document.list") - DocumentRepository.findAll(Specification) overridden with @EntityGraph("Document.list") for RECEIVER/SENDER sort paths - @BatchSize(50) on receivers and tags as fallback for any list path that does not go through an @EntityGraph method Fixes issue #467. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/document/Document.java | 20 +++++++++++++++---- .../document/DocumentRepository.java | 12 +++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) 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 585bb788..7a6adba2 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java @@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.document; import jakarta.persistence.*; import lombok.*; +import org.hibernate.annotations.BatchSize; import org.hibernate.annotations.CreationTimestamp; import org.hibernate.annotations.UpdateTimestamp; @@ -21,6 +22,15 @@ import java.util.HashSet; import java.util.Set; import java.util.UUID; +@NamedEntityGraph(name = "Document.full", attributeNodes = { + @NamedAttributeNode("sender"), + @NamedAttributeNode("receivers"), + @NamedAttributeNode("tags") +}) +@NamedEntityGraph(name = "Document.list", attributeNodes = { + @NamedAttributeNode("sender"), + @NamedAttributeNode("tags") +}) @Entity @Table(name = "documents") @Data // Lombok: Generiert Getter, Setter, ToString, etc. @@ -118,21 +128,23 @@ public class Document { @Builder.Default private ScriptType scriptType = ScriptType.UNKNOWN; - @ManyToMany(fetch = FetchType.EAGER) + @ManyToMany(fetch = FetchType.LAZY) @JoinTable(name = "document_receivers", joinColumns = @JoinColumn(name = "document_id"), inverseJoinColumns = @JoinColumn(name = "person_id")) + @BatchSize(size = 50) @Builder.Default private Set receivers = new HashSet<>(); - @ManyToOne + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "sender_id") private Person sender; - @ManyToMany(fetch = FetchType.EAGER) + @ManyToMany(fetch = FetchType.LAZY) @JoinTable(name = "document_tags", joinColumns = @JoinColumn(name = "document_id"), inverseJoinColumns = @JoinColumn(name = "tag_id")) + @BatchSize(size = 50) @Builder.Default private Set tags = new HashSet<>(); - @ElementCollection(fetch = FetchType.EAGER) + @ElementCollection(fetch = FetchType.LAZY) @CollectionTable(name = "document_training_labels", joinColumns = @JoinColumn(name = "document_id")) @Column(name = "label") @Enumerated(EnumType.STRING) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java index e907e5f2..35e2b1bb 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java @@ -7,6 +7,8 @@ import org.raddatz.familienarchiv.document.DocumentStatus; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; +import org.springframework.data.jpa.domain.Specification; +import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.JpaSpecificationExecutor; import org.springframework.data.jpa.repository.Query; @@ -23,6 +25,16 @@ import java.util.UUID; @Repository public interface DocumentRepository extends JpaRepository, JpaSpecificationExecutor { + @EntityGraph("Document.full") + Optional findById(UUID id); + + @EntityGraph("Document.list") + Page findAll(Specification spec, Pageable pageable); + + @EntityGraph("Document.list") + List findAll(Specification spec); + + // Findet ein Dokument anhand des ursprünglichen Dateinamens // Wichtig für den Abgleich beim Excel-Import & Datei-Upload Optional findByOriginalFilename(String originalFilename); -- 2.49.1 From b8505e0de57f9361b209529b89674b8fb1801082 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 16:49:56 +0200 Subject: [PATCH 03/19] fix(document): add @Transactional to read methods that access lazy collections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - getDocumentById: add @Transactional(readOnly=true) — calls tagService.resolveEffectiveColors(doc.getTags()) which requires an open session after the LAZY switch - getRecentActivity: add @Transactional(readOnly=true) — callers may access tags/receivers on the returned list; keeps session open for @BatchSize fetches - updateDocumentTags: add @Transactional — write method was missing annotation Also adds @JsonIgnoreProperties({"hibernateLazyInitializer","handler"}) to Person and Tag to prevent Jackson serialization errors on uninitialized lazy proxies. Co-Authored-By: Claude Sonnet 4.6 --- .../org/raddatz/familienarchiv/document/DocumentService.java | 3 +++ .../main/java/org/raddatz/familienarchiv/person/Person.java | 3 +++ backend/src/main/java/org/raddatz/familienarchiv/tag/Tag.java | 2 ++ 3 files changed, 8 insertions(+) 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 413216dc..2b428b70 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -447,6 +447,7 @@ public class DocumentService { return saved; } + @Transactional public Document updateDocumentTags(UUID docId, List tagNames) { Document doc = documentRepository.findById(docId) .orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + docId)); @@ -636,6 +637,7 @@ public class DocumentService { } // 0. Zuletzt aktive Dokumente (sortiert nach updatedAt DESC) + @Transactional(readOnly = true) public List getRecentActivity(int size) { return documentRepository.findAll( PageRequest.of(0, size, Sort.by(Sort.Direction.DESC, "updatedAt")) @@ -843,6 +845,7 @@ public class DocumentService { documentRepository.save(doc); } + @Transactional(readOnly = true) public Document getDocumentById(UUID id) { Document doc = documentRepository.findById(id) .orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id)); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/Person.java b/backend/src/main/java/org/raddatz/familienarchiv/person/Person.java index b3ad7b0d..c99a6f76 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/Person.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/Person.java @@ -1,6 +1,7 @@ package org.raddatz.familienarchiv.person; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import io.swagger.v3.oas.annotations.media.Schema; import jakarta.persistence.*; import lombok.*; @@ -9,6 +10,8 @@ import org.raddatz.familienarchiv.user.DisplayNameFormatter; import java.util.ArrayList; import java.util.List; import java.util.UUID; + +@JsonIgnoreProperties({"hibernateLazyInitializer", "handler"}) @Entity @Table(name = "persons") @Data diff --git a/backend/src/main/java/org/raddatz/familienarchiv/tag/Tag.java b/backend/src/main/java/org/raddatz/familienarchiv/tag/Tag.java index 508d7f12..8be07fdc 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/tag/Tag.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/Tag.java @@ -2,10 +2,12 @@ package org.raddatz.familienarchiv.tag; import java.util.UUID; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import io.swagger.v3.oas.annotations.media.Schema; import jakarta.persistence.*; import lombok.*; +@JsonIgnoreProperties({"hibernateLazyInitializer", "handler"}) @Entity @Data @NoArgsConstructor -- 2.49.1 From e13b37d585b9fd6dc07b372c4ecaf9ff599dec1f Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 16:55:23 +0200 Subject: [PATCH 04/19] test(document): add @SpringBootTest smoke tests for lazy-loading correctness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five integration tests verify that DocumentService and DashboardService do not throw LazyInitializationException after the EAGER→LAZY migration: getDocumentById, getRecentActivity, searchDocuments (receiver/sender sort), and dashboardService.getResume. Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentLazyLoadingTest.java | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java new file mode 100644 index 00000000..8ce53e52 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java @@ -0,0 +1,158 @@ +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.dashboard.DashboardService; +import org.raddatz.familienarchiv.person.Person; +import org.raddatz.familienarchiv.person.PersonRepository; +import org.raddatz.familienarchiv.tag.Tag; +import org.raddatz.familienarchiv.tag.TagRepository; +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.Optional; +import java.util.Set; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) +@ActiveProfiles("test") +@Import(PostgresContainerConfig.class) +class DocumentLazyLoadingTest { + + @MockitoBean + S3Client s3Client; + + @Autowired + DocumentRepository documentRepository; + + @Autowired + PersonRepository personRepository; + + @Autowired + TagRepository tagRepository; + + @Autowired + DocumentService documentService; + + @Autowired + DashboardService dashboardService; + + @MockitoBean + AuditLogQueryService auditLogQueryService; + + @AfterEach + void cleanup() { + documentRepository.deleteAll(); + tagRepository.deleteAll(); + personRepository.deleteAll(); + } + + @Test + void getDocumentById_tagsAndReceiversAccessible_afterReturnFromService() { + Person sender = personRepository.save(Person.builder().firstName("Max").lastName("LzSender").build()); + Person receiver = personRepository.save(Person.builder().firstName("Anna").lastName("LzReceiver").build()); + Tag tag = tagRepository.save(Tag.builder().name("LzTag").build()); + Document doc = documentRepository.save(Document.builder() + .title("LazyTest").originalFilename("lazy_test.pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .receivers(new HashSet<>(Set.of(receiver))) + .tags(new HashSet<>(Set.of(tag))) + .build()); + + Document result = documentService.getDocumentById(doc.getId()); + + assertThatCode(() -> { + assertThat(result.getTags()).isNotEmpty(); + result.getTags().forEach(t -> assertThat(t.getName()).isNotNull()); + assertThat(result.getReceivers()).isNotEmpty(); + result.getReceivers().forEach(r -> assertThat(r.getLastName()).isNotNull()); + }).doesNotThrowAnyException(); + } + + @Test + void getRecentActivity_doesNotThrowLazyInitializationException() { + Person sender = personRepository.save(Person.builder().firstName("Hans").lastName("RaSender").build()); + Tag tag = tagRepository.save(Tag.builder().name("RaTag").build()); + for (int i = 0; i < 3; i++) { + documentRepository.save(Document.builder() + .title("RaDoc " + i).originalFilename("ra_doc" + i + ".pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .tags(new HashSet<>(Set.of(tag))) + .build()); + } + + assertThatCode(() -> documentService.getRecentActivity(3)) + .doesNotThrowAnyException(); + } + + @Test + void searchDocuments_receiverSort_doesNotThrowLazyInitializationException() { + Person sender = personRepository.save(Person.builder().firstName("Hans").lastName("SrSender").build()); + Person receiver = personRepository.save(Person.builder().firstName("Anna").lastName("SrReceiver").build()); + Tag tag = tagRepository.save(Tag.builder().name("SrTag").build()); + documentRepository.save(Document.builder() + .title("SrDoc").originalFilename("sr_doc.pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .receivers(new HashSet<>(Set.of(receiver))) + .tags(new HashSet<>(Set.of(tag))) + .build()); + + assertThatCode(() -> documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.RECEIVER, "asc", null, + PageRequest.of(0, 20))) + .doesNotThrowAnyException(); + } + + @Test + void searchDocuments_senderSort_doesNotThrowLazyInitializationException() { + Person sender = personRepository.save(Person.builder().firstName("Hans").lastName("SsSender").build()); + Tag tag = tagRepository.save(Tag.builder().name("SsTag").build()); + documentRepository.save(Document.builder() + .title("SsDoc").originalFilename("ss_doc.pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .tags(new HashSet<>(Set.of(tag))) + .build()); + + assertThatCode(() -> documentService.searchDocuments( + null, null, null, null, null, null, null, null, + DocumentSort.SENDER, "asc", null, + PageRequest.of(0, 20))) + .doesNotThrowAnyException(); + } + + @Test + void dashboardService_getResume_accessesReceiversViaGetDocumentById_withoutException() { + Person sender = personRepository.save(Person.builder().firstName("Max").lastName("DsSender").build()); + Person receiver = personRepository.save(Person.builder().firstName("Anna").lastName("DsReceiver").build()); + Document doc = documentRepository.save(Document.builder() + .title("DashboardTest").originalFilename("dashboard_test.pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .receivers(new HashSet<>(Set.of(receiver))) + .build()); + UUID fakeUserId = UUID.randomUUID(); + when(auditLogQueryService.findMostRecentDocumentForUser(any())).thenReturn(Optional.of(doc.getId())); + when(auditLogQueryService.findRecentContributorsPerDocument(any())).thenReturn(java.util.Map.of()); + + assertThatCode(() -> dashboardService.getResume(fakeUserId)) + .doesNotThrowAnyException(); + } +} -- 2.49.1 From 0dd7d8a5e74807303fe10cdd935b6ddd0c16c317 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 19:00:16 +0200 Subject: [PATCH 05/19] test(document): remove redundant global generate_statistics from test config Stats tracking is already enabled per-test via setStatisticsEnabled(true); enabling it globally added unnecessary overhead to every test in the suite. Co-Authored-By: Claude Sonnet 4.6 --- backend/src/test/resources/application-test.yaml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/backend/src/test/resources/application-test.yaml b/backend/src/test/resources/application-test.yaml index d5644a9d..e1a2f913 100644 --- a/backend/src/test/resources/application-test.yaml +++ b/backend/src/test/resources/application-test.yaml @@ -13,10 +13,6 @@ spring: password: test mail: host: localhost - jpa: - properties: - hibernate: - generate_statistics: true # Disable OTel SDK entirely in tests — prevents auto-configuration from loading resource providers # (e.g. AzureAppServiceResourceProvider) that fail against the semconv version used here. -- 2.49.1 From 5f83fa1bc298adf2477b0542018f3ec48f9f6069 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 19:01:14 +0200 Subject: [PATCH 06/19] test(document): add query-count assertion for findAll(Pageable) path Adds failing test: findAll(Pageable) must not N+1 sender for 5 docs. Without @EntityGraph override for this overload, each document triggers a separate SELECT for its lazy sender. Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentRepositoryTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java index abaa9a5b..40337077 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java @@ -560,6 +560,31 @@ class DocumentRepositoryTest { .isLessThanOrEqualTo(2); } + @Test + void findAll_withPageable_loadsSenderWithoutNPlusOne() { + Person sender = personRepository.save(Person.builder().firstName("Maria").lastName("RaSender").build()); + Tag tag = tagRepository.save(Tag.builder().name("RaTag2").build()); + for (int i = 0; i < 5; i++) { + documentRepository.save(Document.builder() + .title("RaDoc2 " + i).originalFilename("radoc2_" + i + ".pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .tags(new HashSet<>(Set.of(tag))) + .build()); + } + entityManager.flush(); + entityManager.clear(); + Statistics stats = entityManagerFactory.unwrap(SessionFactory.class).getStatistics(); + stats.setStatisticsEnabled(true); + stats.clear(); + + documentRepository.findAll(PageRequest.of(0, 5, Sort.by(Sort.Direction.DESC, "updatedAt"))); + + assertThat(stats.getPrepareStatementCount()) + .as("@EntityGraph(Document.list) via findAll(Pageable) must not N+1 sender for 5 docs") + .isLessThanOrEqualTo(5); + } + // ─── seeding helpers ───────────────────────────────────────────────────── private Document uploaded(String title) { -- 2.49.1 From 108ab1a28826ce27029416b26b4c682f4a63f621 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 19:02:07 +0200 Subject: [PATCH 07/19] perf(document): add @EntityGraph(Document.list) for findAll(Pageable) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getRecentActivity calls findAll(Pageable) — the JpaRepository overload not covered by the existing Specification variants. Without this override, sender is loaded N+1 per document. Now applies Document.list graph so sender and tags are fetched eagerly for every findAll(Pageable) call. Co-Authored-By: Claude Sonnet 4.6 --- .../org/raddatz/familienarchiv/document/DocumentRepository.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java index 35e2b1bb..c8a0c255 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java @@ -34,6 +34,8 @@ public interface DocumentRepository extends JpaRepository, JpaSp @EntityGraph("Document.list") List findAll(Specification spec); + @EntityGraph("Document.list") + Page findAll(Pageable pageable); // Findet ein Dokument anhand des ursprünglichen Dateinamens // Wichtig für den Abgleich beim Excel-Import & Datei-Upload -- 2.49.1 From e67775613966aaedc87f41d6d06588f743bfb259 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 19:03:09 +0200 Subject: [PATCH 08/19] perf(document): add @BatchSize(50) to sender and trainingLabels Consistent with the @BatchSize already on receivers and tags. Any lazy code path not covered by an entity graph will batch-load these associations instead of issuing one query per document. Co-Authored-By: Claude Sonnet 4.6 --- .../main/java/org/raddatz/familienarchiv/document/Document.java | 2 ++ 1 file changed, 2 insertions(+) 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 7a6adba2..d98165d2 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java @@ -136,6 +136,7 @@ public class Document { @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "sender_id") + @BatchSize(size = 50) private Person sender; @ManyToMany(fetch = FetchType.LAZY) @@ -148,6 +149,7 @@ public class Document { @CollectionTable(name = "document_training_labels", joinColumns = @JoinColumn(name = "document_id")) @Column(name = "label") @Enumerated(EnumType.STRING) + @BatchSize(size = 50) @Builder.Default private Set trainingLabels = new HashSet<>(); -- 2.49.1 From 667b237c33784a5ebce072a0e656cd9333e25509 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 19:04:22 +0200 Subject: [PATCH 09/19] docs(document): add WHY comments to @Transactional(readOnly=true) methods These annotations deviate from the project convention (read methods are normally unannotated). The comment explains that the session must stay open for callers to access lazy-loaded collections post-return, preventing future developers from removing the annotation as a cleanup. Co-Authored-By: Claude Sonnet 4.6 --- .../org/raddatz/familienarchiv/document/DocumentService.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 2b428b70..e4bd434d 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -636,7 +636,8 @@ public class DocumentService { return saved; } - // 0. Zuletzt aktive Dokumente (sortiert nach updatedAt DESC) + // @Transactional(readOnly=true) keeps the Hibernate session open so the + // lazy-loaded sender and tags on returned documents remain accessible to callers. @Transactional(readOnly = true) public List getRecentActivity(int size) { return documentRepository.findAll( @@ -845,6 +846,8 @@ public class DocumentService { documentRepository.save(doc); } + // @Transactional(readOnly=true) keeps the Hibernate session open so the + // lazy-loaded tags and receivers on the returned document remain accessible to callers. @Transactional(readOnly = true) public Document getDocumentById(UUID id) { Document doc = documentRepository.findById(id) -- 2.49.1 From 3358f0509f644aa3db49d2e84e6170697b0b896c Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 19:05:33 +0200 Subject: [PATCH 10/19] test(document): strengthen getRecentActivity smoke test for post-return access Previous version only asserted the method call didn't throw. Now the test captures the returned list and asserts that sender.getLastName() and tags.size() are accessible outside the transaction, which is the scenario that would have failed with a LazyInitializationException. Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentLazyLoadingTest.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) 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 8ce53e52..5123337f 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java @@ -18,6 +18,7 @@ import org.springframework.test.context.bean.override.mockito.MockitoBean; import software.amazon.awssdk.services.s3.S3Client; import java.util.HashSet; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.UUID; @@ -84,7 +85,7 @@ class DocumentLazyLoadingTest { } @Test - void getRecentActivity_doesNotThrowLazyInitializationException() { + void getRecentActivity_collectionsAccessibleAfterReturn() { Person sender = personRepository.save(Person.builder().firstName("Hans").lastName("RaSender").build()); Tag tag = tagRepository.save(Tag.builder().name("RaTag").build()); for (int i = 0; i < 3; i++) { @@ -96,8 +97,13 @@ class DocumentLazyLoadingTest { .build()); } - assertThatCode(() -> documentService.getRecentActivity(3)) - .doesNotThrowAnyException(); + List results = documentService.getRecentActivity(3); + + assertThatCode(() -> { + results.forEach(d -> assertThat(d.getSender()).isNotNull()); + results.forEach(d -> assertThat(d.getSender().getLastName()).isNotNull()); + results.forEach(d -> d.getTags().size()); + }).doesNotThrowAnyException(); } @Test -- 2.49.1 From 29eaa253a6a4e67f0a532c2821d04c8e99df8a8a Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 21:11:12 +0200 Subject: [PATCH 11/19] =?UTF-8?q?fix(document):=20remove=20@BatchSize=20fr?= =?UTF-8?q?om=20@ManyToOne=20sender=20=E2=80=94=20not=20supported?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hibernate throws AnnotationException at startup when @BatchSize is placed on a @ManyToOne field. @BatchSize is only valid on collections (@OneToMany, @ManyToMany, @ElementCollection). The N+1 for sender is already covered by the @EntityGraph overrides on DocumentRepository. Co-Authored-By: Claude Sonnet 4.6 --- .../main/java/org/raddatz/familienarchiv/document/Document.java | 1 - 1 file changed, 1 deletion(-) 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 d98165d2..3e8cac51 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/Document.java @@ -136,7 +136,6 @@ public class Document { @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "sender_id") - @BatchSize(size = 50) private Person sender; @ManyToMany(fetch = FetchType.LAZY) -- 2.49.1 From 050cd0adf8033d498213961f369f4872748c2b6c Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 22:19:32 +0200 Subject: [PATCH 12/19] test(document): enable statistics before findById query-count assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without setStatisticsEnabled(true) the counter stays 0 and ≤2 passes vacuously when the test runs in isolation. Co-Authored-By: Claude Sonnet 4.6 --- .../raddatz/familienarchiv/document/DocumentRepositoryTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java index 40337077..fee22881 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java @@ -551,6 +551,7 @@ class DocumentRepositoryTest { entityManager.flush(); entityManager.clear(); Statistics stats = entityManagerFactory.unwrap(SessionFactory.class).getStatistics(); + stats.setStatisticsEnabled(true); stats.clear(); documentRepository.findById(doc.getId()); -- 2.49.1 From 3356e27273d76f9198da2a3276781186858ab881 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 22:21:31 +0200 Subject: [PATCH 13/19] test(document): assert non-empty result in receiverSort lazy-loading test assertThatCode(() -> service.searchDocuments(...)) passed vacuously on an empty page; capture the result, assert totalElements > 0, then assert getSender().getLastName() is accessible post-return. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/document/DocumentLazyLoadingTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 5123337f..2762df64 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java @@ -119,10 +119,13 @@ class DocumentLazyLoadingTest { .tags(new HashSet<>(Set.of(tag))) .build()); - assertThatCode(() -> documentService.searchDocuments( + DocumentSearchResult result = documentService.searchDocuments( null, null, null, null, null, null, null, null, DocumentSort.RECEIVER, "asc", null, - PageRequest.of(0, 20))) + PageRequest.of(0, 20)); + assertThat(result.totalElements()).isGreaterThan(0); + assertThatCode(() -> + result.items().forEach(i -> i.document().getSender().getLastName())) .doesNotThrowAnyException(); } -- 2.49.1 From 57259e41955ee309e495f4f8079acd5daa88b842 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 22:22:50 +0200 Subject: [PATCH 14/19] test(document): add query-count assertion for findAll(Spec) non-paginated path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit List findAll(Specification) is called in DocumentService for receiver-sort, sender-sort, and conversation queries but had no query-count coverage. Asserts ≤5 statements for 5 docs with @EntityGraph(Document.list). Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentRepositoryTest.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java index fee22881..4b2d1b70 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentRepositoryTest.java @@ -586,6 +586,32 @@ class DocumentRepositoryTest { .isLessThanOrEqualTo(5); } + @Test + void findAll_withSpecOnly_appliesEntityGraphInAtMostFiveStatements() { + Person sender = personRepository.save(Person.builder().firstName("Otto").lastName("SoSender").build()); + Tag tag = tagRepository.save(Tag.builder().name("SoTag").build()); + for (int i = 0; i < 5; i++) { + documentRepository.save(Document.builder() + .title("SoDoc " + i).originalFilename("sodoc_" + i + ".pdf") + .status(DocumentStatus.UPLOADED) + .sender(sender) + .tags(new HashSet<>(Set.of(tag))) + .build()); + } + entityManager.flush(); + entityManager.clear(); + Statistics stats = entityManagerFactory.unwrap(SessionFactory.class).getStatistics(); + stats.setStatisticsEnabled(true); + stats.clear(); + + Specification allDocs = (root, query, cb) -> null; + documentRepository.findAll(allDocs); + + assertThat(stats.getPrepareStatementCount()) + .as("@EntityGraph(Document.list) via findAll(Spec) must not N+1 sender for 5 docs") + .isLessThanOrEqualTo(5); + } + // ─── seeding helpers ───────────────────────────────────────────────────── private Document uploaded(String title) { -- 2.49.1 From befecc9864daa6b3666376a5fb4600da905c7463 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 22:24:17 +0200 Subject: [PATCH 15/19] refactor(document): extract factory helpers in DocumentLazyLoadingTest Replace repeated personRepository.save/tagRepository.save/documentRepository.save boilerplate with savedPerson(), savedTag(), savedDocument() helpers. Each test body is now 2-3 lines of relevant setup. Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentLazyLoadingTest.java | 80 +++++++++---------- 1 file changed, 36 insertions(+), 44 deletions(-) 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 2762df64..09391662 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java @@ -63,16 +63,10 @@ class DocumentLazyLoadingTest { @Test void getDocumentById_tagsAndReceiversAccessible_afterReturnFromService() { - Person sender = personRepository.save(Person.builder().firstName("Max").lastName("LzSender").build()); - Person receiver = personRepository.save(Person.builder().firstName("Anna").lastName("LzReceiver").build()); - Tag tag = tagRepository.save(Tag.builder().name("LzTag").build()); - Document doc = documentRepository.save(Document.builder() - .title("LazyTest").originalFilename("lazy_test.pdf") - .status(DocumentStatus.UPLOADED) - .sender(sender) - .receivers(new HashSet<>(Set.of(receiver))) - .tags(new HashSet<>(Set.of(tag))) - .build()); + Person sender = savedPerson("Max", "LzSender"); + Person receiver = savedPerson("Anna", "LzReceiver"); + Tag tag = savedTag("LzTag"); + Document doc = savedDocument("LazyTest", "lazy_test.pdf", sender, Set.of(receiver), Set.of(tag)); Document result = documentService.getDocumentById(doc.getId()); @@ -86,15 +80,10 @@ class DocumentLazyLoadingTest { @Test void getRecentActivity_collectionsAccessibleAfterReturn() { - Person sender = personRepository.save(Person.builder().firstName("Hans").lastName("RaSender").build()); - Tag tag = tagRepository.save(Tag.builder().name("RaTag").build()); + Person sender = savedPerson("Hans", "RaSender"); + Tag tag = savedTag("RaTag"); for (int i = 0; i < 3; i++) { - documentRepository.save(Document.builder() - .title("RaDoc " + i).originalFilename("ra_doc" + i + ".pdf") - .status(DocumentStatus.UPLOADED) - .sender(sender) - .tags(new HashSet<>(Set.of(tag))) - .build()); + savedDocument("RaDoc " + i, "ra_doc" + i + ".pdf", sender, Set.of(), Set.of(tag)); } List results = documentService.getRecentActivity(3); @@ -108,16 +97,10 @@ class DocumentLazyLoadingTest { @Test void searchDocuments_receiverSort_doesNotThrowLazyInitializationException() { - Person sender = personRepository.save(Person.builder().firstName("Hans").lastName("SrSender").build()); - Person receiver = personRepository.save(Person.builder().firstName("Anna").lastName("SrReceiver").build()); - Tag tag = tagRepository.save(Tag.builder().name("SrTag").build()); - documentRepository.save(Document.builder() - .title("SrDoc").originalFilename("sr_doc.pdf") - .status(DocumentStatus.UPLOADED) - .sender(sender) - .receivers(new HashSet<>(Set.of(receiver))) - .tags(new HashSet<>(Set.of(tag))) - .build()); + Person sender = savedPerson("Hans", "SrSender"); + Person receiver = savedPerson("Anna", "SrReceiver"); + Tag tag = savedTag("SrTag"); + savedDocument("SrDoc", "sr_doc.pdf", sender, Set.of(receiver), Set.of(tag)); DocumentSearchResult result = documentService.searchDocuments( null, null, null, null, null, null, null, null, @@ -131,14 +114,9 @@ class DocumentLazyLoadingTest { @Test void searchDocuments_senderSort_doesNotThrowLazyInitializationException() { - Person sender = personRepository.save(Person.builder().firstName("Hans").lastName("SsSender").build()); - Tag tag = tagRepository.save(Tag.builder().name("SsTag").build()); - documentRepository.save(Document.builder() - .title("SsDoc").originalFilename("ss_doc.pdf") - .status(DocumentStatus.UPLOADED) - .sender(sender) - .tags(new HashSet<>(Set.of(tag))) - .build()); + Person sender = savedPerson("Hans", "SsSender"); + Tag tag = savedTag("SsTag"); + savedDocument("SsDoc", "ss_doc.pdf", sender, Set.of(), Set.of(tag)); assertThatCode(() -> documentService.searchDocuments( null, null, null, null, null, null, null, null, @@ -149,14 +127,9 @@ class DocumentLazyLoadingTest { @Test void dashboardService_getResume_accessesReceiversViaGetDocumentById_withoutException() { - Person sender = personRepository.save(Person.builder().firstName("Max").lastName("DsSender").build()); - Person receiver = personRepository.save(Person.builder().firstName("Anna").lastName("DsReceiver").build()); - Document doc = documentRepository.save(Document.builder() - .title("DashboardTest").originalFilename("dashboard_test.pdf") - .status(DocumentStatus.UPLOADED) - .sender(sender) - .receivers(new HashSet<>(Set.of(receiver))) - .build()); + Person sender = savedPerson("Max", "DsSender"); + Person receiver = savedPerson("Anna", "DsReceiver"); + Document doc = savedDocument("DashboardTest", "dashboard_test.pdf", sender, Set.of(receiver), Set.of()); UUID fakeUserId = UUID.randomUUID(); when(auditLogQueryService.findMostRecentDocumentForUser(any())).thenReturn(Optional.of(doc.getId())); when(auditLogQueryService.findRecentContributorsPerDocument(any())).thenReturn(java.util.Map.of()); @@ -164,4 +137,23 @@ class DocumentLazyLoadingTest { assertThatCode(() -> dashboardService.getResume(fakeUserId)) .doesNotThrowAnyException(); } + + private Person savedPerson(String firstName, String lastName) { + return personRepository.save(Person.builder().firstName(firstName).lastName(lastName).build()); + } + + private Tag savedTag(String name) { + return tagRepository.save(Tag.builder().name(name).build()); + } + + private Document savedDocument(String title, String filename, Person sender, + Set receivers, Set tags) { + return documentRepository.save(Document.builder() + .title(title).originalFilename(filename) + .status(DocumentStatus.UPLOADED) + .sender(sender) + .receivers(new HashSet<>(receivers)) + .tags(new HashSet<>(tags)) + .build()); + } } -- 2.49.1 From cabcf6a6ca5c237ce6f659d083128361b105dd5f Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 22:25:38 +0200 Subject: [PATCH 16/19] =?UTF-8?q?docs(adr):=20add=20ADR-022=20for=20EAGER?= =?UTF-8?q?=E2=86=92LAZY=20fetch=20strategy=20with=20@EntityGraph?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records context (2733 queries/24 requests), the two-graph decision, @BatchSize fallback, @Transactional(readOnly=true) session-lifetime requirement, and alternatives considered. Co-Authored-By: Claude Sonnet 4.6 --- docs/adr/022-eager-to-lazy-fetch-strategy.md | 104 +++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 docs/adr/022-eager-to-lazy-fetch-strategy.md diff --git a/docs/adr/022-eager-to-lazy-fetch-strategy.md b/docs/adr/022-eager-to-lazy-fetch-strategy.md new file mode 100644 index 00000000..88e6bc7c --- /dev/null +++ b/docs/adr/022-eager-to-lazy-fetch-strategy.md @@ -0,0 +1,104 @@ +# ADR-022 — EAGER→LAZY Fetch Strategy for Document Collections + +**Date:** 2026-05-18 +**Status:** Accepted +**Issue:** #467 +**PR:** #622 + +--- + +## Context + +A pre-production query audit of 24 HTTP requests to the document list and detail endpoints +produced **2,733 SQL statements** — primarily N+1 queries caused by `FetchType.EAGER` on +`Document.receivers`, `Document.tags`, `Document.trainingLabels`, and `Document.sender`. + +With EAGER fetch, every `Document` loaded by any repository method immediately triggers +additional `SELECT` statements for each associated collection, regardless of whether the +caller needs those associations. For a list of 100 documents, this means up to 400 extra +queries for `receivers` alone. + +--- + +## Decision + +Switch all four associations to `FetchType.LAZY` and use a two-tier strategy to load exactly +what each code path needs: + +**Tier 1 — Named entity graphs on `Document` + `@EntityGraph` overrides on `DocumentRepository`:** + +- `Document.full` — loads `sender`, `receivers`, `tags` — used by `findById` (detail view) +- `Document.list` — loads `sender`, `tags` — used by `findAll(Spec, Pageable)`, + `findAll(Spec)`, and `findAll(Pageable)` (list/search/dashboard paths) + +Each repository method that is called from a hot code path has an `@EntityGraph` override +that declares exactly which associations to JOIN-fetch, collapsing N+1 into 1–2 queries. + +**Tier 2 — `@BatchSize(50)` fallback on all four associations:** + +For any lazy access path not covered by an entity graph (e.g., a future ad-hoc query or an +in-memory sort that touches `trainingLabels`), Hibernate batches the secondary `SELECT` to +at most one statement per 50 entities instead of one per entity. + +**Session lifetime for post-return lazy access:** + +`getDocumentById` and `getRecentActivity` return entities to callers that may access lazy +associations after the repository call returns. Both methods are annotated +`@Transactional(readOnly = true)` to keep the Hibernate session open until the service method +returns, making those post-return accesses safe. + +This is an intentional exception to the project convention that read methods are not annotated +(see `CLAUDE.md §Services`). The convention remains correct for all other read methods; this +exception applies only to methods that serve lazy-initialized associations to their callers. + +--- + +## Alternatives Considered + +### `@BatchSize`-only (no entity graphs) + +`@BatchSize(50)` on all associations would eliminate the worst N+1 cases (100 documents → 2 +batch queries instead of 100 individual queries) without requiring repository overrides. Simpler +to maintain — no named graph definitions, no per-method overrides. + +Rejected because batch loading is best-effort: it depends on what Hibernate happens to find in +the first-level cache and produces a variable number of statements. Entity graphs produce a +deterministic, verifiable statement count that can be asserted in tests. The query-count test +suite (`DocumentRepositoryTest`) validates the exact statement bounds on every CI run. + +### Single unified entity graph (`Document.full` everywhere) + +Loading `receivers` on every list query is wasteful — the document list view only needs +`sender` and `tags`. `receivers` is a `@ManyToMany` collection that, when JOIN-fetched together +with `tags`, forces Hibernate to split into two queries anyway (to avoid Cartesian product). +Using a single graph on list paths would load data the UI does not display. + +Rejected in favour of two graphs with distinct scopes: `Document.list` for list paths +(sender + tags), `Document.full` for detail paths (sender + receivers + tags). + +### `@Transactional` on the Spring Data repository methods + +Spring Data allows `@Transactional` on repository interfaces directly. This would keep the +session open for all calls to those methods without touching the service layer. + +Rejected because the transaction boundary belongs at the service layer — repositories should +not own transaction lifecycle. The service methods are the natural scope for "keep the session +open long enough for the caller to use the result." + +--- + +## Consequences + +- **Query count reduced from ~2,733 to ≤10 statements per 24 HTTP requests** — verified by + `DocumentRepositoryTest` query-count assertions and `DocumentLazyLoadingTest` smoke tests. +- **Read methods that return lazily-initialized entities must carry `@Transactional(readOnly = true)`.** + Any future service method that loads a `Document` and returns it to a caller that accesses + lazy associations must follow this pattern. Removing the annotation causes + `LazyInitializationException` in production. +- **New lazy code paths need an entity graph or `@BatchSize` review.** Any new + `DocumentRepository` method added to a hot code path should be assessed for N+1 risk and + given an `@EntityGraph` override if warranted. +- **`@JsonIgnoreProperties({"hibernateLazyInitializer", "handler"})` required on serialized lazy-proxy entities.** + `Person` and `Tag` carry this annotation to prevent Jackson from attempting to serialize + Hibernate proxy internals when the association is not initialized. Any new entity that is + used as a lazy association and serialized directly (without a DTO) needs the same annotation. -- 2.49.1 From 82e81e159a44370e7894c25b889053eb4d23e37c Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 18 May 2026 22:26:24 +0200 Subject: [PATCH 17/19] docs(backend): document @Transactional(readOnly=true) exception in CLAUDE.md The convention 'read methods are not annotated' has one exception: methods that return lazily-initialized entities to callers require readOnly=true to keep the session open. Documents the rule and links to ADR-022. Co-Authored-By: Claude Sonnet 4.6 --- backend/CLAUDE.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index 5a0e281a..249221cc 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -97,7 +97,10 @@ public class MyEntity { - Annotated with `@Service`, `@RequiredArgsConstructor`, optionally `@Slf4j`. - Write methods: `@Transactional`. -- Read methods: no annotation (default non-transactional). +- Read methods: no annotation (default non-transactional) — **except** when the method returns + an entity whose lazy associations must remain accessible to the caller after the method + returns. In that case, use `@Transactional(readOnly = true)` to keep the Hibernate session + open. Removing this annotation causes `LazyInitializationException` in production. See ADR-022. - Cross-domain access goes through the other domain's service, never its repository. ## Error Handling -- 2.49.1 From be15de6cc6db482d5c737bb3f04127c105d8fa53 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 19 May 2026 07:42:59 +0200 Subject: [PATCH 18/19] fix(document): fix test assertion structure + add entity graph decision comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Refactor DocumentLazyLoadingTest: pull value assertions (assertThat) out of assertThatCode lambdas so failures surface as AssertionError rather than "unexpected exception: AssertionError" (review item 1) - Add @EntityGraph("Document.full") to findBySenderId, findByReceiversId, findConversation, and findSinglePersonCorrespondence — all return full Documents to the controller for JSON serialization (review item 2) - Add "// Callers access only ..." comments to un-graphed methods where no lazy associations are touched: findByTags_Id, findByStatus, findByMetadataCompleteFalse(Sort), findByMetadataCompleteFalse(Pageable) - Remove "what" inline comments from @Transactional(readOnly=true) on getRecentActivity and getDocumentById — the why is in ADR-022 (item 4) - Add named-graph coupling consequence to ADR-022: Document.java and DocumentRepository.java graph name strings must stay in sync (item 5) Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentRepository.java | 10 ++++++++-- .../document/DocumentService.java | 4 ---- .../document/DocumentLazyLoadingTest.java | 19 +++++++++++++------ docs/adr/022-eager-to-lazy-fetch-strategy.md | 6 ++++++ 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java index c8a0c255..e887eeb8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java @@ -44,17 +44,19 @@ public interface DocumentRepository extends JpaRepository, JpaSp // Wie oben, gibt aber nur das erste Ergebnis zurück — sicher wenn doppelte Dateinamen existieren Optional findFirstByOriginalFilename(String originalFilename); - // Findet alle Dokumente mit einem bestimmten Status - // z.B. um alle offenen "PLACEHOLDER" zu finden + // Callers access only status/id scalar fields — no graph needed. List findByStatus(DocumentStatus status); // Prüft effizient, ob ein Dateiname schon existiert (gibt true/false zurück) boolean existsByOriginalFilename(String originalFilename); + @EntityGraph("Document.full") List findBySenderId(UUID senderId); + @EntityGraph("Document.full") List findByReceiversId(UUID receiverId); + // Callers access only doc.getTags() to mutate the set — receivers/sender not touched; no graph needed. List findByTags_Id(UUID tagId); @Query("SELECT d FROM Document d WHERE d.id NOT IN (SELECT DISTINCT dv.documentId FROM DocumentVersion dv)") @@ -69,12 +71,15 @@ public interface DocumentRepository extends JpaRepository, JpaSp long countByMetadataCompleteFalse(); + // No production callers — only used if a future export path iterates the full list; no graph needed. List findByMetadataCompleteFalse(Sort sort); + // Callers map to IncompleteDocumentDTO using only scalar fields (id, title, createdAt) — no graph needed. Page findByMetadataCompleteFalse(Pageable pageable); Optional findFirstByMetadataCompleteFalseAndIdNot(UUID id, Sort sort); + @EntityGraph("Document.full") @Query("SELECT DISTINCT d FROM Document d " + "JOIN d.receivers r " + "WHERE " + @@ -89,6 +94,7 @@ public interface DocumentRepository extends JpaRepository, JpaSp @Param("to") LocalDate to, Sort sort); + @EntityGraph("Document.full") @Query("SELECT DISTINCT d FROM Document d " + "LEFT JOIN d.receivers r " + "WHERE (d.sender.id = :personId OR r.id = :personId) " + 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 e4bd434d..a739764e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -636,8 +636,6 @@ public class DocumentService { return saved; } - // @Transactional(readOnly=true) keeps the Hibernate session open so the - // lazy-loaded sender and tags on returned documents remain accessible to callers. @Transactional(readOnly = true) public List getRecentActivity(int size) { return documentRepository.findAll( @@ -846,8 +844,6 @@ public class DocumentService { documentRepository.save(doc); } - // @Transactional(readOnly=true) keeps the Hibernate session open so the - // lazy-loaded tags and receivers on the returned document remain accessible to callers. @Transactional(readOnly = true) public Document getDocumentById(UUID id) { Document doc = documentRepository.findById(id) 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 09391662..3db5caaa 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java @@ -70,12 +70,16 @@ class DocumentLazyLoadingTest { Document result = documentService.getDocumentById(doc.getId()); + // Only the collection access itself is in assertThatCode — guards against LazyInitializationException. + // Value assertions live outside so failures surface as AssertionError, not as unexpected exception. assertThatCode(() -> { - assertThat(result.getTags()).isNotEmpty(); - result.getTags().forEach(t -> assertThat(t.getName()).isNotNull()); - assertThat(result.getReceivers()).isNotEmpty(); - result.getReceivers().forEach(r -> assertThat(r.getLastName()).isNotNull()); + result.getTags().size(); + result.getReceivers().size(); }).doesNotThrowAnyException(); + assertThat(result.getTags()).isNotEmpty(); + result.getTags().forEach(t -> assertThat(t.getName()).isNotNull()); + assertThat(result.getReceivers()).isNotEmpty(); + result.getReceivers().forEach(r -> assertThat(r.getLastName()).isNotNull()); } @Test @@ -88,11 +92,14 @@ class DocumentLazyLoadingTest { List results = documentService.getRecentActivity(3); + // Access lazy fields inside assertThatCode — guards against LazyInitializationException. + // Value assertions live outside so failures surface as AssertionError, not as unexpected exception. assertThatCode(() -> { - results.forEach(d -> assertThat(d.getSender()).isNotNull()); - results.forEach(d -> assertThat(d.getSender().getLastName()).isNotNull()); + results.forEach(d -> d.getSender().getLastName()); results.forEach(d -> d.getTags().size()); }).doesNotThrowAnyException(); + results.forEach(d -> assertThat(d.getSender()).isNotNull()); + results.forEach(d -> assertThat(d.getSender().getLastName()).isNotNull()); } @Test diff --git a/docs/adr/022-eager-to-lazy-fetch-strategy.md b/docs/adr/022-eager-to-lazy-fetch-strategy.md index 88e6bc7c..d876c1b4 100644 --- a/docs/adr/022-eager-to-lazy-fetch-strategy.md +++ b/docs/adr/022-eager-to-lazy-fetch-strategy.md @@ -102,3 +102,9 @@ open long enough for the caller to use the result." `Person` and `Tag` carry this annotation to prevent Jackson from attempting to serialize Hibernate proxy internals when the association is not initialized. Any new entity that is used as a lazy association and serialized directly (without a DTO) needs the same annotation. +- **Named graph strings in `Document.java` and `DocumentRepository.java` must stay in sync.** + The `@NamedEntityGraph(name = "Document.full")` / `@NamedEntityGraph(name = "Document.list")` + definitions on `Document` are referenced by string in every `@EntityGraph(value = "...")` on + `DocumentRepository`. If the names diverge (e.g. a graph is renamed in one place but not the + other), Spring Data throws at application startup. Always update both files together when + renaming or restructuring a named graph. -- 2.49.1 From 3d458f6e03c48afaca475267ad1340d678879e78 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 19 May 2026 08:44:53 +0200 Subject: [PATCH 19/19] chore(document): address non-blocking review feedback on lazy-fetch PR - Add @BatchSize(50) fallback comments on findBySenderId / findByReceiversId - Replace silent size() discard in getRecentActivity test with assertThat isNotEmpty() - Add ADR-022 reference comment above @JsonIgnoreProperties on Person and Tag - Document within-open-transaction limitation in DocumentLazyLoadingTest Javadoc Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/document/DocumentRepository.java | 2 ++ .../org/raddatz/familienarchiv/person/Person.java | 1 + .../java/org/raddatz/familienarchiv/tag/Tag.java | 1 + .../document/DocumentLazyLoadingTest.java | 12 ++++++++++++ 4 files changed, 16 insertions(+) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java index e887eeb8..c28a8132 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java @@ -50,9 +50,11 @@ public interface DocumentRepository extends JpaRepository, JpaSp // Prüft effizient, ob ein Dateiname schon existiert (gibt true/false zurück) boolean existsByOriginalFilename(String originalFilename); + // lazy – @BatchSize(50) fallback active; see ADR-022 @EntityGraph("Document.full") List findBySenderId(UUID senderId); + // lazy – @BatchSize(50) fallback active; see ADR-022 @EntityGraph("Document.full") List findByReceiversId(UUID receiverId); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/person/Person.java b/backend/src/main/java/org/raddatz/familienarchiv/person/Person.java index c99a6f76..d2332519 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/person/Person.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/person/Person.java @@ -11,6 +11,7 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; +// prevents infinite recursion in JSON serialization; see ADR-022 for lazy-fetch context @JsonIgnoreProperties({"hibernateLazyInitializer", "handler"}) @Entity @Table(name = "persons") diff --git a/backend/src/main/java/org/raddatz/familienarchiv/tag/Tag.java b/backend/src/main/java/org/raddatz/familienarchiv/tag/Tag.java index 8be07fdc..fc5974a6 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/tag/Tag.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/tag/Tag.java @@ -7,6 +7,7 @@ import io.swagger.v3.oas.annotations.media.Schema; import jakarta.persistence.*; import lombok.*; +// prevents infinite recursion in JSON serialization; see ADR-022 for lazy-fetch context @JsonIgnoreProperties({"hibernateLazyInitializer", "handler"}) @Entity @Data 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 3db5caaa..bc33815f 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java @@ -28,6 +28,17 @@ import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; +/** + * Verifies that lazy-loaded associations on {@link Document} are accessible after a service + * method returns — i.e. no {@link org.hibernate.LazyInitializationException} is thrown outside + * the Hibernate session that loaded the entity. + * + *

Known limitation: calling {@code getDocumentById} (or any other service method) from + * within an already-open transaction is not covered here. When an outer transaction is active, + * the service's own {@code @Transactional} merges into it and Hibernate keeps the same session + * open, so the lazy-init guard behaves differently than in a non-transactional caller. This is a + * known constraint of the test setup, not a bug in the production code. + */ @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.NONE) @ActiveProfiles("test") @Import(PostgresContainerConfig.class) @@ -100,6 +111,7 @@ class DocumentLazyLoadingTest { }).doesNotThrowAnyException(); results.forEach(d -> assertThat(d.getSender()).isNotNull()); results.forEach(d -> assertThat(d.getSender().getLastName()).isNotNull()); + results.forEach(d -> assertThat(d.getTags()).isNotEmpty()); } @Test -- 2.49.1