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

fix: break Spring Framework 7 circular bean dependency cycles #426

Merged
marcel merged 6 commits from fix/spring7-circular-dependency-cycles into main 2026-05-05 16:39:38 +02:00
Conversation 8 Commits 6 Files Changed 8 +49 -58
8 changed files with 49 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -29,7 +29,6 @@ import org.raddatz.familienarchiv.ocr.TrainingLabel;
import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.person.Person;
import org.raddatz.familienarchiv.tag.Tag; import org.raddatz.familienarchiv.tag.Tag;
import org.raddatz.familienarchiv.document.DocumentRepository; import org.raddatz.familienarchiv.document.DocumentRepository;
import org.springframework.context.annotation.Lazy;
import org.springframework.data.domain.Page; import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Pageable;
@@ -77,10 +76,6 @@ public class DocumentService {
private final AuditService auditService; private final AuditService auditService;
private final TranscriptionBlockQueryService transcriptionBlockQueryService; private final TranscriptionBlockQueryService transcriptionBlockQueryService;
private final AuditLogQueryService auditLogQueryService; private final AuditLogQueryService auditLogQueryService;
// @Lazy breaks the DocumentService ↔ ThumbnailAsyncRunner cycle: the runner
// now reaches Document data through DocumentService (per the layering rule),
// and Spring needs a proxy here to defer the back-edge until both beans exist.
@Lazy
private final ThumbnailAsyncRunner thumbnailAsyncRunner; private final ThumbnailAsyncRunner thumbnailAsyncRunner;
public record StoreResult(Document document, boolean isNew) {} public record StoreResult(Document document, boolean isNew) {}
@@ -97,10 +92,6 @@ public class DocumentService {
return documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull(); return documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull();
} }
public Document updateThumbnailMetadata(Document doc) {
return documentRepository.save(doc);
}
public Optional<Document> findByOriginalFilename(String originalFilename) { public Optional<Document> findByOriginalFilename(String originalFilename) {
return documentRepository.findByOriginalFilename(originalFilename); return documentRepository.findByOriginalFilename(originalFilename);
} }

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

@@ -28,7 +28,7 @@ import java.util.concurrent.TimeoutException;
@Slf4j @Slf4j
public class ThumbnailAsyncRunner { public class ThumbnailAsyncRunner {
private final DocumentService documentService; private final DocumentRepository documentRepository;
private final ThumbnailService thumbnailService; private final ThumbnailService thumbnailService;
/** Per-document timeout for the whole generate() call — defense against corrupt PDFs. */ /** Per-document timeout for the whole generate() call — defense against corrupt PDFs. */
@@ -59,7 +59,7 @@ public class ThumbnailAsyncRunner {
*/ */
@Async("thumbnailExecutor") @Async("thumbnailExecutor")
public void generateAsync(UUID documentId) { public void generateAsync(UUID documentId) {
Optional<Document> docOpt = documentService.findById(documentId); Optional<Document> docOpt = documentRepository.findById(documentId);
if (docOpt.isEmpty()) { if (docOpt.isEmpty()) {
log.warn("Thumbnail generation skipped: document not found id={}", documentId); log.warn("Thumbnail generation skipped: document not found id={}", documentId);
return; return;

8
backend/src/main/java/org/raddatz/familienarchiv/document/ThumbnailService.java
View File

@@ -62,16 +62,16 @@ public class ThumbnailService {
private final FileService fileService; private final FileService fileService;
private final S3Client s3Client; private final S3Client s3Client;
private final DocumentService documentService; private final DocumentRepository documentRepository;
@Value("${app.s3.bucket}") @Value("${app.s3.bucket}")
private String bucketName; private String bucketName;
public ThumbnailService(FileService fileService, S3Client s3Client, public ThumbnailService(FileService fileService, S3Client s3Client,
DocumentService documentService) { DocumentRepository documentRepository) {
this.fileService = fileService; this.fileService = fileService;
this.s3Client = s3Client; this.s3Client = s3Client;
this.documentService = documentService; this.documentRepository = documentRepository;
} }
public Outcome generate(Document doc) { public Outcome generate(Document doc) {
@@ -167,7 +167,7 @@ public class ThumbnailService {
doc.setThumbnailGeneratedAt(LocalDateTime.now()); doc.setThumbnailGeneratedAt(LocalDateTime.now());
doc.setThumbnailAspect(result.aspect()); doc.setThumbnailAspect(result.aspect());
doc.setPageCount(result.pageCount()); doc.setPageCount(result.pageCount());
documentService.updateThumbnailMetadata(doc); documentRepository.save(doc);
return Outcome.SUCCESS; return Outcome.SUCCESS;
} catch (Exception e) { } catch (Exception e) {
// Thumbnail is already in S3 but the entity update failed. Because the S3 // Thumbnail is already in S3 but the entity update failed. Because the S3

11
backend/src/main/java/org/raddatz/familienarchiv/document/annotation/AnnotationService.java
View File

@@ -6,12 +6,11 @@ import org.raddatz.familienarchiv.audit.AuditKind;
import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.audit.AuditService;
import org.raddatz.familienarchiv.document.annotation.CreateAnnotationDTO; import org.raddatz.familienarchiv.document.annotation.CreateAnnotationDTO;
import org.raddatz.familienarchiv.document.annotation.UpdateAnnotationDTO; import org.raddatz.familienarchiv.document.annotation.UpdateAnnotationDTO;
import org.raddatz.familienarchiv.document.transcription.TranscriptionService; import org.raddatz.familienarchiv.document.transcription.TranscriptionBlockRepository;
import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.DomainException;
import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.exception.ErrorCode;
import org.raddatz.familienarchiv.document.annotation.DocumentAnnotation; import org.raddatz.familienarchiv.document.annotation.DocumentAnnotation;
import org.raddatz.familienarchiv.document.annotation.AnnotationRepository; import org.raddatz.familienarchiv.document.annotation.AnnotationRepository;
import org.springframework.context.annotation.Lazy;
import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.stereotype.Service; import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.annotation.Transactional;
@@ -27,11 +26,7 @@ import java.util.UUID;
public class AnnotationService { public class AnnotationService {
private final AnnotationRepository annotationRepository; private final AnnotationRepository annotationRepository;
// @Lazy: AnnotationService and TranscriptionService have a mutual cleanup private final TranscriptionBlockRepository transcriptionBlockRepository;
// dependency (deleting an annotation cascades to its blocks; deleting a block
// cascades to its annotation). Lazy resolution lets Spring construct both beans.
@Lazy
private final TranscriptionService transcriptionService;
private final AuditService auditService; private final AuditService auditService;
public List<DocumentAnnotation> listAnnotations(UUID documentId) { public List<DocumentAnnotation> listAnnotations(UUID documentId) {
@@ -123,7 +118,7 @@ public class AnnotationService {
throw DomainException.forbidden("Only the annotation author can delete it"); throw DomainException.forbidden("Only the annotation author can delete it");
} }
transcriptionService.deleteByAnnotationId(annotationId); transcriptionBlockRepository.deleteByAnnotationId(annotationId);
annotationRepository.delete(annotation); annotationRepository.delete(annotation);
} }

11
backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java
View File

@@ -2301,17 +2301,6 @@ class DocumentServiceTest {
assertThat(documentService.findForThumbnailBackfill()).containsExactly(a, b); assertThat(documentService.findForThumbnailBackfill()).containsExactly(a, b);
} }
// ─── updateThumbnailMetadata ───────────────────────────────────────────────
@Test
void updateThumbnailMetadata_savesDocument() {
Document doc = Document.builder().id(UUID.randomUUID()).title("T").build();
when(documentRepository.save(doc)).thenReturn(doc);
assertThat(documentService.updateThumbnailMetadata(doc)).isEqualTo(doc);
verify(documentRepository).save(doc);
}
// ─── findByOriginalFilename ──────────────────────────────────────────────── // ─── findByOriginalFilename ────────────────────────────────────────────────
@Test @Test

16
backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailAsyncRunnerTest.java
View File

@@ -17,22 +17,22 @@ import static org.mockito.Mockito.*;
class ThumbnailAsyncRunnerTest { class ThumbnailAsyncRunnerTest {
private DocumentService documentService; private DocumentRepository documentRepository;
private ThumbnailService thumbnailService; private ThumbnailService thumbnailService;
private ThumbnailAsyncRunner runner; private ThumbnailAsyncRunner runner;
@BeforeEach @BeforeEach
void setUp() { void setUp() {
documentService = mock(DocumentService.class); documentRepository = mock(DocumentRepository.class);
thumbnailService = mock(ThumbnailService.class); thumbnailService = mock(ThumbnailService.class);
runner = new ThumbnailAsyncRunner(documentService, thumbnailService); runner = new ThumbnailAsyncRunner(documentRepository, thumbnailService);
} }
@Test @Test
void dispatchAfterCommit_whenNoTransaction_dispatchesImmediately() { void dispatchAfterCommit_whenNoTransaction_dispatchesImmediately() {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
Document doc = Document.builder().id(id).originalFilename("f.pdf").title("t").build(); Document doc = Document.builder().id(id).originalFilename("f.pdf").title("t").build();
when(documentService.findById(id)).thenReturn(Optional.of(doc)); when(documentRepository.findById(id)).thenReturn(Optional.of(doc));
runner.dispatchAfterCommit(id); runner.dispatchAfterCommit(id);
@@ -43,7 +43,7 @@ class ThumbnailAsyncRunnerTest {
void dispatchAfterCommit_whenTransactionActive_registersAfterCommitSynchronization() { void dispatchAfterCommit_whenTransactionActive_registersAfterCommitSynchronization() {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
Document doc = Document.builder().id(id).originalFilename("f.pdf").title("t").build(); Document doc = Document.builder().id(id).originalFilename("f.pdf").title("t").build();
when(documentService.findById(id)).thenReturn(Optional.of(doc)); when(documentRepository.findById(id)).thenReturn(Optional.of(doc));
TransactionSynchronizationManager.initSynchronization(); TransactionSynchronizationManager.initSynchronization();
try { try {
@@ -68,7 +68,7 @@ class ThumbnailAsyncRunnerTest {
void dispatchAfterCommit_whenRollback_doesNotDispatch() { void dispatchAfterCommit_whenRollback_doesNotDispatch() {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
Document doc = Document.builder().id(id).originalFilename("f.pdf").title("t").build(); Document doc = Document.builder().id(id).originalFilename("f.pdf").title("t").build();
when(documentService.findById(id)).thenReturn(Optional.of(doc)); when(documentRepository.findById(id)).thenReturn(Optional.of(doc));
TransactionSynchronizationManager.initSynchronization(); TransactionSynchronizationManager.initSynchronization();
try { try {
@@ -87,7 +87,7 @@ class ThumbnailAsyncRunnerTest {
@Test @Test
void generateAsync_skipsWhenDocumentMissing() { void generateAsync_skipsWhenDocumentMissing() {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
when(documentService.findById(id)).thenReturn(Optional.empty()); when(documentRepository.findById(id)).thenReturn(Optional.empty());
runner.generateAsync(id); runner.generateAsync(id);
@@ -98,7 +98,7 @@ class ThumbnailAsyncRunnerTest {
void generateAsync_timesOutWhenGenerateExceedsLimit() throws Exception { void generateAsync_timesOutWhenGenerateExceedsLimit() throws Exception {
UUID id = UUID.randomUUID(); UUID id = UUID.randomUUID();
Document doc = Document.builder().id(id).originalFilename("f.pdf").title("t").build(); Document doc = Document.builder().id(id).originalFilename("f.pdf").title("t").build();
when(documentService.findById(id)).thenReturn(Optional.of(doc)); when(documentRepository.findById(id)).thenReturn(Optional.of(doc));
// generate sleeps longer than the timeout — simulates a hung PDFBox render // generate sleeps longer than the timeout — simulates a hung PDFBox render
when(thumbnailService.generate(doc)).thenAnswer(inv -> { when(thumbnailService.generate(doc)).thenAnswer(inv -> {
Thread.sleep(5_000); Thread.sleep(5_000);

22
backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailServiceTest.java
View File

@@ -39,17 +39,17 @@ class ThumbnailServiceTest {
private FileService fileService; private FileService fileService;
private S3Client s3Client; private S3Client s3Client;
private DocumentService documentService; private DocumentRepository documentRepository;
private ThumbnailService thumbnailService; private ThumbnailService thumbnailService;
@BeforeEach @BeforeEach
void setUp() { void setUp() {
fileService = mock(FileService.class); fileService = mock(FileService.class);
s3Client = mock(S3Client.class); s3Client = mock(S3Client.class);
documentService = mock(DocumentService.class); documentRepository = mock(DocumentRepository.class);
thumbnailService = new ThumbnailService(fileService, s3Client, documentService); thumbnailService = new ThumbnailService(fileService, s3Client, documentRepository);
ReflectionTestUtils.setField(thumbnailService, "bucketName", "test-bucket"); ReflectionTestUtils.setField(thumbnailService, "bucketName", "test-bucket");
when(documentService.updateThumbnailMetadata(any(Document.class))).thenAnswer(i -> i.getArgument(0)); when(documentRepository.save(any(Document.class))).thenAnswer(i -> i.getArgument(0));
} }
@Test @Test
@@ -103,7 +103,7 @@ class ThumbnailServiceTest {
assertThat(doc.getThumbnailKey()).isEqualTo("thumbnails/" + doc.getId() + ".jpg"); assertThat(doc.getThumbnailKey()).isEqualTo("thumbnails/" + doc.getId() + ".jpg");
assertThat(doc.getThumbnailGeneratedAt()).isNotNull(); assertThat(doc.getThumbnailGeneratedAt()).isNotNull();
verify(documentService).updateThumbnailMetadata(doc); verify(documentRepository).save(doc);
} }
@Test @Test
@@ -152,7 +152,7 @@ class ThumbnailServiceTest {
assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED);
assertThat(doc.getThumbnailKey()).isNull(); assertThat(doc.getThumbnailKey()).isNull();
verify(documentService, never()).updateThumbnailMetadata(any()); verify(documentRepository, never()).save(any());
} }
@Test @Test
@@ -165,7 +165,7 @@ class ThumbnailServiceTest {
assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED);
verifyNoInteractions(s3Client); verifyNoInteractions(s3Client);
verify(documentService, never()).updateThumbnailMetadata(any()); verify(documentRepository, never()).save(any());
} }
@Test @Test
@@ -260,7 +260,7 @@ class ThumbnailServiceTest {
assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED);
verifyNoInteractions(s3Client); verifyNoInteractions(s3Client);
verify(documentService, never()).updateThumbnailMetadata(any()); verify(documentRepository, never()).save(any());
} }
@Test @Test
@@ -275,7 +275,7 @@ class ThumbnailServiceTest {
assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED);
verifyNoInteractions(s3Client); verifyNoInteractions(s3Client);
verify(documentService, never()).updateThumbnailMetadata(any()); verify(documentRepository, never()).save(any());
} }
@Test @Test
@@ -286,14 +286,14 @@ class ThumbnailServiceTest {
Document doc = makeDoc("application/pdf", "documents/letter.pdf"); Document doc = makeDoc("application/pdf", "documents/letter.pdf");
when(fileService.downloadFileStream(anyString())) when(fileService.downloadFileStream(anyString()))
.thenReturn(new ByteArrayInputStream(createSamplePdf())); .thenReturn(new ByteArrayInputStream(createSamplePdf()));
when(documentService.updateThumbnailMetadata(any())) when(documentRepository.save(any()))
.thenThrow(new RuntimeException("constraint violation")); .thenThrow(new RuntimeException("constraint violation"));
ThumbnailService.Outcome outcome = thumbnailService.generate(doc); ThumbnailService.Outcome outcome = thumbnailService.generate(doc);
assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED);
verify(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class)); verify(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class));
verify(documentService).updateThumbnailMetadata(any()); verify(documentRepository).save(any());
} }
// ─── helpers ────────────────────────────────────────────────────────────── // ─── helpers ──────────────────────────────────────────────────────────────

26
backend/src/test/java/org/raddatz/familienarchiv/document/annotation/AnnotationServiceTest.java
View File

@@ -8,7 +8,7 @@ import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
import org.raddatz.familienarchiv.audit.AuditKind; import org.raddatz.familienarchiv.audit.AuditKind;
import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.audit.AuditService;
import org.raddatz.familienarchiv.document.transcription.TranscriptionService; import org.raddatz.familienarchiv.document.transcription.TranscriptionBlockRepository;
import org.raddatz.familienarchiv.document.annotation.CreateAnnotationDTO; import org.raddatz.familienarchiv.document.annotation.CreateAnnotationDTO;
import org.raddatz.familienarchiv.document.annotation.UpdateAnnotationDTO; import org.raddatz.familienarchiv.document.annotation.UpdateAnnotationDTO;
import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.DomainException;
@@ -36,7 +36,7 @@ import static org.springframework.http.HttpStatus.NOT_FOUND;
class AnnotationServiceTest { class AnnotationServiceTest {
@Mock AnnotationRepository annotationRepository; @Mock AnnotationRepository annotationRepository;
@Mock TranscriptionService transcriptionService; @Mock TranscriptionBlockRepository transcriptionBlockRepository;
@Mock AuditService auditService; @Mock AuditService auditService;
@InjectMocks AnnotationService annotationService; @InjectMocks AnnotationService annotationService;
@@ -208,7 +208,7 @@ class AnnotationServiceTest {
annotationService.deleteAnnotation(docId, annotId, ownerId); annotationService.deleteAnnotation(docId, annotId, ownerId);
verify(transcriptionService).deleteByAnnotationId(annotId); verify(transcriptionBlockRepository).deleteByAnnotationId(annotId);
verify(annotationRepository).delete(annotation); verify(annotationRepository).delete(annotation);
} }
@@ -225,11 +225,27 @@ class AnnotationServiceTest {
annotationService.deleteAnnotation(docId, annotId, ownerId); annotationService.deleteAnnotation(docId, annotId, ownerId);
var inOrder = org.mockito.Mockito.inOrder(transcriptionService, annotationRepository); var inOrder = org.mockito.Mockito.inOrder(transcriptionBlockRepository, annotationRepository);
inOrder.verify(transcriptionService).deleteByAnnotationId(annotId); inOrder.verify(transcriptionBlockRepository).deleteByAnnotationId(annotId);
inOrder.verify(annotationRepository).delete(annotation); inOrder.verify(annotationRepository).delete(annotation);
} }
@Test
void deleteAnnotation_cascadesToTranscriptionBlocks() {
UUID docId = UUID.randomUUID();
UUID annotId = UUID.randomUUID();
UUID ownerId = UUID.randomUUID();
DocumentAnnotation annotation = DocumentAnnotation.builder()
.id(annotId).documentId(docId).createdBy(ownerId).build();
when(annotationRepository.findByIdAndDocumentId(annotId, docId))
.thenReturn(Optional.of(annotation));
annotationService.deleteAnnotation(docId, annotId, ownerId);
verify(transcriptionBlockRepository).deleteByAnnotationId(annotId);
}
@Test @Test
void deleteAnnotation_throwsForbidden_whenUserIdIsNull() { void deleteAnnotation_throwsForbidden_whenUserIdIsNull() {
UUID docId = UUID.randomUUID(); UUID docId = UUID.randomUUID();