Refactor calendar sync source updates

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:
+def _lock_sync_source_for_update(
-    source = get_sync_source(session, tenant_id=tenant_id, source_id=source_id)
+    session: Session,
-    source = (
+    *,
+    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)
+            calendar_changed
-            or normalized_collection_url != source.collection_url
+            or endpoint_changed
             or payload.sync_enabled is False
             or payload.sync_direction == "inbound"
         )
     )
-    endpoint_or_calendar_changed = bool(
+    return (
-        source.source_kind == "caldav"
+        normalized_collection_url,
-        and (
+        materially_reconfigured,
-            (payload.calendar_id is not None and payload.calendar_id != source.calendar_id)
+        endpoint_or_calendar_changed,
-            or normalized_collection_url != source.collection_url
+    )
+
+
+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:
+    if stops_outbound_delivery and unresolved_desired_state:
-        from govoplan_calendar.backend.outbox import (
+        raise CalendarError(
-            calendar_outbox_has_live_lease,
+            "CalDAV source cannot disable outbound delivery while unresolved local desired "
-            calendar_outbox_has_unresolved_desired_state,
+            "changes exist; deliver or explicitly discard them first"
-            cancel_calendar_outbox_for_source,
         )
+    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(
+    calendar = get_calendar(session, tenant_id=tenant_id, calendar_id=calendar_id)
-            (payload.sync_enabled is False and source.sync_enabled)
+    calendar = (
-            or (
+        session.query(CalendarCollection)
-                payload.sync_direction == "inbound"
+        .filter(
-                and source.sync_direction == "two_way"
+            CalendarCollection.id == calendar.id,
-            )
+            CalendarCollection.tenant_id == tenant_id,
+            CalendarCollection.deleted_at.is_(None),
         )
-        if stops_outbound_delivery and unresolved_desired_state:
+        .populate_existing()
-            raise CalendarError(
+        .with_for_update()
-                "CalDAV source cannot disable outbound delivery while unresolved local desired "
+        .one()
-                "changes exist; deliver or explicitly discard them first"
+    )
-            )
+    conflicting_source = (
-        if endpoint_or_calendar_changed and unresolved_desired_state:
+        session.query(CalendarSyncSource.id)
-            raise CalendarError(
+        .filter(
-                "CalDAV source endpoint or calendar cannot change while unresolved local desired "
+            CalendarSyncSource.tenant_id == tenant_id,
-                "changes exist; deliver or explicitly discard them first"
+            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(
+def _apply_sync_source_values(
-            session,
+    source: CalendarSyncSource,
-            source_id=source.id,
+    *,
-            reason="CalDAV source endpoint/calendar changed, was disabled, or was made inbound-only",
+    payload: CalendarSyncSourceUpdateRequest,
-        )
+    normalized_collection_url: str,
-    if payload.calendar_id is not None:
+) -> 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)
     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")
+        raise CalendarError(
-    credential_value = caldav_secret_from_payload(auth_type=source.auth_type, password=payload.password, bearer_token=payload.bearer_token)
+            "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