core, python: fix refused renames, extend the client API (#7379)

* core: do not commit refused name changes * feat(python): add error base and missing commands * feat(python): let callers drive startup themselves * style(python): satisfy the linter, skip generated types * feat(python): expose the message of a command error * core: update query plans * core: fix the batch limit parse error on GHC 8.10 * feat(python): reject profile images no client can render
2026-08-27 22:34:51 +00:00 · 2026-08-17 09:02:14 +01:00
parent caac5a61a9
commit 55d18efe24
19 changed files with 718 additions and 82 deletions
@@ -0,0 +1,174 @@
+"""ChatApi commands and error classification, without the native controller.
+
+`ChatApi` only touches the FFI through `send_chat_cmd`, so replacing that one
+method exercises every wrapper: the command string it builds and the response
+shape it accepts.
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+import pytest
+
+from simplex_chat import ChatApi, ChatAPIError, ChatCommandError, ChatError
+
+
+class FakeCtrl(ChatApi):
+    """ChatApi with the FFI call replaced by a scripted response."""
+
+    def __init__(self, response: Any = None, raises: Exception | None = None) -> None:
+        super().__init__(ctrl=1)
+        self.response = response
+        self.raises = raises
+        self.sent: list[str] = []
+
+    async def send_chat_cmd(self, cmd: str) -> Any:
+        self.sent.append(cmd)
+        if self.raises is not None:
+            raise self.raises
+        return self.response
+
+
+# ---------------------------------------------------------------------- #
+# Error hierarchy
+# ---------------------------------------------------------------------- #
+
+
+def test_both_command_failures_share_one_base():
+    # The two are raised from different layers for the same kind of failure;
+    # callers should not have to name both.
+    assert issubclass(ChatAPIError, ChatError)
+    assert issubclass(ChatCommandError, ChatError)
+
+
+def test_store_error_type_reads_the_nested_tag():
+    e = ChatAPIError("x", {"type": "errorStore", "storeError": {"type": "duplicateName"}})
+    assert e.store_error_type == "duplicateName"
+    assert e.error_type is None
+
+
+def test_error_type_reads_the_nested_tag():
+    e = ChatAPIError("x", {"type": "error", "errorType": {"type": "noActiveUser"}})
+    assert e.error_type == "noActiveUser"
+    assert e.store_error_type is None
+
+
+def test_command_error_carries_the_message():
+    # The tag is always "commandError"; the message is the whole content.
+    e = ChatAPIError(
+        "x", {"type": "error", "errorType": {"type": "commandError", "message": "name too long"}}
+    )
+    assert e.command_error == "name too long"
+
+
+def test_command_error_of_another_failure():
+    e = ChatAPIError("x", {"type": "errorStore", "storeError": {"type": "duplicateName"}})
+    assert e.command_error is None
+
+
+def test_error_tags_of_an_unrelated_error():
+    e = ChatAPIError("x", {"type": "errorAgent", "agentError": {"type": "CRITICAL"}})
+    assert e.error_type is None
+    assert e.store_error_type is None
+
+
+def test_error_tags_without_a_chat_error():
+    # Raised when the controller returns something that is not valid JSON-RPC.
+    e = ChatAPIError("invalid chat command result")
+    assert e.error_type is None
+    assert e.store_error_type is None
+
+
+# ---------------------------------------------------------------------- #
+# Errors surfaced as absence
+# ---------------------------------------------------------------------- #
+
+
+async def test_missing_address_reads_as_none():
+    api = FakeCtrl(
+        raises=ChatAPIError(
+            "x", {"type": "errorStore", "storeError": {"type": "userContactLinkNotFound"}}
+        )
+    )
+    assert await api.api_get_user_address(1) is None
+
+
+async def test_another_store_error_still_raises():
+    api = FakeCtrl(
+        raises=ChatAPIError("x", {"type": "errorStore", "storeError": {"type": "dBBusyError"}})
+    )
+    with pytest.raises(ChatAPIError):
+        await api.api_get_user_address(1)
+
+
+async def test_no_active_user_reads_as_none():
+    api = FakeCtrl(
+        raises=ChatAPIError("x", {"type": "error", "errorType": {"type": "noActiveUser"}})
+    )
+    assert await api.api_get_active_user() is None
+
+
+async def test_another_error_from_the_user_query_still_raises():
+    api = FakeCtrl(
+        raises=ChatAPIError("x", {"type": "error", "errorType": {"type": "invalidConnReq"}})
+    )
+    with pytest.raises(ChatAPIError):
+        await api.api_get_active_user()
+
+
+# ---------------------------------------------------------------------- #
+# Member contacts
+# ---------------------------------------------------------------------- #
+
+
+async def test_accept_member_contact():
+    contact = {"contactId": 7}
+    api = FakeCtrl({"type": "memberContactAccepted", "contact": contact})
+    assert await api.api_accept_member_contact(7) is contact
+    assert api.sent == ["/_accept member contact @7"]
+
+
+async def test_accept_member_contact_rejected():
+    # The core answers a second accept with a command error, not a contact.
+    api = FakeCtrl({"type": "chatCmdError"})
+    with pytest.raises(ChatCommandError):
+        await api.api_accept_member_contact(7)
+
+
+# ---------------------------------------------------------------------- #
+# Custom data
+# ---------------------------------------------------------------------- #
+
+
+async def test_merge_contact_custom_data_keeps_other_keys():
+    api = FakeCtrl({"type": "cmdOk"})
+    contact = {"contactId": 4, "customData": {"other": 1}}
+    await api.api_merge_contact_custom_data(contact, "mine", {"roster": "active"})
+    assert api.sent == ['/_set custom @4 {"other": 1, "mine": {"roster": "active"}}']
+
+
+async def test_merge_contact_custom_data_removing_the_last_key_clears_the_column():
+    api = FakeCtrl({"type": "cmdOk"})
+    contact = {"contactId": 4, "customData": {"mine": 1}}
+    await api.api_merge_contact_custom_data(contact, "mine", None)
+    assert api.sent == ["/_set custom @4"]
+
+
+async def test_merge_group_custom_data_keeps_other_keys():
+    api = FakeCtrl({"type": "cmdOk"})
+    group = {"groupId": 9, "customData": {"other": 1}}
+    await api.api_merge_group_custom_data(group, "mine", {"rostered": True})
+    assert api.sent == ['/_set custom #9 {"other": 1, "mine": {"rostered": true}}']
+
+
+async def test_merge_group_custom_data_on_a_group_with_no_custom_data():
+    api = FakeCtrl({"type": "cmdOk"})
+    await api.api_merge_group_custom_data({"groupId": 9}, "mine", 1)
+    assert api.sent == ['/_set custom #9 {"mine": 1}']
+
+
+async def test_a_failed_custom_data_write_raises():
+    api = FakeCtrl({"type": "chatCmdError"})
+    with pytest.raises(ChatCommandError):
+        await api.api_merge_group_custom_data({"groupId": 9}, "mine", 1)
@@ -614,3 +614,238 @@ def test_events_raises_if_already_serving():
                pass

    asyncio.run(go())
+
+
+class _StubApi:
+    """The controller calls `__aenter__` makes, with nothing behind them."""
+
+    def __init__(self) -> None:
+        self.profiles: list[dict] = []
+        self.user: dict = {"userId": 1, "profile": {"displayName": "x", "fullName": ""}}
+        self.address: dict | None = None
+
+    @classmethod
+    async def init(cls, *_a, **_kw):
+        return cls()
+
+    @property
+    def started(self):
+        return False
+
+    async def start_chat(self):
+        pass
+
+    async def stop_chat(self):
+        pass
+
+    async def close(self):
+        pass
+
+    async def api_get_active_user(self):
+        return self.user
+
+    async def api_update_profile(self, _user_id, profile):
+        self.profiles.append(profile)
+
+    async def api_get_user_address(self, _user_id):
+        return self.address
+
+    async def api_set_address_settings(self, _user_id, _settings):
+        pass
+
+    async def send_chat_cmd(self, _cmd):
+        return {"type": "cmdOk"}
+
+
+def _client_with_stub_api(monkeypatch, **kw) -> tuple[Client, _StubApi]:
+    import simplex_chat.client as client_mod
+
+    api = _StubApi()
+    monkeypatch.setattr(client_mod, "ChatApi", _init_returning(api))
+    client = Client(profile=Profile(display_name="x"), db=SqliteDb(file_prefix="/tmp/test"), **kw)
+    return client, api
+
+
+def _init_returning(api: _StubApi):
+    """A stand-in for the ChatApi class whose `init` hands back `api`."""
+    return type("_Init", (), {"init": staticmethod(lambda *_a, **_kw: _done(api))})
+
+
+async def _done(value):
+    return value
+
+
+def test_stop_before_start_is_not_lost(monkeypatch):
+    """A signal handler installed before startup — the only way to survive a
+    Ctrl+C during database migrations — sets the stop event before __aenter__
+    runs. Clearing it there would begin serving a client the operator has
+    already stopped."""
+    c, api = _client_with_stub_api(monkeypatch)
+
+    async def go():
+        c.stop()
+        assert c.stop_requested
+        async with c:
+            assert c.stop_requested, "stop intent was cleared by __aenter__"
+            await c.serve_forever()  # must return immediately, never polling
+
+    api.recv_chat_event = _never_called  # type: ignore[attr-defined]
+    asyncio.run(go())
+
+
+async def _never_called(*_a, **_kw):
+    raise AssertionError("receive loop should have exited immediately")
+
+
+def test_stop_requested_is_false_until_stopped(monkeypatch):
+    c, _ = _client_with_stub_api(monkeypatch)
+    assert not c.stop_requested
+    c.stop()
+    assert c.stop_requested
+
+
+def test_install_signal_handlers_routes_both_signals(monkeypatch):
+    import signal as signal_mod
+
+    c, _ = _client_with_stub_api(monkeypatch)
+    registered: dict[int, object] = {}
+
+    async def go():
+        loop = asyncio.get_running_loop()
+        monkeypatch.setattr(
+            loop, "add_signal_handler", lambda sig, cb, *a: registered.__setitem__(sig, cb)
+        )
+        c.install_signal_handlers()
+
+    asyncio.run(go())
+    assert set(registered) == {signal_mod.SIGINT, signal_mod.SIGTERM}
+    registered[signal_mod.SIGINT]()  # type: ignore[operator]
+    assert c.stop_requested
+
+
+def test_install_signal_handlers_is_idempotent(monkeypatch):
+    c, _ = _client_with_stub_api(monkeypatch)
+    calls: list[int] = []
+
+    async def go():
+        loop = asyncio.get_running_loop()
+        monkeypatch.setattr(loop, "add_signal_handler", lambda sig, cb, *a: calls.append(sig))
+        c.install_signal_handlers()
+        c.install_signal_handlers()
+
+    asyncio.run(go())
+    assert len(calls) == 2, "second call re-registered the handlers"
+
+
+def test_second_interrupt_force_exits(monkeypatch):
+    """A stop that hangs in stop_chat/close must not trap the operator."""
+    import signal as signal_mod
+
+    import simplex_chat.client as client_mod
+
+    c, _ = _client_with_stub_api(monkeypatch)
+    registered: dict[int, object] = {}
+    exits: list[int] = []
+    monkeypatch.setattr(client_mod.os, "_exit", lambda code: exits.append(code))
+
+    async def go():
+        loop = asyncio.get_running_loop()
+        monkeypatch.setattr(
+            loop, "add_signal_handler", lambda sig, cb, *a: registered.__setitem__(sig, cb)
+        )
+        c.install_signal_handlers()
+
+    asyncio.run(go())
+    on_interrupt = registered[signal_mod.SIGINT]
+    on_interrupt()  # type: ignore[operator]
+    assert exits == []
+    on_interrupt()  # type: ignore[operator]
+    assert exits == [130]
+
+
+def test_sync_profile_applies_a_change_made_after_start(monkeypatch):
+    """The name a bot can use may only be knowable once the database is
+    readable, which is after start. Without this the profile could only be
+    set before the client was started."""
+    c, api = _client_with_stub_api(monkeypatch, update_profile=False)
+
+    async def go():
+        async with c:
+            assert api.profiles == [], "update_profile=False still synced on start"
+            c.profile.display_name = "Helpdesk"
+            assert await c.sync_profile() is True
+
+    asyncio.run(go())
+    assert api.profiles == [{"displayName": "Helpdesk", "fullName": ""}]
+
+
+def test_sync_profile_is_a_no_op_when_nothing_differs(monkeypatch):
+    """api_update_profile broadcasts to every contact; an unchanged profile
+    must not become traffic for all of them."""
+    c, api = _client_with_stub_api(monkeypatch, update_profile=False)
+
+    async def go():
+        async with c:
+            assert await c.sync_profile() is False
+
+    asyncio.run(go())
+    assert api.profiles == []
+
+
+def test_sync_profile_without_an_active_user(monkeypatch):
+    c, api = _client_with_stub_api(monkeypatch, update_profile=False)
+
+    async def go():
+        async with c:
+            api.user = None  # type: ignore[assignment]
+            with pytest.raises(RuntimeError, match="no active user"):
+                await c.sync_profile()
+
+    asyncio.run(go())
+
+
+def test_sync_profile_keeps_the_bot_address_in_the_profile(monkeypatch):
+    """The address is embedded by the startup sync; a later sync must not
+    drop it, or the profile would stop advertising where to connect."""
+    import simplex_chat.client as client_mod
+
+    api = _StubApi()
+    api.address = {
+        "connLinkContact": {"connFullLink": "https://l"},
+        "addressSettings": {"businessAddress": False, "autoAccept": {"acceptIncognito": False}},
+    }
+    api.user = {
+        "userId": 1,
+        "profile": {"displayName": "x", "fullName": "", "contactLink": "https://l"},
+    }
+    monkeypatch.setattr(client_mod, "ChatApi", _init_returning(api))
+    bot = Bot(
+        profile=BotProfile(display_name="x"),
+        db=SqliteDb(file_prefix="/tmp/test"),
+        update_profile=False,
+    )
+
+    async def go():
+        async with bot:
+            bot.profile.display_name = "Helpdesk"
+            await bot.sync_profile()
+
+    asyncio.run(go())
+    assert api.profiles[0]["contactLink"] == "https://l"
+
+
+def test_profile_can_be_replaced(monkeypatch):
+    c, _ = _client_with_stub_api(monkeypatch)
+    c.profile = Profile(display_name="other", full_name="Other")
+    assert c._profile_to_wire() == {"displayName": "other", "fullName": "Other"}
+
+
+def test_the_profile_image_is_checked_before_it_is_sent(monkeypatch):
+    """An image the apps cannot decode is stored and broadcast by the core,
+    and then shows as an empty avatar to every contact."""
+    c, _ = _client_with_stub_api(monkeypatch)
+    c.profile = Profile(display_name="x", image="data:image/jpeg;base64,AAA")
+    with pytest.raises(ValueError, match="must start with"):
+        c._profile_to_wire()
+    c.profile = Profile(display_name="x", image="data:image/png;base64,AAA")
+    assert c._profile_to_wire()["image"] == "data:image/png;base64,AAA"
@@ -2,7 +2,7 @@

 import typing

-from simplex_chat.types import CC, CEvt, CR, T
+from simplex_chat.types import CC, CR, CEvt, T


 def test_types_module_imports():
@@ -3,7 +3,7 @@ from pathlib import Path

 import pytest

-from simplex_chat._native import _cache_root, _resolve_libs_dir, _download
+from simplex_chat._native import _cache_root, _download, _resolve_libs_dir
 from simplex_chat._version import LIBS_VERSION


@@ -1,6 +1,8 @@
 from unittest.mock import patch
+
 import pytest
-from simplex_chat._native import _platform_tag, _libs_url, _libname
+
+from simplex_chat._native import _libname, _libs_url, _platform_tag
 from simplex_chat._version import LIBS_VERSION


@@ -1,3 +1,5 @@
+import pytest
+
 from simplex_chat import util


@@ -173,3 +175,73 @@ def test_reaction_text_emoji():
 def test_reaction_text_tag():
    r = {"chatReaction": {"reaction": {"type": "unknown", "tag": "thumbs_up"}}}
    assert util.reaction_text(r) == "thumbs_up"
+
+
+def test_merged_custom_data_adds_a_key_keeping_the_others():
+    data = {"other": {"kept": True}}
+    assert util.merged_custom_data(data, "mine", {"roster": "active"}) == {
+        "other": {"kept": True},
+        "mine": {"roster": "active"},
+    }
+
+
+def test_merged_custom_data_does_not_mutate_the_original():
+    data = {"other": 1}
+    util.merged_custom_data(data, "mine", 2)
+    assert data == {"other": 1}
+
+
+def test_merged_custom_data_replaces_an_existing_key():
+    assert util.merged_custom_data({"mine": "old"}, "mine", "new") == {"mine": "new"}
+
+
+def test_merged_custom_data_on_an_empty_column():
+    assert util.merged_custom_data(None, "mine", 1) == {"mine": 1}
+
+
+def test_merged_custom_data_removes_a_key():
+    assert util.merged_custom_data({"mine": 1, "other": 2}, "mine", None) == {"other": 2}
+
+
+def test_merged_custom_data_clears_the_column_when_nothing_is_left():
+    # None is what the set commands read as "clear"; {} would be a wasted write
+    # of an empty object.
+    assert util.merged_custom_data({"mine": 1}, "mine", None) is None
+
+
+def test_merged_custom_data_removing_a_key_that_is_not_there():
+    assert util.merged_custom_data({"other": 2}, "mine", None) == {"other": 2}
+
+
+def test_conn_status_reads_the_tag():
+    contact = {"activeConn": {"connStatus": {"type": "ready"}}}
+    assert util.conn_status(contact) == "ready"
+
+
+def test_conn_status_without_a_connection():
+    # api_create_member_contact produces exactly this: a contact row before
+    # any connection exists.
+    assert util.conn_status({"contactId": 3}) is None
+
+
+def test_conn_status_with_a_null_connection():
+    assert util.conn_status({"activeConn": None}) is None
+
+
+def test_check_profile_image_accepts_what_the_apps_decode():
+    png = "data:image/png;base64,AAA"
+    jpg = "data:image/jpg;base64,AAA"
+    assert util.check_profile_image(png) == png
+    assert util.check_profile_image(jpg) == jpg
+
+
+def test_check_profile_image_rejects_another_media_type():
+    # image/jpeg is the easy mistake: the file extension is .jpeg, and the
+    # core stores it, but no client strips that prefix before decoding.
+    with pytest.raises(ValueError, match="must start with"):
+        util.check_profile_image("data:image/jpeg;base64,AAA")
+
+
+def test_check_profile_image_rejects_a_remote_url():
+    with pytest.raises(ValueError, match="must start with"):
+        util.check_profile_image("https://simplex.chat/logo.png")