marcel/familienarchiv
1
0
Fork 0
Code Issues 109 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(document): fix test assertion structure + add entity graph decision comments

Browse Source 
- 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
committed by marcel
parent 328bd2c3b4
commit 7342c60952
4 changed files with 27 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
backend/src/main/java/org/raddatz/familienarchiv/document/DocumentRepository.java
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
Optional<Document> 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<Document> findByStatus(DocumentStatus status);
// Prüft effizient, ob ein Dateiname schon existiert (gibt true/false zurück)
boolean existsByOriginalFilename(String originalFilename);
@EntityGraph("Document.full")
List<Document> findBySenderId(UUID senderId);
@EntityGraph("Document.full")
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);
@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();
// No production callers — only used if a future export path iterates the full list; no graph needed.
List<Document> findByMetadataCompleteFalse(Sort sort);
// Callers map to IncompleteDocumentDTO using only scalar fields (id, title, createdAt) — no graph needed.
Page<Document> findByMetadataCompleteFalse(Pageable pageable);
Optional<Document> 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<Document, UUID>, 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) " +

4
backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java
View File

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

19
backend/src/test/java/org/raddatz/familienarchiv/document/DocumentLazyLoadingTest.java
View File

@@ -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<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(() -> {
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