From d5e0e969ef0a200c86fca461be5c9e0512732a4e Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 4 May 2026 22:20:14 +0200 Subject: [PATCH 1/9] refactor(stats): introduce StatsService and require READ_ALL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StatsController previously injected PersonRepository and DocumentRepository directly, violating the controller→service→repository layering rule. Move the two count() calls into a thin StatsService that delegates to PersonService.count and DocumentService.count. While here, add the missing @RequirePermission(READ_ALL) flagged by AUDIT-2 §7 — anonymous callers were able to read aggregate document/ person counts. Refs #417 (C6.1 violation #1). Co-Authored-By: Claude Opus 4.7 --- .../controller/StatsController.java | 14 +++---- .../service/DocumentService.java | 4 ++ .../familienarchiv/service/PersonService.java | 4 ++ .../familienarchiv/service/StatsService.java | 17 ++++++++ .../controller/StatsControllerTest.java | 22 ++++++---- .../service/StatsServiceTest.java | 41 +++++++++++++++++++ 6 files changed, 86 insertions(+), 16 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/service/StatsService.java create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/service/StatsServiceTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/StatsController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/StatsController.java index c735d74a..8ed1759a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/StatsController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/StatsController.java @@ -1,25 +1,25 @@ package org.raddatz.familienarchiv.controller; +import lombok.RequiredArgsConstructor; import org.raddatz.familienarchiv.dto.StatsDTO; -import org.raddatz.familienarchiv.repository.DocumentRepository; -import org.raddatz.familienarchiv.repository.PersonRepository; +import org.raddatz.familienarchiv.security.Permission; +import org.raddatz.familienarchiv.security.RequirePermission; +import org.raddatz.familienarchiv.service.StatsService; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; -import lombok.RequiredArgsConstructor; - @RestController @RequestMapping("/api/stats") @RequiredArgsConstructor public class StatsController { - private final PersonRepository personRepository; - private final DocumentRepository documentRepository; + private final StatsService statsService; + @RequirePermission(Permission.READ_ALL) @GetMapping public ResponseEntity getStats() { - return ResponseEntity.ok(new StatsDTO(personRepository.count(), documentRepository.count())); + return ResponseEntity.ok(statsService.getStats()); } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index 7bc37953..6cf4cb61 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -73,6 +73,10 @@ public class DocumentService { public record StoreResult(Document document, boolean isNew) {} + public long count() { + return documentRepository.count(); + } + public Map findTitlesByIds(Collection ids) { if (ids.isEmpty()) return Map.of(); Map titles = new HashMap<>(); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java index e1adbdba..ff16f63c 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PersonService.java @@ -46,6 +46,10 @@ public class PersonService { .orElseThrow(() -> DomainException.notFound(ErrorCode.PERSON_NOT_FOUND, "Person not found: " + id)); } + public long count() { + return personRepository.count(); + } + public List findCorrespondents(UUID personId, String q) { if (q != null && !q.isBlank()) { return personRepository.findCorrespondentsWithFilter(personId, q); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/StatsService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/StatsService.java new file mode 100644 index 00000000..64979473 --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/StatsService.java @@ -0,0 +1,17 @@ +package org.raddatz.familienarchiv.service; + +import lombok.RequiredArgsConstructor; +import org.raddatz.familienarchiv.dto.StatsDTO; +import org.springframework.stereotype.Service; + +@Service +@RequiredArgsConstructor +public class StatsService { + + private final PersonService personService; + private final DocumentService documentService; + + public StatsDTO getStats() { + return new StatsDTO(personService.count(), documentService.count()); + } +} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/StatsControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/StatsControllerTest.java index 44228a7b..85e8bff2 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/StatsControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/StatsControllerTest.java @@ -2,10 +2,10 @@ package org.raddatz.familienarchiv.controller; import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.config.SecurityConfig; -import org.raddatz.familienarchiv.repository.DocumentRepository; -import org.raddatz.familienarchiv.repository.PersonRepository; +import org.raddatz.familienarchiv.dto.StatsDTO; import org.raddatz.familienarchiv.security.PermissionAspect; import org.raddatz.familienarchiv.service.CustomUserDetailsService; +import org.raddatz.familienarchiv.service.StatsService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.aop.AopAutoConfiguration; import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; @@ -25,8 +25,7 @@ class StatsControllerTest { @Autowired MockMvc mockMvc; - @MockitoBean PersonRepository personRepository; - @MockitoBean DocumentRepository documentRepository; + @MockitoBean StatsService statsService; @MockitoBean CustomUserDetailsService customUserDetailsService; @Test @@ -37,9 +36,15 @@ class StatsControllerTest { @Test @WithMockUser + void getStats_returns403_whenUserLacksReadAll() throws Exception { + mockMvc.perform(get("/api/stats")) + .andExpect(status().isForbidden()); + } + + @Test + @WithMockUser(authorities = "READ_ALL") void getStats_returns200_withCorrectCounts() throws Exception { - when(personRepository.count()).thenReturn(4L); - when(documentRepository.count()).thenReturn(12L); + when(statsService.getStats()).thenReturn(new StatsDTO(4L, 12L)); mockMvc.perform(get("/api/stats")) .andExpect(status().isOk()) @@ -48,10 +53,9 @@ class StatsControllerTest { } @Test - @WithMockUser + @WithMockUser(authorities = "READ_ALL") void getStats_returns200_withZeroCounts() throws Exception { - when(personRepository.count()).thenReturn(0L); - when(documentRepository.count()).thenReturn(0L); + when(statsService.getStats()).thenReturn(new StatsDTO(0L, 0L)); mockMvc.perform(get("/api/stats")) .andExpect(status().isOk()) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/StatsServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/StatsServiceTest.java new file mode 100644 index 00000000..4a001a78 --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/StatsServiceTest.java @@ -0,0 +1,41 @@ +package org.raddatz.familienarchiv.service; + +import org.junit.jupiter.api.Test; +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.StatsDTO; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class StatsServiceTest { + + @Mock PersonService personService; + @Mock DocumentService documentService; + @InjectMocks StatsService statsService; + + @Test + void getStats_returnsCountsFromServices() { + when(personService.count()).thenReturn(4L); + when(documentService.count()).thenReturn(12L); + + StatsDTO stats = statsService.getStats(); + + assertThat(stats.totalPersons()).isEqualTo(4L); + assertThat(stats.totalDocuments()).isEqualTo(12L); + } + + @Test + void getStats_returnsZero_whenNoEntities() { + when(personService.count()).thenReturn(0L); + when(documentService.count()).thenReturn(0L); + + StatsDTO stats = statsService.getStats(); + + assertThat(stats.totalPersons()).isZero(); + assertThat(stats.totalDocuments()).isZero(); + } +} -- 2.49.1 From 5c1332cb0e65cf0290864b29473ce92433af5643 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 4 May 2026 22:26:11 +0200 Subject: [PATCH 2/9] refactor(auth): route password reset through service layer + e2e helper - PasswordResetService injects UserService instead of AppUserRepository. - New UserService.findByEmailOptional preserves the silent-fail behaviour of the old findByEmail-returning-Optional path; the existing throwing findByEmail is unchanged. - New PasswordResetService.findLatestActiveTokenForEmail exposes the latest active reset token without leaking the repository upward. - New @Profile("e2e") PasswordResetTestHelper wraps that read so the AuthE2EController no longer touches PasswordResetTokenRepository directly. Profile guard moves from the controller-only annotation to also cover the helper bean, so the production graph never instantiates either. Refs #417 (C6.1 violation #2 + C6.2 violation #12). Co-Authored-By: Claude Opus 4.7 --- .../controller/AuthE2EController.java | 14 +++----- .../service/PasswordResetService.java | 16 ++++++--- .../service/PasswordResetTestHelper.java | 24 +++++++++++++ .../familienarchiv/service/UserService.java | 8 +++++ .../service/PasswordResetServiceTest.java | 13 ++++--- .../service/PasswordResetTestHelperTest.java | 35 +++++++++++++++++++ .../service/UserServiceTest.java | 28 +++++++++++++++ 7 files changed, 118 insertions(+), 20 deletions(-) create mode 100644 backend/src/main/java/org/raddatz/familienarchiv/service/PasswordResetTestHelper.java create mode 100644 backend/src/test/java/org/raddatz/familienarchiv/service/PasswordResetTestHelperTest.java diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/AuthE2EController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/AuthE2EController.java index 7d9be97b..7d37e441 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/AuthE2EController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/AuthE2EController.java @@ -1,8 +1,8 @@ package org.raddatz.familienarchiv.controller; -import java.time.LocalDateTime; - -import org.raddatz.familienarchiv.repository.PasswordResetTokenRepository; +import io.swagger.v3.oas.annotations.Operation; +import lombok.RequiredArgsConstructor; +import org.raddatz.familienarchiv.service.PasswordResetTestHelper; import org.springframework.context.annotation.Profile; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; @@ -10,10 +10,6 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; -import io.swagger.v3.oas.annotations.Operation; - -import lombok.RequiredArgsConstructor; - /** * Test-only endpoint to retrieve a password reset token by email. * Only active under the "e2e" Spring profile. @@ -24,14 +20,14 @@ import lombok.RequiredArgsConstructor; @RequiredArgsConstructor public class AuthE2EController { - private final PasswordResetTokenRepository tokenRepository; + private final PasswordResetTestHelper passwordResetTestHelper; // Hidden from the OpenAPI spec — this endpoint must never appear in the generated api.ts // even when the e2e profile is active alongside the dev profile during spec generation. @Operation(hidden = true) @GetMapping("/reset-token-for-test") public ResponseEntity getResetTokenForTest(@RequestParam String email) { - return tokenRepository.findLatestActiveTokenByEmail(email, LocalDateTime.now()) + return passwordResetTestHelper.getResetTokenForTest(email) .map(ResponseEntity::ok) .orElse(ResponseEntity.notFound().build()); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PasswordResetService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PasswordResetService.java index 6f15ddb8..ec6ab08b 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/PasswordResetService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PasswordResetService.java @@ -10,7 +10,6 @@ import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.PasswordResetToken; -import org.raddatz.familienarchiv.repository.AppUserRepository; import org.raddatz.familienarchiv.repository.PasswordResetTokenRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -30,7 +29,7 @@ import lombok.extern.slf4j.Slf4j; @Slf4j public class PasswordResetService { - private final AppUserRepository userRepository; + private final UserService userService; private final PasswordResetTokenRepository tokenRepository; private final PasswordEncoder passwordEncoder; @@ -49,7 +48,7 @@ public class PasswordResetService { * If no mail sender is configured, logs a warning. */ public void requestReset(String email, String appBaseUrl) { - Optional userOpt = userRepository.findByEmail(email); + Optional userOpt = userService.findByEmailOptional(email); if (userOpt.isEmpty()) { log.debug("Password reset requested for unknown email: {}", email); return; @@ -82,12 +81,21 @@ public class PasswordResetService { AppUser user = resetToken.getUser(); user.setPassword(passwordEncoder.encode(request.getNewPassword())); - userRepository.save(user); + userService.save(user); resetToken.setUsed(true); tokenRepository.save(resetToken); } + /** + * Returns the raw token string of the most recent active (unused, unexpired) + * reset token for the given email, if any. Used by the e2e helper to drive + * automated password-reset flows; production code paths never call this. + */ + public Optional findLatestActiveTokenForEmail(String email) { + return tokenRepository.findLatestActiveTokenByEmail(email, LocalDateTime.now()); + } + /** Nightly cleanup of expired and used tokens. */ @Scheduled(cron = "0 0 3 * * *") @Transactional diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/PasswordResetTestHelper.java b/backend/src/main/java/org/raddatz/familienarchiv/service/PasswordResetTestHelper.java new file mode 100644 index 00000000..0ddbacdb --- /dev/null +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/PasswordResetTestHelper.java @@ -0,0 +1,24 @@ +package org.raddatz.familienarchiv.service; + +import lombok.RequiredArgsConstructor; +import org.springframework.context.annotation.Profile; +import org.springframework.stereotype.Service; + +import java.util.Optional; + +/** + * E2E-only thin wrapper around {@link PasswordResetService} that exposes + * the latest active reset token for a given email. Loaded only when the + * {@code e2e} Spring profile is active so production code paths never see it. + */ +@Service +@Profile("e2e") +@RequiredArgsConstructor +public class PasswordResetTestHelper { + + private final PasswordResetService passwordResetService; + + public Optional getResetTokenForTest(String email) { + return passwordResetService.findLatestActiveTokenForEmail(email); + } +} diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java index 971be5b3..8ab3f843 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/UserService.java @@ -248,6 +248,14 @@ public class UserService { .orElseThrow(() -> DomainException.notFound(ErrorCode.USER_NOT_FOUND, "No user found for email: " + email)); } + public Optional findByEmailOptional(String email) { + return userRepository.findByEmail(email); + } + + public AppUser save(AppUser user) { + return userRepository.save(user); + } + public List getAllUsers() { return userRepository.findAll(); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/PasswordResetServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/PasswordResetServiceTest.java index 336ff0ff..ccf4f487 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/PasswordResetServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/PasswordResetServiceTest.java @@ -22,7 +22,6 @@ import org.raddatz.familienarchiv.dto.ResetPasswordRequest; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.model.AppUser; import org.raddatz.familienarchiv.model.PasswordResetToken; -import org.raddatz.familienarchiv.repository.AppUserRepository; import org.raddatz.familienarchiv.repository.PasswordResetTokenRepository; import org.springframework.mail.MailSendException; import org.springframework.mail.SimpleMailMessage; @@ -33,7 +32,7 @@ import org.springframework.test.util.ReflectionTestUtils; @ExtendWith(MockitoExtension.class) class PasswordResetServiceTest { - @Mock AppUserRepository userRepository; + @Mock UserService userService; @Mock PasswordResetTokenRepository tokenRepository; @Mock PasswordEncoder passwordEncoder; @Mock JavaMailSender mailSender; @@ -53,7 +52,7 @@ class PasswordResetServiceTest { @Test void requestReset_savesTokenForKnownEmail() { AppUser user = makeUser("user@example.com"); - when(userRepository.findByEmail("user@example.com")).thenReturn(Optional.of(user)); + when(userService.findByEmailOptional("user@example.com")).thenReturn(Optional.of(user)); service.requestReset("user@example.com", "http://localhost:3000"); @@ -65,7 +64,7 @@ class PasswordResetServiceTest { @Test void requestReset_doesNothingForUnknownEmail() { - when(userRepository.findByEmail("ghost@example.com")).thenReturn(Optional.empty()); + when(userService.findByEmailOptional("ghost@example.com")).thenReturn(Optional.empty()); service.requestReset("ghost@example.com", "http://localhost:3000"); @@ -93,7 +92,7 @@ class PasswordResetServiceTest { service.resetPassword(req); verify(passwordEncoder).encode("newpass"); - verify(userRepository).save(argThat(u -> u.getPassword().equals("hashed-newpass"))); + verify(userService).save(argThat(u -> u.getPassword().equals("hashed-newpass"))); assertThat(token.isUsed()).isTrue(); } @@ -153,7 +152,7 @@ class PasswordResetServiceTest { void requestReset_skipsEmail_whenMailSenderIsNull() { ReflectionTestUtils.setField(service, "mailSender", null); AppUser user = makeUser("user@example.com"); - when(userRepository.findByEmail("user@example.com")).thenReturn(Optional.of(user)); + when(userService.findByEmailOptional("user@example.com")).thenReturn(Optional.of(user)); // Must not throw even without mail sender service.requestReset("user@example.com", "http://localhost:3000"); @@ -167,7 +166,7 @@ class PasswordResetServiceTest { // mailSender is @Autowired(required=false) — not in constructor, so needs explicit injection ReflectionTestUtils.setField(service, "mailSender", mailSender); AppUser user = makeUser("user@example.com"); - when(userRepository.findByEmail("user@example.com")).thenReturn(Optional.of(user)); + when(userService.findByEmailOptional("user@example.com")).thenReturn(Optional.of(user)); doThrow(new MailSendException("SMTP error")).when(mailSender).send(any(SimpleMailMessage.class)); // Must not propagate the MailException diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/PasswordResetTestHelperTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/PasswordResetTestHelperTest.java new file mode 100644 index 00000000..c770fc0f --- /dev/null +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/PasswordResetTestHelperTest.java @@ -0,0 +1,35 @@ +package org.raddatz.familienarchiv.service; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class PasswordResetTestHelperTest { + + @Mock PasswordResetService passwordResetService; + @InjectMocks PasswordResetTestHelper helper; + + @Test + void getResetTokenForTest_returnsToken_whenPresent() { + when(passwordResetService.findLatestActiveTokenForEmail("user@example.com")) + .thenReturn(Optional.of("abc123")); + + assertThat(helper.getResetTokenForTest("user@example.com")).contains("abc123"); + } + + @Test + void getResetTokenForTest_returnsEmpty_whenAbsent() { + when(passwordResetService.findLatestActiveTokenForEmail("ghost@example.com")) + .thenReturn(Optional.empty()); + + assertThat(helper.getResetTokenForTest("ghost@example.com")).isEmpty(); + } +} diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java index c146323f..abe0a969 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/UserServiceTest.java @@ -58,6 +58,34 @@ class UserServiceTest { assertThat(userService.findByEmail("admin@example.com")).isEqualTo(user); } + // ─── findByEmailOptional ────────────────────────────────────────────────── + + @Test + void findByEmailOptional_returnsEmpty_whenMissing() { + when(userRepository.findByEmail("ghost@example.com")).thenReturn(Optional.empty()); + + assertThat(userService.findByEmailOptional("ghost@example.com")).isEmpty(); + } + + @Test + void findByEmailOptional_returnsUser_whenFound() { + AppUser user = AppUser.builder().id(UUID.randomUUID()).email("admin@example.com").build(); + when(userRepository.findByEmail("admin@example.com")).thenReturn(Optional.of(user)); + + assertThat(userService.findByEmailOptional("admin@example.com")).contains(user); + } + + // ─── save ───────────────────────────────────────────────────────────────── + + @Test + void save_delegatesToRepository() { + AppUser user = AppUser.builder().id(UUID.randomUUID()).email("u@example.com").build(); + when(userRepository.save(user)).thenReturn(user); + + assertThat(userService.save(user)).isEqualTo(user); + verify(userRepository).save(user); + } + // ─── deleteUser ─────────────────────────────────────────────────────────── @Test -- 2.49.1 From e2e7b790678bf271c9ede28a1c98a303254054b2 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 07:20:01 +0200 Subject: [PATCH 3/9] refactor(thumbnail): route document access through DocumentService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Thumbnail trio (ThumbnailService, ThumbnailBackfillService, ThumbnailAsyncRunner) all injected DocumentRepository directly. They now go through three new DocumentService delegations: - findById(UUID): Optional — no-throw variant for the runner's log-and-skip behaviour on missing documents. - findForThumbnailBackfill() — wraps the existing findByFilePathIsNotNullAndThumbnailKeyIsNull query. - updateThumbnailMetadata(Document) — wraps save() for the post-thumbnail entity update. DocumentService also gains @Lazy on its existing ThumbnailAsyncRunner field to break the new DocumentService ↔ ThumbnailAsyncRunner cycle. lombok.config adds @Lazy to copyableAnnotations so the field annotation reaches the generated constructor parameter. Refs #417 (C6.2 violations #2, #3, #4). Co-Authored-By: Claude Opus 4.7 --- backend/lombok.config | 1 + .../service/DocumentService.java | 17 ++++++++ .../service/ThumbnailAsyncRunner.java | 5 +-- .../service/ThumbnailBackfillService.java | 5 +-- .../service/ThumbnailService.java | 9 ++-- .../service/DocumentServiceTest.java | 42 +++++++++++++++++++ .../service/ThumbnailAsyncRunnerTest.java | 17 ++++---- .../service/ThumbnailBackfillServiceTest.java | 17 ++++---- .../service/ThumbnailServiceTest.java | 23 +++++----- 9 files changed, 95 insertions(+), 41 deletions(-) create mode 100644 backend/lombok.config diff --git a/backend/lombok.config b/backend/lombok.config new file mode 100644 index 00000000..a0191809 --- /dev/null +++ b/backend/lombok.config @@ -0,0 +1 @@ +lombok.copyableAnnotations += org.springframework.context.annotation.Lazy diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index 6cf4cb61..399395b4 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -25,6 +25,7 @@ import org.raddatz.familienarchiv.model.TrainingLabel; import org.raddatz.familienarchiv.model.Person; import org.raddatz.familienarchiv.model.Tag; import org.raddatz.familienarchiv.repository.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; @@ -69,6 +70,10 @@ 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) {} @@ -77,6 +82,18 @@ public class DocumentService { return documentRepository.count(); } + public Optional findById(UUID id) { + return documentRepository.findById(id); + } + + public List findForThumbnailBackfill() { + return documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull(); + } + + public Document updateThumbnailMetadata(Document doc) { + return documentRepository.save(doc); + } + public Map findTitlesByIds(Collection ids) { if (ids.isEmpty()) return Map.of(); Map titles = new HashMap<>(); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailAsyncRunner.java b/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailAsyncRunner.java index a90a7a39..1b5cd867 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailAsyncRunner.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailAsyncRunner.java @@ -3,7 +3,6 @@ package org.raddatz.familienarchiv.service; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.raddatz.familienarchiv.model.Document; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Service; import org.springframework.transaction.support.TransactionSynchronization; @@ -29,7 +28,7 @@ import java.util.concurrent.TimeoutException; @Slf4j public class ThumbnailAsyncRunner { - private final DocumentRepository documentRepository; + private final DocumentService documentService; private final ThumbnailService thumbnailService; /** Per-document timeout for the whole generate() call — defense against corrupt PDFs. */ @@ -60,7 +59,7 @@ public class ThumbnailAsyncRunner { */ @Async("thumbnailExecutor") public void generateAsync(UUID documentId) { - Optional docOpt = documentRepository.findById(documentId); + Optional docOpt = documentService.findById(documentId); if (docOpt.isEmpty()) { log.warn("Thumbnail generation skipped: document not found id={}", documentId); return; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailBackfillService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailBackfillService.java index 60855ba6..9c230d1c 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailBackfillService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailBackfillService.java @@ -5,7 +5,6 @@ import lombok.extern.slf4j.Slf4j; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.Document; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Service; @@ -37,7 +36,7 @@ public class ThumbnailBackfillService { LocalDateTime startedAt ) {} - private final DocumentRepository documentRepository; + private final DocumentService documentService; private final ThumbnailService thumbnailService; private volatile BackfillStatus currentStatus = new BackfillStatus( @@ -57,7 +56,7 @@ public class ThumbnailBackfillService { LocalDateTime startedAt = LocalDateTime.now(); List docs; try { - docs = documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull(); + docs = documentService.findForThumbnailBackfill(); } catch (Exception e) { currentStatus = new BackfillStatus(State.FAILED, "Backfill fehlgeschlagen: " + e.getMessage(), diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailService.java index f654b922..b1441437 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/ThumbnailService.java @@ -8,7 +8,6 @@ import org.apache.pdfbox.rendering.ImageType; import org.apache.pdfbox.rendering.PDFRenderer; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.ThumbnailAspect; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Service; import software.amazon.awssdk.core.sync.RequestBody; @@ -62,16 +61,16 @@ public class ThumbnailService { private final FileService fileService; private final S3Client s3Client; - private final DocumentRepository documentRepository; + private final DocumentService documentService; @Value("${app.s3.bucket}") private String bucketName; public ThumbnailService(FileService fileService, S3Client s3Client, - DocumentRepository documentRepository) { + DocumentService documentService) { this.fileService = fileService; this.s3Client = s3Client; - this.documentRepository = documentRepository; + this.documentService = documentService; } public Outcome generate(Document doc) { @@ -167,7 +166,7 @@ public class ThumbnailService { doc.setThumbnailGeneratedAt(LocalDateTime.now()); doc.setThumbnailAspect(result.aspect()); doc.setPageCount(result.pageCount()); - documentRepository.save(doc); + documentService.updateThumbnailMetadata(doc); return Outcome.SUCCESS; } catch (Exception e) { // Thumbnail is already in S3 but the entity update failed. Because the S3 diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java index f3a95e4f..7136d245 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -2264,4 +2264,46 @@ class DocumentServiceTest { assertThat(doc.getArchiveFolder()).isEqualTo("KeepFolder"); assertThat(doc.getDocumentLocation()).isEqualTo("KeepLocation"); } + + // ─── findById (no-throw variant) ─────────────────────────────────────────── + + @Test + void findById_returnsEmpty_whenDocumentDoesNotExist() { + UUID id = UUID.randomUUID(); + when(documentRepository.findById(id)).thenReturn(Optional.empty()); + + assertThat(documentService.findById(id)).isEmpty(); + } + + @Test + void findById_returnsDocument_whenPresent() { + UUID id = UUID.randomUUID(); + Document doc = Document.builder().id(id).title("T").build(); + when(documentRepository.findById(id)).thenReturn(Optional.of(doc)); + + assertThat(documentService.findById(id)).contains(doc); + } + + // ─── findForThumbnailBackfill ────────────────────────────────────────────── + + @Test + void findForThumbnailBackfill_returnsRepositoryResult() { + Document a = Document.builder().id(UUID.randomUUID()).title("A").build(); + Document b = Document.builder().id(UUID.randomUUID()).title("B").build(); + when(documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull()) + .thenReturn(List.of(a, b)); + + 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); + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailAsyncRunnerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailAsyncRunnerTest.java index 2f55b889..71909848 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailAsyncRunnerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailAsyncRunnerTest.java @@ -4,7 +4,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.raddatz.familienarchiv.model.Document; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -18,22 +17,22 @@ import static org.mockito.Mockito.*; class ThumbnailAsyncRunnerTest { - private DocumentRepository documentRepository; + private DocumentService documentService; private ThumbnailService thumbnailService; private ThumbnailAsyncRunner runner; @BeforeEach void setUp() { - documentRepository = mock(DocumentRepository.class); + documentService = mock(DocumentService.class); thumbnailService = mock(ThumbnailService.class); - runner = new ThumbnailAsyncRunner(documentRepository, thumbnailService); + runner = new ThumbnailAsyncRunner(documentService, thumbnailService); } @Test void dispatchAfterCommit_whenNoTransaction_dispatchesImmediately() { UUID id = UUID.randomUUID(); Document doc = Document.builder().id(id).originalFilename("f.pdf").title("t").build(); - when(documentRepository.findById(id)).thenReturn(Optional.of(doc)); + when(documentService.findById(id)).thenReturn(Optional.of(doc)); runner.dispatchAfterCommit(id); @@ -44,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(documentRepository.findById(id)).thenReturn(Optional.of(doc)); + when(documentService.findById(id)).thenReturn(Optional.of(doc)); TransactionSynchronizationManager.initSynchronization(); try { @@ -69,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(documentRepository.findById(id)).thenReturn(Optional.of(doc)); + when(documentService.findById(id)).thenReturn(Optional.of(doc)); TransactionSynchronizationManager.initSynchronization(); try { @@ -88,7 +87,7 @@ class ThumbnailAsyncRunnerTest { @Test void generateAsync_skipsWhenDocumentMissing() { UUID id = UUID.randomUUID(); - when(documentRepository.findById(id)).thenReturn(Optional.empty()); + when(documentService.findById(id)).thenReturn(Optional.empty()); runner.generateAsync(id); @@ -99,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(documentRepository.findById(id)).thenReturn(Optional.of(doc)); + when(documentService.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); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailBackfillServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailBackfillServiceTest.java index 4e076395..319e7af4 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailBackfillServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailBackfillServiceTest.java @@ -5,7 +5,6 @@ import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.Document; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.test.util.ReflectionTestUtils; import java.time.LocalDateTime; @@ -19,15 +18,15 @@ import static org.mockito.Mockito.*; class ThumbnailBackfillServiceTest { - private DocumentRepository documentRepository; + private DocumentService documentService; private ThumbnailService thumbnailService; private ThumbnailBackfillService backfillService; @BeforeEach void setUp() { - documentRepository = mock(DocumentRepository.class); + documentService = mock(DocumentService.class); thumbnailService = mock(ThumbnailService.class); - backfillService = new ThumbnailBackfillService(documentRepository, thumbnailService); + backfillService = new ThumbnailBackfillService(documentService, thumbnailService); } @Test @@ -45,7 +44,7 @@ class ThumbnailBackfillServiceTest { Document a = doc(); Document b = doc(); Document c = doc(); - when(documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull()) + when(documentService.findForThumbnailBackfill()) .thenReturn(List.of(a, b, c)); when(thumbnailService.generate(any())).thenReturn(ThumbnailService.Outcome.SUCCESS); @@ -64,7 +63,7 @@ class ThumbnailBackfillServiceTest { void runBackfillAsync_countsSkippedSeparately() { Document a = doc(); Document b = doc(); - when(documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull()) + when(documentService.findForThumbnailBackfill()) .thenReturn(List.of(a, b)); when(thumbnailService.generate(a)).thenReturn(ThumbnailService.Outcome.SUCCESS); when(thumbnailService.generate(b)).thenReturn(ThumbnailService.Outcome.SKIPPED); @@ -83,7 +82,7 @@ class ThumbnailBackfillServiceTest { Document a = doc(); Document b = doc(); Document c = doc(); - when(documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull()) + when(documentService.findForThumbnailBackfill()) .thenReturn(List.of(a, b, c)); when(thumbnailService.generate(a)).thenReturn(ThumbnailService.Outcome.SUCCESS); when(thumbnailService.generate(b)).thenReturn(ThumbnailService.Outcome.FAILED); @@ -102,7 +101,7 @@ class ThumbnailBackfillServiceTest { void runBackfillAsync_continuesWhenServiceThrowsUnexpectedException() { Document a = doc(); Document b = doc(); - when(documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull()) + when(documentService.findForThumbnailBackfill()) .thenReturn(List.of(a, b)); when(thumbnailService.generate(a)).thenThrow(new RuntimeException("boom")); when(thumbnailService.generate(b)).thenReturn(ThumbnailService.Outcome.SUCCESS); @@ -130,7 +129,7 @@ class ThumbnailBackfillServiceTest { @Test void runBackfillAsync_setsStartedAtAndMessage() { - when(documentRepository.findByFilePathIsNotNullAndThumbnailKeyIsNull()) + when(documentService.findForThumbnailBackfill()) .thenReturn(List.of(doc())); when(thumbnailService.generate(any())).thenReturn(ThumbnailService.Outcome.SUCCESS); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailServiceTest.java index ad423401..43394eab 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/ThumbnailServiceTest.java @@ -12,7 +12,6 @@ import org.mockito.ArgumentCaptor; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.model.ThumbnailAspect; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.test.util.ReflectionTestUtils; import software.amazon.awssdk.core.sync.RequestBody; import software.amazon.awssdk.services.s3.S3Client; @@ -39,17 +38,17 @@ class ThumbnailServiceTest { private FileService fileService; private S3Client s3Client; - private DocumentRepository documentRepository; + private DocumentService documentService; private ThumbnailService thumbnailService; @BeforeEach void setUp() { fileService = mock(FileService.class); s3Client = mock(S3Client.class); - documentRepository = mock(DocumentRepository.class); - thumbnailService = new ThumbnailService(fileService, s3Client, documentRepository); + documentService = mock(DocumentService.class); + thumbnailService = new ThumbnailService(fileService, s3Client, documentService); ReflectionTestUtils.setField(thumbnailService, "bucketName", "test-bucket"); - when(documentRepository.save(any(Document.class))).thenAnswer(i -> i.getArgument(0)); + when(documentService.updateThumbnailMetadata(any(Document.class))).thenAnswer(i -> i.getArgument(0)); } @Test @@ -103,7 +102,7 @@ class ThumbnailServiceTest { assertThat(doc.getThumbnailKey()).isEqualTo("thumbnails/" + doc.getId() + ".jpg"); assertThat(doc.getThumbnailGeneratedAt()).isNotNull(); - verify(documentRepository).save(doc); + verify(documentService).updateThumbnailMetadata(doc); } @Test @@ -152,7 +151,7 @@ class ThumbnailServiceTest { assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); assertThat(doc.getThumbnailKey()).isNull(); - verify(documentRepository, never()).save(any()); + verify(documentService, never()).updateThumbnailMetadata(any()); } @Test @@ -165,7 +164,7 @@ class ThumbnailServiceTest { assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); verifyNoInteractions(s3Client); - verify(documentRepository, never()).save(any()); + verify(documentService, never()).updateThumbnailMetadata(any()); } @Test @@ -260,7 +259,7 @@ class ThumbnailServiceTest { assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); verifyNoInteractions(s3Client); - verify(documentRepository, never()).save(any()); + verify(documentService, never()).updateThumbnailMetadata(any()); } @Test @@ -275,7 +274,7 @@ class ThumbnailServiceTest { assertThat(outcome).isEqualTo(ThumbnailService.Outcome.FAILED); verifyNoInteractions(s3Client); - verify(documentRepository, never()).save(any()); + verify(documentService, never()).updateThumbnailMetadata(any()); } @Test @@ -286,14 +285,14 @@ class ThumbnailServiceTest { Document doc = makeDoc("application/pdf", "documents/letter.pdf"); when(fileService.downloadFileStream(anyString())) .thenReturn(new ByteArrayInputStream(createSamplePdf())); - when(documentRepository.save(any())) + when(documentService.updateThumbnailMetadata(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(documentRepository).save(any()); + verify(documentService).updateThumbnailMetadata(any()); } // ─── helpers ────────────────────────────────────────────────────────────── -- 2.49.1 From 8b177b94303a6d512ba1cd340b1980ef20f8f273 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 07:23:25 +0200 Subject: [PATCH 4/9] refactor(transcription-queue): route through DocumentService projections TranscriptionQueueService injected DocumentRepository to fetch the four queue projections. Move the four read methods (findSegmentationQueue, findTranscriptionQueue, findReadyToReadQueue, findWeeklyStats) onto DocumentService as 1-line delegations and update the consumer. Refs #417 (C6.2 violation #5). Co-Authored-By: Claude Opus 4.7 --- .../service/DocumentService.java | 16 ++++++++++ .../service/TranscriptionQueueService.java | 11 ++++--- .../TranscriptionQueueServiceTest.java | 29 +++++++++---------- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index 399395b4..d02c36aa 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -94,6 +94,22 @@ public class DocumentService { return documentRepository.save(doc); } + public List findSegmentationQueue(int limit) { + return documentRepository.findSegmentationQueue(limit); + } + + public List findTranscriptionQueue(int limit) { + return documentRepository.findTranscriptionQueue(limit); + } + + public List findReadyToReadQueue(int limit) { + return documentRepository.findReadyToReadQueue(limit); + } + + public org.raddatz.familienarchiv.repository.TranscriptionWeeklyStatsProjection findWeeklyStats() { + return documentRepository.findWeeklyStats(); + } + public Map findTitlesByIds(Collection ids) { if (ids.isEmpty()) return Map.of(); Map titles = new HashMap<>(); diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionQueueService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionQueueService.java index 6b82abb9..50ffbac6 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionQueueService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionQueueService.java @@ -5,7 +5,6 @@ import org.raddatz.familienarchiv.audit.ActivityActorDTO; import org.raddatz.familienarchiv.audit.AuditLogQueryService; import org.raddatz.familienarchiv.dto.TranscriptionQueueItemDTO; import org.raddatz.familienarchiv.dto.TranscriptionWeeklyStatsDTO; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.raddatz.familienarchiv.repository.TranscriptionQueueProjection; import org.springframework.stereotype.Service; @@ -20,23 +19,23 @@ public class TranscriptionQueueService { private static final int DEFAULT_QUEUE_SIZE = 5; private static final int MAX_CONTRIBUTORS = 5; - private final DocumentRepository documentRepository; + private final DocumentService documentService; private final AuditLogQueryService auditLogQueryService; public List getSegmentationQueue() { - return enrichWithContributors(documentRepository.findSegmentationQueue(DEFAULT_QUEUE_SIZE)); + return enrichWithContributors(documentService.findSegmentationQueue(DEFAULT_QUEUE_SIZE)); } public List getTranscriptionQueue() { - return enrichWithContributors(documentRepository.findTranscriptionQueue(DEFAULT_QUEUE_SIZE)); + return enrichWithContributors(documentService.findTranscriptionQueue(DEFAULT_QUEUE_SIZE)); } public List getReadyToReadQueue() { - return enrichWithContributors(documentRepository.findReadyToReadQueue(DEFAULT_QUEUE_SIZE)); + return enrichWithContributors(documentService.findReadyToReadQueue(DEFAULT_QUEUE_SIZE)); } public TranscriptionWeeklyStatsDTO getWeeklyStats() { - var stats = documentRepository.findWeeklyStats(); + var stats = documentService.findWeeklyStats(); return new TranscriptionWeeklyStatsDTO( stats.getSegmentationCount(), stats.getTranscriptionCount() diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionQueueServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionQueueServiceTest.java index 6c7a47ac..624a3f82 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionQueueServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionQueueServiceTest.java @@ -10,7 +10,6 @@ import org.raddatz.familienarchiv.audit.ActivityActorDTO; import org.raddatz.familienarchiv.audit.AuditLogQueryService; import org.raddatz.familienarchiv.dto.TranscriptionQueueItemDTO; import org.raddatz.familienarchiv.dto.TranscriptionWeeklyStatsDTO; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.raddatz.familienarchiv.repository.TranscriptionQueueProjection; import org.raddatz.familienarchiv.repository.TranscriptionWeeklyStatsProjection; @@ -26,7 +25,7 @@ import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) class TranscriptionQueueServiceTest { - @Mock DocumentRepository documentRepository; + @Mock DocumentService documentService; @Mock AuditLogQueryService auditLogQueryService; @InjectMocks TranscriptionQueueService service; @@ -41,11 +40,11 @@ class TranscriptionQueueServiceTest { void getSegmentationQueue_delegatesToRepositoryWithDefaultSize() { UUID id = UUID.randomUUID(); TranscriptionQueueProjection proj = mockQueueProjection(id, "Brief von 1920", null, 0, 0, 0); - when(documentRepository.findSegmentationQueue(5)).thenReturn(List.of(proj)); + when(documentService.findSegmentationQueue(5)).thenReturn(List.of(proj)); List result = service.getSegmentationQueue(); - verify(documentRepository).findSegmentationQueue(5); + verify(documentService).findSegmentationQueue(5); assertThat(result).hasSize(1); assertThat(result.get(0).id()).isEqualTo(id); assertThat(result.get(0).title()).isEqualTo("Brief von 1920"); @@ -55,7 +54,7 @@ class TranscriptionQueueServiceTest { @Test void getSegmentationQueue_returnsEmptyList_whenQueueIsEmpty() { - when(documentRepository.findSegmentationQueue(5)).thenReturn(List.of()); + when(documentService.findSegmentationQueue(5)).thenReturn(List.of()); List result = service.getSegmentationQueue(); @@ -67,7 +66,7 @@ class TranscriptionQueueServiceTest { void getSegmentationQueue_returnsAllFive_andHasMoreFalse_whenExactlyFiveContributors() { UUID docId = UUID.randomUUID(); TranscriptionQueueProjection proj = mockQueueProjection(docId, "Brief", null, 0, 0, 0); - when(documentRepository.findSegmentationQueue(5)).thenReturn(List.of(proj)); + when(documentService.findSegmentationQueue(5)).thenReturn(List.of(proj)); List fiveActors = List.of( new ActivityActorDTO("A1", "#111", "Alice One"), @@ -89,7 +88,7 @@ class TranscriptionQueueServiceTest { void getSegmentationQueue_mapsDocumentDateWhenPresent() { LocalDate date = LocalDate.of(1920, 6, 15); TranscriptionQueueProjection proj = mockQueueProjection(UUID.randomUUID(), "Brief", date, 0, 0, 0); - when(documentRepository.findSegmentationQueue(5)).thenReturn(List.of(proj)); + when(documentService.findSegmentationQueue(5)).thenReturn(List.of(proj)); List result = service.getSegmentationQueue(); @@ -102,11 +101,11 @@ class TranscriptionQueueServiceTest { void getTranscriptionQueue_delegatesToRepositoryWithDefaultSize() { UUID id = UUID.randomUUID(); TranscriptionQueueProjection proj = mockQueueProjection(id, "Tagebuch", LocalDate.of(1943, 1, 1), 3, 1, 0); - when(documentRepository.findTranscriptionQueue(5)).thenReturn(List.of(proj)); + when(documentService.findTranscriptionQueue(5)).thenReturn(List.of(proj)); List result = service.getTranscriptionQueue(); - verify(documentRepository).findTranscriptionQueue(5); + verify(documentService).findTranscriptionQueue(5); assertThat(result).hasSize(1); assertThat(result.get(0).annotationCount()).isEqualTo(3); assertThat(result.get(0).textedBlockCount()).isEqualTo(1); @@ -118,11 +117,11 @@ class TranscriptionQueueServiceTest { @Test void getReadyToReadQueue_delegatesToRepositoryWithDefaultSize() { TranscriptionQueueProjection proj = mockQueueProjection(UUID.randomUUID(), "Urkunde", null, 4, 4, 4); - when(documentRepository.findReadyToReadQueue(5)).thenReturn(List.of(proj)); + when(documentService.findReadyToReadQueue(5)).thenReturn(List.of(proj)); List result = service.getReadyToReadQueue(); - verify(documentRepository).findReadyToReadQueue(5); + verify(documentService).findReadyToReadQueue(5); assertThat(result).hasSize(1); assertThat(result.get(0).reviewedBlockCount()).isEqualTo(4); } @@ -132,7 +131,7 @@ class TranscriptionQueueServiceTest { @Test void getWeeklyStats_mapsProjectionToDTO() { TranscriptionWeeklyStatsProjection proj = mockStatsProjection(3L, 7L); - when(documentRepository.findWeeklyStats()).thenReturn(proj); + when(documentService.findWeeklyStats()).thenReturn(proj); TranscriptionWeeklyStatsDTO result = service.getWeeklyStats(); @@ -143,7 +142,7 @@ class TranscriptionQueueServiceTest { @Test void getWeeklyStats_returnsZeros_whenAllCountsAreZero() { TranscriptionWeeklyStatsProjection proj = mockStatsProjection(0L, 0L); - when(documentRepository.findWeeklyStats()).thenReturn(proj); + when(documentService.findWeeklyStats()).thenReturn(proj); TranscriptionWeeklyStatsDTO result = service.getWeeklyStats(); @@ -157,7 +156,7 @@ class TranscriptionQueueServiceTest { void getSegmentationQueue_includesContributors_whenAuditDataPresent() { UUID docId = UUID.randomUUID(); TranscriptionQueueProjection proj = mockQueueProjection(docId, "Brief", null, 0, 0, 0); - when(documentRepository.findSegmentationQueue(5)).thenReturn(List.of(proj)); + when(documentService.findSegmentationQueue(5)).thenReturn(List.of(proj)); ActivityActorDTO actor = new ActivityActorDTO("MR", "#a6dad8", "Max Raddatz"); when(auditLogQueryService.findContributorsPerDocument(List.of(docId))) @@ -173,7 +172,7 @@ class TranscriptionQueueServiceTest { void getSegmentationQueue_capsContributorsAtFive_andSetsHasMoreFlag() { UUID docId = UUID.randomUUID(); TranscriptionQueueProjection proj = mockQueueProjection(docId, "Brief", null, 0, 0, 0); - when(documentRepository.findSegmentationQueue(5)).thenReturn(List.of(proj)); + when(documentService.findSegmentationQueue(5)).thenReturn(List.of(proj)); List sixActors = List.of( new ActivityActorDTO("A1", "#111", "Alice One"), -- 2.49.1 From 0ca95d5ad7c6703d7d82838cf474cd9761b3e527 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 07:27:30 +0200 Subject: [PATCH 5/9] refactor(import): route MassImportService through DocumentService MassImportService injected DocumentRepository for the find-or-create pattern during ODS/Excel import. Move the two repository touchpoints (findByOriginalFilename, save) onto DocumentService as 1-line delegations and update the consumer. Refs #417 (C6.2 violation #1). Co-Authored-By: Claude Opus 4.7 --- .../service/DocumentService.java | 8 ++ .../service/MassImportService.java | 7 +- .../service/DocumentServiceTest.java | 21 ++++ .../service/MassImportServiceTest.java | 101 +++++++++--------- 4 files changed, 82 insertions(+), 55 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index d02c36aa..0c691c03 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -94,6 +94,14 @@ public class DocumentService { return documentRepository.save(doc); } + public Optional findByOriginalFilename(String originalFilename) { + return documentRepository.findByOriginalFilename(originalFilename); + } + + public Document save(Document doc) { + return documentRepository.save(doc); + } + public List findSegmentationQueue(int limit) { return documentRepository.findSegmentationQueue(limit); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/MassImportService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/MassImportService.java index 7e847ff8..ebfca16e 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/MassImportService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/MassImportService.java @@ -10,7 +10,6 @@ import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.model.Person; import org.raddatz.familienarchiv.model.Tag; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.beans.factory.annotation.Value; import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Service; @@ -55,7 +54,7 @@ public class MassImportService { return currentStatus; } - private final DocumentRepository documentRepository; + private final DocumentService documentService; private final PersonService personService; private final TagService tagService; private final S3Client s3Client; @@ -257,7 +256,7 @@ public class MassImportService { @Transactional protected void importSingleDocument(List cells, Optional file, String originalFilename, String index) { - Optional existing = documentRepository.findByOriginalFilename(originalFilename); + Optional existing = documentService.findByOriginalFilename(originalFilename); if (existing.isPresent() && existing.get().getStatus() != DocumentStatus.PLACEHOLDER) { log.info("Dokument {} existiert bereits, überspringe.", originalFilename); return; @@ -333,7 +332,7 @@ public class MassImportService { if (tag != null) doc.getTags().add(tag); doc.setMetadataComplete(metadataComplete); - Document saved = documentRepository.save(doc); + Document saved = documentService.save(doc); if (file.isPresent()) { thumbnailAsyncRunner.dispatchAfterCommit(saved.getId()); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java index 7136d245..015d6302 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/DocumentServiceTest.java @@ -2306,4 +2306,25 @@ class DocumentServiceTest { assertThat(documentService.updateThumbnailMetadata(doc)).isEqualTo(doc); verify(documentRepository).save(doc); } + + // ─── findByOriginalFilename ──────────────────────────────────────────────── + + @Test + void findByOriginalFilename_returnsRepositoryResult() { + Document doc = Document.builder().id(UUID.randomUUID()).title("T").build(); + when(documentRepository.findByOriginalFilename("scan.pdf")).thenReturn(Optional.of(doc)); + + assertThat(documentService.findByOriginalFilename("scan.pdf")).contains(doc); + } + + // ─── save ────────────────────────────────────────────────────────────────── + + @Test + void save_delegatesToRepository() { + Document doc = Document.builder().id(UUID.randomUUID()).title("T").build(); + when(documentRepository.save(doc)).thenReturn(doc); + + assertThat(documentService.save(doc)).isEqualTo(doc); + verify(documentRepository).save(doc); + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/MassImportServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/MassImportServiceTest.java index 62b138b7..00416943 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/MassImportServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/MassImportServiceTest.java @@ -11,7 +11,6 @@ import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentStatus; import org.raddatz.familienarchiv.model.Person; import org.raddatz.familienarchiv.model.Tag; -import org.raddatz.familienarchiv.repository.DocumentRepository; import org.springframework.test.util.ReflectionTestUtils; import software.amazon.awssdk.core.sync.RequestBody; import software.amazon.awssdk.services.s3.S3Client; @@ -35,7 +34,7 @@ import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) class MassImportServiceTest { - @Mock DocumentRepository documentRepository; + @Mock DocumentService documentService; @Mock PersonService personService; @Mock TagService tagService; @Mock S3Client s3Client; @@ -45,7 +44,7 @@ class MassImportServiceTest { @BeforeEach void setUp() { - service = new MassImportService(documentRepository, personService, tagService, s3Client, thumbnailAsyncRunner); + service = new MassImportService(documentService, personService, tagService, s3Client, thumbnailAsyncRunner); ReflectionTestUtils.setField(service, "bucketName", "test-bucket"); ReflectionTestUtils.setField(service, "colIndex", 0); ReflectionTestUtils.setField(service, "colBox", 1); @@ -96,23 +95,23 @@ class MassImportServiceTest { .originalFilename("doc001.pdf") .status(DocumentStatus.UPLOADED) .build(); - when(documentRepository.findByOriginalFilename("doc001.pdf")).thenReturn(Optional.of(existing)); + when(documentService.findByOriginalFilename("doc001.pdf")).thenReturn(Optional.of(existing)); service.importSingleDocument(minimalCells("doc001.pdf"), Optional.empty(), "doc001.pdf", "doc001"); - verify(documentRepository, never()).save(any()); + verify(documentService, never()).save(any()); } // ─── importSingleDocument — create new document (metadata only) ─────────── @Test void importSingleDocument_createsNewDocument_whenNotExists() { - when(documentRepository.findByOriginalFilename("doc002.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("doc002.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); service.importSingleDocument(minimalCells("doc002.pdf"), Optional.empty(), "doc002.pdf", "doc002"); - verify(documentRepository).save(argThat(d -> + verify(documentService).save(argThat(d -> d.getOriginalFilename().equals("doc002.pdf") && d.getStatus() == DocumentStatus.PLACEHOLDER)); } @@ -126,12 +125,12 @@ class MassImportServiceTest { .originalFilename("existing.pdf") .status(DocumentStatus.PLACEHOLDER) .build(); - when(documentRepository.findByOriginalFilename("existing.pdf")).thenReturn(Optional.of(placeholder)); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("existing.pdf")).thenReturn(Optional.of(placeholder)); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); service.importSingleDocument(minimalCells("existing.pdf"), Optional.empty(), "existing.pdf", "existing"); - verify(documentRepository).save(same(placeholder)); + verify(documentService).save(same(placeholder)); } // ─── importSingleDocument — with file (S3 upload) ───────────────────────── @@ -141,14 +140,14 @@ class MassImportServiceTest { Path tempFile = tempDir.resolve("doc003.pdf"); Files.write(tempFile, "PDF content".getBytes()); - when(documentRepository.findByOriginalFilename("doc003.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("doc003.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); service.importSingleDocument( minimalCells("doc003.pdf"), Optional.of(tempFile.toFile()), "doc003.pdf", "doc003"); verify(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class)); - verify(documentRepository).save(argThat(d -> d.getStatus() == DocumentStatus.UPLOADED)); + verify(documentService).save(argThat(d -> d.getStatus() == DocumentStatus.UPLOADED)); } @Test @@ -156,42 +155,42 @@ class MassImportServiceTest { Path tempFile = tempDir.resolve("fail.pdf"); Files.write(tempFile, "data".getBytes()); - when(documentRepository.findByOriginalFilename("fail.pdf")).thenReturn(Optional.empty()); + when(documentService.findByOriginalFilename("fail.pdf")).thenReturn(Optional.empty()); doThrow(new RuntimeException("S3 error")) .when(s3Client).putObject(any(PutObjectRequest.class), any(RequestBody.class)); service.importSingleDocument( minimalCells("fail.pdf"), Optional.of(tempFile.toFile()), "fail.pdf", "fail"); - verify(documentRepository, never()).save(any()); + verify(documentService, never()).save(any()); } // ─── importSingleDocument — sender handling ─────────────────────────────── @Test void importSingleDocument_setsNullSender_whenSenderCellIsBlank() { - when(documentRepository.findByOriginalFilename("nosender.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("nosender.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); List cells = buildCells("nosender.pdf", "", "", ""); service.importSingleDocument(cells, Optional.empty(), "nosender.pdf", "nosender"); - verify(documentRepository).save(argThat(d -> d.getSender() == null)); + verify(documentService).save(argThat(d -> d.getSender() == null)); verify(personService, never()).findOrCreateByAlias(any()); } @Test void importSingleDocument_createsSender_whenSenderCellIsNonBlank() { Person sender = Person.builder().id(UUID.randomUUID()).firstName("Walter").lastName("Müller").build(); - when(documentRepository.findByOriginalFilename("withsender.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("withsender.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); when(personService.findOrCreateByAlias("Walter Müller")).thenReturn(sender); List cells = buildCells("withsender.pdf", "Walter Müller", "", ""); service.importSingleDocument(cells, Optional.empty(), "withsender.pdf", "withsender"); verify(personService).findOrCreateByAlias("Walter Müller"); - verify(documentRepository).save(argThat(d -> d.getSender() == sender)); + verify(documentService).save(argThat(d -> d.getSender() == sender)); } // ─── importSingleDocument — tag handling ───────────────────────────────── @@ -199,8 +198,8 @@ class MassImportServiceTest { @Test void importSingleDocument_createsTag_whenTagCellIsNonBlank() { Tag tag = Tag.builder().id(UUID.randomUUID()).name("Familie").build(); - when(documentRepository.findByOriginalFilename("tagged.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("tagged.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); when(tagService.findOrCreate("Familie")).thenReturn(tag); List cells = buildCells("tagged.pdf", "", "", "Familie"); @@ -211,8 +210,8 @@ class MassImportServiceTest { @Test void importSingleDocument_doesNotCreateTag_whenTagCellIsBlank() { - when(documentRepository.findByOriginalFilename("notag.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("notag.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); List cells = buildCells("notag.pdf", "", "", ""); service.importSingleDocument(cells, Optional.empty(), "notag.pdf", "notag"); @@ -225,38 +224,38 @@ class MassImportServiceTest { @Test void importSingleDocument_metadataComplete_whenSenderPresent() { Person sender = Person.builder().id(UUID.randomUUID()).firstName("A").lastName("B").build(); - when(documentRepository.findByOriginalFilename("meta.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("meta.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); when(personService.findOrCreateByAlias("A B")).thenReturn(sender); List cells = buildCells("meta.pdf", "A B", "", ""); service.importSingleDocument(cells, Optional.empty(), "meta.pdf", "meta"); - verify(documentRepository).save(argThat(Document::isMetadataComplete)); + verify(documentService).save(argThat(Document::isMetadataComplete)); } @Test void importSingleDocument_metadataIncomplete_whenNoKeyFieldsPresent() { - when(documentRepository.findByOriginalFilename("nometa.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("nometa.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); List cells = buildCells("nometa.pdf", "", "", ""); service.importSingleDocument(cells, Optional.empty(), "nometa.pdf", "nometa"); - verify(documentRepository).save(argThat(d -> !d.isMetadataComplete())); + verify(documentService).save(argThat(d -> !d.isMetadataComplete())); } // ─── importSingleDocument — blank fields set to null ───────────────────── @Test void importSingleDocument_setsBlankFieldsToNull() { - when(documentRepository.findByOriginalFilename("blank.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("blank.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); List cells = buildCells("blank.pdf", "", "", ""); service.importSingleDocument(cells, Optional.empty(), "blank.pdf", "blank"); - verify(documentRepository).save(argThat(d -> + verify(documentService).save(argThat(d -> d.getLocation() == null && d.getSummary() == null && d.getTranscription() == null && @@ -281,13 +280,13 @@ class MassImportServiceTest { ); Integer result = ReflectionTestUtils.invokeMethod(service, "processRows", rows); assertThat(result).isEqualTo(0); - verify(documentRepository, never()).findByOriginalFilename(any()); + verify(documentService, never()).findByOriginalFilename(any()); } @Test void processRows_addsExtension_whenIndexHasNoDot() { - when(documentRepository.findByOriginalFilename("doc001.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("doc001.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); List> rows = List.of( List.of("header"), @@ -296,13 +295,13 @@ class MassImportServiceTest { Integer result = ReflectionTestUtils.invokeMethod(service, "processRows", rows); assertThat(result).isEqualTo(1); - verify(documentRepository).findByOriginalFilename("doc001.pdf"); + verify(documentService).findByOriginalFilename("doc001.pdf"); } @Test void processRows_usesFilenameAsIs_whenIndexHasDot() { - when(documentRepository.findByOriginalFilename("doc002.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("doc002.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); List> rows = List.of( List.of("header"), @@ -311,15 +310,15 @@ class MassImportServiceTest { Integer result = ReflectionTestUtils.invokeMethod(service, "processRows", rows); assertThat(result).isEqualTo(1); - verify(documentRepository).findByOriginalFilename("doc002.pdf"); + verify(documentService).findByOriginalFilename("doc002.pdf"); } // ─── importSingleDocument — non-blank optional fields ──────────────────── @Test void importSingleDocument_setsNonNullOptionalFields_whenPresent() { - when(documentRepository.findByOriginalFilename("rich.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("rich.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); // box=1, folder=2, location=9, summary=11, transcription=13 List cells = List.of( @@ -341,7 +340,7 @@ class MassImportServiceTest { service.importSingleDocument(cells, Optional.empty(), "rich.pdf", "rich"); - verify(documentRepository).save(argThat(d -> + verify(documentService).save(argThat(d -> "Box A".equals(d.getArchiveBox()) && "Folder B".equals(d.getArchiveFolder()) && "Hamburg".equals(d.getLocation()) && @@ -352,27 +351,27 @@ class MassImportServiceTest { @Test void importSingleDocument_setsMetadataComplete_whenReceiversArePresent() { Person receiver = Person.builder().id(UUID.randomUUID()).firstName("Walter").lastName("Müller").build(); - when(documentRepository.findByOriginalFilename("rcv.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("rcv.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); when(personService.findOrCreateByAlias("Walter Müller")).thenReturn(receiver); List cells = List.of( "rcv.pdf", "", "", "", "", "Walter Müller", "", "", "", "", "", "", "", ""); service.importSingleDocument(cells, Optional.empty(), "rcv.pdf", "rcv"); - verify(documentRepository).save(argThat(Document::isMetadataComplete)); + verify(documentService).save(argThat(Document::isMetadataComplete)); } @Test void importSingleDocument_setsMetadataComplete_whenDateIsPresent() { - when(documentRepository.findByOriginalFilename("dated.pdf")).thenReturn(Optional.empty()); - when(documentRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); + when(documentService.findByOriginalFilename("dated.pdf")).thenReturn(Optional.empty()); + when(documentService.save(any())).thenAnswer(inv -> inv.getArgument(0)); List cells = List.of( "dated.pdf", "", "", "", "", "", "", "2024-03-15", "", "", "", "", "", ""); service.importSingleDocument(cells, Optional.empty(), "dated.pdf", "dated"); - verify(documentRepository).save(argThat(Document::isMetadataComplete)); + verify(documentService).save(argThat(Document::isMetadataComplete)); } // ─── buildTitle — null location ─────────────────────────────────────────── -- 2.49.1 From 310bb5b2d5f0c80f736f3f11b1736128a98f5970 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 07:36:20 +0200 Subject: [PATCH 6/9] refactor(training-export): route export services through owning services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SegmentationTrainingExportService and TrainingDataExportService each injected TranscriptionBlockRepository, AnnotationRepository and DocumentRepository directly. They now go through: - TranscriptionBlockQueryService (extended) for the three eligible-block queries — used over TranscriptionService to keep SenderModelService → TrainingDataExportService → TranscriptionService cycle-free. - AnnotationService.findById (new) — read API on the annotation domain. - DocumentService.findById (already added in #417 commit 3). The TrainingDataExportServiceTest @DataJpaTest delegates the new service reads to the real JPA repositories via Mockito stubs in the new makeService helper, so the integration coverage stays unchanged. Refs #417 (C6.2 violations #6 and #7). Co-Authored-By: Claude Opus 4.7 --- .../service/AnnotationService.java | 5 +++ .../SegmentationTrainingExportService.java | 15 +++---- .../service/TrainingDataExportService.java | 17 ++++---- .../TranscriptionBlockQueryService.java | 13 ++++++ .../TrainingDataExportServiceTest.java | 42 ++++++++++++++----- 5 files changed, 63 insertions(+), 29 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java index 1c6da350..1484c796 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java @@ -17,6 +17,7 @@ import org.springframework.transaction.annotation.Transactional; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.UUID; @Slf4j @@ -32,6 +33,10 @@ public class AnnotationService { return annotationRepository.findByDocumentId(documentId); } + public Optional findById(UUID id) { + return annotationRepository.findById(id); + } + @Transactional public DocumentAnnotation createAnnotation(UUID documentId, CreateAnnotationDTO dto, UUID userId, String fileHash) { DocumentAnnotation annotation = DocumentAnnotation.builder() diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/SegmentationTrainingExportService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/SegmentationTrainingExportService.java index 3b2a1428..c41e2abf 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/SegmentationTrainingExportService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/SegmentationTrainingExportService.java @@ -8,9 +8,6 @@ import org.apache.pdfbox.rendering.PDFRenderer; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentAnnotation; import org.raddatz.familienarchiv.model.TranscriptionBlock; -import org.raddatz.familienarchiv.repository.AnnotationRepository; -import org.raddatz.familienarchiv.repository.DocumentRepository; -import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.springframework.stereotype.Service; import org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBody; @@ -27,13 +24,13 @@ import java.util.zip.ZipOutputStream; @Slf4j public class SegmentationTrainingExportService { - private final TranscriptionBlockRepository blockRepository; - private final AnnotationRepository annotationRepository; - private final DocumentRepository documentRepository; + private final TranscriptionBlockQueryService transcriptionBlockQueryService; + private final AnnotationService annotationService; + private final DocumentService documentService; private final FileService fileService; public List querySegmentationBlocks() { - return blockRepository.findSegmentationBlocks(); + return transcriptionBlockQueryService.findSegmentationBlocks(); } public StreamingResponseBody exportToZip() { @@ -51,14 +48,14 @@ public class SegmentationTrainingExportService { // Pre-fetch annotations keyed by id Map annotations = new HashMap<>(); for (TranscriptionBlock b : blocks) { - annotationRepository.findById(b.getAnnotationId()) + annotationService.findById(b.getAnnotationId()) .ifPresent(a -> annotations.put(a.getId(), a)); } // Pre-fetch documents keyed by id Map documents = new HashMap<>(); for (UUID docId : byDoc.keySet()) { - documentRepository.findById(docId).ifPresent(d -> documents.put(d.getId(), d)); + documentService.findById(docId).ifPresent(d -> documents.put(d.getId(), d)); } return out -> { diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/TrainingDataExportService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/TrainingDataExportService.java index 86c81053..89f69223 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/TrainingDataExportService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/TrainingDataExportService.java @@ -8,9 +8,6 @@ import org.apache.pdfbox.rendering.PDFRenderer; import org.raddatz.familienarchiv.model.Document; import org.raddatz.familienarchiv.model.DocumentAnnotation; import org.raddatz.familienarchiv.model.TranscriptionBlock; -import org.raddatz.familienarchiv.repository.AnnotationRepository; -import org.raddatz.familienarchiv.repository.DocumentRepository; -import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.springframework.stereotype.Service; import org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBody; @@ -28,13 +25,13 @@ import java.util.zip.ZipOutputStream; @Slf4j public class TrainingDataExportService { - private final TranscriptionBlockRepository blockRepository; - private final AnnotationRepository annotationRepository; - private final DocumentRepository documentRepository; + private final TranscriptionBlockQueryService transcriptionBlockQueryService; + private final AnnotationService annotationService; + private final DocumentService documentService; private final FileService fileService; public List queryEligibleBlocks() { - return blockRepository.findEligibleKurrentBlocks(); + return transcriptionBlockQueryService.findEligibleKurrentBlocks(); } public StreamingResponseBody exportToZip() { @@ -42,7 +39,7 @@ public class TrainingDataExportService { } public List queryBlocksForSender(UUID personId) { - return blockRepository.findManualKurrentBlocksByPerson(personId); + return transcriptionBlockQueryService.findManualKurrentBlocksByPerson(personId); } public StreamingResponseBody exportForSender(UUID personId) { @@ -63,14 +60,14 @@ public class TrainingDataExportService { // Pre-fetch annotations keyed by id Map annotations = new HashMap<>(); for (TranscriptionBlock b : blocks) { - annotationRepository.findById(b.getAnnotationId()) + annotationService.findById(b.getAnnotationId()) .ifPresent(a -> annotations.put(a.getId(), a)); } // Pre-fetch documents keyed by id Map documents = new HashMap<>(); for (UUID docId : byDoc.keySet()) { - documentRepository.findById(docId).ifPresent(d -> documents.put(d.getId(), d)); + documentService.findById(docId).ifPresent(d -> documents.put(d.getId(), d)); } return out -> { 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 945b45d7..591a8f83 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionBlockQueryService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionBlockQueryService.java @@ -1,6 +1,7 @@ package org.raddatz.familienarchiv.service; import lombok.RequiredArgsConstructor; +import org.raddatz.familienarchiv.model.TranscriptionBlock; import org.raddatz.familienarchiv.repository.CompletionStatsRow; import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.springframework.stereotype.Service; @@ -24,4 +25,16 @@ public class TranscriptionBlockQueryService { } return result; } + + public List findSegmentationBlocks() { + return blockRepository.findSegmentationBlocks(); + } + + public List findEligibleKurrentBlocks() { + return blockRepository.findEligibleKurrentBlocks(); + } + + public List findManualKurrentBlocksByPerson(UUID personId) { + return blockRepository.findManualKurrentBlocksByPerson(personId); + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TrainingDataExportServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TrainingDataExportServiceTest.java index cce70601..ffe60ba8 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TrainingDataExportServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TrainingDataExportServiceTest.java @@ -27,6 +27,7 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.*; @@ -60,7 +61,7 @@ class TrainingDataExportServiceTest { blockRepository.save(manualBlock(docId, annotId, "Liebe Mutter")); FileService fileService = mockFileService(); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); StreamingResponseBody body = service.exportToZip(); byte[] zipBytes = stream(body); @@ -79,7 +80,7 @@ class TrainingDataExportServiceTest { blockRepository.save(block); FileService fileService = mockFileService(); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); StreamingResponseBody body = service.exportToZip(); assertThat(zipEntryNames(stream(body))).isEmpty(); @@ -92,7 +93,7 @@ class TrainingDataExportServiceTest { blockRepository.save(manualBlock(docId, annotId, "Liebe Tante")); FileService fileService = mockFileService(); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); StreamingResponseBody body = service.exportToZip(); byte[] zipBytes = stream(body); @@ -110,7 +111,7 @@ class TrainingDataExportServiceTest { blockRepository.save(block); FileService fileService = mockFileService(); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); StreamingResponseBody body = service.exportToZip(); assertThat(zipEntryNames(stream(body))).isNotEmpty(); @@ -127,7 +128,7 @@ class TrainingDataExportServiceTest { blockRepository.save(block); FileService fileService = mockFileService(); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); StreamingResponseBody body = service.exportToZip(); assertThat(zipEntryNames(stream(body))).isEmpty(); @@ -143,7 +144,7 @@ class TrainingDataExportServiceTest { blockRepository.save(manualBlock(docId, annotId, "Zweite Zeile")); FileService fileService = mockFileService(); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); byte[] zipBytes = stream(service.exportToZip()); var names = zipEntryNames(zipBytes); @@ -160,7 +161,7 @@ class TrainingDataExportServiceTest { blockRepository.save(manualBlock(docId, annotId, expectedText)); FileService fileService = mockFileService(); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); byte[] zipBytes = stream(service.exportToZip()); String xmlContent = readZipEntry(zipBytes, ".xml"); @@ -174,7 +175,7 @@ class TrainingDataExportServiceTest { blockRepository.save(manualBlock(docId, annotId, "A & B < C > D")); FileService fileService = mockFileService(); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); byte[] zipBytes = stream(service.exportToZip()); String xmlContent = readZipEntry(zipBytes, ".xml"); @@ -196,7 +197,7 @@ class TrainingDataExportServiceTest { when(fileService.downloadFileBytes("fail.pdf")).thenThrow(new FileService.StorageFileNotFoundException("missing")); when(fileService.downloadFileBytes("ok.pdf")).thenReturn(minimalPdfBytes); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); byte[] zipBytes = stream(service.exportToZip()); var names = zipEntryNames(zipBytes); @@ -209,13 +210,34 @@ class TrainingDataExportServiceTest { @Test void queryEligibleBlocks_returnsEmpty_whenNoEnrolledDocuments() { FileService fileService = mockFileService(); - TrainingDataExportService service = new TrainingDataExportService(blockRepository, annotationRepository, documentRepository, fileService); + TrainingDataExportService service = makeService(fileService); assertThat(service.queryEligibleBlocks()).isEmpty(); } // ─── helpers ───────────────────────────────────────────────────────────── + /** + * Builds the export service with mocked owning services that transparently + * delegate every read to the real JPA repositories provided by {@code @DataJpaTest}. + * Keeps the integration test green after #417's layering refactor without + * pulling the full transcription/annotation/document service trees into scope. + */ + private TrainingDataExportService makeService(FileService fileService) { + TranscriptionBlockQueryService blockQueryService = mock(TranscriptionBlockQueryService.class); + AnnotationService annotationService = mock(AnnotationService.class); + DocumentService documentService = mock(DocumentService.class); + when(blockQueryService.findEligibleKurrentBlocks()) + .thenAnswer(inv -> blockRepository.findEligibleKurrentBlocks()); + when(blockQueryService.findManualKurrentBlocksByPerson(any(UUID.class))) + .thenAnswer(inv -> blockRepository.findManualKurrentBlocksByPerson(inv.getArgument(0))); + when(annotationService.findById(any(UUID.class))) + .thenAnswer(inv -> annotationRepository.findById(inv.getArgument(0))); + when(documentService.findById(any(UUID.class))) + .thenAnswer(inv -> documentRepository.findById(inv.getArgument(0))); + return new TrainingDataExportService(blockQueryService, annotationService, documentService, fileService); + } + private UUID enrolledDoc(String filename) { Document doc = documentRepository.save(Document.builder() .title(filename).originalFilename(filename).filePath(filename) -- 2.49.1 From f5151f394970ef7d8ee2d66313ad56705e7d7b11 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 07:40:34 +0200 Subject: [PATCH 7/9] 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 -- 2.49.1 From 2506523f3bf176fa88291b20c088288599226692 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 07:48:26 +0200 Subject: [PATCH 8/9] refactor(transcription/annotation): break mutual repo dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../service/AnnotationService.java | 20 ++++++++++++++++--- .../service/TranscriptionService.java | 13 +++++++----- .../service/AnnotationServiceTest.java | 9 ++++----- .../TranscriptionServiceGuidedTest.java | 5 +---- .../service/TranscriptionServiceTest.java | 8 +++----- 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java index 1484c796..2250e8fc 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/AnnotationService.java @@ -10,7 +10,7 @@ import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.exception.ErrorCode; import org.raddatz.familienarchiv.model.DocumentAnnotation; 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.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -26,7 +26,11 @@ import java.util.UUID; public class AnnotationService { 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; public List listAnnotations(UUID documentId) { @@ -37,6 +41,16 @@ public class AnnotationService { return annotationRepository.findById(id); } + @Transactional + public void deleteById(UUID annotationId) { + annotationRepository.deleteById(annotationId); + } + + @Transactional + public void deleteAllById(java.util.Collection annotationIds) { + annotationRepository.deleteAllById(annotationIds); + } + @Transactional public DocumentAnnotation createAnnotation(UUID documentId, CreateAnnotationDTO dto, UUID userId, String fileHash) { DocumentAnnotation annotation = DocumentAnnotation.builder() @@ -108,7 +122,7 @@ public class AnnotationService { throw DomainException.forbidden("Only the annotation author can delete it"); } - blockRepository.deleteByAnnotationId(annotationId); + transcriptionService.deleteByAnnotationId(annotationId); annotationRepository.delete(annotation); } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java index b96f1a2a..3065eefa 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/TranscriptionService.java @@ -16,7 +16,6 @@ import org.raddatz.familienarchiv.model.DocumentAnnotation; import org.raddatz.familienarchiv.model.ScriptType; import org.raddatz.familienarchiv.model.TranscriptionBlock; import org.raddatz.familienarchiv.model.TranscriptionBlockVersion; -import org.raddatz.familienarchiv.repository.AnnotationRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository; import org.springframework.stereotype.Service; @@ -37,7 +36,6 @@ public class TranscriptionService { private final TranscriptionBlockRepository blockRepository; private final TranscriptionBlockVersionRepository versionRepository; - private final AnnotationRepository annotationRepository; private final AnnotationService annotationService; private final DocumentService documentService; private final SenderModelService senderModelService; @@ -47,6 +45,11 @@ public class TranscriptionService { return blockRepository.findByDocumentIdOrderBySortOrderAsc(documentId); } + @Transactional + public void deleteByAnnotationId(UUID annotationId) { + blockRepository.deleteByAnnotationId(annotationId); + } + public TranscriptionBlock getBlock(UUID documentId, UUID blockId) { return blockRepository.findByIdAndDocumentId(blockId, documentId) .orElseThrow(() -> DomainException.notFound( @@ -142,7 +145,7 @@ public class TranscriptionService { saveVersion(saved, userId); if (!text.equals(previousText)) { - Optional annotation = annotationRepository.findById(block.getAnnotationId()); + Optional annotation = annotationService.findById(block.getAnnotationId()); int pageNumber = annotation.map(DocumentAnnotation::getPageNumber).orElse(0); auditService.logAfterCommit(AuditKind.TEXT_SAVED, userId, documentId, 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) blockRepository.delete(block); blockRepository.flush(); - annotationRepository.deleteById(annotationId); + annotationService.deleteById(annotationId); log.info("Deleted transcription block {} and annotation {} for document {}", blockId, annotationId, documentId); } @@ -181,7 +184,7 @@ public class TranscriptionService { blockRepository.deleteAll(blocks); blockRepository.flush(); - annotationRepository.deleteAllById(annotationIds); + annotationService.deleteAllById(annotationIds); log.info("Bulk-deleted {} transcription blocks for document {}", blocks.size(), documentId); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java index 19277bec..02bc6f9b 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/AnnotationServiceTest.java @@ -13,7 +13,6 @@ import org.raddatz.familienarchiv.dto.UpdateAnnotationDTO; import org.raddatz.familienarchiv.exception.DomainException; import org.raddatz.familienarchiv.model.DocumentAnnotation; import org.raddatz.familienarchiv.repository.AnnotationRepository; -import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.springframework.dao.DataIntegrityViolationException; import java.util.Map; @@ -36,7 +35,7 @@ import static org.springframework.http.HttpStatus.NOT_FOUND; class AnnotationServiceTest { @Mock AnnotationRepository annotationRepository; - @Mock TranscriptionBlockRepository blockRepository; + @Mock TranscriptionService transcriptionService; @Mock AuditService auditService; @InjectMocks AnnotationService annotationService; @@ -208,7 +207,7 @@ class AnnotationServiceTest { annotationService.deleteAnnotation(docId, annotId, ownerId); - verify(blockRepository).deleteByAnnotationId(annotId); + verify(transcriptionService).deleteByAnnotationId(annotId); verify(annotationRepository).delete(annotation); } @@ -225,8 +224,8 @@ class AnnotationServiceTest { annotationService.deleteAnnotation(docId, annotId, ownerId); - var inOrder = org.mockito.Mockito.inOrder(blockRepository, annotationRepository); - inOrder.verify(blockRepository).deleteByAnnotationId(annotId); + var inOrder = org.mockito.Mockito.inOrder(transcriptionService, annotationRepository); + inOrder.verify(transcriptionService).deleteByAnnotationId(annotId); inOrder.verify(annotationRepository).delete(annotation); } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java index 0e786d83..dc423326 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceGuidedTest.java @@ -5,7 +5,6 @@ import org.junit.jupiter.api.Test; import org.raddatz.familienarchiv.audit.AuditService; import org.raddatz.familienarchiv.model.BlockSource; import org.raddatz.familienarchiv.model.TranscriptionBlock; -import org.raddatz.familienarchiv.repository.AnnotationRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository; @@ -20,7 +19,6 @@ class TranscriptionServiceGuidedTest { TranscriptionBlockRepository blockRepository; TranscriptionBlockVersionRepository versionRepository; - AnnotationRepository annotationRepository; AnnotationService annotationService; DocumentService documentService; SenderModelService senderModelService; @@ -35,14 +33,13 @@ class TranscriptionServiceGuidedTest { void setUp() { blockRepository = mock(TranscriptionBlockRepository.class); versionRepository = mock(TranscriptionBlockVersionRepository.class); - annotationRepository = mock(AnnotationRepository.class); annotationService = mock(AnnotationService.class); documentService = mock(DocumentService.class); senderModelService = mock(SenderModelService.class); auditService = mock(AuditService.class); service = new TranscriptionService(blockRepository, versionRepository, - annotationRepository, annotationService, documentService, senderModelService, auditService); + annotationService, documentService, senderModelService, auditService); when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); when(versionRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java index 15a098c0..f2b44ae4 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TranscriptionServiceTest.java @@ -21,7 +21,6 @@ import org.raddatz.familienarchiv.model.PersonMention; import org.raddatz.familienarchiv.model.ScriptType; import org.raddatz.familienarchiv.model.TranscriptionBlock; import org.raddatz.familienarchiv.model.TranscriptionBlockVersion; -import org.raddatz.familienarchiv.repository.AnnotationRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockRepository; import org.raddatz.familienarchiv.repository.TranscriptionBlockVersionRepository; @@ -44,7 +43,6 @@ class TranscriptionServiceTest { @Mock TranscriptionBlockRepository blockRepository; @Mock TranscriptionBlockVersionRepository versionRepository; - @Mock AnnotationRepository annotationRepository; @Mock AnnotationService annotationService; @Mock DocumentService documentService; @Mock SenderModelService senderModelService; @@ -320,7 +318,7 @@ class TranscriptionServiceTest { verify(blockRepository).delete(block); verify(blockRepository).flush(); - verify(annotationRepository).deleteById(annotId); + verify(annotationService).deleteById(annotId); } @Test @@ -354,7 +352,7 @@ class TranscriptionServiceTest { verify(blockRepository).deleteAll(List.of(block1, block2)); verify(blockRepository).flush(); - verify(annotationRepository).deleteAllById(List.of(annId1, annId2)); + verify(annotationService).deleteAllById(List.of(annId1, annId2)); } @Test @@ -532,7 +530,7 @@ class TranscriptionServiceTest { when(blockRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); when(documentService.getDocumentById(any())).thenReturn( 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); -- 2.49.1 From 89e9a2452e1fcc49e4261c5489b09bf5301a119d Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 5 May 2026 10:37:06 +0200 Subject: [PATCH 9/9] refactor(test): remove issue reference from makeService javadoc Issue numbers in code comments rot as the codebase evolves. The why (keeping real-database fidelity without pulling full service trees in) is what matters, not the fix number. Co-Authored-By: Claude Sonnet 4.6 --- .../familienarchiv/service/TrainingDataExportServiceTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/src/test/java/org/raddatz/familienarchiv/service/TrainingDataExportServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/service/TrainingDataExportServiceTest.java index ffe60ba8..eca0c0e0 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/service/TrainingDataExportServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/service/TrainingDataExportServiceTest.java @@ -220,8 +220,7 @@ class TrainingDataExportServiceTest { /** * Builds the export service with mocked owning services that transparently * delegate every read to the real JPA repositories provided by {@code @DataJpaTest}. - * Keeps the integration test green after #417's layering refactor without - * pulling the full transcription/annotation/document service trees into scope. + * Keeps real-database fidelity without pulling the full service trees into scope. */ private TrainingDataExportService makeService(FileService fileService) { TranscriptionBlockQueryService blockQueryService = mock(TranscriptionBlockQueryService.class); -- 2.49.1