refactor(transcription/annotation): break mutual repo dependency
Some checks failed
CI / Unit & Component Tests (push) Failing after 4m2s
CI / OCR Service Tests (push) Successful in 42s
CI / Backend Unit Tests (push) Failing after 3m17s
CI / Unit & Component Tests (pull_request) Failing after 3m49s
CI / OCR Service Tests (pull_request) Successful in 39s
CI / Backend Unit Tests (pull_request) Failing after 3m17s
Some checks failed
CI / Unit & Component Tests (push) Failing after 4m2s
CI / OCR Service Tests (push) Successful in 42s
CI / Backend Unit Tests (push) Failing after 3m17s
CI / Unit & Component Tests (pull_request) Failing after 3m49s
CI / OCR Service Tests (pull_request) Successful in 39s
CI / Backend Unit Tests (pull_request) Failing after 3m17s
TranscriptionService injected AnnotationRepository; AnnotationService injected TranscriptionBlockRepository. Each side now talks through the other domain's service: - TranscriptionService.deleteByAnnotationId — new write delegation; called from AnnotationService.deleteAnnotation in place of the foreign repo. - AnnotationService.deleteById / deleteAllById — new write delegations; called from TranscriptionService for cascading annotation cleanup. - AnnotationService.findById (added in #417 commit 6) replaces the read. - @Lazy on AnnotationService's TranscriptionService field breaks the resulting two-bean cycle at construction time, mirroring the existing @Lazy self-reference pattern in SenderModelService. Refs #417 (C6.2 violations #10 and #11). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -10,7 +10,7 @@ import org.raddatz.familienarchiv.exception.DomainException;
|
|||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
import org.raddatz.familienarchiv.model.DocumentAnnotation;
|
import org.raddatz.familienarchiv.model.DocumentAnnotation;
|
||||||
import org.raddatz.familienarchiv.repository.AnnotationRepository;
|
import org.raddatz.familienarchiv.repository.AnnotationRepository;
|
||||||
import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository;
|
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;
|
||||||
@@ -26,7 +26,11 @@ import java.util.UUID;
|
|||||||
public class AnnotationService {
|
public class AnnotationService {
|
||||||
|
|
||||||
private final AnnotationRepository annotationRepository;
|
private final AnnotationRepository annotationRepository;
|
||||||
private final TranscriptionBlockRepository blockRepository;
|
// @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 AuditService auditService;
|
private final AuditService auditService;
|
||||||
|
|
||||||
public List<DocumentAnnotation> listAnnotations(UUID documentId) {
|
public List<DocumentAnnotation> listAnnotations(UUID documentId) {
|
||||||
@@ -37,6 +41,16 @@ public class AnnotationService {
|
|||||||
return annotationRepository.findById(id);
|
return annotationRepository.findById(id);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Transactional
|
||||||
|
public void deleteById(UUID annotationId) {
|
||||||
|
annotationRepository.deleteById(annotationId);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Transactional
|
||||||
|
public void deleteAllById(java.util.Collection<UUID> annotationIds) {
|
||||||
|
annotationRepository.deleteAllById(annotationIds);
|
||||||
|
}
|
||||||
|
|
||||||
@Transactional
|
@Transactional
|
||||||
public DocumentAnnotation createAnnotation(UUID documentId, CreateAnnotationDTO dto, UUID userId, String fileHash) {
|
public DocumentAnnotation createAnnotation(UUID documentId, CreateAnnotationDTO dto, UUID userId, String fileHash) {
|
||||||
DocumentAnnotation annotation = DocumentAnnotation.builder()
|
DocumentAnnotation annotation = DocumentAnnotation.builder()
|
||||||
@@ -108,7 +122,7 @@ public class AnnotationService {
|
|||||||
throw DomainException.forbidden("Only the annotation author can delete it");
|
throw DomainException.forbidden("Only the annotation author can delete it");
|
||||||
}
|
}
|
||||||
|
|
||||||
blockRepository.deleteByAnnotationId(annotationId);
|
transcriptionService.deleteByAnnotationId(annotationId);
|
||||||
annotationRepository.delete(annotation);
|
annotationRepository.delete(annotation);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -16,7 +16,6 @@ import org.raddatz.familienarchiv.model.DocumentAnnotation;
|
|||||||
import org.raddatz.familienarchiv.model.ScriptType;
|
import org.raddatz.familienarchiv.model.ScriptType;
|
||||||
import org.raddatz.familienarchiv.model.TranscriptionBlock;
|
import org.raddatz.familienarchiv.model.TranscriptionBlock;
|
||||||
import org.raddatz.familienarchiv.model.TranscriptionBlockVersion;
|
import org.raddatz.familienarchiv.model.TranscriptionBlockVersion;
|
||||||
import org.raddatz.familienarchiv.repository.AnnotationRepository;
|
|
||||||
import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository;
|
import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository;
|
||||||
import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository;
|
import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository;
|
||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
@@ -37,7 +36,6 @@ public class TranscriptionService {
|
|||||||
|
|
||||||
private final TranscriptionBlockRepository blockRepository;
|
private final TranscriptionBlockRepository blockRepository;
|
||||||
private final TranscriptionBlockVersionRepository versionRepository;
|
private final TranscriptionBlockVersionRepository versionRepository;
|
||||||
private final AnnotationRepository annotationRepository;
|
|
||||||
private final AnnotationService annotationService;
|
private final AnnotationService annotationService;
|
||||||
private final DocumentService documentService;
|
private final DocumentService documentService;
|
||||||
private final SenderModelService senderModelService;
|
private final SenderModelService senderModelService;
|
||||||
@@ -47,6 +45,11 @@ public class TranscriptionService {
|
|||||||
return blockRepository.findByDocumentIdOrderBySortOrderAsc(documentId);
|
return blockRepository.findByDocumentIdOrderBySortOrderAsc(documentId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Transactional
|
||||||
|
public void deleteByAnnotationId(UUID annotationId) {
|
||||||
|
blockRepository.deleteByAnnotationId(annotationId);
|
||||||
|
}
|
||||||
|
|
||||||
public TranscriptionBlock getBlock(UUID documentId, UUID blockId) {
|
public TranscriptionBlock getBlock(UUID documentId, UUID blockId) {
|
||||||
return blockRepository.findByIdAndDocumentId(blockId, documentId)
|
return blockRepository.findByIdAndDocumentId(blockId, documentId)
|
||||||
.orElseThrow(() -> DomainException.notFound(
|
.orElseThrow(() -> DomainException.notFound(
|
||||||
@@ -142,7 +145,7 @@ public class TranscriptionService {
|
|||||||
saveVersion(saved, userId);
|
saveVersion(saved, userId);
|
||||||
|
|
||||||
if (!text.equals(previousText)) {
|
if (!text.equals(previousText)) {
|
||||||
Optional<DocumentAnnotation> annotation = annotationRepository.findById(block.getAnnotationId());
|
Optional<DocumentAnnotation> annotation = annotationService.findById(block.getAnnotationId());
|
||||||
int pageNumber = annotation.map(DocumentAnnotation::getPageNumber).orElse(0);
|
int pageNumber = annotation.map(DocumentAnnotation::getPageNumber).orElse(0);
|
||||||
auditService.logAfterCommit(AuditKind.TEXT_SAVED, userId, documentId,
|
auditService.logAfterCommit(AuditKind.TEXT_SAVED, userId, documentId,
|
||||||
Map.of("pageNumber", pageNumber, "blockId", saved.getId().toString()));
|
Map.of("pageNumber", pageNumber, "blockId", saved.getId().toString()));
|
||||||
@@ -165,7 +168,7 @@ public class TranscriptionService {
|
|||||||
// then delete the dependent annotation directly (no ownership check needed)
|
// then delete the dependent annotation directly (no ownership check needed)
|
||||||
blockRepository.delete(block);
|
blockRepository.delete(block);
|
||||||
blockRepository.flush();
|
blockRepository.flush();
|
||||||
annotationRepository.deleteById(annotationId);
|
annotationService.deleteById(annotationId);
|
||||||
log.info("Deleted transcription block {} and annotation {} for document {}",
|
log.info("Deleted transcription block {} and annotation {} for document {}",
|
||||||
blockId, annotationId, documentId);
|
blockId, annotationId, documentId);
|
||||||
}
|
}
|
||||||
@@ -181,7 +184,7 @@ public class TranscriptionService {
|
|||||||
|
|
||||||
blockRepository.deleteAll(blocks);
|
blockRepository.deleteAll(blocks);
|
||||||
blockRepository.flush();
|
blockRepository.flush();
|
||||||
annotationRepository.deleteAllById(annotationIds);
|
annotationService.deleteAllById(annotationIds);
|
||||||
log.info("Bulk-deleted {} transcription blocks for document {}", blocks.size(), documentId);
|
log.info("Bulk-deleted {} transcription blocks for document {}", blocks.size(), documentId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -13,7 +13,6 @@ import org.raddatz.familienarchiv.dto.UpdateAnnotationDTO;
|
|||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
import org.raddatz.familienarchiv.model.DocumentAnnotation;
|
import org.raddatz.familienarchiv.model.DocumentAnnotation;
|
||||||
import org.raddatz.familienarchiv.repository.AnnotationRepository;
|
import org.raddatz.familienarchiv.repository.AnnotationRepository;
|
||||||
import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository;
|
|
||||||
import org.springframework.dao.DataIntegrityViolationException;
|
import org.springframework.dao.DataIntegrityViolationException;
|
||||||
|
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
@@ -36,7 +35,7 @@ import static org.springframework.http.HttpStatus.NOT_FOUND;
|
|||||||
class AnnotationServiceTest {
|
class AnnotationServiceTest {
|
||||||
|
|
||||||
@Mock AnnotationRepository annotationRepository;
|
@Mock AnnotationRepository annotationRepository;
|
||||||
@Mock TranscriptionBlockRepository blockRepository;
|
@Mock TranscriptionService transcriptionService;
|
||||||
@Mock AuditService auditService;
|
@Mock AuditService auditService;
|
||||||
@InjectMocks AnnotationService annotationService;
|
@InjectMocks AnnotationService annotationService;
|
||||||
|
|
||||||
@@ -208,7 +207,7 @@ class AnnotationServiceTest {
|
|||||||
|
|
||||||
annotationService.deleteAnnotation(docId, annotId, ownerId);
|
annotationService.deleteAnnotation(docId, annotId, ownerId);
|
||||||
|
|
||||||
verify(blockRepository).deleteByAnnotationId(annotId);
|
verify(transcriptionService).deleteByAnnotationId(annotId);
|
||||||
verify(annotationRepository).delete(annotation);
|
verify(annotationRepository).delete(annotation);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -225,8 +224,8 @@ class AnnotationServiceTest {
|
|||||||
|
|
||||||
annotationService.deleteAnnotation(docId, annotId, ownerId);
|
annotationService.deleteAnnotation(docId, annotId, ownerId);
|
||||||
|
|
||||||
var inOrder = org.mockito.Mockito.inOrder(blockRepository, annotationRepository);
|
var inOrder = org.mockito.Mockito.inOrder(transcriptionService, annotationRepository);
|
||||||
inOrder.verify(blockRepository).deleteByAnnotationId(annotId);
|
inOrder.verify(transcriptionService).deleteByAnnotationId(annotId);
|
||||||
inOrder.verify(annotationRepository).delete(annotation);
|
inOrder.verify(annotationRepository).delete(annotation);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -5,7 +5,6 @@ import org.junit.jupiter.api.Test;
|
|||||||
import org.raddatz.familienarchiv.audit.AuditService;
|
import org.raddatz.familienarchiv.audit.AuditService;
|
||||||
import org.raddatz.familienarchiv.model.BlockSource;
|
import org.raddatz.familienarchiv.model.BlockSource;
|
||||||
import org.raddatz.familienarchiv.model.TranscriptionBlock;
|
import org.raddatz.familienarchiv.model.TranscriptionBlock;
|
||||||
import org.raddatz.familienarchiv.repository.AnnotationRepository;
|
|
||||||
import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository;
|
import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository;
|
||||||
import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository;
|
import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository;
|
||||||
|
|
||||||
@@ -20,7 +19,6 @@ class TranscriptionServiceGuidedTest {
|
|||||||
|
|
||||||
TranscriptionBlockRepository blockRepository;
|
TranscriptionBlockRepository blockRepository;
|
||||||
TranscriptionBlockVersionRepository versionRepository;
|
TranscriptionBlockVersionRepository versionRepository;
|
||||||
AnnotationRepository annotationRepository;
|
|
||||||
AnnotationService annotationService;
|
AnnotationService annotationService;
|
||||||
DocumentService documentService;
|
DocumentService documentService;
|
||||||
SenderModelService senderModelService;
|
SenderModelService senderModelService;
|
||||||
@@ -35,14 +33,13 @@ class TranscriptionServiceGuidedTest {
|
|||||||
void setUp() {
|
void setUp() {
|
||||||
blockRepository = mock(TranscriptionBlockRepository.class);
|
blockRepository = mock(TranscriptionBlockRepository.class);
|
||||||
versionRepository = mock(TranscriptionBlockVersionRepository.class);
|
versionRepository = mock(TranscriptionBlockVersionRepository.class);
|
||||||
annotationRepository = mock(AnnotationRepository.class);
|
|
||||||
annotationService = mock(AnnotationService.class);
|
annotationService = mock(AnnotationService.class);
|
||||||
documentService = mock(DocumentService.class);
|
documentService = mock(DocumentService.class);
|
||||||
senderModelService = mock(SenderModelService.class);
|
senderModelService = mock(SenderModelService.class);
|
||||||
auditService = mock(AuditService.class);
|
auditService = mock(AuditService.class);
|
||||||
|
|
||||||
service = new TranscriptionService(blockRepository, versionRepository,
|
service = new TranscriptionService(blockRepository, versionRepository,
|
||||||
annotationRepository, annotationService, documentService, senderModelService, auditService);
|
annotationService, documentService, senderModelService, auditService);
|
||||||
|
|
||||||
when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
when(versionRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
when(versionRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
|
|||||||
@@ -21,7 +21,6 @@ import org.raddatz.familienarchiv.model.PersonMention;
|
|||||||
import org.raddatz.familienarchiv.model.ScriptType;
|
import org.raddatz.familienarchiv.model.ScriptType;
|
||||||
import org.raddatz.familienarchiv.model.TranscriptionBlock;
|
import org.raddatz.familienarchiv.model.TranscriptionBlock;
|
||||||
import org.raddatz.familienarchiv.model.TranscriptionBlockVersion;
|
import org.raddatz.familienarchiv.model.TranscriptionBlockVersion;
|
||||||
import org.raddatz.familienarchiv.repository.AnnotationRepository;
|
|
||||||
import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository;
|
import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository;
|
||||||
import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository;
|
import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository;
|
||||||
|
|
||||||
@@ -44,7 +43,6 @@ class TranscriptionServiceTest {
|
|||||||
|
|
||||||
@Mock TranscriptionBlockRepository blockRepository;
|
@Mock TranscriptionBlockRepository blockRepository;
|
||||||
@Mock TranscriptionBlockVersionRepository versionRepository;
|
@Mock TranscriptionBlockVersionRepository versionRepository;
|
||||||
@Mock AnnotationRepository annotationRepository;
|
|
||||||
@Mock AnnotationService annotationService;
|
@Mock AnnotationService annotationService;
|
||||||
@Mock DocumentService documentService;
|
@Mock DocumentService documentService;
|
||||||
@Mock SenderModelService senderModelService;
|
@Mock SenderModelService senderModelService;
|
||||||
@@ -320,7 +318,7 @@ class TranscriptionServiceTest {
|
|||||||
|
|
||||||
verify(blockRepository).delete(block);
|
verify(blockRepository).delete(block);
|
||||||
verify(blockRepository).flush();
|
verify(blockRepository).flush();
|
||||||
verify(annotationRepository).deleteById(annotId);
|
verify(annotationService).deleteById(annotId);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -354,7 +352,7 @@ class TranscriptionServiceTest {
|
|||||||
|
|
||||||
verify(blockRepository).deleteAll(List.of(block1, block2));
|
verify(blockRepository).deleteAll(List.of(block1, block2));
|
||||||
verify(blockRepository).flush();
|
verify(blockRepository).flush();
|
||||||
verify(annotationRepository).deleteAllById(List.of(annId1, annId2));
|
verify(annotationService).deleteAllById(List.of(annId1, annId2));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -532,7 +530,7 @@ class TranscriptionServiceTest {
|
|||||||
when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0));
|
||||||
when(documentService.getDocumentById(any())).thenReturn(
|
when(documentService.getDocumentById(any())).thenReturn(
|
||||||
Document.builder().scriptType(ScriptType.TYPEWRITER).build());
|
Document.builder().scriptType(ScriptType.TYPEWRITER).build());
|
||||||
when(annotationRepository.findById(annotId)).thenReturn(Optional.of(annotation));
|
when(annotationService.findById(annotId)).thenReturn(Optional.of(annotation));
|
||||||
|
|
||||||
transcriptionService.updateBlock(docId, blockId, UpdateTranscriptionBlockDTO.builder().text("new text").build(), userId);
|
transcriptionService.updateBlock(docId, blockId, UpdateTranscriptionBlockDTO.builder().text("new text").build(), userId);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user