diff --git a/Dockerfile b/Dockerfile index 00bf0d65..8fc8ce0c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -68,7 +68,7 @@ RUN apt-get update && \ # For debug zip iputils-ping \ # For user switching - sudo \ + gosu \ # --- Tor support (activated via USING_TOR=true) --- tor \ supervisor \ @@ -151,8 +151,7 @@ RUN --mount=type=cache,target=/root/.cache/pip \ pip install -r requirements-shelfmark.txt # Grant read/execute permissions to others -RUN chmod -R o+rx /usr/bin/chromium && \ - chmod -R o+rwx /usr/local/lib/python3.10/site-packages/seleniumbase/drivers/ +RUN chmod -R o+rx /usr/bin/chromium # Default command to run the application entrypoint script CMD ["/app/entrypoint.sh"] diff --git a/compose/docker-compose.lite.yml b/compose/docker-compose.lite.yml index e0a1bddb..4fc1fb36 100644 --- a/compose/docker-compose.lite.yml +++ b/compose/docker-compose.lite.yml @@ -1,6 +1,7 @@ services: shelfmark-lite: image: ghcr.io/calibrain/shelfmark-lite:latest + container_name: shelfmark-lite environment: # EXT_BYPASSER_URL: http://flaresolverr:8191 #If using Flaresolverr PUID: 1000 @@ -12,4 +13,4 @@ services: - /path/to/books:/books # Default destination for book downloads - /path/to/config:/config # App configuration # Required for torrent / usenet - path must match your download client's volume exactly - # - /path/to/downloads:/path/to/downloads \ No newline at end of file + # - /path/to/downloads:/path/to/downloads diff --git a/entrypoint.sh b/entrypoint.sh index 3532cbc3..64114ae2 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -45,9 +45,9 @@ if is_truthy "$ENABLE_LOGGING_VALUE"; then LOG_DIR=${LOG_ROOT:-/var/log/}/shelfmark mkdir -p "$LOG_DIR" LOG_FILE="${LOG_DIR}/shelfmark_entrypoint.log" - - # Cleanup any existing files or folders in the log directory - rm -rf "$LOG_DIR"/* + # Keep the previous entrypoint log instead of deleting all history on boot. + [ -f "${LOG_FILE}.prev" ] && rm -f "${LOG_FILE}.prev" + [ -f "$LOG_FILE" ] && mv "$LOG_FILE" "${LOG_FILE}.prev" fi ( @@ -127,14 +127,26 @@ USERNAME=$(getent passwd "$RUN_UID" | cut -d: -f1) echo "Username for UID $RUN_UID is $USERNAME" test_write() { - folder=$1 - test_file=$folder/shelfmark_TEST_WRITE - mkdir -p $folder - ( - echo 0123456789_TEST | sudo -E -u "$USERNAME" HOME=/app tee $test_file > /dev/null - ) - FILE_CONTENT=$(cat $test_file || echo "") - rm -f $test_file + local folder=$1 + local test_file="$folder/shelfmark_TEST_WRITE" + local FILE_CONTENT + local result + local result_text + + if ! mkdir -p "$folder"; then + echo "Failed to create directory for write test: $folder" + return 1 + fi + + if ! ( + echo 0123456789_TEST | gosu "$USERNAME" env HOME=/app tee "$test_file" > /dev/null + ); then + echo "Failed to write test file in $folder as $USERNAME" + return 1 + fi + + FILE_CONTENT=$(cat "$test_file" 2>/dev/null || echo "") + rm -f "$test_file" [ "$FILE_CONTENT" = "0123456789_TEST" ] result=$? if [ $result -eq 0 ]; then @@ -342,4 +354,4 @@ echo "Setting umask to $UMASK_VALUE" umask $UMASK_VALUE stop_file_logging -exec sudo -E -u "$USERNAME" HOME=/app $command +exec gosu "$USERNAME" env HOME=/app $command diff --git a/shelfmark/config/security.py b/shelfmark/config/security.py index de816074..ccce680d 100644 --- a/shelfmark/config/security.py +++ b/shelfmark/config/security.py @@ -97,6 +97,15 @@ def security_settings(): default="none", env_supported=False, ), + CustomComponentField( + key="builtin_admin_requirement", + component="oidc_admin_hint", + label=( + "Local authentication is inactive until a local admin account with a " + "password is created." + ), + show_when=_auth_condition("builtin"), + ), CustomComponentField( key="oidc_admin_requirement", component="oidc_admin_hint", diff --git a/shelfmark/core/activity_routes.py b/shelfmark/core/activity_routes.py index 6d9cd249..f2626aad 100644 --- a/shelfmark/core/activity_routes.py +++ b/shelfmark/core/activity_routes.py @@ -8,12 +8,11 @@ from flask import Flask, jsonify, request, session from shelfmark.core.activity_service import ActivityService from shelfmark.core.logger import setup_logger -from shelfmark.core.user_db import UserDB +from shelfmark.core.request_helpers import extract_release_source_id +from shelfmark.core.user_db import NO_AUTH_ACTIVITY_USERNAME, UserDB logger = setup_logger(__name__) -_NO_AUTH_ACTIVITY_USERNAME = "__shelfmark_noauth_activity__" - def _require_authenticated(resolve_auth_mode: Callable[[], str]): auth_mode = resolve_auth_mode() @@ -95,18 +94,18 @@ def _resolve_db_user_id( def _ensure_no_auth_activity_user_id(user_db: UserDB) -> int | None: """Resolve a stable users.db identity for no-auth activity state.""" try: - user = user_db.get_user(username=_NO_AUTH_ACTIVITY_USERNAME) + user = user_db.get_user(username=NO_AUTH_ACTIVITY_USERNAME) if user is None: try: user_db.create_user( - username=_NO_AUTH_ACTIVITY_USERNAME, + username=NO_AUTH_ACTIVITY_USERNAME, display_name="No-auth Activity", role="admin", ) except ValueError: # Another request may have created it between lookup and insert. pass - user = user_db.get_user(username=_NO_AUTH_ACTIVITY_USERNAME) + user = user_db.get_user(username=NO_AUTH_ACTIVITY_USERNAME) if user is None: return None @@ -168,6 +167,8 @@ def _list_admin_user_ids(user_db: UserDB) -> list[int]: for user in users: if not isinstance(user, dict): continue + if str(user.get("username") or "").strip() == NO_AUTH_ACTIVITY_USERNAME: + continue role = str(user.get("role") or "").strip().lower() if role != "admin": continue @@ -279,14 +280,7 @@ def _collect_active_download_item_keys(status: dict[str, dict[str, Any]]) -> set def _extract_request_source_id(row: dict[str, Any]) -> str | None: - release_data = row.get("release_data") - if not isinstance(release_data, dict): - return None - source_id = release_data.get("source_id") - if not isinstance(source_id, str): - return None - normalized = source_id.strip() - return normalized or None + return extract_release_source_id(row.get("release_data")) def _request_terminal_status(row: dict[str, Any]) -> str | None: diff --git a/shelfmark/core/admin_routes.py b/shelfmark/core/admin_routes.py index fb3e9365..47d83ad7 100644 --- a/shelfmark/core/admin_routes.py +++ b/shelfmark/core/admin_routes.py @@ -9,7 +9,7 @@ import os import sqlite3 from typing import Any -from flask import Flask, jsonify, request, session +from flask import Flask, g, jsonify, request, session from werkzeug.security import generate_password_hash from shelfmark.config.booklore_settings import ( @@ -26,17 +26,16 @@ from shelfmark.core.auth_modes import ( AUTH_SOURCE_CWA, AUTH_SOURCE_OIDC, AUTH_SOURCE_PROXY, - determine_auth_mode, - has_local_password_admin, + is_user_active_for_auth_mode, + load_active_auth_mode, normalize_auth_source, ) from shelfmark.core.cwa_user_sync import sync_cwa_users_from_rows from shelfmark.core.logger import setup_logger from shelfmark.core.settings_registry import load_config_file -from shelfmark.core.user_db import UserDB +from shelfmark.core.user_db import NO_AUTH_ACTIVITY_USERNAME, UserDB logger = setup_logger(__name__) -_NO_AUTH_ACTIVITY_USERNAME = "__shelfmark_noauth_activity__" def _get_user_edit_capabilities( @@ -66,37 +65,6 @@ def _get_user_edit_capabilities( } -def _get_auth_mode(): - """Get current auth mode from config.""" - try: - config = load_config_file("security") - return determine_auth_mode( - config, - CWA_DB_PATH, - has_local_admin=has_local_password_admin(), - ) - except Exception: - return "none" - - -def _require_admin(f): - """Decorator to require admin session for admin routes. - - In no-auth mode, everyone has access (is_admin defaults True). - In auth-required modes, requires an authenticated session with admin role. - """ - @wraps(f) - def decorated(*args, **kwargs): - auth_mode = _get_auth_mode() - if auth_mode != "none": - if "user_id" not in session: - return jsonify({"error": "Authentication required"}), 401 - if not session.get("is_admin", False): - return jsonify({"error": "Admin access required"}), 403 - return f(*args, **kwargs) - return decorated - - def _sanitize_user(user: dict) -> dict: """Remove sensitive fields from user dict before returning to client.""" sanitized = dict(user) @@ -117,14 +85,6 @@ def _oidc_role_management_message(security_config: dict[str, Any]) -> str: ) -def _is_user_active(user: dict[str, Any], auth_method: str) -> bool: - """Determine whether a user can authenticate in the current auth mode.""" - source = normalize_auth_source(user.get("auth_source"), user.get("oidc_subject")) - if source == AUTH_SOURCE_BUILTIN: - return auth_method in (AUTH_SOURCE_BUILTIN, AUTH_SOURCE_OIDC) - return source == auth_method - - def _serialize_user( user: dict[str, Any], auth_method: str, @@ -136,7 +96,7 @@ def _serialize_user( payload.get("auth_source"), payload.get("oidc_subject"), ) - payload["is_active"] = _is_user_active(payload, auth_method) + payload["is_active"] = is_user_active_for_auth_mode(payload, auth_method) payload["edit_capabilities"] = _get_user_edit_capabilities( payload, security_config=security_config, @@ -146,7 +106,7 @@ def _serialize_user( def _is_internal_system_user(user: dict[str, Any]) -> bool: username = str(user.get("username") or "").strip() - return username == _NO_AUTH_ACTIVITY_USERNAME + return username == NO_AUTH_ACTIVITY_USERNAME def _sync_all_cwa_users(user_db: UserDB) -> dict[str, int]: @@ -170,12 +130,31 @@ def _sync_all_cwa_users(user_db: UserDB) -> dict[str, int]: def register_admin_routes(app: Flask, user_db: UserDB) -> None: """Register admin user management routes on the Flask app.""" + def _require_admin(f): + """Decorator to require admin session for admin routes. + + In no-auth mode, everyone has access (is_admin defaults True). + In auth-required modes, requires an authenticated session with admin role. + Caches the resolved auth_mode in ``g.auth_mode`` for the request. + """ + @wraps(f) + def decorated(*args, **kwargs): + auth_mode = load_active_auth_mode(CWA_DB_PATH, user_db=user_db) + g.auth_mode = auth_mode + if auth_mode != "none": + if "user_id" not in session: + return jsonify({"error": "Authentication required"}), 401 + if not session.get("is_admin", False): + return jsonify({"error": "Admin access required"}), 403 + return f(*args, **kwargs) + return decorated + @app.route("/api/admin/users", methods=["GET"]) @_require_admin def admin_list_users(): """List all users.""" users = [u for u in user_db.list_users() if not _is_internal_system_user(u)] - auth_mode = _get_auth_mode() + auth_mode = g.auth_mode security_config = load_config_file("security") return jsonify([ _serialize_user(u, auth_mode, security_config=security_config) @@ -187,7 +166,7 @@ def register_admin_routes(app: Flask, user_db: UserDB) -> None: def admin_create_user(): """Create a new user with password authentication.""" data = request.get_json() or {} - auth_mode = _get_auth_mode() + auth_mode = g.auth_mode username = (data.get("username") or "").strip() password = data.get("password", "") @@ -240,7 +219,7 @@ def register_admin_routes(app: Flask, user_db: UserDB) -> None: return jsonify( _serialize_user( user, - _get_auth_mode(), + g.auth_mode, security_config=load_config_file("security"), ) ), 201 @@ -255,7 +234,7 @@ def register_admin_routes(app: Flask, user_db: UserDB) -> None: result = _serialize_user( user, - _get_auth_mode(), + g.auth_mode, security_config=load_config_file("security"), ) result["settings"] = user_db.get_user_settings(user_id) @@ -370,7 +349,7 @@ def register_admin_routes(app: Flask, user_db: UserDB) -> None: updated = user_db.get_user(user_id=user_id) result = _serialize_user( updated, - _get_auth_mode(), + g.auth_mode, security_config=security_config, ) result["settings"] = user_db.get_user_settings(user_id) @@ -381,8 +360,7 @@ def register_admin_routes(app: Flask, user_db: UserDB) -> None: @_require_admin def admin_sync_cwa_users(): """Manually sync users from Calibre-Web into users.db.""" - auth_mode = _get_auth_mode() - if auth_mode != AUTH_SOURCE_CWA: + if g.auth_mode != AUTH_SOURCE_CWA: return jsonify({ "error": "CWA sync is only available when CWA authentication is enabled", }), 400 @@ -426,12 +404,11 @@ def register_admin_routes(app: Flask, user_db: UserDB) -> None: if not user: return jsonify({"error": "User not found"}), 404 - auth_mode = _get_auth_mode() auth_source = normalize_auth_source( user.get("auth_source"), user.get("oidc_subject"), ) - if auth_source == AUTH_SOURCE_CWA and auth_source == auth_mode: + if auth_source == AUTH_SOURCE_CWA and auth_source == g.auth_mode: return jsonify({ "error": f"Cannot delete active {auth_source.upper()} users", "message": f"{auth_source.upper()} users are automatically re-provisioned on login.", diff --git a/shelfmark/core/admin_settings_routes.py b/shelfmark/core/admin_settings_routes.py index cfefbe00..b0bb7242 100644 --- a/shelfmark/core/admin_settings_routes.py +++ b/shelfmark/core/admin_settings_routes.py @@ -16,7 +16,7 @@ from shelfmark.core.user_settings_overrides import ( get_ordered_user_overridable_fields as _get_ordered_user_overridable_fields, get_settings_registry as _get_settings_registry, ) -from shelfmark.core.user_db import UserDB +from shelfmark.core.user_db import NO_AUTH_ACTIVITY_USERNAME, UserDB from shelfmark.core.request_policy import parse_policy_mode, validate_policy_rules @@ -205,6 +205,8 @@ def register_admin_settings_routes( keys_payload: dict[str, dict[str, Any]] = {} for user_record in user_db.list_users(): + if str(user_record.get("username") or "").strip() == NO_AUTH_ACTIVITY_USERNAME: + continue user_settings = user_db.get_user_settings(user_record["id"]) if not isinstance(user_settings, dict): continue diff --git a/shelfmark/core/auth_modes.py b/shelfmark/core/auth_modes.py index 0cd86dd6..226e50bb 100644 --- a/shelfmark/core/auth_modes.py +++ b/shelfmark/core/auth_modes.py @@ -28,10 +28,7 @@ def has_local_password_admin(user_db: Any | None = None) -> bool: db = UserDB(os.path.join(config_root, "users.db")) db.initialize() - return any( - user.get("password_hash") and user.get("role") == "admin" - for user in db.list_users() - ) + return db.has_admin_with_password() except Exception: return False @@ -78,6 +75,33 @@ def determine_auth_mode( return "none" +def load_active_auth_mode( + cwa_db_path: Any | None, + *, + user_db: Any | None = None, +) -> str: + """Resolve active auth mode using current security config and runtime prerequisites.""" + try: + from shelfmark.core.settings_registry import load_config_file + + security_config = load_config_file("security") + return determine_auth_mode( + security_config, + cwa_db_path, + has_local_admin=has_local_password_admin(user_db), + ) + except Exception: + return "none" + + +def is_user_active_for_auth_mode(user: Mapping[str, Any], auth_mode: str) -> bool: + """Return whether a user can authenticate under the current auth mode.""" + source = normalize_auth_source(user.get("auth_source"), user.get("oidc_subject")) + if source == AUTH_SOURCE_BUILTIN: + return auth_mode in (AUTH_SOURCE_BUILTIN, AUTH_SOURCE_OIDC) + return source == auth_mode + + def is_settings_or_onboarding_path(path: str) -> bool: """Return True when request path targets protected admin settings routes.""" return path.startswith("/api/settings") or path.startswith("/api/onboarding") diff --git a/shelfmark/core/queue.py b/shelfmark/core/queue.py index d72ff99a..3b28dcbf 100644 --- a/shelfmark/core/queue.py +++ b/shelfmark/core/queue.py @@ -150,8 +150,8 @@ class BookQueue: """Get current queue status grouped by status. Args: - user_id: If provided, only return tasks belonging to this user - (plus legacy tasks with no user_id). If None, return all. + user_id: If provided, only return tasks belonging to this user. + If None, return all. """ self.refresh() with self._lock: @@ -159,7 +159,7 @@ class BookQueue: for task_id, status in self._status.items(): if task_id in self._task_data: task = self._task_data[task_id] - if user_id is not None and task.user_id is not None and task.user_id != user_id: + if user_id is not None and task.user_id != user_id: continue result[status][task_id] = task return result @@ -329,8 +329,8 @@ class BookQueue: """Remove terminal tasks from tracking, optionally scoped to one user. Args: - user_id: If provided, only clear tasks belonging to this user, - plus legacy tasks with no user_id. If None, clear all. + user_id: If provided, only clear tasks belonging to this user. + If None, clear all. """ terminal_statuses = {QueueStatus.COMPLETE, QueueStatus.DONE, QueueStatus.AVAILABLE, QueueStatus.ERROR, QueueStatus.CANCELLED} with self._lock: @@ -347,7 +347,7 @@ class BookQueue: if task is None: # Without task ownership metadata we cannot safely scope removal. continue - if task.user_id is None or task.user_id == user_id: + if task.user_id == user_id: to_remove.append(task_id) for task_id in to_remove: diff --git a/shelfmark/core/request_helpers.py b/shelfmark/core/request_helpers.py new file mode 100644 index 00000000..f8277f1e --- /dev/null +++ b/shelfmark/core/request_helpers.py @@ -0,0 +1,54 @@ +"""Shared request-related helper functions used by routes and services.""" + +from __future__ import annotations + +from typing import Any + +from shelfmark.core.settings_registry import load_config_file + + +def load_users_request_policy_settings() -> dict[str, Any]: + """Load global request-policy settings from the users config file.""" + return load_config_file("users") + + +def coerce_bool(value: Any, default: bool = False) -> bool: + """Coerce arbitrary values into booleans with string-friendly semantics.""" + if isinstance(value, bool): + return value + if value is None: + return default + if isinstance(value, str): + normalized = value.strip().lower() + if normalized in {"1", "true", "yes", "on"}: + return True + if normalized in {"0", "false", "no", "off", ""}: + return False + return bool(value) + + +def coerce_int(value: Any, default: int) -> int: + """Best-effort integer coercion with fallback to default.""" + try: + return int(value) + except (TypeError, ValueError): + return default + + +def normalize_optional_text(value: Any) -> str | None: + """Return a trimmed string or None for empty/non-string input.""" + if not isinstance(value, str): + return None + normalized = value.strip() + return normalized or None + + +def extract_release_source_id(release_data: Any) -> str | None: + """Extract and normalize release_data.source_id.""" + if not isinstance(release_data, dict): + return None + source_id = release_data.get("source_id") + if not isinstance(source_id, str): + return None + normalized = source_id.strip() + return normalized or None diff --git a/shelfmark/core/request_policy.py b/shelfmark/core/request_policy.py index dde9d364..d3004e12 100644 --- a/shelfmark/core/request_policy.py +++ b/shelfmark/core/request_policy.py @@ -43,6 +43,13 @@ def cap_mode(mode: PolicyMode, ceiling: PolicyMode) -> PolicyMode: return mode +def _normalize_direct_source_mode(source: Any, mode: PolicyMode) -> PolicyMode: + """Direct search results are concrete releases; normalize request_book to request_release.""" + if normalize_source(source) == "direct_download" and mode == PolicyMode.REQUEST_BOOK: + return PolicyMode.REQUEST_RELEASE + return mode + + REQUEST_POLICY_KEYS = frozenset( { "REQUESTS_ENABLED", @@ -320,6 +327,10 @@ def resolve_policy_mode( The content-type default acts as a ceiling — matrix rules can only match or restrict further, never upgrade beyond the default. + + Direct-download exception: + - direct_download results are concrete releases, so request_book is + normalized to request_release for that source. """ effective = merge_request_policy_settings(global_settings, user_settings) @@ -346,6 +357,9 @@ def resolve_policy_mode( for candidate_source, candidate_content_type in candidates: for rule_source, rule_content_type, rule_mode in rules: if rule_source == candidate_source and rule_content_type == candidate_content_type: - return cap_mode(rule_mode, ceiling) + return _normalize_direct_source_mode( + normalized_source, + cap_mode(rule_mode, ceiling), + ) - return ceiling + return _normalize_direct_source_mode(normalized_source, ceiling) diff --git a/shelfmark/core/request_routes.py b/shelfmark/core/request_routes.py index 11d17de9..c94cbd8d 100644 --- a/shelfmark/core/request_routes.py +++ b/shelfmark/core/request_routes.py @@ -31,39 +31,18 @@ from shelfmark.core.notifications import ( notify_admin, notify_user, ) -from shelfmark.core.settings_registry import load_config_file +from shelfmark.core.request_helpers import ( + coerce_bool, + coerce_int, + extract_release_source_id, + load_users_request_policy_settings, + normalize_optional_text, +) from shelfmark.core.user_db import UserDB logger = setup_logger(__name__) -def _load_users_request_policy_settings() -> dict[str, Any]: - """Load global request-policy settings from users config.""" - return load_config_file("users") - - -def _as_bool(value: Any, default: bool = False) -> bool: - if isinstance(value, bool): - return value - if value is None: - return default - if isinstance(value, str): - normalized = value.strip().lower() - if normalized in {"1", "true", "yes", "on"}: - return True - if normalized in {"0", "false", "no", "off", ""}: - return False - return bool(value) - - -def _as_int(value: Any, default: int) -> int: - try: - parsed = int(value) - except (TypeError, ValueError): - return default - return parsed - - def _error_response( message: str, status_code: int, @@ -115,10 +94,10 @@ def _resolve_effective_policy( *, db_user_id: int | None, ) -> tuple[dict[str, Any], dict[str, Any], dict[str, Any], bool]: - global_settings = _load_users_request_policy_settings() + global_settings = load_users_request_policy_settings() user_settings = user_db.get_user_settings(db_user_id) if db_user_id is not None else {} effective = merge_request_policy_settings(global_settings, user_settings) - requests_enabled = _as_bool(effective.get("REQUESTS_ENABLED"), False) + requests_enabled = coerce_bool(effective.get("REQUESTS_ENABLED"), False) return global_settings, user_settings, effective, requests_enabled @@ -141,16 +120,6 @@ def _emit_request_event( logger.warning(f"Failed to emit WebSocket event '{event_name}' to room '{room}': {exc}") -def _extract_release_source_id(release_data: Any) -> str | None: - if not isinstance(release_data, dict): - return None - source_id = release_data.get("source_id") - if not isinstance(source_id, str): - return None - normalized = source_id.strip() - return normalized or None - - def _record_terminal_request_snapshot( activity_service: ActivityService | None, *, @@ -177,7 +146,7 @@ def _record_terminal_request_snapshot( except (TypeError, ValueError): user_id = None - source_id = _extract_release_source_id(request_row.get("release_data")) + source_id = extract_release_source_id(request_row.get("release_data")) try: activity_service.record_terminal_snapshot( @@ -194,27 +163,76 @@ def _record_terminal_request_snapshot( logger.warning("Failed to record terminal request snapshot for request %s: %s", request_id, exc) -def _normalize_optional_text(value: Any) -> str | None: - if not isinstance(value, str): - return None - normalized = value.strip() - return normalized or None - - def _resolve_title_from_book_data(book_data: Any) -> str: if isinstance(book_data, dict): - title = _normalize_optional_text(book_data.get("title")) + title = normalize_optional_text(book_data.get("title")) if title is not None: return title return "Unknown title" +def _build_direct_release_data_from_book_data( + *, + book_data: dict[str, Any], + content_type: str, +) -> dict[str, Any]: + """Build release-level payload fields for direct-download requests.""" + payload: dict[str, Any] = { + "source": "direct_download", + "source_id": book_data.get("provider_id") or book_data.get("id"), + "title": book_data.get("title"), + "author": book_data.get("author"), + "year": book_data.get("year"), + "format": book_data.get("format"), + "size": book_data.get("size"), + "preview": book_data.get("preview"), + "content_type": content_type, + } + return {key: value for key, value in payload.items() if value is not None} + + +def _normalize_direct_request_payload( + *, + source: str, + request_level: Any, + book_data: Any, + release_data: Any, + content_type: str, +) -> tuple[Any, Any]: + """Direct-search requests are always release-level with direct source metadata.""" + if source != "direct_download": + return request_level, release_data + + normalized_release_data = release_data + if normalized_release_data is None and isinstance(book_data, dict): + normalized_release_data = _build_direct_release_data_from_book_data( + book_data=book_data, + content_type=content_type, + ) + elif isinstance(normalized_release_data, dict): + normalized_release_data = dict(normalized_release_data) + + if isinstance(normalized_release_data, dict): + normalized_release_data["source"] = "direct_download" + if normalized_release_data.get("content_type") is None: + normalized_release_data["content_type"] = content_type + + if normalize_optional_text(normalized_release_data.get("source_id")) is None and isinstance(book_data, dict): + fallback_source_id = normalize_optional_text(book_data.get("provider_id")) or normalize_optional_text( + book_data.get("id") + ) + if fallback_source_id is not None: + normalized_release_data["source_id"] = fallback_source_id + + return "release", normalized_release_data + + def _resolve_request_title(request_row: dict[str, Any]) -> str: return _resolve_title_from_book_data(request_row.get("book_data")) def _format_user_label(username: str | None, user_id: int | None = None) -> str: - normalized_username = _normalize_optional_text(username) + normalized_username = normalize_optional_text(username) if normalized_username is not None: return normalized_username if user_id is not None and user_id > 0: @@ -228,7 +246,7 @@ def _resolve_request_username( request_row: dict[str, Any], fallback_username: str | None = None, ) -> str | None: - normalized_fallback = _normalize_optional_text(fallback_username) + normalized_fallback = normalize_optional_text(fallback_username) raw_user_id = request_row.get("user_id") try: request_user_id = int(raw_user_id) @@ -238,14 +256,14 @@ def _resolve_request_username( requester = user_db.get_user(user_id=request_user_id) if not isinstance(requester, dict): return normalized_fallback - return _normalize_optional_text(requester.get("username")) or normalized_fallback + return normalize_optional_text(requester.get("username")) or normalized_fallback def _resolve_request_source_and_format(request_row: dict[str, Any]) -> tuple[str, str | None]: release_data = request_row.get("release_data") if isinstance(release_data, dict): source = normalize_source(release_data.get("source") or request_row.get("source_hint")) - release_format = _normalize_optional_text( + release_format = normalize_optional_text( release_data.get("format") or release_data.get("filetype") or release_data.get("extension") @@ -289,7 +307,7 @@ def _notify_admin_for_request_event( ), format=release_format, source=source, - admin_note=_normalize_optional_text(request_row.get("admin_note")), + admin_note=normalize_optional_text(request_row.get("admin_note")), error_message=None, ) @@ -382,7 +400,7 @@ def register_request_routes( { "requests_enabled": requests_enabled, "is_admin": is_admin, - "allow_notes": _as_bool(effective.get("REQUESTS_ALLOW_NOTES"), default=True), + "allow_notes": coerce_bool(effective.get("REQUESTS_ALLOW_NOTES"), default=True), "defaults": { "ebook": ( default_ebook_mode.value @@ -409,7 +427,7 @@ def register_request_routes( db_user_id, db_gate = _require_db_user_id() if db_gate is not None or db_user_id is None: return db_gate - actor_username = _normalize_optional_text(session.get("user_id")) + actor_username = normalize_optional_text(session.get("user_id")) actor_label = _format_user_label(actor_username, db_user_id) data = request.get_json(silent=True) @@ -436,6 +454,13 @@ def register_request_routes( or data.get("content_type") or book_data.get("content_type") ) + request_level, release_data = _normalize_direct_request_payload( + source=source, + request_level=request_level, + book_data=book_data, + release_data=release_data, + content_type=content_type, + ) global_settings, user_settings, effective, requests_enabled = _resolve_effective_policy( user_db, @@ -453,7 +478,7 @@ def register_request_routes( code="requests_unavailable", ) - max_pending = _as_int( + max_pending = coerce_int( effective.get("MAX_PENDING_REQUESTS_PER_USER"), default=20, ) @@ -461,7 +486,7 @@ def register_request_routes( max_pending = 1 if max_pending > 1000: max_pending = 1000 - allow_notes = _as_bool(effective.get("REQUESTS_ALLOW_NOTES"), default=True) + allow_notes = coerce_bool(effective.get("REQUESTS_ALLOW_NOTES"), default=True) note_value = data.get("note") if allow_notes else None resolved_mode = resolve_policy_mode( @@ -495,10 +520,7 @@ def register_request_routes( if resolved_mode == PolicyMode.REQUEST_BOOK: requested_level = str(request_level).strip().lower() if isinstance(request_level, str) else "" - # Direct search results are already concrete releases, so allow release-level - # request payloads even when the policy default is request_book. - allow_direct_release_payload = source == "direct_download" and requested_level == "release" - if requested_level != "book" and not allow_direct_release_payload: + if requested_level != "book": logger.debug( "Request not created for '%s' by %s: policy requires book-level requests", request_title, @@ -611,7 +633,7 @@ def register_request_routes( "status": updated["status"], "title": _resolve_request_title(updated), } - actor_label = _format_user_label(_normalize_optional_text(session.get("user_id")), db_user_id) + actor_label = _format_user_label(normalize_optional_text(session.get("user_id")), db_user_id) logger.info( "Request cancelled #%s for '%s' by %s", updated["id"], @@ -718,7 +740,7 @@ def register_request_routes( "status": updated["status"], "title": _resolve_request_title(updated), } - admin_label = _format_user_label(_normalize_optional_text(session.get("user_id")), admin_user_id) + admin_label = _format_user_label(normalize_optional_text(session.get("user_id")), admin_user_id) requester_label = _format_user_label( _resolve_request_username(user_db, request_row=updated), _resolve_request_user_id(updated), @@ -788,7 +810,7 @@ def register_request_routes( "status": updated["status"], "title": _resolve_request_title(updated), } - admin_label = _format_user_label(_normalize_optional_text(session.get("user_id")), admin_user_id) + admin_label = _format_user_label(normalize_optional_text(session.get("user_id")), admin_user_id) requester_label = _format_user_label( _resolve_request_username(user_db, request_row=updated), _resolve_request_user_id(updated), diff --git a/shelfmark/core/request_validation.py b/shelfmark/core/request_validation.py new file mode 100644 index 00000000..713183ef --- /dev/null +++ b/shelfmark/core/request_validation.py @@ -0,0 +1,90 @@ +"""Shared request validation and normalization helpers.""" + +from __future__ import annotations + +from typing import Any + +from shelfmark.core.request_policy import parse_policy_mode + + +VALID_REQUEST_STATUSES = frozenset({"pending", "fulfilled", "rejected", "cancelled"}) +TERMINAL_REQUEST_STATUSES = frozenset({"fulfilled", "rejected", "cancelled"}) +VALID_REQUEST_LEVELS = frozenset({"book", "release"}) +VALID_DELIVERY_STATES = frozenset( + { + "none", + "unknown", + "queued", + "resolving", + "locating", + "downloading", + "complete", + "error", + "cancelled", + } +) + + +def normalize_request_status(status: Any) -> str: + """Validate and normalize request status values.""" + if not isinstance(status, str): + raise ValueError(f"Invalid request status: {status}") + normalized = status.strip().lower() + if normalized not in VALID_REQUEST_STATUSES: + raise ValueError(f"Invalid request status: {status}") + return normalized + + +def normalize_policy_mode(mode: Any) -> str: + """Validate and normalize policy mode values.""" + parsed = parse_policy_mode(mode) + if parsed is None: + raise ValueError(f"Invalid policy_mode: {mode}") + return parsed.value + + +def normalize_request_level(request_level: Any) -> str: + """Validate and normalize request level values.""" + if not isinstance(request_level, str): + raise ValueError(f"Invalid request_level: {request_level}") + normalized = request_level.strip().lower() + if normalized not in VALID_REQUEST_LEVELS: + raise ValueError(f"Invalid request_level: {request_level}") + return normalized + + +def normalize_delivery_state(state: Any) -> str: + """Validate and normalize delivery-state values.""" + if not isinstance(state, str): + raise ValueError(f"Invalid delivery_state: {state}") + normalized = state.strip().lower() + if normalized not in VALID_DELIVERY_STATES: + raise ValueError(f"Invalid delivery_state: {state}") + return normalized + + +def safe_delivery_state(state: Any, default: str = "none") -> str: + """Normalize delivery-state with a fallback for invalid/missing values.""" + if not isinstance(state, str): + return default + normalized = state.strip().lower() + return normalized if normalized in VALID_DELIVERY_STATES else default + + +def validate_request_level_payload(request_level: Any, release_data: Any) -> str: + """Validate request_level and release_data shape coupling.""" + normalized_level = normalize_request_level(request_level) + if normalized_level == "release" and release_data is None: + raise ValueError("request_level=release requires non-null release_data") + if normalized_level == "book" and release_data is not None: + raise ValueError("request_level=book requires null release_data") + return normalized_level + + +def validate_status_transition(current_status: Any, new_status: Any) -> tuple[str, str]: + """Validate request status transitions and terminal immutability.""" + current = normalize_request_status(current_status) + new = normalize_request_status(new_status) + if current in TERMINAL_REQUEST_STATUSES and new != current: + raise ValueError("Terminal request statuses are immutable") + return current, new diff --git a/shelfmark/core/requests_service.py b/shelfmark/core/requests_service.py index df7f5bc0..5b714186 100644 --- a/shelfmark/core/requests_service.py +++ b/shelfmark/core/requests_service.py @@ -6,25 +6,18 @@ from datetime import datetime, timezone import json from typing import Any, Callable, TYPE_CHECKING -from shelfmark.core.request_policy import normalize_content_type, parse_policy_mode - - -VALID_REQUEST_STATUSES = frozenset({"pending", "fulfilled", "rejected", "cancelled"}) -TERMINAL_REQUEST_STATUSES = frozenset({"fulfilled", "rejected", "cancelled"}) -VALID_REQUEST_LEVELS = frozenset({"book", "release"}) -VALID_DELIVERY_STATES = frozenset( - { - "none", - "unknown", - "queued", - "resolving", - "locating", - "downloading", - "complete", - "error", - "cancelled", - } +from shelfmark.core.request_policy import normalize_content_type +from shelfmark.core.request_validation import ( + normalize_policy_mode, + normalize_request_level, + normalize_request_status, + safe_delivery_state, + validate_request_level_payload, + validate_status_transition, ) +from shelfmark.core.request_helpers import extract_release_source_id + + MAX_REQUEST_NOTE_LENGTH = 1000 MAX_REQUEST_JSON_BLOB_BYTES = 10 * 1024 @@ -48,63 +41,6 @@ class RequestServiceError(ValueError): self.code = code -def normalize_request_status(status: Any) -> str: - """Validate and normalize request status values.""" - if not isinstance(status, str): - raise ValueError(f"Invalid request status: {status}") - normalized = status.strip().lower() - if normalized not in VALID_REQUEST_STATUSES: - raise ValueError(f"Invalid request status: {status}") - return normalized - - -def normalize_policy_mode(mode: Any) -> str: - """Validate and normalize policy mode values.""" - parsed = parse_policy_mode(mode) - if parsed is None: - raise ValueError(f"Invalid policy_mode: {mode}") - return parsed.value - - -def normalize_request_level(request_level: Any) -> str: - """Validate and normalize request level values.""" - if not isinstance(request_level, str): - raise ValueError(f"Invalid request_level: {request_level}") - normalized = request_level.strip().lower() - if normalized not in VALID_REQUEST_LEVELS: - raise ValueError(f"Invalid request_level: {request_level}") - return normalized - - -def normalize_delivery_state(state: Any) -> str: - """Validate and normalize delivery-state values.""" - if not isinstance(state, str): - raise ValueError(f"Invalid delivery_state: {state}") - normalized = state.strip().lower() - if normalized not in VALID_DELIVERY_STATES: - raise ValueError(f"Invalid delivery_state: {state}") - return normalized - - -def validate_request_level_payload(request_level: Any, release_data: Any) -> str: - """Validate request_level and release_data shape coupling.""" - normalized_level = normalize_request_level(request_level) - if normalized_level == "release" and release_data is None: - raise ValueError("request_level=release requires non-null release_data") - if normalized_level == "book" and release_data is not None: - raise ValueError("request_level=book requires null release_data") - return normalized_level - - -def validate_status_transition(current_status: Any, new_status: Any) -> tuple[str, str]: - """Validate request status transitions and terminal immutability.""" - current = normalize_request_status(current_status) - new = normalize_request_status(new_status) - if current in TERMINAL_REQUEST_STATUSES and new != current: - raise ValueError("Terminal request statuses are immutable") - return current, new - - def _normalize_match_text(value: Any) -> str: if not isinstance(value, str): return "" @@ -186,22 +122,8 @@ def _now_timestamp() -> str: return datetime.now(timezone.utc).isoformat(timespec="seconds") -def _extract_release_source_id(release_data: Any) -> str | None: - if not isinstance(release_data, dict): - return None - source_id = release_data.get("source_id") - if not isinstance(source_id, str): - return None - normalized = source_id.strip() - return normalized or None - - def _existing_delivery_state(request_row: dict[str, Any]) -> str: - raw_state = request_row.get("delivery_state") - if not isinstance(raw_state, str): - return "none" - normalized = raw_state.strip().lower() - return normalized if normalized in VALID_DELIVERY_STATES else "none" + return safe_delivery_state(request_row.get("delivery_state")) def sync_delivery_states_from_queue_status( @@ -226,7 +148,7 @@ def sync_delivery_states_from_queue_status( updated: list[dict[str, Any]] = [] for row in fulfilled_rows: - source_id = _extract_release_source_id(row.get("release_data")) + source_id = extract_release_source_id(row.get("release_data")) if source_id is None: continue @@ -516,50 +438,7 @@ def reopen_failed_request( failure_reason: str | None = None, ) -> dict[str, Any] | None: """Reopen a failed fulfilled request so admins can re-approve with a new release.""" - normalized_failure_reason = None - if isinstance(failure_reason, str): - normalized_failure_reason = failure_reason.strip() or None - - with user_db._lock: - conn = user_db._connect() - try: - current_row = conn.execute( - "SELECT * FROM download_requests WHERE id = ?", - (request_id,), - ).fetchone() - current_request = user_db._parse_request_row(current_row) - if current_request is None: - return None - - if current_request.get("status") != "fulfilled": - return None - current_delivery_state = _existing_delivery_state(current_request) - # Terminal hook callbacks can run before delivery-state sync persists "error". - # Allow reopening fulfilled requests unless they are already complete. - if current_delivery_state == "complete": - return None - if current_delivery_state not in {"error", "cancelled"} and normalized_failure_reason is None: - return None - - conn.execute( - """ - UPDATE download_requests - SET status = 'pending', - delivery_state = 'none', - delivery_updated_at = NULL, - release_data = NULL, - last_failure_reason = ?, - reviewed_by = NULL, - reviewed_at = NULL - WHERE id = ? - """, - (normalized_failure_reason, request_id), - ) - updated_row = conn.execute( - "SELECT * FROM download_requests WHERE id = ?", - (request_id,), - ).fetchone() - conn.commit() - return user_db._parse_request_row(updated_row) - finally: - conn.close() + return user_db.reopen_failed_request( + request_id, + failure_reason=failure_reason, + ) diff --git a/shelfmark/core/self_user_routes.py b/shelfmark/core/self_user_routes.py index 75d04d55..dff0a73a 100644 --- a/shelfmark/core/self_user_routes.py +++ b/shelfmark/core/self_user_routes.py @@ -3,7 +3,7 @@ from functools import wraps from typing import Any, Callable, Mapping -from flask import Flask, jsonify, request, session +from flask import Flask, g, jsonify, request, session from werkzeug.security import generate_password_hash from shelfmark.config.env import CWA_DB_PATH @@ -16,8 +16,8 @@ from shelfmark.core.auth_modes import ( AUTH_SOURCE_CWA, AUTH_SOURCE_OIDC, AUTH_SOURCE_PROXY, - determine_auth_mode, - has_local_password_admin, + is_user_active_for_auth_mode, + load_active_auth_mode, normalize_auth_source, ) from shelfmark.core.logger import setup_logger @@ -43,34 +43,6 @@ _VALID_SELF_SETTINGS_SECTIONS = ( _DEFAULT_VISIBLE_SELF_SETTINGS_SECTIONS = list(_VALID_SELF_SETTINGS_SECTIONS) -def _get_auth_mode() -> str: - """Get current auth mode from config.""" - try: - config = load_config_file("security") - return determine_auth_mode( - config, - CWA_DB_PATH, - has_local_admin=has_local_password_admin(), - ) - except Exception: - return "none" - - -def _require_authenticated_user(f: Callable[..., Any]) -> Callable[..., Any]: - """Decorator requiring an authenticated session linked to a local user row.""" - - @wraps(f) - def decorated(*args, **kwargs): - auth_mode = _get_auth_mode() - if auth_mode != "none" and "user_id" not in session: - return jsonify({"error": "Authentication required"}), 401 - if "db_user_id" not in session: - return jsonify({"error": "Authenticated session is missing local user context"}), 403 - return f(*args, **kwargs) - - return decorated - - def _get_current_user(user_db: UserDB) -> tuple[int | None, dict[str, Any] | None, tuple[Any, int] | None]: raw_user_id = session.get("db_user_id") try: @@ -84,13 +56,6 @@ def _get_current_user(user_db: UserDB) -> tuple[int | None, dict[str, Any] | Non return user_id, user, None -def _is_user_active(user: Mapping[str, Any], auth_method: str) -> bool: - source = normalize_auth_source(user.get("auth_source"), user.get("oidc_subject")) - if source == AUTH_SOURCE_BUILTIN: - return auth_method in (AUTH_SOURCE_BUILTIN, AUTH_SOURCE_OIDC) - return source == auth_method - - def _get_self_edit_capabilities(user: Mapping[str, Any]) -> dict[str, Any]: auth_source = normalize_auth_source( user.get("auth_source"), @@ -113,7 +78,7 @@ def _serialize_self_user(user: Mapping[str, Any], auth_mode: str) -> dict[str, A payload.get("auth_source"), payload.get("oidc_subject"), ) - payload["is_active"] = _is_user_active(payload, auth_mode) + payload["is_active"] = is_user_active_for_auth_mode(payload, auth_mode) payload["edit_capabilities"] = _get_self_edit_capabilities(payload) return payload @@ -173,6 +138,22 @@ def _get_allowed_self_settings_keys(visible_sections: list[str]) -> set[str]: def register_self_user_routes(app: Flask, user_db: UserDB) -> None: """Register self-service user endpoints.""" + def _require_authenticated_user(f: Callable[..., Any]) -> Callable[..., Any]: + """Decorator requiring an authenticated session linked to a local user row. + + Caches the resolved auth_mode in ``g.auth_mode`` for the request. + """ + @wraps(f) + def decorated(*args, **kwargs): + auth_mode = load_active_auth_mode(CWA_DB_PATH, user_db=user_db) + g.auth_mode = auth_mode + if auth_mode != "none" and "user_id" not in session: + return jsonify({"error": "Authentication required"}), 401 + if "db_user_id" not in session: + return jsonify({"error": "Authenticated session is missing local user context"}), 403 + return f(*args, **kwargs) + return decorated + @app.route("/api/users/me/edit-context", methods=["GET"]) @_require_authenticated_user def users_me_edit_context(): @@ -180,8 +161,7 @@ def register_self_user_routes(app: Flask, user_db: UserDB) -> None: if user_error: return user_error - auth_mode = _get_auth_mode() - serialized_user = _serialize_self_user(user, auth_mode) + serialized_user = _serialize_self_user(user, g.auth_mode) serialized_user["settings"] = user_db.get_user_settings(user_id) visible_self_settings_sections = _get_visible_self_settings_sections() @@ -367,7 +347,7 @@ def register_self_user_routes(app: Flask, user_db: UserDB) -> None: if not updated: return jsonify({"error": "User not found"}), 404 - result = _serialize_self_user(updated, _get_auth_mode()) + result = _serialize_self_user(updated, g.auth_mode) result["settings"] = user_db.get_user_settings(user_id) logger.info(f"User {user_id} updated their own account") return jsonify(result) diff --git a/shelfmark/core/user_db.py b/shelfmark/core/user_db.py index 9c0b0ae6..53e8fcd5 100644 --- a/shelfmark/core/user_db.py +++ b/shelfmark/core/user_db.py @@ -8,15 +8,18 @@ from typing import Any, Dict, List, Optional from shelfmark.core.auth_modes import AUTH_SOURCE_BUILTIN, AUTH_SOURCE_SET from shelfmark.core.logger import setup_logger -from shelfmark.core.requests_service import ( +from shelfmark.core.request_validation import ( normalize_delivery_state, normalize_policy_mode, normalize_request_level, normalize_request_status, + safe_delivery_state, validate_request_level_payload, validate_status_transition, ) +NO_AUTH_ACTIVITY_USERNAME = "__shelfmark_noauth_activity__" + logger = setup_logger(__name__) _CREATE_TABLES_SQL = """ @@ -395,6 +398,19 @@ class UserDB: finally: conn.close() + def has_admin_with_password(self) -> bool: + """Return True when at least one admin user with a password hash exists.""" + conn = self._connect() + try: + row = conn.execute( + "SELECT 1 FROM users WHERE role = 'admin'" + " AND password_hash IS NOT NULL AND password_hash != ''" + " LIMIT 1", + ).fetchone() + return row is not None + finally: + conn.close() + def get_user_settings(self, user_id: int) -> Dict[str, Any]: """Get per-user settings. Returns empty dict if none set.""" conn = self._connect() @@ -731,6 +747,68 @@ class UserDB: finally: conn.close() + def reopen_failed_request( + self, + request_id: int, + *, + failure_reason: Optional[str] = None, + ) -> Optional[Dict[str, Any]]: + """Reopen a failed fulfilled request so admins can re-approve it.""" + normalized_failure_reason = None + if isinstance(failure_reason, str): + normalized_failure_reason = failure_reason.strip() or None + + with self._lock: + conn = self._connect() + try: + current_row = conn.execute( + "SELECT * FROM download_requests WHERE id = ?", + (request_id,), + ).fetchone() + current_request = self._parse_request_row(current_row) + if current_request is None: + return None + + if current_request.get("status") != "fulfilled": + return None + + current_delivery_state = safe_delivery_state( + current_request.get("delivery_state"), + ) + + # Terminal hook callbacks can run before delivery-state sync persists "error". + # Allow reopening fulfilled requests unless they are already complete. + if current_delivery_state == "complete": + return None + if ( + current_delivery_state not in {"error", "cancelled"} + and normalized_failure_reason is None + ): + return None + + conn.execute( + """ + UPDATE download_requests + SET status = 'pending', + delivery_state = 'none', + delivery_updated_at = NULL, + release_data = NULL, + last_failure_reason = ?, + reviewed_by = NULL, + reviewed_at = NULL + WHERE id = ? + """, + (normalized_failure_reason, request_id), + ) + updated_row = conn.execute( + "SELECT * FROM download_requests WHERE id = ?", + (request_id,), + ).fetchone() + conn.commit() + return self._parse_request_row(updated_row) + finally: + conn.close() + def count_pending_requests(self) -> int: """Count all pending requests.""" conn = self._connect() diff --git a/shelfmark/download/fs.py b/shelfmark/download/fs.py index 67737a74..5c5c30de 100644 --- a/shelfmark/download/fs.py +++ b/shelfmark/download/fs.py @@ -52,6 +52,20 @@ def _call_and_capture(func: Callable[..., T], args: tuple[Any, ...], kwargs: dic return False, exc +def _must_avoid_gevent_threadpool(func: Callable[..., Any]) -> bool: + """Return True when `func` is unsafe to execute inside gevent's threadpool.""" + if not _use_gevent_threadpool() or not _gevent_monkey: + return False + + # gevent.subprocess requires child watchers on the default event loop. + # Executing patched subprocess functions in a worker thread can raise: + # "TypeError: child watchers are only available on the default loop". + if _gevent_monkey.is_object_patched("subprocess", "run") and func is subprocess.run: + return True + + return False + + def run_blocking_io(func: Callable[..., T], *args: Any, **kwargs: Any) -> T: """Run blocking I/O in a native thread when under gevent. @@ -60,6 +74,9 @@ def run_blocking_io(func: Callable[..., T], *args: Any, **kwargs: Any) -> T: collision retries, EXDEV for cross-device moves). Capture and re-raise in the caller to avoid noisy, misleading tracebacks. """ + if _must_avoid_gevent_threadpool(func): + return func(*args, **kwargs) + if _use_gevent_threadpool(): ok, result = _get_io_threadpool().apply(_call_and_capture, (func, args, kwargs)) if ok: diff --git a/shelfmark/main.py b/shelfmark/main.py index 42f8dd4c..44162139 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -30,10 +30,9 @@ from shelfmark.core.logger import setup_logger from shelfmark.core.models import SearchFilters, QueueStatus from shelfmark.core.prefix_middleware import PrefixMiddleware from shelfmark.core.auth_modes import ( - determine_auth_mode, get_auth_check_admin_status, - has_local_password_admin, is_settings_or_onboarding_path, + load_active_auth_mode, requires_admin_for_settings_access, ) from shelfmark.core.cwa_user_sync import upsert_cwa_user @@ -52,6 +51,12 @@ from shelfmark.core.requests_service import ( ) from shelfmark.core.activity_service import ActivityService, build_download_item_key from shelfmark.core.notifications import NotificationContext, NotificationEvent, notify_admin, notify_user +from shelfmark.core.request_helpers import ( + coerce_bool, + extract_release_source_id, + load_users_request_policy_settings, + normalize_optional_text, +) from shelfmark.core.utils import normalize_base_path from shelfmark.api.websocket import ws_manager @@ -74,12 +79,13 @@ if BASE_PATH: # We run this app under Gunicorn with a gevent websocket worker (even when DEBUG=true), # so Socket.IO should always use gevent here. async_mode = 'gevent' +socketio_cors_allowed_origins = "*" if DEBUG else None # Initialize Flask-SocketIO with reverse proxy support socketio_path = f"{BASE_PATH}/socket.io" if BASE_PATH else "/socket.io" socketio = SocketIO( app, - cors_allowed_origins="*", + cors_allowed_origins=socketio_cors_allowed_origins, async_mode=async_mode, logger=False, engineio_logger=False, @@ -99,6 +105,7 @@ socketio = SocketIO( ws_manager.init_app(app, socketio) ws_manager.set_queue_status_fn(backend.queue_status) logger.info(f"Flask-SocketIO initialized with async_mode='{async_mode}'") +logger.info("Socket.IO CORS allowed origins: %s", socketio_cors_allowed_origins) # Ensure all plugins are loaded before starting the download coordinator. # This prevents a race condition where the download loop could try to process @@ -213,38 +220,7 @@ def get_auth_mode() -> str: Uses configured AUTH_METHOD plus runtime prerequisites. Returns "none" when config is invalid or unavailable. """ - from shelfmark.core.settings_registry import load_config_file - - try: - security_config = load_config_file("security") - return determine_auth_mode( - security_config, - CWA_DB_PATH, - has_local_admin=has_local_password_admin(user_db), - ) - except Exception: - return "none" - - -def _load_users_request_policy_settings() -> dict[str, Any]: - """Load global request policy settings from users config.""" - from shelfmark.core.settings_registry import load_config_file - - return load_config_file("users") - - -def _as_bool(value: Any, default: bool = False) -> bool: - if isinstance(value, bool): - return value - if value is None: - return default - if isinstance(value, str): - normalized = value.strip().lower() - if normalized in {"1", "true", "yes", "on"}: - return True - if normalized in {"0", "false", "no", "off", ""}: - return False - return bool(value) + return load_active_auth_mode(CWA_DB_PATH, user_db=user_db) _AUDIOBOOK_CATEGORY_RANGE = (3030, 3049) @@ -331,7 +307,7 @@ def _resolve_policy_mode_for_current_user(*, source: Any, content_type: Any) -> if user_db is None: return None - global_settings = _load_users_request_policy_settings() + global_settings = load_users_request_policy_settings() db_user_id = session.get("db_user_id") user_settings: dict[str, Any] | None = None if db_user_id is not None: @@ -341,7 +317,7 @@ def _resolve_policy_mode_for_current_user(*, source: Any, content_type: Any) -> user_settings = None effective = merge_request_policy_settings(global_settings, user_settings) - if not _as_bool(effective.get("REQUESTS_ENABLED"), False): + if not coerce_bool(effective.get("REQUESTS_ENABLED"), False): return None resolved_mode = resolve_policy_mode( @@ -500,14 +476,45 @@ werkzeug_logger.setLevel(logger.level) werkzeug_logger.addFilter(LogNoiseFilter()) # Set up authentication defaults -# The secret key will reset every time we restart, which will -# require users to authenticate again from shelfmark.config.env import SESSION_COOKIE_NAME, SESSION_COOKIE_SECURE_ENV, string_to_bool SESSION_COOKIE_SECURE = string_to_bool(SESSION_COOKIE_SECURE_ENV) + +def _load_or_create_secret_key() -> bytes: + """Load a persisted Flask secret key from config, or create one.""" + secret_path = CONFIG_DIR / ".flask_secret" + + try: + if secret_path.exists(): + secret_key = secret_path.read_bytes() + if len(secret_key) >= 32: + return secret_key + logger.warning( + "Invalid persisted Flask secret key at %s (length=%s). Regenerating.", + secret_path, + len(secret_key), + ) + except OSError as exc: + logger.warning("Failed to read Flask secret key at %s: %s", secret_path, exc) + + secret_key = os.urandom(64) + try: + secret_path.parent.mkdir(parents=True, exist_ok=True) + secret_path.write_bytes(secret_key) + os.chmod(secret_path, 0o600) + except OSError as exc: + logger.warning( + "Failed to persist Flask secret key at %s. Sessions may reset on restart: %s", + secret_path, + exc, + ) + + return secret_key + + app.config.update( - SECRET_KEY = os.urandom(64), + SECRET_KEY = _load_or_create_secret_key(), SESSION_COOKIE_HTTPONLY = True, SESSION_COOKIE_SAMESITE = 'Lax', SESSION_COOKIE_SECURE = SESSION_COOKIE_SECURE, @@ -1096,16 +1103,6 @@ def _resolve_status_scope(*, require_authenticated: bool = True) -> tuple[bool, return False, db_user_id, True -def _extract_release_source_id(release_data: Any) -> str | None: - if not isinstance(release_data, dict): - return None - source_id = release_data.get("source_id") - if not isinstance(source_id, str): - return None - normalized = source_id.strip() - return normalized or None - - def _queue_status_to_final_activity_status(status: QueueStatus) -> str | None: if status == QueueStatus.COMPLETE: return "complete" @@ -1115,14 +1112,6 @@ def _queue_status_to_final_activity_status(status: QueueStatus) -> str | None: return "cancelled" return None - -def _normalize_optional_text(value: Any) -> str | None: - if not isinstance(value, str): - return None - normalized = value.strip() - return normalized or None - - def _queue_status_to_notification_event(status: QueueStatus) -> NotificationEvent | None: if status in {QueueStatus.COMPLETE, QueueStatus.AVAILABLE, QueueStatus.DONE}: return NotificationEvent.DOWNLOAD_COMPLETE @@ -1142,17 +1131,17 @@ def _notify_admin_for_terminal_download_status(*, task_id: str, status: QueueSta except (TypeError, ValueError): owner_user_id = None - content_type = _normalize_optional_text(getattr(task, "content_type", None)) + content_type = normalize_optional_text(getattr(task, "content_type", None)) context = NotificationContext( event=event, title=str(getattr(task, "title", "Unknown title") or "Unknown title"), author=str(getattr(task, "author", "Unknown author") or "Unknown author"), - username=_normalize_optional_text(getattr(task, "username", None)), + username=normalize_optional_text(getattr(task, "username", None)), content_type=normalize_content_type(content_type) if content_type is not None else None, - format=_normalize_optional_text(getattr(task, "format", None)), + format=normalize_optional_text(getattr(task, "format", None)), source=normalize_source(getattr(task, "source", None)), error_message=( - _normalize_optional_text(getattr(task, "status_message", None)) + normalize_optional_text(getattr(task, "status_message", None)) if event == NotificationEvent.DOWNLOAD_FAILED else None ), @@ -1199,7 +1188,7 @@ def _record_download_terminal_snapshot(task_id: str, status: QueueStatus, task: if user_db is not None and owner_user_id is not None: fulfilled_rows = user_db.list_requests(user_id=owner_user_id, status="fulfilled") for row in fulfilled_rows: - source_id = _extract_release_source_id(row.get("release_data")) + source_id = extract_release_source_id(row.get("release_data")) if source_id == task_id: linked_request = row origin = "requested" @@ -1293,7 +1282,7 @@ def _is_graduated_request_download(task_id: str, *, user_id: int) -> bool: fulfilled_rows = user_db.list_requests(user_id=user_id, status="fulfilled") for row in fulfilled_rows: - source_id = _extract_release_source_id(row.get("release_data")) + source_id = extract_release_source_id(row.get("release_data")) if source_id == task_id: return True return False diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx index 7a738d41..c5b74c65 100644 --- a/src/frontend/src/App.tsx +++ b/src/frontend/src/App.tsx @@ -786,8 +786,8 @@ function App() { requiredMode, code: isApiResponseError(error) ? error.code : null, }); - if (requiredMode === 'request_release' || requiredMode === 'request_book') { - openRequestConfirmation(buildDirectRequestPayload(book, requiredMode)); + if (requiredMode === 'request_release') { + openRequestConfirmation(buildDirectRequestPayload(book)); await refreshRequestPolicy({ force: true }); return; } @@ -927,9 +927,9 @@ function App() { return; } - if (mode === 'request_release' || mode === 'request_book') { + if (mode === 'request_release') { policyTrace('direct.action:request_modal', { bookId: book.id, mode }); - openRequestConfirmation(buildDirectRequestPayload(book, mode)); + openRequestConfirmation(buildDirectRequestPayload(book)); return; } diff --git a/src/frontend/src/hooks/requestPolicyCore.ts b/src/frontend/src/hooks/requestPolicyCore.ts index cd2ba433..3ba91e1f 100644 --- a/src/frontend/src/hooks/requestPolicyCore.ts +++ b/src/frontend/src/hooks/requestPolicyCore.ts @@ -30,6 +30,15 @@ export const normalizeSource = (value: string): string => { return source || '*'; }; +const normalizeDirectSourceMode = ( + source: string, + mode: RequestPolicyMode +): RequestPolicyMode => { + return source === 'direct_download' && mode === 'request_book' + ? 'request_release' + : mode; +}; + const normalizeRuleSource = (value: unknown): string | null => { if (typeof value !== 'string') { return null; @@ -87,19 +96,22 @@ export const resolveSourceModeFromPolicy = ( source: string, contentType: ContentType | string ): RequestPolicyMode => { + const normalizedSource = normalizeSource(source); const defaultMode = resolveDefaultModeFromPolicy(policy, isAdmin, contentType); if (defaultMode === 'download' && (isAdmin || !policy || !policy.requests_enabled)) { return 'download'; } - const normalizedSource = normalizeSource(source); const normalizedContentType = normalizeContentType(contentType); const sourceModes = policy?.source_modes?.find( (sourceMode) => normalizeSource(sourceMode.source) === normalizedSource ); const fromSource = sourceModes?.modes?.[normalizedContentType]; if (fromSource) { - return capModeToCeiling(fromSource, defaultMode); + return normalizeDirectSourceMode( + normalizedSource, + capModeToCeiling(fromSource, defaultMode) + ); } const rules = Array.isArray(policy?.rules) ? policy.rules : []; @@ -129,10 +141,13 @@ export const resolveSourceModeFromPolicy = ( continue; } - return capModeToCeiling(parsedMode, defaultMode); + return normalizeDirectSourceMode( + normalizedSource, + capModeToCeiling(parsedMode, defaultMode) + ); } - return defaultMode; + return normalizeDirectSourceMode(normalizedSource, defaultMode); }; export class RequestPolicyCache { diff --git a/src/frontend/src/tests/requestPayload.node.test.ts b/src/frontend/src/tests/requestPayload.node.test.ts index cba0b305..5907ca58 100644 --- a/src/frontend/src/tests/requestPayload.node.test.ts +++ b/src/frontend/src/tests/requestPayload.node.test.ts @@ -35,18 +35,8 @@ describe('requestPayload utilities', () => { assert.equal(toContentType('something-else'), 'ebook'); }); - it('creates direct request payload at release level for request_release mode', () => { - const payload = buildDirectRequestPayload(baseBook, 'request_release'); - - assert.equal(payload.context.request_level, 'release'); - assert.equal(payload.context.source, 'direct_download'); - assert.equal(payload.context.content_type, 'ebook'); - assert.ok(payload.release_data); - assert.equal(payload.release_data?.source, 'direct_download'); - }); - - it('creates direct request payload with attached release for request_book mode', () => { - const payload = buildDirectRequestPayload(baseBook, 'request_book'); + it('creates direct request payload as release-level with attached release data', () => { + const payload = buildDirectRequestPayload(baseBook); assert.equal(payload.context.request_level, 'release'); assert.equal(payload.context.source, 'direct_download'); diff --git a/src/frontend/src/tests/requestPolicyCore.node.test.ts b/src/frontend/src/tests/requestPolicyCore.node.test.ts index f037877d..864493b1 100644 --- a/src/frontend/src/tests/requestPolicyCore.node.test.ts +++ b/src/frontend/src/tests/requestPolicyCore.node.test.ts @@ -51,6 +51,19 @@ describe('requestPolicyCore mode resolution', () => { assert.equal(resolveSourceModeFromPolicy(policy, false, 'unknown', 'audiobook'), 'request_release'); }); + it('normalizes direct source request_book mode to request_release', () => { + const policy = makePolicy({ + defaults: { + ebook: 'request_book', + audiobook: 'request_release', + }, + source_modes: [], + rules: [], + }); + + assert.equal(resolveSourceModeFromPolicy(policy, false, 'direct_download', 'ebook'), 'request_release'); + }); + it('short-circuits to download for admins and requests-disabled policy', () => { const blockedPolicy = makePolicy({ requests_enabled: false, diff --git a/src/frontend/src/tests/requestPolicyUi.node.test.ts b/src/frontend/src/tests/requestPolicyUi.node.test.ts index 062ad165..1a58ecbd 100644 --- a/src/frontend/src/tests/requestPolicyUi.node.test.ts +++ b/src/frontend/src/tests/requestPolicyUi.node.test.ts @@ -9,15 +9,11 @@ import { describe('requestPolicyUi', () => { const baseDownload: ButtonStateInfo = { text: 'Download', state: 'download' }; - it('maps direct mode to request for request_release/request_book', () => { + it('maps direct mode to request for request_release', () => { assert.deepEqual(applyDirectPolicyModeToButtonState(baseDownload, 'request_release'), { text: 'Request', state: 'download', }); - assert.deepEqual(applyDirectPolicyModeToButtonState(baseDownload, 'request_book'), { - text: 'Request', - state: 'download', - }); }); it('maps direct mode to unavailable for blocked', () => { diff --git a/src/frontend/src/utils/requestPayload.ts b/src/frontend/src/utils/requestPayload.ts index a8286095..daad3fe8 100644 --- a/src/frontend/src/utils/requestPayload.ts +++ b/src/frontend/src/utils/requestPayload.ts @@ -3,7 +3,6 @@ import { ContentType, CreateRequestPayload, Release, - RequestPolicyMode, } from '../types'; export const toContentType = (value: ContentType | string): ContentType => { @@ -86,26 +85,13 @@ export const buildReleaseDataFromDirectBook = (book: Book) => { }; export const buildDirectRequestPayload = ( - book: Book, - mode: Extract + book: Book ): CreateRequestPayload => { const bookData = buildDirectBookRequestData(book); // In direct mode, every result already represents a concrete downloadable release. - // Even when policy defaults resolve to request_book, attach the selected release so - // admins can approve immediately or browse alternatives from the same record. - if (mode === 'request_book') { - return { - book_data: bookData, - release_data: buildReleaseDataFromDirectBook(book), - context: { - source: 'direct_download', - content_type: 'ebook', - request_level: 'release', - }, - }; - } - + // Keep request payloads release-level so admins can approve immediately while still + // allowing alternate release selection from the same direct record. return { book_data: bookData, release_data: buildReleaseDataFromDirectBook(book), diff --git a/src/frontend/src/utils/requestPolicyUi.ts b/src/frontend/src/utils/requestPolicyUi.ts index 522748bc..738edf48 100644 --- a/src/frontend/src/utils/requestPolicyUi.ts +++ b/src/frontend/src/utils/requestPolicyUi.ts @@ -12,7 +12,7 @@ export const applyDirectPolicyModeToButtonState = ( return { text: 'Unavailable', state: 'blocked' }; } - if (mode === 'request_release' || mode === 'request_book') { + if (mode === 'request_release') { return { text: 'Request', state: 'download' }; } @@ -37,4 +37,3 @@ export const applyUniversalPolicyModeToButtonState = ( return { ...baseState, text: 'Get' }; }; - diff --git a/tests/config/test_security.py b/tests/config/test_security.py index f9f26e4c..1b90254d 100644 --- a/tests/config/test_security.py +++ b/tests/config/test_security.py @@ -324,6 +324,18 @@ class TestSecuritySettings: notice = next((f for f in fields if f.key == "builtin_auth_notice"), None) assert notice is None + def test_builtin_admin_requirement_hint_present(self): + """Builtin mode should show local-admin requirement warning.""" + from shelfmark.config.security import security_settings + + fields = security_settings() + hint = next((f for f in fields if f.key == "builtin_admin_requirement"), None) + assert hint is not None + assert hint.component == "oidc_admin_hint" + assert hint.show_when == {"field": "AUTH_METHOD", "value": "builtin"} + assert "inactive" in hint.label.lower() + assert "local admin" in hint.label.lower() + def test_builtin_option_label_is_local(self): """Builtin auth option should be labeled Local.""" from shelfmark.config.security import security_settings diff --git a/tests/core/test_admin_users_api.py b/tests/core/test_admin_users_api.py index c15336c2..30c6f943 100644 --- a/tests/core/test_admin_users_api.py +++ b/tests/core/test_admin_users_api.py @@ -57,7 +57,7 @@ def regular_client(app): with client.session_transaction() as sess: sess["user_id"] = "user" sess["is_admin"] = False - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="builtin"): yield client @@ -71,7 +71,7 @@ def no_session_client(app): def no_session_auth_client(app): """Client with no session but auth mode enabled (should be rejected).""" client = app.test_client() - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="builtin"): yield client @@ -129,7 +129,7 @@ class TestAdminUsersListEndpoint: ) user_db.create_user(username="proxy_user", auth_source="proxy") - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="builtin"): resp = admin_client.get("/api/admin/users") assert resp.status_code == 200 @@ -340,7 +340,7 @@ class TestAdminUserCreateEndpoint: assert resp.json["role"] == "user" def test_create_user_rejected_in_proxy_mode(self, admin_client): - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="proxy"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="proxy"): resp = admin_client.post( "/api/admin/users", json={"username": "alice", "password": "pass1234"}, @@ -350,7 +350,7 @@ class TestAdminUserCreateEndpoint: assert "Local user creation is disabled" in resp.json["error"] def test_create_user_rejected_in_cwa_mode(self, admin_client): - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="cwa"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="cwa"): resp = admin_client.post( "/api/admin/users", json={"username": "alice", "password": "pass1234"}, @@ -360,7 +360,7 @@ class TestAdminUserCreateEndpoint: assert "Local user creation is disabled" in resp.json["error"] def test_create_user_allowed_in_oidc_mode(self, admin_client): - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="oidc"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="oidc"): resp = admin_client.post( "/api/admin/users", json={"username": "alice", "password": "pass1234"}, @@ -932,7 +932,7 @@ class TestAdminSyncCwaUsersEndpoint: auth_source="cwa", ) - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="cwa"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="cwa"): with patch("shelfmark.core.admin_routes.CWA_DB_PATH", cwa_db_path): resp = admin_client.post("/api/admin/users/sync-cwa") @@ -966,7 +966,7 @@ class TestAdminSyncCwaUsersEndpoint: assert user_db.get_user(user_id=stale_cwa["id"]) is None def test_sync_cwa_users_rejected_when_not_in_cwa_mode(self, admin_client): - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="builtin"): resp = admin_client.post("/api/admin/users/sync-cwa") assert resp.status_code == 400 @@ -974,7 +974,7 @@ class TestAdminSyncCwaUsersEndpoint: def test_sync_cwa_users_returns_503_when_db_unavailable(self, admin_client, tmp_path): missing_db_path = tmp_path / "missing.db" - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="cwa"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="cwa"): with patch("shelfmark.core.admin_routes.CWA_DB_PATH", missing_db_path): resp = admin_client.post("/api/admin/users/sync-cwa") @@ -1578,7 +1578,7 @@ class TestAdminUserDeleteEndpoint: def test_delete_active_proxy_user_allowed(self, admin_client, user_db): user = user_db.create_user(username="proxyuser", auth_source="proxy") - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="proxy"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="proxy"): resp = admin_client.delete(f"/api/admin/users/{user['id']}") assert resp.status_code == 200 @@ -1587,7 +1587,7 @@ class TestAdminUserDeleteEndpoint: def test_delete_active_cwa_user_rejected(self, admin_client, user_db): user = user_db.create_user(username="cwauser", auth_source="cwa") - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="cwa"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="cwa"): resp = admin_client.delete(f"/api/admin/users/{user['id']}") assert resp.status_code == 400 @@ -1596,7 +1596,7 @@ class TestAdminUserDeleteEndpoint: def test_delete_inactive_proxy_user_allowed(self, admin_client, user_db): user = user_db.create_user(username="proxyuser", auth_source="proxy") - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="builtin"): resp = admin_client.delete(f"/api/admin/users/{user['id']}") assert resp.status_code == 200 @@ -1609,7 +1609,7 @@ class TestAdminUserDeleteEndpoint: auth_source="oidc", ) - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="oidc"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="oidc"): resp = admin_client.delete(f"/api/admin/users/{user['id']}") assert resp.status_code == 200 @@ -1622,7 +1622,7 @@ class TestAdminUserDeleteEndpoint: role="admin", ) - with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.admin_routes.load_active_auth_mode", return_value="builtin"): resp = admin_client.delete(f"/api/admin/users/{user['id']}") assert resp.status_code == 200 diff --git a/tests/core/test_per_user_downloads.py b/tests/core/test_per_user_downloads.py index 5c11f5ac..eddf1a6f 100644 --- a/tests/core/test_per_user_downloads.py +++ b/tests/core/test_per_user_downloads.py @@ -103,8 +103,8 @@ class TestQueueFilterByUser: all_tasks.update(tasks_by_status) assert len(all_tasks) == 2 - def test_get_status_user_filter_includes_legacy_tasks(self): - """Tasks without user_id are visible to any user (backward compat).""" + def test_get_status_user_filter_excludes_legacy_tasks(self): + """Tasks without user_id are admin-only and hidden from user-scoped views.""" q = BookQueue() q.add(self._make_task("book-1", user_id=None)) q.add(self._make_task("book-2", user_id=1)) @@ -113,8 +113,9 @@ class TestQueueFilterByUser: all_tasks = {} for tasks_by_status in status.values(): all_tasks.update(tasks_by_status) - # User 1 sees their own + legacy (no user_id) - assert len(all_tasks) == 2 + assert len(all_tasks) == 1 + assert "book-1" not in all_tasks + assert "book-2" in all_tasks def test_clear_completed_for_user_only_removes_user_terminal_tasks(self): q = BookQueue() @@ -138,7 +139,7 @@ class TestQueueFilterByUser: assert "book-2" in all_tasks assert "book-3" in all_tasks - def test_clear_completed_for_user_includes_legacy_tasks(self): + def test_clear_completed_for_user_excludes_legacy_tasks(self): q = BookQueue() q.add(self._make_task("legacy-book", user_id=None)) q.add(self._make_task("user-book", user_id=1)) @@ -147,7 +148,15 @@ class TestQueueFilterByUser: q.update_status("user-book", QueueStatus.COMPLETE) removed = q.clear_completed(user_id=1) - assert removed == 2 + assert removed == 1 + + status = q.get_status() + all_tasks = {} + for tasks_by_status in status.values(): + all_tasks.update(tasks_by_status) + + assert "legacy-book" in all_tasks + assert "user-book" not in all_tasks def test_enqueue_existing_deduplicates_queue_entries(self): q = BookQueue() diff --git a/tests/core/test_request_policy.py b/tests/core/test_request_policy.py index 05bab854..f1c46518 100644 --- a/tests/core/test_request_policy.py +++ b/tests/core/test_request_policy.py @@ -196,7 +196,7 @@ def test_resolve_policy_mode_caps_at_content_type_default_ceiling(): def test_resolve_policy_mode_request_book_ceiling_overrides_all_rules(): - """When default is request_book, no matrix rule can open the release modal.""" + """request_book defaults stay capped for non-direct sources.""" settings = { "REQUEST_POLICY_DEFAULT_EBOOK": "request_book", "REQUEST_POLICY_DEFAULT_AUDIOBOOK": "blocked", @@ -206,17 +206,18 @@ def test_resolve_policy_mode_request_book_ceiling_overrides_all_rules(): ], } - # Both rules try to upgrade beyond request_book → capped + # Prowlarr rule tries to upgrade beyond request_book → capped assert resolve_policy_mode( source="prowlarr", content_type="ebook", global_settings=settings, ) == PolicyMode.REQUEST_BOOK + # Direct-download requests are concrete releases, so request_book normalizes to request_release. assert resolve_policy_mode( source="direct_download", content_type="ebook", global_settings=settings, - ) == PolicyMode.REQUEST_BOOK + ) == PolicyMode.REQUEST_RELEASE # audiobook default is blocked → even more restrictive ceiling assert resolve_policy_mode( source="prowlarr", @@ -236,7 +237,7 @@ def test_resolve_policy_mode_falls_back_to_request_book_when_unset(): source="direct_download", content_type="ebook", global_settings=settings, - ) == PolicyMode.REQUEST_BOOK + ) == PolicyMode.REQUEST_RELEASE assert resolve_policy_mode( source="prowlarr", content_type="audiobook", diff --git a/tests/core/test_request_routes_api.py b/tests/core/test_request_routes_api.py index c27d80bb..ef93e16f 100644 --- a/tests/core/test_request_routes_api.py +++ b/tests/core/test_request_routes_api.py @@ -75,10 +75,10 @@ class TestDownloadPolicyGuards: with patch.object(main_module, "get_auth_mode", return_value="builtin"): with patch.object( main_module, - "_load_users_request_policy_settings", + "load_users_request_policy_settings", return_value=_policy(default_ebook="request_release"), ): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=_policy(default_ebook="request_release")): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=_policy(default_ebook="request_release")): with patch.object(main_module.backend, "queue_book") as mock_queue_book: resp = client.get("/api/download?id=book-123") @@ -94,10 +94,10 @@ class TestDownloadPolicyGuards: with patch.object(main_module, "get_auth_mode", return_value="builtin"): with patch.object( main_module, - "_load_users_request_policy_settings", + "load_users_request_policy_settings", return_value=_policy(default_ebook="blocked"), ): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=_policy(default_ebook="blocked")): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=_policy(default_ebook="blocked")): with patch.object(main_module.backend, "queue_release") as mock_queue_release: resp = client.post( "/api/releases/download", @@ -116,10 +116,10 @@ class TestDownloadPolicyGuards: with patch.object(main_module, "get_auth_mode", return_value="builtin"): with patch.object( main_module, - "_load_users_request_policy_settings", + "load_users_request_policy_settings", return_value=_policy(default_ebook="blocked"), ): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=_policy(default_ebook="blocked")): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=_policy(default_ebook="blocked")): with patch.object(main_module.backend, "queue_book", return_value=(True, None)) as mock_queue_book: resp = client.get("/api/download?id=book-123") @@ -131,10 +131,10 @@ class TestDownloadPolicyGuards: with patch.object(main_module, "get_auth_mode", return_value="none"): with patch.object( main_module, - "_load_users_request_policy_settings", + "load_users_request_policy_settings", return_value=_policy(default_ebook="blocked"), ): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=_policy(default_ebook="blocked")): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=_policy(default_ebook="blocked")): with patch.object(main_module.backend, "queue_book", return_value=(True, None)) as mock_queue_book: resp = client.get("/api/download?id=book-123") @@ -157,8 +157,8 @@ class TestRequestRoutes: policy = _policy(default_ebook="request_release") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.get("/api/request-policy") assert resp.status_code == 200 @@ -166,6 +166,25 @@ class TestRequestRoutes: assert resp.json["defaults"]["ebook"] == "request_release" assert "source_modes" in resp.json + def test_request_policy_endpoint_normalizes_direct_request_book_to_request_release(self, main_module, client): + user = _create_user(main_module, prefix="reader") + _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) + policy = _policy(default_ebook="request_book") + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): + with patch( + "shelfmark.core.request_routes.get_source_content_type_capabilities", + return_value={"direct_download": {"ebook"}}, + ): + resp = client.get("/api/request-policy") + + assert resp.status_code == 200 + assert resp.json["defaults"]["ebook"] == "request_book" + assert resp.json["source_modes"][0]["source"] == "direct_download" + assert resp.json["source_modes"][0]["modes"]["ebook"] == "request_release" + def test_create_list_and_cancel_request(self, main_module, client): user = _create_user(main_module, prefix="reader") _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) @@ -188,8 +207,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=payload) list_resp = client.get("/api/requests") @@ -235,8 +254,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch.object(main_module.ws_manager, "is_enabled", return_value=True): with patch.object(main_module.ws_manager.socketio, "emit") as mock_emit: resp = client.post("/api/requests", json=payload) @@ -275,8 +294,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch("shelfmark.core.request_routes.notify_admin") as mock_notify: with patch("shelfmark.core.request_routes.notify_user") as mock_notify_user: resp = client.post("/api/requests", json=payload) @@ -315,8 +334,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch( "shelfmark.core.request_routes.notify_admin", side_effect=RuntimeError("admin notification unavailable"), @@ -353,8 +372,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch.object(main_module.ws_manager, "is_enabled", return_value=True): with patch.object(main_module.ws_manager.socketio, "emit") as mock_emit: create_resp = client.post("/api/requests", json=payload) @@ -397,8 +416,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json=payload) assert resp.status_code == 400 @@ -425,8 +444,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): first_resp = client.post("/api/requests", json=payload) second_resp = client.post("/api/requests", json=payload) @@ -469,8 +488,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): first_resp = client.post("/api/requests", json=payload_1) second_resp = client.post("/api/requests", json=payload_2) @@ -500,8 +519,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json=payload) assert resp.status_code == 201 @@ -533,8 +552,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json=payload) assert resp.status_code == 403 @@ -569,13 +588,13 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json=payload) assert resp.status_code == 201 assert resp.json["request_level"] == "release" - assert resp.json["policy_mode"] == "request_book" + assert resp.json["policy_mode"] == "request_release" assert resp.json["release_data"]["source"] == "direct_download" assert resp.json["release_data"]["source_id"] == "dd-1" @@ -611,8 +630,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -667,8 +686,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -710,8 +729,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -773,8 +792,8 @@ class TestRequestRoutes: return True, None with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -818,8 +837,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -867,8 +886,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -909,15 +928,15 @@ class TestRequestRoutes: "provider_id": "ol-7", }, "context": { - "source": "direct_download", + "source": "prowlarr", "content_type": "ebook", "request_level": "book", }, } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -942,15 +961,15 @@ class TestRequestRoutes: "provider_id": "ol-manual-approval", }, "context": { - "source": "direct_download", + "source": "prowlarr", "content_type": "ebook", "request_level": "book", }, } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -984,7 +1003,7 @@ class TestRequestRoutes: "provider_id": "ol-book-fulfil", }, "context": { - "source": "direct_download", + "source": "prowlarr", "content_type": "ebook", "request_level": "book", }, @@ -1000,8 +1019,8 @@ class TestRequestRoutes: return True, None with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -1011,7 +1030,7 @@ class TestRequestRoutes: f"/api/admin/requests/{request_id}/fulfil", json={ "release_data": { - "source": "direct_download", + "source": "prowlarr", "source_id": "book-level-picked-release", "title": "Book Level Fulfil.epub", } @@ -1057,8 +1076,8 @@ class TestRequestRoutes: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json=create_payload) request_id = create_resp.json["id"] @@ -1085,8 +1104,8 @@ class TestRequestCreationEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", content_type="text/plain", data="garbage") assert resp.status_code == 400 @@ -1098,8 +1117,8 @@ class TestRequestCreationEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json={"context": {"source": "direct_download"}}) assert resp.status_code == 400 @@ -1111,8 +1130,8 @@ class TestRequestCreationEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json={ "context": "not-a-dict", "book_data": {"title": "X", "author": "Y", "provider": "z", "provider_id": "1"}, @@ -1127,8 +1146,8 @@ class TestRequestCreationEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json={ "book_data": {"title": "Only a title"}, "context": {"source": "direct_download", "content_type": "ebook", "request_level": "book"}, @@ -1155,8 +1174,8 @@ class TestRequestCreationEdgeCases: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json=payload) assert resp.status_code == 400 @@ -1168,8 +1187,8 @@ class TestRequestCreationEdgeCases: policy = _policy(requests_enabled=False, default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json={ "book_data": {"title": "T", "author": "A", "provider": "p", "provider_id": "1"}, "context": {"source": "direct_download", "content_type": "ebook", "request_level": "book"}, @@ -1184,8 +1203,8 @@ class TestRequestCreationEdgeCases: policy = _policy(default_ebook="blocked") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json={ "book_data": {"title": "T", "author": "A", "provider": "p", "provider_id": "1", "content_type": "ebook"}, "context": {"source": "direct_download", "content_type": "ebook", "request_level": "book"}, @@ -1195,7 +1214,7 @@ class TestRequestCreationEdgeCases: assert resp.json["code"] == "policy_blocked" assert resp.json["required_mode"] == "blocked" - def test_auto_infers_book_level_when_no_release_data(self, main_module, client): + def test_direct_requests_are_forced_to_release_level(self, main_module, client): user = _create_user(main_module, prefix="reader") _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) policy = _policy(default_ebook="request_book") @@ -1212,12 +1231,15 @@ class TestRequestCreationEdgeCases: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json=payload) assert resp.status_code == 201 - assert resp.json["request_level"] == "book" + assert resp.json["request_level"] == "release" + assert resp.json["policy_mode"] == "request_release" + assert resp.json["release_data"]["source"] == "direct_download" + assert resp.json["release_data"]["source_id"] == "ol-auto-1" def test_auto_infers_release_level_when_release_data_present(self, main_module, client): user = _create_user(main_module, prefix="reader") @@ -1237,8 +1259,8 @@ class TestRequestCreationEdgeCases: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json=payload) assert resp.status_code == 201 @@ -1263,8 +1285,8 @@ class TestRequestCreationEdgeCases: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json=payload) assert resp.status_code == 201 @@ -1305,8 +1327,8 @@ class TestRequestCreationEdgeCases: } with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.post("/api/requests", json=payload) assert resp.status_code == 201 @@ -1352,8 +1374,8 @@ class TestRequestListAndFilterEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): ids = self._seed_requests(main_module, client, user, policy, count=3) # Cancel the first request. @@ -1378,8 +1400,8 @@ class TestRequestListAndFilterEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): self._seed_requests(main_module, client, user, policy, count=5) page1 = client.get("/api/requests?limit=2&offset=0") @@ -1402,8 +1424,8 @@ class TestRequestListAndFilterEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): # Alice creates a request. _set_session(client, user_id=alice["username"], db_user_id=alice["id"], is_admin=False) client.post("/api/requests", json={ @@ -1437,8 +1459,8 @@ class TestRequestListAndFilterEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) client.post("/api/requests", json={ "book_data": {"title": "Admin View", "author": "AV", "provider": "p", "provider_id": "av1", "content_type": "ebook"}, @@ -1459,8 +1481,8 @@ class TestRequestListAndFilterEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) create_resp = client.post("/api/requests", json={ "book_data": {"title": f"FilterTest-{uuid.uuid4().hex[:6]}", "author": "FT", "provider": "p", "provider_id": f"ft-{uuid.uuid4().hex[:6]}", "content_type": "ebook"}, @@ -1498,8 +1520,8 @@ class TestCancelEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): _set_session(client, user_id=alice["username"], db_user_id=alice["id"], is_admin=False) create_resp = client.post("/api/requests", json={ "book_data": {"title": "Alice Only", "author": "A", "provider": "p", "provider_id": "ao1", "content_type": "ebook"}, @@ -1518,8 +1540,8 @@ class TestCancelEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json={ "book_data": {"title": "Cancel Twice", "author": "CT", "provider": "p", "provider_id": "ct1", "content_type": "ebook"}, "context": {"source": "direct_download", "content_type": "ebook", "request_level": "book"}, @@ -1556,8 +1578,8 @@ class TestAdminFulfilEdgeCases: _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json={ "book_data": {"title": "Queue Fail", "author": "QF", "provider": "p", "provider_id": "qf1", "content_type": "ebook"}, "context": {"source": "prowlarr", "content_type": "ebook", "request_level": "release"}, @@ -1586,8 +1608,8 @@ class TestAdminFulfilEdgeCases: return True, None with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json={ "book_data": {"title": "Override RD", "author": "OR", "provider": "p", "provider_id": "or1", "content_type": "ebook"}, "context": {"source": "prowlarr", "content_type": "ebook", "request_level": "release"}, @@ -1626,8 +1648,8 @@ class TestAdminFulfilEdgeCases: _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json={ "book_data": { "title": "Manual Flag Validation", @@ -1672,8 +1694,8 @@ class TestAdminRejectEdgeCases: _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): create_resp = client.post("/api/requests", json={ "book_data": {"title": "Rej After Ful", "author": "RAF", "provider": "p", "provider_id": "raf1", "content_type": "ebook"}, "context": {"source": "prowlarr", "content_type": "ebook", "request_level": "release"}, @@ -1700,8 +1722,8 @@ class TestAdminCountEdgeCases: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) # Create 3 requests. @@ -1747,8 +1769,8 @@ class TestPolicyEndpointEdgeCases: policy = _policy(default_ebook="download") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.get("/api/request-policy") assert resp.status_code == 200 @@ -1763,8 +1785,8 @@ class TestPolicyEndpointEdgeCases: main_module.user_db.set_user_settings(user["id"], {"REQUEST_POLICY_DEFAULT_EBOOK": "request_release"}) with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=global_policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=global_policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=global_policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=global_policy): resp = client.get("/api/request-policy") assert resp.status_code == 200 @@ -1783,8 +1805,8 @@ class TestPolicyEndpointEdgeCases: policy = _policy(default_ebook="download", requests_allow_notes=False) with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): resp = client.get("/api/request-policy") assert resp.status_code == 200 @@ -1798,8 +1820,8 @@ class TestPolicyEndpointEdgeCases: main_module.user_db.set_user_settings(user["id"], {"REQUESTS_ALLOW_NOTES": True}) with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=global_policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=global_policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=global_policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=global_policy): resp = client.get("/api/request-policy") assert resp.status_code == 200 @@ -1818,8 +1840,8 @@ class TestDownloadPolicyGuardsExtended: policy = _policy(requests_enabled=False, default_ebook="blocked") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch.object(main_module.backend, "queue_book", return_value=(True, None)): resp = client.get("/api/download?id=book-pass") @@ -1832,8 +1854,8 @@ class TestDownloadPolicyGuardsExtended: policy = _policy(default_ebook="download") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch.object(main_module.backend, "queue_book", return_value=(True, None)): resp = client.get("/api/download?id=book-free") @@ -1846,8 +1868,8 @@ class TestDownloadPolicyGuardsExtended: policy = _policy(default_ebook="request_release") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch.object(main_module.backend, "queue_release") as mock_queue: resp = client.post("/api/releases/download", json={ "source": "prowlarr", @@ -1866,8 +1888,8 @@ class TestDownloadPolicyGuardsExtended: policy = _policy(default_ebook="download", default_audiobook="blocked") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch.object(main_module.backend, "queue_release") as mock_queue: resp = client.post("/api/releases/download", json={ "source": "prowlarr", @@ -1887,8 +1909,8 @@ class TestDownloadPolicyGuardsExtended: policy = _policy(default_ebook="request_book") with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch.object(main_module.backend, "queue_release") as mock_queue: resp = client.post("/api/releases/download", json={ "source": "direct_download", @@ -1898,7 +1920,7 @@ class TestDownloadPolicyGuardsExtended: assert resp.status_code == 403 assert resp.json["code"] == "policy_requires_request" - assert resp.json["required_mode"] == "request_book" + assert resp.json["required_mode"] == "request_release" mock_queue.assert_not_called() def test_release_download_with_per_source_matrix_rule(self, main_module, client): @@ -1911,8 +1933,8 @@ class TestDownloadPolicyGuardsExtended: ) with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=policy): with patch.object(main_module.backend, "queue_release") as mock_queue: # Prowlarr should be blocked. prowlarr_resp = client.post("/api/releases/download", json={ @@ -1953,8 +1975,8 @@ class TestDownloadPolicyGuardsExtended: }) with patch.object(main_module, "get_auth_mode", return_value="builtin"): - with patch.object(main_module, "_load_users_request_policy_settings", return_value=global_policy): - with patch("shelfmark.core.request_routes._load_users_request_policy_settings", return_value=global_policy): + with patch.object(main_module, "load_users_request_policy_settings", return_value=global_policy): + with patch("shelfmark.core.request_routes.load_users_request_policy_settings", return_value=global_policy): with patch.object(main_module.backend, "queue_release", return_value=(True, None)): resp = client.post("/api/releases/download", json={ "source": "prowlarr", diff --git a/tests/core/test_requests_service.py b/tests/core/test_requests_service.py index 05254bbe..9a1b8314 100644 --- a/tests/core/test_requests_service.py +++ b/tests/core/test_requests_service.py @@ -6,6 +6,14 @@ import tempfile import pytest from shelfmark.core.request_policy import PolicyMode +from shelfmark.core.request_validation import ( + normalize_delivery_state, + normalize_policy_mode, + normalize_request_level, + normalize_request_status, + validate_request_level_payload, + validate_status_transition, +) from shelfmark.core.requests_service import ( MAX_REQUEST_JSON_BLOB_BYTES, MAX_REQUEST_NOTE_LENGTH, @@ -13,15 +21,9 @@ from shelfmark.core.requests_service import ( cancel_request, create_request, fulfil_request, - normalize_policy_mode, - normalize_delivery_state, - normalize_request_level, - normalize_request_status, reopen_failed_request, reject_request, sync_delivery_states_from_queue_status, - validate_request_level_payload, - validate_status_transition, ) from shelfmark.core.user_db import UserDB @@ -565,6 +567,33 @@ def test_reopen_failed_request_reverts_to_pending_from_queued_and_clears_on_refu assert refulfilled["last_failure_reason"] is None +def test_reopen_failed_request_delegates_to_user_db_public_api(): + class StubUserDB: + def __init__(self) -> None: + self.calls: list[tuple[int, str | None]] = [] + + def reopen_failed_request( + self, + request_id: int, + *, + failure_reason: str | None = None, + ) -> dict[str, object]: + self.calls.append((request_id, failure_reason)) + return {"id": request_id, "status": "pending"} + + user_db = StubUserDB() + + reopened = reopen_failed_request( + user_db, # type: ignore[arg-type] + request_id=7, + failure_reason=" Download failed: Timeout ", + ) + + assert reopened == {"id": 7, "status": "pending"} + # Service layer passes failure_reason through; normalization happens in UserDB. + assert user_db.calls == [(7, " Download failed: Timeout ")] + + def test_reopen_failed_request_does_not_reopen_completed_delivery(user_db): alice = user_db.create_user(username="alice") admin = user_db.create_user(username="admin", role="admin") diff --git a/tests/core/test_self_user_routes.py b/tests/core/test_self_user_routes.py index 3a952b85..036cdd26 100644 --- a/tests/core/test_self_user_routes.py +++ b/tests/core/test_self_user_routes.py @@ -60,7 +60,7 @@ def test_users_me_edit_context_respects_visible_sections(app, user_db): } raise AssertionError(f"Unexpected tab requested: {tab_name}") - with patch("shelfmark.core.self_user_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.self_user_routes.load_active_auth_mode", return_value="builtin"): with patch( "shelfmark.core.self_user_routes.load_config_file", side_effect=lambda tab_name: {"VISIBLE_SELF_SETTINGS_SECTIONS": ["delivery"]} if tab_name == "users" else {}, @@ -105,7 +105,7 @@ def test_users_me_edit_context_includes_search_preferences_when_visible(app, use raise AssertionError(f"Unexpected tab requested: {tab_name}") return payloads[tab_name] - with patch("shelfmark.core.self_user_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.self_user_routes.load_active_auth_mode", return_value="builtin"): with patch( "shelfmark.core.self_user_routes.load_config_file", side_effect=lambda tab_name: { @@ -135,7 +135,7 @@ def test_users_me_update_rejects_hidden_section_settings(app, user_db): return [("DESTINATION", object())] raise AssertionError(f"Unexpected tab requested: {tab_name}") - with patch("shelfmark.core.self_user_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.self_user_routes.load_active_auth_mode", return_value="builtin"): with patch( "shelfmark.core.self_user_routes.load_config_file", side_effect=lambda tab_name: {"VISIBLE_SELF_SETTINGS_SECTIONS": ["delivery"]} if tab_name == "users" else {}, @@ -167,7 +167,7 @@ def test_users_me_update_accepts_visible_section_settings(app, user_db): return [("DESTINATION", object())] raise AssertionError(f"Unexpected tab requested: {tab_name}") - with patch("shelfmark.core.self_user_routes._get_auth_mode", return_value="builtin"): + with patch("shelfmark.core.self_user_routes.load_active_auth_mode", return_value="builtin"): with patch( "shelfmark.core.self_user_routes.load_config_file", side_effect=lambda tab_name: {"VISIBLE_SELF_SETTINGS_SECTIONS": ["delivery"]} if tab_name == "users" else {}, diff --git a/tests/core/test_user_db.py b/tests/core/test_user_db.py index e17b0798..6e49edf5 100644 --- a/tests/core/test_user_db.py +++ b/tests/core/test_user_db.py @@ -628,6 +628,70 @@ class TestDownloadRequests: with pytest.raises(ValueError, match="release_data must be an object when provided"): user_db.update_request(created["id"], release_data="not-an-object") + def test_reopen_failed_request_resets_fulfilled_request_for_reapproval(self, user_db): + user = user_db.create_user(username="alice") + created = user_db.create_request( + user_id=user["id"], + content_type="ebook", + request_level="release", + policy_mode="request_release", + book_data=self._book_data(), + release_data=self._release_data(), + status="fulfilled", + delivery_state="queued", + reviewed_by=user["id"], + reviewed_at="2026-01-01T00:00:00+00:00", + delivery_updated_at="2026-01-01T00:00:01+00:00", + ) + + reopened = user_db.reopen_failed_request( + created["id"], + failure_reason=" Download timed out ", + ) + + assert reopened is not None + assert reopened["status"] == "pending" + assert reopened["delivery_state"] == "none" + assert reopened["delivery_updated_at"] is None + assert reopened["release_data"] is None + assert reopened["last_failure_reason"] == "Download timed out" + assert reopened["reviewed_by"] is None + assert reopened["reviewed_at"] is None + + def test_reopen_failed_request_requires_reason_for_non_failure_states(self, user_db): + user = user_db.create_user(username="alice") + created = user_db.create_request( + user_id=user["id"], + content_type="ebook", + request_level="release", + policy_mode="request_release", + book_data=self._book_data(), + release_data=self._release_data(), + status="fulfilled", + delivery_state="queued", + ) + + reopened = user_db.reopen_failed_request(created["id"]) + assert reopened is None + + def test_reopen_failed_request_allows_failure_states_without_reason(self, user_db): + user = user_db.create_user(username="alice") + created = user_db.create_request( + user_id=user["id"], + content_type="ebook", + request_level="release", + policy_mode="request_release", + book_data=self._book_data(), + release_data=self._release_data(), + status="fulfilled", + delivery_state="error", + ) + + reopened = user_db.reopen_failed_request(created["id"]) + assert reopened is not None + assert reopened["status"] == "pending" + assert reopened["last_failure_reason"] is None + def test_count_pending_requests(self, user_db): alice = user_db.create_user(username="alice") bob = user_db.create_user(username="bob") diff --git a/tests/download/test_fs_run_blocking_io.py b/tests/download/test_fs_run_blocking_io.py new file mode 100644 index 00000000..5dd691f3 --- /dev/null +++ b/tests/download/test_fs_run_blocking_io.py @@ -0,0 +1,51 @@ +from __future__ import annotations + +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest + + +def test_run_blocking_io_handles_gevent_patched_subprocess_run() -> None: + pytest.importorskip("gevent") + + repo_root = Path(__file__).resolve().parents[2] + script = textwrap.dedent( + """ + import os + import subprocess + import sys + import tempfile + + from gevent import monkey + + monkey.patch_all() + os.environ.setdefault("LOG_ROOT", tempfile.mkdtemp(prefix="shelfmark-log-root-")) + + from shelfmark.download.fs import run_blocking_io + + result = run_blocking_io( + subprocess.run, + [sys.executable, "-c", "print('ok')"], + check=True, + capture_output=True, + text=True, + ) + assert result.stdout.strip() == "ok" + """ + ) + + completed = subprocess.run( + [sys.executable, "-c", script], + cwd=str(repo_root), + capture_output=True, + text=True, + ) + + assert completed.returncode == 0, ( + f"Subprocess failed with exit code {completed.returncode}\n" + f"stdout:\n{completed.stdout}\n" + f"stderr:\n{completed.stderr}" + )