Skip to content

Deepen calendar time-window handling#43

Merged
Pan14ek merged 1 commit into
mainfrom
bug/calendar-time-window-depth
Jun 1, 2026
Merged

Deepen calendar time-window handling#43
Pan14ek merged 1 commit into
mainfrom
bug/calendar-time-window-depth

Conversation

@Pan14ek
Copy link
Copy Markdown
Owner

@Pan14ek Pan14ek commented Jun 1, 2026

Purpose

Deepen calendar event scheduling internals so create, update, list, conflict, and user-conflict flows use one validated time-window path and keep overlap analysis behind the event scheduling module interface.

Type

  • 🧪 Experiment (fitness function research)
  • ✨ Feature (new business logic)
  • 🐛 Bug fix
  • ♻️ Refactor / neutral change
  • 📝 Documentation only

Changes

  • Added CalendarTimeWindow for validated half-open event windows.
  • Added EventConflictAnalyzer for pairwise overlap detection and conflict DTO mapping.
  • Refactored EventServiceImpl to reuse the shared time-window path.
  • Fixed invalid partial event updates so validation happens before entity mutation.
  • Bound conflict requester checks to X-User-Id to prevent spoofing requesterId.
  • Added focused unit tests for window validation, conflict analysis, controller requester checks, and service regressions.

Files changed

File Change
src/main/java/io/backend/lined/event/service/CalendarTimeWindow.java Added validated half-open time-window value object.
src/main/java/io/backend/lined/event/service/EventConflictAnalyzer.java Added scheduling seam for overlap/conflict analysis.
src/main/java/io/backend/lined/event/service/EventServiceImpl.java Reused validated windows across event flows and added own-calendar check.
src/main/java/io/backend/lined/event/api/EventController.java Bound conflict requester identity to X-User-Id.
src/test/java/io/backend/lined/event/** Added and updated focused tests for the new behavior.

Expected result

Metric Baseline (main) Branch Direction
checkstyle_violations neutral
spotbugs_total neutral
line_coverage increase
critical_violations neutral
code_smells neutral
duplicated_lines_density neutral
F score neutral/increase
SonarQube QG neutral

Checklist

  • ./gradlew check passes locally
  • ./gradlew jacocoTestReport passes locally
  • No unintended changes to main business logic
  • Branch name matches experiment/feature naming convention

Copy link
Copy Markdown
Owner Author

@Pan14ek Pan14ek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Good refactor overall. The CalendarTimeWindow value object centralises validation cleanly, the EventConflictAnalyzer extraction removes duplication, the mutation-before-validation bug in update() is fixed, and the spoofable requesterId is now locked to the X-User-Id header.

Two things to address before merge:

  1. Missing regression test for a partial update where one timestamp is null in the DTO but the resolved pair is invalid (see inline comment on EventServiceImpl).
  2. BadRequestException thrown from persisted entity data in EventConflictAnalyzer (see inline comment) — maps a data-integrity failure to HTTP 400.

One post-merge candidate: the now-redundant requesterId query param on the conflict endpoints (see inline comment on the controller).

@Pan14ek Pan14ek force-pushed the bug/calendar-time-window-depth branch from c4e54c2 to 38f0185 Compare June 1, 2026 14:48
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 1, 2026

Copy link
Copy Markdown
Owner Author

@Pan14ek Pan14ek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All review comments addressed — LGTM.

What changed since the first review:

  • EventConflictAnalyzer.windowOf() now throws IllegalStateException for stored entities with invalid timestamps, correctly separating data-integrity failures from client errors.
  • Windows are now pre-computed via .stream().map(this::windowOf).toList() before the O(n²) loop — a bonus improvement: each entity's window is built exactly once instead of potentially multiple times across addConflict calls.
  • CalendarTimeWindow.overlapWith() has the invariant comment explaining why the direct constructor call is safe.
  • update_throwsBadRequest_whenNewStartAtExceedsExistingEndAt covers the missing partial-update regression path (new startAt beyond existing endAt with a null DTO endAt).
  • EventConflictAnalyzerTest adds two IllegalStateException tests for entities with invalid stored windows.

The redundant requesterId query param remains as a post-merge candidate — no objection.

@Pan14ek Pan14ek merged commit 86356a9 into main Jun 1, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant