From 7e6e809aa4ca42eb00c7ffa96b04e34793372c4f Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 15:55:33 +0200 Subject: [PATCH 1/6] =?UTF-8?q?fix(annotation):=20break=20AnnotationServic?= =?UTF-8?q?e=20=E2=86=94=20TranscriptionService=20cycle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spring Framework 7 prohibits constructor injection cycles even with @Lazy. Replace the TranscriptionService dependency in AnnotationService with a direct TranscriptionBlockRepository call for the cascade-delete, which is an intra-domain operation within the document package. Co-Authored-By: Claude Sonnet 4.6 --- .../document/annotation/AnnotationService.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/annotation/AnnotationService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/annotation/AnnotationService.java index ff605286..1da0dd55 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/annotation/AnnotationService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/annotation/AnnotationService.java @@ -6,12 +6,11 @@ import org.raddatz.familienarchiv.audit.AuditKind; import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.document.annotation.CreateAnnotationDTO; 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.ErrorCode; import org.raddatz.familienarchiv.document.annotation.DocumentAnnotation; import org.raddatz.familienarchiv.document.annotation.AnnotationRepository; -import org.springframework.context.annotation.Lazy; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -27,11 +26,7 @@ import java.util.UUID; public class AnnotationService { private final AnnotationRepository annotationRepository; - // @Lazy: AnnotationService and TranscriptionService have a mutual cleanup - // 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 TranscriptionBlockRepository transcriptionBlockRepository; private final AuditService auditService; public List listAnnotations(UUID documentId) { @@ -123,7 +118,7 @@ public class AnnotationService { throw DomainException.forbidden("Only the annotation author can delete it"); } - transcriptionService.deleteByAnnotationId(annotationId); + transcriptionBlockRepository.deleteByAnnotationId(annotationId); annotationRepository.delete(annotation); } -- 2.49.1 From fbbe0789d0c2a4b99079584266eacbd26c6d8422 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 15:56:05 +0200 Subject: [PATCH 2/6] =?UTF-8?q?fix(document):=20break=20DocumentService=20?= =?UTF-8?q?=E2=86=94=20ThumbnailAsyncRunner=20=E2=86=94=20ThumbnailService?= =?UTF-8?q?=20cycle?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spring Framework 7 prohibits constructor injection cycles even with @Lazy. Replace DocumentService dependencies in ThumbnailAsyncRunner and ThumbnailService with direct DocumentRepository calls — both are intra-domain reads/saves. Update ThumbnailServiceTest to mock DocumentRepository accordingly. Co-Authored-By: Claude Sonnet 4.6 --- .../document/DocumentService.java | 5 ----- .../document/ThumbnailAsyncRunner.java | 4 ++-- .../document/ThumbnailService.java | 8 +++---- .../document/ThumbnailServiceTest.java | 22 +++++++++---------- 4 files changed, 17 insertions(+), 22 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index 3e3f1b4b..a69a77e0 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -29,7 +29,6 @@ import org.raddatz.familienarchiv.ocr.TrainingLabel; import org.raddatz.familienarchiv.person.Person; import org.raddatz.familienarchiv.tag.Tag; import org.raddatz.familienarchiv.document.DocumentRepository; -import org.springframework.context.annotation.Lazy; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; @@ -77,10 +76,6 @@ public class DocumentService { private final AuditService auditService; private final TranscriptionBlockQueryService transcriptionBlockQueryService; 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; public record StoreResult(Document document, boolean isNew) {} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/ThumbnailAsyncRunner.java b/backend/src/main/java/org/raddatz/familienarchiv/document/ThumbnailAsyncRunner.java index ed239ade..3b82aff1 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/ThumbnailAsyncRunner.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/ThumbnailAsyncRunner.java @@ -28,7 +28,7 @@ import java.util.concurrent.TimeoutException; @Slf4j public class ThumbnailAsyncRunner { - private final DocumentService documentService; + private final DocumentRepository documentRepository; private final ThumbnailService thumbnailService; /** Per-document timeout for the whole generate() call — defense against corrupt PDFs. */ @@ -59,7 +59,7 @@ public class ThumbnailAsyncRunner { */ @Async("thumbnailExecutor") public void generateAsync(UUID documentId) { - Optional docOpt = documentService.findById(documentId); + Optional docOpt = documentRepository.findById(documentId); if (docOpt.isEmpty()) { log.warn("Thumbnail generation skipped: document not found id={}", documentId); return; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/ThumbnailService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/ThumbnailService.java index b937a126..26b9e67e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/ThumbnailService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/ThumbnailService.java @@ -62,16 +62,16 @@ public class ThumbnailService { private final FileService fileService; private final S3Client s3Client; - private final DocumentService documentService; + private final DocumentRepository documentRepository; @Value("${app.s3.bucket}") private String bucketName; public ThumbnailService(FileService fileService, S3Client s3Client, - DocumentService documentService) { + DocumentRepository documentRepository) { this.fileService = fileService; this.s3Client = s3Client; - this.documentService = documentService; + this.documentRepository = documentRepository; } public Outcome generate(Document doc) { @@ -167,7 +167,7 @@ public class ThumbnailService { doc.setThumbnailGeneratedAt(LocalDateTime.now()); doc.setThumbnailAspect(result.aspect()); doc.setPageCount(result.pageCount()); - documentService.updateThumbnailMetadata(doc); + documentRepository.save(doc); return Outcome.SUCCESS; } catch (Exception e) { // Thumbnail is already in S3 but the entity update failed. Because the S3 diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailServiceTest.java index 97611274..55f140bf 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailServiceTest.java @@ -39,17 +39,17 @@ class ThumbnailServiceTest { private FileService fileService; private S3Client s3Client; - private DocumentService documentService; + private DocumentRepository documentRepository; private ThumbnailService thumbnailService; @BeforeEach void setUp() { fileService = mock(FileService.class); s3Client = mock(S3Client.class); - documentService = mock(DocumentService.class); - thumbnailService = new ThumbnailService(fileService, s3Client, documentService); + documentRepository = mock(DocumentRepository.class); + thumbnailService = new ThumbnailService(fileService, s3Client, documentRepository); 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 @@ -103,7 +103,7 @@ class ThumbnailServiceTest { assertThat(doc.getThumbnailKey()).isEqualTo("thumbnails/" + doc.getId() + ".jpg"); assertThat(doc.getThumbnailGeneratedAt()).isNotNull(); - verify(documentService).updateThumbnailMetadata(doc); + verify(documentRepository).save(doc); } @Test @@ -152,7 +152,7 @@ class ThumbnailServiceTest { assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); assertThat(doc.getThumbnailKey()).isNull(); - verify(documentService, never()).updateThumbnailMetadata(any()); + verify(documentRepository, never()).save(any()); } @Test @@ -165,7 +165,7 @@ class ThumbnailServiceTest { assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); verifyNoInteractions(s3Client); - verify(documentService, never()).updateThumbnailMetadata(any()); + verify(documentRepository, never()).save(any()); } @Test @@ -260,7 +260,7 @@ class ThumbnailServiceTest { assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); verifyNoInteractions(s3Client); - verify(documentService, never()).updateThumbnailMetadata(any()); + verify(documentRepository, never()).save(any()); } @Test @@ -275,7 +275,7 @@ class ThumbnailServiceTest { assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); verifyNoInteractions(s3Client); - verify(documentService, never()).updateThumbnailMetadata(any()); + verify(documentRepository, never()).save(any()); } @Test @@ -286,14 +286,14 @@ class ThumbnailServiceTest { Document doc = makeDoc("application/pdf", "documents/letter.pdf"); when(fileService.downloadFileStream(anyString())) .thenReturn(new ByteArrayInputStream(createSamplePdf())); - when(documentService.updateThumbnailMetadata(any())) + when(documentRepository.save(any())) .thenThrow(new RuntimeException("constraint violation")); ThumbnailService.Outcome outcome = thumbnailService.generate(doc); assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); verify(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class)); - verify(documentService).updateThumbnailMetadata(any()); + verify(documentRepository).save(any()); } // ─── helpers ────────────────────────────────────────────────────────────── -- 2.49.1 From 16dacd8f4cc50dedc6f0dd3c24bf56b4925770f5 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 16:22:23 +0200 Subject: [PATCH 3/6] fix(test): update ThumbnailAsyncRunnerTest to use DocumentRepository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ThumbnailAsyncRunner was changed to inject DocumentRepository directly (breaking the DocumentService cycle), but the test still passed DocumentService to the constructor — a type mismatch that prevented the test suite from compiling. Co-Authored-By: Claude Sonnet 4.6 --- .../document/ThumbnailAsyncRunnerTest.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailAsyncRunnerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailAsyncRunnerTest.java index c281136e..4da4d2d6 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailAsyncRunnerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/ThumbnailAsyncRunnerTest.java @@ -17,22 +17,22 @@ import static org.mockito.Mockito.*; class ThumbnailAsyncRunnerTest { - private DocumentService documentService; + private DocumentRepository documentRepository; private ThumbnailService thumbnailService; private ThumbnailAsyncRunner runner; @BeforeEach void setUp() { - documentService = mock(DocumentService.class); + documentRepository = mock(DocumentRepository.class); thumbnailService = mock(ThumbnailService.class); - runner = new ThumbnailAsyncRunner(documentService, thumbnailService); + runner = new ThumbnailAsyncRunner(documentRepository, thumbnailService); } @Test void dispatchAfterCommit_whenNoTransaction_dispatchesImmediately() { UUID id = UUID.randomUUID(); 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); @@ -43,7 +43,7 @@ class ThumbnailAsyncRunnerTest { void dispatchAfterCommit_whenTransactionActive_registersAfterCommitSynchronization() { UUID id = UUID.randomUUID(); 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(); try { @@ -68,7 +68,7 @@ class ThumbnailAsyncRunnerTest { void dispatchAfterCommit_whenRollback_doesNotDispatch() { UUID id = UUID.randomUUID(); 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(); try { @@ -87,7 +87,7 @@ class ThumbnailAsyncRunnerTest { @Test void generateAsync_skipsWhenDocumentMissing() { UUID id = UUID.randomUUID(); - when(documentService.findById(id)).thenReturn(Optional.empty()); + when(documentRepository.findById(id)).thenReturn(Optional.empty()); runner.generateAsync(id); @@ -98,7 +98,7 @@ class ThumbnailAsyncRunnerTest { void generateAsync_timesOutWhenGenerateExceedsLimit() throws Exception { UUID id = UUID.randomUUID(); 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 when(thumbnailService.generate(doc)).thenAnswer(inv -> { Thread.sleep(5_000); -- 2.49.1 From 3db5b48cda2b2e969a10aeb569f3af3259088483 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 16:22:58 +0200 Subject: [PATCH 4/6] test(document): remove dead updateThumbnailMetadata test ThumbnailService now calls documentRepository.save() directly. DocumentService.updateThumbnailMetadata() has no production callers, so its test describes behaviour that no longer exists in the production path. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/document/DocumentServiceTest.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java index 3a346ec7..e3390024 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/DocumentServiceTest.java @@ -2301,17 +2301,6 @@ class DocumentServiceTest { 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 ──────────────────────────────────────────────── @Test -- 2.49.1 From ef43cba4d79d29e80c79683c4164fcc20caa5340 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 16:24:06 +0200 Subject: [PATCH 5/6] refactor(document): remove dead DocumentService.updateThumbnailMetadata() No production code calls this method since ThumbnailService was changed to write thumbnail metadata via documentRepository.save() directly. Removing the unreachable wrapper eliminates false coverage and noise during future security audits. Co-Authored-By: Claude Sonnet 4.6 --- .../org/raddatz/familienarchiv/document/DocumentService.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java index a69a77e0..8cef03a5 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/document/DocumentService.java @@ -92,10 +92,6 @@ public class DocumentService { return documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull(); } - public Document updateThumbnailMetadata(Document doc) { - return documentRepository.save(doc); - } - public Optional findByOriginalFilename(String originalFilename) { return documentRepository.findByOriginalFilename(originalFilename); } -- 2.49.1 From 548df84219c47653332dc70f1b012edc9a13111f Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 16:25:41 +0200 Subject: [PATCH 6/6] test(annotation): wire TranscriptionBlockRepository mock and add cascade test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AnnotationService was changed to call transcriptionBlockRepository directly, but the test still mocked TranscriptionService — causing a NPE and leaving the cascade path uncovered. Replace the @Mock TranscriptionService with @Mock TranscriptionBlockRepository, update the two existing delete-test verifications, and add a dedicated deleteAnnotation_cascadesToTranscriptionBlocks test. Co-Authored-By: Claude Sonnet 4.6 --- .../annotation/AnnotationServiceTest.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/document/annotation/AnnotationServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/document/annotation/AnnotationServiceTest.java index 2b81d67a..474f7f0b 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/document/annotation/AnnotationServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/document/annotation/AnnotationServiceTest.java @@ -8,7 +8,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.ArgumentCaptor; import org.raddatz.familienarchiv.audit.AuditKind; 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.UpdateAnnotationDTO; import org.raddatz.familienarchiv.exception.DomainException; @@ -36,7 +36,7 @@ import static org.springframework.http.HttpStatus.NOT_FOUND; class AnnotationServiceTest { @Mock AnnotationRepository annotationRepository; - @Mock TranscriptionService transcriptionService; + @Mock TranscriptionBlockRepository transcriptionBlockRepository; @Mock AuditService auditService; @InjectMocks AnnotationService annotationService; @@ -208,7 +208,7 @@ class AnnotationServiceTest { annotationService.deleteAnnotation(docId, annotId, ownerId); - verify(transcriptionService).deleteByAnnotationId(annotId); + verify(transcriptionBlockRepository).deleteByAnnotationId(annotId); verify(annotationRepository).delete(annotation); } @@ -225,11 +225,27 @@ class AnnotationServiceTest { annotationService.deleteAnnotation(docId, annotId, ownerId); - var inOrder = org.mockito.Mockito.inOrder(transcriptionService, annotationRepository); - inOrder.verify(transcriptionService).deleteByAnnotationId(annotId); + var inOrder = org.mockito.Mockito.inOrder(transcriptionBlockRepository, annotationRepository); + inOrder.verify(transcriptionBlockRepository).deleteByAnnotationId(annotId); 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 void deleteAnnotation_throwsForbidden_whenUserIdIsNull() { UUID docId = UUID.randomUUID(); -- 2.49.1