From e6f757c12d121664aab4090c4193cd544326dffb Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 29 Jul 2026 15:02:12 +0530 Subject: [PATCH 1/3] objects: UTS test-spec corrections from cross-SDK audit Corrections and clarifications to the objects UTS unit test specs, surfaced while translating them to ably-java and cross-checked against ably-js and ably-cocoa source. - RTO17-RTO18 (realtime_object.md): remove the "re-attach after detach" sync-event scenario. It is redundant with "re-sync on new ATTACHED" (identical onAttached -> new-sync path) and not portably expressible - an unsolicited server DETACHED transitions the channel to SUSPENDED (not DETACHED) in some SDKs. ably-js already folds it into "re-sync on new ATTACHED". - RTO18d (realtime_object.md, objects-features.md): remove the duplicate-listener UTS test and add an editor's note. Whether re-registering the same listener fires once or twice is a platform-idiomatic design choice (verified in source: ably-js and ably-cocoa append -> twice; ably-java de-duplicates by listener instance -> once), rooted in the general EventEmitter contract (RTE4). Not universally testable; each SDK pins its own. - Op-path LiveObjectUpdate portability (internal_live_counter.md, internal_live_map.md): note that the update returned by applyOperation may be observed via the return value OR the emitted update event / resulting state, so event-based / typed SDKs can satisfy the same requirement. - Read-after-send quiescence (path_object.md): add a poll_until barrier before the positive reads in RTPO13c5 / RTPO14 (inbound ops apply asynchronously). - objects_pool.md: add a NOTE documenting the SDK-internal "DETACHED/FAILED clears objects data; SUSPENDED retains" behaviour and why it is deliberately not a UTS requirement (no normative point; unobservable via the public API), guarded by SDK-local tests. - internal_live_counter_api.md: assert the null-means-omitted increment contract directly where null is not distinguishable from an omitted argument. --- specifications/objects-features.md | 25 +++++++ uts/objects/unit/internal_live_counter.md | 9 +++ uts/objects/unit/internal_live_counter_api.md | 3 + uts/objects/unit/internal_live_map.md | 10 +++ uts/objects/unit/objects_pool.md | 18 +++++ uts/objects/unit/path_object.md | 10 +++ uts/objects/unit/realtime_object.md | 71 ++++++++----------- 7 files changed, 106 insertions(+), 40 deletions(-) diff --git a/specifications/objects-features.md b/specifications/objects-features.md index 194703f19..a3236fedc 100644 --- a/specifications/objects-features.md +++ b/specifications/objects-features.md @@ -292,6 +292,31 @@ Objects feature enables clients to store shared data as "objects" on a channel. - `(RTO18b2)` `SYNCED`: Indicates that the [RTO17](#RTO17) sync state has transitioned to `SYNCED` - `(RTO18c)` Registers the provided listener for the specified event - `(RTO18d)` If `on` is called more than once with the same listener and event, the listener is added multiple times to the listener registry. As such, if the same listener is registered twice and an event is emitted once, the listener will be invoked twice + > **EDITOR'S NOTE — SDKs diverge on this by design; it is not universally testable.** RTO18d + > restates the general `EventEmitter#on` contract **RTE4** (`features.md`, mirrored in + > `api-docstrings.md`). Registering the **same listener reference** twice for the same event is + > handled differently across Ably's own SDKs (verified in source): + > - **ably-js** (`eventemitter.ts`): `on` does a plain `listeners.push(listener)` with **no + > identity check** → **fires twice** (matches RTE4/RTO18d). + > - **ably-cocoa** (`ARTEventEmitter.m`): `on:` mints a *fresh* `ARTEventListener` wrapping the + > block on every call, and the registry's "AsSet" dedup is pointer-identity on that wrapper (the + > block itself is never the key), so two calls never collide → **fires twice**. + > - **ably-java** (`EventEmitter.java`): `on(event, Listener)` does `filters.put(listener, …)` into + > a map keyed by the listener *instance* (and the all-events `on(Listener)` is `contains`-guarded) + > → **de-duplicates → fires once**. ably-java's own Javadoc flags this as a deliberate, SDK-wide + > deviation from RTE4 — for a state-change listener it avoids the double-side-effect footgun the + > WHATWG DOM `addEventListener` also avoids by de-duplicating. + > + > This is a **design choice, not a language limitation**: the test passes the *same reference*, and + > reference identity is available in every one of these languages (JS `===`, ObjC pointer, Java + > identity) — js/cocoa simply choose to append, java chooses to dedupe. Because the SDKs genuinely + > diverge, the assertion cannot hold identically everywhere, so the corresponding UTS test + > (`RTO18d/duplicate-listener-0`) has been **removed** from the objects UTS suite. RTE4's "fires + > twice" is retained as the reference behaviour; SDKs that de-duplicate document it locally + > (ably-java tracks it in its `deviations.md`), and any SDK may pin its own behaviour in an + > SDK-local (non-UTS) test. + > Any move to make this *uniform* would be an ecosystem-wide RTE4 change (redefining `off`'s + > remove-one-vs-all semantics and closure identity) — out of scope here. - `(RTO18e)` When an event is emitted, all registered listeners for that event must be called with no arguments - `(RTO18f)` The client library may return a subscription object (or the idiomatic equivalent for the language) as a result of this operation: - `(RTO18f1)` The subscription object includes an `off` function diff --git a/uts/objects/unit/internal_live_counter.md b/uts/objects/unit/internal_live_counter.md index 73a36b46d..080db29b4 100644 --- a/uts/objects/unit/internal_live_counter.md +++ b/uts/objects/unit/internal_live_counter.md @@ -11,6 +11,15 @@ Tests the `InternalLiveCounter` CRDT data structure. InternalLiveCounter holds a Tests operate directly on InternalLiveCounter by calling `applyOperation()` and `replaceData()` with constructed messages. No channel or connection infrastructure is needed. +> **SDK portability note — op-path update return value.** Several tests below assert on the +> `LiveCounterUpdate` **returned by `applyOperation(msg, source)`** (`noop`, `update.amount`, +> `tombstone`, `objectMessage`). This models SDKs whose operation application returns the update +> synchronously. An SDK whose operation path returns only a success/failure boolean and instead +> delivers the update via the object's subscription/emit path (e.g. a statically-typed SDK — see +> `objects-features.md` RTTS) may satisfy the same requirement by asserting the **emitted update +> event** and/or the **resulting object state** — these are equivalent observables of the same +> `LiveCounterUpdate`. The sync path (`replaceData()`) returns the update in all SDKs. + ## Shared Helpers See `helpers/standard_test_pool.md` for `build_counter_inc`, `build_counter_create`, `build_object_delete`, `build_object_state`. diff --git a/uts/objects/unit/internal_live_counter_api.md b/uts/objects/unit/internal_live_counter_api.md index 3cf71d535..53f2dc7ac 100644 --- a/uts/objects/unit/internal_live_counter_api.md +++ b/uts/objects/unit/internal_live_counter_api.md @@ -237,6 +237,9 @@ The `null` row applies only where `null` is passable and distinguishable from an argument (e.g. a boxed `Integer` in Java). In SDKs whose signature makes null equivalent to "omitted" (e.g. `increment(amount?: number)` in JavaScript, where a nullish amount takes the default of 1), the row is not applicable — see the pseudocode conventions in `uts/README.md`. +Such SDKs should instead assert the equivalence directly — `increment(null)` succeeds and +increments by the default of 1 — pinning the null-means-omitted contract so a later signature +or coalescing change surfaces as a conscious decision. ### Setup ```pseudo diff --git a/uts/objects/unit/internal_live_map.md b/uts/objects/unit/internal_live_map.md index 348601398..08fcd07d8 100644 --- a/uts/objects/unit/internal_live_map.md +++ b/uts/objects/unit/internal_live_map.md @@ -11,6 +11,16 @@ Tests the `InternalLiveMap` LWW-map CRDT data structure. InternalLiveMap holds a Tests operate directly on InternalLiveMap by calling `applyOperation()` and `replaceData()` with constructed messages. +> **SDK portability note — op-path update return value.** Several tests below assert on the +> `LiveMapUpdate` **returned by `applyOperation(msg, source)`** (`noop`, the per-key `update` diff +> such as `{ "name": "updated"/"removed" }`, `tombstone`, `objectMessage`). This models SDKs whose +> operation application returns the update synchronously. An SDK whose operation path returns only a +> success/failure boolean and instead delivers the update via the object's subscription/emit path +> (e.g. a statically-typed SDK — see `objects-features.md` RTTS) may satisfy the same requirement by +> asserting the **emitted update event** and/or the **resulting map state** — these are equivalent +> observables of the same `LiveMapUpdate`. The sync path (`replaceData()`) returns the update in all +> SDKs. + ## Shared Helpers See `helpers/standard_test_pool.md` for builder functions. diff --git a/uts/objects/unit/objects_pool.md b/uts/objects/unit/objects_pool.md index ada2c79c9..d0a1d3662 100644 --- a/uts/objects/unit/objects_pool.md +++ b/uts/objects/unit/objects_pool.md @@ -119,6 +119,24 @@ ASSERT updates[0].objectMessage IS null --- +> **NOTE — channel DETACHED/FAILED clears objects data (SDK-internal; intentionally not a UTS test).** +> When the channel transitions to DETACHED or FAILED, the objects data is cleared **without emitting +> update events** (the current data is unknown in those states); a SUSPENDED channel **retains** its +> data. This is implemented in both reference SDKs (ably-js `RealtimeObject.actOnChannelState` → +> `objectsPool.clearObjectsData(false)` + clear SyncObjectsPool; ably-java +> `DefaultRealtimeObject.handleStateChange`), but it is **deliberately not a shared UTS requirement**: +> it has no normative `RTO` point, and it is not observable through the public API — access on a +> DETACHED channel throws (RTO25b), and on re-attach the RTO5c sync **replaces** the pool regardless of +> whether detach cleared it, so an SDK that skipped the clear would still be observably correct. It is a +> defensive internal cleanup, so each SDK guards it with its own **SDK-local** test rather than a UTS +> spec test. ably-java pins it in `DefaultRealtimeObjectChannelStateTest` (asserts the pool is cleared +> after DETACHED/FAILED and retained after SUSPENDED, driving `handleStateChange` and inspecting the +> pool directly); other SDKs may add an equivalent local test (e.g. ably-js in +> `test/realtime/liveobjects.test.js`). Referenced from the "no re-attach-after-detach scenario" note +> in `realtime_object.md`. + +--- + ## RTO5 - OBJECT_SYNC complete sequence **Test ID**: `objects/unit/RTO5/sync-complete-sequence-0` diff --git a/uts/objects/unit/path_object.md b/uts/objects/unit/path_object.md index 8a4000a4e..1a55aed8d 100644 --- a/uts/objects/unit/path_object.md +++ b/uts/objects/unit/path_object.md @@ -574,6 +574,11 @@ ASSERT result["profile"]["prefs"]["theme"] == "dark" mock_ws.send_to_client(build_object_message("test", [ build_map_set("map:prefs@1000", "back_ref", { objectId: "map:profile@1000" }, "99", "remote") ])) + +# Quiescence barrier for a POSITIVE read-after-send: SDKs may apply inbound OBJECT messages +# asynchronously, so wait until the MAP_SET has been applied before reading. (The spec's +# read-after-send conventions otherwise cover only negative assertions.) +poll_until("back_ref" IN root.get("profile").get("prefs").keys(), timeout: 5s) ``` ### Test Steps @@ -626,6 +631,11 @@ ASSERT root.get("score").compact() == 100 mock_ws.send_to_client(build_object_message("test", [ build_map_set("map:prefs@1000", "back_ref", { objectId: "map:profile@1000" }, "99", "remote") ])) + +# Quiescence barrier for a POSITIVE read-after-send: SDKs may apply inbound OBJECT messages +# asynchronously, so wait until the MAP_SET has been applied before reading. (The spec's +# read-after-send conventions otherwise cover only negative assertions.) +poll_until("back_ref" IN root.get("profile").get("prefs").keys(), timeout: 5s) ``` ### Test Steps diff --git a/uts/objects/unit/realtime_object.md b/uts/objects/unit/realtime_object.md index 411fd9126..5df537725 100644 --- a/uts/objects/unit/realtime_object.md +++ b/uts/objects/unit/realtime_object.md @@ -545,35 +545,25 @@ ASSERT events CONTAINS_IN_ORDER ["SYNCING", "SYNCED"] --- -## RTO18d - Duplicate listener registered twice fires twice - -**Test ID**: `objects/unit/RTO18d/duplicate-listener-0` - -**Spec requirement:** If same listener registered twice, it is invoked twice per event. - -### Setup -```pseudo -{ client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") -call_count = 0 -listener = () => { call_count++ } -channel.object.on(SYNCED, listener) -channel.object.on(SYNCED, listener) -``` - -### Test Steps -```pseudo -mock_ws.send_to_client(ProtocolMessage( - action: ATTACHED, channel: "test", channelSerial: "sync2:cursor", flags: HAS_OBJECTS -)) -mock_ws.send_to_client(build_object_sync_message("test", "sync2:", STANDARD_POOL_OBJECTS)) - -poll_until(call_count >= 2, timeout: 5s) -``` - -### Assertions -```pseudo -ASSERT call_count == 2 -``` +## RTO18d - Duplicate listener registration — REMOVED (platform-idiomatic, not universally testable) + +The former test `objects/unit/RTO18d/duplicate-listener-0` (registering the same listener twice → +invoked twice) has been **removed** from the UTS suite: the behaviour is contingent on each SDK's +listener-registration idiom, so it cannot pass identically across SDKs (the UTS contract). + +Registering the **same listener reference** twice for the same event diverges by design across SDKs +(verified in source): +- **ably-js:** `on` does a plain `listeners.push(listener)` with no identity check → fires **twice** + (matches RTE4 / RTO18d). +- **ably-cocoa:** `on:` mints a fresh `ARTEventListener` per call; the "AsSet" dedup is pointer-identity + on that wrapper (the block is never the key) → fires **twice**. +- **ably-java:** `on(event, Listener)` keys the registry by the listener *instance* → de-duplicates → + fires **once** (a deliberate, SDK-wide choice its `EventEmitter` documents as a spec deviation). + +This is a design choice, not a language limitation — the same reference is detectable in all three +languages; js/cocoa append, java dedupes. Because the SDKs genuinely diverge, the assertion can't hold +identically everywhere. See the RTO18d editor's note in `objects-features.md`; an SDK that wants to pin +its own behaviour should do so in an SDK-local (non-UTS) test. --- @@ -1538,17 +1528,18 @@ scenarios = [ }, expected_events: ["SYNCING", "SYNCED"] }, - { - name: "re-attach after detach", - trigger: () => { - mock_ws.send_to_client(ProtocolMessage(action: DETACHED, channel: "test")) - mock_ws.send_to_client(ProtocolMessage( - action: ATTACHED, channel: "test", channelSerial: "sync2:cursor", flags: HAS_OBJECTS - )) - mock_ws.send_to_client(build_object_sync_message("test", "sync2:", STANDARD_POOL_OBJECTS)) - }, - expected_events: ["SYNCING", "SYNCED"] - }, + // NOTE: there is intentionally NO "re-attach after detach" scenario here. At the objects layer, + // a detach emits no sync events (the detached/failed handler clears objects data WITHOUT + // emitting), and the re-attach drives the SAME onAttached -> new-sync path (RTO4c) as the + // "re-sync on new ATTACHED" scenario below — so it is redundant for a sync-EVENT test. It is also + // not portably expressible: in some SDKs an unsolicited server DETACHED transitions the channel to + // SUSPENDED (not DETACHED), so the fixture cannot be written identically everywhere. + // + // A SEPARATE, SDK-internal behaviour lives near here: on a channel DETACHED/FAILED the objects data + // is cleared (no events emitted), while SUSPENDED retains it. It is deliberately NOT a UTS test (no + // normative RTO point; not observable through the public API — access on DETACHED throws per RTO25b, + // and re-sync replaces the pool regardless), so each SDK guards it with an SDK-local test. See the + // NOTE in objects_pool.md. { name: "re-sync on new ATTACHED", trigger: () => { From 65e6dd5ec756ddfa13779f1c659dc5fc57e5274e Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 29 Jul 2026 18:29:58 +0530 Subject: [PATCH 2/3] =?UTF-8?q?objects:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20restore=20RTO18d=20test,=20add=20RTO27=20spec=20poi?= =?UTF-8?q?nt=20+=20UTS=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the PR review on the objects UTS cross-SDK corrections. - RTO18d (realtime_object.md, objects-features.md): restore the duplicate-listener UTS test (asserting the RTE4 "fires twice" contract) instead of removing it, and drop the editor's note from the spec. The UTS tests the spec's assertion; SDKs that de-duplicate listeners by instance (e.g. ably-java) may assert fires-once via an optional note on the test — the deviation stays with the SDK, not the UTS. - RTO27 (objects-features.md, NEW): add an explicit normative point for the objects-data lifecycle on channel state transitions — on DETACHED/FAILED clear every pooled object's data without emitting update events (and clear the SyncObjectsPool); on SUSPENDED retain it. Placed in the channel-lifecycle cluster (after RTO5) and cross-referenced from RTO20e1. Closes a previously-unspecified gap both reference SDKs already implement (ably-js RealtimeObject.actOnChannelState, ably-java DefaultRealtimeObject.handleStateChange). - RTO27 UTS test (realtime_object.md): add a white-box test that drives the internal channel-state handler directly and inspects the ObjectsPool (mirroring the internal_live_counter / internal_live_map tests), so the clear (not observable via the public API) and SUSPENDED (a connection-level state) are both testable at the unit tier. Reframe the objects_pool.md / realtime_object.md notes to reference RTO27. - path_object.md: reword the RTPO13c5 / RTPO14 read-after-send quiescence barrier to clarify it guards an INBOUND server OBJECT message (no apply-on-ack point to await), not a local publish. --- specifications/objects-features.md | 30 ++----- uts/objects/unit/objects_pool.md | 24 +++--- uts/objects/unit/path_object.md | 14 ++-- uts/objects/unit/realtime_object.md | 119 ++++++++++++++++++++++------ 4 files changed, 117 insertions(+), 70 deletions(-) diff --git a/specifications/objects-features.md b/specifications/objects-features.md index a3236fedc..9a078ef64 100644 --- a/specifications/objects-features.md +++ b/specifications/objects-features.md @@ -192,6 +192,11 @@ Objects feature enables clients to store shared data as "objects" on a channel. - `(RTO5c5)` The `bufferedObjectOperations` list must be cleared - `(RTO5c9)` The `appliedOnAckSerials` set ([RTO7b](#RTO7b)) must be cleared. A state sync causes the channel's LiveObjects data to be replaced, so after a state sync the `appliedOnAckSerials` no longer accurately describes which operations have been applied to the channel's LiveObjects data - `(RTO5c8)` The [RTO17](#RTO17) sync state must transition to `SYNCED` +- `(RTO27)` When the channel transitions to a state other than `ATTACHED`, the client library must manage the stored objects data as follows (for the effect of these transitions on an in-progress `publishAndApply`, see [RTO20e1](#RTO20e1)): + - `(RTO27a)` When the channel transitions to the `DETACHED` or `FAILED` state, the current state of the objects data can no longer be known, so the client library must: + - `(RTO27a1)` For every object in the internal `ObjectsPool`, clear its internal data, resetting it to the zero value for its type (an empty map, or a counter with value `0`), without emitting any `LiveObjectUpdate` events. The objects themselves remain in the `ObjectsPool`; only their data is cleared + - `(RTO27a2)` The `SyncObjectsPool` must be cleared + - `(RTO27b)` When the channel transitions to the `SUSPENDED` state, the client library must retain the stored objects data unchanged, since the connection may still recover and the retained data remains a valid best-effort local copy - `(RTO6)` Certain object operations may require creating a new object if one does not already exist in the internal `ObjectsPool` for the given `objectId`. This can be done as follows: - `(RTO6a)` If an object with `objectId` exists in `ObjectsPool`, do not create a new object - `(RTO6b)` The expected type of the object can be inferred from the provided `objectId`: @@ -292,31 +297,6 @@ Objects feature enables clients to store shared data as "objects" on a channel. - `(RTO18b2)` `SYNCED`: Indicates that the [RTO17](#RTO17) sync state has transitioned to `SYNCED` - `(RTO18c)` Registers the provided listener for the specified event - `(RTO18d)` If `on` is called more than once with the same listener and event, the listener is added multiple times to the listener registry. As such, if the same listener is registered twice and an event is emitted once, the listener will be invoked twice - > **EDITOR'S NOTE — SDKs diverge on this by design; it is not universally testable.** RTO18d - > restates the general `EventEmitter#on` contract **RTE4** (`features.md`, mirrored in - > `api-docstrings.md`). Registering the **same listener reference** twice for the same event is - > handled differently across Ably's own SDKs (verified in source): - > - **ably-js** (`eventemitter.ts`): `on` does a plain `listeners.push(listener)` with **no - > identity check** → **fires twice** (matches RTE4/RTO18d). - > - **ably-cocoa** (`ARTEventEmitter.m`): `on:` mints a *fresh* `ARTEventListener` wrapping the - > block on every call, and the registry's "AsSet" dedup is pointer-identity on that wrapper (the - > block itself is never the key), so two calls never collide → **fires twice**. - > - **ably-java** (`EventEmitter.java`): `on(event, Listener)` does `filters.put(listener, …)` into - > a map keyed by the listener *instance* (and the all-events `on(Listener)` is `contains`-guarded) - > → **de-duplicates → fires once**. ably-java's own Javadoc flags this as a deliberate, SDK-wide - > deviation from RTE4 — for a state-change listener it avoids the double-side-effect footgun the - > WHATWG DOM `addEventListener` also avoids by de-duplicating. - > - > This is a **design choice, not a language limitation**: the test passes the *same reference*, and - > reference identity is available in every one of these languages (JS `===`, ObjC pointer, Java - > identity) — js/cocoa simply choose to append, java chooses to dedupe. Because the SDKs genuinely - > diverge, the assertion cannot hold identically everywhere, so the corresponding UTS test - > (`RTO18d/duplicate-listener-0`) has been **removed** from the objects UTS suite. RTE4's "fires - > twice" is retained as the reference behaviour; SDKs that de-duplicate document it locally - > (ably-java tracks it in its `deviations.md`), and any SDK may pin its own behaviour in an - > SDK-local (non-UTS) test. - > Any move to make this *uniform* would be an ecosystem-wide RTE4 change (redefining `off`'s - > remove-one-vs-all semantics and closure identity) — out of scope here. - `(RTO18e)` When an event is emitted, all registered listeners for that event must be called with no arguments - `(RTO18f)` The client library may return a subscription object (or the idiomatic equivalent for the language) as a result of this operation: - `(RTO18f1)` The subscription object includes an `off` function diff --git a/uts/objects/unit/objects_pool.md b/uts/objects/unit/objects_pool.md index d0a1d3662..1f77c4412 100644 --- a/uts/objects/unit/objects_pool.md +++ b/uts/objects/unit/objects_pool.md @@ -119,21 +119,15 @@ ASSERT updates[0].objectMessage IS null --- -> **NOTE — channel DETACHED/FAILED clears objects data (SDK-internal; intentionally not a UTS test).** -> When the channel transitions to DETACHED or FAILED, the objects data is cleared **without emitting -> update events** (the current data is unknown in those states); a SUSPENDED channel **retains** its -> data. This is implemented in both reference SDKs (ably-js `RealtimeObject.actOnChannelState` → -> `objectsPool.clearObjectsData(false)` + clear SyncObjectsPool; ably-java -> `DefaultRealtimeObject.handleStateChange`), but it is **deliberately not a shared UTS requirement**: -> it has no normative `RTO` point, and it is not observable through the public API — access on a -> DETACHED channel throws (RTO25b), and on re-attach the RTO5c sync **replaces** the pool regardless of -> whether detach cleared it, so an SDK that skipped the clear would still be observably correct. It is a -> defensive internal cleanup, so each SDK guards it with its own **SDK-local** test rather than a UTS -> spec test. ably-java pins it in `DefaultRealtimeObjectChannelStateTest` (asserts the pool is cleared -> after DETACHED/FAILED and retained after SUSPENDED, driving `handleStateChange` and inspecting the -> pool directly); other SDKs may add an equivalent local test (e.g. ably-js in -> `test/realtime/liveobjects.test.js`). Referenced from the "no re-attach-after-detach scenario" note -> in `realtime_object.md`. +> **NOTE — channel DETACHED/FAILED clears objects data (normative: [RTO27](../../../specifications/objects-features.md#RTO27)).** +> On a channel transition to DETACHED or FAILED the objects data is cleared **without emitting update +> events** (RTO27a); a SUSPENDED channel **retains** its data (RTO27b) (ably-js +> `RealtimeObject.actOnChannelState` → `objectsPool.clearObjectsData(false)`; ably-java +> `DefaultRealtimeObject.handleStateChange`). This is exercised **white-box** by the **RTO27** test in +> `realtime_object.md`: it drives the internal channel-state handler directly and inspects the internal +> `ObjectsPool`, because the behaviour is not reachable black-box (access after DETACHED throws per +> RTO25b, and SUSPENDED is a connection-level state a channel-level mock cannot drive). SDKs may +> additionally pin it in a local test (ably-java: `DefaultRealtimeObjectChannelStateTest`). --- diff --git a/uts/objects/unit/path_object.md b/uts/objects/unit/path_object.md index 1a55aed8d..b7c46e003 100644 --- a/uts/objects/unit/path_object.md +++ b/uts/objects/unit/path_object.md @@ -575,9 +575,10 @@ mock_ws.send_to_client(build_object_message("test", [ build_map_set("map:prefs@1000", "back_ref", { objectId: "map:profile@1000" }, "99", "remote") ])) -# Quiescence barrier for a POSITIVE read-after-send: SDKs may apply inbound OBJECT messages -# asynchronously, so wait until the MAP_SET has been applied before reading. (The spec's -# read-after-send conventions otherwise cover only negative assertions.) +# Quiescence barrier: this is an INBOUND server OBJECT message (send_to_client), not a locally +# published operation — so there is no publish-completion / apply-on-ack point to await here. SDKs +# may apply inbound OBJECT messages asynchronously, so wait until the MAP_SET has been applied before +# this POSITIVE read. (The spec's read-after-send conventions otherwise cover only negative assertions.) poll_until("back_ref" IN root.get("profile").get("prefs").keys(), timeout: 5s) ``` @@ -632,9 +633,10 @@ mock_ws.send_to_client(build_object_message("test", [ build_map_set("map:prefs@1000", "back_ref", { objectId: "map:profile@1000" }, "99", "remote") ])) -# Quiescence barrier for a POSITIVE read-after-send: SDKs may apply inbound OBJECT messages -# asynchronously, so wait until the MAP_SET has been applied before reading. (The spec's -# read-after-send conventions otherwise cover only negative assertions.) +# Quiescence barrier: this is an INBOUND server OBJECT message (send_to_client), not a locally +# published operation — so there is no publish-completion / apply-on-ack point to await here. SDKs +# may apply inbound OBJECT messages asynchronously, so wait until the MAP_SET has been applied before +# this POSITIVE read. (The spec's read-after-send conventions otherwise cover only negative assertions.) poll_until("back_ref" IN root.get("profile").get("prefs").keys(), timeout: 5s) ``` diff --git a/uts/objects/unit/realtime_object.md b/uts/objects/unit/realtime_object.md index 5df537725..20382f948 100644 --- a/uts/objects/unit/realtime_object.md +++ b/uts/objects/unit/realtime_object.md @@ -545,25 +545,42 @@ ASSERT events CONTAINS_IN_ORDER ["SYNCING", "SYNCED"] --- -## RTO18d - Duplicate listener registration — REMOVED (platform-idiomatic, not universally testable) - -The former test `objects/unit/RTO18d/duplicate-listener-0` (registering the same listener twice → -invoked twice) has been **removed** from the UTS suite: the behaviour is contingent on each SDK's -listener-registration idiom, so it cannot pass identically across SDKs (the UTS contract). - -Registering the **same listener reference** twice for the same event diverges by design across SDKs -(verified in source): -- **ably-js:** `on` does a plain `listeners.push(listener)` with no identity check → fires **twice** - (matches RTE4 / RTO18d). -- **ably-cocoa:** `on:` mints a fresh `ARTEventListener` per call; the "AsSet" dedup is pointer-identity - on that wrapper (the block is never the key) → fires **twice**. -- **ably-java:** `on(event, Listener)` keys the registry by the listener *instance* → de-duplicates → - fires **once** (a deliberate, SDK-wide choice its `EventEmitter` documents as a spec deviation). - -This is a design choice, not a language limitation — the same reference is detectable in all three -languages; js/cocoa append, java dedupes. Because the SDKs genuinely diverge, the assertion can't hold -identically everywhere. See the RTO18d editor's note in `objects-features.md`; an SDK that wants to pin -its own behaviour should do so in an SDK-local (non-UTS) test. +## RTO18d - Duplicate listener registered twice fires twice + +**Test ID**: `objects/unit/RTO18d/duplicate-listener-0` + +**Spec requirement:** If same listener registered twice, it is invoked twice per event. + +> **OPTIONAL for SDKs that de-duplicate listeners by instance.** RTO18d restates the general +> `EventEmitter#on` contract (RTE4): registering the same listener reference twice adds it twice, so it +> fires twice. Some SDKs deliberately de-duplicate listeners keyed by instance (e.g. ably-java's +> `EventEmitter` documents this as an SDK-wide deviation from RTE4). Such an SDK may instead assert +> `call_count == 1` here — or skip this test — and pin its own behaviour; the assertion below is the +> RTE4 reference behaviour. + +### Setup +```pseudo +{ client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") +call_count = 0 +listener = () => { call_count++ } +channel.object.on(SYNCED, listener) +channel.object.on(SYNCED, listener) +``` + +### Test Steps +```pseudo +mock_ws.send_to_client(ProtocolMessage( + action: ATTACHED, channel: "test", channelSerial: "sync2:cursor", flags: HAS_OBJECTS +)) +mock_ws.send_to_client(build_object_sync_message("test", "sync2:", STANDARD_POOL_OBJECTS)) + +poll_until(call_count >= 2, timeout: 5s) +``` + +### Assertions +```pseudo +ASSERT call_count == 2 +``` --- @@ -1535,11 +1552,10 @@ scenarios = [ // not portably expressible: in some SDKs an unsolicited server DETACHED transitions the channel to // SUSPENDED (not DETACHED), so the fixture cannot be written identically everywhere. // - // A SEPARATE, SDK-internal behaviour lives near here: on a channel DETACHED/FAILED the objects data - // is cleared (no events emitted), while SUSPENDED retains it. It is deliberately NOT a UTS test (no - // normative RTO point; not observable through the public API — access on DETACHED throws per RTO25b, - // and re-sync replaces the pool regardless), so each SDK guards it with an SDK-local test. See the - // NOTE in objects_pool.md. + // A SEPARATE behaviour lives near here: on a channel DETACHED/FAILED the objects data is cleared + // (no events emitted), while SUSPENDED retains it. This is normative (RTO27) and is covered by the + // RTO27 white-box test at the end of this file (it drives the internal channel-state handler directly + // and inspects the pool, since the behaviour is not reachable black-box). See objects_pool.md. { name: "re-sync on new ATTACHED", trigger: () => { @@ -1580,3 +1596,58 @@ FOR scenario IN scenarios: ASSERT events == scenario.expected_events ``` + +--- + +## RTO27 - Objects data lifecycle on channel state transitions + +**Test ID**: `objects/unit/RTO27/channel-state-data-lifecycle-0` + +**Spec requirement:** On a channel transition to `DETACHED`/`FAILED`, every object's data is cleared +without emitting update events (RTO27a); on `SUSPENDED`, the data is retained (RTO27b). + +**White-box test.** Like the `internal_live_counter` / `internal_live_map` tests (which call +`applyOperation` directly), this drives the RealtimeObject's internal channel-state handler directly +and inspects the internal `ObjectsPool`. A direct call is required because the behaviour is not +reachable black-box: access after `DETACHED`/`FAILED` throws (RTO25b), and `SUSPENDED` is a +connection-level state that a channel-level mock cannot drive. The abstract symbols map as follows: +- `channel.object.handle_channel_state(state)` → the SDK's channel-state handler (ably-js + `RealtimeObject.actOnChannelState(state)`, ably-java `DefaultRealtimeObject.handleStateChange(state, false)`). +- `channel.object.objects_pool` → the internal `ObjectsPool` (ably-js `_objectsPool`, ably-java `objectsPool`). + +### RTO27a - DETACHED / FAILED clears data without emitting update events + +```pseudo +FOR state IN [DETACHED, FAILED]: + { client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") + pool = channel.object.objects_pool # internal ObjectsPool (RTO3) + + # sanity: STANDARD_POOL_OBJECTS has been materialised + ASSERT "name" IN pool["root"].data + ASSERT pool["counter:score@1000"].value == 100 + + # RTO27a1: no update events must be emitted during the clear + updates = [] + pool["root"].subscribe((update) => updates.append(update)) + + channel.object.handle_channel_state(state) # internal channel-state handler (RTO27) + + # RTO27a1: every object's data is cleared; the objects themselves remain in the pool + ASSERT pool["root"].data == {} + ASSERT "counter:score@1000" IN pool + ASSERT pool["counter:score@1000"].value == 0 + ASSERT updates.length == 0 +``` + +### RTO27b - SUSPENDED retains data + +```pseudo +{ client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") +pool = channel.object.objects_pool + +channel.object.handle_channel_state(SUSPENDED) # RTO27b: no-op for objects data + +# RTO27b: data retained unchanged +ASSERT "name" IN pool["root"].data +ASSERT pool["counter:score@1000"].value == 100 +``` From 85b47680bf42a378ba84dbb3ff34fa0b5b4f2bbd Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 29 Jul 2026 20:44:35 +0530 Subject: [PATCH 3/3] objects/uts: drop redundant detach-clear note, document identifier-naming conventions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the RTO27 work and the PR review. - objects_pool.md: remove the DETACHED/FAILED detach-clear NOTE. It was a stopgap for a missing spec point; now that RTO27 is normative and has its own white-box test in realtime_object.md, the note is redundant (and was a RealtimeObject concern, not a bare ObjectsPool one). - realtime_object.md: rename the RTO27 test's abstract white-box symbols to the UTS camelCase convention (objects_pool -> objectsPool, handle_channel_state -> processChannelState); drop the dangling "See objects_pool.md" pointer and the redundant RTO27 cross-reference from the no-re-attach-after-detach note, trimming that note to its two load-bearing reasons (redundant + not portably expressible); extend the header spec points to RTO22-RTO27 to match the file's contents. - docs/writing-test-specs.md, README.md: add an "Identifier Naming" convention section (spec surface mirrors the specification's names — PascalCase types, camelCase fields/methods; behaviours the spec describes but doesn't name get a camelCase coinage; snake_case is reserved for test-harness constructs; wire/enum values keep protocol/spec casing). Validated against objects/realtime/rest UTS specs and the features.md/protocol.md parent specs. --- uts/README.md | 8 ++++ uts/docs/writing-test-specs.md | 67 +++++++++++++++++++++++++++++ uts/objects/unit/objects_pool.md | 12 ------ uts/objects/unit/realtime_object.md | 28 +++++------- 4 files changed, 85 insertions(+), 30 deletions(-) diff --git a/uts/README.md b/uts/README.md index 4b8d6bc8c..8cc4d9b9d 100644 --- a/uts/README.md +++ b/uts/README.md @@ -87,6 +87,14 @@ ASSERT error.code == 40160 AWAIT_STATE client.connection.state == ConnectionState.connected ``` +**Identifier naming.** Identifiers that name SDK/spec surface mirror the specification's own names and +casing — `PascalCase` types (`ObjectsPool`), `camelCase` fields and methods (`bufferedObjectOperations`, +`applyOperation`) — so they grep straight back to the spec. Behaviours the spec describes but does not +name get a `camelCase` coinage in the same style (`processAttached`, `processChannelState`), never +`snake_case`. `snake_case` is reserved for test-harness constructs (`setup_synced_channel`, `mock_ws`, +`send_to_client`, `build_*`, `poll_until`). Spec *file* names are `snake_case` (`objects_pool.md`) — +unrelated to symbol casing. Full rules: [docs/writing-test-specs.md](docs/writing-test-specs.md#identifier-naming). + Pseudocode maps to language idioms rather than prescribing exact syntax: - **Absent values**: `== null` means the language-appropriate "absent" value — `undefined` in diff --git a/uts/docs/writing-test-specs.md b/uts/docs/writing-test-specs.md index e60f6fd67..aa403e6b2 100644 --- a/uts/docs/writing-test-specs.md +++ b/uts/docs/writing-test-specs.md @@ -508,6 +508,73 @@ Tests that all REST requests include the `Ably-Agent` header with correct format ## Pseudocode Conventions +### Identifier Naming + +Every identifier in pseudocode is either **spec surface** (an SDK/spec name, cases 1–2 below) or +**test harness** (scaffolding, case 3), and its **casing is the visual signal** of which. Getting this +right keeps the spec surface greppable straight back to the specification and keeps test scaffolding +visually distinct from it. + +**1. Spec surface — mirror the specification's own names exactly.** Anything that names an SDK type, +field, property, or method that a feature specification (`features.md`, `objects-features.md`, …) +already defines must use the **identical name and casing** the spec uses: + +- **Types / concepts → `PascalCase`**: `ObjectsPool`, `SyncObjectsPool`, `InternalLiveMap`, + `InternalLiveCounter`, `Connection`. +- **Fields / properties → `camelCase`**: `bufferedObjectOperations`, `appliedOnAckSerials`, + `clearTimeserial`, `siteTimeserials`, `createOperationIsMerged`. +- **Methods → the spec's member, written as a call**: the spec's `Type#method` form (e.g. + `InternalLiveMap#set`, `RealtimeObject#on`) is written `object.method(...)`; bare method names + (`applyOperation`, `replaceData`) match the spec verbatim. + +Do not rename, abbreviate, or re-case a spec identifier — a reader must be able to grep it out of the +specification unchanged. + +**2. Behaviour the specification describes but does not name.** Some clauses describe an action without +giving it a method name (e.g. RTO4: *"when a channel `ATTACHED` `ProtocolMessage` is received, the +client library must …"* — no method is named). When a white-box test needs to invoke that behaviour +directly, coin a name **in the specification's own style — `camelCase`, never `snake_case`** — and use +it consistently across the suite. Established coinages: `processAttached`, `processObjectSync`, +`processObjectMessage`, `syncState`, `processChannelState`. These are UTS conventions rather than spec +names, but they read as though the spec could have named them. + +**3. Test-harness constructs → `snake_case`.** Everything that is scaffolding rather than SDK/spec +surface — fixtures, mocks, builders, polling/flush helpers — is `snake_case`, so it is instantly +distinguishable from the spec surface: `setup_synced_channel`, `mock_ws`, `send_to_client`, +`build_object_message`, `build_counter_inc`, `install_mock`, `respond_with_success`, `poll_until`, +`process_pending_events`, `encode_uri_component`. + +**Internal (white-box) access** uses the spec's internal name, `camelCase`d: the internal `ObjectsPool` +(RTO3) is reached as `object.objectsPool`; internal fields keep their spec names +(`bufferedObjectOperations`, `appliedOnAckSerials`). How each SDK exposes these is covered in the +derived-test mapping docs. + +**Wire and enum values follow the protocol/spec — not these two buckets.** Identifiers that name +wire-format data or enum members come from `protocol.md` / `features.md` and are matched verbatim; they +are **not** harness, even when `snake_case`: + +- **Wire fields & query parameters** — mostly `camelCase` (`channelSerial`, `connectionId`, + `msgSerial`), with a few `snake_case` (the `request_id` query parameter, RSC7c). +- **Enum members** — the spec's enum casing, typically `SCREAMING_SNAKE_CASE` + (the `ChannelMode.PRESENCE_SUBSCRIBE` enum member / `TR3` flag, also written `presence_subscribe` in + some prose; `SYNCED`; `DETACHED`). Render these per the *Enum values* note in the UTS README. + +**File names are not identifiers.** Spec *files* are `snake_case` `.md` (`objects_pool.md`, +`realtime_object.md`); this is unrelated to symbol casing. The `ObjectsPool` type stays `PascalCase` +even though it is documented in `objects_pool.md`. + +```pseudo +# GOOD — spec surface mirrors the spec; harness is snake_case +{ client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") # harness → snake_case +pool = channel.object.objectsPool # ObjectsPool (RTO3) → camelCase accessor +update = counter.applyOperation(msg, source: CHANNEL) # spec method, verbatim +ASSERT counter.bufferedObjectOperations.length == 0 # spec field, verbatim + +# BAD — spec surface in snake_case: looks like harness, no longer greps back to the spec +pool = channel.object.objects_pool +update = counter.apply_operation(msg) +``` + ### URI Path Component Encoding Use `encode_uri_component()` for any variable path segment or query parameter in URL assertions. This is defined in the UTS README. Always use exact equality (`==`) for path assertions, not `CONTAINS`. diff --git a/uts/objects/unit/objects_pool.md b/uts/objects/unit/objects_pool.md index 1f77c4412..ada2c79c9 100644 --- a/uts/objects/unit/objects_pool.md +++ b/uts/objects/unit/objects_pool.md @@ -119,18 +119,6 @@ ASSERT updates[0].objectMessage IS null --- -> **NOTE — channel DETACHED/FAILED clears objects data (normative: [RTO27](../../../specifications/objects-features.md#RTO27)).** -> On a channel transition to DETACHED or FAILED the objects data is cleared **without emitting update -> events** (RTO27a); a SUSPENDED channel **retains** its data (RTO27b) (ably-js -> `RealtimeObject.actOnChannelState` → `objectsPool.clearObjectsData(false)`; ably-java -> `DefaultRealtimeObject.handleStateChange`). This is exercised **white-box** by the **RTO27** test in -> `realtime_object.md`: it drives the internal channel-state handler directly and inspects the internal -> `ObjectsPool`, because the behaviour is not reachable black-box (access after DETACHED throws per -> RTO25b, and SUSPENDED is a connection-level state a channel-level mock cannot drive). SDKs may -> additionally pin it in a local test (ably-java: `DefaultRealtimeObjectChannelStateTest`). - ---- - ## RTO5 - OBJECT_SYNC complete sequence **Test ID**: `objects/unit/RTO5/sync-complete-sequence-0` diff --git a/uts/objects/unit/realtime_object.md b/uts/objects/unit/realtime_object.md index 20382f948..60ffaef8c 100644 --- a/uts/objects/unit/realtime_object.md +++ b/uts/objects/unit/realtime_object.md @@ -1,6 +1,6 @@ # RealtimeObject Tests -Spec points: `RTO2`, `RTO10`, `RTO15`, `RTO17`–`RTO20`, `RTO22`–`RTO26` +Spec points: `RTO2`, `RTO10`, `RTO15`, `RTO17`–`RTO20`, `RTO22`–`RTO27` ## Test Type Unit test with mocked WebSocket client @@ -1545,17 +1545,9 @@ scenarios = [ }, expected_events: ["SYNCING", "SYNCED"] }, - // NOTE: there is intentionally NO "re-attach after detach" scenario here. At the objects layer, - // a detach emits no sync events (the detached/failed handler clears objects data WITHOUT - // emitting), and the re-attach drives the SAME onAttached -> new-sync path (RTO4c) as the - // "re-sync on new ATTACHED" scenario below — so it is redundant for a sync-EVENT test. It is also - // not portably expressible: in some SDKs an unsolicited server DETACHED transitions the channel to - // SUSPENDED (not DETACHED), so the fixture cannot be written identically everywhere. - // - // A SEPARATE behaviour lives near here: on a channel DETACHED/FAILED the objects data is cleared - // (no events emitted), while SUSPENDED retains it. This is normative (RTO27) and is covered by the - // RTO27 white-box test at the end of this file (it drives the internal channel-state handler directly - // and inspects the pool, since the behaviour is not reachable black-box). See objects_pool.md. + // No "re-attach after detach" scenario, deliberately: it is redundant with "re-sync on new + // ATTACHED" below (same onAttached -> new-sync path, RTO4c), and not portably expressible — an + // unsolicited server DETACHED transitions to SUSPENDED (not DETACHED) in some SDKs. { name: "re-sync on new ATTACHED", trigger: () => { @@ -1611,16 +1603,16 @@ without emitting update events (RTO27a); on `SUSPENDED`, the data is retained (R and inspects the internal `ObjectsPool`. A direct call is required because the behaviour is not reachable black-box: access after `DETACHED`/`FAILED` throws (RTO25b), and `SUSPENDED` is a connection-level state that a channel-level mock cannot drive. The abstract symbols map as follows: -- `channel.object.handle_channel_state(state)` → the SDK's channel-state handler (ably-js +- `channel.object.processChannelState(state)` → the SDK's channel-state handler (ably-js `RealtimeObject.actOnChannelState(state)`, ably-java `DefaultRealtimeObject.handleStateChange(state, false)`). -- `channel.object.objects_pool` → the internal `ObjectsPool` (ably-js `_objectsPool`, ably-java `objectsPool`). +- `channel.object.objectsPool` → the internal `ObjectsPool` (RTO3) (ably-js `_objectsPool`, ably-java `objectsPool`). ### RTO27a - DETACHED / FAILED clears data without emitting update events ```pseudo FOR state IN [DETACHED, FAILED]: { client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") - pool = channel.object.objects_pool # internal ObjectsPool (RTO3) + pool = channel.object.objectsPool # internal ObjectsPool (RTO3) # sanity: STANDARD_POOL_OBJECTS has been materialised ASSERT "name" IN pool["root"].data @@ -1630,7 +1622,7 @@ FOR state IN [DETACHED, FAILED]: updates = [] pool["root"].subscribe((update) => updates.append(update)) - channel.object.handle_channel_state(state) # internal channel-state handler (RTO27) + channel.object.processChannelState(state) # internal channel-state handler (RTO27) # RTO27a1: every object's data is cleared; the objects themselves remain in the pool ASSERT pool["root"].data == {} @@ -1643,9 +1635,9 @@ FOR state IN [DETACHED, FAILED]: ```pseudo { client, channel, root, mock_ws } = AWAIT setup_synced_channel("test") -pool = channel.object.objects_pool +pool = channel.object.objectsPool -channel.object.handle_channel_state(SUSPENDED) # RTO27b: no-op for objects data +channel.object.processChannelState(SUSPENDED) # RTO27b: no-op for objects data # RTO27b: data retained unchanged ASSERT "name" IN pool["root"].data