From d9e431cd121ce4a9d2328d15f311480e12e85108 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 15 Jun 2026 22:00:52 +0200 Subject: [PATCH] =?UTF-8?q?perf(timeline):=20bound=20the=20letter=E2=86=92?= =?UTF-8?q?event=20link=20pass=20to=20filtered=20events?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveLetterEventLinks iterated `eventRepository.findAll()` and touched the lazy `documents` collection on every event — including events removed by the type/person/generation/year filters and never shown. On a large archive that hydrates join rows that are immediately discarded. It now runs over the events that survived the filter (collected once in the existing event loop). A letter whose only linking event was filtered out links to nothing, which matches the frontend's filter-then-cluster (the letter renders loose either way). Fixes review finding #10 (DevOps). Refs #850 Co-Authored-By: Claude Opus 4.8 --- .../timeline/TimelineService.java | 17 ++++++++------ .../timeline/TimelineServiceTest.java | 22 +++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineService.java b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineService.java index cd918627..75dc381a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineService.java @@ -80,18 +80,20 @@ public class TimelineService { // Resolve generation person IDs once — used across all three layers Set genPersonIds = resolveGenerationPersonIds(filter.generation()); - // Fetch curated events once — reused for both the event entries below and the - // batched letter→event link resolution (resolveLetterEventLinks), so the - // membership pass costs no extra query. REQ-009. + // Fetch curated events once; the events that survive the filter below feed both the + // event entries and the batched letter→event link pass (resolveLetterEventLinks), so the + // membership pass costs no extra query and touches only on-screen events. REQ-009. List allEvents = eventRepository.findAll(); // ── curated events ─────────────────────────────────────────────────── List entries = new ArrayList<>(); + List filteredEvents = new ArrayList<>(); for (TimelineEvent ev : allEvents) { if (!passesTypeFilter(ev.getType(), filter.type())) continue; if (!passesPersonFilter(ev.getPersons(), filter.personId())) continue; if (!passesGenerationFilter(ev.getPersons(), genPersonIds)) continue; if (!passesYearFilter(ev.getEventDate(), ev.getPrecision(), filter)) continue; + filteredEvents.add(ev); entries.add(mapEvent(ev)); } @@ -112,7 +114,7 @@ public class TimelineService { letters.add(doc); } Map rootByDocId = resolveLetterRootTags(letters); - Map eventByDocId = resolveLetterEventLinks(letters, allEvents); + Map eventByDocId = resolveLetterEventLinks(letters, filteredEvents); for (Document doc : letters) { entries.add(mapDocument(doc, rootByDocId, eventByDocId)); } @@ -272,9 +274,10 @@ public class TimelineService { * event, the earliest-dated event wins (then lowest {@code eventId}); the pass runs over a * stably-ordered copy so the link is a deterministic property of the data, not a coin-flip on * the undefined repository iteration order ({@code putIfAbsent} keeps the first = winner). The - * map is built from all events (not just the year/type-filtered ones) so the link is a - * stable property of the data; the frontend's filter-then-cluster decides whether the linked - * event is actually on screen (#850). Logs nothing — the assembly path keeps UUIDs out of logs (§2.7). + * map is built only over the events that survived the timeline filter, so the lazy + * {@code documents} collection is hydrated for on-screen events alone (finding #10); a letter + * whose only linking event was filtered out links to nothing, matching the frontend's + * filter-then-cluster (#850). Logs nothing — the assembly path keeps UUIDs out of logs (§2.7). */ private Map resolveLetterEventLinks(List letters, List events) { Set letterDocIds = letters.stream().map(Document::getId).collect(Collectors.toSet()); diff --git a/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineServiceTest.java index 2409f3ca..b146f66e 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineServiceTest.java @@ -581,6 +581,28 @@ class TimelineServiceTest { .isEqualTo(earlier.getId()); } + @Test + void letter_linked_only_to_a_filtered_out_event_has_null_linkedEventId() { + // finding #10: the link pass runs over the events that survived the filter, not all of + // them. A letter whose only linking event is excluded by the active filter links to + // nothing (the frontend would drop it loose anyway) — and the lazy `documents` collection + // is never hydrated for events that are off-screen. + Document letterDoc = docWithDate(LocalDate.of(1916, 5, 1), DatePrecision.MONTH); + TimelineEvent worldEvent = TimelineEvent.builder().id(UUID.randomUUID()) + .title("Somme").type(EventType.HISTORICAL) + .documents(new HashSet<>(Set.of(letterDoc))) + .build(); + when(eventRepository.findAll()).thenReturn(List.of(worldEvent)); + when(timelineEventService.assembleDerivedEvents()).thenReturn(List.of()); + when(documentService.getAllForTimeline()).thenReturn(List.of(letterDoc)); + + // Filter to PERSONAL only → the HISTORICAL event is filtered out of the view. + TimelineEntryDTO entry = theLetter(timelineService.assemble( + new TimelineFilter(null, null, EventType.PERSONAL, null, null))); + + assertThat(entry.linkedEventId()).isNull(); + } + private static TimelineEntryDTO theLetter(TimelineDTO result) { return java.util.stream.Stream.concat( result.years().stream().flatMap(y -> y.entries().stream()),