diff --git a/modules/commands/version_command.py b/modules/commands/version_command.py index 25193c6..6e6e387 100644 --- a/modules/commands/version_command.py +++ b/modules/commands/version_command.py @@ -7,7 +7,7 @@ Returns the currently running bot version string. from typing import Any from ..models import MeshMessage -from ..version_info import resolve_runtime_version +from ..version_info import resolve_application_version from .base_command import BaseCommand @@ -43,9 +43,7 @@ class VersionCommand(BaseCommand): async def execute(self, message: MeshMessage) -> bool: version_value = getattr(self.bot, "bot_version", None) if not version_value: - bot_root = getattr(self.bot, "bot_root", ".") - version_value = resolve_runtime_version(bot_root).get("display", "unknown") + version_value = resolve_application_version().get("display", "unknown") sender = message.sender_id or "Unknown" response = f"@[{sender}] Bot version: {version_value}" return await self.send_response(message, response) - diff --git a/modules/db_manager.py b/modules/db_manager.py index c287c4b..cac4139 100644 --- a/modules/db_manager.py +++ b/modules/db_manager.py @@ -8,8 +8,9 @@ import json import os import re import sqlite3 +import threading from collections.abc import AsyncGenerator, Generator -from contextlib import asynccontextmanager, contextmanager +from contextlib import asynccontextmanager, contextmanager, suppress from datetime import date, datetime, timezone from pathlib import Path from typing import Any, Optional @@ -64,6 +65,13 @@ class DBManager: self.bot = bot self.logger = bot.logger self.db_path = db_path + # WAL is persistent database state, and re-applying it on every + # short-lived connection is surprisingly expensive (and can take a + # lock), so it is set once per config section. The rollback journal + # modes are connection-local; see _apply_sqlite_pragmas. + self._journal_mode_lock = threading.Lock() + self._journal_mode_initialized: set[str] = set() + self._journal_mode_warned: set[str] = set() self._init_database() def _init_database(self) -> None: @@ -572,17 +580,47 @@ class DBManager: foreign_keys = bool(foreign_keys) journal_mode = str(journal_mode).strip() or "WAL" if journal_mode.upper() not in VALID_JOURNAL_MODES: - self.logger.warning(f"Invalid journal_mode {journal_mode!r}, falling back to WAL") + # Warn once per section: this runs on every connection, so an + # unconditional warning here would flood the log. + if section not in self._journal_mode_warned: + self._journal_mode_warned.add(section) + self.logger.warning( + f"Invalid journal_mode {journal_mode!r} in [{section}], falling back to WAL" + ) journal_mode = "WAL" try: conn.execute(f"PRAGMA foreign_keys={'ON' if foreign_keys else 'OFF'}") conn.execute(f"PRAGMA busy_timeout={busy_timeout_ms}") - conn.execute(f"PRAGMA journal_mode={journal_mode}") except sqlite3.OperationalError: - # journal_mode can fail if DB is locked; others are best-effort. + # Connection-local tuning is best-effort when the database is busy. pass + # Only WAL is recorded in the database header and so survives to later + # connections. DELETE/TRUNCATE/PERSIST/MEMORY/OFF are connection-local: + # setting one of those just once would leave every subsequent connection + # silently running the SQLite default instead of the configured mode. + if journal_mode.upper() != "WAL": + with suppress(sqlite3.OperationalError): + conn.execute(f"PRAGMA journal_mode={journal_mode}") + return + + # Tracked per section: [Bot] and [Web_Viewer] are read by different + # callers against this same database, so one must not starve the other. + if section in self._journal_mode_initialized: + return + + # The first successful connection initializes persistent WAL mode. + # If SQLite is locked, leave the section clear so a later connection retries. + with self._journal_mode_lock: + if section in self._journal_mode_initialized: + return + try: + conn.execute(f"PRAGMA journal_mode={journal_mode}") + except sqlite3.OperationalError: + return + self._journal_mode_initialized.add(section) + @contextmanager def connection(self) -> Generator[sqlite3.Connection, None, None]: """Context manager that yields a configured connection and closes it on exit. diff --git a/modules/mesh_graph.py b/modules/mesh_graph.py index ce1a599..6f00fdd 100644 --- a/modules/mesh_graph.py +++ b/modules/mesh_graph.py @@ -45,11 +45,15 @@ def _merge_avg_hop_position( class MeshGraph: """Graph structure tracking observed connections between mesh nodes.""" - def __init__(self, bot): + def __init__(self, bot, capture: Optional[bool] = None): """Initialize the mesh graph. Args: bot: Bot instance with db_manager and config access. + capture: Force the capture kill-switch instead of reading it from + config. Pass False for a read-only consumer (e.g. the web + viewer's path decoding) so it loads the graph without writing + edges or running a batch-writer thread. None reads config. """ self.bot = bot self.logger = bot.logger @@ -58,7 +62,10 @@ class MeshGraph: # Capture/validation feature flags # graph_capture_enabled: controls whether new edge data is collected from packets # When False, no new edges are added and the batch writer thread is not started. - self.capture_enabled = bot.config.getboolean('Path_Command', 'graph_capture_enabled', fallback=True) + if capture is None: + self.capture_enabled = bot.config.getboolean('Path_Command', 'graph_capture_enabled', fallback=True) + else: + self.capture_enabled = bool(capture) # In-memory graph storage: {(from_prefix, to_prefix): edge_data} self.edges: dict[tuple[str, str], dict] = {} diff --git a/modules/repeater_manager.py b/modules/repeater_manager.py index 4481e7e..e0e9ac6 100644 --- a/modules/repeater_manager.py +++ b/modules/repeater_manager.py @@ -64,6 +64,46 @@ def collect_protected_pubkeys_for_device_mode(config: Any, logger: Any) -> set[s return set(keys) +REQUIRED_REPEATER_TABLES = ( + "repeater_contacts", + "complete_contact_tracking", + "daily_stats", + "unique_advert_packets", + "purging_log", + "mesh_connections", + "observed_paths", +) + + +def validate_repeater_tables(db_manager: Any, logger: Any) -> None: + """Raise RuntimeError if the repeater/graph tables migrations create are missing. + + Split out of RepeaterManager.__init__ so callers that build the manager + lazily (the web viewer) can still fail fast at startup with an actionable + message, instead of surfacing a migration problem from inside whichever + request first happens to need the manager. + """ + with db_manager.connection() as conn: + missing = [ + table + for table in REQUIRED_REPEATER_TABLES + if conn.execute( + "SELECT name FROM sqlite_master WHERE type='table' AND name=?", + (table,), + ).fetchone() + is None + ] + + if missing: + msg = ( + "Missing repeater/graph database tables: " + + ", ".join(missing) + + ". Run the bot once to apply migrations." + ) + logger.error(msg) + raise RuntimeError(msg) + + class RepeaterManager: """Manages repeater contacts database and purging operations""" @@ -156,34 +196,7 @@ class RepeaterManager: def _init_repeater_tables(self): """Ensure repeater-specific tables exist (created by migrations).""" try: - with self.db_manager.connection() as conn: - required_tables = [ - "repeater_contacts", - "complete_contact_tracking", - "daily_stats", - "unique_advert_packets", - "purging_log", - "mesh_connections", - "observed_paths", - ] - missing = [] - for t in required_tables: - cur = conn.execute( - "SELECT name FROM sqlite_master WHERE type='table' AND name=?", - (t,), - ) - if cur.fetchone() is None: - missing.append(t) - - if missing: - msg = ( - "Missing repeater/graph database tables: " - + ", ".join(missing) - + ". Run the bot once to apply migrations." - ) - self.logger.error(msg) - raise RuntimeError(msg) - + validate_repeater_tables(self.db_manager, self.logger) self.logger.info("Repeater contacts database initialized successfully") except Exception as e: diff --git a/modules/service_plugins/packet_capture_service.py b/modules/service_plugins/packet_capture_service.py index ff73879..18412ad 100644 --- a/modules/service_plugins/packet_capture_service.py +++ b/modules/service_plugins/packet_capture_service.py @@ -32,7 +32,7 @@ from ..utils import ( parse_trace_payload_route_hashes, verify_meshcore_advert_ed25519, ) -from ..version_info import resolve_runtime_version +from ..version_info import resolve_application_version # Import MQTT client try: @@ -1904,7 +1904,7 @@ class PacketCaptureService(BaseServicePlugin): def _load_client_version(self) -> str: """Load client version from shared runtime resolver.""" try: - info = resolve_runtime_version(self.bot.bot_root) + info = resolve_application_version() display = info.get("display") or "unknown" return f"meshcore-bot/{display}" except Exception as e: diff --git a/modules/version_info.py b/modules/version_info.py index 645dc41..ce547d9 100644 --- a/modules/version_info.py +++ b/modules/version_info.py @@ -90,6 +90,21 @@ def _read_pyproject_version(repo_root: Path) -> str | None: return None +def get_application_root() -> Path: + """Return the installed/source directory containing the bot application. + + Runtime configuration may live elsewhere (for example, ``/etc/meshcore-bot`` + or ``/data/config``), so version metadata must be resolved relative to this + module rather than relative to the config file. + """ + return Path(__file__).resolve().parent.parent + + +def resolve_application_version() -> dict[str, str | None]: + """Resolve version metadata for the running bot application.""" + return resolve_runtime_version(get_application_root()) + + def resolve_runtime_version(repo_root: Path | str) -> dict[str, str | None]: """Resolve version metadata and a single runtime display value. @@ -161,4 +176,3 @@ def resolve_runtime_version(repo_root: Path | str) -> dict[str, str | None]: "date": date, "display": display, } - diff --git a/modules/web_viewer/app.py b/modules/web_viewer/app.py index 8d2cfc9..1c11ee9 100644 --- a/modules/web_viewer/app.py +++ b/modules/web_viewer/app.py @@ -55,7 +55,6 @@ from modules.db_retention import ( ) from modules.ini_writer import IniValueError, update_ini_values from modules.security_utils import ( - VALID_JOURNAL_MODES, SafeUrlPolicy, create_safe_requests_session, safe_requests_request, @@ -68,7 +67,7 @@ from modules.settings_schema import ( validate_field, ) from modules.settings_store import get_settings_store -from modules.version_info import resolve_runtime_version +from modules.version_info import resolve_application_version from modules.web_viewer.dashboard_stats import ( SERIES_METRICS, TOP_KINDS, @@ -158,7 +157,7 @@ from modules.feed_manager import ( FeedManager, _useful_feed_content_type, ) -from modules.repeater_manager import RepeaterManager +from modules.repeater_manager import RepeaterManager, validate_repeater_tables from modules.url_shortener import _coerce_url_string from modules.utils import resolve_path from modules.web_viewer.config_panels import CONFIG_PANELS, PANEL_CATEGORIES @@ -298,6 +297,8 @@ class BotDataViewer: self._db_lock = threading.Lock() self._db_last_used = 0 self._db_timeout = 300 # 5 minutes connection timeout + # SQLite pragma handling (including the once-per-section WAL setup) + # lives in DBManager; see _configure_db_connection. # The contacts list needs all-time multibyte hop-prefix evidence for its # capability badge. Cache that derived set between requests and invalidate @@ -489,7 +490,7 @@ class BotDataViewer: def _get_version_info(self) -> dict[str, str | None]: """Get version info for footer via centralized version resolver. Never raises.""" - info = resolve_runtime_version(self.bot_root) + info = resolve_application_version() display = info.get("display") return { # 'display' is what the footer renders — same value !version reports, @@ -581,13 +582,24 @@ class BotDataViewer: # Now set db_manager on the minimal bot for RepeaterManager minimal_bot.db_manager = self.db_manager - # Initialize repeater manager for geocoding functionality - self.repeater_manager = RepeaterManager(minimal_bot) + # The viewer only needs RepeaterManager for the manual geocode + # endpoint, so defer its setup until that endpoint is actually used. + self._repeater_manager_bot = minimal_bot + self._repeater_manager_lock = threading.Lock() + self.repeater_manager: RepeaterManager | None = None - # Initialize mesh graph for path resolution (uses same logic as path command) - from modules.mesh_graph import MeshGraph - minimal_bot.mesh_graph = MeshGraph(minimal_bot) - self.mesh_graph = minimal_bot.mesh_graph + # RepeaterManager's constructor is what used to validate these at + # startup. It is lazy now, so validate here: a missing migration is + # a startup failure, not a mystery 500 from the geocode endpoint. + validate_repeater_tables(self.db_manager, self.logger) + + # MeshGraph is only read by the two path-decoding routes, so build it + # on first use rather than at startup. It is NOT optional for them: + # without it decode_path_nodes() falls back to geographic-only + # selection and disagrees with the bot's `path` command. + self._mesh_graph_bot = minimal_bot + self._mesh_graph_lock = threading.Lock() + self.mesh_graph = None # Store database paths for direct connection self.db_path = self.db_path @@ -597,23 +609,56 @@ class BotDataViewer: self.logger.error(f"Failed to initialize databases: {e}") raise + def _get_repeater_manager(self) -> RepeaterManager: + """Lazily construct the manual-geocoding helper once.""" + if self.repeater_manager is not None: + return self.repeater_manager + with self._repeater_manager_lock: + if self.repeater_manager is None: + self.repeater_manager = RepeaterManager(self._repeater_manager_bot) + return self.repeater_manager + + def _get_mesh_graph(self): + """Lazily load the read-only mesh graph used to disambiguate path prefixes. + + Returns None only if the graph cannot be loaded, in which case path + decoding degrades to geographic-only selection rather than failing the + request. Loaded with capture disabled: the bot process owns edge + capture, so the viewer must not write edges or run a batch writer. + """ + if self.mesh_graph is not None: + return self.mesh_graph + with self._mesh_graph_lock: + if self.mesh_graph is None: + from modules.mesh_graph import MeshGraph + try: + graph = MeshGraph(self._mesh_graph_bot, capture=False) + except Exception as e: + self.logger.error( + f"Mesh graph unavailable; path decoding will fall back to " + f"geographic selection only: {e}" + ) + return None + self._mesh_graph_bot.mesh_graph = graph + self.mesh_graph = graph + return self.mesh_graph + + def _configure_db_connection(self, conn: sqlite3.Connection) -> None: + """Apply SQLite pragmas to a viewer connection. + + Delegates to DBManager, which owns the single implementation and the + once-per-section WAL bookkeeping. Safe because this DBManager was built + for self.db_path, which is the file every viewer connection opens — the + journal-mode state it tracks belongs to that same database. + """ + self.db_manager._apply_sqlite_pragmas(conn, for_web_viewer=True) + def _get_db_connection(self): """Get database connection - create new connection for each request to avoid threading issues""" try: conn = sqlite3.connect(self.db_path, timeout=60) conn.row_factory = sqlite3.Row - try: - foreign_keys = self.config.getboolean("Web_Viewer", "sqlite_foreign_keys", fallback=True) - busy_timeout_ms = self.config.getint("Web_Viewer", "sqlite_busy_timeout_ms", fallback=60000) - journal_mode = self.config.get("Web_Viewer", "sqlite_journal_mode", fallback="WAL").strip() or "WAL" - if journal_mode.upper() not in VALID_JOURNAL_MODES: - self.logger.warning(f"Invalid journal_mode {journal_mode!r}, falling back to WAL") - journal_mode = "WAL" - conn.execute(f"PRAGMA foreign_keys={'ON' if foreign_keys else 'OFF'}") - conn.execute(f"PRAGMA busy_timeout={int(busy_timeout_ms)}") - conn.execute(f"PRAGMA journal_mode={journal_mode}") - except sqlite3.OperationalError: - pass + self._configure_db_connection(conn) return conn except Exception as e: self.logger.error(f"Failed to create database connection: {e}") @@ -626,18 +671,7 @@ class BotDataViewer: """ conn = sqlite3.connect(self.db_path, timeout=60) conn.row_factory = sqlite3.Row - try: - foreign_keys = self.config.getboolean("Web_Viewer", "sqlite_foreign_keys", fallback=True) - busy_timeout_ms = self.config.getint("Web_Viewer", "sqlite_busy_timeout_ms", fallback=60000) - journal_mode = self.config.get("Web_Viewer", "sqlite_journal_mode", fallback="WAL").strip() or "WAL" - if journal_mode.upper() not in VALID_JOURNAL_MODES: - self.logger.warning(f"Invalid journal_mode {journal_mode!r}, falling back to WAL") - journal_mode = "WAL" - conn.execute(f"PRAGMA foreign_keys={'ON' if foreign_keys else 'OFF'}") - conn.execute(f"PRAGMA busy_timeout={int(busy_timeout_ms)}") - conn.execute(f"PRAGMA journal_mode={journal_mode}") - except sqlite3.OperationalError: - pass + self._configure_db_connection(conn) try: yield conn finally: @@ -957,7 +991,7 @@ class BotDataViewer: config=self.config, db_manager=self.db_manager, logger=self.logger, - mesh_graph=getattr(self, "mesh_graph", None), + mesh_graph=self._get_mesh_graph(), include_location=True, ) node_ids = [n["node_id"] for n in nodes] @@ -3151,8 +3185,13 @@ class BotDataViewer: current_country = contact['country'] self.logger.debug(f"Current location data - city: {current_city}, state: {current_state}, country: {current_country}") + # Outside the try below: a failure to build the manager is a + # setup problem, not a geocoding one, and must not be reported + # to the user as "Geocoding exception". + repeater_manager = self._get_repeater_manager() + try: - location_info = self.repeater_manager._get_full_location_from_coordinates(lat, lon) + location_info = repeater_manager._get_full_location_from_coordinates(lat, lon) self.logger.debug(f"Geocoding result for {name}: {location_info}") except Exception as geocode_error: self.logger.error(f"Exception during geocoding for {name} at {lat}, {lon}: {geocode_error}", exc_info=True) @@ -4736,6 +4775,21 @@ class BotDataViewer: import sqlite3 import time + # Subscription handlers replay recent history themselves. + # With no live command/packet/message subscribers there is + # nothing to broadcast, so avoid opening SQLite and decoding + # every packet-stream row merely to discard it. + if not self._has_live_stream_subscribers(): + last_timestamp = time.time() + # An idle period is not a failure. Without this, an + # error burst before the last subscriber left would + # still be counted against the first poll after the + # next one arrives, mis-escalating its log level and + # backoff. + consecutive_errors = 0 + time.sleep(2.0) + continue + # Check if database file exists and is accessible db_file = Path(self.db_path) if not db_file.exists(): @@ -4854,6 +4908,19 @@ class BotDataViewer: polling_thread.start() self.logger.info("Database polling started") + def _has_live_stream_subscribers(self) -> bool: + """Return whether any client consumes a DB-backed live stream.""" + subscription_keys = ( + 'subscribed_commands', + 'subscribed_packets', + 'subscribed_messages', + ) + with self._clients_lock: + return any( + any(client.get(key, False) for key in subscription_keys) + for client in self.connected_clients.values() + ) + def _config_int(self, section: str, option: str, fallback: int) -> int: """Read an int config value, falling back on a missing or malformed entry.""" try: @@ -4950,9 +5017,7 @@ class BotDataViewer: """ conn = sqlite3.connect(self.db_path, timeout=60, isolation_level=None) conn.row_factory = sqlite3.Row - with suppress(sqlite3.OperationalError): - conn.execute(f"PRAGMA busy_timeout={self._config_int('Web_Viewer', 'sqlite_busy_timeout_ms', 60000)}") - conn.execute("PRAGMA journal_mode=WAL") + self._configure_db_connection(conn) return conn def _start_dashboard_refresher(self): @@ -8463,7 +8528,7 @@ class BotDataViewer: config=self.config, db_manager=self.db_manager, logger=self.logger, - mesh_graph=getattr(self, "mesh_graph", None), + mesh_graph=self._get_mesh_graph(), ) def run(self, host='127.0.0.1', port=8080, debug=False): diff --git a/tests/commands/test_version_command.py b/tests/commands/test_version_command.py index 66a0d14..b3d0eed 100644 --- a/tests/commands/test_version_command.py +++ b/tests/commands/test_version_command.py @@ -39,14 +39,25 @@ class TestVersionCommand: assert call_args[0][1] == "@[TestUser] Bot version: dev-abc1234" @pytest.mark.asyncio - async def test_execute_falls_back_to_resolver(self, command_mock_bot, monkeypatch): + async def test_execute_resolves_version_from_application_root( + self, command_mock_bot, monkeypatch, tmp_path + ): command_mock_bot.config.add_section("Version_Command") command_mock_bot.config.set("Version_Command", "enabled", "true") command_mock_bot.bot_version = None - command_mock_bot.bot_root = "." + command_mock_bot.bot_root = tmp_path / "etc" / "meshcore-bot" + + application_root = tmp_path / "opt" / "meshcore-bot" + application_root.mkdir(parents=True) + (application_root / ".version_info").write_text( + '{"installer_version": "v0.9"}', encoding="utf-8" + ) + monkeypatch.delenv("MESHCORE_BOT_VERSION", raising=False) monkeypatch.setattr( - "modules.commands.version_command.resolve_runtime_version", - lambda _root: {"display": "v0.9"}, + "modules.version_info.get_application_root", lambda: application_root + ) + monkeypatch.setattr( + "modules.version_info._safe_git_run", lambda *_args, **_kwargs: None ) cmd = VersionCommand(command_mock_bot) @@ -57,4 +68,3 @@ class TestVersionCommand: call_args = command_mock_bot.command_manager.send_response.call_args assert call_args is not None assert call_args[0][1] == "@[TestUser] Bot version: v0.9" - diff --git a/tests/test_db_manager.py b/tests/test_db_manager.py index f18e0e3..c02964c 100644 --- a/tests/test_db_manager.py +++ b/tests/test_db_manager.py @@ -1,5 +1,6 @@ """Tests for modules.db_manager.""" +import configparser import sqlite3 from contextlib import closing from unittest.mock import Mock @@ -35,6 +36,140 @@ class TestDatabaseInitialization: assert "parent=" in logged assert "exists=False" in logged + def test_journal_mode_is_initialized_once_per_manager( + self, mock_logger, monkeypatch, tmp_path + ): + statements = [] + real_connect = sqlite3.connect + + class TracingConnection(sqlite3.Connection): + def execute(self, sql, parameters=()): + statements.append(sql) + return super().execute(sql, parameters) + + def tracing_connect(*args, **kwargs): + kwargs["factory"] = TracingConnection + return real_connect(*args, **kwargs) + + monkeypatch.setattr(sqlite3, "connect", tracing_connect) + bot = Mock() + bot.logger = mock_logger + bot.config = configparser.ConfigParser() + bot.config["Bot"] = {"sqlite_journal_mode": "WAL"} + + manager = DBManager(bot, str(tmp_path / "journal-once.db")) + manager.get_metadata("missing-one") + manager.get_metadata("missing-two") + + journal_statements = [ + sql for sql in statements if sql.upper().startswith("PRAGMA JOURNAL_MODE=") + ] + foreign_key_statements = [ + sql for sql in statements if sql.upper().startswith("PRAGMA FOREIGN_KEYS=") + ] + assert journal_statements == ["PRAGMA journal_mode=WAL"] + assert len(foreign_key_statements) == 3 + + def test_journal_mode_retries_after_database_lock(self, db): + db._journal_mode_initialized.clear() + locked_connection = Mock() + + def locked_execute(sql): + if sql.upper().startswith("PRAGMA JOURNAL_MODE="): + raise sqlite3.OperationalError("database is locked") + return Mock() + + locked_connection.execute.side_effect = locked_execute + db._apply_sqlite_pragmas(locked_connection) + assert "Bot" not in db._journal_mode_initialized + + available_connection = Mock() + db._apply_sqlite_pragmas(available_connection) + assert "Bot" in db._journal_mode_initialized + assert any( + call.args[0].upper().startswith("PRAGMA JOURNAL_MODE=") + for call in available_connection.execute.call_args_list + ) + + def test_config_sections_do_not_starve_each_other(self, db): + """[Bot] and [Web_Viewer] are read by different callers against one file. + + A single shared flag would let whichever ran first suppress the other's + journal-mode setup entirely. + """ + db._journal_mode_initialized.clear() + + bot_conn = Mock() + db._apply_sqlite_pragmas(bot_conn, for_web_viewer=False) + viewer_conn = Mock() + db._apply_sqlite_pragmas(viewer_conn, for_web_viewer=True) + + def journal_pragmas(conn): + return [ + c.args[0] for c in conn.execute.call_args_list + if c.args[0].upper().startswith("PRAGMA JOURNAL_MODE=") + ] + + assert journal_pragmas(bot_conn) == ["PRAGMA journal_mode=WAL"] + assert journal_pragmas(viewer_conn) == ["PRAGMA journal_mode=WAL"] + assert db._journal_mode_initialized == {"Bot", "Web_Viewer"} + + # ...and each section is still only initialized once. + again = Mock() + db._apply_sqlite_pragmas(again, for_web_viewer=True) + assert journal_pragmas(again) == [] + + def test_invalid_journal_mode_warns_once_per_section(self, mock_logger, tmp_path): + bot = Mock() + bot.logger = mock_logger + bot.config = configparser.ConfigParser() + bot.config["Bot"] = {"sqlite_journal_mode": "NOT_A_MODE"} + manager = DBManager(bot, str(tmp_path / "bad-mode.db")) + # Construction runs migrations, which already consumed the one warning + # and initialized the mode for [Bot]. + manager._journal_mode_warned.clear() + manager._journal_mode_initialized.clear() + mock_logger.warning.reset_mock() + + applied = [] + conn = Mock() + conn.execute.side_effect = lambda sql: applied.append(sql) + for _ in range(3): + manager._apply_sqlite_pragmas(conn) + + assert mock_logger.warning.call_count == 1 + assert "NOT_A_MODE" in str(mock_logger.warning.call_args) + assert [s for s in applied if s.upper().startswith("PRAGMA JOURNAL_MODE=")] == [ + "PRAGMA journal_mode=WAL" + ] + + def test_rollback_journal_mode_is_applied_to_every_connection( + self, mock_logger, tmp_path + ): + """Only WAL persists in the file header; the rollback modes are per-connection. + + Caching a rollback mode after the first connection would leave every + later connection silently running SQLite's default DELETE. + """ + bot = Mock() + bot.logger = mock_logger + bot.config = configparser.ConfigParser() + bot.config["Bot"] = {"sqlite_journal_mode": "TRUNCATE"} + manager = DBManager(bot, str(tmp_path / "rollback-mode.db")) + + applied = [] + conn = Mock() + conn.execute.side_effect = lambda sql: applied.append(sql) + manager._apply_sqlite_pragmas(conn) + manager._apply_sqlite_pragmas(conn) + + assert [s for s in applied if s.upper().startswith("PRAGMA JOURNAL_MODE=")] == [ + "PRAGMA journal_mode=TRUNCATE", + "PRAGMA journal_mode=TRUNCATE", + ] + # A non-persistent mode must not consume the WAL-only fast path. + assert "Bot" not in manager._journal_mode_initialized + class TestGeocoding: """Tests for geocoding cache.""" diff --git a/tests/test_repeater_manager.py b/tests/test_repeater_manager.py index c1c3367..2ac84ca 100644 --- a/tests/test_repeater_manager.py +++ b/tests/test_repeater_manager.py @@ -10,11 +10,38 @@ import pytest from meshcore import EventType from modules.repeater_manager import ( + REQUIRED_REPEATER_TABLES, RepeaterManager, collect_protected_pubkeys_for_device_mode, + validate_repeater_tables, ) +class TestValidateRepeaterTables: + """Callable without a RepeaterManager so lazy callers can still fail fast.""" + + def test_passes_on_a_migrated_database(self, test_db, mock_logger): + validate_repeater_tables(test_db, mock_logger) + + def test_names_every_missing_table(self, test_db, mock_logger): + with test_db.connection() as conn: + conn.execute("DROP TABLE observed_paths") + conn.execute("DROP TABLE purging_log") + conn.commit() + + with pytest.raises(RuntimeError) as excinfo: + validate_repeater_tables(test_db, mock_logger) + + message = str(excinfo.value) + assert "observed_paths" in message + assert "purging_log" in message + assert "Run the bot once to apply migrations" in message + + def test_covers_the_tables_the_manager_depends_on(self): + assert "mesh_connections" in REQUIRED_REPEATER_TABLES + assert "repeater_contacts" in REQUIRED_REPEATER_TABLES + + @pytest.fixture def bot(mock_logger, test_db): """Minimal bot mock for RepeaterManager — uses a real test DB.""" diff --git a/tests/test_web_viewer.py b/tests/test_web_viewer.py index 1d9bdde..e70cf28 100644 --- a/tests/test_web_viewer.py +++ b/tests/test_web_viewer.py @@ -10,7 +10,7 @@ import configparser import json import logging import sqlite3 -from contextlib import closing +from contextlib import closing, contextmanager from pathlib import Path from unittest.mock import MagicMock, Mock, patch @@ -35,6 +35,30 @@ def _write_config(path: Path, db_path: str) -> None: cfg.write(f) +@contextmanager +def _temp_config_option(config, section: str, option: str, value: str): + """Set a config option for the body of a test, restoring prior state after. + + Patching ConfigParser.get is not a substitute: getboolean/getint delegate to + it internally, so a stubbed get breaks every other option read too. + """ + added_section = not config.has_section(section) + if added_section: + config.add_section(section) + had_option = config.has_option(section, option) + previous = config.get(section, option) if had_option else None + config.set(section, option, value) + try: + yield + finally: + if added_section: + config.remove_section(section) + elif had_option: + config.set(section, option, previous) + else: + config.remove_option(section, option) + + def _fake_setup_logging(self: BotDataViewer) -> None: """Replace file-based logging with an in-memory logger for tests.""" self.logger = logging.getLogger("test_web_viewer") @@ -2250,6 +2274,214 @@ class TestSubscribeCommandsHistoryReplay: "last_timestamp must be initialized to time.time()-300, not 0" ) + def test_mesh_graph_is_not_built_at_startup(self, tmp_path): + """Uses its own viewer: the shared one may already have decoded a path.""" + db_path = str(tmp_path / "startup.db") + config_path = str(tmp_path / "config.ini") + _write_config(Path(config_path), db_path) + + factory = Mock() + with ( + patch("modules.mesh_graph.MeshGraph", factory), + patch.object(BotDataViewer, "_setup_logging", _fake_setup_logging), + patch.object(BotDataViewer, "_start_database_polling", lambda self: None), + patch.object(BotDataViewer, "_start_log_tailing", lambda self: None), + patch.object(BotDataViewer, "_start_cleanup_scheduler", lambda self: None), + patch.object(BotDataViewer, "_start_dashboard_refresher", lambda self: None), + ): + v = BotDataViewer(db_path=db_path, config_path=config_path) + + assert v.mesh_graph is None + factory.assert_not_called() + + def test_mesh_graph_is_lazy_singleton_and_read_only(self, viewer, monkeypatch): + graph = object() + factory = Mock(return_value=graph) + monkeypatch.setattr("modules.mesh_graph.MeshGraph", factory) + viewer.mesh_graph = None + try: + assert viewer._get_mesh_graph() is graph + assert viewer._get_mesh_graph() is graph + # capture=False keeps edge writes and the batch-writer thread in the + # bot process, which owns capture. + factory.assert_called_once_with(viewer._mesh_graph_bot, capture=False) + finally: + viewer.mesh_graph = None + + def test_path_decoding_passes_mesh_graph_to_shared_engine(self, viewer, monkeypatch): + """Without the graph, decoding silently disagrees with the bot's `path` command.""" + graph = object() + monkeypatch.setattr( + "modules.mesh_graph.MeshGraph", Mock(return_value=graph) + ) + decode = Mock(return_value=[]) + monkeypatch.setattr("modules.path_inference.decode_path_nodes", decode) + viewer.mesh_graph = None + try: + viewer._decode_path_hex("aabb") + assert decode.call_args.kwargs["mesh_graph"] is graph + + decode.reset_mock() + viewer._resolve_path("aabb") + assert decode.call_args.kwargs["mesh_graph"] is graph + finally: + viewer.mesh_graph = None + + def test_path_decoding_degrades_when_mesh_graph_fails_to_load(self, viewer, monkeypatch): + monkeypatch.setattr( + "modules.mesh_graph.MeshGraph", Mock(side_effect=RuntimeError("no table")) + ) + decode = Mock(return_value=[]) + monkeypatch.setattr("modules.path_inference.decode_path_nodes", decode) + viewer.mesh_graph = None + try: + viewer._decode_path_hex("aabb") + assert decode.call_args.kwargs["mesh_graph"] is None + finally: + viewer.mesh_graph = None + + def test_missing_migrations_fail_at_startup_not_in_a_request(self, tmp_path): + """RepeaterManager is lazy now, so the viewer must validate up front. + + Otherwise a migration problem first surfaces as a 500 from whichever + request happens to need the manager. + """ + db_path = str(tmp_path / "unmigrated.db") + config_path = str(tmp_path / "config.ini") + _write_config(Path(config_path), db_path) + + boom = Mock(side_effect=RuntimeError("Missing repeater/graph database tables: purging_log")) + with ( + patch("modules.web_viewer.app.validate_repeater_tables", boom), + patch.object(BotDataViewer, "_setup_logging", _fake_setup_logging), + patch.object(BotDataViewer, "_start_database_polling", lambda self: None), + patch.object(BotDataViewer, "_start_log_tailing", lambda self: None), + patch.object(BotDataViewer, "_start_cleanup_scheduler", lambda self: None), + patch.object(BotDataViewer, "_start_dashboard_refresher", lambda self: None), + pytest.raises(RuntimeError, match="Missing repeater/graph database tables"), + ): + BotDataViewer(db_path=db_path, config_path=config_path) + + def test_repeater_manager_is_lazy_and_singleton(self, viewer, monkeypatch): + manager = object() + factory = Mock(return_value=manager) + monkeypatch.setattr("modules.web_viewer.app.RepeaterManager", factory) + viewer.repeater_manager = None + try: + assert viewer._get_repeater_manager() is manager + assert viewer._get_repeater_manager() is manager + factory.assert_called_once_with(viewer._repeater_manager_bot) + finally: + viewer.repeater_manager = None + + def test_live_stream_polling_is_idle_without_relevant_subscribers(self, viewer): + with viewer._clients_lock: + original_clients = dict(viewer.connected_clients) + viewer.connected_clients.clear() + viewer.connected_clients["mesh-only"] = {"subscribed_mesh": True} + try: + assert viewer._has_live_stream_subscribers() is False + with viewer._clients_lock: + viewer.connected_clients["packet-client"] = { + "subscribed_packets": True + } + assert viewer._has_live_stream_subscribers() is True + finally: + with viewer._clients_lock: + viewer.connected_clients.clear() + viewer.connected_clients.update(original_clients) + + def test_viewer_journal_mode_is_initialized_once(self, viewer, monkeypatch): + statements = [] + real_connect = sqlite3.connect + + class TracingConnection(sqlite3.Connection): + def execute(self, sql, parameters=()): + statements.append(sql) + return super().execute(sql, parameters) + + def tracing_connect(*args, **kwargs): + kwargs["factory"] = TracingConnection + return real_connect(*args, **kwargs) + + monkeypatch.setattr(sqlite3, "connect", tracing_connect) + viewer.db_manager._journal_mode_initialized.discard("Web_Viewer") + for _ in range(3): + with closing(viewer._get_db_connection()): + pass + + journal_statements = [ + sql for sql in statements if sql.upper().startswith("PRAGMA JOURNAL_MODE=") + ] + foreign_key_statements = [ + sql for sql in statements if sql.upper().startswith("PRAGMA FOREIGN_KEYS=") + ] + assert journal_statements == ["PRAGMA journal_mode=WAL"] + assert len(foreign_key_statements) == 3 + + def test_viewer_connections_use_the_shared_dbmanager_implementation(self, viewer): + """The viewer must not carry its own copy of the pragma logic.""" + conn = Mock() + with patch.object(viewer.db_manager, "_apply_sqlite_pragmas") as shared: + viewer._configure_db_connection(conn) + shared.assert_called_once_with(conn, for_web_viewer=True) + + # The DBManager it delegates to must own the same file every viewer + # connection opens, or its journal-mode bookkeeping would be tracking + # a different database. + assert str(viewer.db_manager.db_path) == str(viewer.db_path) + + def test_rollback_journal_mode_is_applied_to_every_connection(self, viewer): + """DELETE/TRUNCATE/PERSIST/MEMORY/OFF are connection-local, not persistent. + + Caching them like WAL would leave every connection after the first + silently running SQLite's default mode instead of the configured one. + """ + applied = [] + conn = Mock() + conn.execute.side_effect = lambda sql: applied.append(sql) + viewer.db_manager._journal_mode_initialized.discard("Web_Viewer") + + with _temp_config_option( + viewer.config, "Web_Viewer", "sqlite_journal_mode", "MEMORY" + ): + viewer._configure_db_connection(conn) + viewer._configure_db_connection(conn) + + assert [s for s in applied if s.upper().startswith("PRAGMA JOURNAL_MODE=")] == [ + "PRAGMA journal_mode=MEMORY", + "PRAGMA journal_mode=MEMORY", + ] + # A non-persistent mode must not consume the WAL-only fast path. + assert "Web_Viewer" not in viewer.db_manager._journal_mode_initialized + + def test_invalid_journal_mode_warns_once_and_falls_back(self, viewer): + """The mode is read per connection now, so the warning must not repeat.""" + viewer.db_manager._journal_mode_warned.discard("Web_Viewer") + viewer.db_manager._journal_mode_initialized.discard("Web_Viewer") + applied = [] + conn = Mock() + conn.execute.side_effect = lambda sql: applied.append(sql) + fake_logger = Mock() + try: + with ( + _temp_config_option( + viewer.config, "Web_Viewer", "sqlite_journal_mode", "BOGUS" + ), + patch.object(viewer.db_manager, "logger", fake_logger), + ): + for _ in range(3): + viewer._configure_db_connection(conn) + finally: + viewer.db_manager._journal_mode_warned.discard("Web_Viewer") + + assert fake_logger.warning.call_count == 1 + assert "BOGUS" in str(fake_logger.warning.call_args) + # Falls back to WAL rather than sending an invalid pragma to SQLite. + assert [s for s in applied if s.upper().startswith("PRAGMA JOURNAL_MODE=")] == [ + "PRAGMA journal_mode=WAL" + ] + # --------------------------------------------------------------------------- # TASK-03: GET /api/connected_clients