Compare commits
17 Commits
worktree-f
...
worktree-f
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
82e81e159a | ||
|
|
cabcf6a6ca | ||
|
|
befecc9864 | ||
|
|
57259e4195 | ||
|
|
3356e27273 | ||
|
|
050cd0adf8 | ||
|
|
29eaa253a6 | ||
|
|
3358f0509f | ||
|
|
667b237c33 | ||
|
|
e677756139 | ||
|
|
108ab1a288 | ||
|
|
5f83fa1bc2 | ||
|
|
0dd7d8a5e7 | ||
|
|
e13b37d585 | ||
|
|
b8505e0de5 | ||
|
|
b88573c432 | ||
|
|
ff792e6625 |
@@ -97,7 +97,10 @@ public class MyEntity {
|
|||||||
|
|
||||||
- Annotated with `@Service`, `@RequiredArgsConstructor`, optionally `@Slf4j`.
|
- Annotated with `@Service`, `@RequiredArgsConstructor`, optionally `@Slf4j`.
|
||||||
- Write methods: `@Transactional`.
|
- 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.
|
- Cross-domain access goes through the other domain's service, never its repository.
|
||||||
|
|
||||||
## Error Handling
|
## Error Handling
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.document;
|
|||||||
|
|
||||||
import jakarta.persistence.*;
|
import jakarta.persistence.*;
|
||||||
import lombok.*;
|
import lombok.*;
|
||||||
|
import org.hibernate.annotations.BatchSize;
|
||||||
import org.hibernate.annotations.CreationTimestamp;
|
import org.hibernate.annotations.CreationTimestamp;
|
||||||
import org.hibernate.annotations.UpdateTimestamp;
|
import org.hibernate.annotations.UpdateTimestamp;
|
||||||
|
|
||||||
@@ -21,6 +22,15 @@ import java.util.HashSet;
|
|||||||
import java.util.Set;
|
import java.util.Set;
|
||||||
import java.util.UUID;
|
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
|
@Entity
|
||||||
@Table(name = "documents")
|
@Table(name = "documents")
|
||||||
@Data // Lombok: Generiert Getter, Setter, ToString, etc.
|
@Data // Lombok: Generiert Getter, Setter, ToString, etc.
|
||||||
@@ -118,24 +128,27 @@ public class Document {
|
|||||||
@Builder.Default
|
@Builder.Default
|
||||||
private ScriptType scriptType = ScriptType.UNKNOWN;
|
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"))
|
@JoinTable(name = "document_receivers", joinColumns = @JoinColumn(name = "document_id"), inverseJoinColumns = @JoinColumn(name = "person_id"))
|
||||||
|
@BatchSize(size = 50)
|
||||||
@Builder.Default
|
@Builder.Default
|
||||||
private Set<Person> receivers = new HashSet<>();
|
private Set<Person> receivers = new HashSet<>();
|
||||||
|
|
||||||
@ManyToOne
|
@ManyToOne(fetch = FetchType.LAZY)
|
||||||
@JoinColumn(name = "sender_id")
|
@JoinColumn(name = "sender_id")
|
||||||
private Person sender;
|
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"))
|
@JoinTable(name = "document_tags", joinColumns = @JoinColumn(name = "document_id"), inverseJoinColumns = @JoinColumn(name = "tag_id"))
|
||||||
|
@BatchSize(size = 50)
|
||||||
@Builder.Default
|
@Builder.Default
|
||||||
private Set<Tag> tags = new HashSet<>();
|
private Set<Tag> tags = new HashSet<>();
|
||||||
|
|
||||||
@ElementCollection(fetch = FetchType.EAGER)
|
@ElementCollection(fetch = FetchType.LAZY)
|
||||||
@CollectionTable(name = "document_training_labels", joinColumns = @JoinColumn(name = "document_id"))
|
@CollectionTable(name = "document_training_labels", joinColumns = @JoinColumn(name = "document_id"))
|
||||||
@Column(name = "label")
|
@Column(name = "label")
|
||||||
@Enumerated(EnumType.STRING)
|
@Enumerated(EnumType.STRING)
|
||||||
|
@BatchSize(size = 50)
|
||||||
@Builder.Default
|
@Builder.Default
|
||||||
private Set<TrainingLabel> trainingLabels = new HashSet<>();
|
private Set<TrainingLabel> trainingLabels = new HashSet<>();
|
||||||
|
|
||||||
|
|||||||
@@ -7,6 +7,8 @@ import org.raddatz.familienarchiv.document.DocumentStatus;
|
|||||||
import org.springframework.data.domain.Page;
|
import org.springframework.data.domain.Page;
|
||||||
import org.springframework.data.domain.Pageable;
|
import org.springframework.data.domain.Pageable;
|
||||||
import org.springframework.data.domain.Sort;
|
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.JpaRepository;
|
||||||
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
|
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
|
||||||
import org.springframework.data.jpa.repository.Query;
|
import org.springframework.data.jpa.repository.Query;
|
||||||
@@ -23,6 +25,18 @@ import java.util.UUID;
|
|||||||
@Repository
|
@Repository
|
||||||
public interface DocumentRepository extends JpaRepository<Document, UUID>, JpaSpecificationExecutor<Document> {
|
public interface DocumentRepository extends JpaRepository<Document, UUID>, JpaSpecificationExecutor<Document> {
|
||||||
|
|
||||||
|
@EntityGraph("Document.full")
|
||||||
|
Optional<Document> findById(UUID id);
|
||||||
|
|
||||||
|
@EntityGraph("Document.list")
|
||||||
|
Page<Document> findAll(Specification<Document> spec, Pageable pageable);
|
||||||
|
|
||||||
|
@EntityGraph("Document.list")
|
||||||
|
List<Document> findAll(Specification<Document> spec);
|
||||||
|
|
||||||
|
@EntityGraph("Document.list")
|
||||||
|
Page<Document> findAll(Pageable pageable);
|
||||||
|
|
||||||
// Findet ein Dokument anhand des ursprünglichen Dateinamens
|
// Findet ein Dokument anhand des ursprünglichen Dateinamens
|
||||||
// Wichtig für den Abgleich beim Excel-Import & Datei-Upload
|
// Wichtig für den Abgleich beim Excel-Import & Datei-Upload
|
||||||
Optional<Document> findByOriginalFilename(String originalFilename);
|
Optional<Document> findByOriginalFilename(String originalFilename);
|
||||||
|
|||||||
@@ -447,6 +447,7 @@ public class DocumentService {
|
|||||||
return saved;
|
return saved;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Transactional
|
||||||
public Document updateDocumentTags(UUID docId, List<String> tagNames) {
|
public Document updateDocumentTags(UUID docId, List<String> tagNames) {
|
||||||
Document doc = documentRepository.findById(docId)
|
Document doc = documentRepository.findById(docId)
|
||||||
.orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + docId));
|
.orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + docId));
|
||||||
@@ -635,7 +636,9 @@ public class DocumentService {
|
|||||||
return saved;
|
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<Document> getRecentActivity(int size) {
|
public List<Document> getRecentActivity(int size) {
|
||||||
return documentRepository.findAll(
|
return documentRepository.findAll(
|
||||||
PageRequest.of(0, size, Sort.by(Sort.Direction.DESC, "updatedAt"))
|
PageRequest.of(0, size, Sort.by(Sort.Direction.DESC, "updatedAt"))
|
||||||
@@ -843,6 +846,9 @@ 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)
|
||||||
public Document getDocumentById(UUID id) {
|
public Document getDocumentById(UUID id) {
|
||||||
Document doc = documentRepository.findById(id)
|
Document doc = documentRepository.findById(id)
|
||||||
.orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id));
|
.orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + id));
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package org.raddatz.familienarchiv.person;
|
package org.raddatz.familienarchiv.person;
|
||||||
|
|
||||||
import com.fasterxml.jackson.annotation.JsonIgnore;
|
import com.fasterxml.jackson.annotation.JsonIgnore;
|
||||||
|
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
|
||||||
import io.swagger.v3.oas.annotations.media.Schema;
|
import io.swagger.v3.oas.annotations.media.Schema;
|
||||||
import jakarta.persistence.*;
|
import jakarta.persistence.*;
|
||||||
import lombok.*;
|
import lombok.*;
|
||||||
@@ -9,6 +10,8 @@ import org.raddatz.familienarchiv.user.DisplayNameFormatter;
|
|||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
|
@JsonIgnoreProperties({"hibernateLazyInitializer", "handler"})
|
||||||
@Entity
|
@Entity
|
||||||
@Table(name = "persons")
|
@Table(name = "persons")
|
||||||
@Data
|
@Data
|
||||||
|
|||||||
@@ -2,10 +2,12 @@ package org.raddatz.familienarchiv.tag;
|
|||||||
|
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
|
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
|
||||||
import io.swagger.v3.oas.annotations.media.Schema;
|
import io.swagger.v3.oas.annotations.media.Schema;
|
||||||
import jakarta.persistence.*;
|
import jakarta.persistence.*;
|
||||||
import lombok.*;
|
import lombok.*;
|
||||||
|
|
||||||
|
@JsonIgnoreProperties({"hibernateLazyInitializer", "handler"})
|
||||||
@Entity
|
@Entity
|
||||||
@Data
|
@Data
|
||||||
@NoArgsConstructor
|
@NoArgsConstructor
|
||||||
|
|||||||
@@ -0,0 +1,159 @@
|
|||||||
|
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;
|
||||||
|
|
||||||
|
@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());
|
||||||
|
|
||||||
|
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_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<Document> 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
|
||||||
|
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<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());
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1,5 +1,9 @@
|
|||||||
package org.raddatz.familienarchiv.document;
|
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.junit.jupiter.api.Test;
|
||||||
import org.raddatz.familienarchiv.PostgresContainerConfig;
|
import org.raddatz.familienarchiv.PostgresContainerConfig;
|
||||||
import org.raddatz.familienarchiv.config.FlywayConfig;
|
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.jdbc.test.autoconfigure.AutoConfigureTestDatabase;
|
||||||
import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest;
|
import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest;
|
||||||
import org.springframework.context.annotation.Import;
|
import org.springframework.context.annotation.Import;
|
||||||
|
import org.springframework.data.jpa.domain.Specification;
|
||||||
|
|
||||||
import org.springframework.data.domain.Page;
|
import org.springframework.data.domain.Page;
|
||||||
import org.springframework.data.domain.PageRequest;
|
import org.springframework.data.domain.PageRequest;
|
||||||
@@ -55,6 +60,12 @@ class DocumentRepositoryTest {
|
|||||||
@Autowired
|
@Autowired
|
||||||
private TranscriptionBlockRepository transcriptionBlockRepository;
|
private TranscriptionBlockRepository transcriptionBlockRepository;
|
||||||
|
|
||||||
|
@Autowired
|
||||||
|
private EntityManagerFactory entityManagerFactory;
|
||||||
|
|
||||||
|
@Autowired
|
||||||
|
private EntityManager entityManager;
|
||||||
|
|
||||||
// ─── save and findById ────────────────────────────────────────────────────
|
// ─── save and findById ────────────────────────────────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -490,6 +501,117 @@ class DocumentRepositoryTest {
|
|||||||
assertThat(ids).containsExactlyInAnyOrder(grandparent.getId(), parent2.getId(), child2.getId());
|
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<Document> 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<Person> receivers = new HashSet<>();
|
||||||
|
for (int i = 0; i < 3; i++) {
|
||||||
|
receivers.add(personRepository.save(
|
||||||
|
Person.builder().firstName("R" + i).lastName("FbReceiver").build()));
|
||||||
|
}
|
||||||
|
Set<Tag> 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<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 ─────────────────────────────────────────────────────
|
// ─── seeding helpers ─────────────────────────────────────────────────────
|
||||||
|
|
||||||
private Document uploaded(String title) {
|
private Document uploaded(String title) {
|
||||||
|
|||||||
104
docs/adr/022-eager-to-lazy-fetch-strategy.md
Normal file
104
docs/adr/022-eager-to-lazy-fetch-strategy.md
Normal 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 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.
|
||||||
Reference in New Issue
Block a user