diff --git a/packages/simplex-chat-python/pyproject.toml b/packages/simplex-chat-python/pyproject.toml index 76f22cdabe..ecc6a9d6b4 100644 --- a/packages/simplex-chat-python/pyproject.toml +++ b/packages/simplex-chat-python/pyproject.toml @@ -42,6 +42,10 @@ asyncio_mode = "auto" line-length = 100 target-version = "py311" +[tool.ruff.lint] +# Generated by the Haskell codegen; regenerating is the only way to change them. +exclude = ["src/simplex_chat/types/_*.py"] + [tool.ruff.format] # `src/simplex_chat/types/*.py` are generated by the Haskell codegen # (bots/src/API/Docs/Generate/Python.hs). Re-formatting them locally diff --git a/packages/simplex-chat-python/src/simplex_chat/__init__.py b/packages/simplex-chat-python/src/simplex_chat/__init__.py index c353b74935..a60e60c820 100644 --- a/packages/simplex-chat-python/src/simplex_chat/__init__.py +++ b/packages/simplex-chat-python/src/simplex_chat/__init__.py @@ -1,5 +1,6 @@ """SimpleX Chat — Python client library for chat bots.""" +from . import util as util # re-export the util namespace from ._version import __version__ from .api import ( ChatApi, @@ -32,17 +33,16 @@ from .bot import ( VideoMessage, VoiceMessage, ) -from .core import ChatAPIError, ChatInitError, CryptoArgs, MigrationConfirmation -from . import util as util # re-export the util namespace +from .core import ChatAPIError, ChatError, ChatInitError, CryptoArgs, MigrationConfirmation __all__ = [ - "__version__", "Bot", "BotCommand", "BotProfile", "ChatAPIError", "ChatApi", "ChatCommandError", + "ChatError", "ChatInitError", "ChatMessage", "Client", @@ -68,5 +68,6 @@ __all__ = [ "UnknownMessage", "VideoMessage", "VoiceMessage", + "__version__", "util", ] diff --git a/packages/simplex-chat-python/src/simplex_chat/__main__.py b/packages/simplex-chat-python/src/simplex_chat/__main__.py index 2fa4f3cd37..d14fa377b0 100644 --- a/packages/simplex-chat-python/src/simplex_chat/__main__.py +++ b/packages/simplex-chat-python/src/simplex_chat/__main__.py @@ -26,7 +26,7 @@ def main(argv: list[str] | None = None) -> int: path = _native._resolve_libs_dir(args.backend) print(f"libsimplex installed at: {path}") return 0 - except Exception as e: + except Exception as e: # noqa: BLE001 - a CLI: report any failure, don't traceback print(f"install failed: {e}", file=sys.stderr) return 1 diff --git a/packages/simplex-chat-python/src/simplex_chat/_native.py b/packages/simplex-chat-python/src/simplex_chat/_native.py index 313c606883..4c408479bb 100644 --- a/packages/simplex-chat-python/src/simplex_chat/_native.py +++ b/packages/simplex-chat-python/src/simplex_chat/_native.py @@ -99,7 +99,7 @@ def _stream_to_file(url: str, dest: Path, *, timeout: float = 60.0) -> None: `timeout` is per-request; we don't touch `socket.setdefaulttimeout` so other socket users in the same process aren't affected. """ - with urllib.request.urlopen(url, timeout=timeout) as resp: # noqa: S310 - https://github.com/... + with urllib.request.urlopen(url, timeout=timeout) as resp: total = int(resp.headers.get("Content-Length") or 0) received = 0 with dest.open("wb") as out: @@ -112,7 +112,7 @@ def _stream_to_file(url: str, dest: Path, *, timeout: float = 60.0) -> None: else: msg = f"\r download: {received >> 20} MiB" print(msg, end="", file=sys.stderr, flush=True) - print("", file=sys.stderr, flush=True) # newline after final progress line + print(file=sys.stderr, flush=True) # newline after final progress line def _download(target: Path, backend: Backend) -> None: diff --git a/packages/simplex-chat-python/src/simplex_chat/api.py b/packages/simplex-chat-python/src/simplex_chat/api.py index 51de063329..e3d36c45df 100644 --- a/packages/simplex-chat-python/src/simplex_chat/api.py +++ b/packages/simplex-chat-python/src/simplex_chat/api.py @@ -8,7 +8,7 @@ from typing import Any, Literal from . import _native, core, util from .core import MigrationConfirmation -from .types import CC, CEvt, CR, T +from .types import CC, CR, CEvt, T # Mirrors Node `ConnReqType` enum (api.ts:15-18) — the two possible outcomes # of `api_connect` / `api_connect_active_user` depending on the link kind. @@ -39,7 +39,7 @@ def _db_to_migrate_args(db: Db) -> tuple[str, str, _native.Backend]: raise TypeError(f"Unknown db: {db!r}") -class ChatCommandError(Exception): +class ChatCommandError(core.ChatError): """A chat command returned an unexpected response type. `response` is the raw wire response; `response_type` exposes its `type` @@ -71,7 +71,7 @@ class ChatApi: cls, db: Db, confirm: MigrationConfirmation = MigrationConfirmation.YES_UP, - ) -> "ChatApi": + ) -> ChatApi: path_or_prefix, key_or_conn, backend = _db_to_migrate_args(db) # Trigger lazy lib load with the right backend BEFORE chat_migrate_init. _native.lib_for(backend) @@ -96,8 +96,12 @@ class ChatApi: return self._started async def start_chat(self) -> None: + # serviceRequests is off: a bot answers its own address, it does not + # serve requests routed to it as a service. r = await self.send_chat_cmd( - CC.StartChat_cmd_string({"mainApp": True, "enableSndFiles": True}) + CC.StartChat_cmd_string( + {"mainApp": True, "enableSndFiles": True, "serviceRequests": False} + ) ) if r.get("type") not in ("chatStarted", "chatRunning"): raise ChatCommandError("error starting chat", r) @@ -142,12 +146,7 @@ class ChatApi: return r["contactLink"] raise ChatCommandError("error loading user address", r) except core.ChatAPIError as e: - ce = e.chat_error - if ( - ce is not None - and ce.get("type") == "errorStore" - and ce.get("storeError", {}).get("type") == "userContactLinkNotFound" - ): + if e.store_error_type == "userContactLinkNotFound": return None raise @@ -510,8 +509,10 @@ class ChatApi: raise ChatCommandError("error accepting contact request", r) async def api_reject_contact_request(self, contact_req_id: int) -> None: + # notify is not rendered into the command string, so the core reads its + # own default of off; this only keeps the argument type satisfied. r = await self.send_chat_cmd( - CC.APIRejectContact_cmd_string({"contactReqId": contact_req_id}) + CC.APIRejectContact_cmd_string({"contactReqId": contact_req_id, "notify": False}) ) if r["type"] != "contactRequestRejected": raise ChatCommandError("error rejecting contact request", r) @@ -607,6 +608,28 @@ class ChatApi: if r["type"] != "cmdOk": raise ChatCommandError("error setting contact custom data", r) + async def api_merge_contact_custom_data( + self, contact: T.Contact, key: str, value: object | None + ) -> None: + """Set or drop one key of a contact's custom data, keeping the rest. + + The set command replaces the whole column. `value=None` removes `key`. + """ + await self.api_set_contact_custom_data( + contact["contactId"], util.merged_custom_data(contact.get("customData"), key, value) + ) + + async def api_merge_group_custom_data( + self, group: T.GroupInfo, key: str, value: object | None + ) -> None: + """Set or drop one key of a group's custom data, keeping the rest. + + See `api_merge_contact_custom_data`. + """ + await self.api_set_group_custom_data( + group["groupId"], util.merged_custom_data(group.get("customData"), key, value) + ) + async def api_set_auto_accept_member_contacts(self, user_id: int, on_off: bool) -> None: r = await self.send_chat_cmd( CC.APISetUserAutoAcceptMemberContacts_cmd_string({"userId": user_id, "onOff": on_off}) @@ -632,12 +655,7 @@ class ChatApi: return r["user"] raise ChatCommandError("unexpected response", r) except core.ChatAPIError as e: - ce = e.chat_error - if ( - ce is not None - and ce.get("type") == "error" - and ce.get("errorType", {}).get("type") == "noActiveUser" - ): + if e.error_type == "noActiveUser": return None raise @@ -719,3 +737,13 @@ class ChatApi: if r["type"] == "newMemberContactSentInv": return r["contact"] raise ChatCommandError("error sending member contact invitation", r) + + async def api_accept_member_contact(self, contact_id: int) -> T.Contact: + """Accept a direct connection a group member opened with us. + + The core rejects a second accept with "connection already started". + """ + r = await self.send_chat_cmd(f"/_accept member contact @{contact_id}") + if r["type"] == "memberContactAccepted": + return r["contact"] + raise ChatCommandError("error accepting member contact", r) diff --git a/packages/simplex-chat-python/src/simplex_chat/bot.py b/packages/simplex-chat-python/src/simplex_chat/bot.py index 4e385493b2..b3e5b5ec03 100644 --- a/packages/simplex-chat-python/src/simplex_chat/bot.py +++ b/packages/simplex-chat-python/src/simplex_chat/bot.py @@ -121,8 +121,8 @@ class Bot(Client): async def _post_start(self, user: T.User) -> None: """Bots sync address first, then embed the link in the profile.""" - link = await self._sync_address(user) - await self._maybe_sync_profile(user, contact_link=link) + self._contact_link = await self._sync_address(user) + await self._maybe_sync_profile(user) async def _sync_address(self, user: T.User) -> str | None: """Address sync. Returns the public link if any, for embedding in the profile.""" diff --git a/packages/simplex-chat-python/src/simplex_chat/client.py b/packages/simplex-chat-python/src/simplex_chat/client.py index b0d144b8b9..8ec955b54a 100644 --- a/packages/simplex-chat-python/src/simplex_chat/client.py +++ b/packages/simplex-chat-python/src/simplex_chat/client.py @@ -14,7 +14,7 @@ import os import signal as _signal from collections.abc import AsyncIterator, Awaitable, Callable from dataclasses import dataclass -from typing import Any, Generic, Literal, TypeVar, overload +from typing import Any, Generic, Literal, Self, TypeVar, overload from . import util from .api import ChatApi, ChatCommandError, ContactAlreadyExistsError, Db @@ -58,7 +58,7 @@ class ParsedCommand: class Message(Generic[C]): chat_item: T.AChatItem content: C - client: "Client" + client: Client @property def chat_info(self) -> T.ChatInfo: @@ -71,7 +71,7 @@ class Message(Generic[C]): return c.get("text") # type: ignore[return-value] return None - async def reply(self, text: str) -> "Message[T.MsgContent]": + async def reply(self, text: str) -> Message[T.MsgContent]: items = await self.client.api.api_send_text_reply(self.chat_item, text) ci = items[0] content = ci["chatItem"]["content"] @@ -79,7 +79,7 @@ class Message(Generic[C]): msg_content: T.MsgContent = content["msgContent"] # type: ignore[index] return Message(chat_item=ci, content=msg_content, client=self.client) - async def reply_content(self, content: T.MsgContent) -> "Message[T.MsgContent]": + async def reply_content(self, content: T.MsgContent) -> Message[T.MsgContent]: items = await self.client.api.api_send_messages( self.chat_info, [{"msgContent": content, "mentions": {}}] ) @@ -162,6 +162,11 @@ class Client: self._api: ChatApi | None = None self._serving = False self._stop_event = asyncio.Event() + # Set by Bot once its address is known, so a later `sync_profile()` + # embeds the same link the startup sync would have. + self._contact_link: str | None = None + self._signal_handlers_installed = False + self._interrupts = 0 self._message_handlers: list[tuple[Callable[[Message[Any]], bool], MessageHandler]] = [] self._command_handlers: list[ tuple[tuple[str, ...], Callable[[Message[Any]], bool], CommandHandler] @@ -184,6 +189,26 @@ class Client: raise RuntimeError("Client not initialized — call run() or use `async with client:`") return self._api + @property + def profile(self) -> Profile: + """The profile this client identifies with. + + Mutable: change a field and call `sync_profile()` to apply it. + """ + return self._profile + + @profile.setter + def profile(self, profile: Profile) -> None: + self._profile = profile + + @property + def stop_requested(self) -> bool: + """Whether `stop()` has been called, including during startup. + + Sticky: a caller doing its own setup can unwind instead of serving. + """ + return self._stop_event.is_set() + # ------------------------------------------------------------------ # # Decorators # ------------------------------------------------------------------ # @@ -312,16 +337,12 @@ class Client: # Lifecycle # ------------------------------------------------------------------ # - async def __aenter__(self) -> "Client": + async def __aenter__(self) -> Self: # Order matters: libsimplex `/_start` requires an active user, so # ensure (or create) the user first, THEN start the chat, THEN # do post-start setup (profile sync; Bot adds address sync). - # Clear `_stop_event` here (not in `serve_forever`/`events`) so that - # a `stop()` call landing between `__aenter__` and the receive loop - # — e.g. a signal handler firing while signal handlers are being - # wired up — is preserved and causes the loop to exit immediately - # on entry. - self._stop_event.clear() + # `_stop_event` is never cleared: a stop requested during startup has + # to survive into the receive loop. A stopped client is spent. self._api = await ChatApi.init(self._db, self._confirm_migrations) try: user = await self._ensure_active_user() @@ -372,7 +393,7 @@ class Client: Default (Client): sync profile only. Bot overrides to also sync its address and embed the connection link in the profile. """ - await self._maybe_sync_profile(user, contact_link=None) + await self._maybe_sync_profile(user) def run(self) -> None: """Blocking entry: runs serve_forever() with SIGINT/SIGTERM handlers installed. @@ -390,33 +411,39 @@ class Client: ) async def _main() -> None: + # Before startup: a signal during migrations would otherwise hit + # the default disposition and kill the process mid-write. + self.install_signal_handlers() async with self: - loop = asyncio.get_running_loop() - # First Ctrl+C → graceful stop (~500ms, bounded by the - # receive-loop poll interval). Second Ctrl+C → force-exit - # immediately (in case stop_chat / close hang on a wedged - # FFI call). Standard CLI UX (jupyter, ipython, …). - sigint_count = 0 - - def on_interrupt() -> None: - nonlocal sigint_count - sigint_count += 1 - if sigint_count == 1: - log.info("stopping... (press Ctrl+C again to force exit)") - self.stop() - else: - os._exit(130) # 128 + SIGINT - - if hasattr(_signal, "SIGINT"): - try: - loop.add_signal_handler(_signal.SIGINT, on_interrupt) - loop.add_signal_handler(_signal.SIGTERM, self.stop) - except NotImplementedError: # Windows - _signal.signal(_signal.SIGINT, lambda *_: on_interrupt()) await self.serve_forever() asyncio.run(_main()) + def install_signal_handlers(self) -> None: + """Route SIGINT and SIGTERM to `stop()`. Idempotent. + + `run()` calls this itself; call it directly when driving the client + yourself. First Ctrl+C stops, a second force-exits. Needs a running loop. + """ + if self._signal_handlers_installed or not hasattr(_signal, "SIGINT"): + return + self._signal_handlers_installed = True + + def on_interrupt() -> None: + self._interrupts += 1 + if self._interrupts == 1: + log.info("stopping... (press Ctrl+C again to force exit)") + self.stop() + else: + os._exit(130) # 128 + SIGINT + + try: + loop = asyncio.get_running_loop() + loop.add_signal_handler(_signal.SIGINT, on_interrupt) + loop.add_signal_handler(_signal.SIGTERM, self.stop) + except NotImplementedError: # Windows + _signal.signal(_signal.SIGINT, lambda *_: on_interrupt()) + async def serve_forever(self) -> None: if self._serving: raise RuntimeError("already serving") @@ -450,10 +477,7 @@ class Client: self._serving = True try: while not self._stop_event.is_set(): - try: - event = await self.api.recv_chat_event(wait_us=500_000) - except asyncio.CancelledError: - raise + event = await self.api.recv_chat_event(wait_us=500_000) if event is None: continue try: @@ -551,7 +575,7 @@ class Client: text: str, *, timeout: float = 30.0, - ) -> "Message[T.MsgContent]": + ) -> Message[T.MsgContent]: """Send text to a direct contact and wait for the next reply from them. Waiters are FIFO per contact_id: two concurrent calls to the same @@ -815,20 +839,35 @@ class Client: log.info("user: %s", user["profile"]["displayName"]) return user - async def _maybe_sync_profile(self, user: T.User, *, contact_link: str | None) -> None: + async def sync_profile(self) -> bool: + """Apply the current `profile` to the active user. True if it changed. + + For what the startup sync cannot know yet, such as a display name that + depends on the database. Raises `ChatAPIError` if the core refuses it. + """ + user = await self.api.api_get_active_user() + if user is None: + raise RuntimeError("no active user") + return await self._sync_profile(user) + + async def _maybe_sync_profile(self, user: T.User) -> bool: + """The startup sync — `sync_profile()` unless the caller opted out.""" + if not self._update_profile: + return False + return await self._sync_profile(user) + + async def _sync_profile(self, user: T.User) -> bool: """Update the user profile on the wire if its fields changed. - `contact_link` is only set by Bot (to embed its address). Mirrors + `_contact_link` is only set by Bot (to embed its address). Mirrors Node `updateBotUserProfile` (bot.ts:199-214). Field-by-field comparison because user["profile"] is LocalProfile (has extra fields profileId, localAlias, preferences, peerType) so a full dict equality would always differ. """ - if not self._update_profile: - return new_profile = self._profile_to_wire() - if contact_link is not None: - new_profile["contactLink"] = contact_link + if self._contact_link is not None: + new_profile["contactLink"] = self._contact_link cur = user["profile"] changed = ( cur["displayName"] != new_profile["displayName"] @@ -842,6 +881,7 @@ class Client: if changed: log.info("profile changed, updating...") await self.api.api_update_profile(user["userId"], new_profile) + return changed def _profile_to_wire(self) -> T.Profile: """Convert the user-facing Profile dataclass to wire format. @@ -857,7 +897,7 @@ class Client: if self._profile.short_descr is not None: p["shortDescr"] = self._profile.short_descr if self._profile.image is not None: - p["image"] = self._profile.image + p["image"] = util.check_profile_image(self._profile.image) return p # ------------------------------------------------------------------ # diff --git a/packages/simplex-chat-python/src/simplex_chat/core.py b/packages/simplex-chat-python/src/simplex_chat/core.py index 075db34b52..4fc847f7de 100644 --- a/packages/simplex-chat-python/src/simplex_chat/core.py +++ b/packages/simplex-chat-python/src/simplex_chat/core.py @@ -13,16 +13,46 @@ from enum import StrEnum from typing import Any, TypedDict from . import _native -from .types import T, CR, CEvt +from .types import CR, CEvt, T -class ChatAPIError(Exception): +class ChatError(Exception): + """Base class for every failure of a chat command. + + Catch this for both `ChatAPIError` and `api.ChatCommandError`. + """ + + +class ChatAPIError(ChatError): """Raised when chat_send_cmd / chat_recv_msg_wait returns a chat error.""" def __init__(self, message: str, chat_error: T.ChatError | None = None): super().__init__(message) self.chat_error = chat_error + @property + def error_type(self) -> str | None: + """Tag of the nested `errorType`, e.g. `noActiveUser`, or None.""" + return self._nested("errorType").get("type") + + @property + def store_error_type(self) -> str | None: + """Tag of the nested `storeError`, e.g. `duplicateName`, or None.""" + return self._nested("storeError").get("type") + + @property + def command_error(self) -> str | None: + """What the core says the caller did wrong, or None. + + The only part of a `commandError` worth reading: the tag says nothing. + """ + error = self._nested("errorType") + return error.get("message") if error.get("type") == "commandError" else None + + def _nested(self, key: str) -> dict[str, Any]: + nested = (self.chat_error or {}).get(key) # type: ignore[attr-defined] + return nested if isinstance(nested, dict) else {} + class ChatInitError(Exception): """Raised when chat_migrate_init returns a DBMigrationResult error.""" diff --git a/packages/simplex-chat-python/src/simplex_chat/filters.py b/packages/simplex-chat-python/src/simplex_chat/filters.py index 8af15c1c66..a119ede25a 100644 --- a/packages/simplex-chat-python/src/simplex_chat/filters.py +++ b/packages/simplex-chat-python/src/simplex_chat/filters.py @@ -3,7 +3,8 @@ from __future__ import annotations import re -from typing import Any, Callable +from collections.abc import Callable +from typing import Any def compile_message_filter(kw: dict[str, Any]) -> Callable[[Any], bool]: diff --git a/packages/simplex-chat-python/src/simplex_chat/util.py b/packages/simplex-chat-python/src/simplex_chat/util.py index 158bb72a79..e5fbbf3fab 100644 --- a/packages/simplex-chat-python/src/simplex_chat/util.py +++ b/packages/simplex-chat-python/src/simplex_chat/util.py @@ -120,6 +120,46 @@ def ci_bot_command(chat_item: T.ChatItem) -> tuple[str, str] | None: return m.group(1), m.group(2).strip() +def merged_custom_data( + custom_data: dict[str, object] | None, key: str, value: object | None +) -> dict[str, object] | None: + """`custom_data` with `key` set to `value`, or removed when `value` is None. + + Returns None, which the set commands read as "clear the column", if empty. + """ + data = dict(custom_data or {}) + if value is None: + data.pop(key, None) + else: + data[key] = value + return data or None + + +# The apps decode these two and nothing else (base64ToBitmap in mobile and +# desktop), while the core stores any string starting with "data:". +PROFILE_IMAGE_PREFIXES = ("data:image/png;base64,", "data:image/jpg;base64,") + + +def check_profile_image(image: str) -> str: + """`image` unchanged, or ValueError if no client could render it. + + An image the apps cannot decode is still stored and broadcast, and shows + as an empty avatar to everyone. + """ + if image.startswith(PROFILE_IMAGE_PREFIXES): + return image + raise ValueError(f"profile image must start with {' or '.join(PROFILE_IMAGE_PREFIXES)}") + + +def conn_status(contact: T.Contact) -> str | None: + """Tag of a contact's active connection status, or None if it has none. + + A contact exists before its connection does, so the two are not the same. + """ + status = (contact.get("activeConn") or {}).get("connStatus") or {} + return status.get("type") + + def reaction_text(reaction: T.ACIReaction) -> str: """Format an `ACIReaction` as the emoji character or tag string.""" r = reaction["chatReaction"]["reaction"] # type: ignore[index] diff --git a/packages/simplex-chat-python/tests/test_api.py b/packages/simplex-chat-python/tests/test_api.py new file mode 100644 index 0000000000..09b5ce4c03 --- /dev/null +++ b/packages/simplex-chat-python/tests/test_api.py @@ -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) diff --git a/packages/simplex-chat-python/tests/test_client_and_waiters.py b/packages/simplex-chat-python/tests/test_client_and_waiters.py index 7c01ae576a..d40c74a0eb 100644 --- a/packages/simplex-chat-python/tests/test_client_and_waiters.py +++ b/packages/simplex-chat-python/tests/test_client_and_waiters.py @@ -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" diff --git a/packages/simplex-chat-python/tests/test_codegen.py b/packages/simplex-chat-python/tests/test_codegen.py index 509d919cfd..c5842f5d56 100644 --- a/packages/simplex-chat-python/tests/test_codegen.py +++ b/packages/simplex-chat-python/tests/test_codegen.py @@ -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(): diff --git a/packages/simplex-chat-python/tests/test_native_cache.py b/packages/simplex-chat-python/tests/test_native_cache.py index 55084eeae8..c2938ee3e4 100644 --- a/packages/simplex-chat-python/tests/test_native_cache.py +++ b/packages/simplex-chat-python/tests/test_native_cache.py @@ -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 diff --git a/packages/simplex-chat-python/tests/test_native_url.py b/packages/simplex-chat-python/tests/test_native_url.py index df96fff8ae..12270c9db1 100644 --- a/packages/simplex-chat-python/tests/test_native_url.py +++ b/packages/simplex-chat-python/tests/test_native_url.py @@ -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 diff --git a/packages/simplex-chat-python/tests/test_util.py b/packages/simplex-chat-python/tests/test_util.py index 983b1c2a56..3ea0d87e6d 100644 --- a/packages/simplex-chat-python/tests/test_util.py +++ b/packages/simplex-chat-python/tests/test_util.py @@ -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") diff --git a/src/Simplex/Chat/Protocol.hs b/src/Simplex/Chat/Protocol.hs index 15550d34e4..2f57140200 100644 --- a/src/Simplex/Chat/Protocol.hs +++ b/src/Simplex/Chat/Protocol.hs @@ -965,9 +965,10 @@ parseChatMessages msg = checkBatchLimit $ case B.head msg of checkBatchLimit ms | ms `lengthLE` maxBatchElementCount = ms | otherwise = [Left "too many messages in batch"] + -- defined prefix: GHC 8.10 does not parse a bang operand in an infix definition lengthLE :: [a] -> Int -> Bool - [] `lengthLE` !n = n >= 0 - (_ : xs) `lengthLE` !n = n > 0 && xs `lengthLE` (n - 1) + lengthLE [] !n = n >= 0 + lengthLE (_ : xs) !n = n > 0 && lengthLE xs (n - 1) parseUncompressed c s = case c of '[' -> case J.eitherDecodeStrict' s of Right v -> map (fmap plainMsg . parseItem) v diff --git a/src/Simplex/Chat/Store/Profiles.hs b/src/Simplex/Chat/Store/Profiles.hs index ce6a8c4c9f..0613069dd7 100644 --- a/src/Simplex/Chat/Store/Profiles.hs +++ b/src/Simplex/Chat/Store/Profiles.hs @@ -135,7 +135,6 @@ import Database.SQLite.Simple.QQ (sql) createUserRecordAt :: DB.Connection -> AgentUserId -> Bool -> Bool -> Profile -> Bool -> UTCTime -> ExceptT StoreError IO User createUserRecordAt db (AgentUserId auId) userChatRelay clientService Profile {displayName, fullName, shortDescr, description, image, peerType, preferences = userPreferences} activeUser currentTs = checkConstraint SEDuplicateName . liftIO $ do - when activeUser $ DB.execute_ db "UPDATE users SET active_user = 0" let showNtfs = True sendRcptsContacts = True sendRcptsSmallGroups = True @@ -148,6 +147,9 @@ createUserRecordAt db (AgentUserId auId) userChatRelay clientService Profile {di :. (BI showNtfs, BI sendRcptsContacts, BI sendRcptsSmallGroups, BI autoAcceptMemberContacts, BI clientService, currentTs, currentTs) ) userId <- insertedRowId db + -- After the insert: the name is unique in users, so a duplicate fails + -- above, and deactivating first would commit a database with no active user. + when activeUser $ DB.execute db "UPDATE users SET active_user = 0 WHERE user_id != ?" (Only userId) DB.execute db "INSERT INTO display_names (local_display_name, ldn_base, user_id, created_at, updated_at) VALUES (?,?,?,?,?)" @@ -338,12 +340,14 @@ updateUserProfile db user p' | otherwise = checkConstraint SEDuplicateName . liftIO $ do currentTs <- getCurrentTime - DB.execute db "UPDATE users SET local_display_name = ?, updated_at = ? WHERE user_id = ?" (newName, currentTs, userId) - userMemberProfileUpdatedAt' <- updateUserMemberProfileUpdatedAt_ currentTs + -- Insert first: checkConstraint returns the violation as a value, so the + -- transaction commits, keeping whatever ran before the failing insert. DB.execute db "INSERT INTO display_names (local_display_name, ldn_base, user_id, created_at, updated_at) VALUES (?,?,?,?,?)" (newName, newName, userId, currentTs, currentTs) + DB.execute db "UPDATE users SET local_display_name = ?, updated_at = ? WHERE user_id = ?" (newName, currentTs, userId) + userMemberProfileUpdatedAt' <- updateUserMemberProfileUpdatedAt_ currentTs updateUserProfileFields_' db userId profileId p' currentTs updateContactLDN_ db user userContactId localDisplayName newName currentTs pure user {localDisplayName = newName, profile = (toLocalProfile profileId p' localAlias currentTs (Just False) Nothing) {localBadge}, fullPreferences, userMemberProfileUpdatedAt = userMemberProfileUpdatedAt'} diff --git a/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt b/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt index 6884bf7e04..f4dcb5c5a7 100644 --- a/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt +++ b/src/Simplex/Chat/Store/SQLite/Migrations/chat_query_plans.txt @@ -7931,6 +7931,10 @@ Query: UPDATE users SET active_user = 0 Plan: SCAN users +Query: UPDATE users SET active_user = 0 WHERE user_id != ? +Plan: +SCAN users + Query: UPDATE users SET active_user = 1, active_order = ? WHERE user_id = ? Plan: SEARCH users USING INTEGER PRIMARY KEY (rowid=?)