From 1b7051a8108fe1fc2df2f1affa65b3719bb578e6 Mon Sep 17 00:00:00 2001 From: Ivan Date: Mon, 13 Apr 2026 17:37:06 -0500 Subject: [PATCH] feat(tests): add comprehensive tests for memory leak fixes, name overwrite handling, and frontend display name protection; refactor auto propagation tests for clarity --- tests/backend/test_auto_propagation.py | 12 +- tests/backend/test_memory_leak_fixes.py | 443 +++++++++++++++++++ tests/backend/test_name_overwrite_fixes.py | 481 +++++++++++++++++++++ tests/frontend/NameOverwriteFixes.test.js | 478 ++++++++++++++++++++ tests/frontend/ToolsPage.test.js | 5 +- 5 files changed, 1411 insertions(+), 8 deletions(-) create mode 100644 tests/backend/test_memory_leak_fixes.py create mode 100644 tests/backend/test_name_overwrite_fixes.py create mode 100644 tests/frontend/NameOverwriteFixes.test.js diff --git a/tests/backend/test_auto_propagation.py b/tests/backend/test_auto_propagation.py index f3ff204..c7b7271 100644 --- a/tests/backend/test_auto_propagation.py +++ b/tests/backend/test_auto_propagation.py @@ -54,8 +54,8 @@ async def test_auto_propagation_logic(): patch.object(manager, "_wait_for_path", return_value=True), patch.object(manager, "_probe_propagation_sync", return_value=True), ): - mock_hops.side_effect = ( - lambda dh: 1 if dh == bytes.fromhex(_VALID_HASH_A) else 3 + mock_hops.side_effect = lambda dh: ( + 1 if dh == bytes.fromhex(_VALID_HASH_A) else 3 ) await manager.check_and_update_propagation_node() @@ -79,8 +79,8 @@ async def test_auto_propagation_logic(): patch.object(manager, "_wait_for_path", return_value=True), patch.object(manager, "_probe_propagation_sync", side_effect=[False, True]), ): - mock_hops.side_effect = ( - lambda dh: 1 if dh == bytes.fromhex(_VALID_HASH_A) else 3 + mock_hops.side_effect = lambda dh: ( + 1 if dh == bytes.fromhex(_VALID_HASH_A) else 3 ) await manager.check_and_update_propagation_node() @@ -106,8 +106,8 @@ async def test_auto_propagation_logic(): patch.object(manager, "_wait_for_path", return_value=True), patch.object(manager, "_probe_propagation_sync", return_value=True), ): - mock_hops.side_effect = ( - lambda dh: 1 if dh == bytes.fromhex(_VALID_HASH_A) else 2 + mock_hops.side_effect = lambda dh: ( + 1 if dh == bytes.fromhex(_VALID_HASH_A) else 2 ) await manager.check_and_update_propagation_node() diff --git a/tests/backend/test_memory_leak_fixes.py b/tests/backend/test_memory_leak_fixes.py new file mode 100644 index 0000000..9162a44 --- /dev/null +++ b/tests/backend/test_memory_leak_fixes.py @@ -0,0 +1,443 @@ +"""Tests for the memory-leak fixes applied to websocket_broadcast, +update_lxmf_message_state, AsyncUtils.run_async future tracking, +nomadnet link cache sweeping, and the telemetry warning cap. +""" + +import asyncio +import gc +import json +import secrets +import threading +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +import RNS + +from meshchatx.src.backend.async_utils import AsyncUtils +from meshchatx.src.backend.database.messages import MessageDAO +from meshchatx.src.backend.nomadnet_downloader import ( + _nomadnet_links_lock, + nomadnet_cached_links, + sweep_stale_links, +) + + +# --------------------------------------------------------------------------- +# MessageDAO.update_lxmf_message_state +# --------------------------------------------------------------------------- + + +class TestUpdateLxmfMessageState: + @pytest.fixture + def dao(self): + provider = MagicMock() + return MessageDAO(provider), provider + + def test_update_issues_correct_sql(self, dao): + message_dao, provider = dao + message_dao.update_lxmf_message_state( + message_hash="abc123", + state="delivered", + progress=100.0, + delivery_attempts=3, + next_delivery_attempt_at=None, + rssi=-55, + snr=6.5, + quality=4, + ) + provider.execute.assert_called_once() + query, params = provider.execute.call_args[0] + assert "UPDATE lxmf_messages SET" in query + assert "WHERE hash = ?" in query + assert params[-1] == "abc123" + assert params[0] == "delivered" + assert params[1] == 100.0 + assert params[2] == 3 + + def test_update_with_none_optionals(self, dao): + message_dao, provider = dao + message_dao.update_lxmf_message_state( + message_hash="xyz", + state="sending", + progress=50.0, + delivery_attempts=1, + next_delivery_attempt_at=None, + ) + _, params = provider.execute.call_args[0] + assert params[4] is None # rssi + assert params[5] is None # snr + assert params[6] is None # quality + + def test_update_does_not_touch_fields_column(self, dao): + """The lightweight path must never rewrite the fields/content columns.""" + message_dao, provider = dao + message_dao.update_lxmf_message_state( + message_hash="h", + state="sent", + progress=99.0, + delivery_attempts=2, + next_delivery_attempt_at=None, + ) + query = provider.execute.call_args[0][0] + assert "fields" not in query.lower().split("set")[1].split("where")[0] + assert "content" not in query.lower().split("set")[1].split("where")[0] + + def test_full_roundtrip_via_real_db(self, tmp_path): + """Insert with upsert, then update state only, verify fields unchanged.""" + from meshchatx.src.backend.database import Database + + db_path = str(tmp_path / "test.db") + db = Database(db_path) + db.initialize() + + big_fields = json.dumps({"image": {"image_bytes": "A" * 5000}}) + msg = { + "hash": "deadbeef", + "source_hash": "src", + "destination_hash": "dst", + "peer_hash": "src", + "state": "outbound", + "progress": 0.0, + "is_incoming": 0, + "method": "direct", + "delivery_attempts": 0, + "next_delivery_attempt_at": None, + "title": "hi", + "content": "hello", + "fields": big_fields, + "timestamp": 12345, + "rssi": None, + "snr": None, + "quality": None, + "is_spam": 0, + "reply_to_hash": None, + "attachments_stripped": 0, + } + db.messages.upsert_lxmf_message(msg) + + db.messages.update_lxmf_message_state( + message_hash="deadbeef", + state="delivered", + progress=100.0, + delivery_attempts=2, + next_delivery_attempt_at=None, + rssi=-60, + snr=7.0, + quality=5, + ) + + row = db.messages.get_lxmf_message_by_hash("deadbeef") + assert row["state"] == "delivered" + assert row["progress"] == 100.0 + assert row["delivery_attempts"] == 2 + assert row["rssi"] == -60 + stored_fields = json.loads(row["fields"]) + assert stored_fields["image"]["image_bytes"] == "A" * 5000 + + db.close_all() + + +# --------------------------------------------------------------------------- +# sweep_stale_links +# --------------------------------------------------------------------------- + + +class TestSweepStaleLinks: + @pytest.fixture(autouse=True) + def _clear_cache(self): + with _nomadnet_links_lock: + nomadnet_cached_links.clear() + yield + with _nomadnet_links_lock: + nomadnet_cached_links.clear() + + def test_sweep_removes_inactive_links(self): + active = MagicMock() + active.status = RNS.Link.ACTIVE + dead = MagicMock() + dead.status = RNS.Link.CLOSED + + with _nomadnet_links_lock: + nomadnet_cached_links[b"a"] = active + nomadnet_cached_links[b"b"] = dead + + sweep_stale_links() + + with _nomadnet_links_lock: + assert b"a" in nomadnet_cached_links + assert b"b" not in nomadnet_cached_links + + def test_sweep_empty_cache_is_noop(self): + sweep_stale_links() + with _nomadnet_links_lock: + assert len(nomadnet_cached_links) == 0 + + def test_sweep_all_active_keeps_all(self): + for i in range(5): + link = MagicMock() + link.status = RNS.Link.ACTIVE + with _nomadnet_links_lock: + nomadnet_cached_links[bytes([i])] = link + + sweep_stale_links() + + with _nomadnet_links_lock: + assert len(nomadnet_cached_links) == 5 + + def test_sweep_thread_safety(self): + for i in range(20): + link = MagicMock() + link.status = RNS.Link.CLOSED if i % 2 else RNS.Link.ACTIVE + with _nomadnet_links_lock: + nomadnet_cached_links[bytes([i])] = link + + errors = [] + + def worker(): + try: + sweep_stale_links() + except Exception as e: + errors.append(e) + + threads = [threading.Thread(target=worker) for _ in range(8)] + for t in threads: + t.start() + for t in threads: + t.join() + + assert not errors + with _nomadnet_links_lock: + assert len(nomadnet_cached_links) == 10 + + +# --------------------------------------------------------------------------- +# AsyncUtils.run_async future tracking +# --------------------------------------------------------------------------- + + +class TestAsyncUtilsFutureTracking: + """Tests operate directly on the real AsyncUtils class internals, + bypassing the conftest global mock for the tracking logic. + """ + + @pytest.fixture(autouse=True) + def _reset(self): + AsyncUtils._pending_futures = [] + yield + AsyncUtils._pending_futures = [] + + def test_future_appended_on_schedule(self): + mock_loop = MagicMock() + mock_loop.is_running.return_value = True + future = MagicMock() + future.done.return_value = False + + real_main_loop = AsyncUtils.main_loop + real_run_async = AsyncUtils.run_async.__func__ + + try: + AsyncUtils.main_loop = mock_loop + with patch( + "meshchatx.src.backend.async_utils.asyncio.run_coroutine_threadsafe", + return_value=future, + ): + coro = asyncio.coroutine(lambda: None)() + real_run_async(coro) + finally: + AsyncUtils.main_loop = real_main_loop + + assert future in AsyncUtils._pending_futures + + def test_completed_futures_swept_at_threshold(self): + done = MagicMock() + done.done.return_value = True + pending = MagicMock() + pending.done.return_value = False + + AsyncUtils._pending_futures = ( + [done] * (AsyncUtils._FUTURES_SWEEP_THRESHOLD - 2) + [pending] + ) + + mock_loop = MagicMock() + mock_loop.is_running.return_value = True + new_future = MagicMock() + new_future.done.return_value = False + + real_main_loop = AsyncUtils.main_loop + real_run_async = AsyncUtils.run_async.__func__ + + try: + AsyncUtils.main_loop = mock_loop + with patch( + "meshchatx.src.backend.async_utils.asyncio.run_coroutine_threadsafe", + return_value=new_future, + ): + coro = asyncio.coroutine(lambda: None)() + real_run_async(coro) + finally: + AsyncUtils.main_loop = real_main_loop + + remaining = AsyncUtils._pending_futures + assert done not in remaining + assert pending in remaining + + def test_no_loop_prints_warning(self, capsys): + real_main_loop = AsyncUtils.main_loop + real_run_async = AsyncUtils.run_async.__func__ + try: + AsyncUtils.main_loop = None + coro = asyncio.coroutine(lambda: None)() + real_run_async(coro) + finally: + AsyncUtils.main_loop = real_main_loop + + captured = capsys.readouterr() + assert "WARNING" in captured.out + + +# --------------------------------------------------------------------------- +# websocket_broadcast dead-client pruning +# --------------------------------------------------------------------------- + + +class TestWebsocketBroadcastPruning: + @pytest.mark.asyncio + async def test_dead_clients_removed_on_send_failure(self): + from meshchatx.meshchat import ReticulumMeshChat + + good_ws = AsyncMock() + bad_ws = AsyncMock() + bad_ws.send_str.side_effect = ConnectionResetError("gone") + + app = MagicMock(spec=ReticulumMeshChat) + app.websocket_clients = [good_ws, bad_ws] + + await ReticulumMeshChat.websocket_broadcast(app, '{"type":"ping"}') + + good_ws.send_str.assert_called_once_with('{"type":"ping"}') + bad_ws.send_str.assert_called_once() + assert bad_ws not in app.websocket_clients + assert good_ws in app.websocket_clients + + @pytest.mark.asyncio + async def test_all_healthy_clients_kept(self): + from meshchatx.meshchat import ReticulumMeshChat + + clients = [AsyncMock() for _ in range(3)] + app = MagicMock(spec=ReticulumMeshChat) + app.websocket_clients = list(clients) + + await ReticulumMeshChat.websocket_broadcast(app, '{"ok":true}') + + assert len(app.websocket_clients) == 3 + for c in clients: + c.send_str.assert_called_once() + + @pytest.mark.asyncio + async def test_all_dead_clients_cleared(self): + from meshchatx.meshchat import ReticulumMeshChat + + clients = [AsyncMock() for _ in range(3)] + for c in clients: + c.send_str.side_effect = BrokenPipeError + + app = MagicMock(spec=ReticulumMeshChat) + app.websocket_clients = list(clients) + + await ReticulumMeshChat.websocket_broadcast(app, "data") + + assert len(app.websocket_clients) == 0 + + +# --------------------------------------------------------------------------- +# _telemetry_no_location_warned cap +# --------------------------------------------------------------------------- + + +class TestTelemetryWarnedCap: + def test_cap_at_256_then_cleared(self): + warned = set() + for i in range(256): + if len(warned) >= 256: + warned.clear() + warned.add(f"hash_{i}") + + assert len(warned) == 256 + + if len(warned) >= 256: + warned.clear() + warned.add("hash_256") + + assert len(warned) == 1 + assert "hash_256" in warned + + +# --------------------------------------------------------------------------- +# Integration: state updates don't bloat memory with attachment re-encoding +# --------------------------------------------------------------------------- + + +class TestStateUpdateMemory: + def test_state_update_avoids_large_fields_write(self, tmp_path): + """Verify that update_lxmf_message_state does NOT cause the provider + to receive the large fields blob that upsert_lxmf_message does.""" + from meshchatx.src.backend.database import Database + + db = Database(str(tmp_path / "t.db")) + db.initialize() + + big_attachment = "X" * 100_000 + fields = json.dumps({"image": {"image_bytes": big_attachment}}) + msg = { + "hash": "msg1", + "source_hash": "s", + "destination_hash": "d", + "peer_hash": "s", + "state": "outbound", + "progress": 0.0, + "is_incoming": 0, + "method": "direct", + "delivery_attempts": 0, + "next_delivery_attempt_at": None, + "title": "", + "content": "", + "fields": fields, + "timestamp": 1, + "rssi": None, + "snr": None, + "quality": None, + "is_spam": 0, + "reply_to_hash": None, + "attachments_stripped": 0, + } + db.messages.upsert_lxmf_message(msg) + + gc.collect() + import psutil, os + + mem_before = psutil.Process(os.getpid()).memory_info().rss + + for _ in range(50): + db.messages.update_lxmf_message_state( + message_hash="msg1", + state="sending", + progress=50.0, + delivery_attempts=1, + next_delivery_attempt_at=None, + ) + + gc.collect() + mem_after = psutil.Process(os.getpid()).memory_info().rss + delta_mb = (mem_after - mem_before) / (1024 * 1024) + + assert delta_mb < 5.0, ( + f"State-only updates grew memory by {delta_mb:.1f} MB; " + "they should not re-serialize attachment data" + ) + + row = db.messages.get_lxmf_message_by_hash("msg1") + assert row["state"] == "sending" + stored = json.loads(row["fields"]) + assert stored["image"]["image_bytes"] == big_attachment + + db.close_all() diff --git a/tests/backend/test_name_overwrite_fixes.py b/tests/backend/test_name_overwrite_fixes.py new file mode 100644 index 0000000..a2fb0d0 --- /dev/null +++ b/tests/backend/test_name_overwrite_fixes.py @@ -0,0 +1,481 @@ +"""Tests for the contact/conversation name overwrite bug fixes. + +Covers: +- Announce upsert preserving app_data via COALESCE when new value is NULL +- Contacts DAO upsert conflict behaviour +- Custom display name lifecycle (set, get, delete, re-set) +- Contact name update propagation +- Edge cases: empty strings, unicode, very long names, concurrent upserts +""" + +import base64 +import os +import tempfile + +import pytest + +from meshchatx.src.backend.database.announces import AnnounceDAO +from meshchatx.src.backend.database.contacts import ContactsDAO +from meshchatx.src.backend.database.provider import DatabaseProvider +from meshchatx.src.backend.database.schema import DatabaseSchema +from meshchatx.src.backend.meshchat_utils import parse_lxmf_display_name + +try: + import RNS.vendor.umsgpack as msgpack +except ImportError: + msgpack = None + + +@pytest.fixture +def temp_db(): + fd, path = tempfile.mkstemp(suffix=".db") + os.close(fd) + yield path + if os.path.exists(path): + os.remove(path) + + +@pytest.fixture +def provider(temp_db): + p = DatabaseProvider(temp_db) + schema = DatabaseSchema(p) + schema.initialize() + yield p + p.close() + + +@pytest.fixture +def announce_dao(provider): + return AnnounceDAO(provider) + + +@pytest.fixture +def contacts_dao(provider): + return ContactsDAO(provider) + + +def _make_app_data_b64(name: str) -> str: + if msgpack is None: + return base64.b64encode(name.encode()).decode() + packed = msgpack.packb([name.encode(), None, None]) + return base64.b64encode(packed).decode() + + +def _base_announce(dest="d" * 32, app_data="some_data"): + return { + "destination_hash": dest, + "aspect": "lxmf.delivery", + "identity_hash": "i" * 32, + "identity_public_key": "k" * 32, + "app_data": app_data, + "rssi": -60, + "snr": 8, + "quality": 0.9, + } + + +# --------------------------------------------------------------------------- +# Announce upsert COALESCE tests +# --------------------------------------------------------------------------- + + +class TestAnnounceUpsertPreservesAppData: + """The core bug: upserting an announce with app_data=None must NOT wipe + the previously stored app_data.""" + + def test_null_app_data_preserves_existing(self, announce_dao): + announce_dao.upsert_announce(_base_announce(app_data="original_name")) + row = announce_dao.get_announce_by_hash("d" * 32) + assert row["app_data"] == "original_name" + + announce_dao.upsert_announce(_base_announce(app_data=None)) + row = announce_dao.get_announce_by_hash("d" * 32) + assert row["app_data"] == "original_name" + + def test_new_app_data_overwrites_existing(self, announce_dao): + announce_dao.upsert_announce(_base_announce(app_data="old_name")) + announce_dao.upsert_announce(_base_announce(app_data="new_name")) + row = announce_dao.get_announce_by_hash("d" * 32) + assert row["app_data"] == "new_name" + + def test_first_insert_with_null_app_data(self, announce_dao): + announce_dao.upsert_announce(_base_announce(app_data=None)) + row = announce_dao.get_announce_by_hash("d" * 32) + assert row["app_data"] is None + + def test_first_null_then_real_then_null_again(self, announce_dao): + dest = "a" * 32 + announce_dao.upsert_announce(_base_announce(dest=dest, app_data=None)) + row = announce_dao.get_announce_by_hash(dest) + assert row["app_data"] is None + + announce_dao.upsert_announce(_base_announce(dest=dest, app_data="real_name")) + row = announce_dao.get_announce_by_hash(dest) + assert row["app_data"] == "real_name" + + announce_dao.upsert_announce(_base_announce(dest=dest, app_data=None)) + row = announce_dao.get_announce_by_hash(dest) + assert row["app_data"] == "real_name" + + def test_multiple_null_upserts_no_degradation(self, announce_dao): + dest = "b" * 32 + announce_dao.upsert_announce(_base_announce(dest=dest, app_data="stable_name")) + for _ in range(10): + announce_dao.upsert_announce(_base_announce(dest=dest, app_data=None)) + row = announce_dao.get_announce_by_hash(dest) + assert row["app_data"] == "stable_name" + + def test_other_fields_still_update_on_null_app_data(self, announce_dao): + dest = "c" * 32 + announce_dao.upsert_announce( + {**_base_announce(dest=dest, app_data="keep_me"), "rssi": -50} + ) + announce_dao.upsert_announce( + {**_base_announce(dest=dest, app_data=None), "rssi": -90} + ) + row = announce_dao.get_announce_by_hash(dest) + assert row["app_data"] == "keep_me" + assert row["rssi"] == -90 + + def test_empty_string_app_data_is_not_null(self, announce_dao): + dest = "e" * 32 + announce_dao.upsert_announce(_base_announce(dest=dest, app_data="good_name")) + announce_dao.upsert_announce(_base_announce(dest=dest, app_data="")) + row = announce_dao.get_announce_by_hash(dest) + assert row["app_data"] == "" + + +# --------------------------------------------------------------------------- +# Custom display name lifecycle +# --------------------------------------------------------------------------- + + +class TestCustomDisplayNameLifecycle: + def test_set_and_get(self, announce_dao): + announce_dao.upsert_custom_display_name("dest1", "Alice") + assert announce_dao.get_custom_display_name("dest1") == "Alice" + + def test_update_overwrites(self, announce_dao): + announce_dao.upsert_custom_display_name("dest1", "Alice") + announce_dao.upsert_custom_display_name("dest1", "Bob") + assert announce_dao.get_custom_display_name("dest1") == "Bob" + + def test_delete_removes(self, announce_dao): + announce_dao.upsert_custom_display_name("dest1", "Alice") + announce_dao.delete_custom_display_name("dest1") + assert announce_dao.get_custom_display_name("dest1") is None + + def test_get_nonexistent_returns_none(self, announce_dao): + assert announce_dao.get_custom_display_name("nonexistent") is None + + def test_delete_nonexistent_is_noop(self, announce_dao): + announce_dao.delete_custom_display_name("nonexistent") + + def test_set_after_delete(self, announce_dao): + announce_dao.upsert_custom_display_name("dest1", "First") + announce_dao.delete_custom_display_name("dest1") + announce_dao.upsert_custom_display_name("dest1", "Second") + assert announce_dao.get_custom_display_name("dest1") == "Second" + + def test_unicode_display_name(self, announce_dao): + announce_dao.upsert_custom_display_name("dest1", "\u5c71\u7530\u592a\u90ce") + assert ( + announce_dao.get_custom_display_name("dest1") == "\u5c71\u7530\u592a\u90ce" + ) + + def test_very_long_display_name(self, announce_dao): + long_name = "A" * 10000 + announce_dao.upsert_custom_display_name("dest1", long_name) + assert announce_dao.get_custom_display_name("dest1") == long_name + + def test_empty_string_display_name(self, announce_dao): + announce_dao.upsert_custom_display_name("dest1", "") + assert announce_dao.get_custom_display_name("dest1") == "" + + def test_multiple_destinations_independent(self, announce_dao): + announce_dao.upsert_custom_display_name("dest1", "Alice") + announce_dao.upsert_custom_display_name("dest2", "Bob") + announce_dao.delete_custom_display_name("dest1") + assert announce_dao.get_custom_display_name("dest1") is None + assert announce_dao.get_custom_display_name("dest2") == "Bob" + + +# --------------------------------------------------------------------------- +# Contacts DAO edge cases +# --------------------------------------------------------------------------- + + +class TestContactsEdgeCases: + def test_add_contact_upsert_preserves_addresses(self, contacts_dao, provider): + contacts_dao.add_contact( + "Alice", "ih1", lxmf_address="lxmf1", lxst_address="lxst1" + ) + contacts_dao.add_contact( + "Alice Updated", "ih1", lxmf_address=None, lxst_address=None + ) + row = provider.fetchone( + "SELECT * FROM contacts WHERE remote_identity_hash = ?", ("ih1",) + ) + assert row["name"] == "Alice Updated" + assert row["lxmf_address"] == "lxmf1" + assert row["lxst_address"] == "lxst1" + + def test_add_contact_upsert_replaces_name_unconditionally( + self, contacts_dao, provider + ): + """Verifies add_contact always overwrites name on conflict.""" + contacts_dao.add_contact("Real Name", "ih2") + contacts_dao.add_contact("Overwritten", "ih2") + row = provider.fetchone( + "SELECT * FROM contacts WHERE remote_identity_hash = ?", ("ih2",) + ) + assert row["name"] == "Overwritten" + + def test_update_contact_partial(self, contacts_dao, provider): + contacts_dao.add_contact("Alice", "ih3", lxmf_address="lx3") + row = provider.fetchone( + "SELECT id FROM contacts WHERE remote_identity_hash = ?", ("ih3",) + ) + cid = row["id"] + contacts_dao.update_contact(cid, name="Alice Renamed") + updated = provider.fetchone("SELECT * FROM contacts WHERE id = ?", (cid,)) + assert updated["name"] == "Alice Renamed" + assert updated["lxmf_address"] == "lx3" + + def test_update_contact_no_fields_is_noop(self, contacts_dao, provider): + contacts_dao.add_contact("NoOp", "ih4") + row = provider.fetchone( + "SELECT id FROM contacts WHERE remote_identity_hash = ?", ("ih4",) + ) + contacts_dao.update_contact(row["id"]) + updated = provider.fetchone("SELECT * FROM contacts WHERE id = ?", (row["id"],)) + assert updated["name"] == "NoOp" + + def test_update_contact_clear_image(self, contacts_dao, provider): + contacts_dao.add_contact( + "WithImage", "ih5", custom_image="data:image/png;base64,abc" + ) + row = provider.fetchone( + "SELECT id FROM contacts WHERE remote_identity_hash = ?", ("ih5",) + ) + assert row is not None + cid = row["id"] + contacts_dao.update_contact(cid, clear_image=True) + updated = provider.fetchone("SELECT * FROM contacts WHERE id = ?", (cid,)) + assert updated["custom_image"] is None + + def test_get_contact_by_identity_hash_matches_lxmf_address(self, contacts_dao): + contacts_dao.add_contact("Via LXMF", "ih6", lxmf_address="lxmf6") + result = contacts_dao.get_contact_by_identity_hash("lxmf6") + assert result is not None + assert result["name"] == "Via LXMF" + + def test_get_contact_by_identity_hash_matches_lxst_address(self, contacts_dao): + contacts_dao.add_contact("Via LXST", "ih7", lxst_address="lxst7") + result = contacts_dao.get_contact_by_identity_hash("lxst7") + assert result is not None + assert result["name"] == "Via LXST" + + def test_delete_nonexistent_contact(self, contacts_dao): + contacts_dao.delete_contact(99999) + + def test_unicode_contact_name(self, contacts_dao, provider): + contacts_dao.add_contact("\u00c9milie \u00d6sterreich", "ih8") + row = provider.fetchone( + "SELECT * FROM contacts WHERE remote_identity_hash = ?", ("ih8",) + ) + assert row["name"] == "\u00c9milie \u00d6sterreich" + + def test_contacts_search_case_insensitive_name(self, contacts_dao): + contacts_dao.add_contact("Charlie Delta", "ih9") + results = contacts_dao.get_contacts(search="charlie") + assert len(results) >= 1 + assert any(r["name"] == "Charlie Delta" for r in results) + + def test_contacts_count_with_search(self, contacts_dao): + contacts_dao.add_contact("Alpha", "ih10") + contacts_dao.add_contact("Beta", "ih11") + contacts_dao.add_contact("AlphaTwo", "ih12") + assert contacts_dao.get_contacts_count(search="alpha") == 2 + assert contacts_dao.get_contacts_count() == 3 + + +# --------------------------------------------------------------------------- +# parse_lxmf_display_name fallback behaviour +# --------------------------------------------------------------------------- + + +class TestParseLxmfDisplayNameFallback: + def test_none_returns_anonymous_peer(self): + assert parse_lxmf_display_name(None) == "Anonymous Peer" + + def test_none_with_custom_default(self): + assert parse_lxmf_display_name(None, default_value="Fallback") == "Fallback" + + def test_none_with_none_default(self): + assert parse_lxmf_display_name(None, default_value=None) is None + + @pytest.mark.skipif(msgpack is None, reason="msgpack not available") + def test_valid_app_data_returns_name(self): + name = "TestNode" + packed = msgpack.packb([name.encode(), None, None]) + b64 = base64.b64encode(packed).decode() + assert parse_lxmf_display_name(b64) == name + + @pytest.mark.skipif(msgpack is None, reason="msgpack not available") + def test_empty_name_in_app_data(self): + packed = msgpack.packb([b"", None, None]) + b64 = base64.b64encode(packed).decode() + result = parse_lxmf_display_name(b64) + assert result == "" + + def test_garbage_base64(self): + result = parse_lxmf_display_name("!!!not-base64!!!") + assert isinstance(result, str) + + @pytest.mark.skipif(msgpack is None, reason="msgpack not available") + def test_none_name_in_app_data_returns_default(self): + packed = msgpack.packb([None, None, None]) + b64 = base64.b64encode(packed).decode() + assert parse_lxmf_display_name(b64) == "Anonymous Peer" + + +# --------------------------------------------------------------------------- +# Integration: announce + custom name + contact name resolution priority +# --------------------------------------------------------------------------- + + +class TestNameResolutionPriority: + """Simulates the priority chain: + custom_display_name > announce app_data > contact name > 'Anonymous Peer' + """ + + def test_custom_name_wins_over_announce(self, announce_dao): + dest = "f" * 32 + announce_dao.upsert_announce( + _base_announce(dest=dest, app_data="AnnounceAlice") + ) + announce_dao.upsert_custom_display_name(dest, "CustomAlice") + + custom = announce_dao.get_custom_display_name(dest) + row = announce_dao.get_announce_by_hash(dest) + display = custom or row["app_data"] or "Anonymous Peer" + assert display == "CustomAlice" + + def test_announce_used_when_no_custom(self, announce_dao): + dest = "f" * 32 + announce_dao.upsert_announce(_base_announce(dest=dest, app_data="AnnounceOnly")) + + custom = announce_dao.get_custom_display_name(dest) + row = announce_dao.get_announce_by_hash(dest) + display = custom or row["app_data"] or "Anonymous Peer" + assert display == "AnnounceOnly" + + def test_fallback_to_anonymous_when_nothing(self, announce_dao): + dest = "f" * 32 + announce_dao.upsert_announce(_base_announce(dest=dest, app_data=None)) + + custom = announce_dao.get_custom_display_name(dest) + row = announce_dao.get_announce_by_hash(dest) + display = custom or row["app_data"] or "Anonymous Peer" + assert display == "Anonymous Peer" + + def test_contact_name_used_when_no_announce_no_custom( + self, announce_dao, contacts_dao, provider + ): + dest = "f" * 32 + announce_dao.upsert_announce(_base_announce(dest=dest, app_data=None)) + contacts_dao.add_contact("ContactAlice", dest, lxmf_address=dest) + + custom = announce_dao.get_custom_display_name(dest) + row = announce_dao.get_announce_by_hash(dest) + contact = contacts_dao.get_contact_by_identity_hash(dest) + contact_name = contact["name"] if contact else None + display = custom or row["app_data"] or contact_name or "Anonymous Peer" + assert display == "ContactAlice" + + def test_clearing_custom_name_falls_back_to_announce(self, announce_dao): + dest = "f" * 32 + announce_dao.upsert_announce(_base_announce(dest=dest, app_data="AnnounceName")) + announce_dao.upsert_custom_display_name(dest, "Custom") + announce_dao.delete_custom_display_name(dest) + + custom = announce_dao.get_custom_display_name(dest) + row = announce_dao.get_announce_by_hash(dest) + display = custom or row["app_data"] or "Anonymous Peer" + assert display == "AnnounceName" + + def test_wiping_announce_with_contact_still_resolves( + self, announce_dao, contacts_dao + ): + """After the COALESCE fix, this should not happen, but if app_data + was already NULL, the contact name should still be available.""" + dest = "f" * 32 + contacts_dao.add_contact("ContactFallback", dest, lxmf_address=dest) + announce_dao.upsert_announce(_base_announce(dest=dest, app_data=None)) + + custom = announce_dao.get_custom_display_name(dest) + row = announce_dao.get_announce_by_hash(dest) + contact = contacts_dao.get_contact_by_identity_hash(dest) + contact_name = contact["name"] if contact else None + display = custom or row["app_data"] or contact_name or "Anonymous Peer" + assert display == "ContactFallback" + + +# --------------------------------------------------------------------------- +# Announce trim does not break name resolution +# --------------------------------------------------------------------------- + + +class TestAnnounceTrimSafety: + def test_trim_does_not_remove_active_announce(self, announce_dao): + for i in range(5): + announce_dao.upsert_announce( + _base_announce(dest=f"{i:032x}", app_data=f"name_{i}") + ) + announce_dao.trim_announces_for_aspect("lxmf.delivery", max_rows=3) + remaining = announce_dao.get_announces(aspect="lxmf.delivery") + assert len(remaining) == 3 + for r in remaining: + assert r["app_data"] is not None + assert r["app_data"].startswith("name_") + + +# --------------------------------------------------------------------------- +# Contacts + custom display name sync edge cases +# --------------------------------------------------------------------------- + + +class TestContactCustomNameSync: + def test_renaming_contact_and_custom_name_independently( + self, contacts_dao, announce_dao, provider + ): + contacts_dao.add_contact("Alice", "ih20", lxmf_address="lx20") + row = provider.fetchone( + "SELECT id FROM contacts WHERE remote_identity_hash = ?", ("ih20",) + ) + cid = row["id"] + + announce_dao.upsert_custom_display_name("lx20", "Alice Custom") + contacts_dao.update_contact(cid, name="Alice Renamed") + + assert announce_dao.get_custom_display_name("lx20") == "Alice Custom" + contact = contacts_dao.get_contact(cid) + assert contact["name"] == "Alice Renamed" + + def test_synced_rename_updates_both(self, contacts_dao, announce_dao, provider): + """Simulates what the UI should do: update both contact and custom name.""" + contacts_dao.add_contact("Bob", "ih21", lxmf_address="lx21") + row = provider.fetchone( + "SELECT id FROM contacts WHERE remote_identity_hash = ?", ("ih21",) + ) + cid = row["id"] + + new_name = "Bob Renamed" + contacts_dao.update_contact(cid, name=new_name) + announce_dao.upsert_custom_display_name("lx21", new_name) + + contact = contacts_dao.get_contact(cid) + custom = announce_dao.get_custom_display_name("lx21") + assert contact["name"] == new_name + assert custom == new_name diff --git a/tests/frontend/NameOverwriteFixes.test.js b/tests/frontend/NameOverwriteFixes.test.js new file mode 100644 index 0000000..80b716a --- /dev/null +++ b/tests/frontend/NameOverwriteFixes.test.js @@ -0,0 +1,478 @@ +import { mount } from "@vue/test-utils"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import MessagesPage from "@/components/messages/MessagesPage.vue"; +import ContactsPage from "@/components/contacts/ContactsPage.vue"; +import DialogUtils from "@/js/DialogUtils"; + +vi.mock("@/js/DialogUtils", () => ({ + default: { + prompt: vi.fn(), + alert: vi.fn(), + confirm: vi.fn(() => Promise.resolve(true)), + }, +})); + +vi.mock("@/js/WebSocketConnection", () => ({ + default: { + on: vi.fn(), + off: vi.fn(), + send: vi.fn(), + }, +})); + +vi.mock("qrcode", () => ({ + default: { + toDataURL: vi.fn().mockResolvedValue("data:image/png;base64,test"), + }, +})); + +describe("MessagesPage display name protection", () => { + let axiosMock; + + beforeEach(() => { + vi.clearAllMocks(); + axiosMock = { + get: vi.fn(), + post: vi.fn(), + }; + window.api = axiosMock; + + axiosMock.get.mockImplementation((url) => { + if (url === "/api/v1/config") + return Promise.resolve({ data: { config: { lxmf_address_hash: "my-hash" } } }); + if (url === "/api/v1/lxmf/conversations") return Promise.resolve({ data: { conversations: [] } }); + if (url === "/api/v1/announces") return Promise.resolve({ data: { announces: [] } }); + if (url === "/api/v1/lxmf/conversation-pins") return Promise.resolve({ data: { peer_hashes: [] } }); + return Promise.resolve({ data: {} }); + }); + }); + + afterEach(() => { + delete window.api; + }); + + const mountMessagesPage = (props = { destinationHash: "" }) => { + return mount(MessagesPage, { + props, + global: { + mocks: { + $t: (key) => key, + $route: { query: {} }, + $router: { replace: vi.fn() }, + }, + stubs: { + MaterialDesignIcon: true, + LoadingSpinner: true, + MessagesSidebar: { + template: '
', + props: ["conversations", "selectedDestinationHash"], + }, + ConversationViewer: { + template: '
', + props: ["selectedPeer", "myLxmfAddressHash"], + }, + Modal: true, + }, + }, + }); + }; + + it("does not overwrite a known display name with Anonymous Peer from announce", async () => { + const destHash = "a".repeat(32); + const wrapper = mountMessagesPage(); + await wrapper.vm.$nextTick(); + + wrapper.vm.peers[destHash] = { + destination_hash: destHash, + display_name: "Real Name", + custom_display_name: null, + }; + + wrapper.vm.updatePeerFromAnnounce({ + destination_hash: destHash, + display_name: "Anonymous Peer", + }); + + expect(wrapper.vm.peers[destHash].display_name).toBe("Real Name"); + }); + + it("allows overwriting Anonymous Peer with a real name from announce", async () => { + const destHash = "b".repeat(32); + const wrapper = mountMessagesPage(); + await wrapper.vm.$nextTick(); + + wrapper.vm.peers[destHash] = { + destination_hash: destHash, + display_name: "Anonymous Peer", + }; + + wrapper.vm.updatePeerFromAnnounce({ + destination_hash: destHash, + display_name: "Newly Announced", + }); + + expect(wrapper.vm.peers[destHash].display_name).toBe("Newly Announced"); + }); + + it("allows updating from one real name to another real name via announce", async () => { + const destHash = "c".repeat(32); + const wrapper = mountMessagesPage(); + await wrapper.vm.$nextTick(); + + wrapper.vm.peers[destHash] = { + destination_hash: destHash, + display_name: "Old Name", + }; + + wrapper.vm.updatePeerFromAnnounce({ + destination_hash: destHash, + display_name: "New Name", + }); + + expect(wrapper.vm.peers[destHash].display_name).toBe("New Name"); + }); + + it("preserves known name when conversation list returns Anonymous Peer", async () => { + const destHash = "d".repeat(32); + const wrapper = mountMessagesPage(); + await wrapper.vm.$nextTick(); + + wrapper.vm.peers[destHash] = { + destination_hash: destHash, + display_name: "Known Peer", + custom_display_name: null, + }; + + axiosMock.get.mockImplementation((url) => { + if (url === "/api/v1/lxmf/conversations") + return Promise.resolve({ + data: { + conversations: [ + { + destination_hash: destHash, + display_name: "Anonymous Peer", + custom_display_name: null, + }, + ], + }, + }); + if (url === "/api/v1/config") + return Promise.resolve({ data: { config: { lxmf_address_hash: "my-hash" } } }); + if (url === "/api/v1/lxmf/conversation-pins") return Promise.resolve({ data: { peer_hashes: [] } }); + return Promise.resolve({ data: {} }); + }); + + await wrapper.vm.getConversations(); + await wrapper.vm.$nextTick(); + + expect(wrapper.vm.peers[destHash].display_name).toBe("Known Peer"); + }); + + it("accepts new display name from conversation list when peer was Anonymous", async () => { + const destHash = "e".repeat(32); + const wrapper = mountMessagesPage(); + await wrapper.vm.$nextTick(); + + wrapper.vm.peers[destHash] = { + destination_hash: destHash, + display_name: "Anonymous Peer", + }; + + axiosMock.get.mockImplementation((url) => { + if (url === "/api/v1/lxmf/conversations") + return Promise.resolve({ + data: { + conversations: [ + { + destination_hash: destHash, + display_name: "Resolved Name", + custom_display_name: null, + }, + ], + }, + }); + if (url === "/api/v1/config") + return Promise.resolve({ data: { config: { lxmf_address_hash: "my-hash" } } }); + if (url === "/api/v1/lxmf/conversation-pins") return Promise.resolve({ data: { peer_hashes: [] } }); + return Promise.resolve({ data: {} }); + }); + + await wrapper.vm.getConversations(); + await wrapper.vm.$nextTick(); + + expect(wrapper.vm.peers[destHash].display_name).toBe("Resolved Name"); + }); + + it("does not overwrite name via resolvePeerDisplayName when server returns Anonymous", async () => { + const destHash = "f".repeat(32); + const wrapper = mountMessagesPage(); + await wrapper.vm.$nextTick(); + + wrapper.vm.conversations = [ + { destination_hash: destHash, display_name: "Original Name", custom_display_name: null }, + ]; + wrapper.vm.selectedPeer = { destination_hash: destHash, display_name: "Original Name" }; + + axiosMock.get.mockImplementation((url) => { + if (url === "/api/v1/lxmf/conversations") + return Promise.resolve({ + data: { + conversations: [ + { + destination_hash: destHash, + display_name: "Anonymous Peer", + custom_display_name: null, + }, + ], + }, + }); + return Promise.resolve({ data: {} }); + }); + + await wrapper.vm.resolvePeerDisplayName(destHash); + await wrapper.vm.$nextTick(); + + expect(wrapper.vm.conversations[0].display_name).toBe("Original Name"); + expect(wrapper.vm.selectedPeer.display_name).toBe("Original Name"); + }); + + it("handles new peer with no prior entry gracefully", async () => { + const destHash = "1".repeat(32); + const wrapper = mountMessagesPage(); + await wrapper.vm.$nextTick(); + + wrapper.vm.updatePeerFromAnnounce({ + destination_hash: destHash, + display_name: "Anonymous Peer", + }); + + expect(wrapper.vm.peers[destHash].display_name).toBe("Anonymous Peer"); + }); + + it("preserves custom_display_name when announce does not carry it", async () => { + const destHash = "2".repeat(32); + const wrapper = mountMessagesPage(); + await wrapper.vm.$nextTick(); + + wrapper.vm.peers[destHash] = { + destination_hash: destHash, + display_name: "Announced Name", + custom_display_name: "My Custom Name", + }; + + wrapper.vm.updatePeerFromAnnounce({ + destination_hash: destHash, + display_name: "Anonymous Peer", + }); + + expect(wrapper.vm.peers[destHash].custom_display_name).toBe("My Custom Name"); + expect(wrapper.vm.peers[destHash].display_name).toBe("Announced Name"); + }); + + it("server-provided custom_display_name in announce is authoritative", async () => { + const destHash = "3".repeat(32); + const wrapper = mountMessagesPage(); + await wrapper.vm.$nextTick(); + + wrapper.vm.peers[destHash] = { + destination_hash: destHash, + display_name: "Old", + custom_display_name: "Stale Custom", + }; + + wrapper.vm.updatePeerFromAnnounce({ + destination_hash: destHash, + display_name: "Anonymous Peer", + custom_display_name: "Server Custom", + }); + + expect(wrapper.vm.peers[destHash].custom_display_name).toBe("Server Custom"); + expect(wrapper.vm.peers[destHash].display_name).toBe("Old"); + }); +}); + +describe("ContactsPage edit contact name", () => { + let axiosMock; + + beforeEach(() => { + vi.clearAllMocks(); + axiosMock = { + get: vi.fn(), + post: vi.fn(), + patch: vi.fn(), + delete: vi.fn(), + }; + window.api = axiosMock; + + axiosMock.get.mockImplementation((url) => { + if (url === "/api/v1/config") + return Promise.resolve({ + data: { + config: { + lxmf_address_hash: "a".repeat(32), + identity_public_key: "b".repeat(128), + }, + }, + }); + if (url === "/api/v1/telephone/contacts") + return Promise.resolve({ data: { contacts: [], total_count: 0 } }); + return Promise.resolve({ data: {} }); + }); + + axiosMock.patch.mockResolvedValue({ data: { message: "Contact updated" } }); + axiosMock.post.mockResolvedValue({ data: { message: "OK" } }); + }); + + afterEach(() => { + delete window.api; + }); + + const mountPage = () => + mount(ContactsPage, { + global: { + mocks: { + $t: (key) => key, + $router: { push: vi.fn() }, + }, + stubs: { + MaterialDesignIcon: true, + LxmfUserIcon: true, + }, + }, + }); + + it("editContactName updates both contact and custom display name", async () => { + DialogUtils.prompt.mockResolvedValue("Renamed Alice"); + + const wrapper = mountPage(); + await wrapper.vm.$nextTick(); + + const contact = { + id: 42, + name: "Alice", + remote_identity_hash: "a".repeat(32), + lxmf_address: "a".repeat(32), + remote_destination_hash: "a".repeat(32), + }; + + await wrapper.vm.editContactName(contact); + + expect(axiosMock.patch).toHaveBeenCalledWith("/api/v1/telephone/contacts/42", { + name: "Renamed Alice", + }); + expect(axiosMock.post).toHaveBeenCalledWith( + `/api/v1/destination/${"a".repeat(32)}/custom-display-name/update`, + { display_name: "Renamed Alice" } + ); + }); + + it("editContactName does nothing when user cancels prompt", async () => { + DialogUtils.prompt.mockResolvedValue(null); + + const wrapper = mountPage(); + await wrapper.vm.$nextTick(); + + await wrapper.vm.editContactName({ id: 1, name: "Alice" }); + + expect(axiosMock.patch).not.toHaveBeenCalled(); + expect(axiosMock.post).not.toHaveBeenCalled(); + }); + + it("editContactName does nothing when name unchanged", async () => { + DialogUtils.prompt.mockResolvedValue("Alice"); + + const wrapper = mountPage(); + await wrapper.vm.$nextTick(); + + await wrapper.vm.editContactName({ id: 1, name: "Alice" }); + + expect(axiosMock.patch).not.toHaveBeenCalled(); + }); + + it("editContactName skips custom display name when no dest hash", async () => { + DialogUtils.prompt.mockResolvedValue("New Name"); + + const wrapper = mountPage(); + await wrapper.vm.$nextTick(); + + const contact = { + id: 99, + name: "Old Name", + remote_identity_hash: null, + lxmf_address: null, + }; + + await wrapper.vm.editContactName(contact); + + expect(axiosMock.patch).toHaveBeenCalledWith("/api/v1/telephone/contacts/99", { + name: "New Name", + }); + expect(axiosMock.post).not.toHaveBeenCalled(); + }); + + it("editContactName uses lxmf_address when remote_destination_hash absent", async () => { + DialogUtils.prompt.mockResolvedValue("Updated"); + + const wrapper = mountPage(); + await wrapper.vm.$nextTick(); + + const contact = { + id: 10, + name: "Old", + remote_identity_hash: "i".repeat(32), + lxmf_address: "l".repeat(32), + }; + + await wrapper.vm.editContactName(contact); + + expect(axiosMock.post).toHaveBeenCalledWith( + `/api/v1/destination/${"l".repeat(32)}/custom-display-name/update`, + { display_name: "Updated" } + ); + }); + + it("editContactName handles API error gracefully", async () => { + DialogUtils.prompt.mockResolvedValue("Fail Name"); + axiosMock.patch.mockRejectedValue(new Error("Network error")); + + const wrapper = mountPage(); + await wrapper.vm.$nextTick(); + + const contact = { id: 5, name: "Before", remote_identity_hash: "x".repeat(32) }; + + await wrapper.vm.editContactName(contact); + + expect(axiosMock.patch).toHaveBeenCalled(); + }); + + it("editContactName with empty string still calls patch but not custom display name", async () => { + DialogUtils.prompt.mockResolvedValue(""); + + const wrapper = mountPage(); + await wrapper.vm.$nextTick(); + + const contact = { + id: 7, + name: "Was Something", + remote_identity_hash: "r".repeat(32), + lxmf_address: "r".repeat(32), + }; + + await wrapper.vm.editContactName(contact); + + expect(axiosMock.patch).toHaveBeenCalledWith("/api/v1/telephone/contacts/7", { + name: "", + }); + expect(axiosMock.post).not.toHaveBeenCalled(); + }); + + it("editContactName with no contact id does nothing", async () => { + const wrapper = mountPage(); + await wrapper.vm.$nextTick(); + + await wrapper.vm.editContactName({}); + + expect(DialogUtils.prompt).not.toHaveBeenCalled(); + expect(axiosMock.patch).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/frontend/ToolsPage.test.js b/tests/frontend/ToolsPage.test.js index af51ae5..85516fc 100644 --- a/tests/frontend/ToolsPage.test.js +++ b/tests/frontend/ToolsPage.test.js @@ -17,6 +17,7 @@ describe("ToolsPage.vue", () => { { path: "/bots", name: "bots", component: { template: "div" } }, { path: "/forwarder", name: "forwarder", component: { template: "div" } }, { path: "/documentation", name: "documentation", component: { template: "div" } }, + { path: "/licenses", name: "licenses", component: { template: "div" } }, { path: "/micron-editor", name: "micron-editor", component: { template: "div" } }, { path: "/paper-message", name: "paper-message", component: { template: "div" } }, { path: "/rnode-flasher", name: "rnode-flasher", component: { template: "div" } }, @@ -51,7 +52,7 @@ describe("ToolsPage.vue", () => { it("renders all tool rows", () => { const wrapper = mountToolsPage(); const toolRows = wrapper.findAll(".tool-row"); - expect(toolRows.length).toBe(17); + expect(toolRows.length).toBe(18); }); it("filters tools based on search query", async () => { @@ -76,6 +77,6 @@ describe("ToolsPage.vue", () => { await clearButton.trigger("click"); expect(wrapper.vm.searchQuery).toBe(""); - expect(wrapper.vm.filteredTools.length).toBe(17); + expect(wrapper.vm.filteredTools.length).toBe(18); }); });