From 2d43f091726dcfd746bc268f0511620d9dbe387a Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 13 Apr 2026 12:26:14 +0200 Subject: [PATCH] refactor(ocr): move repository access from OcrController into OcrService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OcrController was injecting OcrJobRepository and OcrJobDocumentRepository directly, violating the Controller → Service → Repository layering rule. Moved getJob() and getDocumentOcrStatus() logic into OcrService. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../controller/OcrController.java | 37 ++--------- .../familienarchiv/service/OcrService.java | 29 +++++++++ .../controller/OcrControllerTest.java | 15 ++--- .../service/OcrServiceTest.java | 65 +++++++++++++++++++ 4 files changed, 105 insertions(+), 41 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/OcrController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/OcrController.java index bd1e41f9..ded4c760 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/OcrController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/OcrController.java @@ -5,11 +5,8 @@ import lombok.extern.slf4j.Slf4j; import org.raddatz.familienarchiv.dto.BatchOcrDTO; import org.raddatz.familienarchiv.dto.OcrStatusDTO; import org.raddatz.familienarchiv.dto.TriggerOcrDTO; -import org.raddatz.familienarchiv.exception.DomainException; -import org.raddatz.familienarchiv.exception.ErrorCode; -import org.raddatz.familienarchiv.model.*; -import org.raddatz.familienarchiv.repository.OcrJobDocumentRepository; -import org.raddatz.familienarchiv.repository.OcrJobRepository; +import org.raddatz.familienarchiv.model.AppUser; +import org.raddatz.familienarchiv.model.OcrJob; import org.raddatz.familienarchiv.security.Permission; import org.raddatz.familienarchiv.security.RequirePermission; import org.raddatz.familienarchiv.service.OcrBatchService; @@ -23,9 +20,7 @@ import org.springframework.web.bind.annotation.*; import org.springframework.web.servlet.mvc.method.annotation.SseEmitter; import jakarta.validation.Valid; -import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.UUID; @RestController @@ -36,8 +31,6 @@ public class OcrController { private final OcrService ocrService; private final OcrBatchService ocrBatchService; private final OcrProgressService ocrProgressService; - private final OcrJobRepository ocrJobRepository; - private final OcrJobDocumentRepository ocrJobDocumentRepository; private final UserService userService; @PostMapping("/api/documents/{documentId}/ocr") @@ -66,40 +59,20 @@ public class OcrController { @GetMapping("/api/ocr/jobs/{jobId}") @RequirePermission(Permission.READ_ALL) public OcrJob getJobStatus(@PathVariable UUID jobId) { - return ocrJobRepository.findById(jobId) - .orElseThrow(() -> DomainException.notFound( - ErrorCode.OCR_JOB_NOT_FOUND, "OCR job not found: " + jobId)); + return ocrService.getJob(jobId); } @GetMapping(value = "/api/ocr/jobs/{jobId}/progress", produces = MediaType.TEXT_EVENT_STREAM_VALUE) @RequirePermission(Permission.READ_ALL) public SseEmitter streamProgress(@PathVariable UUID jobId) { - ocrJobRepository.findById(jobId) - .orElseThrow(() -> DomainException.notFound( - ErrorCode.OCR_JOB_NOT_FOUND, "OCR job not found: " + jobId)); + ocrService.getJob(jobId); return ocrProgressService.register(jobId); } @GetMapping("/api/documents/{documentId}/ocr-status") @RequirePermission(Permission.READ_ALL) public OcrStatusDTO getDocumentOcrStatus(@PathVariable UUID documentId) { - List activeStatuses = List.of( - OcrDocumentStatus.PENDING, OcrDocumentStatus.RUNNING); - - Optional activeJobDoc = ocrJobDocumentRepository - .findFirstByDocumentIdAndStatusIn(documentId, activeStatuses); - - if (activeJobDoc.isEmpty()) { - return OcrStatusDTO.builder().status("NONE").build(); - } - - OcrJobDocument jobDoc = activeJobDoc.get(); - return OcrStatusDTO.builder() - .status(jobDoc.getStatus().name()) - .jobId(jobDoc.getJobId()) - .currentPage(jobDoc.getCurrentPage()) - .totalPages(jobDoc.getTotalPages()) - .build(); + return ocrService.getDocumentOcrStatus(documentId); } private UUID resolveUserId(Authentication authentication) { diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/OcrService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/OcrService.java index 75ade1c7..38b783da 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/OcrService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/OcrService.java @@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.service; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.raddatz.familienarchiv.dto.OcrStatusDTO; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.*; @@ -9,6 +10,8 @@ import org.raddatz.familienarchiv.repository.OcrJobDocumentRepository; import org.raddatz.familienarchiv.repository.OcrJobRepository; import org.springframework.stereotype.Service; +import java.util.List; +import java.util.Optional; import java.util.UUID; @Service @@ -22,6 +25,32 @@ public class OcrService { private final OcrJobDocumentRepository ocrJobDocumentRepository; private final OcrAsyncRunner ocrAsyncRunner; + public OcrJob getJob(UUID jobId) { + return ocrJobRepository.findById(jobId) + .orElseThrow(() -> DomainException.notFound( + ErrorCode.OCR_JOB_NOT_FOUND, "OCR job not found: " + jobId)); + } + + public OcrStatusDTO getDocumentOcrStatus(UUID documentId) { + List activeStatuses = List.of( + OcrDocumentStatus.PENDING, OcrDocumentStatus.RUNNING); + + Optional activeJobDoc = ocrJobDocumentRepository + .findFirstByDocumentIdAndStatusIn(documentId, activeStatuses); + + if (activeJobDoc.isEmpty()) { + return OcrStatusDTO.builder().status("NONE").build(); + } + + OcrJobDocument jobDoc = activeJobDoc.get(); + return OcrStatusDTO.builder() + .status(jobDoc.getStatus().name()) + .jobId(jobDoc.getJobId()) + .currentPage(jobDoc.getCurrentPage()) + .totalPages(jobDoc.getTotalPages()) + .build(); + } + public UUID startOcr(UUID documentId, ScriptType scriptTypeOverride, UUID userId) { Document doc = documentService.getDocumentById(documentId); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/OcrControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/OcrControllerTest.java index aef427ef..a7d6d5cf 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/OcrControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/OcrControllerTest.java @@ -4,12 +4,11 @@ import tools.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.config.SecurityConfig; import org.raddatz.familienarchiv.dto.BatchOcrDTO; +import org.raddatz.familienarchiv.dto.OcrStatusDTO; import org.raddatz.familienarchiv.dto.TriggerOcrDTO; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.*; -import org.raddatz.familienarchiv.repository.OcrJobDocumentRepository; -import org.raddatz.familienarchiv.repository.OcrJobRepository; import org.raddatz.familienarchiv.security.PermissionAspect; import org.raddatz.familienarchiv.service.*; import org.springframework.beans.factory.annotation.Autowired; @@ -22,7 +21,6 @@ import org.springframework.test.context.bean.override.mockito.MockitoBean; import org.springframework.test.web.servlet.MockMvc; import java.util.List; -import java.util.Optional; import java.util.UUID; import static org.mockito.ArgumentMatchers.any; @@ -43,8 +41,6 @@ class OcrControllerTest { @MockitoBean OcrService ocrService; @MockitoBean OcrBatchService ocrBatchService; @MockitoBean OcrProgressService ocrProgressService; - @MockitoBean OcrJobRepository ocrJobRepository; - @MockitoBean OcrJobDocumentRepository ocrJobDocumentRepository; @MockitoBean UserService userService; @MockitoBean CustomUserDetailsService customUserDetailsService; @@ -81,7 +77,8 @@ class OcrControllerTest { @WithMockUser(authorities = "READ_ALL") void getJobStatus_returns404_whenJobNotFound() throws Exception { UUID jobId = UUID.randomUUID(); - when(ocrJobRepository.findById(jobId)).thenReturn(Optional.empty()); + when(ocrService.getJob(jobId)) + .thenThrow(DomainException.notFound(ErrorCode.OCR_JOB_NOT_FOUND, "OCR job not found")); mockMvc.perform(get("/api/ocr/jobs/{jobId}", jobId)) .andExpect(status().isNotFound()); @@ -99,7 +96,7 @@ class OcrControllerTest { .errorCount(1) .skippedCount(0) .build(); - when(ocrJobRepository.findById(jobId)).thenReturn(Optional.of(job)); + when(ocrService.getJob(jobId)).thenReturn(job); mockMvc.perform(get("/api/ocr/jobs/{jobId}", jobId)) .andExpect(status().isOk()) @@ -128,8 +125,8 @@ class OcrControllerTest { @WithMockUser(authorities = "READ_ALL") void getDocumentOcrStatus_returnsNone_whenNoOcrJobExists() throws Exception { UUID docId = UUID.randomUUID(); - when(ocrJobDocumentRepository.findFirstByDocumentIdAndStatusIn(eq(docId), any())) - .thenReturn(Optional.empty()); + when(ocrService.getDocumentOcrStatus(docId)) + .thenReturn(OcrStatusDTO.builder().status("NONE").build()); mockMvc.perform(get("/api/documents/{id}/ocr-status", docId)) .andExpect(status().isOk()) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/OcrServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/OcrServiceTest.java index a94958a3..6827e03b 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/OcrServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/OcrServiceTest.java @@ -5,12 +5,14 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.raddatz.familienarchiv.dto.OcrStatusDTO; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.*; import org.raddatz.familienarchiv.repository.OcrJobDocumentRepository; import org.raddatz.familienarchiv.repository.OcrJobRepository; +import java.util.Optional; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -30,6 +32,69 @@ class OcrServiceTest { @InjectMocks OcrService ocrService; + // ─── getJob ────────────────────────────────────────────────────────────────── + + @Test + void getJob_returnsJob_whenFound() { + UUID jobId = UUID.randomUUID(); + OcrJob job = OcrJob.builder().id(jobId).status(OcrJobStatus.RUNNING).build(); + when(ocrJobRepository.findById(jobId)).thenReturn(Optional.of(job)); + + OcrJob result = ocrService.getJob(jobId); + + assertThat(result).isEqualTo(job); + } + + @Test + void getJob_throwsNotFound_whenJobDoesNotExist() { + UUID jobId = UUID.randomUUID(); + when(ocrJobRepository.findById(jobId)).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> ocrService.getJob(jobId)) + .isInstanceOf(DomainException.class) + .satisfies(e -> { + DomainException de = (DomainException) e; + assertThat(de.getStatus()).isEqualTo(NOT_FOUND); + assertThat(de.getCode()).isEqualTo(ErrorCode.OCR_JOB_NOT_FOUND); + }); + } + + // ─── getDocumentOcrStatus ─────────────────────────────────────────────────── + + @Test + void getDocumentOcrStatus_returnsNone_whenNoActiveJob() { + UUID docId = UUID.randomUUID(); + when(ocrJobDocumentRepository.findFirstByDocumentIdAndStatusIn(any(), any())) + .thenReturn(Optional.empty()); + + OcrStatusDTO result = ocrService.getDocumentOcrStatus(docId); + + assertThat(result.getStatus()).isEqualTo("NONE"); + assertThat(result.getJobId()).isNull(); + } + + @Test + void getDocumentOcrStatus_returnsActiveStatus_whenJobExists() { + UUID docId = UUID.randomUUID(); + UUID jobId = UUID.randomUUID(); + OcrJobDocument jobDoc = OcrJobDocument.builder() + .jobId(jobId).documentId(docId) + .status(OcrDocumentStatus.RUNNING) + .currentPage(2).totalPages(5) + .build(); + when(ocrJobDocumentRepository.findFirstByDocumentIdAndStatusIn(any(), any())) + .thenReturn(Optional.of(jobDoc)); + + OcrStatusDTO result = ocrService.getDocumentOcrStatus(docId); + + assertThat(result.getStatus()).isEqualTo("RUNNING"); + assertThat(result.getJobId()).isEqualTo(jobId); + assertThat(result.getCurrentPage()).isEqualTo(2); + assertThat(result.getTotalPages()).isEqualTo(5); + } + + // ─── startOcr ─────────────────────────────────────────────────────────────── + @Test void startOcr_throwsBadRequest_whenDocumentIsPlaceholder() { UUID docId = UUID.randomUUID();