[AIT-1147] Add objects UTS translation notes and first spec-derived LiveObjects integration tests - #2226
Conversation
…e-agnostic references/objects-mapping.md goes from placeholder stub to the full ably-js <-> Swift rewrite table for the objects UTS module (the path-based LiveObjects API): the three-layers disambiguation, entry point, typed-throws async mapping (incl. deferred-await Task bridging), the three typed PathObject views with the Primitive consolidation, the Instance enum, LiveMapValue literals, wrong-type write patterns, subscriptions/sync-state events, public ObjectMessage types, the full spec error-code table, internal-graph caveats and helper-authoring guidance, REST fixture provisioning, a worked example, and a symbol index. Includes the runtime-status warning: the path-based API is a skeleton (notImplemented() traps), so objects specs are translate-only and generated suites must carry a .disabled trait. SKILL.md stays a module-agnostic workflow document: the plugin wiring, natively-async examples, and stale objects-placeholder note move out (the concrete wiring now lives in the module's translation notes), replaced by generic phrasing that defers to each module's notes file.
…disabled) Three suites derived from uts/objects/integration via the uts-to-swift skill, against the new path-based LiveObjects public API: - ObjectsSyncTests (objects_sync_test.md, RTO4/RTO5/RTO17): attach -> OBJECT_SYNC -> SYNCED, get() resolution, re-attach re-sync. - ObjectsLifecycleTests (objects_lifecycle_test.md): root PathObject shape, REST-provisioned pre-existing data observed after sync. - ObjectsGcTests (objects_gc_test.md, RTO10/RTLM19): tombstoned counter is recreatable under a new objectId. Module helpers in integration/standard/objects/helpers/: ObjectsRestProvisioning.swift (the spec's provision_objects_via_rest against the V2 objects REST API) and ObjectsIntegrationHelpers.swift (plugin-wired client options, objects channel builder, non-throwing typed value() readers for pollUntil closures, counterInstanceId). The UTS target gains the AblyLiveObjects dependency in Package.swift. Every suite carries a .disabled trait besides .serialized: the path-based API's Default* implementations all trap via notImplemented() (verified: an enabled run fatal-errors in get()), so these tests are the translate-only deliverable - they pin the API shape at compile time and become runnable by removing the trait once the implementation lands. swift build --build-tests is clean and swift test --filter UTS.Objects skips all 12 cases cleanly.
WalkthroughAdds UTS-to-Swift translation guidance and a complete LiveObjects mapping reference. Wires LiveObjects into UTS tests and adds disabled object synchronization, lifecycle, garbage-collection, proxy-fault, REST history, presence, and publish integration suites. ChangesUTS translation and integration coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Translate the UTS rest integration specs (uts/rest/integration/{publish,
history,presence}.md) into direct-sandbox suites under
Test/UTS/integration/standard/rest, each parameterised over the json/msgpack
protocol variants:
- PublishTests (RSL1d, RSL1n, RSL1k5, RSL1l1, RSL1m4) — RSL1l1 is gated
behind RUN_DEVIATIONS and recorded in deviations.md (Failing Tests):
ably-cocoa has no publish-with-params API.
- HistoryTests (RSL2, RSL2a, RSL2b1-b3)
- PresenceTests (RSP1, RSP3/RSP3a1-a2, RSP4/RSP4b1-b3, RSP5) — 17 cases,
including cipher-decoding fixtures and realtime-provisioned members.
Tests are translated and compiled, not yet evaluated against the sandbox.
Also includes ObjectsFaultsTests, the first proxy-tier objects suite from
the earlier LiveObjects UTS work.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/uts-to-swift/SKILL.md:
- Around line 938-942: Update the direct-sandbox template’s ARTClientOptions
construction to expose an explicit plugin-builder hook, requiring plugin-backed
modules to install their module helper’s plugin before accessing module entry
points. Preserve plain options for non-plugin modules and direct translators to
the existing Module helpers guidance rather than hand-wiring plugins per test.
In `@Test/UTS/integration/standard/objects/helpers/ObjectsRestProvisioning.swift`:
- Around line 63-96: Update withTarget to reject calls that provide both
objectId and path, and require exactly one target for operations that need an
existing object or path. Preserve targetless payloads for create operations such
as mapCreateOp and counterCreateOp, while ensuring mapSetOp, mapRemoveOp, and
counterIncOp cannot construct targetless payloads; propagate or surface
validation before sending the REST request.
In `@Test/UTS/integration/standard/objects/ObjectsLifecycleTests.swift`:
- Around line 44-56: Update the assertions in this lifecycle test to validate
the captured subscription events in eventsB, not only the value returned by
rootB.get. Assert that the expected greeting update was delivered through
rootB.subscribe while preserving the existing value assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f9d27634-df35-4936-acbb-77df8287157a
📒 Files selected for processing (13)
.claude/skills/uts-to-swift/SKILL.md.claude/skills/uts-to-swift/references/objects-mapping.mdPackage.swiftTest/UTS/deviations.mdTest/UTS/integration/proxy/objects/ObjectsFaultsTests.swiftTest/UTS/integration/standard/objects/ObjectsGcTests.swiftTest/UTS/integration/standard/objects/ObjectsLifecycleTests.swiftTest/UTS/integration/standard/objects/ObjectsSyncTests.swiftTest/UTS/integration/standard/objects/helpers/ObjectsIntegrationHelpers.swiftTest/UTS/integration/standard/objects/helpers/ObjectsRestProvisioning.swiftTest/UTS/integration/standard/rest/HistoryTests.swiftTest/UTS/integration/standard/rest/PresenceTests.swiftTest/UTS/integration/standard/rest/PublishTests.swift
| For a **plugin-backed module**, the client options must also install the module's plugin — the | ||
| module's entry-point property traps without it. The module's translation notes name the exact | ||
| wiring, and the module packages it as a client-options builder in its module `helpers/` directory | ||
| (see **Module helpers** below) — use that builder; don't hand-wire the plugin per test. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the plugin wiring explicit in the template.
The new rule requires plugin-backed suites to use a module helper, but the direct-sandbox template still constructs plain ARTClientOptions without installing the plugin. A translator following that template can generate tests that trap when accessing channel.object. Add a plugin-builder hook to the template or clearly mark the module-specific wiring step as mandatory.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/skills/uts-to-swift/SKILL.md around lines 938 - 942, Update the
direct-sandbox template’s ARTClientOptions construction to expose an explicit
plugin-builder hook, requiring plugin-backed modules to install their module
helper’s plugin before accessing module entry points. Preserve plain options for
non-plugin modules and direct translators to the existing Module helpers
guidance rather than hand-wiring plugins per test.
| func mapSetOp(key: String, value: [String: Any], objectId: String? = nil, path: String? = nil, id: String? = nil) -> [String: Any] { | ||
| withTarget(["mapSet": ["key": key, "value": value]], objectId: objectId, path: path, id: id) | ||
| } | ||
|
|
||
| /// `{ mapRemove: { key }, objectId|path, id? }` — remove `key` from the target map. | ||
| func mapRemoveOp(key: String, objectId: String? = nil, path: String? = nil, id: String? = nil) -> [String: Any] { | ||
| withTarget(["mapRemove": ["key": key]], objectId: objectId, path: path, id: id) | ||
| } | ||
|
|
||
| /// `{ mapCreate: { semantics, entries }, objectId|path?, id? }` — create a map (semantics 0 = LWW). | ||
| /// Entry values are wrapped as `{ data: <value> }` per the V2 schema. A create with no target makes | ||
| /// a standalone object. | ||
| func mapCreateOp(entries: [String: [String: Any]], semantics: Int = 0, objectId: String? = nil, path: String? = nil, id: String? = nil) -> [String: Any] { | ||
| let wrappedEntries = entries.mapValues { value in ["data": value] } | ||
| return withTarget(["mapCreate": ["semantics": semantics, "entries": wrappedEntries]], objectId: objectId, path: path, id: id) | ||
| } | ||
|
|
||
| /// `{ counterCreate: { count }, objectId|path?, id? }` — create a counter. | ||
| func counterCreateOp(count: Double, objectId: String? = nil, path: String? = nil, id: String? = nil) -> [String: Any] { | ||
| withTarget(["counterCreate": ["count": count]], objectId: objectId, path: path, id: id) | ||
| } | ||
|
|
||
| /// `{ counterInc: { number }, objectId|path, id? }` — increment the target counter | ||
| /// (a negative number decrements). | ||
| func counterIncOp(number: Double, objectId: String? = nil, path: String? = nil, id: String? = nil) -> [String: Any] { | ||
| withTarget(["counterInc": ["number": number]], objectId: objectId, path: path, id: id) | ||
| } | ||
|
|
||
| private func withTarget(_ operation: [String: Any], objectId: String?, path: String?, id: String?) -> [String: Any] { | ||
| var operation = operation | ||
| if let objectId { operation["objectId"] = objectId } | ||
| if let path { operation["path"] = path } | ||
| if let id { operation["id"] = id } | ||
| return operation |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate REST operation targets before sending.
withTarget allows both objectId and path, and also allows neither. This lets mapSetOp, mapRemoveOp, and counterIncOp construct invalid V2 payloads; the failure is deferred to the sandbox as a generic HTTP error. Enforce mutual exclusivity in withTarget, and require one target for the non-create operations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Test/UTS/integration/standard/objects/helpers/ObjectsRestProvisioning.swift`
around lines 63 - 96, Update withTarget to reject calls that provide both
objectId and path, and require exactly one target for operations that need an
existing object or path. Preserve targetless payloads for create operations such
as mapCreateOp and counterCreateOp, while ensuring mapSetOp, mapRemoveOp, and
counterIncOp cannot construct targetless payloads; propagate or surface
validation before sending the REST request.
| // Test Steps | ||
| // Client A sets a value | ||
| try await rootA.set(key: "greeting", value: "hello") | ||
|
|
||
| // Client B subscribes and waits for the update | ||
| let eventsB = Captured<PathObjectSubscriptionEvent>() | ||
| try rootB.subscribe { event in eventsB.append(event) } | ||
| guard await pollUntil("rootB.greeting == \"hello\"", timeout: 10, { | ||
| stringValue(at: rootB.get(key: "greeting")) == "hello" | ||
| }) else { return } | ||
|
|
||
| // Assertions | ||
| #expect(stringValue(at: rootB.get(key: "greeting")) == "hello") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Subscription events captured but never asserted.
eventsB is populated via rootB.subscribe (Line 50) but nothing in the Assertions block checks its contents — the test only re-reads the value via rootB.get. This leaves the subscription-delivery behavior itself unverified even though setting it up appears to be the point of this test.
✅ Proposed fix to assert on the captured subscription event
// Assertions
`#expect`(stringValue(at: rootB.get(key: "greeting")) == "hello")
+ `#expect`(eventsB.values.contains { $0.update["greeting"] != nil })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Test Steps | |
| // Client A sets a value | |
| try await rootA.set(key: "greeting", value: "hello") | |
| // Client B subscribes and waits for the update | |
| let eventsB = Captured<PathObjectSubscriptionEvent>() | |
| try rootB.subscribe { event in eventsB.append(event) } | |
| guard await pollUntil("rootB.greeting == \"hello\"", timeout: 10, { | |
| stringValue(at: rootB.get(key: "greeting")) == "hello" | |
| }) else { return } | |
| // Assertions | |
| #expect(stringValue(at: rootB.get(key: "greeting")) == "hello") | |
| // Test Steps | |
| // Client A sets a value | |
| try await rootA.set(key: "greeting", value: "hello") | |
| // Client B subscribes and waits for the update | |
| let eventsB = Captured<PathObjectSubscriptionEvent>() | |
| try rootB.subscribe { event in eventsB.append(event) } | |
| guard await pollUntil("rootB.greeting == \"hello\"", timeout: 10, { | |
| stringValue(at: rootB.get(key: "greeting")) == "hello" | |
| }) else { return } | |
| // Assertions | |
| `#expect`(stringValue(at: rootB.get(key: "greeting")) == "hello") | |
| `#expect`(eventsB.values.contains { $0.update["greeting"] != nil }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Test/UTS/integration/standard/objects/ObjectsLifecycleTests.swift` around
lines 44 - 56, Update the assertions in this lifecycle test to validate the
captured subscription events in eventsB, not only the value returned by
rootB.get. Assert that the expected greeting update was delivered through
rootB.subscribe while preserving the existing value assertion.
|
Closing in favor of https://github.com/ably/ably-cocoa/pull/2235/changes |
Builds on #2225 (hence the base branch) — the first UTS work against the new path-based LiveObjects public API.
🎯 What this PR does
Two things, one per commit:
objectsmodule translation notes for theuts-to-swiftskill —references/objects-mapping.mdgoes from an intentionally-empty placeholder to the full ably-js ⇄ Swift rewrite table the skill requires before translating anyobjectsspec.objectsoutput: three spec-derived integration suites + the module's REST-provisioning helpers, deliberately compile-only (.disabled) until the path-based API is implemented — they pin the API shape today and become runnable by deleting one trait later.📖 Commit 1 — the translation notes
The
objectsUTS specs are written against ably-js's dynamically-typed LiveObjects surface; ably-cocoa implements the spec's Typed-SDK variant with two Swift-idiomatic consolidations (a singlePrimitiveview instead of six sub-types;Instanceas an exhaustively-switchable enum). Nearly every spec line therefore needs a mechanical rewrite, and the notes are that rewrite table — 16 sections covering the three-layers disambiguation (creation value types vs public views vs internal CRDT nodes), typed-throws async mapping (including the deferred-AWAIT→Taskbridge and its typed-throw erasure), the typedPathObjectviews,LiveMapValueliteral conformances, wrong-type write failure patterns, subscriptions/sync-state events, the publicObjectMessagetypes, the full spec error-code table, internal-graph caveats with helper-authoring guidance for the unit tier, REST fixture provisioning, a worked example, and a quick symbol index.Every API claim in the table was verified against the Swift source (a symbol-existence sweep of all referenced signatures), and the pseudocode forms were validated against the actual spec files (which surfaced, e.g., that specs write
on(SYNCED, cb)constants and deferredx_futureawaits).As part of this, SKILL.md is scrubbed back to a module-agnostic workflow document: the LiveObjects plugin wiring, natively-async examples, and the stale "objects notes are a placeholder" claim move out; module-specific facts now live where they belong — in the module's translation notes, which the skill already routes to via
uts-package-mapping.json.🧪 Commit 2 — first spec-derived objects integration tests
ObjectsSyncTestsobjects_sync_test.md(RTO4/RTO5/RTO17)OBJECT_SYNC→ SYNCED,get()resolution, re-attach re-syncObjectsLifecycleTestsobjects_lifecycle_test.mdPathObjectshape; REST-provisioned pre-existing data observed after syncObjectsGcTestsobjects_gc_test.md(RTO10/RTLM19)objectIdPlus module helpers in
integration/standard/objects/helpers/:ObjectsRestProvisioning.swift— the spec'sprovision_objects_via_restagainst the V2 objects REST API (POST /channels/{channel}/object, singular; nonprod sandbox host), returning the createdobjectIds.ObjectsIntegrationHelpers.swift— plugin-wired client options (objectsClientOptions(key:useBinaryProtocol:)), objects channel builder, non-throwing typedvalue()readers forpollUntilclosures (SE-0230try?-flattening makes inline reads a footgun), andcounterInstanceId(at:).Package.swift: theUTStest target gains theAblyLiveObjectsdependency.⏸️ Why the suites are
.disabledThe path-based API is currently a skeleton — every
Default*implementation traps vianotImplemented(). Verified empirically: enabling a suite fatal-errors immediately (Fatal error: Not implemented: get()), and sincefatalErrorkills the test process, un-disabled suites would crash the documentedswift test --filter UTSworkflow for the whole target. So each suite carries:@Suite(.serialized, .disabled("The path-based LiveObjects public API is not yet implemented (every operation traps via notImplemented()). Compile-only until the implementation lands - then remove this trait."))This is the UTS "translate-only" deliverable: the compiler keeps every spec-derived call site honest against the API as it evolves, and flipping the suites live is a one-line-per-file change once the implementation lands. The translation notes document this as a standing rule for all generated
objectspublic-API suites.✅ Verification
swift build --build-testsclean (Swift 6 language mode; no@availableannotations needed in test code — SPM raises the test deployment target above the package floor).swift test --filter UTS.Objects: all 12 parameterised cases skip cleanly with the disable reason; run green in 0.001s.uts/objectsspecs.Summary by CodeRabbit
Documentation
Tests
Known Limitations