From baf624c7a617d5895cd8522333035ba0e62086a9 Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Tue, 21 Jul 2026 12:33:34 +0200 Subject: [PATCH] Refactor calendar sync source updates --- src/govoplan_calendar/backend/service.py | 315 ++++++++++++++++------- 1 file changed, 216 insertions(+), 99 deletions(-) diff --git a/src/govoplan_calendar/backend/service.py b/src/govoplan_calendar/backend/service.py index ee79a44..7d737d9 100644 --- a/src/govoplan_calendar/backend/service.py +++ b/src/govoplan_calendar/backend/service.py @@ -922,12 +922,16 @@ def create_caldav_source(session: Session, *, tenant_id: str, user_id: str | Non return create_sync_source(session, tenant_id=tenant_id, user_id=user_id, payload=payload) -def update_sync_source(session: Session, *, tenant_id: str, source_id: str, payload: CalendarSyncSourceUpdateRequest) -> CalendarSyncSource: - source = get_sync_source(session, tenant_id=tenant_id, source_id=source_id) - source = ( +def _lock_sync_source_for_update( + session: Session, + *, + tenant_id: str, + source_id: str, +) -> CalendarSyncSource: + return ( session.query(CalendarSyncSource) .filter( - CalendarSyncSource.id == source.id, + CalendarSyncSource.id == source_id, CalendarSyncSource.tenant_id == tenant_id, CalendarSyncSource.deleted_at.is_(None), ) @@ -935,119 +939,162 @@ def update_sync_source(session: Session, *, tenant_id: str, source_id: str, payl .with_for_update() .one() ) - if payload.credential_ref is not None and payload.credential_ref != source.credential_ref: - raise CalendarError( - "Caller-supplied credential references are not accepted; provide a replacement password or bearer token" - ) + + +def _sync_source_reconfiguration( + source: CalendarSyncSource, + payload: CalendarSyncSourceUpdateRequest, +) -> tuple[str, bool, bool]: normalized_collection_url = ( normalize_sync_source_url(source.source_kind, payload.collection_url) if payload.collection_url is not None else source.collection_url ) + calendar_changed = bool( + payload.calendar_id is not None and payload.calendar_id != source.calendar_id + ) + endpoint_changed = normalized_collection_url != source.collection_url + endpoint_or_calendar_changed = bool( + source.source_kind == "caldav" and (calendar_changed or endpoint_changed) + ) materially_reconfigured = bool( source.source_kind == "caldav" and ( - (payload.calendar_id is not None and payload.calendar_id != source.calendar_id) - or normalized_collection_url != source.collection_url + calendar_changed + or endpoint_changed or payload.sync_enabled is False or payload.sync_direction == "inbound" ) ) - endpoint_or_calendar_changed = bool( - source.source_kind == "caldav" - and ( - (payload.calendar_id is not None and payload.calendar_id != source.calendar_id) - or normalized_collection_url != source.collection_url + return ( + normalized_collection_url, + materially_reconfigured, + endpoint_or_calendar_changed, + ) + + +def _caldav_source_has_linked_events( + session: Session, + *, + tenant_id: str, + source: CalendarSyncSource, +) -> bool: + return ( + session.query(CalendarEvent.id) + .filter( + CalendarEvent.tenant_id == tenant_id, + CalendarEvent.calendar_id == source.calendar_id, + CalendarEvent.source_kind == "caldav", + CalendarEvent.source_href.is_not(None), + CalendarEvent.deleted_at.is_(None), + ) + .first() + is not None + ) + + +def _guard_sync_source_reconfiguration( + session: Session, + *, + tenant_id: str, + source: CalendarSyncSource, + payload: CalendarSyncSourceUpdateRequest, + endpoint_or_calendar_changed: bool, +) -> None: + from govoplan_calendar.backend.outbox import ( + calendar_outbox_has_live_lease, + calendar_outbox_has_unresolved_desired_state, + cancel_calendar_outbox_for_source, + ) + + if calendar_outbox_has_live_lease(session, source_id=source.id): + raise CalendarError( + "CalDAV source cannot be materially changed while an outbound operation has an active lease" + ) + unresolved_desired_state = calendar_outbox_has_unresolved_desired_state( + session, + source_id=source.id, + ) + stops_outbound_delivery = bool( + (payload.sync_enabled is False and source.sync_enabled) + or ( + payload.sync_direction == "inbound" + and source.sync_direction == "two_way" ) ) - if materially_reconfigured: - from govoplan_calendar.backend.outbox import ( - calendar_outbox_has_live_lease, - calendar_outbox_has_unresolved_desired_state, - cancel_calendar_outbox_for_source, + if stops_outbound_delivery and unresolved_desired_state: + raise CalendarError( + "CalDAV source cannot disable outbound delivery while unresolved local desired " + "changes exist; deliver or explicitly discard them first" ) + if endpoint_or_calendar_changed and unresolved_desired_state: + raise CalendarError( + "CalDAV source endpoint or calendar cannot change while unresolved local desired " + "changes exist; deliver or explicitly discard them first" + ) + if endpoint_or_calendar_changed and _caldav_source_has_linked_events( + session, + tenant_id=tenant_id, + source=source, + ): + raise CalendarError( + "CalDAV source endpoint or calendar cannot change while events still reference it; " + "detach or resync those events first" + ) + cancel_calendar_outbox_for_source( + session, + source_id=source.id, + reason="CalDAV source endpoint/calendar changed, was disabled, or was made inbound-only", + ) - if calendar_outbox_has_live_lease(session, source_id=source.id): - raise CalendarError( - "CalDAV source cannot be materially changed while an outbound operation has an active lease" - ) - unresolved_desired_state = calendar_outbox_has_unresolved_desired_state( +def _calendar_for_sync_source_update( + session: Session, + *, + tenant_id: str, + source: CalendarSyncSource, + calendar_id: str | None, +) -> CalendarCollection: + if calendar_id is None: + return get_calendar( session, - source_id=source.id, + tenant_id=tenant_id, + calendar_id=source.calendar_id, ) - stops_outbound_delivery = bool( - (payload.sync_enabled is False and source.sync_enabled) - or ( - payload.sync_direction == "inbound" - and source.sync_direction == "two_way" - ) + calendar = get_calendar(session, tenant_id=tenant_id, calendar_id=calendar_id) + calendar = ( + session.query(CalendarCollection) + .filter( + CalendarCollection.id == calendar.id, + CalendarCollection.tenant_id == tenant_id, + CalendarCollection.deleted_at.is_(None), ) - if stops_outbound_delivery and unresolved_desired_state: - raise CalendarError( - "CalDAV source cannot disable outbound delivery while unresolved local desired " - "changes exist; deliver or explicitly discard them first" - ) - if endpoint_or_calendar_changed and unresolved_desired_state: - raise CalendarError( - "CalDAV source endpoint or calendar cannot change while unresolved local desired " - "changes exist; deliver or explicitly discard them first" - ) + .populate_existing() + .with_for_update() + .one() + ) + conflicting_source = ( + session.query(CalendarSyncSource.id) + .filter( + CalendarSyncSource.tenant_id == tenant_id, + CalendarSyncSource.calendar_id == calendar.id, + CalendarSyncSource.id != source.id, + CalendarSyncSource.deleted_at.is_(None), + ) + .first() + ) + if conflicting_source is not None: + raise CalendarError("A calendar can have only one active synchronization source") + source.calendar_id = calendar.id + return calendar - if endpoint_or_calendar_changed: - sourced_event_exists = ( - session.query(CalendarEvent.id) - .filter( - CalendarEvent.tenant_id == tenant_id, - CalendarEvent.calendar_id == source.calendar_id, - CalendarEvent.source_kind == "caldav", - CalendarEvent.source_href.is_not(None), - CalendarEvent.deleted_at.is_(None), - ) - .first() - is not None - ) - if sourced_event_exists: - raise CalendarError( - "CalDAV source endpoint or calendar cannot change while events still reference it; " - "detach or resync those events first" - ) - cancel_calendar_outbox_for_source( - session, - source_id=source.id, - reason="CalDAV source endpoint/calendar changed, was disabled, or was made inbound-only", - ) - if payload.calendar_id is not None: - calendar = get_calendar(session, tenant_id=tenant_id, calendar_id=payload.calendar_id) - calendar = ( - session.query(CalendarCollection) - .filter( - CalendarCollection.id == calendar.id, - CalendarCollection.tenant_id == tenant_id, - CalendarCollection.deleted_at.is_(None), - ) - .populate_existing() - .with_for_update() - .one() - ) - conflicting_source = ( - session.query(CalendarSyncSource.id) - .filter( - CalendarSyncSource.tenant_id == tenant_id, - CalendarSyncSource.calendar_id == calendar.id, - CalendarSyncSource.id != source.id, - CalendarSyncSource.deleted_at.is_(None), - ) - .first() - ) - if conflicting_source is not None: - raise CalendarError( - "A calendar can have only one active synchronization source" - ) - source.calendar_id = calendar.id - else: - calendar = get_calendar(session, tenant_id=tenant_id, calendar_id=source.calendar_id) +def _apply_sync_source_values( + source: CalendarSyncSource, + *, + payload: CalendarSyncSourceUpdateRequest, + normalized_collection_url: str, +) -> None: for field_name in ( "display_name", "auth_type", @@ -1069,15 +1116,85 @@ def update_sync_source(session: Session, *, tenant_id: str, source_id: str, payl source.metadata_ = payload.metadata if source.source_kind in READ_ONLY_SYNC_SOURCE_KINDS: source.sync_direction = "inbound" + + +def _validate_sync_source_auth(source: CalendarSyncSource) -> None: if source.source_kind == "graph" and source.auth_type != "bearer": - raise CalendarError("Microsoft Graph calendar sync requires bearer token authentication") + raise CalendarError( + "Microsoft Graph calendar sync requires bearer token authentication" + ) if source.source_kind == "ews" and source.auth_type not in {"basic", "bearer"}: - raise CalendarError("Exchange Web Services sync requires basic or bearer authentication") - credential_value = caldav_secret_from_payload(auth_type=source.auth_type, password=payload.password, bearer_token=payload.bearer_token) + raise CalendarError( + "Exchange Web Services sync requires basic or bearer authentication" + ) + + +def _update_sync_source_credential_and_schedule( + session: Session, + *, + tenant_id: str, + source: CalendarSyncSource, + payload: CalendarSyncSourceUpdateRequest, +) -> None: + credential_value = caldav_secret_from_payload( + auth_type=source.auth_type, + password=payload.password, + bearer_token=payload.bearer_token, + ) if credential_value is not None: - source.credential_ref = store_caldav_credential(session, tenant_id=tenant_id, user_id=None, source=source, secret=credential_value) + source.credential_ref = store_caldav_credential( + session, + tenant_id=tenant_id, + user_id=None, + source=source, + secret=credential_value, + ) if payload.sync_enabled is not None or payload.sync_interval_seconds is not None: source.next_sync_at = utcnow() if source.sync_enabled else None + + +def update_sync_source(session: Session, *, tenant_id: str, source_id: str, payload: CalendarSyncSourceUpdateRequest) -> CalendarSyncSource: + source = get_sync_source(session, tenant_id=tenant_id, source_id=source_id) + source = _lock_sync_source_for_update( + session, + tenant_id=tenant_id, + source_id=source.id, + ) + if payload.credential_ref is not None and payload.credential_ref != source.credential_ref: + raise CalendarError( + "Caller-supplied credential references are not accepted; provide a replacement password or bearer token" + ) + ( + normalized_collection_url, + materially_reconfigured, + endpoint_or_calendar_changed, + ) = _sync_source_reconfiguration(source, payload) + if materially_reconfigured: + _guard_sync_source_reconfiguration( + session, + tenant_id=tenant_id, + source=source, + payload=payload, + endpoint_or_calendar_changed=endpoint_or_calendar_changed, + ) + calendar = _calendar_for_sync_source_update( + session, + tenant_id=tenant_id, + source=source, + calendar_id=payload.calendar_id, + ) + _apply_sync_source_values( + source, + payload=payload, + normalized_collection_url=normalized_collection_url, + ) + _validate_sync_source_auth(source) + _update_sync_source_credential_and_schedule( + session, + tenant_id=tenant_id, + source=source, + payload=payload, + ) mark_calendar_sync_source(calendar, source) session.flush() return source