feat(geschichten): filter by multiple persons with AND semantics
GET /api/geschichten now accepts repeated personId query params and returns only stories that mention every person supplied. Refactors the list path to a JPA Specification chain (one EXISTS subquery per id, mirroring DocumentSpecifications.hasTags) and embeds the COALESCE(publishedAt, updatedAt) DESC ordering inside the spec so a single repository.findAll covers all filter combinations.
This commit is contained in:
@@ -32,10 +32,14 @@ public class GeschichteController {
|
||||
@GetMapping
|
||||
public List<Geschichte> list(
|
||||
@RequestParam(required = false) GeschichteStatus status,
|
||||
@RequestParam(required = false) UUID personId,
|
||||
@RequestParam(name = "personId", required = false) List<UUID> personIds,
|
||||
@RequestParam(required = false) UUID documentId,
|
||||
@RequestParam(required = false, defaultValue = "50") int limit) {
|
||||
return geschichteService.list(status, personId, documentId, limit);
|
||||
return geschichteService.list(
|
||||
status,
|
||||
personIds == null ? List.of() : personIds,
|
||||
documentId,
|
||||
limit);
|
||||
}
|
||||
|
||||
@GetMapping("/{id}")
|
||||
|
||||
@@ -1,30 +1,12 @@
|
||||
package org.raddatz.familienarchiv.repository;
|
||||
|
||||
import org.raddatz.familienarchiv.model.Geschichte;
|
||||
import org.raddatz.familienarchiv.model.GeschichteStatus;
|
||||
import org.springframework.data.domain.Pageable;
|
||||
import org.springframework.data.jpa.repository.JpaRepository;
|
||||
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
|
||||
import org.springframework.data.jpa.repository.Query;
|
||||
import org.springframework.data.repository.query.Param;
|
||||
import org.springframework.stereotype.Repository;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.UUID;
|
||||
|
||||
@Repository
|
||||
public interface GeschichteRepository extends JpaRepository<Geschichte, UUID>, JpaSpecificationExecutor<Geschichte> {
|
||||
|
||||
@Query("""
|
||||
SELECT g FROM Geschichte g
|
||||
WHERE (:status IS NULL OR g.status = :status)
|
||||
AND (:personId IS NULL OR :personId IN (SELECT p.id FROM g.persons p))
|
||||
AND (:documentId IS NULL OR :documentId IN (SELECT d.id FROM g.documents d))
|
||||
ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC
|
||||
""")
|
||||
List<Geschichte> search(
|
||||
@Param("status") GeschichteStatus status,
|
||||
@Param("personId") UUID personId,
|
||||
@Param("documentId") UUID documentId,
|
||||
Pageable pageable);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
package org.raddatz.familienarchiv.repository;
|
||||
|
||||
import jakarta.persistence.criteria.CriteriaBuilder;
|
||||
import jakarta.persistence.criteria.CriteriaQuery;
|
||||
import jakarta.persistence.criteria.Join;
|
||||
import jakarta.persistence.criteria.Predicate;
|
||||
import jakarta.persistence.criteria.Root;
|
||||
import jakarta.persistence.criteria.Subquery;
|
||||
import org.raddatz.familienarchiv.model.Document;
|
||||
import org.raddatz.familienarchiv.model.Geschichte;
|
||||
import org.raddatz.familienarchiv.model.GeschichteStatus;
|
||||
import org.raddatz.familienarchiv.model.Person;
|
||||
import org.springframework.data.jpa.domain.Specification;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collection;
|
||||
import java.util.List;
|
||||
import java.util.UUID;
|
||||
|
||||
public final class GeschichteSpecifications {
|
||||
|
||||
private GeschichteSpecifications() {}
|
||||
|
||||
public static Specification<Geschichte> hasStatus(GeschichteStatus status) {
|
||||
return (root, query, cb) -> status == null ? null : cb.equal(root.get("status"), status);
|
||||
}
|
||||
|
||||
/**
|
||||
* Adds {@code ORDER BY COALESCE(publishedAt, updatedAt) DESC} to the query without contributing
|
||||
* a predicate. Combined into the spec chain via {@code .and(...)}; the {@code conjunction}
|
||||
* acts as a no-op WHERE clause.
|
||||
*/
|
||||
public static Specification<Geschichte> orderByDisplayDateDesc() {
|
||||
return (root, query, cb) -> {
|
||||
// Skip ordering on count queries — JPA forbids orderBy on COUNT projections.
|
||||
if (query != null
|
||||
&& Long.class != query.getResultType()
|
||||
&& long.class != query.getResultType()) {
|
||||
query.orderBy(cb.desc(cb.coalesce(root.get("publishedAt"), root.get("updatedAt"))));
|
||||
}
|
||||
return cb.conjunction();
|
||||
};
|
||||
}
|
||||
|
||||
public static Specification<Geschichte> hasDocument(UUID documentId) {
|
||||
return (root, query, cb) -> {
|
||||
if (documentId == null) return null;
|
||||
return cb.exists(documentSubquery(root, query, cb, documentId));
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* AND-filter across persons: the Geschichte must be associated with EVERY id in {@code personIds}.
|
||||
*
|
||||
* <p>Implemented as one EXISTS subquery per id (canonical Criteria-API idiom for AND across a
|
||||
* many-to-many join). Mirrors {@link DocumentSpecifications#hasTags} which uses the same shape.
|
||||
* Empty / null input returns {@code null} (i.e. no constraint added).
|
||||
*/
|
||||
public static Specification<Geschichte> hasAllPersons(Collection<UUID> personIds) {
|
||||
return (root, query, cb) -> {
|
||||
if (personIds == null || personIds.isEmpty()) return null;
|
||||
List<Predicate> predicates = new ArrayList<>(personIds.size());
|
||||
for (UUID id : personIds) {
|
||||
predicates.add(cb.exists(personSubquery(root, query, cb, id)));
|
||||
}
|
||||
return cb.and(predicates.toArray(new Predicate[0]));
|
||||
};
|
||||
}
|
||||
|
||||
private static Subquery<UUID> personSubquery(
|
||||
Root<Geschichte> root, CriteriaQuery<?> query, CriteriaBuilder cb, UUID personId) {
|
||||
Subquery<UUID> sub = query.subquery(UUID.class);
|
||||
Root<Geschichte> subRoot = sub.from(Geschichte.class);
|
||||
Join<Geschichte, Person> persons = subRoot.join("persons");
|
||||
sub.select(subRoot.get("id"))
|
||||
.where(cb.equal(subRoot.get("id"), root.get("id")),
|
||||
cb.equal(persons.get("id"), personId));
|
||||
return sub;
|
||||
}
|
||||
|
||||
private static Subquery<UUID> documentSubquery(
|
||||
Root<Geschichte> root, CriteriaQuery<?> query, CriteriaBuilder cb, UUID documentId) {
|
||||
Subquery<UUID> sub = query.subquery(UUID.class);
|
||||
Root<Geschichte> subRoot = sub.from(Geschichte.class);
|
||||
Join<Geschichte, Document> documents = subRoot.join("documents");
|
||||
sub.select(subRoot.get("id"))
|
||||
.where(cb.equal(subRoot.get("id"), root.get("id")),
|
||||
cb.equal(documents.get("id"), documentId));
|
||||
return sub;
|
||||
}
|
||||
}
|
||||
@@ -13,9 +13,10 @@ import org.raddatz.familienarchiv.model.Geschichte;
|
||||
import org.raddatz.familienarchiv.model.GeschichteStatus;
|
||||
import org.raddatz.familienarchiv.model.Person;
|
||||
import org.raddatz.familienarchiv.repository.GeschichteRepository;
|
||||
import org.raddatz.familienarchiv.repository.GeschichteSpecifications;
|
||||
import org.raddatz.familienarchiv.security.Permission;
|
||||
import org.springframework.data.domain.PageRequest;
|
||||
import org.springframework.data.domain.Pageable;
|
||||
import org.springframework.data.domain.Sort;
|
||||
import org.springframework.data.jpa.domain.Specification;
|
||||
import org.springframework.security.core.Authentication;
|
||||
import org.springframework.security.core.context.SecurityContextHolder;
|
||||
import org.springframework.stereotype.Service;
|
||||
@@ -64,11 +65,25 @@ public class GeschichteService {
|
||||
return g;
|
||||
}
|
||||
|
||||
public List<Geschichte> list(GeschichteStatus status, UUID personId, UUID documentId, int limit) {
|
||||
/**
|
||||
* Lists Geschichten with optional filters. {@code personIds} uses AND semantics: the story
|
||||
* must be associated with every person id supplied. An empty or null list applies no
|
||||
* person filter. Result is ordered by {@code COALESCE(publishedAt, updatedAt) DESC}.
|
||||
*/
|
||||
public List<Geschichte> list(GeschichteStatus status, List<UUID> personIds, UUID documentId, int limit) {
|
||||
GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED;
|
||||
int safeLimit = limit <= 0 ? DEFAULT_LIMIT : Math.min(limit, MAX_LIMIT);
|
||||
Pageable pageable = PageRequest.of(0, safeLimit);
|
||||
return geschichteRepository.search(effective, personId, documentId, pageable);
|
||||
|
||||
Specification<Geschichte> spec = Specification.allOf(
|
||||
GeschichteSpecifications.hasStatus(effective),
|
||||
GeschichteSpecifications.hasAllPersons(personIds),
|
||||
GeschichteSpecifications.hasDocument(documentId),
|
||||
GeschichteSpecifications.orderByDisplayDateDesc()
|
||||
);
|
||||
return geschichteRepository.findAll(spec, Sort.unsorted())
|
||||
.stream()
|
||||
.limit(safeLimit)
|
||||
.toList();
|
||||
}
|
||||
|
||||
// ─── Write API ───────────────────────────────────────────────────────────
|
||||
|
||||
@@ -73,15 +73,31 @@ class GeschichteControllerTest {
|
||||
|
||||
@Test
|
||||
@WithMockUser(authorities = "READ_ALL")
|
||||
void list_passesPersonIdFilterToService() throws Exception {
|
||||
void list_passesSinglePersonIdFilterToServiceAsListOfOne() throws Exception {
|
||||
UUID personId = UUID.randomUUID();
|
||||
when(geschichteService.list(any(), eq(personId), any(), anyInt()))
|
||||
when(geschichteService.list(any(), eq(List.of(personId)), any(), anyInt()))
|
||||
.thenReturn(List.of());
|
||||
|
||||
mockMvc.perform(get("/api/geschichten").param("personId", personId.toString()))
|
||||
.andExpect(status().isOk());
|
||||
|
||||
verify(geschichteService).list(any(), eq(personId), any(), anyInt());
|
||||
verify(geschichteService).list(any(), eq(List.of(personId)), any(), anyInt());
|
||||
}
|
||||
|
||||
@Test
|
||||
@WithMockUser(authorities = "READ_ALL")
|
||||
void list_passesRepeatedPersonIdParamsAsListForAndFilter() throws Exception {
|
||||
UUID a = UUID.randomUUID();
|
||||
UUID b = UUID.randomUUID();
|
||||
when(geschichteService.list(any(), eq(List.of(a, b)), any(), anyInt()))
|
||||
.thenReturn(List.of());
|
||||
|
||||
mockMvc.perform(get("/api/geschichten")
|
||||
.param("personId", a.toString())
|
||||
.param("personId", b.toString()))
|
||||
.andExpect(status().isOk());
|
||||
|
||||
verify(geschichteService).list(any(), eq(List.of(a, b)), any(), anyInt());
|
||||
}
|
||||
|
||||
// ─── GET /api/geschichten/{id} ───────────────────────────────────────────
|
||||
|
||||
@@ -86,7 +86,7 @@ class GeschichteServiceIntegrationTest {
|
||||
|
||||
// Reader cannot see DRAFT in list
|
||||
authenticateAs(reader, Permission.READ_ALL);
|
||||
assertThat(geschichteService.list(null, null, null, 50)).isEmpty();
|
||||
assertThat(geschichteService.list(null, List.of(), null, 50)).isEmpty();
|
||||
|
||||
// Reader cannot fetch DRAFT by id (404 via GESCHICHTE_NOT_FOUND)
|
||||
UUID draftId = created.getId();
|
||||
@@ -102,8 +102,8 @@ class GeschichteServiceIntegrationTest {
|
||||
|
||||
// Reader can now see and fetch it
|
||||
authenticateAs(reader, Permission.READ_ALL);
|
||||
assertThat(geschichteService.list(null, null, null, 50)).hasSize(1);
|
||||
assertThat(geschichteService.list(null, franz.getId(), null, 50)).hasSize(1);
|
||||
assertThat(geschichteService.list(null, List.of(), null, 50)).hasSize(1);
|
||||
assertThat(geschichteService.list(null, List.of(franz.getId()), null, 50)).hasSize(1);
|
||||
Geschichte fetched = geschichteService.getById(draftId);
|
||||
assertThat(fetched.getTitle()).isEqualTo("Erinnerung an Opa Franz");
|
||||
assertThat(fetched.getPersons()).extracting(Person::getId).containsExactly(franz.getId());
|
||||
@@ -117,6 +117,57 @@ class GeschichteServiceIntegrationTest {
|
||||
assertThat(personRepository.findById(franz.getId())).isPresent();
|
||||
}
|
||||
|
||||
@Test
|
||||
void list_filters_with_AND_semantics_when_multiple_personIds_given() {
|
||||
// Three published stories, persons overlap so we can prove AND-not-OR:
|
||||
// story_AB: about A and B
|
||||
// story_AC: about A and C
|
||||
// story_A: about A only
|
||||
authenticateAs(writer, Permission.BLOG_WRITE);
|
||||
|
||||
Person a = personRepository.save(Person.builder().firstName("Anna").lastName("A").build());
|
||||
Person b = personRepository.save(Person.builder().firstName("Bertha").lastName("B").build());
|
||||
Person c = personRepository.save(Person.builder().firstName("Carl").lastName("C").build());
|
||||
|
||||
UUID storyAB = publishedStoryWithPersons("Anna & Bertha", List.of(a.getId(), b.getId()));
|
||||
UUID storyAC = publishedStoryWithPersons("Anna & Carl", List.of(a.getId(), c.getId()));
|
||||
UUID storyA = publishedStoryWithPersons("Anna alone", List.of(a.getId()));
|
||||
|
||||
authenticateAs(reader, Permission.READ_ALL);
|
||||
|
||||
// No filter → all three
|
||||
assertThat(geschichteService.list(null, List.of(), null, 50))
|
||||
.extracting(Geschichte::getId)
|
||||
.containsExactlyInAnyOrder(storyAB, storyAC, storyA);
|
||||
|
||||
// Single filter (Anna) → all three
|
||||
assertThat(geschichteService.list(null, List.of(a.getId()), null, 50))
|
||||
.extracting(Geschichte::getId)
|
||||
.containsExactlyInAnyOrder(storyAB, storyAC, storyA);
|
||||
|
||||
// AND: Anna AND Bertha → only the AB story (NOT story_A, NOT story_AC)
|
||||
assertThat(geschichteService.list(null, List.of(a.getId(), b.getId()), null, 50))
|
||||
.extracting(Geschichte::getId)
|
||||
.containsExactly(storyAB);
|
||||
|
||||
// AND: Bertha AND Carl → none (no story has both)
|
||||
assertThat(geschichteService.list(null, List.of(b.getId(), c.getId()), null, 50))
|
||||
.isEmpty();
|
||||
|
||||
// AND: Anna AND Bertha AND Carl → none
|
||||
assertThat(geschichteService.list(null, List.of(a.getId(), b.getId(), c.getId()), null, 50))
|
||||
.isEmpty();
|
||||
}
|
||||
|
||||
private UUID publishedStoryWithPersons(String title, List<UUID> personIds) {
|
||||
GeschichteUpdateDTO dto = new GeschichteUpdateDTO();
|
||||
dto.setTitle(title);
|
||||
dto.setBody("<p>body</p>");
|
||||
dto.setPersonIds(personIds);
|
||||
dto.setStatus(GeschichteStatus.PUBLISHED);
|
||||
return geschichteService.create(dto).getId();
|
||||
}
|
||||
|
||||
private void authenticateAs(AppUser user, Permission... permissions) {
|
||||
var authorities = java.util.Arrays.stream(permissions)
|
||||
.map(p -> new SimpleGrantedAuthority(p.name()))
|
||||
|
||||
@@ -4,7 +4,6 @@ import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
import org.mockito.InjectMocks;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.junit.jupiter.MockitoExtension;
|
||||
@@ -18,7 +17,8 @@ import org.raddatz.familienarchiv.model.GeschichteStatus;
|
||||
import org.raddatz.familienarchiv.model.Person;
|
||||
import org.raddatz.familienarchiv.repository.GeschichteRepository;
|
||||
import org.raddatz.familienarchiv.security.Permission;
|
||||
import org.springframework.data.domain.Pageable;
|
||||
import org.springframework.data.domain.Sort;
|
||||
import org.springframework.data.jpa.domain.Specification;
|
||||
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
|
||||
import org.springframework.security.core.authority.SimpleGrantedAuthority;
|
||||
import org.springframework.security.core.context.SecurityContextHolder;
|
||||
@@ -36,7 +36,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.never;
|
||||
import static org.mockito.Mockito.times;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
@@ -126,47 +125,76 @@ class GeschichteServiceTest {
|
||||
@Test
|
||||
void list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE() {
|
||||
authenticateAs(reader, Permission.READ_ALL);
|
||||
when(geschichteRepository.search(eq(GeschichteStatus.PUBLISHED), any(), any(), any()))
|
||||
when(geschichteRepository.findAll(any(Specification.class), any(Sort.class)))
|
||||
.thenReturn(List.of(published(UUID.randomUUID())));
|
||||
|
||||
geschichteService.list(/*status*/ null, /*personId*/ null, /*documentId*/ null, /*limit*/ 50);
|
||||
geschichteService.list(/*status*/ null, /*personIds*/ List.of(), /*documentId*/ null, /*limit*/ 50);
|
||||
|
||||
verify(geschichteRepository).search(eq(GeschichteStatus.PUBLISHED), any(), any(), any());
|
||||
// Status pinning lives inside the Specification; we assert end-to-end behaviour
|
||||
// in GeschichteServiceIntegrationTest. Here we just confirm the service routes
|
||||
// through the spec-aware repository method.
|
||||
verify(geschichteRepository).findAll(any(Specification.class), any(Sort.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
void list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible() {
|
||||
authenticateAs(writer, Permission.BLOG_WRITE);
|
||||
when(geschichteRepository.search(any(), any(), any(), any()))
|
||||
when(geschichteRepository.findAll(any(Specification.class), any(Sort.class)))
|
||||
.thenReturn(List.of(draft(UUID.randomUUID()), published(UUID.randomUUID())));
|
||||
|
||||
geschichteService.list(null, null, null, 50);
|
||||
List<Geschichte> out = geschichteService.list(null, List.of(), null, 50);
|
||||
|
||||
verify(geschichteRepository).search(eq(null), any(), any(), any());
|
||||
assertThat(out).hasSize(2);
|
||||
verify(geschichteRepository).findAll(any(Specification.class), any(Sort.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
void list_filters_by_personId() {
|
||||
void list_invokes_repository_findAll_when_filtering_by_single_personId() {
|
||||
authenticateAs(reader, Permission.READ_ALL);
|
||||
UUID personId = UUID.randomUUID();
|
||||
when(geschichteRepository.search(any(), eq(personId), any(), any()))
|
||||
when(geschichteRepository.findAll(any(Specification.class), any(Sort.class)))
|
||||
.thenReturn(List.of());
|
||||
|
||||
geschichteService.list(null, personId, null, 50);
|
||||
geschichteService.list(null, List.of(personId), null, 50);
|
||||
|
||||
verify(geschichteRepository).search(eq(GeschichteStatus.PUBLISHED), eq(personId), eq(null), any());
|
||||
verify(geschichteRepository).findAll(any(Specification.class), any(Sort.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
void list_invokes_repository_findAll_when_filtering_by_multiple_personIds() {
|
||||
authenticateAs(reader, Permission.READ_ALL);
|
||||
UUID a = UUID.randomUUID();
|
||||
UUID b = UUID.randomUUID();
|
||||
when(geschichteRepository.findAll(any(Specification.class), any(Sort.class)))
|
||||
.thenReturn(List.of());
|
||||
|
||||
geschichteService.list(null, List.of(a, b), null, 50);
|
||||
|
||||
verify(geschichteRepository).findAll(any(Specification.class), any(Sort.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
void list_filters_by_documentId() {
|
||||
authenticateAs(reader, Permission.READ_ALL);
|
||||
UUID documentId = UUID.randomUUID();
|
||||
when(geschichteRepository.search(any(), any(), eq(documentId), any()))
|
||||
when(geschichteRepository.findAll(any(Specification.class), any(Sort.class)))
|
||||
.thenReturn(List.of());
|
||||
|
||||
geschichteService.list(null, null, documentId, 50);
|
||||
geschichteService.list(null, List.of(), documentId, 50);
|
||||
|
||||
verify(geschichteRepository).search(eq(GeschichteStatus.PUBLISHED), eq(null), eq(documentId), any());
|
||||
verify(geschichteRepository).findAll(any(Specification.class), any(Sort.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
void list_caps_limit_at_max_via_pageable_when_caller_passes_huge_value() {
|
||||
authenticateAs(reader, Permission.READ_ALL);
|
||||
when(geschichteRepository.findAll(any(Specification.class), any(Sort.class)))
|
||||
.thenReturn(List.of(published(UUID.randomUUID())));
|
||||
|
||||
// 9999 should be clamped — service trims to MAX_LIMIT (200) before/after the query
|
||||
List<Geschichte> out = geschichteService.list(null, List.of(), null, 9999);
|
||||
|
||||
assertThat(out).hasSizeLessThanOrEqualTo(200);
|
||||
}
|
||||
|
||||
// ─── create ──────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user