Compare commits

...

6 Commits

Author SHA1 Message Date
Marcel
82e81e159a docs(backend): document @Transactional(readOnly=true) exception in CLAUDE.md
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m42s
CI / OCR Service Tests (pull_request) Successful in 21s
CI / Backend Unit Tests (pull_request) Successful in 3m17s
CI / fail2ban Regex (pull_request) Successful in 42s
CI / Semgrep Security Scan (pull_request) Successful in 20s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m1s
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 <noreply@anthropic.com>
2026-05-18 22:26:24 +02:00
Marcel
cabcf6a6ca docs(adr): add ADR-022 for EAGER→LAZY fetch strategy with @EntityGraph
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 <noreply@anthropic.com>
2026-05-18 22:25:38 +02:00
Marcel
befecc9864 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 <noreply@anthropic.com>
2026-05-18 22:24:17 +02:00
Marcel
57259e4195 test(document): add query-count assertion for findAll(Spec) non-paginated path
List<Document> 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 <noreply@anthropic.com>
2026-05-18 22:22:50 +02:00
Marcel
3356e27273 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 <noreply@anthropic.com>
2026-05-18 22:21:31 +02:00
Marcel
050cd0adf8 test(document): enable statistics before findById query-count assertion
Without setStatisticsEnabled(true) the counter stays 0 and ≤2 passes
vacuously when the test runs in isolation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-18 22:19:32 +02:00
4 changed files with 176 additions and 47 deletions

View File

@@ -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

View File

@@ -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<Document> results = documentService.getRecentActivity(3);
@@ -108,34 +97,26 @@ 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));
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();
}
@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,
@@ -146,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());
@@ -161,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<Person> receivers, Set<Tag> tags) {
return documentRepository.save(Document.builder()
.title(title).originalFilename(filename)
.status(DocumentStatus.UPLOADED)
.sender(sender)
.receivers(new HashSet<>(receivers))
.tags(new HashSet<>(tags))
.build());
}
}

View File

@@ -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());
@@ -585,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<Document> 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) {

View File

@@ -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 12 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.