fix(timeline): reject reversed RANGE events; thread precision
All checks were successful
CI / Unit & Component Tests (push) Successful in 5m56s
CI / OCR Service Tests (push) Successful in 29s
CI / Backend Unit Tests (push) Successful in 5m49s
CI / fail2ban Regex (push) Successful in 49s
CI / Semgrep Security Scan (push) Successful in 22s
CI / Compose Bucket Idempotency (push) Successful in 1m6s
All checks were successful
CI / Unit & Component Tests (push) Successful in 5m56s
CI / OCR Service Tests (push) Successful in 29s
CI / Backend Unit Tests (push) Successful in 5m49s
CI / fail2ban Regex (push) Successful in 49s
CI / Semgrep Security Scan (push) Successful in 22s
CI / Compose Bucket Idempotency (push) Successful in 1m6s
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 <noreply@anthropic.com>
This commit was merged in pull request #822.
This commit is contained in:
@@ -43,9 +43,9 @@ public class TimelineEventService {
|
|||||||
|
|
||||||
@Transactional
|
@Transactional
|
||||||
public TimelineEventView create(TimelineEventRequest request, UUID actorId) {
|
public TimelineEventView create(TimelineEventRequest request, UUID actorId) {
|
||||||
validateRangeInvariant(request);
|
|
||||||
validateTitleLength(request);
|
|
||||||
DatePrecision precision = effectivePrecision(request);
|
DatePrecision precision = effectivePrecision(request);
|
||||||
|
validateRangeInvariant(request, precision);
|
||||||
|
validateTitleLength(request);
|
||||||
|
|
||||||
TimelineEvent event = TimelineEvent.builder()
|
TimelineEvent event = TimelineEvent.builder()
|
||||||
.title(request.title())
|
.title(request.title())
|
||||||
@@ -69,9 +69,10 @@ public class TimelineEventService {
|
|||||||
.orElseThrow(() -> DomainException.notFound(ErrorCode.TIMELINE_EVENT_NOT_FOUND,
|
.orElseThrow(() -> DomainException.notFound(ErrorCode.TIMELINE_EVENT_NOT_FOUND,
|
||||||
"Timeline event not found: " + id));
|
"Timeline event not found: " + id));
|
||||||
requireVersionMatch(request, event);
|
requireVersionMatch(request, event);
|
||||||
validateRangeInvariant(request);
|
DatePrecision precision = effectivePrecision(request);
|
||||||
|
validateRangeInvariant(request, precision);
|
||||||
validateTitleLength(request);
|
validateTitleLength(request);
|
||||||
applyUpdate(event, request, actorId);
|
applyUpdate(event, request, precision, actorId);
|
||||||
|
|
||||||
// saveAndFlush (not save) so the versioned UPDATE …WHERE version=? fires HERE, inside the
|
// 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
|
// 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 ---
|
// --- update mechanics: mutate the managed entity, never reassign collections ---
|
||||||
|
|
||||||
private void applyUpdate(TimelineEvent event, TimelineEventRequest request, UUID actorId) {
|
private void applyUpdate(TimelineEvent event, TimelineEventRequest request, DatePrecision precision, UUID actorId) {
|
||||||
DatePrecision precision = effectivePrecision(request);
|
|
||||||
event.setTitle(request.title());
|
event.setTitle(request.title());
|
||||||
event.setType(request.type());
|
event.setType(request.type());
|
||||||
event.setEventDate(normalizeEventDate(request.eventDate(), precision));
|
event.setEventDate(normalizeEventDate(request.eventDate(), precision));
|
||||||
@@ -154,9 +154,15 @@ public class TimelineEventService {
|
|||||||
|
|
||||||
// --- validation / normalization ---
|
// --- validation / normalization ---
|
||||||
|
|
||||||
/** Mirrors the DB biconditional CHECK chk_timeline_event_range — both directions. */
|
/**
|
||||||
private void validateRangeInvariant(TimelineEventRequest request) {
|
* Mirrors the DB biconditional CHECK chk_timeline_event_range — both presence directions — and
|
||||||
boolean isRange = effectivePrecision(request) == DatePrecision.RANGE;
|
* 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) {
|
if (request.eventDateEnd() != null && !isRange) {
|
||||||
throw DomainException.badRequest(ErrorCode.INVALID_DATE_RANGE,
|
throw DomainException.badRequest(ErrorCode.INVALID_DATE_RANGE,
|
||||||
"eventDateEnd is only valid when precision is RANGE");
|
"eventDateEnd is only valid when precision is RANGE");
|
||||||
@@ -165,6 +171,10 @@ public class TimelineEventService {
|
|||||||
throw DomainException.badRequest(ErrorCode.INVALID_DATE_RANGE,
|
throw DomainException.badRequest(ErrorCode.INVALID_DATE_RANGE,
|
||||||
"A RANGE event requires a non-null eventDateEnd");
|
"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");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -153,6 +153,32 @@ class TimelineEventServiceTest {
|
|||||||
verify(events, never()).saveAndFlush(any());
|
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 ---
|
// --- title-length service guard ---
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -287,6 +313,21 @@ class TimelineEventServiceTest {
|
|||||||
assertThat(existing.getDocuments()).isEmpty();
|
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
|
@Test
|
||||||
void update_of_missing_id_throws_TIMELINE_EVENT_NOT_FOUND() {
|
void update_of_missing_id_throws_TIMELINE_EVENT_NOT_FOUND() {
|
||||||
UUID id = UUID.randomUUID();
|
UUID id = UUID.randomUUID();
|
||||||
|
|||||||
Reference in New Issue
Block a user