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 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..3e8cac51 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,24 +128,27 @@ 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) + @BatchSize(size = 50) @Builder.Default private Set trainingLabels = new HashSet<>(); 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..c28a8132 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,18 @@ 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); + + @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 Optional findByOriginalFilename(String originalFilename); @@ -30,17 +44,21 @@ 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); + // 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); + // 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)") @@ -55,12 +73,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 " + @@ -75,6 +96,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 413216dc..a739764e 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)); @@ -635,7 +636,7 @@ public class DocumentService { return saved; } - // 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 +844,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..d2332519 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,9 @@ import org.raddatz.familienarchiv.user.DisplayNameFormatter; 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") @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..fc5974a6 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,13 @@ 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.*; +// prevents infinite recursion in JSON serialization; see ADR-022 for lazy-fetch context +@JsonIgnoreProperties({"hibernateLazyInitializer", "handler"}) @Entity @Data @NoArgsConstructor 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..bc33815f --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java @@ -0,0 +1,178 @@ +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.List; +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; + +/** + * 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) +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 = 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()); + + // 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(() -> { + 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 + void getRecentActivity_collectionsAccessibleAfterReturn() { + Person sender = savedPerson("Hans", "RaSender"); + Tag tag = savedTag("RaTag"); + for (int i = 0; i < 3; i++) { + savedDocument("RaDoc " + i, "ra_doc" + i + ".pdf", sender, Set.of(), Set.of(tag)); + } + + 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 -> 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()); + results.forEach(d -> assertThat(d.getTags()).isNotEmpty()); + } + + @Test + void searchDocuments_receiverSort_doesNotThrowLazyInitializationException() { + 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, + DocumentSort.RECEIVER, "asc", null, + PageRequest.of(0, 20)); + assertThat(result.totalElements()).isGreaterThan(0); + assertThatCode(() -> + result.items().forEach(i -> i.document().getSender().getLastName())) + .doesNotThrowAnyException(); + } + + @Test + void searchDocuments_senderSort_doesNotThrowLazyInitializationException() { + 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, + DocumentSort.SENDER, "asc", null, + PageRequest.of(0, 20))) + .doesNotThrowAnyException(); + } + + @Test + void dashboardService_getResume_accessesReceiversViaGetDocumentById_withoutException() { + 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()); + + 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()); + } +} 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..4b2d1b70 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,117 @@ 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.setStatisticsEnabled(true); + 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); + } + + @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); + } + + @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) { 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..d876c1b4 --- /dev/null +++ b/docs/adr/022-eager-to-lazy-fetch-strategy.md @@ -0,0 +1,110 @@ +# 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. +- **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.