Переглянути джерело

Harden types._snake_case against acronym runs (same bug class as StartDM)

Audit for other instances of the naive per-capital snake_case splitter:
types._snake_case is the only other one in the codebase. Its inputs are
hand-written camelCase oneof candidates (18 across timeline +
notification parsing) and NO current oneof field in the proto set has
an acronym run, so its output is unchanged today -- latent, not live.
Made it acronym-aware with the same boundary rule as the plugin's
_method_name, added a unit test pinning it (startDMReceived ->
start_dm_received), and noted the post2 hardening in the AGENTS.md
gotcha.

Also verified by cross-checking compiled descriptors: realtime event
oneof (48 fields), RoomTimelineEvent.event (11), NotificationSignal.kind
(10) -- no capitals anywhere; every hardcoded kind string in bot.py
resolves to a real oneof field; and every self._svc.* call name in
client.py round-trips through the fixed plugin mapper unchanged.

Verification: ruff clean; mypy unchanged vs baseline (6 pre-existing);
pytest 95 passed 2 skipped.

Bumped version 0.5.0b15.post1 -> 0.5.0b15.post2.
the-codemancer 2 тижнів тому
батько
коміт
8a86aa5890
4 змінених файлів з 28 додано та 4 видалено
  1. 1 1
      AGENTS.md
  2. 1 1
      pyproject.toml
  3. 14 2
      src/chattolib/types.py
  4. 12 0
      tests/test_client.py

+ 1 - 1
AGENTS.md

@@ -142,7 +142,7 @@ to its grants.
 - **0.5.0b10: DM rooms keep deleted participants in `member_user_ids`.** Since 0.5.0-beta.10, `RoomWithViewerState.member_user_ids` on a DM includes participants whose accounts were deleted (the directory no longer prunes them); clients render those as deleted users. Channel rooms still carry an empty list. The wire shape is unchanged (same field, same number) and `RoomWithViewerState.parse` already copies the full list — so no code change was needed, but code that resolves every `member_user_ids` entry to a `User` must tolerate users that `get_user` no longer returns.
 - **0.5.0b14: pinning is gated by a dedicated `message.pin` permission (server-side behavior change; no wire change).** `RoomService.CreatePinnedMessage`/`DeletePinnedMessage` required `room.manage` until 0.5.0-beta.13; since 0.5.0-beta.14 they require the new `message.pin` permission instead. The RPCs, their request/response messages, and chattolib's `pin_message`/`unpin_message`/`list_pinned_messages` wrappers are unchanged — but a bot/role that pinned via `room.manage` must now hold `message.pin` (or the server returns `PERMISSION_DENIED`). Same release added an additive `livekit` (`AdminLiveKitStatus`) field 6 to admin `GetSystemInfoResponse` (LiveKit setup/config state, server-API reachability as `AdminLiveKitConnectionState`, webhook receipt times; never the API/webhook secrets) — `admin_get_system_info()` returns it raw in `data["livekit"]` with no client change.
 - **Never regenerate protos from `chattocorp/chatto@main`.** `scripts/generate_pb.sh` defaults to a tag derived from `pyproject.toml`: `v<base>` for finals, and `v<base>-<kind>.<n>` for PEP 440 pre-releases (`0.5.0b6` → `v0.5.0-beta.6`), probing candidates with `git ls-remote` (exact ref match; the REST `/tags/{name}` endpoint 404s on lightweight tags). Override with `CHATTO_REF=<tag>` only when you know the deployed server has caught up. chattolib 0.4.19 shipped once from `main` and broke realtime for every downstream client because Chatto's main had a protocol-v2 rewrite that the deployed 0.4.19 server did not speak.
-- **0.5.0b15.post1: the protoc plugin's `_method_name` is acronym-aware.** The old splitter inserted `_` before *every* capital, so `StartDM` generated `start_d_m` — while `client.py` called `start_dm`, making `start_dm()` an `AttributeError` at call time (shipped broken since the 0.5.x ConnectRPC regeneration; `StartDM` is the only RPC in the current proto set with an acronym run, so nothing else changed). Fixed in `src/chattolib/_protoc_plugin.py` (`StartDM` → `start_dm`; `GetRoomDMStats` → `get_room_dm_stats`); regenerate to pick it up. Note the server side: a **bot key cannot call `StartDM` at all** (`PERMISSION_DENIED` — DMs are human-initiated; a bot can only answer inside a DM room a human already opened), so the wrapper is kept for human-login-style use but is not exercised by bot integration tests.
+- **0.5.0b15.post1: the protoc plugin's `_method_name` is acronym-aware.** The old splitter inserted `_` before *every* capital, so `StartDM` generated `start_d_m` — while `client.py` called `start_dm`, making `start_dm()` an `AttributeError` at call time (shipped broken since the 0.5.x ConnectRPC regeneration; `StartDM` is the only RPC in the current proto set with an acronym run, so nothing else changed). Fixed in `src/chattolib/_protoc_plugin.py` (`StartDM` → `start_dm`; `GetRoomDMStats` → `get_room_dm_stats`); regenerate to pick it up. Note the server side: a **bot key cannot call `StartDM` at all** (`PERMISSION_DENIED` — DMs are human-initiated; a bot can only answer inside a DM room a human already opened), so the wrapper is kept for human-login-style use but is not exercised by bot integration tests. `types._snake_case` (the same bug class, used for timeline/notification `kind` names) was hardened the same way in 0.5.0b15.post2 — latent today: no oneof field in the current proto set has an acronym run, so its output is unchanged.
 
 ### Known bugs
 

+ 1 - 1
pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "chattolib"
-version = "0.5.0b15.post1"
+version = "0.5.0b15.post2"
 description = "Async Python client library for the Chatto webchat Connect API"
 requires-python = ">=3.11"
 license = "MPL-2.0 AND Apache-2.0"

+ 14 - 2
src/chattolib/types.py

@@ -1012,10 +1012,22 @@ class TimelineEvent:
 
 
 def _snake_case(name: str) -> str:
+    """snake_case a camelCase oneof field name (acronym-aware).
+
+    Mirrors the chattolib protoc plugin's ``_method_name``: an underscore
+    goes before an uppercase letter only at a word boundary — after a
+    lowercase/digit, or before the final capital of a run that is followed
+    by a lowercase letter. A naive per-capital split would turn a future
+    acronym field like ``startDMReceived`` into ``start_d_m_received`` and
+    silently break any ``kind == "..."`` comparison.
+    """
     out: list[str] = []
-    for ch in name:
+    for i, ch in enumerate(name):
         if ch.isupper() and out:
-            out.append("_")
+            prev = name[i - 1]
+            nxt = name[i + 1] if i + 1 < len(name) else ""
+            if not prev.isupper() or nxt.islower():
+                out.append("_")
         out.append(ch.lower())
     return "".join(out)
 

+ 12 - 0
tests/test_client.py

@@ -1024,3 +1024,15 @@ async def test_update_settings_share_timezone(client):
 
     assert settings.timezone == "Europe/Berlin"
     assert settings.share_timezone is True
+
+
+def test_snake_case_is_acronym_aware():
+    """``types._snake_case`` must not chop acronym runs (same bug class as
+    the protoc plugin's old ``StartDM`` -> ``start_d_m``)."""
+    from chattolib.types import _snake_case
+
+    assert _snake_case("messagePosted") == "message_posted"
+    assert _snake_case("directMessageReceived") == "direct_message_received"
+    # hypothetical acronym field: DM stays whole, boundary before the word after it
+    assert _snake_case("startDMReceived") == "start_dm_received"
+    assert _snake_case("getDMStats") == "get_dm_stats"