fix(geschichten): restore documentId filter via journey_items EXISTS subquery

The PR removed the documentId filter from list() along with the old
Geschichte.documents ManyToMany, but the document-detail page and its
frontend server still query GET /api/geschichten?documentId=<id> to show
related stories. Without the filter the endpoint silently returned every
published story. Restores the filter through a JPQL EXISTS check on
journey_items so only journeys that include the given document are returned.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-06-09 17:18:28 +02:00
parent 353945c952
commit 949421a076
7 changed files with 58 additions and 39 deletions

View File

@@ -37,10 +37,12 @@ public class GeschichteController {
public List<GeschichteSummary> list( public List<GeschichteSummary> list(
@RequestParam(required = false) GeschichteStatus status, @RequestParam(required = false) GeschichteStatus status,
@RequestParam(name = "personId", required = false) List<UUID> personIds, @RequestParam(name = "personId", required = false) List<UUID> personIds,
@RequestParam(required = false) UUID documentId,
@RequestParam(required = false, defaultValue = "50") int limit) { @RequestParam(required = false, defaultValue = "50") int limit) {
return geschichteService.list( return geschichteService.list(
status, status,
personIds == null ? List.of() : personIds, personIds == null ? List.of() : personIds,
documentId,
limit); limit);
} }

View File

@@ -33,11 +33,15 @@ public interface GeschichteRepository extends JpaRepository<Geschichte, UUID>, J
(SELECT COUNT(DISTINCT p.id) (SELECT COUNT(DISTINCT p.id)
FROM Geschichte g2 JOIN g2.persons p FROM Geschichte g2 JOIN g2.persons p
WHERE g2.id = g.id AND p.id IN :personIds) = :personCount) WHERE g2.id = g.id AND p.id IN :personIds) = :personCount)
AND (:documentId IS NULL OR
EXISTS (SELECT 1 FROM JourneyItem ji
WHERE ji.geschichte = g AND ji.document.id = :documentId))
ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC ORDER BY COALESCE(g.publishedAt, g.updatedAt) DESC
""") """)
List<GeschichteSummary> findSummaries( List<GeschichteSummary> findSummaries(
@Param("effectiveStatus") GeschichteStatus effectiveStatus, @Param("effectiveStatus") GeschichteStatus effectiveStatus,
@Param("authorId") UUID authorId, @Param("authorId") UUID authorId,
@Param("personIds") Collection<UUID> personIds, @Param("personIds") Collection<UUID> personIds,
@Param("personCount") long personCount); @Param("personCount") long personCount,
@Param("documentId") UUID documentId);
} }

View File

@@ -105,7 +105,7 @@ public class GeschichteService {
* <p>Returns a {@link GeschichteSummary} projection — never carries items, preventing * <p>Returns a {@link GeschichteSummary} projection — never carries items, preventing
* LazyInitializationException on the non-transactional list path. * LazyInitializationException on the non-transactional list path.
*/ */
public List<GeschichteSummary> list(GeschichteStatus status, List<UUID> personIds, int limit) { public List<GeschichteSummary> list(GeschichteStatus status, List<UUID> personIds, UUID documentId, int limit) {
GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED; GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED;
int safeLimit = limit <= 0 ? DEFAULT_LIMIT : Math.min(limit, MAX_LIMIT); int safeLimit = limit <= 0 ? DEFAULT_LIMIT : Math.min(limit, MAX_LIMIT);
@@ -119,7 +119,7 @@ public class GeschichteService {
long personCount = (personIds == null) ? 0 : personIds.size(); long personCount = (personIds == null) ? 0 : personIds.size();
return geschichteRepository return geschichteRepository
.findSummaries(effective, authorId, safePersonIds, personCount) .findSummaries(effective, authorId, safePersonIds, personCount, documentId)
.stream() .stream()
.limit(safeLimit) .limit(safeLimit)
.toList(); .toList();

View File

@@ -63,7 +63,7 @@ class GeschichteControllerTest {
@Test @Test
@WithMockUser(authorities = "READ_ALL") @WithMockUser(authorities = "READ_ALL")
void list_returns200_forReader() throws Exception { void list_returns200_forReader() throws Exception {
when(geschichteService.list(any(), any(), anyInt())) when(geschichteService.list(any(), any(), any(), anyInt()))
.thenReturn(List.of(summaryStub("Story A"))); .thenReturn(List.of(summaryStub("Story A")));
mockMvc.perform(get("/api/geschichten")) mockMvc.perform(get("/api/geschichten"))
@@ -75,13 +75,13 @@ class GeschichteControllerTest {
@WithMockUser(authorities = "READ_ALL") @WithMockUser(authorities = "READ_ALL")
void list_passesSinglePersonIdFilterToServiceAsListOfOne() throws Exception { void list_passesSinglePersonIdFilterToServiceAsListOfOne() throws Exception {
UUID personId = UUID.randomUUID(); UUID personId = UUID.randomUUID();
when(geschichteService.list(any(), eq(List.of(personId)), anyInt())) when(geschichteService.list(any(), eq(List.of(personId)), any(), anyInt()))
.thenReturn(List.of()); .thenReturn(List.of());
mockMvc.perform(get("/api/geschichten").param("personId", personId.toString())) mockMvc.perform(get("/api/geschichten").param("personId", personId.toString()))
.andExpect(status().isOk()); .andExpect(status().isOk());
verify(geschichteService).list(any(), eq(List.of(personId)), anyInt()); verify(geschichteService).list(any(), eq(List.of(personId)), any(), anyInt());
} }
@Test @Test
@@ -89,7 +89,7 @@ class GeschichteControllerTest {
void list_passesRepeatedPersonIdParamsAsListForAndFilter() throws Exception { void list_passesRepeatedPersonIdParamsAsListForAndFilter() throws Exception {
UUID a = UUID.randomUUID(); UUID a = UUID.randomUUID();
UUID b = UUID.randomUUID(); UUID b = UUID.randomUUID();
when(geschichteService.list(any(), eq(List.of(a, b)), anyInt())) when(geschichteService.list(any(), eq(List.of(a, b)), any(), anyInt()))
.thenReturn(List.of()); .thenReturn(List.of());
mockMvc.perform(get("/api/geschichten") mockMvc.perform(get("/api/geschichten")
@@ -97,7 +97,7 @@ class GeschichteControllerTest {
.param("personId", b.toString())) .param("personId", b.toString()))
.andExpect(status().isOk()); .andExpect(status().isOk());
verify(geschichteService).list(any(), eq(List.of(a, b)), anyInt()); verify(geschichteService).list(any(), eq(List.of(a, b)), any(), anyInt());
} }
// ─── GET /api/geschichten/{id} ─────────────────────────────────────────── // ─── GET /api/geschichten/{id} ───────────────────────────────────────────

View File

@@ -48,7 +48,7 @@ class GeschichteListProjectionTest {
geschichteRepository.save(draft("Entwurf", author)); geschichteRepository.save(draft("Entwurf", author));
List<GeschichteSummary> result = geschichteRepository.findSummaries( List<GeschichteSummary> result = geschichteRepository.findSummaries(
GeschichteStatus.PUBLISHED, null, sentinel(), 0); GeschichteStatus.PUBLISHED, null, sentinel(), 0, null);
assertThat(result).hasSize(1); assertThat(result).hasSize(1);
assertThat(result.get(0).getTitle()).isEqualTo("Veröffentlicht"); assertThat(result.get(0).getTitle()).isEqualTo("Veröffentlicht");
@@ -59,7 +59,7 @@ class GeschichteListProjectionTest {
geschichteRepository.save(draft("Nur Entwurf", author)); geschichteRepository.save(draft("Nur Entwurf", author));
List<GeschichteSummary> result = geschichteRepository.findSummaries( List<GeschichteSummary> result = geschichteRepository.findSummaries(
GeschichteStatus.PUBLISHED, null, sentinel(), 0); GeschichteStatus.PUBLISHED, null, sentinel(), 0, null);
assertThat(result).isEmpty(); assertThat(result).isEmpty();
} }
@@ -73,7 +73,7 @@ class GeschichteListProjectionTest {
geschichteRepository.save(published("Briefe aus der Front", richAuthor)); geschichteRepository.save(published("Briefe aus der Front", richAuthor));
List<GeschichteSummary> result = geschichteRepository.findSummaries( List<GeschichteSummary> result = geschichteRepository.findSummaries(
GeschichteStatus.PUBLISHED, null, sentinel(), 0); GeschichteStatus.PUBLISHED, null, sentinel(), 0, null);
assertThat(result).hasSize(1); assertThat(result).hasSize(1);
GeschichteSummary.AuthorSummary a = result.get(0).getAuthor(); GeschichteSummary.AuthorSummary a = result.get(0).getAuthor();
@@ -94,7 +94,7 @@ class GeschichteListProjectionTest {
geschichteRepository.save(journey); geschichteRepository.save(journey);
List<GeschichteSummary> result = geschichteRepository.findSummaries( List<GeschichteSummary> result = geschichteRepository.findSummaries(
GeschichteStatus.PUBLISHED, null, sentinel(), 0); GeschichteStatus.PUBLISHED, null, sentinel(), 0, null);
assertThat(result).hasSize(1); assertThat(result).hasSize(1);
assertThat(result.get(0).getType()).isEqualTo(GeschichteType.JOURNEY); assertThat(result.get(0).getType()).isEqualTo(GeschichteType.JOURNEY);
@@ -108,7 +108,7 @@ class GeschichteListProjectionTest {
geschichteRepository.save(draft("Fremder Entwurf", otherAuthor)); geschichteRepository.save(draft("Fremder Entwurf", otherAuthor));
List<GeschichteSummary> result = geschichteRepository.findSummaries( List<GeschichteSummary> result = geschichteRepository.findSummaries(
GeschichteStatus.DRAFT, author.getId(), sentinel(), 0); GeschichteStatus.DRAFT, author.getId(), sentinel(), 0, null);
assertThat(result).hasSize(1); assertThat(result).hasSize(1);
assertThat(result.get(0).getTitle()).isEqualTo("Mein Entwurf"); assertThat(result.get(0).getTitle()).isEqualTo("Mein Entwurf");
@@ -122,7 +122,7 @@ class GeschichteListProjectionTest {
geschichteRepository.save(published("B", author)); geschichteRepository.save(published("B", author));
List<GeschichteSummary> result = geschichteRepository.findSummaries( List<GeschichteSummary> result = geschichteRepository.findSummaries(
GeschichteStatus.PUBLISHED, null, sentinel(), 0); GeschichteStatus.PUBLISHED, null, sentinel(), 0, null);
assertThat(result).hasSize(2); assertThat(result).hasSize(2);
} }
@@ -143,7 +143,7 @@ class GeschichteListProjectionTest {
geschichteRepository.save(withAnna); geschichteRepository.save(withAnna);
List<GeschichteSummary> result = geschichteRepository.findSummaries( List<GeschichteSummary> result = geschichteRepository.findSummaries(
GeschichteStatus.PUBLISHED, null, List.of(franz.getId()), 1); GeschichteStatus.PUBLISHED, null, List.of(franz.getId()), 1, null);
assertThat(result).hasSize(1); assertThat(result).hasSize(1);
assertThat(result.get(0).getTitle()).isEqualTo("Franz story"); assertThat(result.get(0).getTitle()).isEqualTo("Franz story");
@@ -164,7 +164,7 @@ class GeschichteListProjectionTest {
geschichteRepository.save(onlyFranz); geschichteRepository.save(onlyFranz);
List<GeschichteSummary> result = geschichteRepository.findSummaries( List<GeschichteSummary> result = geschichteRepository.findSummaries(
GeschichteStatus.PUBLISHED, null, List.of(franz.getId(), anna.getId()), 2); GeschichteStatus.PUBLISHED, null, List.of(franz.getId(), anna.getId()), 2, null);
assertThat(result).hasSize(1); assertThat(result).hasSize(1);
assertThat(result.get(0).getTitle()).isEqualTo("Both"); assertThat(result.get(0).getTitle()).isEqualTo("Both");

View File

@@ -90,7 +90,7 @@ class GeschichteServiceIntegrationTest {
// Reader cannot see DRAFT in list // Reader cannot see DRAFT in list
authenticateAs(reader, Permission.READ_ALL); authenticateAs(reader, Permission.READ_ALL);
assertThat(geschichteService.list(null, List.of(), 50)).isEmpty(); assertThat(geschichteService.list(null, List.of(), null, 50)).isEmpty();
// Reader cannot fetch DRAFT by id (404 via GESCHICHTE_NOT_FOUND) // Reader cannot fetch DRAFT by id (404 via GESCHICHTE_NOT_FOUND)
UUID draftId = created.getId(); UUID draftId = created.getId();
@@ -106,8 +106,8 @@ class GeschichteServiceIntegrationTest {
// Reader can now see and fetch it // Reader can now see and fetch it
authenticateAs(reader, Permission.READ_ALL); authenticateAs(reader, Permission.READ_ALL);
assertThat(geschichteService.list(null, List.of(), 50)).hasSize(1); assertThat(geschichteService.list(null, List.of(), null, 50)).hasSize(1);
assertThat(geschichteService.list(null, List.of(franz.getId()), 50)).hasSize(1); assertThat(geschichteService.list(null, List.of(franz.getId()), null, 50)).hasSize(1);
Geschichte fetched = geschichteService.getById(draftId); Geschichte fetched = geschichteService.getById(draftId);
GeschichteView fetchedView = geschichteService.toView(fetched, journeyItemService.getItems(draftId)); GeschichteView fetchedView = geschichteService.toView(fetched, journeyItemService.getItems(draftId));
assertThat(fetchedView.title()).isEqualTo("Erinnerung an Opa Franz"); assertThat(fetchedView.title()).isEqualTo("Erinnerung an Opa Franz");
@@ -141,26 +141,26 @@ class GeschichteServiceIntegrationTest {
authenticateAs(reader, Permission.READ_ALL); authenticateAs(reader, Permission.READ_ALL);
// No filter → all three // No filter → all three
assertThat(geschichteService.list(null, List.of(), 50)) assertThat(geschichteService.list(null, List.of(), null, 50))
.extracting(GeschichteSummary::getId) .extracting(GeschichteSummary::getId)
.containsExactlyInAnyOrder(storyAB, storyAC, storyA); .containsExactlyInAnyOrder(storyAB, storyAC, storyA);
// Single filter (Anna) → all three // Single filter (Anna) → all three
assertThat(geschichteService.list(null, List.of(a.getId()), 50)) assertThat(geschichteService.list(null, List.of(a.getId()), null, 50))
.extracting(GeschichteSummary::getId) .extracting(GeschichteSummary::getId)
.containsExactlyInAnyOrder(storyAB, storyAC, storyA); .containsExactlyInAnyOrder(storyAB, storyAC, storyA);
// AND: Anna AND Bertha → only the AB story (NOT story_A, NOT story_AC) // AND: Anna AND Bertha → only the AB story (NOT story_A, NOT story_AC)
assertThat(geschichteService.list(null, List.of(a.getId(), b.getId()), 50)) assertThat(geschichteService.list(null, List.of(a.getId(), b.getId()), null, 50))
.extracting(GeschichteSummary::getId) .extracting(GeschichteSummary::getId)
.containsExactly(storyAB); .containsExactly(storyAB);
// AND: Bertha AND Carl → none (no story has both) // AND: Bertha AND Carl → none (no story has both)
assertThat(geschichteService.list(null, List.of(b.getId(), c.getId()), 50)) assertThat(geschichteService.list(null, List.of(b.getId(), c.getId()), null, 50))
.isEmpty(); .isEmpty();
// AND: Anna AND Bertha AND Carl → none // AND: Anna AND Bertha AND Carl → none
assertThat(geschichteService.list(null, List.of(a.getId(), b.getId(), c.getId()), 50)) assertThat(geschichteService.list(null, List.of(a.getId(), b.getId(), c.getId()), null, 50))
.isEmpty(); .isEmpty();
} }
@@ -179,7 +179,7 @@ class GeschichteServiceIntegrationTest {
geschichteService.create(dto); geschichteService.create(dto);
authenticateAs(writer2, Permission.BLOG_WRITE); authenticateAs(writer2, Permission.BLOG_WRITE);
List<GeschichteSummary> result = geschichteService.list(GeschichteStatus.DRAFT, List.of(), 50); List<GeschichteSummary> result = geschichteService.list(GeschichteStatus.DRAFT, List.of(), null, 50);
assertThat(result).isEmpty(); assertThat(result).isEmpty();
} }

View File

@@ -34,6 +34,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
@@ -222,12 +223,12 @@ class GeschichteServiceTest {
@Test @Test
void list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE() { void list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE() {
authenticateAs(reader, Permission.READ_ALL); authenticateAs(reader, Permission.READ_ALL);
when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any()))
.thenReturn(List.of()); .thenReturn(List.of());
geschichteService.list(null, List.of(), 50); geschichteService.list(null, List.of(), null, 50);
verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong()); verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any());
} }
@Test @Test
@@ -235,25 +236,25 @@ class GeschichteServiceTest {
authenticateAs(writer, Permission.BLOG_WRITE); authenticateAs(writer, Permission.BLOG_WRITE);
GeschichteSummary s1 = mock(GeschichteSummary.class); GeschichteSummary s1 = mock(GeschichteSummary.class);
GeschichteSummary s2 = mock(GeschichteSummary.class); GeschichteSummary s2 = mock(GeschichteSummary.class);
when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any()))
.thenReturn(List.of(s1, s2)); .thenReturn(List.of(s1, s2));
List<GeschichteSummary> out = geschichteService.list(null, List.of(), 50); List<GeschichteSummary> out = geschichteService.list(null, List.of(), null, 50);
assertThat(out).hasSize(2); assertThat(out).hasSize(2);
verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong()); verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any());
} }
@Test @Test
void list_invokes_repository_findSummaries_when_filtering_by_single_personId() { void list_invokes_repository_findSummaries_when_filtering_by_single_personId() {
authenticateAs(reader, Permission.READ_ALL); authenticateAs(reader, Permission.READ_ALL);
UUID personId = UUID.randomUUID(); UUID personId = UUID.randomUUID();
when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any()))
.thenReturn(List.of()); .thenReturn(List.of());
geschichteService.list(null, List.of(personId), 50); geschichteService.list(null, List.of(personId), null, 50);
verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong()); verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any());
} }
@Test @Test
@@ -261,21 +262,33 @@ class GeschichteServiceTest {
authenticateAs(reader, Permission.READ_ALL); authenticateAs(reader, Permission.READ_ALL);
UUID a = UUID.randomUUID(); UUID a = UUID.randomUUID();
UUID b = UUID.randomUUID(); UUID b = UUID.randomUUID();
when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any()))
.thenReturn(List.of()); .thenReturn(List.of());
geschichteService.list(null, List.of(a, b), 50); geschichteService.list(null, List.of(a, b), null, 50);
verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong()); verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), any());
}
@Test
void list_passes_documentId_to_repository_as_journey_item_filter() {
authenticateAs(reader, Permission.READ_ALL);
UUID documentId = UUID.randomUUID();
when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any()))
.thenReturn(List.of());
geschichteService.list(null, List.of(), documentId, 50);
verify(geschichteRepository).findSummaries(any(), any(), any(), anyLong(), eq(documentId));
} }
@Test @Test
void list_caps_limit_at_max_when_caller_passes_huge_value() { void list_caps_limit_at_max_when_caller_passes_huge_value() {
authenticateAs(reader, Permission.READ_ALL); authenticateAs(reader, Permission.READ_ALL);
when(geschichteRepository.findSummaries(any(), any(), any(), anyLong())) when(geschichteRepository.findSummaries(any(), any(), any(), anyLong(), any()))
.thenReturn(List.of(mock(GeschichteSummary.class))); .thenReturn(List.of(mock(GeschichteSummary.class)));
List<GeschichteSummary> out = geschichteService.list(null, List.of(), 9999); List<GeschichteSummary> out = geschichteService.list(null, List.of(), null, 9999);
assertThat(out).hasSizeLessThanOrEqualTo(200); assertThat(out).hasSizeLessThanOrEqualTo(200);
} }