From 7c90afca9e036ca998aab855136b4ba04c9ff4dc Mon Sep 17 00:00:00 2001 From: splitsec2 <35583321+splitsec2@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:44:49 -0600 Subject: [PATCH] docs(core): docstrings for the functions the route moves left without one (#1660) Follow-up to the route moves out of main.py. CodeRabbit's docstring coverage check flagged #1637 to #1639 at 63%, so I went back over the modules the moves created and filled in the 19 functions and nested helpers that had none, in queue_routes, release_routes, download_recording, system_routes, metadata_routes and auth_middleware. Comments only, nothing else changes. The route modules that were split out earlier (request_routes, activity_routes and a few others) have more without docstrings, but I left those alone since that code was not part of the moves. ## Summary by CodeRabbit * **Documentation** * Expanded and clarified descriptions of error responses, download and queue statuses, notification handling, queue ownership and visibility, release-policy checks, and administrative access. * No user-facing behavior changes are included in this update. --- shelfmark/core/auth_middleware.py | 1 + shelfmark/core/download_recording.py | 11 +++++++++++ shelfmark/core/metadata_routes.py | 3 +++ shelfmark/core/queue_routes.py | 7 +++++++ shelfmark/core/release_routes.py | 4 ++++ shelfmark/core/system_routes.py | 3 +++ 6 files changed, 29 insertions(+) diff --git a/shelfmark/core/auth_middleware.py b/shelfmark/core/auth_middleware.py index 119e4f8e..14650200 100644 --- a/shelfmark/core/auth_middleware.py +++ b/shelfmark/core/auth_middleware.py @@ -29,6 +29,7 @@ _OPERATIONAL_ERRORS = (OSError, RuntimeError, TypeError, ValueError, sqlite3.Err def _raise_runtime_error(message: str) -> NoReturn: + """Raise RuntimeError with the message, for raising inside a try block (ruff TRY301).""" raise RuntimeError(message) diff --git a/shelfmark/core/download_recording.py b/shelfmark/core/download_recording.py index 5d400787..af0e39b6 100644 --- a/shelfmark/core/download_recording.py +++ b/shelfmark/core/download_recording.py @@ -39,10 +39,12 @@ _OPERATIONAL_ERRORS = (OSError, RuntimeError, TypeError, ValueError, sqlite3.Err def _queue_status_to_final_activity_status(status: QueueStatus) -> str | None: + """The status to record in download history, or None when the download has not finished.""" return status.value if status in TERMINAL_QUEUE_STATUSES else None def _queue_status_to_notification_event(status: QueueStatus) -> NotificationEvent | None: + """The notification for a finished download, or None for a status that sends none.""" if status == QueueStatus.COMPLETE: return NotificationEvent.DOWNLOAD_COMPLETE if status == QueueStatus.ERROR: @@ -53,6 +55,10 @@ def _queue_status_to_notification_event(status: QueueStatus) -> NotificationEven def _notify_admin_for_terminal_download_status( *, task_id: str, status: QueueStatus, task: Any ) -> None: + """Notify the admins, and the user who owns the task, that a download completed or failed. + + A notifier that raises is logged and does not stop the other. + """ event = _queue_status_to_notification_event(status) if event is None: return @@ -112,6 +118,7 @@ def register_download_recording( """Hook the download queue so every queued and finished download is recorded.""" def _emit_activity_update_for_task(*, payload: dict[str, Any], task: Any) -> None: + """Send an activity update to the admins and to the user who owns the task.""" owner_user_id = normalize_positive_int(getattr(task, "user_id", None)) emit_ws_event( ws_manager, @@ -198,6 +205,10 @@ def register_download_recording( logger.warning("Failed to reset activity viewer state for task %s: %s", task_id, exc) def _record_download_terminal_snapshot(task_id: str, status: QueueStatus, task: Any) -> None: + """Record a finished download: notify, finalize its history and tell the Activity view. + + A failed download that cannot be retried also reopens the request it was fulfilling. + """ _notify_admin_for_terminal_download_status(task_id=task_id, status=status, task=task) final_status = _queue_status_to_final_activity_status(status) diff --git a/shelfmark/core/metadata_routes.py b/shelfmark/core/metadata_routes.py index ce11d82c..6696de3f 100644 --- a/shelfmark/core/metadata_routes.py +++ b/shelfmark/core/metadata_routes.py @@ -62,8 +62,11 @@ def _handle_target_errors( def decorator( fn: Callable[..., Response | tuple[Response, int]], ) -> Callable[..., Response | tuple[Response, int]]: + """Wrap one route.""" + @wraps(fn) def wrapper(*args: object, **kwargs: object) -> Response | tuple[Response, int]: + """Map errors to responses: bad input 400, provider failure 502, anything else 500.""" try: return fn(*args, **kwargs) except (NotImplementedError, ValueError) as e: diff --git a/shelfmark/core/queue_routes.py b/shelfmark/core/queue_routes.py index c5ddf233..1e39a78a 100644 --- a/shelfmark/core/queue_routes.py +++ b/shelfmark/core/queue_routes.py @@ -42,6 +42,7 @@ _IMPORT_OPERATIONAL_ERRORS = (ImportError, *_OPERATIONAL_ERRORS) def _task_owned_by_actor( task: Any, *, actor_user_id: int | None, actor_username: str | None ) -> bool: + """Whether a queue task belongs to the actor: by user id when both have one, else by username.""" raw_task_user_id = getattr(task, "user_id", None) try: task_user_id = int(raw_task_user_id) if raw_task_user_id is not None else None @@ -64,6 +65,7 @@ def _download_row_owned_by_actor( actor_user_id: int | None, actor_username: str | None, ) -> bool: + """Whether a download history row belongs to the actor, matched the same way as a task.""" owner_user_id = normalize_positive_int(row.get("user_id")) if actor_user_id is not None and owner_user_id is not None: return owner_user_id == actor_user_id @@ -82,6 +84,7 @@ def _queue_task_visible_to_actor( actor_user_id: int | None, actor_username: str | None, ) -> bool: + """Whether the actor may see a live queue task: admins see all, others only their own.""" if is_admin: return True @@ -144,6 +147,10 @@ def register_queue_routes( """ def _resolve_queue_actor() -> tuple[bool, int | None, str | None, Response | None]: + """Return ``(is_admin, db_user_id, username, error)`` for the session. + + ``error`` is a response to send back when a non-admin has no usable identity. + """ is_admin, db_user_id, can_access_status = resolve_status_scope() actor_username = session.get("user_id") normalized_actor_username = actor_username if isinstance(actor_username, str) else None diff --git a/shelfmark/core/release_routes.py b/shelfmark/core/release_routes.py index d7899752..7d70042f 100644 --- a/shelfmark/core/release_routes.py +++ b/shelfmark/core/release_routes.py @@ -50,6 +50,7 @@ _AUDIOBOOK_FORMAT_HINTS = frozenset(AUDIOBOOK_FORMATS) def _contains_audiobook_format_hint(value: Any) -> bool: + """Whether a string names an audiobook format as one of its words (m4b, mp3 and the like).""" if not isinstance(value, str): return False @@ -110,6 +111,7 @@ def _resolve_release_content_type(data: dict[str, Any], source: Any) -> tuple[st def _policy_block_response( mode: PolicyMode, *, error: str = "Download not allowed by policy" ) -> tuple[Response, int]: + """Build the 403 for a request the user's policy mode refuses, with the mode they need.""" logger.debug( "download policy guard user=%s db_user_id=%s mode=%s", session.get("user_id"), @@ -148,6 +150,7 @@ _RELEASE_LISTING_REFUSED_MODES = frozenset({PolicyMode.REQUEST_BOOK, PolicyMode. def _release_listing_block_response(mode: PolicyMode) -> tuple[Response, int]: + """Build the 403 for a release listing the user's policy mode refuses.""" return _policy_block_response(mode, error="Release listing not allowed by policy") @@ -161,6 +164,7 @@ class _ReleasePolicy: get_user_db: Callable[[], UserDB | None], load_policy_settings: Callable[[], dict[str, Any]], ) -> None: + """Take the lookups as callables, so each check reads the current settings.""" self._resolve_auth_mode = resolve_auth_mode self._get_user_db = get_user_db self._load_policy_settings = load_policy_settings diff --git a/shelfmark/core/system_routes.py b/shelfmark/core/system_routes.py index 24f96405..579a0358 100644 --- a/shelfmark/core/system_routes.py +++ b/shelfmark/core/system_routes.py @@ -34,6 +34,7 @@ _BASE_TAG = '' def _raise_runtime_error(message: str) -> NoReturn: + """Raise RuntimeError with the message, for raising inside a try block (ruff TRY301).""" raise RuntimeError(message) @@ -76,6 +77,7 @@ def register_system_routes( @login_required @wraps(f) def decorated_function(*args: object, **kwargs: object) -> Response | tuple[Response, int]: + """Return 403 to a signed-in user who is not an admin, otherwise run the route.""" if resolve_auth_mode() != "none" and not session.get("is_admin", False): return jsonify({"error": "Admin access required"}), 403 return f(*args, **kwargs) @@ -83,6 +85,7 @@ def register_system_routes( return decorated_function def _base_href() -> str: + """The href for the page's base tag: the configured base path with a trailing slash.""" base_path = get_base_path() if not base_path: return "/"