fix(document): fix test assertion structure + add entity graph decision comments
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m15s
CI / OCR Service Tests (pull_request) Successful in 19s
CI / Backend Unit Tests (pull_request) Successful in 3m25s
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 1m0s

- 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 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-05-19 07:42:59 +02:00
parent 82e81e159a
commit be15de6cc6
4 changed files with 27 additions and 12 deletions

View File

@@ -44,17 +44,19 @@ public interface DocumentRepository extends JpaRepository<Document, UUID>, JpaSp
// Wie oben, gibt aber nur das erste Ergebnis zurück — sicher wenn doppelte Dateinamen existieren // Wie oben, gibt aber nur das erste Ergebnis zurück — sicher wenn doppelte Dateinamen existieren
Optional<Document> findFirstByOriginalFilename(String originalFilename); Optional<Document> findFirstByOriginalFilename(String originalFilename);
// Findet alle Dokumente mit einem bestimmten Status // Callers access only status/id scalar fields — no graph needed.
// z.B. um alle offenen "PLACEHOLDER" zu finden
List<Document> findByStatus(DocumentStatus status); List<Document> findByStatus(DocumentStatus status);
// Prüft effizient, ob ein Dateiname schon existiert (gibt true/false zurück) // Prüft effizient, ob ein Dateiname schon existiert (gibt true/false zurück)
boolean existsByOriginalFilename(String originalFilename); boolean existsByOriginalFilename(String originalFilename);
@EntityGraph("Document.full")
List<Document> findBySenderId(UUID senderId); List<Document> findBySenderId(UUID senderId);
@EntityGraph("Document.full")
List<Document> findByReceiversId(UUID receiverId); List<Document> findByReceiversId(UUID receiverId);
// Callers access only doc.getTags() to mutate the set — receivers/sender not touched; no graph needed.
List<Document> findByTags_Id(UUID tagId); List<Document> findByTags_Id(UUID tagId);
@Query("SELECT d FROM Document d WHERE d.id NOT IN (SELECT DISTINCT dv.documentId FROM DocumentVersion dv)") @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<Document, UUID>, JpaSp
long countByMetadataCompleteFalse(); long countByMetadataCompleteFalse();
// No production callers — only used if a future export path iterates the full list; no graph needed.
List<Document> findByMetadataCompleteFalse(Sort sort); List<Document> findByMetadataCompleteFalse(Sort sort);
// Callers map to IncompleteDocumentDTO using only scalar fields (id, title, createdAt) — no graph needed.
Page<Document> findByMetadataCompleteFalse(Pageable pageable); Page<Document> findByMetadataCompleteFalse(Pageable pageable);
Optional<Document> findFirstByMetadataCompleteFalseAndIdNot(UUID id, Sort sort); Optional<Document> findFirstByMetadataCompleteFalseAndIdNot(UUID id, Sort sort);
@EntityGraph("Document.full")
@Query("SELECT DISTINCT d FROM Document d " + @Query("SELECT DISTINCT d FROM Document d " +
"JOIN d.receivers r " + "JOIN d.receivers r " +
"WHERE " + "WHERE " +
@@ -89,6 +94,7 @@ public interface DocumentRepository extends JpaRepository<Document, UUID>, JpaSp
@Param("to") LocalDate to, @Param("to") LocalDate to,
Sort sort); Sort sort);
@EntityGraph("Document.full")
@Query("SELECT DISTINCT d FROM Document d " + @Query("SELECT DISTINCT d FROM Document d " +
"LEFT JOIN d.receivers r " + "LEFT JOIN d.receivers r " +
"WHERE (d.sender.id = :personId OR r.id = :personId) " + "WHERE (d.sender.id = :personId OR r.id = :personId) " +

View File

@@ -636,8 +636,6 @@ public class DocumentService {
return saved; 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) @Transactional(readOnly = true)
public List<Document> getRecentActivity(int size) { public List<Document> getRecentActivity(int size) {
return documentRepository.findAll( return documentRepository.findAll(
@@ -846,8 +844,6 @@ public class DocumentService {
documentRepository.save(doc); 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) @Transactional(readOnly = true)
public Document getDocumentById(UUID id) { public Document getDocumentById(UUID id) {
Document doc = documentRepository.findById(id) Document doc = documentRepository.findById(id)

View File

@@ -70,12 +70,16 @@ class DocumentLazyLoadingTest {
Document result = documentService.getDocumentById(doc.getId()); 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(() -> { assertThatCode(() -> {
assertThat(result.getTags()).isNotEmpty(); result.getTags().size();
result.getTags().forEach(t -> assertThat(t.getName()).isNotNull()); result.getReceivers().size();
assertThat(result.getReceivers()).isNotEmpty();
result.getReceivers().forEach(r -> assertThat(r.getLastName()).isNotNull());
}).doesNotThrowAnyException(); }).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 @Test
@@ -88,11 +92,14 @@ class DocumentLazyLoadingTest {
List<Document> results = documentService.getRecentActivity(3); List<Document> 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(() -> { assertThatCode(() -> {
results.forEach(d -> assertThat(d.getSender()).isNotNull()); results.forEach(d -> d.getSender().getLastName());
results.forEach(d -> assertThat(d.getSender().getLastName()).isNotNull());
results.forEach(d -> d.getTags().size()); results.forEach(d -> d.getTags().size());
}).doesNotThrowAnyException(); }).doesNotThrowAnyException();
results.forEach(d -> assertThat(d.getSender()).isNotNull());
results.forEach(d -> assertThat(d.getSender().getLastName()).isNotNull());
} }
@Test @Test

View File

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