mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-10 12:28:27 +00:00
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. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
e8ec6cc715
commit
7c90afca9e
6 files changed
+29
No files matched your search
@@ -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)
|
||||
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -34,6 +34,7 @@ _BASE_TAG = '<base href="/" data-shelfmark-base />'
|
||||
|
||||
|
||||
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 "/"
|
||||
|
||||
Reference in new issue
Block a user