feat(person): paginate GET /api/persons and add confirm/delete endpoints
GET /api/persons now returns PersonSearchResult with server-side filter params
(type, familyOnly, hasDocuments, provisional) and page/size bounds (@Min/@Max
-> 400). review=true drops the clean reader default. The legacy
sort=documentCount top-N path is folded into the paged contract. Add
PATCH /{id}/confirm and DELETE /{id}, both WRITE_ALL-guarded. Remove the now
unreachable PersonService.findAll(String).
BREAKING-CHANGE: GET /api/persons response shape changes from a bare list to
PersonSearchResult { items, totalElements, pageNumber, pageSize, totalPages }.
Refs #667
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -22,12 +22,15 @@ import org.springframework.web.bind.annotation.*;
|
|||||||
import org.springframework.web.server.ResponseStatusException;
|
import org.springframework.web.server.ResponseStatusException;
|
||||||
|
|
||||||
import jakarta.validation.Valid;
|
import jakarta.validation.Valid;
|
||||||
|
import jakarta.validation.constraints.Max;
|
||||||
|
import jakarta.validation.constraints.Min;
|
||||||
|
|
||||||
import lombok.RequiredArgsConstructor;
|
import lombok.RequiredArgsConstructor;
|
||||||
|
|
||||||
@RestController
|
@RestController
|
||||||
@RequestMapping("/api/persons")
|
@RequestMapping("/api/persons")
|
||||||
@RequiredArgsConstructor
|
@RequiredArgsConstructor
|
||||||
|
@Validated
|
||||||
public class PersonController {
|
public class PersonController {
|
||||||
|
|
||||||
private final PersonService personService;
|
private final PersonService personService;
|
||||||
@@ -35,15 +38,35 @@ public class PersonController {
|
|||||||
|
|
||||||
@GetMapping
|
@GetMapping
|
||||||
@RequirePermission(Permission.READ_ALL)
|
@RequirePermission(Permission.READ_ALL)
|
||||||
public ResponseEntity<List<PersonSummaryDTO>> getPersons(
|
public ResponseEntity<PersonSearchResult> getPersons(
|
||||||
@RequestParam(required = false) String q,
|
@RequestParam(required = false) String q,
|
||||||
@RequestParam(required = false, defaultValue = "0") int size,
|
@RequestParam(required = false) PersonType type,
|
||||||
@RequestParam(required = false) String sort) {
|
@RequestParam(required = false) Boolean familyOnly,
|
||||||
if ("documentCount".equals(sort) && size > 0 && q == null) {
|
@RequestParam(required = false) Boolean hasDocuments,
|
||||||
|
@RequestParam(required = false) Boolean provisional,
|
||||||
|
// review=true reveals the import noise (transcriber view); absent/false keeps the
|
||||||
|
// clean reader default (familyMember OR documentCount > 0). The explicit filters AND
|
||||||
|
// within whichever base the review flag selects.
|
||||||
|
@RequestParam(required = false, defaultValue = "false") boolean review,
|
||||||
|
@RequestParam(required = false) String sort,
|
||||||
|
@RequestParam(defaultValue = "0") @Min(0) int page,
|
||||||
|
@RequestParam(defaultValue = "50") @Min(1) @Max(100) int size) {
|
||||||
|
// Legacy top-N-by-document-count path (reader dashboard): preserved, now wrapped in the
|
||||||
|
// paged contract so /api/persons always returns one shape.
|
||||||
|
if ("documentCount".equals(sort) && q == null) {
|
||||||
int safeSize = Math.min(size, 50);
|
int safeSize = Math.min(size, 50);
|
||||||
return ResponseEntity.ok(personService.findTopByDocumentCount(safeSize));
|
List<PersonSummaryDTO> top = personService.findTopByDocumentCount(safeSize);
|
||||||
|
return ResponseEntity.ok(PersonSearchResult.paged(top, 0, safeSize, top.size()));
|
||||||
}
|
}
|
||||||
return ResponseEntity.ok(personService.findAll(q));
|
|
||||||
|
PersonFilter filter = PersonFilter.builder()
|
||||||
|
.type(type)
|
||||||
|
.familyOnly(familyOnly)
|
||||||
|
.hasDocuments(hasDocuments)
|
||||||
|
.provisional(provisional)
|
||||||
|
.readerDefault(!review)
|
||||||
|
.build();
|
||||||
|
return ResponseEntity.ok(personService.search(filter, page, size, q));
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/{id}")
|
@GetMapping("/{id}")
|
||||||
@@ -110,6 +133,21 @@ public class PersonController {
|
|||||||
personService.mergePersons(id, UUID.fromString(targetIdStr));
|
personService.mergePersons(id, UUID.fromString(targetIdStr));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Dedicated state transition that clears the provisional flag. A separate verb (not a
|
||||||
|
// mass-assignable DTO field) so provisional can never be smuggled in via create/update.
|
||||||
|
@PatchMapping("/{id}/confirm")
|
||||||
|
@RequirePermission(Permission.WRITE_ALL)
|
||||||
|
public ResponseEntity<Person> confirmPerson(@PathVariable UUID id) {
|
||||||
|
return ResponseEntity.ok(personService.confirmPerson(id));
|
||||||
|
}
|
||||||
|
|
||||||
|
@DeleteMapping("/{id}")
|
||||||
|
@ResponseStatus(HttpStatus.NO_CONTENT)
|
||||||
|
@RequirePermission(Permission.WRITE_ALL)
|
||||||
|
public void deletePerson(@PathVariable UUID id) {
|
||||||
|
personService.deletePerson(id);
|
||||||
|
}
|
||||||
|
|
||||||
// ─── Alias endpoints ────────────────────────────────────────────────────
|
// ─── Alias endpoints ────────────────────────────────────────────────────
|
||||||
|
|
||||||
@GetMapping("/{id}/aliases")
|
@GetMapping("/{id}/aliases")
|
||||||
|
|||||||
@@ -31,16 +31,6 @@ public class PersonService {
|
|||||||
private final PersonRepository personRepository;
|
private final PersonRepository personRepository;
|
||||||
private final PersonNameAliasRepository aliasRepository;
|
private final PersonNameAliasRepository aliasRepository;
|
||||||
|
|
||||||
public List<PersonSummaryDTO> findAll(String q) {
|
|
||||||
if (q == null) {
|
|
||||||
return personRepository.findAllWithDocumentCount();
|
|
||||||
}
|
|
||||||
if (q.isBlank()) {
|
|
||||||
return List.of();
|
|
||||||
}
|
|
||||||
return personRepository.searchWithDocumentCount(q.trim());
|
|
||||||
}
|
|
||||||
|
|
||||||
public List<PersonSummaryDTO> findTopByDocumentCount(int limit) {
|
public List<PersonSummaryDTO> findTopByDocumentCount(int limit) {
|
||||||
return personRepository.findTopByDocumentCount(limit);
|
return personRepository.findTopByDocumentCount(limit);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -65,44 +65,114 @@ class PersonControllerTest {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
@WithMockUser(authorities = "READ_ALL")
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
void getPersons_returns200_withEmptyList() throws Exception {
|
void getPersons_returns200_withEmptyPagedResult() throws Exception {
|
||||||
when(personService.findAll(null)).thenReturn(Collections.emptyList());
|
when(personService.search(any(), eq(0), eq(50), eq(null)))
|
||||||
|
.thenReturn(PersonSearchResult.paged(Collections.emptyList(), 0, 50, 0));
|
||||||
mockMvc.perform(get("/api/persons"))
|
mockMvc.perform(get("/api/persons"))
|
||||||
.andExpect(status().isOk());
|
.andExpect(status().isOk())
|
||||||
|
.andExpect(jsonPath("$.items").isArray())
|
||||||
|
.andExpect(jsonPath("$.totalElements").value(0));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@WithMockUser(authorities = "READ_ALL")
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
void getPersons_delegatesQueryParam_toService() throws Exception {
|
void getPersons_delegatesQueryParam_toService() throws Exception {
|
||||||
PersonSummaryDTO dto = mockPersonSummary("Hans", "Müller");
|
PersonSummaryDTO dto = mockPersonSummary("Hans", "Müller");
|
||||||
when(personService.findAll("Hans")).thenReturn(List.of(dto));
|
when(personService.search(any(), eq(0), eq(50), eq("Hans")))
|
||||||
|
.thenReturn(PersonSearchResult.paged(List.of(dto), 0, 50, 1));
|
||||||
|
|
||||||
mockMvc.perform(get("/api/persons").param("q", "Hans"))
|
mockMvc.perform(get("/api/persons").param("q", "Hans"))
|
||||||
.andExpect(status().isOk())
|
.andExpect(status().isOk())
|
||||||
.andExpect(jsonPath("$[0].firstName").value("Hans"));
|
.andExpect(jsonPath("$.items[0].firstName").value("Hans"));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@WithMockUser(authorities = "READ_ALL")
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
void getPersons_delegatesTopByDocumentCount_whenSortAndSizeGiven() throws Exception {
|
void getPersons_passesFilterParams_toService() throws Exception {
|
||||||
|
ArgumentCaptor<PersonFilter> filterCaptor = ArgumentCaptor.forClass(PersonFilter.class);
|
||||||
|
when(personService.search(filterCaptor.capture(), eq(0), eq(50), eq(null)))
|
||||||
|
.thenReturn(PersonSearchResult.paged(Collections.emptyList(), 0, 50, 0));
|
||||||
|
|
||||||
|
mockMvc.perform(get("/api/persons")
|
||||||
|
.param("type", "INSTITUTION")
|
||||||
|
.param("familyOnly", "true")
|
||||||
|
.param("hasDocuments", "true")
|
||||||
|
.param("provisional", "false"))
|
||||||
|
.andExpect(status().isOk());
|
||||||
|
|
||||||
|
PersonFilter captured = filterCaptor.getValue();
|
||||||
|
assertThat(captured.type()).isEqualTo(PersonType.INSTITUTION);
|
||||||
|
assertThat(captured.familyOnly()).isTrue();
|
||||||
|
assertThat(captured.hasDocuments()).isTrue();
|
||||||
|
assertThat(captured.provisional()).isFalse();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
|
void getPersons_defaultsToReaderDefault_whenNoReviewFlag() throws Exception {
|
||||||
|
ArgumentCaptor<PersonFilter> filterCaptor = ArgumentCaptor.forClass(PersonFilter.class);
|
||||||
|
when(personService.search(filterCaptor.capture(), eq(0), eq(50), eq(null)))
|
||||||
|
.thenReturn(PersonSearchResult.paged(Collections.emptyList(), 0, 50, 0));
|
||||||
|
|
||||||
|
mockMvc.perform(get("/api/persons")).andExpect(status().isOk());
|
||||||
|
|
||||||
|
assertThat(filterCaptor.getValue().readerDefault()).isTrue();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
|
void getPersons_dropsReaderDefault_whenReviewFlagSet() throws Exception {
|
||||||
|
ArgumentCaptor<PersonFilter> filterCaptor = ArgumentCaptor.forClass(PersonFilter.class);
|
||||||
|
when(personService.search(filterCaptor.capture(), eq(0), eq(50), eq(null)))
|
||||||
|
.thenReturn(PersonSearchResult.paged(Collections.emptyList(), 0, 50, 0));
|
||||||
|
|
||||||
|
mockMvc.perform(get("/api/persons").param("review", "true")).andExpect(status().isOk());
|
||||||
|
|
||||||
|
assertThat(filterCaptor.getValue().readerDefault()).isFalse();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
|
void getPersons_passesPageAndSize_toService() throws Exception {
|
||||||
|
when(personService.search(any(), eq(2), eq(25), eq(null)))
|
||||||
|
.thenReturn(PersonSearchResult.paged(Collections.emptyList(), 2, 25, 0));
|
||||||
|
|
||||||
|
mockMvc.perform(get("/api/persons").param("page", "2").param("size", "25"))
|
||||||
|
.andExpect(status().isOk());
|
||||||
|
|
||||||
|
verify(personService).search(any(), eq(2), eq(25), eq(null));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
|
void getPersons_returns400_whenSizeIsZero() throws Exception {
|
||||||
|
mockMvc.perform(get("/api/persons").param("size", "0"))
|
||||||
|
.andExpect(status().isBadRequest());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
|
void getPersons_returns400_whenSizeExceeds100() throws Exception {
|
||||||
|
mockMvc.perform(get("/api/persons").param("size", "101"))
|
||||||
|
.andExpect(status().isBadRequest());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
|
void getPersons_returns400_whenPageIsNegative() throws Exception {
|
||||||
|
mockMvc.perform(get("/api/persons").param("page", "-1"))
|
||||||
|
.andExpect(status().isBadRequest());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
|
void getPersons_delegatesTopByDocumentCount_whenSortGiven() throws Exception {
|
||||||
PersonSummaryDTO top = mockPersonSummary("Käthe", "Raddatz");
|
PersonSummaryDTO top = mockPersonSummary("Käthe", "Raddatz");
|
||||||
when(personService.findTopByDocumentCount(4)).thenReturn(List.of(top));
|
when(personService.findTopByDocumentCount(4)).thenReturn(List.of(top));
|
||||||
|
|
||||||
mockMvc.perform(get("/api/persons").param("sort", "documentCount").param("size", "4"))
|
mockMvc.perform(get("/api/persons").param("sort", "documentCount").param("size", "4"))
|
||||||
.andExpect(status().isOk())
|
.andExpect(status().isOk())
|
||||||
.andExpect(jsonPath("$[0].firstName").value("Käthe"));
|
.andExpect(jsonPath("$.items[0].firstName").value("Käthe"));
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
@WithMockUser(authorities = "READ_ALL")
|
|
||||||
void getPersons_capsTopByDocumentCount_atFifty() throws Exception {
|
|
||||||
ArgumentCaptor<Integer> sizeCaptor = ArgumentCaptor.forClass(Integer.class);
|
|
||||||
when(personService.findTopByDocumentCount(sizeCaptor.capture())).thenReturn(Collections.emptyList());
|
|
||||||
|
|
||||||
mockMvc.perform(get("/api/persons").param("sort", "documentCount").param("size", "999"))
|
|
||||||
.andExpect(status().isOk());
|
|
||||||
|
|
||||||
assertThat(sizeCaptor.getValue()).isEqualTo(50);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private PersonSummaryDTO mockPersonSummary(String firstName, String lastName) {
|
private PersonSummaryDTO mockPersonSummary(String firstName, String lastName) {
|
||||||
@@ -398,6 +468,61 @@ class PersonControllerTest {
|
|||||||
.andExpect(status().isNoContent());
|
.andExpect(status().isNoContent());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── PATCH /api/persons/{id}/confirm ──────────────────────────────────────
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void confirmPerson_returns401_whenUnauthenticated() throws Exception {
|
||||||
|
mockMvc.perform(patch("/api/persons/{id}/confirm", UUID.randomUUID()).with(csrf()))
|
||||||
|
.andExpect(status().isUnauthorized());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
|
void confirmPerson_returns403_whenUserHasOnlyReadPermission() throws Exception {
|
||||||
|
mockMvc.perform(patch("/api/persons/{id}/confirm", UUID.randomUUID()).with(csrf()))
|
||||||
|
.andExpect(status().isForbidden());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "WRITE_ALL")
|
||||||
|
void confirmPerson_returns200_andClearsProvisional() throws Exception {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
Person confirmed = Person.builder().id(id).firstName("Bald").lastName("Bestaetigt").provisional(false).build();
|
||||||
|
when(personService.confirmPerson(id)).thenReturn(confirmed);
|
||||||
|
|
||||||
|
mockMvc.perform(patch("/api/persons/{id}/confirm", id).with(csrf()))
|
||||||
|
.andExpect(status().isOk())
|
||||||
|
.andExpect(jsonPath("$.provisional").value(false));
|
||||||
|
|
||||||
|
verify(personService).confirmPerson(id);
|
||||||
|
}
|
||||||
|
|
||||||
|
// ─── DELETE /api/persons/{id} ──────────────────────────────────────────────
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void deletePerson_returns401_whenUnauthenticated() throws Exception {
|
||||||
|
mockMvc.perform(delete("/api/persons/{id}", UUID.randomUUID()).with(csrf()))
|
||||||
|
.andExpect(status().isUnauthorized());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "READ_ALL")
|
||||||
|
void deletePerson_returns403_whenUserHasOnlyReadPermission() throws Exception {
|
||||||
|
mockMvc.perform(delete("/api/persons/{id}", UUID.randomUUID()).with(csrf()))
|
||||||
|
.andExpect(status().isForbidden());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@WithMockUser(authorities = "WRITE_ALL")
|
||||||
|
void deletePerson_returns204_whenValid() throws Exception {
|
||||||
|
UUID id = UUID.randomUUID();
|
||||||
|
|
||||||
|
mockMvc.perform(delete("/api/persons/{id}", id).with(csrf()))
|
||||||
|
.andExpect(status().isNoContent());
|
||||||
|
|
||||||
|
verify(personService).deletePerson(id);
|
||||||
|
}
|
||||||
|
|
||||||
// ─── PUT /api/persons/{id} — lastName blank branch ────────────────────────
|
// ─── PUT /api/persons/{id} — lastName blank branch ────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -58,35 +58,6 @@ class PersonServiceTest {
|
|||||||
assertThat(personService.getById(id)).isEqualTo(person);
|
assertThat(personService.getById(id)).isEqualTo(person);
|
||||||
}
|
}
|
||||||
|
|
||||||
// ─── findAll ─────────────────────────────────────────────────────────────
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void findAll_returnsAll_whenQueryIsNull() {
|
|
||||||
List<PersonSummaryDTO> expected = List.of();
|
|
||||||
when(personRepository.findAllWithDocumentCount()).thenReturn(expected);
|
|
||||||
|
|
||||||
assertThat(personService.findAll(null)).isEqualTo(expected);
|
|
||||||
verify(personRepository).findAllWithDocumentCount();
|
|
||||||
verify(personRepository, never()).searchWithDocumentCount(any());
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void findAll_returnsEmpty_whenQueryIsWhitespaceOnly() {
|
|
||||||
assertThat(personService.findAll(" ")).isEmpty();
|
|
||||||
verify(personRepository, never()).findAllWithDocumentCount();
|
|
||||||
verify(personRepository, never()).searchWithDocumentCount(any());
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void findAll_searchesByName_whenQueryIsNonBlank() {
|
|
||||||
List<PersonSummaryDTO> expected = List.of();
|
|
||||||
when(personRepository.searchWithDocumentCount("Anna")).thenReturn(expected);
|
|
||||||
|
|
||||||
assertThat(personService.findAll("Anna")).isEqualTo(expected);
|
|
||||||
verify(personRepository).searchWithDocumentCount("Anna");
|
|
||||||
verify(personRepository, never()).findAllWithDocumentCount();
|
|
||||||
}
|
|
||||||
|
|
||||||
// ─── #667: search (filter + pagination) ──────────────────────────────────
|
// ─── #667: search (filter + pagination) ──────────────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
Reference in New Issue
Block a user