4c58603009
Structural fix for the "end before start" bug class observed on prod
2026-04-29. Bad data became inexpressible at the schema level instead
of getting trapped in defensive read-path filters.
The hotfix that landed earlier today (94b169f) is reverted by the
preceding revert commit; this commit supersedes it cleanly with a
proper data-model change.
## Schema (migration 0043)
- Add `duration_minutes INTEGER NULLABLE` column on `events`.
- CHECK constraint: ``duration_minutes IS NULL OR duration_minutes >= 0``.
- Backfill from existing `end_dt`:
- end_dt valid (end > start) → duration_minutes = total minutes
- end_dt == start → duration_minutes = 0 (zero-duration point)
- end_dt NULL or end_dt < start → duration_minutes = NULL
(the corrupt prod row collapses cleanly to a point event)
- Drop the `end_dt` column. The wire format is preserved — `to_dict()`
emits `end_dt` as a derived `start_dt + duration_minutes`. Existing
API consumers (Flutter app, web frontend, CalDAV sync) keep
receiving the same response shape; they just no longer have a way
to PUT a stored `end_dt` that disagrees with `start_dt`.
## Service layer
- `Event.end_dt` becomes a `@property`. Setting it would require a
setter we deliberately don't define — writes always go through
`duration_minutes`.
- `_normalize_duration` is the single source-of-truth for input
reduction. Accepts (start, end_dt, duration_minutes), returns the
canonical `duration_minutes`, raises `ValueError` for negative
durations, end-before-start, or end/duration disagreement.
- `create_event` and `update_event` accept either `end_dt` or
`duration_minutes` for ergonomic compat; both convert via
`_normalize_duration`. Update validates the post-update state when
the patch includes either.
- `list_events` filter is simpler now: a coarse SQL prefilter
(`start_dt <= date_to`) plus Python-side refinement using the
derived `end_dt`. Avoids Postgres-specific interval arithmetic in
the WHERE clause; refinement runs over a per-user result set so
there's no scan-cost concern at personal scale.
- Recurring-event expansion uses `event.duration_minutes` directly
instead of computing `end - start`. No more negative-timedelta
hazard.
## CalDAV sync (incoming + outgoing)
- `caldav_sync.py` (pull) and `calendar_sync.py` (Radicale upsert)
both convert iCal `DTEND` → `duration_minutes` on the way in.
Outbound iCal still emits `DTEND` as `start_dt + duration_minutes`
via the model's derived property. iCal interop is unchanged.
## Behavioral upgrade for `update_event`
Pure end_dt model: moving start past the existing end_dt would either
silently corrupt or hard-reject. Duration model: the duration is
preserved by default, so moving start slides the effective end
forward — which is what users mean when they "move" an event.
Explicit clear is still possible via `end_dt=None`.
## Tests
`tests/test_events_service.py`:
- 6 new `_normalize_duration` unit tests (sugar conversion, zero
duration valid as point event, end-before-start rejected, negative
duration rejected, inconsistent end+duration rejected, none → None)
- New behavioral test: `update_event` preserves duration when only
start_dt changes (sliding semantics)
- New: clearing `end_dt=None` on update collapses to point event
- New: list_events surfaces a point event in the upcoming window
- New: list_events excludes a timed event whose effective end has
already passed
- Existing mock-event helper updated to use `duration_minutes`
instead of stored `end_dt`.
44 event-related tests pass; ruff clean.
## Out of scope (separate task)
Fable #161 — `find_events_by_query` returning multiple matches and
silently picking matches[0]. The exact root cause of how event id=2
got mutated in the first place; orthogonal to the storage model.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
313 lines
12 KiB
Python
313 lines
12 KiB
Python
"""Calendar sync service: bridges the DB Event table with local Radicale.
|
|
|
|
Each user's calendars live at: http://radicale:5232/user_{user_id}/calendar/
|
|
The app connects without auth (Radicale runs with auth.type = none inside Docker).
|
|
|
|
External clients (iOS, macOS, Thunderbird) connect directly to Radicale on port 5232.
|
|
Their CalDAV URL is: http://<host>:5232/user_{user_id}/calendar/
|
|
"""
|
|
|
|
import asyncio
|
|
import logging
|
|
import uuid
|
|
from datetime import datetime, timezone
|
|
|
|
import httpx
|
|
import icalendar
|
|
from sqlalchemy import select
|
|
|
|
from fabledassistant.config import Config
|
|
from fabledassistant.models import async_session
|
|
from fabledassistant.models.event import Event
|
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
# Internal Radicale URL (Docker service name)
|
|
_RADICALE_INTERNAL = "http://radicale:5232"
|
|
|
|
|
|
def _user_calendar_path(user_id: int) -> str:
|
|
return f"/user_{user_id}/calendar/"
|
|
|
|
|
|
def _user_calendar_url(user_id: int) -> str:
|
|
base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL
|
|
return f"{base}/user_{user_id}/calendar/"
|
|
|
|
|
|
async def setup_user_calendar(user_id: int) -> bool:
|
|
"""Create Radicale collection for user if it doesn't exist.
|
|
|
|
Returns True on success or if already exists, False on error.
|
|
"""
|
|
base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL
|
|
user_url = f"{base}/user_{user_id}/"
|
|
cal_url = f"{base}/user_{user_id}/calendar/"
|
|
|
|
try:
|
|
async with httpx.AsyncClient(timeout=10.0) as client:
|
|
# Create user home collection
|
|
await client.request(
|
|
"MKCOL",
|
|
user_url,
|
|
headers={"Content-Type": "text/xml"},
|
|
content="""<?xml version="1.0" encoding="utf-8"?>
|
|
<mkcol xmlns="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
|
|
<set><prop><resourcetype><collection/></resourcetype></prop></set>
|
|
</mkcol>""",
|
|
)
|
|
# Create calendar collection
|
|
resp = await client.request(
|
|
"MKCOL",
|
|
cal_url,
|
|
headers={"Content-Type": "text/xml"},
|
|
content="""<?xml version="1.0" encoding="utf-8"?>
|
|
<mkcol xmlns="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
|
|
<set>
|
|
<prop>
|
|
<resourcetype><collection/><C:calendar/></resourcetype>
|
|
<displayname>Fabled Scribe</displayname>
|
|
</prop>
|
|
</set>
|
|
</mkcol>""",
|
|
)
|
|
if resp.status_code in (201, 405): # 405 = already exists
|
|
logger.info("Calendar collection ready for user %d", user_id)
|
|
return True
|
|
logger.warning("Unexpected status %d creating calendar for user %d", resp.status_code, user_id)
|
|
return False
|
|
except Exception:
|
|
logger.warning("Failed to setup Radicale calendar for user %d", user_id, exc_info=True)
|
|
return False
|
|
|
|
|
|
def _make_vcalendar(event: Event) -> str:
|
|
"""Build iCalendar string from DB Event."""
|
|
cal = icalendar.Calendar()
|
|
cal.add("prodid", "-//FabledAssistant//EN")
|
|
cal.add("version", "2.0")
|
|
|
|
vevent = icalendar.Event()
|
|
vevent.add("uid", event.uid)
|
|
vevent.add("summary", event.title)
|
|
|
|
if event.all_day:
|
|
vevent.add("dtstart", event.start_dt.date())
|
|
if event.end_dt:
|
|
vevent.add("dtend", event.end_dt.date())
|
|
else:
|
|
vevent.add("dtstart", event.start_dt)
|
|
if event.end_dt:
|
|
vevent.add("dtend", event.end_dt)
|
|
|
|
if event.description:
|
|
vevent.add("description", event.description)
|
|
if event.location:
|
|
vevent.add("location", event.location)
|
|
if event.recurrence:
|
|
rrule_parts: dict = {}
|
|
for part in event.recurrence.split(";"):
|
|
if "=" in part:
|
|
key, value = part.split("=", 1)
|
|
rrule_parts[key.strip().lower()] = value.strip()
|
|
if rrule_parts:
|
|
vevent.add("rrule", rrule_parts)
|
|
|
|
vevent.add("dtstamp", datetime.now(timezone.utc))
|
|
cal.add_component(vevent)
|
|
return cal.to_ical().decode("utf-8")
|
|
|
|
|
|
async def push_event_to_radicale(user_id: int, event: Event) -> bool:
|
|
"""Write a DB Event to Radicale. Creates or updates the resource.
|
|
|
|
Returns True on success.
|
|
"""
|
|
base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL
|
|
resource_url = f"{base}/user_{user_id}/calendar/{event.uid}.ics"
|
|
ical_data = _make_vcalendar(event)
|
|
|
|
try:
|
|
async with httpx.AsyncClient(timeout=10.0) as client:
|
|
resp = await client.put(
|
|
resource_url,
|
|
content=ical_data.encode("utf-8"),
|
|
headers={"Content-Type": "text/calendar; charset=utf-8"},
|
|
)
|
|
if resp.status_code in (200, 201, 204):
|
|
return True
|
|
logger.warning("Radicale PUT returned %d for event %s", resp.status_code, event.uid)
|
|
return False
|
|
except Exception:
|
|
logger.warning("Failed to push event %s to Radicale", event.uid, exc_info=True)
|
|
return False
|
|
|
|
|
|
async def delete_event_from_radicale(user_id: int, uid: str) -> bool:
|
|
"""Delete an event from Radicale by UID."""
|
|
base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL
|
|
resource_url = f"{base}/user_{user_id}/calendar/{uid}.ics"
|
|
try:
|
|
async with httpx.AsyncClient(timeout=10.0) as client:
|
|
resp = await client.delete(resource_url)
|
|
return resp.status_code in (200, 204, 404) # 404 is ok (already gone)
|
|
except Exception:
|
|
logger.warning("Failed to delete event %s from Radicale", uid, exc_info=True)
|
|
return False
|
|
|
|
|
|
async def sync_event_to_db(user_id: int, ical_uid: str) -> Event | None:
|
|
"""Fetch an event from Radicale and upsert into the DB Event table.
|
|
|
|
Returns the upserted Event or None on error.
|
|
"""
|
|
base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL
|
|
resource_url = f"{base}/user_{user_id}/calendar/{ical_uid}.ics"
|
|
|
|
try:
|
|
async with httpx.AsyncClient(timeout=10.0) as client:
|
|
resp = await client.get(resource_url)
|
|
if resp.status_code == 404:
|
|
return None
|
|
resp.raise_for_status()
|
|
ical_data = resp.text
|
|
except Exception:
|
|
logger.warning("Failed to fetch event %s from Radicale", ical_uid, exc_info=True)
|
|
return None
|
|
|
|
try:
|
|
cal = icalendar.Calendar.from_ical(ical_data)
|
|
for component in cal.walk():
|
|
if component.name != "VEVENT":
|
|
continue
|
|
title = str(component.get("SUMMARY", ""))
|
|
dtstart = component.get("DTSTART")
|
|
dtend = component.get("DTEND")
|
|
description = str(component.get("DESCRIPTION", ""))
|
|
location = str(component.get("LOCATION", ""))
|
|
rrule = component.get("RRULE")
|
|
recurrence = rrule.to_ical().decode("utf-8") if rrule else None
|
|
|
|
all_day = False
|
|
if dtstart and isinstance(dtstart.dt, datetime):
|
|
start_dt = dtstart.dt if dtstart.dt.tzinfo else dtstart.dt.replace(tzinfo=timezone.utc)
|
|
elif dtstart:
|
|
from datetime import date
|
|
d = dtstart.dt
|
|
start_dt = datetime(d.year, d.month, d.day, tzinfo=timezone.utc)
|
|
all_day = True
|
|
else:
|
|
continue
|
|
|
|
end_dt = None
|
|
if dtend and isinstance(dtend.dt, datetime):
|
|
end_dt = dtend.dt if dtend.dt.tzinfo else dtend.dt.replace(tzinfo=timezone.utc)
|
|
elif dtend:
|
|
from datetime import date
|
|
d = dtend.dt
|
|
end_dt = datetime(d.year, d.month, d.day, tzinfo=timezone.utc)
|
|
|
|
# Storage uses duration, not end_dt. Convert iCal DTEND to a
|
|
# minute count anchored on DTSTART. Treat invalid (end <= start)
|
|
# incoming data as a point event rather than rejecting; we
|
|
# don't control external CalDAV writers.
|
|
duration_minutes = None
|
|
if end_dt is not None and end_dt > start_dt:
|
|
duration_minutes = int((end_dt - start_dt).total_seconds() // 60)
|
|
|
|
async with async_session() as session:
|
|
result = await session.execute(
|
|
select(Event).where(Event.user_id == user_id, Event.uid == ical_uid)
|
|
)
|
|
existing = result.scalars().first()
|
|
if existing:
|
|
existing.title = title
|
|
existing.start_dt = start_dt
|
|
existing.duration_minutes = duration_minutes
|
|
existing.all_day = all_day
|
|
existing.description = description
|
|
existing.location = location
|
|
existing.recurrence = recurrence
|
|
existing.updated_at = datetime.now(timezone.utc)
|
|
await session.commit()
|
|
await session.refresh(existing)
|
|
return existing
|
|
else:
|
|
event_obj = Event(
|
|
user_id=user_id,
|
|
uid=ical_uid,
|
|
title=title,
|
|
start_dt=start_dt,
|
|
duration_minutes=duration_minutes,
|
|
all_day=all_day,
|
|
description=description,
|
|
location=location,
|
|
recurrence=recurrence,
|
|
)
|
|
session.add(event_obj)
|
|
await session.commit()
|
|
await session.refresh(event_obj)
|
|
return event_obj
|
|
except Exception:
|
|
logger.warning("Failed to parse/upsert Radicale event %s", ical_uid, exc_info=True)
|
|
return None
|
|
|
|
|
|
async def sync_all_events_from_radicale(user_id: int) -> int:
|
|
"""Full sync: fetch all events from Radicale calendar and upsert into DB.
|
|
|
|
Returns number of events synced.
|
|
"""
|
|
base = Config.RADICALE_URL.rstrip("/") if Config.RADICALE_URL else _RADICALE_INTERNAL
|
|
cal_url = f"{base}/user_{user_id}/calendar/"
|
|
|
|
try:
|
|
async with httpx.AsyncClient(timeout=30.0) as client:
|
|
# PROPFIND to get all .ics resources
|
|
resp = await client.request(
|
|
"PROPFIND",
|
|
cal_url,
|
|
headers={"Depth": "1", "Content-Type": "text/xml"},
|
|
content="""<?xml version="1.0" encoding="utf-8"?>
|
|
<propfind xmlns="DAV:"><prop><getetag/><getcontenttype/></prop></propfind>""",
|
|
)
|
|
if resp.status_code == 404:
|
|
logger.info("No calendar found for user %d — setting up", user_id)
|
|
await setup_user_calendar(user_id)
|
|
return 0
|
|
if resp.status_code != 207:
|
|
logger.warning("PROPFIND returned %d for user %d", resp.status_code, user_id)
|
|
return 0
|
|
# Extract UIDs from href paths
|
|
import re
|
|
hrefs = re.findall(r"<[Dd]:[Hh]ref>([^<]+\.ics)</[Dd]:[Hh]ref>", resp.text)
|
|
uids = [h.rstrip("/").split("/")[-1].removesuffix(".ics") for h in hrefs]
|
|
except Exception:
|
|
logger.warning("Failed to list Radicale events for user %d", user_id, exc_info=True)
|
|
return 0
|
|
|
|
synced = 0
|
|
for uid in uids:
|
|
if uid:
|
|
event = await sync_event_to_db(user_id, uid)
|
|
if event:
|
|
synced += 1
|
|
|
|
logger.info("Synced %d events from Radicale for user %d", synced, user_id)
|
|
return synced
|
|
|
|
|
|
def get_external_caldav_url(user_id: int) -> str:
|
|
"""Return the CalDAV URL for external clients (iOS, macOS, Thunderbird).
|
|
|
|
Uses BASE_URL from Config with port 5232 substituted, or RADICALE_EXTERNAL_URL.
|
|
"""
|
|
external = Config.RADICALE_EXTERNAL_URL
|
|
if external:
|
|
return f"{external.rstrip('/')}/user_{user_id}/calendar/"
|
|
# Fall back to BASE_URL host with port 5232
|
|
from urllib.parse import urlparse
|
|
parsed = urlparse(Config.BASE_URL)
|
|
host = parsed.hostname or "localhost"
|
|
return f"http://{host}:5232/user_{user_id}/calendar/"
|