refactor: split calendar source setup orchestration

src/govoplan_calendar/backend/router.py

@@ -99,6 +99,15 @@ from govoplan_core.db.session import get_session
 router = APIRouter(prefix="/calendar", tags=["calendar"])
 
 
+def _caldav_discovery_http_error(
+    exc: CalendarError | CalDAVError,
+) -> HTTPException:
+    return HTTPException(
+        status_code=status.HTTP_422_UNPROCESSABLE_CONTENT,
+        detail=str(exc),
+    )
+
+
 def _require_scope(principal: ApiPrincipal, scope: str) -> None:
     if not has_scope(principal, scope):
         raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=f"Missing scope: {scope}")
@@ -492,7 +501,7 @@ def api_discover_caldav_calendars(
         calendars = discover_caldav_calendars(session, tenant_id=principal.tenant_id, payload=payload)
         return CalendarCalDavDiscoveryResponse(calendars=calendars)
     except (CalendarError, CalDAVError) as exc:
-        raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, detail=str(exc)) from exc
+        raise _caldav_discovery_http_error(exc) from exc
 
 
 @router.post("/caldav/sources", response_model=CalendarCalDavSourceResponse, status_code=status.HTTP_201_CREATED)

src/govoplan_calendar/backend/service.py

@@ -200,6 +200,48 @@ class _RemoteSyncSnapshot:
     metadata: dict[str, Any]
 
 
+@dataclass(frozen=True, slots=True)
+class _CalDAVDiscoveryAuth:
+    auth_type: str
+    username: str | None
+    credential_ref: str | None
+    secret: str | None = field(repr=False)
+
+
+@dataclass(frozen=True, slots=True)
+class _SyncSourceCreationPlan:
+    source_kind: str
+    collection_url: str
+    sync_direction: str
+    auth_type: str
+    username: str | None
+    credential_ref: str | None
+    has_inline_secret: bool
+
+    def build_source(
+        self,
+        *,
+        tenant_id: str,
+        calendar_id: str,
+        payload: CalendarSyncSourceCreateRequest,
+    ) -> CalendarSyncSource:
+        return CalendarSyncSource(
+            tenant_id=tenant_id,
+            calendar_id=calendar_id,
+            source_kind=self.source_kind,
+            collection_url=self.collection_url,
+            display_name=payload.display_name,
+            auth_type=self.auth_type,
+            username=self.username,
+            credential_ref=self.credential_ref,
+            sync_enabled=payload.sync_enabled,
+            sync_interval_seconds=payload.sync_interval_seconds,
+            sync_direction=self.sync_direction,
+            conflict_policy=payload.conflict_policy,
+            metadata_=dict(payload.metadata),
+        )
+
+
 def slugify(value: str) -> str:
     slug = re.sub(r"[^a-z0-9]+", "-", value.lower()).strip("-")
     return slug or "calendar"
@@ -892,44 +934,149 @@ def get_caldav_source(session: Session, *, tenant_id: str, source_id: str) -> Ca
         raise CalendarError("CalDAV sync source not found") from exc
 
 
-def discover_caldav_calendars(session: Session, *, tenant_id: str, payload: CalendarCalDavDiscoveryRequest) -> list[dict[str, Any]]:
+def _resolve_caldav_discovery_auth(
-    source = get_caldav_source(session, tenant_id=tenant_id, source_id=payload.source_id) if payload.source_id else None
+    session: Session,
-    auth_type = payload.auth_type or (source.auth_type if source else "none")
+    *,
-    username = payload.username if payload.username is not None else (source.username if source else None)
+    tenant_id: str,
-    credential_ref = payload.credential_ref if payload.credential_ref is not None else (source.credential_ref if source else None)
+    payload: CalendarCalDavDiscoveryRequest,
-    resolved_envelope = _resolve_core_calendar_credential(
+    source: CalendarSyncSource | None,
+) -> _CalDAVDiscoveryAuth:
+    auth_type, username, credential_ref = _caldav_discovery_inputs(
+        payload,
+        source,
+    )
+    resolved_envelope = _caldav_discovery_envelope(
+        session,
+        tenant_id=tenant_id,
+        payload=payload,
+        source=source,
+        credential_ref=credential_ref,
+    )
+    username = username or _credential_username(resolved_envelope)
+    secret = _caldav_discovery_secret(
+        session,
+        payload=payload,
+        source=source,
+        auth_type=auth_type,
+        credential_ref=credential_ref,
+        resolved_envelope=resolved_envelope,
+    )
+    _validate_caldav_discovery_auth(
+        auth_type=auth_type,
+        username=username,
+        secret=secret,
+    )
+    return _CalDAVDiscoveryAuth(
+        auth_type=auth_type,
+        username=username,
+        credential_ref=credential_ref,
+        secret=secret,
+    )
+
+
+def _caldav_discovery_inputs(
+    payload: CalendarCalDavDiscoveryRequest,
+    source: CalendarSyncSource | None,
+) -> tuple[str, str | None, str | None]:
+    return (
+        payload.auth_type or (source.auth_type if source else "none"),
+        (
+            payload.username
+            if payload.username is not None
+            else (source.username if source else None)
+        ),
+        (
+            payload.credential_ref
+            if payload.credential_ref is not None
+            else (source.credential_ref if source else None)
+        ),
+    )
+
+
+def _caldav_discovery_envelope(
+    session: Session,
+    *,
+    tenant_id: str,
+    payload: CalendarCalDavDiscoveryRequest,
+    source: CalendarSyncSource | None,
+    credential_ref: str | None,
+) -> ResolvedCredentialEnvelope | None:
+    resolved = _resolve_core_calendar_credential(
         session,
         tenant_id=tenant_id,
         source_id=source.id if source else None,
         credential_ref=credential_ref,
     )
-    if payload.credential_ref is not None and resolved_envelope is None and (
+    if payload.credential_ref is not None and resolved is None and (
         source is None or payload.credential_ref != source.credential_ref
     ):
         raise CalendarError(
             "Caller-supplied credential references are accepted only for visible reusable credential envelopes"
         )
-    if not username and resolved_envelope is not None:
+    return resolved
-        username = _credential_username(resolved_envelope)
-    secret = caldav_secret_from_payload(auth_type=auth_type, password=payload.password, bearer_token=payload.bearer_token)
-    if secret is None and resolved_envelope is not None:
-        secret = _credential_secret(resolved_envelope, auth_type=auth_type)
-    elif secret is None and source is not None and credential_ref == source.credential_ref:
-        secret = resolve_caldav_secret(session, source=source)
 
+
+def _caldav_discovery_secret(
+    session: Session,
+    *,
+    payload: CalendarCalDavDiscoveryRequest,
+    source: CalendarSyncSource | None,
+    auth_type: str,
+    credential_ref: str | None,
+    resolved_envelope: ResolvedCredentialEnvelope | None,
+) -> str | None:
+    secret = caldav_secret_from_payload(
+        auth_type=auth_type,
+        password=payload.password,
+        bearer_token=payload.bearer_token,
+    )
+    if secret is not None:
+        return secret
+    if resolved_envelope is not None:
+        return _credential_secret(resolved_envelope, auth_type=auth_type)
+    if source is not None and credential_ref == source.credential_ref:
+        return resolve_caldav_secret(session, source=source)
+    return None
+
+
+def _validate_caldav_discovery_auth(
+    *,
+    auth_type: str,
+    username: str | None,
+    secret: str | None,
+) -> None:
     if auth_type == "basic":
         if not username:
             raise CalendarError("CalDAV discovery with basic auth requires a username")
         if not secret:
             raise CalendarError("CalDAV discovery with basic auth requires a password or credential reference")
-        client = CalDAVClient(collection_url=payload.url, username=username, password=secret)
+    if auth_type == "bearer" and not secret:
-    elif auth_type == "bearer":
+        raise CalendarError(
-        if not secret:
+            "CalDAV discovery with bearer auth requires a token or credential reference"
-            raise CalendarError("CalDAV discovery with bearer auth requires a token or credential reference")
+        )
-        client = CalDAVClient(collection_url=payload.url, bearer_token=secret)
-    else:
-        client = CalDAVClient(collection_url=payload.url)
 
+
+def _caldav_discovery_client(
+    url: str,
+    auth: _CalDAVDiscoveryAuth,
+) -> CalDAVClient:
+    if auth.auth_type == "basic":
+        return CalDAVClient(
+            collection_url=url,
+            username=auth.username,
+            password=auth.secret,
+        )
+    if auth.auth_type == "bearer":
+        return CalDAVClient(
+            collection_url=url,
+            bearer_token=auth.secret,
+        )
+    return CalDAVClient(collection_url=url)
+
+
+def _caldav_discovery_response(
+    calendars: Iterable[object],
+) -> list[dict[str, Any]]:
     return [
         {
             "collection_url": calendar.collection_url,
@@ -939,21 +1086,106 @@ def discover_caldav_calendars(session: Session, *, tenant_id: str, payload: Cale
             "ctag": calendar.ctag,
             "sync_token": calendar.sync_token,
         }
-        for calendar in client.discover_calendars()
+        for calendar in calendars
     ]
 
 
-def create_sync_source(session: Session, *, tenant_id: str, user_id: str | None, payload: CalendarSyncSourceCreateRequest) -> CalendarSyncSource:
+def discover_caldav_calendars(
-    if payload.credential_ref is not None and not _core_credential_id(payload.credential_ref):
+    session: Session,
+    *,
+    tenant_id: str,
+    payload: CalendarCalDavDiscoveryRequest,
+) -> list[dict[str, Any]]:
+    source = (
+        get_caldav_source(
+            session,
+            tenant_id=tenant_id,
+            source_id=payload.source_id,
+        )
+        if payload.source_id
+        else None
+    )
+    auth = _resolve_caldav_discovery_auth(
+        session,
+        tenant_id=tenant_id,
+        payload=payload,
+        source=source,
+    )
+    client = _caldav_discovery_client(payload.url, auth)
+    return _caldav_discovery_response(client.discover_calendars())
+
+
+def _plan_sync_source_creation(
+    payload: CalendarSyncSourceCreateRequest,
+) -> _SyncSourceCreationPlan:
+    if payload.credential_ref is not None and not _core_credential_id(
+        payload.credential_ref
+    ):
         raise CalendarError(
             "Caller-supplied credential references are accepted only for visible reusable credential envelopes"
         )
-    if payload.credential_ref is not None and (
+    has_inline_secret = (
         payload.password is not None or payload.bearer_token is not None
-    ):
+    )
-        raise CalendarError("Select a reusable credential or enter a new secret, not both")
+    if payload.credential_ref is not None and has_inline_secret:
-    calendar = get_calendar(session, tenant_id=tenant_id, calendar_id=payload.calendar_id)
+        raise CalendarError(
-    calendar = (
+            "Select a reusable credential or enter a new secret, not both"
+        )
+    source_kind = normalize_source_kind(payload.source_kind)
+    collection_url = normalize_sync_source_url(
+        source_kind,
+        payload.collection_url,
+    )
+    _validate_sync_source_create_auth(source_kind, payload.auth_type)
+    return _SyncSourceCreationPlan(
+        source_kind=source_kind,
+        collection_url=collection_url,
+        sync_direction=(
+            "inbound"
+            if source_kind in READ_ONLY_SYNC_SOURCE_KINDS
+            else payload.sync_direction
+        ),
+        auth_type=payload.auth_type,
+        username=payload.username,
+        credential_ref=payload.credential_ref,
+        has_inline_secret=has_inline_secret,
+    )
+
+
+def _validate_sync_source_create_auth(
+    source_kind: str,
+    auth_type: str,
+) -> None:
+    if source_kind == "graph" and auth_type != "bearer":
+        raise CalendarError(
+            "Microsoft Graph calendar sync requires bearer token authentication"
+        )
+    if source_kind in {"ics", "webcal"} and auth_type not in {
+        "none",
+        "basic",
+        "bearer",
+    }:
+        raise CalendarError(
+            "ICS/webcal subscriptions support none, basic, or bearer authentication"
+        )
+    if source_kind == "ews" and auth_type not in {"basic", "bearer"}:
+        raise CalendarError(
+            "Exchange Web Services sync requires basic or bearer authentication"
+        )
+
+
+def _lock_calendar_for_sync_source(
+    session: Session,
+    *,
+    tenant_id: str,
+    calendar_id: str,
+) -> CalendarCollection:
+    calendar = get_calendar(
+        session,
+        tenant_id=tenant_id,
+        calendar_id=calendar_id,
+    )
+    return (
         session.query(CalendarCollection)
         .filter(
             CalendarCollection.id == calendar.id,
@@ -964,51 +1196,68 @@ def create_sync_source(session: Session, *, tenant_id: str, user_id: str | None,
         .with_for_update()
         .one()
     )
-    existing_calendar_source = (
+
+
+def _ensure_calendar_has_no_sync_source(
+    session: Session,
+    *,
+    tenant_id: str,
+    calendar_id: str,
+) -> None:
+    existing = (
         session.query(CalendarSyncSource.id)
         .filter(
             CalendarSyncSource.tenant_id == tenant_id,
-            CalendarSyncSource.calendar_id == calendar.id,
+            CalendarSyncSource.calendar_id == calendar_id,
             CalendarSyncSource.deleted_at.is_(None),
         )
         .first()
     )
-    if existing_calendar_source is not None:
+    if existing is not None:
         raise CalendarError(
             "A calendar can have only one active synchronization source"
         )
-    source_kind = normalize_source_kind(payload.source_kind)
+
-    collection_url = normalize_sync_source_url(source_kind, payload.collection_url)
+
-    retire_stale_sync_sources_for_url(session, tenant_id=tenant_id, source_kind=source_kind, collection_url=collection_url)
+def _ensure_sync_source_url_available(
-    existing = active_sync_source_for_url(session, tenant_id=tenant_id, source_kind=source_kind, collection_url=collection_url)
+    session: Session,
-    if existing is not None:
+    *,
-        existing_calendar = existing.calendar
+    tenant_id: str,
-        calendar_name = existing_calendar.name if existing_calendar and existing_calendar.deleted_at is None else "another calendar"
+    plan: _SyncSourceCreationPlan,
-        raise CalendarError(f"{sync_source_label(source_kind)} source is already linked to {calendar_name}")
+) -> None:
-    sync_direction = "inbound" if source_kind in READ_ONLY_SYNC_SOURCE_KINDS else payload.sync_direction
+    retire_stale_sync_sources_for_url(
-    if source_kind == "graph" and payload.auth_type != "bearer":
+        session,
-        raise CalendarError("Microsoft Graph calendar sync requires bearer token authentication")
-    if source_kind in {"ics", "webcal"} and payload.auth_type not in {"none", "basic", "bearer"}:
-        raise CalendarError("ICS/webcal subscriptions support none, basic, or bearer authentication")
-    if source_kind == "ews" and payload.auth_type not in {"basic", "bearer"}:
-        raise CalendarError("Exchange Web Services sync requires basic or bearer authentication")
-    source = CalendarSyncSource(
         tenant_id=tenant_id,
-        calendar_id=calendar.id,
+        source_kind=plan.source_kind,
-        source_kind=source_kind,
+        collection_url=plan.collection_url,
-        collection_url=collection_url,
-        display_name=payload.display_name,
-        auth_type=payload.auth_type,
-        username=payload.username,
-        credential_ref=payload.credential_ref,
-        sync_enabled=payload.sync_enabled,
-        sync_interval_seconds=payload.sync_interval_seconds,
-        sync_direction=sync_direction,
-        conflict_policy=payload.conflict_policy,
-        metadata_=payload.metadata,
     )
-    session.add(source)
+    existing = active_sync_source_for_url(
-    session.flush()
+        session,
+        tenant_id=tenant_id,
+        source_kind=plan.source_kind,
+        collection_url=plan.collection_url,
+    )
+    if existing is None:
+        return
+    existing_calendar = existing.calendar
+    calendar_name = (
+        existing_calendar.name
+        if existing_calendar and existing_calendar.deleted_at is None
+        else "another calendar"
+    )
+    raise CalendarError(
+        f"{sync_source_label(plan.source_kind)} source is already linked to {calendar_name}"
+    )
+
+
+def _persist_sync_source_credential(
+    session: Session,
+    *,
+    tenant_id: str,
+    user_id: str | None,
+    source: CalendarSyncSource,
+    payload: CalendarSyncSourceCreateRequest,
+) -> None:
     if source.credential_ref:
         _require_visible_core_calendar_credential(
             session,
@@ -1023,10 +1272,58 @@ def create_sync_source(session: Session, *, tenant_id: str, user_id: str | None,
                 source_id=source.id,
                 credential_ref=source.credential_ref,
             )
-            source.username = _credential_username(resolved) if resolved is not None else None
+            source.username = _credential_username(resolved)
-    credential_value = caldav_secret_from_payload(auth_type=source.auth_type, password=payload.password, bearer_token=payload.bearer_token)
+    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=user_id, source=source, secret=credential_value)
+        source.credential_ref = store_caldav_credential(
+            session,
+            tenant_id=tenant_id,
+            user_id=user_id,
+            source=source,
+            secret=credential_value,
+        )
+
+def create_sync_source(
+    session: Session,
+    *,
+    tenant_id: str,
+    user_id: str | None,
+    payload: CalendarSyncSourceCreateRequest,
+) -> CalendarSyncSource:
+    plan = _plan_sync_source_creation(payload)
+    calendar = _lock_calendar_for_sync_source(
+        session,
+        tenant_id=tenant_id,
+        calendar_id=payload.calendar_id,
+    )
+    _ensure_calendar_has_no_sync_source(
+        session,
+        tenant_id=tenant_id,
+        calendar_id=calendar.id,
+    )
+    _ensure_sync_source_url_available(
+        session,
+        tenant_id=tenant_id,
+        plan=plan,
+    )
+    source = plan.build_source(
+        tenant_id=tenant_id,
+        calendar_id=calendar.id,
+        payload=payload,
+    )
+    session.add(source)
+    session.flush()
+    _persist_sync_source_credential(
+        session,
+        tenant_id=tenant_id,
+        user_id=user_id,
+        source=source,
+        payload=payload,
+    )
     source.next_sync_at = utcnow() if source.sync_enabled else None
     mark_calendar_sync_source(calendar, source)
     session.flush()

tests/test_caldav.py

@@ -8,7 +8,18 @@ from sqlalchemy import create_engine, inspect
 from sqlalchemy.orm import sessionmaker
 
 from govoplan_access.backend.db import models as access_models  # noqa: F401 - populate users/accounts tables
-from govoplan_calendar.backend.caldav import CalDAVClient, CalDAVError, CalDAVNotFound, CalDAVObject, CalDAVPreconditionFailed, CalDAVReportResult, CalDAVWriteResult, parse_multistatus
+from govoplan_calendar.backend.caldav import (
+    CalDAVClient,
+    CalDAVDiscoveryCalendar,
+    CalDAVError,
+    CalDAVNotFound,
+    CalDAVObject,
+    CalDAVPreconditionFailed,
+    CalDAVReportResult,
+    CalDAVWriteResult,
+    parse_discovery_multistatus,
+    parse_multistatus,
+)
 from govoplan_calendar.backend.db.models import CalendarEvent, CalendarOutboxOperation, CalendarSyncCredential
 from govoplan_calendar.backend.manifest import manifest
 from govoplan_calendar.backend.outbox import dispatch_calendar_outbox
@@ -23,6 +34,10 @@ from govoplan_calendar.backend.schemas import (
 from govoplan_calendar.backend.service import (
     CALDAV_INTERNAL_CREDENTIAL_PREFIX,
     CalendarError,
+    _CalDAVDiscoveryAuth,
+    _caldav_discovery_client,
+    _caldav_discovery_response,
+    _plan_sync_source_creation,
     caldav_client_for_source,
     caldav_source_response,
     create_calendar,
@@ -41,6 +56,7 @@ from govoplan_calendar.backend.service import (
     update_caldav_source,
     update_event,
 )
+from govoplan_calendar.backend.router import _caldav_discovery_http_error
 from govoplan_core.db.base import Base
 from govoplan_core.security.credential_envelopes import create_credential_envelope
 from govoplan_core.tenancy.scope import create_scope_tables
@@ -108,6 +124,84 @@ class FakeNotificationProvider:
 
 
 class CalDAVParsingTests(unittest.TestCase):
+    def test_discovery_parser_is_independent_from_transport(self) -> None:
+        result = parse_discovery_multistatus(
+            b"""<?xml version="1.0" encoding="utf-8"?>
+<D:multistatus xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
+  <D:response>
+    <D:href>/calendars/ada/work/</D:href>
+    <D:propstat>
+      <D:prop>
+        <D:displayname>Work</D:displayname>
+        <D:resourcetype><D:collection/><C:calendar/></D:resourcetype>
+        <C:supported-calendar-component-set>
+          <C:comp name="VEVENT"/>
+        </C:supported-calendar-component-set>
+      </D:prop>
+      <D:status>HTTP/1.1 200 OK</D:status>
+    </D:propstat>
+  </D:response>
+</D:multistatus>"""
+        )
+
+        self.assertEqual(1, len(result))
+        self.assertEqual("Work", result[0].display_name)
+        self.assertTrue(result[0].is_calendar)
+        self.assertEqual(("VEVENT",), result[0].supported_components)
+
+    def test_discovery_auth_client_and_response_do_not_expose_secret(self) -> None:
+        auth = _CalDAVDiscoveryAuth(
+            auth_type="basic",
+            username="ada",
+            credential_ref=None,
+            secret="calendar-secret",
+        )
+        client = _caldav_discovery_client(
+            "https://dav.example.test/cal",
+            auth,
+        )
+        response = _caldav_discovery_response(
+            (
+                CalDAVDiscoveryCalendar(
+                    collection_url="https://dav.example.test/cal/work/",
+                    href="/cal/work/",
+                    display_name="Work",
+                ),
+            )
+        )
+
+        self.assertNotIn("calendar-secret", repr(auth))
+        self.assertEqual("calendar-secret", client.password)
+        self.assertEqual("Work", response[0]["display_name"])
+        self.assertNotIn("password", response[0])
+        self.assertNotIn("credential_ref", response[0])
+
+    def test_discovery_error_translation_is_stable(self) -> None:
+        error = _caldav_discovery_http_error(
+            CalDAVError("CalDAV endpoint rejected PROPFIND")
+        )
+
+        self.assertEqual(422, error.status_code)
+        self.assertEqual(
+            "CalDAV endpoint rejected PROPFIND",
+            error.detail,
+        )
+
+    def test_sync_source_creation_plan_is_secret_free(self) -> None:
+        plan = _plan_sync_source_creation(
+            CalendarCalDavSourceCreateRequest(
+                calendar_id="calendar-1",
+                collection_url="dav.example.test/cal",
+                auth_type="basic",
+                username="ada",
+                password="calendar-secret",
+            )
+        )
+
+        self.assertEqual("https://dav.example.test/cal/", plan.collection_url)
+        self.assertTrue(plan.has_inline_secret)
+        self.assertNotIn("calendar-secret", repr(plan))
+
     def test_parse_multistatus_extracts_objects_sync_token_and_deletions(self) -> None:
         result = parse_multistatus(
             b"""<?xml version="1.0" encoding="utf-8"?>