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
8 changed files with 49 additions and 58 deletions

View File

@@ -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) {}
@@ -97,10 +92,6 @@ public class DocumentService {
return documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull();
}
public Document updateThumbnailMetadata(Document doc) {
return documentRepository.save(doc);
}
public Optional<Document> findByOriginalFilename(String originalFilename) {
return documentRepository.findByOriginalFilename(originalFilename);
}

View File

@@ -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<Document> docOpt = documentService.findById(documentId);
Optional<Document> docOpt = documentRepository.findById(documentId);
if (docOpt.isEmpty()) {
log.warn("Thumbnail generation skipped: document not found id={}", documentId);
return;

View File

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

View File

@@ -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<DocumentAnnotation> 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);
}

View File

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

View File

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

View File

@@ -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 ──────────────────────────────────────────────────────────────

View File

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