mirror of
https://github.com/agessaman/meshcore-bot.git
synced 2026-04-27 03:15:19 +00:00
feat(db_migrations, message_handler, repeater_manager): enhance purging log functionality
- Added a new migration to include a 'details' column in the 'purging_log' table for improved logging of actions. - Refactored the `log_purging_action` method in `RepeaterManager` to handle both new and legacy schemas, ensuring compatibility with the updated database structure. - Updated `MessageHandler` to utilize the new logging method, replacing direct database updates with a unified logging approach. - Added tests to verify the presence of the new column and the correct functionality of the logging mechanism across different scenarios.
This commit is contained in:
@@ -484,6 +484,12 @@ def _m0011_repeater_and_graph_indexes(cursor: sqlite3.Cursor) -> None:
|
||||
)
|
||||
|
||||
|
||||
def _m0012_purging_log_details_column(cursor: sqlite3.Cursor) -> None:
|
||||
"""Add details column for newer purging log entries."""
|
||||
if _table_exists(cursor, "purging_log"):
|
||||
_add_column(cursor, "purging_log", "details", "TEXT")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Migration registry — append new entries here, never remove or reorder.
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -502,6 +508,7 @@ MIGRATIONS: list[MigrationEntry] = [
|
||||
(9, "optional repeater/graph indexes", _m0009_repeater_optional_indexes),
|
||||
(10, "create repeater/graph tables", _m0010_create_repeater_and_graph_tables),
|
||||
(11, "repeater/graph indexes", _m0011_repeater_and_graph_indexes),
|
||||
(12, "purging_log: add details column", _m0012_purging_log_details_column),
|
||||
]
|
||||
|
||||
|
||||
|
||||
@@ -3346,12 +3346,9 @@ class MessageHandler:
|
||||
|
||||
await self.bot.repeater_manager.check_and_auto_purge()
|
||||
|
||||
self.bot.repeater_manager.db_manager.execute_update(
|
||||
'INSERT INTO purging_log (action, details) VALUES (?, ?)',
|
||||
(
|
||||
'new_contact_discovered',
|
||||
f'New contact discovered: {contact_name} (key: {public_key[:16]}...)',
|
||||
),
|
||||
self.bot.repeater_manager.log_purging_action(
|
||||
"new_contact_discovered",
|
||||
f"New contact discovered: {contact_name} (key: {public_key[:16]}...)",
|
||||
)
|
||||
return
|
||||
|
||||
@@ -3408,9 +3405,9 @@ class MessageHandler:
|
||||
|
||||
# Log the new contact discovery
|
||||
if hasattr(self.bot, 'repeater_manager'):
|
||||
self.bot.repeater_manager.db_manager.execute_update(
|
||||
'INSERT INTO purging_log (action, details) VALUES (?, ?)',
|
||||
('new_contact_discovered', f'New contact discovered: {contact_name} (key: {public_key[:16]}...)')
|
||||
self.bot.repeater_manager.log_purging_action(
|
||||
"new_contact_discovered",
|
||||
f"New contact discovered: {contact_name} (key: {public_key[:16]}...)",
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
|
||||
+56
-18
@@ -88,6 +88,7 @@ class RepeaterManager:
|
||||
self._auto_purge_lock = asyncio.Lock()
|
||||
self._auto_purge_in_progress = False
|
||||
self._purge_inflight_keys = set()
|
||||
self._purging_log_has_details: Optional[bool] = None
|
||||
|
||||
def _start_purge_attempt(self, public_key: str, contact_type: str) -> bool:
|
||||
"""Mark a purge as in-flight; return False when another attempt is already active."""
|
||||
@@ -103,6 +104,43 @@ class RepeaterManager:
|
||||
"""Clear in-flight marker for a purge attempt."""
|
||||
self._purge_inflight_keys.discard(public_key)
|
||||
|
||||
def _refresh_purging_log_details_capability(self) -> bool:
|
||||
"""Return True if purging_log.details exists."""
|
||||
if self._purging_log_has_details is not None:
|
||||
return self._purging_log_has_details
|
||||
try:
|
||||
with self.db_manager.connection() as conn:
|
||||
rows = conn.execute("PRAGMA table_info(purging_log)").fetchall()
|
||||
self._purging_log_has_details = any(row[1] == "details" for row in rows)
|
||||
except Exception as e:
|
||||
self.logger.debug(f"Unable to inspect purging_log schema: {e}")
|
||||
self._purging_log_has_details = False
|
||||
return self._purging_log_has_details
|
||||
|
||||
def log_purging_action(self, action: str, details: str) -> None:
|
||||
"""Insert purging log rows across both new and legacy schemas."""
|
||||
try:
|
||||
if self._refresh_purging_log_details_capability():
|
||||
self.db_manager.execute_update(
|
||||
"INSERT INTO purging_log (action, details) VALUES (?, ?)",
|
||||
(action, details),
|
||||
)
|
||||
return
|
||||
self.db_manager.execute_update(
|
||||
"INSERT INTO purging_log (action, public_key, name, reason) VALUES (?, ?, ?, ?)",
|
||||
(action, "", action, details),
|
||||
)
|
||||
except Exception as e:
|
||||
err = str(e)
|
||||
if "no column named details" in err:
|
||||
self._purging_log_has_details = False
|
||||
self.db_manager.execute_update(
|
||||
"INSERT INTO purging_log (action, public_key, name, reason) VALUES (?, ?, ?, ?)",
|
||||
(action, "", action, details),
|
||||
)
|
||||
return
|
||||
self.logger.error(f"Failed to write purging log action '{action}': {e}")
|
||||
|
||||
def _init_repeater_tables(self):
|
||||
"""Ensure repeater-specific tables exist (created by migrations)."""
|
||||
try:
|
||||
@@ -2538,9 +2576,9 @@ class RepeaterManager:
|
||||
self.logger.error(f"Failed to discover companion contacts: {e}")
|
||||
|
||||
# Step 3: Log the post-purge management action
|
||||
self.db_manager.execute_update(
|
||||
'INSERT INTO purging_log (action, details) VALUES (?, ?)',
|
||||
('post_purge_management', 'Enabled manual contact addition and initiated companion contact discovery')
|
||||
self.log_purging_action(
|
||||
"post_purge_management",
|
||||
"Enabled manual contact addition and initiated companion contact discovery",
|
||||
)
|
||||
|
||||
self.logger.info("Post-purge contact management completed")
|
||||
@@ -2682,9 +2720,9 @@ class RepeaterManager:
|
||||
|
||||
# Log the management action
|
||||
if actions_taken:
|
||||
self.db_manager.execute_update(
|
||||
'INSERT INTO purging_log (action, details) VALUES (?, ?)',
|
||||
('contact_management', f'Contact list management: {"; ".join(actions_taken)}')
|
||||
self.log_purging_action(
|
||||
"contact_management",
|
||||
f'Contact list management: {"; ".join(actions_taken)}',
|
||||
)
|
||||
|
||||
return {
|
||||
@@ -2725,9 +2763,9 @@ class RepeaterManager:
|
||||
self.logger.info(f"✅ Successfully removed stale contact: {contact_name}")
|
||||
|
||||
# Log the removal
|
||||
self.db_manager.execute_update(
|
||||
'INSERT INTO purging_log (action, details) VALUES (?, ?)',
|
||||
('stale_contact_removal', f'Removed stale contact: {contact_name} (last seen {contact["days_stale"]} days ago)')
|
||||
self.log_purging_action(
|
||||
"stale_contact_removal",
|
||||
f'Removed stale contact: {contact_name} (last seen {contact["days_stale"]} days ago)',
|
||||
)
|
||||
else:
|
||||
error_code = result.payload.get('error_code', 'unknown') if hasattr(result, 'payload') else 'unknown'
|
||||
@@ -3026,9 +3064,9 @@ class RepeaterManager:
|
||||
|
||||
# Log the addition if successful
|
||||
if contact_addition_successful:
|
||||
self.db_manager.execute_update(
|
||||
'INSERT INTO purging_log (action, details) VALUES (?, ?)',
|
||||
('contact_addition', f'Added discovered contact: {contact_name} - {reason}')
|
||||
self.log_purging_action(
|
||||
"contact_addition",
|
||||
f"Added discovered contact: {contact_name} - {reason}",
|
||||
)
|
||||
self.logger.info(f"Successfully added contact '{contact_name}': {reason}")
|
||||
return True
|
||||
@@ -3056,9 +3094,9 @@ class RepeaterManager:
|
||||
self.logger.debug(f"Manual contact addition toggle result: {result}")
|
||||
|
||||
# Log the action
|
||||
self.db_manager.execute_update(
|
||||
'INSERT INTO purging_log (action, details) VALUES (?, ?)',
|
||||
('manual_add_toggle', f'{"Enabled" if enabled else "Disabled"} manual contact addition - {reason}')
|
||||
self.log_purging_action(
|
||||
"manual_add_toggle",
|
||||
f'{"Enabled" if enabled else "Disabled"} manual contact addition - {reason}',
|
||||
)
|
||||
|
||||
return True
|
||||
@@ -3086,9 +3124,9 @@ class RepeaterManager:
|
||||
self.logger.debug(f"Discovery result: {result}")
|
||||
|
||||
# Log the action
|
||||
self.db_manager.execute_update(
|
||||
'INSERT INTO purging_log (action, details) VALUES (?, ?)',
|
||||
('companion_discovery', f'Manual companion contact discovery - {reason}')
|
||||
self.log_purging_action(
|
||||
"companion_discovery",
|
||||
f"Manual companion contact discovery - {reason}",
|
||||
)
|
||||
|
||||
return True
|
||||
|
||||
@@ -286,3 +286,40 @@ class TestSchema:
|
||||
(idx,),
|
||||
)
|
||||
assert cur.fetchone() is not None
|
||||
|
||||
def test_purging_log_has_details_column_after_full_migration(self, runner, conn):
|
||||
runner.run()
|
||||
cursor = conn.cursor()
|
||||
assert _column_exists(cursor, "purging_log", "details") is True
|
||||
|
||||
def test_migration_adds_purging_log_details_for_legacy_schema(self, conn, logger):
|
||||
conn.execute("""
|
||||
CREATE TABLE IF NOT EXISTS schema_version (
|
||||
version INTEGER NOT NULL,
|
||||
description TEXT,
|
||||
applied_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
|
||||
)
|
||||
""")
|
||||
for version, description, _ in MIGRATIONS:
|
||||
if version <= 11:
|
||||
conn.execute(
|
||||
"INSERT INTO schema_version (version, description) VALUES (?, ?)",
|
||||
(version, description),
|
||||
)
|
||||
conn.execute("""
|
||||
CREATE TABLE IF NOT EXISTS purging_log (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
timestamp TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
action TEXT NOT NULL,
|
||||
public_key TEXT NOT NULL,
|
||||
name TEXT NOT NULL,
|
||||
reason TEXT
|
||||
)
|
||||
""")
|
||||
conn.commit()
|
||||
|
||||
runner = MigrationRunner(conn, logger)
|
||||
runner.run()
|
||||
|
||||
cursor = conn.cursor()
|
||||
assert _column_exists(cursor, "purging_log", "details") is True
|
||||
|
||||
@@ -1910,6 +1910,7 @@ def new_contact_env(mock_logger):
|
||||
)
|
||||
rm.manage_contact_list = AsyncMock(return_value={"success": True})
|
||||
rm.add_companion_from_contact_data = AsyncMock(return_value=True)
|
||||
rm.log_purging_action = Mock()
|
||||
rm.db_manager = Mock()
|
||||
rm.db_manager.execute_update = Mock()
|
||||
rm._is_repeater_device = Mock(return_value=False)
|
||||
@@ -1933,7 +1934,7 @@ class TestHandleNewContactAutoManage:
|
||||
rm.track_contact_advertisement.assert_awaited_once()
|
||||
mesh.commands.add_contact.assert_not_called()
|
||||
rm.add_companion_from_contact_data.assert_not_called()
|
||||
rm.db_manager.execute_update.assert_called()
|
||||
rm.log_purging_action.assert_called_once()
|
||||
|
||||
async def test_device_mode_no_bot_add_contact(self, new_contact_env):
|
||||
bot, handler, rm, mesh = new_contact_env
|
||||
|
||||
@@ -90,6 +90,30 @@ class TestDetermineContactRole:
|
||||
assert rm._determine_contact_role({}) == "companion"
|
||||
|
||||
|
||||
class TestPurgingLogCompatibility:
|
||||
def test_log_purging_action_uses_details_when_available(self, rm):
|
||||
rm._purging_log_has_details = True
|
||||
rm.db_manager.execute_update = Mock()
|
||||
|
||||
rm.log_purging_action("contact_management", "managed contacts")
|
||||
|
||||
rm.db_manager.execute_update.assert_called_once_with(
|
||||
"INSERT INTO purging_log (action, details) VALUES (?, ?)",
|
||||
("contact_management", "managed contacts"),
|
||||
)
|
||||
|
||||
def test_log_purging_action_falls_back_to_legacy_columns(self, rm):
|
||||
rm._purging_log_has_details = False
|
||||
rm.db_manager.execute_update = Mock()
|
||||
|
||||
rm.log_purging_action("contact_management", "managed contacts")
|
||||
|
||||
rm.db_manager.execute_update.assert_called_once_with(
|
||||
"INSERT INTO purging_log (action, public_key, name, reason) VALUES (?, ?, ?, ?)",
|
||||
("contact_management", "", "contact_management", "managed contacts"),
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _determine_device_type
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user