diff --git a/shelfmark/core/activity_routes.py b/shelfmark/core/activity_routes.py index 903bb494..c58d0700 100644 --- a/shelfmark/core/activity_routes.py +++ b/shelfmark/core/activity_routes.py @@ -12,6 +12,8 @@ from shelfmark.core.user_db import 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() @@ -50,6 +52,49 @@ def _resolve_db_user_id(require_in_auth_mode: bool = True): ) +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) + if user is None: + try: + user_db.create_user( + 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) + + if user is None: + return None + user_id = int(user.get("id")) + return user_id if user_id > 0 else None + except Exception as exc: + logger.warning("Failed to resolve no-auth activity identity: %s", exc) + return None + + +def _resolve_activity_actor_user_id( + *, + user_db: UserDB, + resolve_auth_mode: Callable[[], str], +) -> tuple[int | None, Any | None]: + """Resolve acting user identity for activity mutations.""" + db_user_id, db_gate = _resolve_db_user_id() + if db_user_id is not None: + return db_user_id, None + + if resolve_auth_mode() == "none": + no_auth_user_id = _ensure_no_auth_activity_user_id(user_db) + if no_auth_user_id is not None: + return no_auth_user_id, None + + return None, db_gate + + def _emit_activity_event(ws_manager: Any | None, *, room: str, payload: dict[str, Any]) -> None: if ws_manager is None: return @@ -316,6 +361,8 @@ def register_activity_routes( ) viewer_db_user_id, _ = _resolve_db_user_id(require_in_auth_mode=False) + if viewer_db_user_id is None and resolve_auth_mode() == "none": + viewer_db_user_id = _ensure_no_auth_activity_user_id(user_db) scoped_user_id = None if is_admin else db_user_id status = queue_status(user_id=scoped_user_id) updated_requests = sync_request_delivery_states( @@ -371,7 +418,10 @@ def register_activity_routes( if auth_gate is not None: return auth_gate - db_user_id, db_gate = _resolve_db_user_id() + db_user_id, db_gate = _resolve_activity_actor_user_id( + user_db=user_db, + resolve_auth_mode=resolve_auth_mode, + ) if db_gate is not None or db_user_id is None: return db_gate @@ -437,7 +487,10 @@ def register_activity_routes( if auth_gate is not None: return auth_gate - db_user_id, db_gate = _resolve_db_user_id() + db_user_id, db_gate = _resolve_activity_actor_user_id( + user_db=user_db, + resolve_auth_mode=resolve_auth_mode, + ) if db_gate is not None or db_user_id is None: return db_gate @@ -517,7 +570,10 @@ def register_activity_routes( if auth_gate is not None: return auth_gate - db_user_id, db_gate = _resolve_db_user_id() + db_user_id, db_gate = _resolve_activity_actor_user_id( + user_db=user_db, + resolve_auth_mode=resolve_auth_mode, + ) if db_gate is not None or db_user_id is None: return db_gate @@ -536,7 +592,10 @@ def register_activity_routes( if auth_gate is not None: return auth_gate - db_user_id, db_gate = _resolve_db_user_id() + db_user_id, db_gate = _resolve_activity_actor_user_id( + user_db=user_db, + resolve_auth_mode=resolve_auth_mode, + ) if db_gate is not None or db_user_id is None: return db_gate diff --git a/shelfmark/core/notifications.py b/shelfmark/core/notifications.py index 59e48dfd..201589f5 100644 --- a/shelfmark/core/notifications.py +++ b/shelfmark/core/notifications.py @@ -2,10 +2,13 @@ from __future__ import annotations +import logging +import threading from concurrent.futures import ThreadPoolExecutor +from contextlib import contextmanager from dataclasses import dataclass from enum import Enum -from typing import Any, Iterable +from typing import Any, Iterable, Iterator from urllib.parse import urlsplit try: @@ -26,6 +29,7 @@ _APPRISE_APP_DESC = "Shelfmark notifications" _APPRISE_LOGO_URL = ( "https://raw.githubusercontent.com/calibrain/shelfmark/main/src/frontend/public/logo.png" ) +_APPRISE_LOGGER_NAME = "apprise" class NotificationEvent(str, Enum): @@ -91,6 +95,56 @@ def _extract_url_schemes(urls: Iterable[str]) -> list[str]: return schemes +class _AppriseLogCapture(logging.Handler): + def __init__(self, *, thread_id: int): + super().__init__(level=logging.INFO) + self.records: list[tuple[int, str, str]] = [] + self._thread_id = thread_id + + def emit(self, record: logging.LogRecord) -> None: + if record.thread != self._thread_id: + return + + message = record.getMessage() + if message: + self.records.append((record.levelno, record.name, str(message))) + + +@contextmanager +def _capture_apprise_logs(*, min_level: int = logging.INFO) -> Iterator[list[tuple[int, str, str]]]: + apprise_logger = logging.getLogger(_APPRISE_LOGGER_NAME) + previous_level = apprise_logger.level + handler = _AppriseLogCapture(thread_id=threading.get_ident()) + apprise_logger.addHandler(handler) + + if previous_level == logging.NOTSET or previous_level > min_level: + apprise_logger.setLevel(min_level) + + try: + yield handler.records + finally: + apprise_logger.removeHandler(handler) + apprise_logger.setLevel(previous_level) + + +def _log_apprise_records(records: Iterable[tuple[int, str, str]]) -> None: + seen: set[tuple[int, str, str]] = set() + for level, source, raw_message in records: + message = str(raw_message or "").strip() + source_name = str(source or "").strip() or _APPRISE_LOGGER_NAME + key = (int(level), source_name, message) + if not message or key in seen: + continue + seen.add(key) + + if level >= logging.ERROR: + logger.error("Apprise source [%s]: %s", source_name, message) + elif level >= logging.WARNING: + logger.warning("Apprise source [%s]: %s", source_name, message) + else: + logger.info("Apprise source [%s]: %s", source_name, message) + + def _normalize_routes(value: Any) -> list[dict[str, str]]: if not isinstance(value, list): return [] @@ -251,45 +305,52 @@ def _dispatch_to_apprise( apobj = _create_apprise_client() if apobj is None: return {"success": False, "message": "Apprise is not installed"} - valid_urls = 0 - invalid_urls = 0 - for url in normalized_urls: - scheme = urlsplit(url).scheme or "unknown" - try: - added = bool(apobj.add(url)) - except Exception as exc: + with _capture_apprise_logs(min_level=logging.INFO) as apprise_records: + valid_urls = 0 + invalid_urls = 0 + for url in normalized_urls: + scheme = urlsplit(url).scheme or "unknown" + try: + added = bool(apobj.add(url)) + except Exception as exc: + logger.warning( + "Failed to register notification route URL for scheme '%s': %s", + scheme, + exc, + ) + added = False + if added: + valid_urls += 1 + else: + invalid_urls += 1 + logger.warning("Apprise rejected notification route URL for scheme '%s'", scheme) + + if valid_urls == 0: + _log_apprise_records(apprise_records) + scheme_summary = ", ".join(url_schemes) if url_schemes else "unknown" logger.warning( - "Failed to register notification route URL for scheme '%s': %s", - scheme, - exc, + "No valid Apprise notification routes after registration for scheme(s): %s", + scheme_summary, ) - added = False - if added: - valid_urls += 1 - else: - invalid_urls += 1 - logger.warning("Apprise rejected notification route URL for scheme '%s'", scheme) + return { + "success": False, + "message": "No valid notification URLs configured", + } - if valid_urls == 0: - scheme_summary = ", ".join(url_schemes) if url_schemes else "unknown" - logger.warning("No valid Apprise notification routes after registration for scheme(s): %s", scheme_summary) - return { - "success": False, - "message": "No valid notification URLs configured", - } - - try: - delivered = bool(apobj.notify(title=title, body=body, notify_type=notify_type)) - except Exception as exc: - scheme_summary = ", ".join(url_schemes) if url_schemes else "unknown" - logger.warning( - "Apprise notify raised %s for scheme(s): %s", - type(exc).__name__, - scheme_summary, - ) - return {"success": False, "message": f"Notification send failed: {type(exc).__name__}: {exc}"} + try: + delivered = bool(apobj.notify(title=title, body=body, notify_type=notify_type)) + except Exception as exc: + _log_apprise_records(apprise_records) + scheme_summary = ", ".join(url_schemes) if url_schemes else "unknown" + logger.warning( + "Apprise notify raised %s for scheme(s): %s", + type(exc).__name__, + scheme_summary, + ) + return {"success": False, "message": f"Notification send failed: {type(exc).__name__}: {exc}"} if not delivered: + _log_apprise_records(apprise_records) scheme_summary = ", ".join(url_schemes) if url_schemes else "unknown" logger.warning( "Apprise notify returned False for scheme(s): %s (valid_urls=%s invalid_urls=%s)", @@ -299,6 +360,8 @@ def _dispatch_to_apprise( ) return {"success": False, "message": "Notification delivery failed"} + _log_apprise_records(apprise_records) + message = f"Notification sent to {valid_urls} URL(s)" if invalid_urls: message += f" ({invalid_urls} invalid URL(s) skipped)" diff --git a/tests/core/test_activity_routes_api.py b/tests/core/test_activity_routes_api.py index 3ad84ed1..73018017 100644 --- a/tests/core/test_activity_routes_api.py +++ b/tests/core/test_activity_routes_api.py @@ -214,6 +214,35 @@ class TestActivityRoutes: to=f"user_{user['id']}", ) + def test_no_auth_dismiss_many_and_history_use_shared_identity(self, main_module): + item_key = f"download:no-auth-{uuid.uuid4().hex[:10]}" + + client_one = main_module.app.test_client() + client_two = main_module.app.test_client() + + with patch.object(main_module, "get_auth_mode", return_value="none"): + dismiss_many_response = client_one.post( + "/api/activity/dismiss-many", + json={"items": [{"item_type": "download", "item_key": item_key}]}, + ) + with patch.object(main_module.backend, "queue_status", return_value=_sample_status_payload()): + snapshot_one = client_one.get("/api/activity/snapshot") + snapshot_two = client_two.get("/api/activity/snapshot") + history_one = client_one.get("/api/activity/history?limit=10&offset=0") + + assert dismiss_many_response.status_code == 200 + assert dismiss_many_response.json["status"] == "dismissed" + assert dismiss_many_response.json["count"] == 1 + + assert snapshot_one.status_code == 200 + assert {"item_type": "download", "item_key": item_key} in snapshot_one.json["dismissed"] + + assert snapshot_two.status_code == 200 + assert {"item_type": "download", "item_key": item_key} in snapshot_two.json["dismissed"] + + assert history_one.status_code == 200 + assert any(row["item_key"] == item_key for row in history_one.json) + def test_queue_clear_does_not_set_request_delivery_state_to_cleared(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) diff --git a/tests/core/test_notifications.py b/tests/core/test_notifications.py index 29c935da..e674b26b 100644 --- a/tests/core/test_notifications.py +++ b/tests/core/test_notifications.py @@ -1,5 +1,7 @@ """Tests for core notification rendering and dispatch helpers.""" +import logging + from shelfmark.core import notifications as notifications_module @@ -24,6 +26,8 @@ class _FakeAppriseClient: self.add_calls = [] self.notify_calls = [] self.notify_result = True + self.notify_warning_messages: list[str] = [] + self.notify_info_messages: list[str] = [] def add(self, url): self.add_calls.append(url) @@ -31,6 +35,10 @@ class _FakeAppriseClient: def notify(self, **kwargs): self.notify_calls.append(kwargs) + for message in self.notify_info_messages: + logging.getLogger("apprise.plugins.pushover").info(message) + for message in self.notify_warning_messages: + logging.getLogger("apprise.plugins.pushover").warning(message) return self.notify_result @@ -183,6 +191,36 @@ def test_dispatch_to_apprise_uses_shelfmark_asset_defaults(monkeypatch): assert "logo.png" in fake_apprise.asset_kwargs["image_url_logo"] +def test_dispatch_to_apprise_logs_captured_apprise_info_messages(monkeypatch): + fake_apprise = _FakeAppriseModule() + fake_apprise.client.notify_info_messages = [ + "Sent Pushover notification to ALL_DEVICES." + ] + monkeypatch.setattr(notifications_module, "apprise", fake_apprise) + + info_messages: list[str] = [] + + def _fake_info(message, *args, **kwargs): + _ = kwargs + info_messages.append(message % args if args else str(message)) + + monkeypatch.setattr(notifications_module.logger, "info", _fake_info) + + result = notifications_module._dispatch_to_apprise( + ["ntfys://ntfy.sh/shelfmark"], + title="Test", + body="Body", + notify_type=_FakeNotifyType.INFO, + ) + + assert result["success"] is True + assert any( + "Apprise source [apprise.plugins.pushover]: Sent Pushover notification to ALL_DEVICES." + in message + for message in info_messages + ) + + def test_dispatch_to_apprise_notify_false_returns_generic_failure_and_logs(monkeypatch): fake_apprise = _FakeAppriseModule() fake_apprise.client.notify_result = False @@ -208,6 +246,37 @@ def test_dispatch_to_apprise_notify_false_returns_generic_failure_and_logs(monke assert any("scheme(s): pover" in message for message in warning_messages) +def test_dispatch_to_apprise_logs_captured_apprise_warning_messages(monkeypatch): + fake_apprise = _FakeAppriseModule() + fake_apprise.client.notify_result = False + fake_apprise.client.notify_warning_messages = [ + "Failed to send Pushover notification to ALL_DEVICES: Unauthorized - Invalid Token., error=401." + ] + monkeypatch.setattr(notifications_module, "apprise", fake_apprise) + + warning_messages: list[str] = [] + + def _fake_warning(message, *args, **kwargs): + _ = kwargs + warning_messages.append(message % args if args else str(message)) + + monkeypatch.setattr(notifications_module.logger, "warning", _fake_warning) + + result = notifications_module._dispatch_to_apprise( + ["pover://user_key@app_token"], + title="Test", + body="Body", + notify_type=_FakeNotifyType.INFO, + ) + + assert result["success"] is False + assert any( + "Apprise source [apprise.plugins.pushover]: Failed to send Pushover notification" + in msg + for msg in warning_messages + ) + + def test_resolve_admin_routes_returns_empty_when_no_routes(monkeypatch): def _fake_get(key, default=None): if key == "ADMIN_NOTIFICATION_ROUTES":