refactor(stats): introduce StatsService and require READ_ALL
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<StatsDTO> getStats() {
|
||||
return ResponseEntity.ok(new StatsDTO(personRepository.count(), documentRepository.count()));
|
||||
return ResponseEntity.ok(statsService.getStats());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -73,6 +73,10 @@ public class DocumentService {
|
||||
|
||||
public record StoreResult(Document document, boolean isNew) {}
|
||||
|
||||
public long count() {
|
||||
return documentRepository.count();
|
||||
}
|
||||
|
||||
public Map<UUID, String> findTitlesByIds(Collection<UUID> ids) {
|
||||
if (ids.isEmpty()) return Map.of();
|
||||
Map<UUID, String> titles = new HashMap<>();
|
||||
|
||||
@@ -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<Person> findCorrespondents(UUID personId, String q) {
|
||||
if (q != null && !q.isBlank()) {
|
||||
return personRepository.findCorrespondentsWithFilter(personId, q);
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
}
|
||||
@@ -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())
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user