mirror of
https://github.com/agessaman/meshcore-bot.git
synced 2026-08-22 02:19:48 +00:00
Merge pull request #231 from agessaman/perf/optimization-review
feat(db-manager): enhance SQLite journal mode handling and validation
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
+42
-4
@@ -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.
|
||||
|
||||
@@ -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] = {}
|
||||
|
||||
+41
-28
@@ -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:
|
||||
|
||||
@@ -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:
|
||||
|
||||
+15
-1
@@ -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,
|
||||
}
|
||||
|
||||
|
||||
+105
-40
@@ -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):
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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."""
|
||||
|
||||
+233
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user