diff --git a/.claude/skills/uts-to-kotlin/SKILL.md b/.claude/skills/uts-to-kotlin/SKILL.md index 59b142f46..7c991e5ed 100644 --- a/.claude/skills/uts-to-kotlin/SKILL.md +++ b/.claude/skills/uts-to-kotlin/SKILL.md @@ -1,13 +1,13 @@ --- -description: "Translate the UTS pseudocode test specs in a whole module directory into runnable Kotlin tests in the ably-java uts module. Takes a UTS module directory (e.g. .../specification/uts/objects), validates its structure, resolves the target ably-java module, lets you pick a tier (unit/integration/proxy) and which specs, then derives a Kotlin test per spec. Usage: /uts-to-kotlin " +description: "Translate the UTS pseudocode test specs in a whole module directory into runnable Kotlin tests in the ably-java uts module. Takes a UTS module directory (e.g. /uts/objects), validates its structure, resolves the target ably-java module, lets you pick a tier (unit/integration/proxy) and which specs, then derives a Kotlin test per spec. Usage: /uts-to-kotlin " allowed-tools: Bash, Read, Edit, Write, WebFetch --- Translate the UTS pseudocode test specs under the **module directory** `$ARGUMENTS` into runnable Kotlin tests in the ably-java `uts` module. -`$ARGUMENTS` is a UTS *module* directory — a directory sitting directly under `.../specification/uts/`, -e.g. `/Users/sachinsh/ably-specification/specification/uts/objects`. Its name (`objects`, `realtime`, +`$ARGUMENTS` is a UTS *module* directory — a directory sitting directly under the spec repo's `uts/`, +e.g. `/uts/objects`. Its name (`objects`, `realtime`, `rest`, …) is the **source module**. A module directory holds many spec files, organised into tiers (`unit/`, `integration/`, and `integration/proxy/`). @@ -31,7 +31,9 @@ bundled script does them — that keeps selection byte-for-byte deterministic in to re-eyeball regexes, join paths, and hand-convert `snake_case` → `PascalCase` each run. > **If `$ARGUMENTS` is empty or blank**, stop and show: `Usage: /uts-to-kotlin ` -> — with the example `/uts-to-kotlin /Users/sachinsh/ably-specification/specification/uts/objects`. +> — with the example `/uts-to-kotlin /uts/objects` +> (the path to a module directory inside a local clone of +> [`ably/specification`](https://github.com/ably/specification)). ## Step A — Resolve the module diff --git a/lib/src/main/java/io/ably/lib/liveobjects/message/CounterCreate.java b/lib/src/main/java/io/ably/lib/liveobjects/message/CounterCreate.java index ca05e0e28..ffcdc73d0 100644 --- a/lib/src/main/java/io/ably/lib/liveobjects/message/CounterCreate.java +++ b/lib/src/main/java/io/ably/lib/liveobjects/message/CounterCreate.java @@ -1,6 +1,6 @@ package io.ably.lib.liveobjects.message; -import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * Payload of a {@link ObjectOperationAction#COUNTER_CREATE} operation, describing the @@ -15,7 +15,8 @@ public interface CounterCreate { * *

Spec: CCR2a * - * @return the initial counter value + * @return the initial counter value, or {@code null} if absent from the operation + * (such an operation marks the create as merged without changing the value, per RTLC16d) */ - @NotNull Double getCount(); + @Nullable Double getCount(); } diff --git a/lib/src/main/java/io/ably/lib/liveobjects/message/CounterInc.java b/lib/src/main/java/io/ably/lib/liveobjects/message/CounterInc.java index 88fe59174..8b8aafc70 100644 --- a/lib/src/main/java/io/ably/lib/liveobjects/message/CounterInc.java +++ b/lib/src/main/java/io/ably/lib/liveobjects/message/CounterInc.java @@ -1,6 +1,6 @@ package io.ably.lib.liveobjects.message; -import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * Payload of a {@link ObjectOperationAction#COUNTER_INC} operation, describing an amount @@ -16,7 +16,8 @@ public interface CounterInc { * *

Spec: CIN2a * - * @return the increment amount (may be negative) + * @return the increment amount (may be negative), or {@code null} if absent from the + * operation (such an operation is applied as a no-op, per RTLC9h) */ - @NotNull Double getNumber(); + @Nullable Double getNumber(); } diff --git a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/message/DefaultObjectMessage.kt b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/message/DefaultObjectMessage.kt index d206b37fe..b085e8a7e 100644 --- a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/message/DefaultObjectMessage.kt +++ b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/message/DefaultObjectMessage.kt @@ -92,12 +92,12 @@ internal class DefaultMapRemove(private val mapRemove: WireMapRemove) : MapRemov /** Spec: CCR2 */ internal class DefaultCounterCreate(private val counterCreate: WireCounterCreate) : CounterCreate { - override fun getCount(): Double = counterCreate.count + override fun getCount(): Double? = counterCreate.count } /** Spec: CIN2 */ internal class DefaultCounterInc(private val counterInc: WireCounterInc) : CounterInc { - override fun getNumber(): Double = counterInc.number + override fun getNumber(): Double? = counterInc.number } /** Spec: ODE2 - no attributes */ diff --git a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/message/WireObjectMessage.kt b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/message/WireObjectMessage.kt index f7abaf431..ea57d482d 100644 --- a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/message/WireObjectMessage.kt +++ b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/message/WireObjectMessage.kt @@ -70,12 +70,12 @@ internal data class WireMapRemove( /** Spec: CCR2 */ internal data class WireCounterCreate( - val count: Double, // CCR2a + val count: Double?, // CCR2a - may be absent on the wire (RTLC16d) ) /** Spec: CIN2 */ internal data class WireCounterInc( - val number: Double, // CIN2a + val number: Double?, // CIN2a - may be absent on the wire (RTLC9h) ) /** Spec: ODE2 - no attributes */ diff --git a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/serialization/MsgpackSerialization.kt b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/serialization/MsgpackSerialization.kt index 18d5c0701..85e4a6270 100644 --- a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/serialization/MsgpackSerialization.kt +++ b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/serialization/MsgpackSerialization.kt @@ -521,9 +521,11 @@ private fun readMapRemove(unpacker: MessageUnpacker): WireMapRemove { * Write WireCounterCreate to MessagePacker */ private fun WireCounterCreate.writeMsgpack(packer: MessagePacker) { - packer.packMapHeader(1) - packer.packString("count") - packer.packDouble(count) + packer.packMapHeader(if (count != null) 1 else 0) + if (count != null) { + packer.packString("count") + packer.packDouble(count) + } } /** @@ -542,16 +544,19 @@ private fun readCounterCreate(unpacker: MessageUnpacker): WireCounterCreate { else -> unpacker.skipValue() } } - return WireCounterCreate(count = count ?: throw objectStateError("Missing 'count' in WireCounterCreate payload")) + // count may legitimately be absent (RTLC16d) - the apply path treats it as a noop + return WireCounterCreate(count = count) } /** * Write WireCounterInc to MessagePacker */ private fun WireCounterInc.writeMsgpack(packer: MessagePacker) { - packer.packMapHeader(1) - packer.packString("number") - packer.packDouble(number) + packer.packMapHeader(if (number != null) 1 else 0) + if (number != null) { + packer.packString("number") + packer.packDouble(number) + } } /** @@ -570,7 +575,8 @@ private fun readCounterInc(unpacker: MessageUnpacker): WireCounterInc { else -> unpacker.skipValue() } } - return WireCounterInc(number = number ?: throw objectStateError("Missing 'number' in WireCounterInc payload")) + // number may legitimately be absent (RTLC9h) - the apply path treats it as a noop + return WireCounterInc(number = number) } /** diff --git a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/BaseRealtimeLiveObject.kt b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/BaseRealtimeLiveObject.kt index 970198208..04883f574 100644 --- a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/BaseRealtimeLiveObject.kt +++ b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/BaseRealtimeLiveObject.kt @@ -138,11 +138,13 @@ internal abstract class BaseRealtimeObject( if (!canApplyOperation(msgSiteCode, msgTimeSerial)) { // RTLC7b, RTLM15b - Log.v( - tag, - "Skipping ${wireObjectOperation.action} op: op serial $msgTimeSerial <= site serial ${siteTimeserials[msgSiteCode]}; " + - "objectId=$objectId" - ) + if (!msgTimeSerial.isNullOrEmpty() && !msgSiteCode.isNullOrEmpty()) { + Log.v( + tag, + "Skipping ${wireObjectOperation.action} op: op serial $msgTimeSerial <= site serial ${siteTimeserials[msgSiteCode]}; " + + "objectId=$objectId" + ) + } return false // RTLC7b / RTLM15b } // RTLC7c / RTLM15c - only update siteTimeserials for CHANNEL source @@ -163,11 +165,11 @@ internal abstract class BaseRealtimeObject( * @spec RTLO4a - Serial comparison logic for LiveMap/LiveCounter operations */ internal fun canApplyOperation(siteCode: String?, timeSerial: String?): Boolean { - if (timeSerial.isNullOrEmpty()) { - throw objectError("Invalid serial: $timeSerial") // RTLO4a3 - } - if (siteCode.isNullOrEmpty()) { - throw objectError("Invalid site code: $siteCode") // RTLO4a3 + if (timeSerial.isNullOrEmpty() || siteCode.isNullOrEmpty()) { + // RTLO4a3 - log a warning and refuse to apply; must not throw, as a throw would abort + // every sibling operation in the same ProtocolMessage batch + Log.w(tag, "Invalid serial values on object operation message; serial=$timeSerial, siteCode=$siteCode; objectId=$objectId") + return false // RTLO4a3 } val existingSiteSerial = siteTimeserials[siteCode] // RTLO4a4 return existingSiteSerial == null || timeSerial > existingSiteSerial // RTLO4a5, RTLO4a6 diff --git a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/livecounter/LiveCounterManager.kt b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/livecounter/LiveCounterManager.kt index b3f7eab59..51a83dff8 100644 --- a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/livecounter/LiveCounterManager.kt +++ b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/livecounter/LiveCounterManager.kt @@ -59,7 +59,10 @@ internal class LiveCounterManager(private val liveCounter: InternalLiveCounter): liveCounter.notifyUpdated(update) // RTLC7d5a true // RTLC7d5b } else { - throw objectError("No payload found for ${operation.action} op for LiveCounter objectId=${objectId}") + // Log a warning and skip only this operation - throwing would abort every + // sibling operation in the same ProtocolMessage batch + Log.w(tag, "No payload found for ${operation.action} op for LiveCounter objectId=${objectId}, skipping") + false } } WireObjectOperationAction.ObjectDelete -> { @@ -100,6 +103,7 @@ internal class LiveCounterManager(private val liveCounter: InternalLiveCounter): */ private fun applyCounterInc(wireCounterInc: WireCounterInc, message: WireObjectMessage): ObjectUpdate { val amount = wireCounterInc.number + ?: return noOpCounterUpdate // RTLC9h - no number means a noop, not a zero increment val previousValue = liveCounter.data.get() liveCounter.data.set(previousValue + amount) // RTLC9f return ObjectUpdate.CounterUpdate(amount, message) // RTLC9g @@ -121,10 +125,14 @@ internal class LiveCounterManager(private val liveCounter: InternalLiveCounter): // which we're going to add now. val count = operation.counterCreateWithObjectId?.derivedFrom?.count ?: operation.counterCreate?.count - ?: 0.0 + // RTLC16b is unconditional and must precede the RTLC16d noop return, so that RTLC8b's + // duplicate-create skip engages even for a create op that carried no count + liveCounter.createOperationIsMerged = true // RTLC16b + if (count == null) { + return noOpCounterUpdate // RTLC16d - no count means a noop, not a zero-amount update + } val previousValue = liveCounter.data.get() liveCounter.data.set(previousValue + count) // RTLC16a - liveCounter.createOperationIsMerged = true // RTLC16b return ObjectUpdate.CounterUpdate(count, message) // RTLC16c } diff --git a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/livemap/LiveMapManager.kt b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/livemap/LiveMapManager.kt index d3b6f178c..7594d0c1a 100644 --- a/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/livemap/LiveMapManager.kt +++ b/liveobjects/src/main/kotlin/io/ably/lib/liveobjects/value/livemap/LiveMapManager.kt @@ -69,7 +69,10 @@ internal class LiveMapManager(private val liveMap: InternalLiveMap): LiveMapChan liveMap.notifyUpdated(update) // RTLM15d6a true // RTLM15d6b } else { - throw objectError("No payload found for ${operation.action} op for LiveMap objectId=${objectId}") + // Log a warning and skip only this operation - throwing would abort every + // sibling operation in the same ProtocolMessage batch + Log.w(tag, "No payload found for ${operation.action} op for LiveMap objectId=${objectId}, skipping") + false } } WireObjectOperationAction.MapRemove -> { @@ -78,7 +81,8 @@ internal class LiveMapManager(private val liveMap: InternalLiveMap): LiveMapChan liveMap.notifyUpdated(update) // RTLM15d7a true // RTLM15d7b } else { - throw objectError("No payload found for ${operation.action} op for LiveMap objectId=${objectId}") + Log.w(tag, "No payload found for ${operation.action} op for LiveMap objectId=${objectId}, skipping") + false } } WireObjectOperationAction.ObjectDelete -> { diff --git a/uts/README.md b/uts/README.md index 462fd87a0..c771335fa 100644 --- a/uts/README.md +++ b/uts/README.md @@ -103,7 +103,7 @@ Key principles (from [`integration-testing.md`](https://github.com/ably/specific understands **text** WebSocket frames and so can't inspect or modify binary msgpack. The tests therefore force JSON regardless of SDK support ([`integration-testing.md`](https://github.com/ably/specification/blob/main/uts/docs/integration-testing.md) §Protocol Variants, - [`helpers/proxy.md`](https://github.com/ably/specification/blob/main/uts/realtime/integration/helpers/proxy.md)). + [`docs/proxy.md`](https://github.com/ably/specification/blob/main/uts/docs/proxy.md)). --- @@ -163,7 +163,7 @@ what's missing". The reference tests this guide walks through correspond to thes `realtime/integration/proxy/auth_reauth.md` → **`AuthReauthTest.kt`**. > There is also a fifth, *referenced* spec: -> [`realtime/integration/helpers/proxy.md`](https://github.com/ably/specification/blob/main/uts/realtime/integration/helpers/proxy.md) +> [`docs/proxy.md`](https://github.com/ably/specification/blob/main/uts/docs/proxy.md) > (in the spec repo under `uts/realtime/integration/helpers/`). It defines the proxy's control API, rule format, > action types, and the **protocol message action-number table** (CONNECTED=4, ATTACH=10, AUTH=17, > …). The Kotlin `ProxySession` is the client for exactly that API. @@ -974,7 +974,7 @@ nothing is left implicit. | Translating specs, deviation patterns, decision tree | [`uts/docs/writing-derived-tests.md`](https://github.com/ably/specification/blob/main/uts/docs/writing-derived-tests.md) | | Integration/proxy policy, late fault injection, tiers | [`uts/docs/integration-testing.md`](https://github.com/ably/specification/blob/main/uts/docs/integration-testing.md) | | Coverage matrix | [`uts/docs/completion-status.md`](https://github.com/ably/specification/blob/main/uts/docs/completion-status.md) | -| Proxy control API, rule format, action numbers | [`uts/realtime/integration/helpers/proxy.md`](https://github.com/ably/specification/blob/main/uts/realtime/integration/helpers/proxy.md) | +| Proxy control API, rule format, action numbers | [`uts/docs/proxy.md`](https://github.com/ably/specification/blob/main/uts/docs/proxy.md) | | SDK seams | `lib/.../debug/DebugOptions.java`, `lib/.../util/Clock.java` | | Module wiring | `uts/build.gradle.kts`, `settings.gradle.kts` | | Unit mocks | `uts/.../uts/infra/unit/*` | diff --git a/uts/src/test/kotlin/io/ably/lib/uts/deviations.md b/uts/src/test/kotlin/io/ably/lib/uts/deviations.md index ab0b5b3fa..0008085d9 100644 --- a/uts/src/test/kotlin/io/ably/lib/uts/deviations.md +++ b/uts/src/test/kotlin/io/ably/lib/uts/deviations.md @@ -80,47 +80,6 @@ Entries are grouped by actionability: --- -# 2) Shared gap — open in BOTH SDKs (objects) - -*ably-js has the same documented deviation — the spec is ahead of both implementations. Optional joint fix.* - -## RTLC9h / RTLC16 / RTLO4b4c1 — missing-field counter ops are not treated as no-ops - -**Spec points:** RTLC9h, RTLC16d, RTLO4b4c1 -**What the spec requires:** A `COUNTER_INC` whose `counterInc.number` is **absent** produces a no-op -`LiveObjectUpdate` (`update.noop == true`, RTLC9h), so a subscribed listener must NOT be invoked -(RTLO4b4c1). For the RTLO4b4c1 stimulus (`01` real inc → `02` missing-number noop → `03` real inc), the -listener fires exactly twice (`updates == 2`). -**What the SDK does:** `WireCounterInc.number` is a **non-nullable `Double`** -(`liveobjects/.../message/WireObjectMessage.kt`), so an absent `number` deserialises to `0.0` — -indistinguishable from `number: 0`. `LiveCounterManager.applyCounterInc` unconditionally returns -`ObjectUpdate.CounterUpdate(amount, message)` (RTLC9g) and calls `notifyUpdated`; there is **no** -missing-number/RTLC9h noop branch on the operation path (the only counter noop, RTLC14b via -`calculateUpdateFromDataDiff`, exists on the sync/`replaceData` path, not the op path). So the -missing-number `02` op fires an amount-0 event and the listener is invoked a 3rd time (`updates == 3`). -**Same family — RTLC16 (COUNTER_CREATE with absent `count`):** RTLC16d requires the create-op merge to -return a noop when `counterCreate.count` does not exist; `LiveCounterManager.mergeInitialDataFromCreateOperation` -returns a normal amount-0 `CounterUpdate` instead (`?: 0.0`). Same root cause: the wire types don't track -field presence, so an absent count is indistinguishable from an explicit `0` (which per RTLC16c correctly -yields an amount-0 update, NOT a noop). -**ably-js status:** same deviations (documented in its `deviations.md`) — `_applyCounterInc` applies -`op.number` unconditionally, so a missing number becomes `NaN` and an event fires; and its create-op merge -does the identical `counterCreate?.count ?? 0`, applying an amount-0 update where RTLC16d wants a noop. -**Workaround in tests:** `RTLO4b4c1 - noop update does not trigger listener` is gated behind -`RUN_DEVIATIONS` (early `return@runTest`), keeping the spec-correct `assertEquals(2, updates.size)`. -Repro: `RUN_DEVIATIONS=1 ./gradlew :uts:runUtsUnitTests --tests "*LiveObjectSubscribeTest"`. -**Root cause / fix (SDK):** make `WireCounterInc.number` (and `WireCounterCreate.count`) nullable (or -track field presence) and add the missing-field → noop branches (RTLC9h in `applyCounterInc`, RTLC16d in -`mergeInitialDataFromCreateOperation`) so no event is emitted. To be fixed in both SDKs together. -**Tests affected (LiveObjectSubscribeTest.kt):** -- `RTLO4b4c1 - noop update does not trigger listener` (RTLO4b4c1/noop-no-trigger-0) — env-gated. - -> Note: the internal `LiveObjectUpdate.noop` diff flag is also not exposed on the public -> `InstanceSubscriptionEvent` (only `getObject()`/`getMessage()`, mapping §8); the spec frames noop -> suppression as *the listener not firing*, which is the observable the env-gated test asserts. - ---- - # 3) Expected — typed-SDK / language adaptations (objects) — NOT bugs, no action *These exist only because ably-java implements the statically-typed **RTTS** variant of the objects spec diff --git a/uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveObjectSubscribeTest.kt b/uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveObjectSubscribeTest.kt index 8a5e7b0a0..268c2d2af 100644 --- a/uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveObjectSubscribeTest.kt +++ b/uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/LiveObjectSubscribeTest.kt @@ -117,15 +117,6 @@ class LiveObjectSubscribeTest { */ @Test fun `RTLO4b4c1 - noop update does not trigger listener`() = runTest { - // DEVIATION (RTLC9h / RTLO4b4c1): spec says a COUNTER_INC whose `counterInc.number` does not exist - // returns a noop (`update.noop == true`), so the listener must NOT fire — expected updates == 2. - // ably-java's `WireCounterInc.number` is a non-nullable Double (absent -> 0.0, indistinguishable - // from `number: 0`), and `LiveCounterManager.applyCounterInc` always returns a CounterUpdate - // (RTLC9g) and notifies; there is no missing-number noop branch on the operation path. So the - // missing-number "02" op fires an amount-0 event and the listener is invoked a 3rd time - // (updates == 3). Spec-correct assertion gated behind RUN_DEVIATIONS. See deviations.md. - if (System.getenv("RUN_DEVIATIONS") == null) return@runTest - val (_, _, root, mockWs) = setupSyncedChannel("test") val updates = mutableListOf() val control = mutableListOf()