refactor: consolidate duplicated request-handling logic into BaseDAVClient

Extract four shared helpers from the identical/near-identical code that
existed in both _sync_request and _async_request:

  _prepare_request(url, method, body, headers)
      → (url_obj, combined_headers)
      Combines client headers with per-request headers, strips Content-Type
      for empty bodies, objectifies the URL, and emits the debug log line.
      Removes 7 identical lines from each client.

  _should_negotiate_auth(status_code, headers) -> bool
      The 401 + WWW-Authenticate + no-existing-auth + credentials-present
      condition, previously copy-pasted into both clients.

  _build_auth_from_401(www_authenticate)
      Calls extract_auth_types / build_auth_object and raises
      NotImplementedError if the server offers no supported method.
      Removes the identical 6-line block from each client.

  _raise_authorization_error(url_str, reason_source) -> NoReturn
      Extracts reason from reason_source.reason (AttributeError → "None
      given") and raises AuthorizationError.  Removes the identical
      try/except block from each client.

Side-effects:
- to_normal_str import removed from both davclient.py and async_davclient.py
  (now only imported in base_client.py where it is used)
- No behaviour changes; all 123 unit tests pass

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: tobixen

caldav/async_davclient.py

@@ -59,7 +59,7 @@
 from caldav.base_client import get_davclient as _base_get_davclient
 from caldav.compatibility_hints import FeatureSet
 from caldav.lib import error
-from caldav.lib.python_utilities import to_normal_str, to_wire
+from caldav.lib.python_utilities import to_wire
 from caldav.lib.url import URL
 from caldav.protocol.types import (
     CalendarQueryResult,
@@ -375,19 +375,7 @@ async def _async_request(
         Handles connection-abort workaround, 429/503 rate-limit detection,
         and 401 auth negotiation (including HTTP/2 fallback).
         """
-        headers = headers or {}
-
-        combined_headers = self.headers.copy()
-        combined_headers.update(headers)
-        if (body is None or body == "") and "Content-Type" in combined_headers:
-            del combined_headers["Content-Type"]
-
-        # Objectify the URL
-        url_obj = URL.objectify(url)
-
-        log.debug(
-            f"sending request - method={method}, url={str(url_obj)}, headers={combined_headers}\nbody:\n{to_normal_str(body)}"
-        )
+        url_obj, combined_headers = self._prepare_request(url, method, body, headers)
 
         # Build request kwargs - different for httpx vs niquests
         if _USE_HTTPX:
@@ -479,25 +467,9 @@ async def _async_request(
         # Handle 429/503 rate-limit responses
         error.raise_if_rate_limited(r.status_code, str(url_obj), r.headers.get("Retry-After"))
 
-        # Handle 401 responses for auth negotiation
-        # httpx headers are already case-insensitive
-        if (
-            r.status_code == 401
-            and "WWW-Authenticate" in r.headers
-            and not self.auth
-            and self.username is not None
-            and self.password is not None  # Empty password OK, but None means not configured
-        ):
-            auth_types = self.extract_auth_types(r.headers["WWW-Authenticate"])
-            self.build_auth_object(auth_types)
-
-            if not self.auth:
-                raise NotImplementedError(
-                    "The server does not provide any of the currently "
-                    "supported authentication methods: basic, digest, bearer"
-                )
-
-            # Retry request with authentication
+        # Handle 401: negotiate auth then retry
+        if self._should_negotiate_auth(r.status_code, r.headers):
+            self._build_auth_from_401(r.headers["WWW-Authenticate"])
             return await self._async_request(url, method, body, headers)
 
         elif (
@@ -530,11 +502,7 @@ async def _async_request(
 
         # Raise AuthorizationError for 401/403 responses
         if response.status in (401, 403):
-            try:
-                reason = response.reason
-            except AttributeError:
-                reason = "None given"
-            raise error.AuthorizationError(url=str(url_obj), reason=reason)
+            self._raise_authorization_error(str(url_obj), response)
 
         return response
 

caldav/base_client.py

@@ -7,15 +7,21 @@
 
 from __future__ import annotations
 
+import logging
 from abc import ABC, abstractmethod
-from typing import TYPE_CHECKING, Any
+from collections.abc import Mapping
+from typing import TYPE_CHECKING, Any, NoReturn
 
 from caldav.lib import error
 from caldav.lib.auth import extract_auth_types, select_auth_type
+from caldav.lib.python_utilities import to_normal_str
+from caldav.lib.url import URL
 
 if TYPE_CHECKING:
     from caldav.compatibility_hints import FeatureSet
 
+log = logging.getLogger("caldav")
+
 
 class BaseDAVClient(ABC):
     """
@@ -127,6 +133,66 @@ def _select_auth_type(self, auth_types: list[str] | None = None) -> str | None:
 
         return auth_type
 
+    def _prepare_request(
+        self,
+        url: str,
+        method: str,
+        body: str,
+        headers: Mapping[str, str] | None,
+    ) -> tuple[URL, dict]:
+        """Combine headers, strip Content-Type for empty bodies, objectify URL, and log.
+
+        Returns:
+            (url_obj, combined_headers) ready to pass to the HTTP library.
+        """
+        headers = headers or {}
+        combined_headers = self.headers.copy()
+        combined_headers.update(headers)
+        if (body is None or body == "") and "Content-Type" in combined_headers:
+            del combined_headers["Content-Type"]
+        url_obj = URL.objectify(url)
+        log.debug(
+            f"sending request - method={method}, url={str(url_obj)}, "
+            f"headers={combined_headers}\nbody:\n{to_normal_str(body)}"
+        )
+        return url_obj, combined_headers
+
+    def _should_negotiate_auth(self, status_code: int, headers: Any) -> bool:
+        """Return True when a 401 response warrants auth negotiation.
+
+        True when: status is 401, WWW-Authenticate header present, no auth
+        object yet, and credentials are configured.
+        """
+        return (
+            status_code == 401
+            and "WWW-Authenticate" in headers
+            and not self.auth
+            and self.username is not None
+            and self.password is not None
+        )
+
+    def _build_auth_from_401(self, www_authenticate: str) -> None:
+        """Build auth object from a WWW-Authenticate header value.
+
+        Raises:
+            NotImplementedError: If the server offers no supported auth method.
+        """
+        auth_types = self.extract_auth_types(www_authenticate)
+        self.build_auth_object(auth_types)
+        if not self.auth:
+            raise NotImplementedError(
+                "The server does not provide any of the currently "
+                "supported authentication methods: basic, digest, bearer"
+            )
+
+    def _raise_authorization_error(self, url_str: str, reason_source: Any) -> NoReturn:
+        """Raise AuthorizationError, extracting reason from reason_source.reason."""
+        try:
+            reason = reason_source.reason
+        except AttributeError:
+            reason = "None given"
+        raise error.AuthorizationError(url=url_str, reason=reason)
+
     @abstractmethod
     def build_auth_object(self, auth_types: list[str] | None = None) -> None:
         """

caldav/davclient.py

@@ -52,7 +52,7 @@
 from caldav.config import CONNKEYS  # noqa: F401
 from caldav.elements import cdav, dav
 from caldav.lib import error
-from caldav.lib.python_utilities import to_normal_str, to_wire
+from caldav.lib.python_utilities import to_wire
 from caldav.lib.url import URL
 from caldav.requests import HTTPBearerAuth
 from caldav.response import BaseDAVResponse
@@ -979,25 +979,13 @@ def _sync_request(
         """
         Sync HTTP request implementation with auth negotiation.
         """
-        headers = headers or {}
-
-        combined_headers = self.headers.copy()
-        combined_headers.update(headers or {})
-        if (body is None or body == "") and "Content-Type" in combined_headers:
-            del combined_headers["Content-Type"]
-
-        # objectify the url
-        url_obj = URL.objectify(url)
+        url_obj, combined_headers = self._prepare_request(url, method, body, headers)
 
         proxies = None
         if self.proxy is not None:
             proxies = {url_obj.scheme: self.proxy}
             log.debug("using proxy - %s" % (proxies))
 
-        log.debug(
-            f"sending request - method={method}, url={str(url_obj)}, headers={combined_headers}\nbody:\n{to_normal_str(body)}"
-        )
-
         r = self.session.request(
             method,
             str(url_obj),
@@ -1015,33 +1003,14 @@ def _sync_request(
         # Handle 429/503 responses: raise RateLimitError so the caller can decide whether to retry
         error.raise_if_rate_limited(r.status_code, str(url_obj), r_headers.get("Retry-After"))
 
-        # Handle 401 responses for auth negotiation
-        if (
-            r.status_code == 401
-            and "WWW-Authenticate" in r_headers
-            and not self.auth
-            and self.username is not None
-            and self.password is not None  # Empty password OK, but None means not configured
-        ):
-            auth_types = self.extract_auth_types(r_headers["WWW-Authenticate"])
-            self.build_auth_object(auth_types)
-
-            if not self.auth:
-                raise NotImplementedError(
-                    "The server does not provide any of the currently "
-                    "supported authentication methods: basic, digest, bearer"
-                )
-
-            # Retry request with authentication
+        # Handle 401: negotiate auth then retry
+        if self._should_negotiate_auth(r.status_code, r_headers):
+            self._build_auth_from_401(r_headers["WWW-Authenticate"])
             return self._sync_request(url, method, body, headers)
 
         # Raise AuthorizationError for 401/403 after auth attempt
         if r.status_code in (401, 403):
-            try:
-                reason = r.reason
-            except AttributeError:
-                reason = "None given"
-            raise error.AuthorizationError(url=str(url_obj), reason=reason)
+            self._raise_authorization_error(str(url_obj), r)
 
         response = DAVResponse(r, self)
         return response