fix: break Spring Framework 7 circular bean dependency cycles #426
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 ──────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user