From d5e0e969ef0a200c86fca461be5c9e0512732a4e Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 4 May 2026 22:20:14 +0200 Subject: [PATCH] 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(); + } +}