From f5151f394970ef7d8ee2d66313ad56705e7d7b11 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 07:40:34 +0200 Subject: [PATCH] refactor(ocr-training): route SenderModelService and OcrTrainingService through TranscriptionBlockQueryService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both services injected TranscriptionBlockRepository directly to read block counts. They now go through TranscriptionBlockQueryService (count() and countManualKurrentBlocksByPerson() added as 1-line delegations) — chosen over TranscriptionService to avoid the existing SenderModelService → TrainingDataExportService → TranscriptionBlockQueryService chain reaching back into TranscriptionService and creating a cycle. Refs #417 (C6.2 violations #8 and #9). Co-Authored-By: Claude Opus 4.7 --- .../service/OcrTrainingService.java | 5 ++- .../service/SenderModelService.java | 15 ++++---- .../TranscriptionBlockQueryService.java | 8 +++++ .../service/OcrTrainingServiceTest.java | 9 +++-- .../service/SenderModelServiceTest.java | 35 +++++++++---------- 5 files changed, 38 insertions(+), 34 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/OcrTrainingService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/OcrTrainingService.java index 83e5086c..97625d29 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/OcrTrainingService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/OcrTrainingService.java @@ -10,7 +10,6 @@ import org.raddatz.familienarchiv.model.OcrTrainingRun; import org.raddatz.familienarchiv.model.SenderModel; import org.raddatz.familienarchiv.model.TrainingStatus; import org.raddatz.familienarchiv.repository.OcrTrainingRunRepository; -import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.slf4j.MDC; import org.springframework.boot.context.event.ApplicationReadyEvent; import org.springframework.context.event.EventListener; @@ -37,7 +36,7 @@ public class OcrTrainingService { private final SegmentationTrainingExportService segmentationTrainingExportService; private final OcrClient ocrClient; private final OcrHealthClient ocrHealthClient; - private final TranscriptionBlockRepository blockRepository; + private final TranscriptionBlockQueryService transcriptionBlockQueryService; private final TransactionTemplate txTemplate; private final PersonService personService; private final SenderModelService senderModelService; @@ -189,7 +188,7 @@ public class OcrTrainingService { .distinct() .count(); - int totalOcrBlocks = (int) blockRepository.count(); + int totalOcrBlocks = (int) transcriptionBlockQueryService.count(); int availableSegBlocks = segmentationTrainingExportService.querySegmentationBlocks().size(); List recentRuns = trainingRunRepository.findTop20ByOrderByCreatedAtDesc(); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/SenderModelService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/SenderModelService.java index e7881474..10be59c1 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/SenderModelService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/SenderModelService.java @@ -9,7 +9,6 @@ import org.raddatz.familienarchiv.model.SenderModel; import org.raddatz.familienarchiv.model.TrainingStatus; import org.raddatz.familienarchiv.repository.OcrTrainingRunRepository; import org.raddatz.familienarchiv.repository.SenderModelRepository; -import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.slf4j.MDC; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -32,7 +31,7 @@ import java.util.UUID; public class SenderModelService { private final SenderModelRepository senderModelRepository; - private final TranscriptionBlockRepository blockRepository; + private final TranscriptionBlockQueryService transcriptionBlockQueryService; private final OcrTrainingRunRepository trainingRunRepository; private final OcrClient ocrClient; private final TransactionTemplate txTemplate; @@ -62,7 +61,7 @@ public class SenderModelService { public OcrTrainingRun triggerManualSenderTraining(UUID personId) { personService.getById(personId); - long correctedLines = blockRepository.countManualKurrentBlocksByPerson(personId); + long correctedLines = transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId); boolean runNow = runOrQueueSenderTraining(personId, (int) correctedLines); TrainingStatus targetStatus = runNow ? TrainingStatus.RUNNING : TrainingStatus.QUEUED; OcrTrainingRun run = trainingRunRepository.findFirstByPersonIdAndStatus(personId, targetStatus) @@ -77,7 +76,7 @@ public class SenderModelService { @Async public void runSenderTraining(UUID personId) { - long correctedLines = blockRepository.countManualKurrentBlocksByPerson(personId); + long correctedLines = transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId); triggerSenderTraining(personId, (int) correctedLines); } @@ -87,7 +86,7 @@ public class SenderModelService { */ @Async public void checkAndTriggerTraining(UUID personId) { - long correctedLines = blockRepository.countManualKurrentBlocksByPerson(personId); + long correctedLines = transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId); Optional existing = senderModelRepository.findByPersonId(personId); boolean shouldActivate = existing.isEmpty() && correctedLines >= activationThreshold; @@ -121,7 +120,7 @@ public class SenderModelService { } if (trainingRunRepository.findFirstByStatus(TrainingStatus.RUNNING).isPresent()) { - int blockCount = (int) blockRepository.countManualKurrentBlocksByPerson(personId); + int blockCount = (int) transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId); trainingRunRepository.save(OcrTrainingRun.builder() .status(TrainingStatus.QUEUED) .personId(personId) @@ -133,7 +132,7 @@ public class SenderModelService { return false; } - long blockCount = blockRepository.countManualKurrentBlocksByPerson(personId); + long blockCount = transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId); trainingRunRepository.save(OcrTrainingRun.builder() .status(TrainingStatus.RUNNING) .personId(personId) @@ -227,7 +226,7 @@ public class SenderModelService { if (queuedOpt != null && queuedOpt.isPresent()) { OcrTrainingRun promoted = queuedOpt.get(); log.info("Promoting queued sender training run {} for person {}", promoted.getId(), promoted.getPersonId()); - long freshCount = blockRepository.countManualKurrentBlocksByPerson(promoted.getPersonId()); + long freshCount = transcriptionBlockQueryService.countManualKurrentBlocksByPerson(promoted.getPersonId()); triggerSenderTraining(promoted.getPersonId(), (int) freshCount); } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionBlockQueryService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionBlockQueryService.java index 591a8f83..bc21c7ad 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionBlockQueryService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionBlockQueryService.java @@ -37,4 +37,12 @@ public class TranscriptionBlockQueryService { public List findManualKurrentBlocksByPerson(UUID personId) { return blockRepository.findManualKurrentBlocksByPerson(personId); } + + public long countManualKurrentBlocksByPerson(UUID personId) { + return blockRepository.countManualKurrentBlocksByPerson(personId); + } + + public long count() { + return blockRepository.count(); + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/OcrTrainingServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/OcrTrainingServiceTest.java index 46142713..3fd42d63 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/OcrTrainingServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/OcrTrainingServiceTest.java @@ -11,7 +11,6 @@ import org.raddatz.familienarchiv.model.SenderModel; import org.raddatz.familienarchiv.model.TrainingStatus; import org.raddatz.familienarchiv.model.TranscriptionBlock; import org.raddatz.familienarchiv.repository.OcrTrainingRunRepository; -import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.raddatz.familienarchiv.service.PersonService; import org.springframework.transaction.support.TransactionCallback; import org.springframework.transaction.support.TransactionTemplate; @@ -34,7 +33,7 @@ class OcrTrainingServiceTest { SegmentationTrainingExportService segExportService; OcrClient ocrClient; OcrHealthClient healthClient; - TranscriptionBlockRepository blockRepository; + TranscriptionBlockQueryService transcriptionBlockQueryService; TransactionTemplate txTemplate; PersonService personService; SenderModelService senderModelService; @@ -47,7 +46,7 @@ class OcrTrainingServiceTest { segExportService = mock(SegmentationTrainingExportService.class); ocrClient = mock(OcrClient.class); healthClient = mock(OcrHealthClient.class); - blockRepository = mock(TranscriptionBlockRepository.class); + transcriptionBlockQueryService = mock(TranscriptionBlockQueryService.class); txTemplate = mock(TransactionTemplate.class); personService = mock(PersonService.class); senderModelService = mock(SenderModelService.class); @@ -58,9 +57,9 @@ class OcrTrainingServiceTest { return callback.doInTransaction(null); }); - service = new OcrTrainingService(runRepository, exportService, segExportService, ocrClient, healthClient, blockRepository, txTemplate, personService, senderModelService); + service = new OcrTrainingService(runRepository, exportService, segExportService, ocrClient, healthClient, transcriptionBlockQueryService, txTemplate, personService, senderModelService); - when(blockRepository.count()).thenReturn(0L); + when(transcriptionBlockQueryService.count()).thenReturn(0L); when(runRepository.findTop20ByOrderByCreatedAtDesc()).thenReturn(List.of()); when(segExportService.querySegmentationBlocks()).thenReturn(List.of()); when(senderModelService.getAllSenderModels()).thenReturn(List.of()); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/SenderModelServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/SenderModelServiceTest.java index 4f18701a..675236f3 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/SenderModelServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/SenderModelServiceTest.java @@ -12,7 +12,6 @@ import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.Person; import org.raddatz.familienarchiv.repository.OcrTrainingRunRepository; import org.raddatz.familienarchiv.repository.SenderModelRepository; -import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.transaction.support.TransactionCallback; import org.springframework.transaction.support.TransactionTemplate; @@ -28,7 +27,7 @@ import static org.mockito.Mockito.*; class SenderModelServiceTest { SenderModelRepository senderModelRepository; - TranscriptionBlockRepository blockRepository; + TranscriptionBlockQueryService transcriptionBlockQueryService; OcrTrainingRunRepository trainingRunRepository; OcrClient ocrClient; TransactionTemplate txTemplate; @@ -42,7 +41,7 @@ class SenderModelServiceTest { @BeforeEach void setUp() { senderModelRepository = mock(SenderModelRepository.class); - blockRepository = mock(TranscriptionBlockRepository.class); + transcriptionBlockQueryService = mock(TranscriptionBlockQueryService.class); trainingRunRepository = mock(OcrTrainingRunRepository.class); ocrClient = mock(OcrClient.class); txTemplate = mock(TransactionTemplate.class); @@ -57,7 +56,7 @@ class SenderModelServiceTest { return callback.doInTransaction(null); }); - service = new SenderModelService(senderModelRepository, blockRepository, + service = new SenderModelService(senderModelRepository, transcriptionBlockQueryService, trainingRunRepository, ocrClient, txTemplate, trainingDataExportService, personService); ReflectionTestUtils.setField(service, "self", selfProxy); ReflectionTestUtils.setField(service, "activationThreshold", 100); @@ -82,7 +81,7 @@ class SenderModelServiceTest { @Test void runSenderTraining_queriesBlockCountForPerson() { - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(42L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(42L); // triggerSenderTraining needs a RUNNING row — return empty to abort early when(trainingRunRepository.findFirstByPersonIdAndStatus(personId, TrainingStatus.RUNNING)) .thenReturn(Optional.empty()); @@ -93,14 +92,14 @@ class SenderModelServiceTest { // triggerSenderTraining will throw when no RUNNING row found } - verify(blockRepository).countManualKurrentBlocksByPerson(personId); + verify(transcriptionBlockQueryService).countManualKurrentBlocksByPerson(personId); } // ─── Activation threshold ───────────────────────────────────────────────── @Test void checkAndTriggerTraining_doesNothing_belowActivationThreshold() { - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(99L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(99L); when(senderModelRepository.findByPersonId(personId)).thenReturn(Optional.empty()); SenderModelService spy = spy(service); @@ -111,7 +110,7 @@ class SenderModelServiceTest { @Test void checkAndTriggerTraining_triggersTraining_atActivationThreshold() { - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(100L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(100L); when(senderModelRepository.findByPersonId(personId)).thenReturn(Optional.empty()); SenderModelService spy = spy(service); @@ -129,7 +128,7 @@ class SenderModelServiceTest { SenderModel existing = SenderModel.builder().personId(personId) .correctedLinesAtTraining(100).build(); when(senderModelRepository.findByPersonId(personId)).thenReturn(Optional.of(existing)); - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(149L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(149L); SenderModelService spy = spy(service); spy.checkAndTriggerTraining(personId); @@ -142,7 +141,7 @@ class SenderModelServiceTest { SenderModel existing = SenderModel.builder().personId(personId) .correctedLinesAtTraining(100).build(); when(senderModelRepository.findByPersonId(personId)).thenReturn(Optional.of(existing)); - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(150L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(150L); SenderModelService spy = spy(service); doReturn(false).when(spy).runOrQueueSenderTraining(personId, 150); @@ -156,7 +155,7 @@ class SenderModelServiceTest { @Test void checkAndTriggerTraining_callsTrigger_whenRunNow() { - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(100L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(100L); when(senderModelRepository.findByPersonId(personId)).thenReturn(Optional.empty()); SenderModelService spy = spy(service); @@ -170,7 +169,7 @@ class SenderModelServiceTest { @Test void checkAndTriggerTraining_doesNotCallTrigger_whenQueued() { - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(100L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(100L); when(senderModelRepository.findByPersonId(personId)).thenReturn(Optional.empty()); SenderModelService spy = spy(service); @@ -200,7 +199,7 @@ class SenderModelServiceTest { when(trainingRunRepository.findFirstByStatus(TrainingStatus.RUNNING)).thenReturn( Optional.of(OcrTrainingRun.builder().id(UUID.randomUUID()).status(TrainingStatus.RUNNING) .blockCount(5).documentCount(1).modelName("german_kurrent").build())); - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(120L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(120L); when(trainingRunRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); boolean result = service.runOrQueueSenderTraining(personId, 120); @@ -226,7 +225,7 @@ class SenderModelServiceTest { // eliminating the race window between the check and a separate triggerSenderTraining call. when(trainingRunRepository.existsByPersonIdAndStatus(personId, TrainingStatus.QUEUED)).thenReturn(false); when(trainingRunRepository.findFirstByStatus(TrainingStatus.RUNNING)).thenReturn(Optional.empty()); - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(120L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(120L); when(trainingRunRepository.save(any())).thenAnswer(inv -> { OcrTrainingRun r = inv.getArgument(0); if (r.getId() == null) r.setId(UUID.randomUUID()); @@ -314,7 +313,7 @@ class SenderModelServiceTest { @Test void triggerManualSenderTraining_returnsRunningRun_whenIdle() { when(personService.getById(personId)).thenReturn(Person.builder().id(personId).build()); - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(0L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(0L); when(trainingRunRepository.existsByPersonIdAndStatus(personId, TrainingStatus.QUEUED)).thenReturn(false); when(trainingRunRepository.findFirstByStatus(TrainingStatus.RUNNING)).thenReturn(Optional.empty()); OcrTrainingRun runningRun = OcrTrainingRun.builder() @@ -333,7 +332,7 @@ class SenderModelServiceTest { @Test void triggerManualSenderTraining_returnsQueuedRun_whenAnotherRunning() { when(personService.getById(personId)).thenReturn(Person.builder().id(personId).build()); - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(0L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(0L); when(trainingRunRepository.existsByPersonIdAndStatus(personId, TrainingStatus.QUEUED)).thenReturn(false); when(trainingRunRepository.findFirstByStatus(TrainingStatus.RUNNING)).thenReturn( Optional.of(OcrTrainingRun.builder().id(UUID.randomUUID()).status(TrainingStatus.RUNNING) @@ -363,7 +362,7 @@ class SenderModelServiceTest { @Test void triggerManualSenderTraining_throwsDomainException_whenRunRowMissingAfterCreate() { when(personService.getById(personId)).thenReturn(Person.builder().id(personId).build()); - when(blockRepository.countManualKurrentBlocksByPerson(personId)).thenReturn(0L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(personId)).thenReturn(0L); when(trainingRunRepository.existsByPersonIdAndStatus(personId, TrainingStatus.QUEUED)).thenReturn(false); when(trainingRunRepository.findFirstByStatus(TrainingStatus.RUNNING)).thenReturn(Optional.empty()); OcrTrainingRun runningRun = OcrTrainingRun.builder() @@ -405,7 +404,7 @@ class SenderModelServiceTest { .modelName("sender_" + nextPersonId).build(); when(trainingRunRepository.findFirstByStatusOrderByCreatedAtAsc(TrainingStatus.QUEUED)) .thenReturn(Optional.of(queued)); - when(blockRepository.countManualKurrentBlocksByPerson(nextPersonId)).thenReturn(5L); + when(transcriptionBlockQueryService.countManualKurrentBlocksByPerson(nextPersonId)).thenReturn(5L); SenderModelService spy = spy(service); // Stub the recursive call to stop the chain after one promotion