From 210dde6562a2e6753c2163e32ccb204727d14a7b Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 13 Jun 2026 12:01:14 +0200 Subject: [PATCH] fix(timeline): reject reversed RANGE events; thread precision The DB CHECK chk_timeline_event_range enforces only the presence biconditional (eventDateEnd non-null IFF RANGE), not date ordering, so a RANGE event with eventDateEnd before eventDate persisted silently and rendered as a negative span. validateRangeInvariant now also rejects end-before-start (INVALID_DATE_RANGE); equal dates remain a valid one-day closed range. Also compute effectivePrecision once per create/update and thread it into validateRangeInvariant and applyUpdate instead of recomputing. Addresses review of #822 (#775). Co-Authored-By: Claude Fable 5 --- .../timeline/TimelineEventService.java | 28 +++++++++---- .../timeline/TimelineEventServiceTest.java | 41 +++++++++++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEventService.java b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEventService.java index c97dc792..e7c0d892 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEventService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/timeline/TimelineEventService.java @@ -43,9 +43,9 @@ public class TimelineEventService { @Transactional public TimelineEventView create(TimelineEventRequest request, UUID actorId) { - validateRangeInvariant(request); - validateTitleLength(request); DatePrecision precision = effectivePrecision(request); + validateRangeInvariant(request, precision); + validateTitleLength(request); TimelineEvent event = TimelineEvent.builder() .title(request.title()) @@ -69,9 +69,10 @@ public class TimelineEventService { .orElseThrow(() -> DomainException.notFound(ErrorCode.TIMELINE_EVENT_NOT_FOUND, "Timeline event not found: " + id)); requireVersionMatch(request, event); - validateRangeInvariant(request); + DatePrecision precision = effectivePrecision(request); + validateRangeInvariant(request, precision); validateTitleLength(request); - applyUpdate(event, request, actorId); + applyUpdate(event, request, precision, actorId); // saveAndFlush (not save) so the versioned UPDATE …WHERE version=? fires HERE, inside the // try — a bare save() flushes at commit, after this method returns, so the exception would @@ -108,8 +109,7 @@ public class TimelineEventService { // --- update mechanics: mutate the managed entity, never reassign collections --- - private void applyUpdate(TimelineEvent event, TimelineEventRequest request, UUID actorId) { - DatePrecision precision = effectivePrecision(request); + private void applyUpdate(TimelineEvent event, TimelineEventRequest request, DatePrecision precision, UUID actorId) { event.setTitle(request.title()); event.setType(request.type()); event.setEventDate(normalizeEventDate(request.eventDate(), precision)); @@ -154,9 +154,15 @@ public class TimelineEventService { // --- validation / normalization --- - /** Mirrors the DB biconditional CHECK chk_timeline_event_range — both directions. */ - private void validateRangeInvariant(TimelineEventRequest request) { - boolean isRange = effectivePrecision(request) == DatePrecision.RANGE; + /** + * Mirrors the DB biconditional CHECK chk_timeline_event_range — both presence directions — and + * additionally enforces date ordering, which the DB CHECK does NOT: {@code eventDateEnd} may + * equal but never precede {@code eventDate}. Without this guard a reversed range (end before + * start) persists silently and renders as a negative span. Equal dates are a valid one-day + * closed range. + */ + private void validateRangeInvariant(TimelineEventRequest request, DatePrecision precision) { + boolean isRange = precision == DatePrecision.RANGE; if (request.eventDateEnd() != null && !isRange) { throw DomainException.badRequest(ErrorCode.INVALID_DATE_RANGE, "eventDateEnd is only valid when precision is RANGE"); @@ -165,6 +171,10 @@ public class TimelineEventService { throw DomainException.badRequest(ErrorCode.INVALID_DATE_RANGE, "A RANGE event requires a non-null eventDateEnd"); } + if (isRange && request.eventDateEnd().isBefore(request.eventDate())) { + throw DomainException.badRequest(ErrorCode.INVALID_DATE_RANGE, + "eventDateEnd must not precede eventDate"); + } } /** diff --git a/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineEventServiceTest.java b/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineEventServiceTest.java index b70f8a6e..658a198f 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineEventServiceTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/timeline/TimelineEventServiceTest.java @@ -153,6 +153,32 @@ class TimelineEventServiceTest { verify(events, never()).saveAndFlush(any()); } + @Test + void create_rejects_RANGE_with_eventDateEnd_before_eventDate() { + TimelineEventRequest request = new TimelineEventRequest( + "Krieg", EventType.HISTORICAL, LocalDate.of(1918, 11, 11), + DatePrecision.RANGE, LocalDate.of(1914, 7, 28), null, null, null, null); // end < start + + assertThatThrownBy(() -> service.create(request, actor)) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getCode()).isEqualTo(ErrorCode.INVALID_DATE_RANGE); + verify(events, never()).saveAndFlush(any()); + } + + @Test + void create_accepts_RANGE_with_eventDateEnd_equal_to_eventDate() { + stubFlushSetsVersion(); + LocalDate sameDay = LocalDate.of(1914, 7, 28); + TimelineEventRequest request = new TimelineEventRequest( + "Eintägiges Ereignis", EventType.HISTORICAL, sameDay, + DatePrecision.RANGE, sameDay, null, null, null, null); // end == start is a valid closed range + + TimelineEventView view = service.create(request, actor); + + assertThat(view.eventDate()).isEqualTo(sameDay); + assertThat(view.eventDateEnd()).isEqualTo(sameDay); + } + // --- title-length service guard --- @Test @@ -287,6 +313,21 @@ class TimelineEventServiceTest { assertThat(existing.getDocuments()).isEmpty(); } + @Test + void update_rejects_RANGE_with_eventDateEnd_before_eventDate_without_saving() { + UUID id = UUID.randomUUID(); + TimelineEvent existing = existingEvent(id, UUID.randomUUID()); + when(events.findById(id)).thenReturn(Optional.of(existing)); + TimelineEventRequest request = new TimelineEventRequest( + "Krieg", EventType.HISTORICAL, LocalDate.of(1918, 11, 11), + DatePrecision.RANGE, LocalDate.of(1914, 7, 28), null, null, null, null); // end < start + + assertThatThrownBy(() -> service.update(id, request, secondEditor)) + .isInstanceOf(DomainException.class) + .extracting(e -> ((DomainException) e).getCode()).isEqualTo(ErrorCode.INVALID_DATE_RANGE); + verify(events, never()).saveAndFlush(any()); + } + @Test void update_of_missing_id_throws_TIMELINE_EVENT_NOT_FOUND() { UUID id = UUID.randomUUID();