refactor(ocr): move repository access from OcrController into OcrService
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) <noreply@anthropic.com>
This commit is contained in:
@@ -5,11 +5,8 @@ import lombok.extern.slf4j.Slf4j;
|
|||||||
import org.raddatz.familienarchiv.dto.BatchOcrDTO;
|
import org.raddatz.familienarchiv.dto.BatchOcrDTO;
|
||||||
import org.raddatz.familienarchiv.dto.OcrStatusDTO;
|
import org.raddatz.familienarchiv.dto.OcrStatusDTO;
|
||||||
import org.raddatz.familienarchiv.dto.TriggerOcrDTO;
|
import org.raddatz.familienarchiv.dto.TriggerOcrDTO;
|
||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.model.AppUser;
|
||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.model.OcrJob;
|
||||||
import org.raddatz.familienarchiv.model.*;
|
|
||||||
import org.raddatz.familienarchiv.repository.OcrJobDocumentRepository;
|
|
||||||
import org.raddatz.familienarchiv.repository.OcrJobRepository;
|
|
||||||
import org.raddatz.familienarchiv.security.Permission;
|
import org.raddatz.familienarchiv.security.Permission;
|
||||||
import org.raddatz.familienarchiv.security.RequirePermission;
|
import org.raddatz.familienarchiv.security.RequirePermission;
|
||||||
import org.raddatz.familienarchiv.service.OcrBatchService;
|
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 org.springframework.web.servlet.mvc.method.annotation.SseEmitter;
|
||||||
|
|
||||||
import jakarta.validation.Valid;
|
import jakarta.validation.Valid;
|
||||||
import java.util.List;
|
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
import java.util.Optional;
|
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
@RestController
|
@RestController
|
||||||
@@ -36,8 +31,6 @@ public class OcrController {
|
|||||||
private final OcrService ocrService;
|
private final OcrService ocrService;
|
||||||
private final OcrBatchService ocrBatchService;
|
private final OcrBatchService ocrBatchService;
|
||||||
private final OcrProgressService ocrProgressService;
|
private final OcrProgressService ocrProgressService;
|
||||||
private final OcrJobRepository ocrJobRepository;
|
|
||||||
private final OcrJobDocumentRepository ocrJobDocumentRepository;
|
|
||||||
private final UserService userService;
|
private final UserService userService;
|
||||||
|
|
||||||
@PostMapping("/api/documents/{documentId}/ocr")
|
@PostMapping("/api/documents/{documentId}/ocr")
|
||||||
@@ -66,40 +59,20 @@ public class OcrController {
|
|||||||
@GetMapping("/api/ocr/jobs/{jobId}")
|
@GetMapping("/api/ocr/jobs/{jobId}")
|
||||||
@RequirePermission(Permission.READ_ALL)
|
@RequirePermission(Permission.READ_ALL)
|
||||||
public OcrJob getJobStatus(@PathVariable UUID jobId) {
|
public OcrJob getJobStatus(@PathVariable UUID jobId) {
|
||||||
return ocrJobRepository.findById(jobId)
|
return ocrService.getJob(jobId);
|
||||||
.orElseThrow(() -> DomainException.notFound(
|
|
||||||
ErrorCode.OCR_JOB_NOT_FOUND, "OCR job not found: " + jobId));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping(value = "/api/ocr/jobs/{jobId}/progress", produces = MediaType.TEXT_EVENT_STREAM_VALUE)
|
@GetMapping(value = "/api/ocr/jobs/{jobId}/progress", produces = MediaType.TEXT_EVENT_STREAM_VALUE)
|
||||||
@RequirePermission(Permission.READ_ALL)
|
@RequirePermission(Permission.READ_ALL)
|
||||||
public SseEmitter streamProgress(@PathVariable UUID jobId) {
|
public SseEmitter streamProgress(@PathVariable UUID jobId) {
|
||||||
ocrJobRepository.findById(jobId)
|
ocrService.getJob(jobId);
|
||||||
.orElseThrow(() -> DomainException.notFound(
|
|
||||||
ErrorCode.OCR_JOB_NOT_FOUND, "OCR job not found: " + jobId));
|
|
||||||
return ocrProgressService.register(jobId);
|
return ocrProgressService.register(jobId);
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/api/documents/{documentId}/ocr-status")
|
@GetMapping("/api/documents/{documentId}/ocr-status")
|
||||||
@RequirePermission(Permission.READ_ALL)
|
@RequirePermission(Permission.READ_ALL)
|
||||||
public OcrStatusDTO getDocumentOcrStatus(@PathVariable UUID documentId) {
|
public OcrStatusDTO getDocumentOcrStatus(@PathVariable UUID documentId) {
|
||||||
List<OcrDocumentStatus> activeStatuses = List.of(
|
return ocrService.getDocumentOcrStatus(documentId);
|
||||||
OcrDocumentStatus.PENDING, OcrDocumentStatus.RUNNING);
|
|
||||||
|
|
||||||
Optional<OcrJobDocument> 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();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private UUID resolveUserId(Authentication authentication) {
|
private UUID resolveUserId(Authentication authentication) {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.service;
|
|||||||
|
|
||||||
import lombok.RequiredArgsConstructor;
|
import lombok.RequiredArgsConstructor;
|
||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
|
import org.raddatz.familienarchiv.dto.OcrStatusDTO;
|
||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
import org.raddatz.familienarchiv.model.*;
|
import org.raddatz.familienarchiv.model.*;
|
||||||
@@ -9,6 +10,8 @@ import org.raddatz.familienarchiv.repository.OcrJobDocumentRepository;
|
|||||||
import org.raddatz.familienarchiv.repository.OcrJobRepository;
|
import org.raddatz.familienarchiv.repository.OcrJobRepository;
|
||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
|
|
||||||
|
import java.util.List;
|
||||||
|
import java.util.Optional;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
@Service
|
@Service
|
||||||
@@ -22,6 +25,32 @@ public class OcrService {
|
|||||||
private final OcrJobDocumentRepository ocrJobDocumentRepository;
|
private final OcrJobDocumentRepository ocrJobDocumentRepository;
|
||||||
private final OcrAsyncRunner ocrAsyncRunner;
|
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<OcrDocumentStatus> activeStatuses = List.of(
|
||||||
|
OcrDocumentStatus.PENDING, OcrDocumentStatus.RUNNING);
|
||||||
|
|
||||||
|
Optional<OcrJobDocument> 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) {
|
public UUID startOcr(UUID documentId, ScriptType scriptTypeOverride, UUID userId) {
|
||||||
Document doc = documentService.getDocumentById(documentId);
|
Document doc = documentService.getDocumentById(documentId);
|
||||||
|
|
||||||
|
|||||||
@@ -4,12 +4,11 @@ import tools.jackson.databind.ObjectMapper;
|
|||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.raddatz.familienarchiv.config.SecurityConfig;
|
import org.raddatz.familienarchiv.config.SecurityConfig;
|
||||||
import org.raddatz.familienarchiv.dto.BatchOcrDTO;
|
import org.raddatz.familienarchiv.dto.BatchOcrDTO;
|
||||||
|
import org.raddatz.familienarchiv.dto.OcrStatusDTO;
|
||||||
import org.raddatz.familienarchiv.dto.TriggerOcrDTO;
|
import org.raddatz.familienarchiv.dto.TriggerOcrDTO;
|
||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
import org.raddatz.familienarchiv.model.*;
|
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.security.PermissionAspect;
|
||||||
import org.raddatz.familienarchiv.service.*;
|
import org.raddatz.familienarchiv.service.*;
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
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 org.springframework.test.web.servlet.MockMvc;
|
||||||
|
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Optional;
|
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
import static org.mockito.ArgumentMatchers.any;
|
import static org.mockito.ArgumentMatchers.any;
|
||||||
@@ -43,8 +41,6 @@ class OcrControllerTest {
|
|||||||
@MockitoBean OcrService ocrService;
|
@MockitoBean OcrService ocrService;
|
||||||
@MockitoBean OcrBatchService ocrBatchService;
|
@MockitoBean OcrBatchService ocrBatchService;
|
||||||
@MockitoBean OcrProgressService ocrProgressService;
|
@MockitoBean OcrProgressService ocrProgressService;
|
||||||
@MockitoBean OcrJobRepository ocrJobRepository;
|
|
||||||
@MockitoBean OcrJobDocumentRepository ocrJobDocumentRepository;
|
|
||||||
@MockitoBean UserService userService;
|
@MockitoBean UserService userService;
|
||||||
@MockitoBean CustomUserDetailsService customUserDetailsService;
|
@MockitoBean CustomUserDetailsService customUserDetailsService;
|
||||||
|
|
||||||
@@ -81,7 +77,8 @@ class OcrControllerTest {
|
|||||||
@WithMockUser(authorities = "READ_ALL")
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
void getJobStatus_returns404_whenJobNotFound() throws Exception {
|
void getJobStatus_returns404_whenJobNotFound() throws Exception {
|
||||||
UUID jobId = UUID.randomUUID();
|
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))
|
mockMvc.perform(get("/api/ocr/jobs/{jobId}", jobId))
|
||||||
.andExpect(status().isNotFound());
|
.andExpect(status().isNotFound());
|
||||||
@@ -99,7 +96,7 @@ class OcrControllerTest {
|
|||||||
.errorCount(1)
|
.errorCount(1)
|
||||||
.skippedCount(0)
|
.skippedCount(0)
|
||||||
.build();
|
.build();
|
||||||
when(ocrJobRepository.findById(jobId)).thenReturn(Optional.of(job));
|
when(ocrService.getJob(jobId)).thenReturn(job);
|
||||||
|
|
||||||
mockMvc.perform(get("/api/ocr/jobs/{jobId}", jobId))
|
mockMvc.perform(get("/api/ocr/jobs/{jobId}", jobId))
|
||||||
.andExpect(status().isOk())
|
.andExpect(status().isOk())
|
||||||
@@ -128,8 +125,8 @@ class OcrControllerTest {
|
|||||||
@WithMockUser(authorities = "READ_ALL")
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
void getDocumentOcrStatus_returnsNone_whenNoOcrJobExists() throws Exception {
|
void getDocumentOcrStatus_returnsNone_whenNoOcrJobExists() throws Exception {
|
||||||
UUID docId = UUID.randomUUID();
|
UUID docId = UUID.randomUUID();
|
||||||
when(ocrJobDocumentRepository.findFirstByDocumentIdAndStatusIn(eq(docId), any()))
|
when(ocrService.getDocumentOcrStatus(docId))
|
||||||
.thenReturn(Optional.empty());
|
.thenReturn(OcrStatusDTO.builder().status("NONE").build());
|
||||||
|
|
||||||
mockMvc.perform(get("/api/documents/{id}/ocr-status", docId))
|
mockMvc.perform(get("/api/documents/{id}/ocr-status", docId))
|
||||||
.andExpect(status().isOk())
|
.andExpect(status().isOk())
|
||||||
|
|||||||
@@ -5,12 +5,14 @@ import org.junit.jupiter.api.extension.ExtendWith;
|
|||||||
import org.mockito.InjectMocks;
|
import org.mockito.InjectMocks;
|
||||||
import org.mockito.Mock;
|
import org.mockito.Mock;
|
||||||
import org.mockito.junit.jupiter.MockitoExtension;
|
import org.mockito.junit.jupiter.MockitoExtension;
|
||||||
|
import org.raddatz.familienarchiv.dto.OcrStatusDTO;
|
||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
import org.raddatz.familienarchiv.model.*;
|
import org.raddatz.familienarchiv.model.*;
|
||||||
import org.raddatz.familienarchiv.repository.OcrJobDocumentRepository;
|
import org.raddatz.familienarchiv.repository.OcrJobDocumentRepository;
|
||||||
import org.raddatz.familienarchiv.repository.OcrJobRepository;
|
import org.raddatz.familienarchiv.repository.OcrJobRepository;
|
||||||
|
|
||||||
|
import java.util.Optional;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
@@ -30,6 +32,69 @@ class OcrServiceTest {
|
|||||||
|
|
||||||
@InjectMocks OcrService ocrService;
|
@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
|
@Test
|
||||||
void startOcr_throwsBadRequest_whenDocumentIsPlaceholder() {
|
void startOcr_throwsBadRequest_whenDocumentIsPlaceholder() {
|
||||||
UUID docId = UUID.randomUUID();
|
UUID docId = UUID.randomUUID();
|
||||||
|
|||||||
Reference in New Issue
Block a user