[LiveObjects] Implement path-based public API end-to-end; port all 15 UTS unit specs - #2234
[LiveObjects] Implement path-based public API end-to-end; port all 15 UTS unit specs#2234sacOO7 wants to merge 3 commits into
Conversation
…t specs Completes the LiveObjects implementation behind the shipped public API (which is unchanged), ported from ably-java as the reference: - Instance and PathObject layers (typed instances, per-call path resolution, compactJson with cycle markers, as* casts) - Path subscriptions with RTO24 depth-windowed coverage, wired to getFullPaths over the new parent-reference graph (RTLO3f/4f/4g/4h, RTO5c10 post-sync rebuild) - RealtimeObject.get()/on()/dispose (RTO23, RTO17/18, RTO20e waiters) - Update enrichment: objectMessage + tombstone on update payloads, PAOM3 wire-to-public conversion, tombstone subscription teardown - RTO27 channel-state data lifecycle; OM3 message-size publish gate; RTO5a sync-cursor reconciliation; channel-config guards + error model - 14 spec-compliance bug fixes found by the ported spec tests (RTLO4e10 root-tombstone protection, exclusive-access crash guards, RTLM24c tie-break, RTLM22 teardown diffs, RTO9a2b check ordering, ...) All 15 UTS objects/unit specs are ported to LiveObjects/Tests/AblyLiveObjectsTests/UTS/ (matching ably-java's uts/unit layout) with @uts traceability tags and a deviations register. 653 unit tests passing; lint clean; public API surface byte-identical to the previously shipped commit.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
🚧 Files skipped from review as they are similar to previous changes (6)
WalkthroughThe LiveObjects implementation now supports path-based reads and writes, public instances, subscriptions, object-message conversion, parent-reference tracking, tombstone handling, synchronization lifecycle behavior, message-size validation, and extensive native and UTS test coverage. ChangesLiveObjects implementation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
LiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift (1)
781-796: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale docstring on
apply.The docstring at Line 783 says the method "Returns:
trueif the operation was applied,falseif skipped (RTLM15g)." The return type at Line 796 isLiveObjectUpdate<DefaultLiveMapUpdate>?, notBool. The docstring was not updated when the signature changed.Compare with the public wrapper
nosync_apply(Line 396), whose docstring was correctly updated to describe the new optional/enriched-update contract. Update this docstring the same way to avoid confusing future readers about the RTLM15g contract.📝 Proposed docstring fix
/// Attempts to apply an operation from an inbound `ObjectMessage`, per RTLM15. /// - /// - Returns: `true` if the operation was applied, `false` if skipped (RTLM15g). + /// - Returns: The update that was emitted if the operation was applied (which may be `.noop`), or `nil` if the operation was skipped (RTLM15g). internal mutating func apply(🤖 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 `@LiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift` around lines 781 - 796, Update the documentation for InternalDefaultLiveMap.apply to describe its LiveObjectUpdate<DefaultLiveMapUpdate>? result and optional/enriched-update behavior, matching the contract documented by the public nosync_apply wrapper. Remove the stale Bool and RTLM15g skipped-operation wording.
🧹 Nitpick comments (20)
LiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift (2)
566-569: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer explicit cases over
default:in the action switch.The
default:clause absorbs both unknown wire actions and every unhandled known action. If a new counter-relevantObjectOperationActioncase is added, this switch silently logs a warning instead of failing the build. Listing the known map-only actions explicitly and keepingcase .unknownseparate restores compile-time exhaustiveness.♻️ Proposed change
- default: + case .known(.mapCreate), .known(.mapSet), .known(.mapRemove), .known(.mapClear), .unknown: // RTLC7d3 logger.log("Operation \(operation) has unsupported action for LiveCounter; discarding", level: .warn) return 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 `@LiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift` around lines 566 - 569, Update the action switch in the LiveCounter operation handling to remove the default clause. List each known map-only ObjectOperationAction explicitly with the existing warning-and-discard behavior, and keep case .unknown separate, so newly added actions require explicit handling at compile time.
320-328: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRemove the default value from
sourceObjectMessage.
sourceObjectMessagedefaults tonil. RTLO4b4d requires the emitted update to carry the source public message for channel-originated operations. A production caller that omits the argument compiles cleanly and emits updates withobjectMessage == nil. The default exists only so existing tests can omit it.Make the parameter required, and pass
nilexplicitly from the sync-originated and test call sites.♻️ Proposed change
objectMessageSerialTimestamp: Date?, - sourceObjectMessage: ObjectMessage? = nil, + sourceObjectMessage: ObjectMessage?, objectsPool: inout ObjectsPool,🤖 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 `@LiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift` around lines 320 - 328, Remove the default value from sourceObjectMessage in nosync_apply so callers must provide it. Update all sync-originated and test call sites to pass nil explicitly, while ensuring channel-originated callers pass the source public message required for emitted updates.LiveObjects/Sources/AblyLiveObjects/Internal/ChannelConfigGuards.swift (1)
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe stub guard silently passes.
throwIfMissingObjectSubscribeModeaccepts both parameters and discards them, and its body never throws. Every call site therefore performs no check while reading as if it enforces RTO2a2. The doc comment explains the plugin-API gap, so this is intentional. Consider tracking the gap in an issue so the stub does not outlive the missing accessor.Do you want me to open a tracking issue for the
object_subscribe/object_publishmode accessors?🤖 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 `@LiveObjects/Sources/AblyLiveObjects/Internal/ChannelConfigGuards.swift` around lines 40 - 43, Track the missing channel-modes accessor gap in an issue covering both object_subscribe and object_publish accessors, and reference that issue from the TODO associated with throwIfMissingObjectSubscribeMode so the intentional no-op stub is not left without follow-up.LiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultPrimitiveInstance.swift (1)
51-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
nosync_checkAccessPreconditionsOnQueue.The
nosync_prefix marks methods in this codebase that must already run on the internal queue and that perform no synchronization. This method does the opposite: it enters the queue withably_syncNoDeadlock. Rename it to something likecheckAccessPreconditions()so the prefix keeps one meaning.♻️ Proposed change
- private func nosync_checkAccessPreconditionsOnQueue() throws(ARTErrorInfo) { + private func checkAccessPreconditions() throws(ARTErrorInfo) {Update both call sites (Lines 31 and 44).
🤖 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 `@LiveObjects/Sources/AblyLiveObjects/Path` Based API/Default/DefaultPrimitiveInstance.swift around lines 51 - 64, Rename nosync_checkAccessPreconditionsOnQueue to checkAccessPreconditions, reflecting that it synchronizes onto internalQueue via ably_syncNoDeadlock. Update both call sites in DefaultPrimitiveInstance while preserving the existing channel-state validation behavior.LiveObjects/Sources/AblyLiveObjects/Internal/ObjectsPool.swift (1)
558-575: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSkip the path DFS when no path subscription exists.
nosync_getFullPathswalks the parent-reference graph and enumerates every simple path. Path enumeration grows exponentially with graph density. This runs for every applied operation (nosync_applyObjectProtocolMessageObjectMessage) and for every non-noop deferred sync update in the loop at Lines 450-465. When an application registers no path subscription, the whole walk is discarded.Add an emptiness check to
PathObjectSubscriptionRegisterand return before the DFS.♻️ Proposed early return
In
PathObjectSubscriptionRegister:/// `true` when no subscription is registered. Callers use this to skip path resolution. internal var nosync_isEmpty: Bool { dispatchPrecondition(condition: .onQueue(internalQueue)) return subscriptions.isEmpty }In
ObjectsPool.nosync_notifyPathSubscriptions:) { + if register.nosync_isEmpty { + return // nothing to dispatch; avoid the getFullPaths walk + } let pathsToThis = nosync_getFullPaths(forObjectID: objectID) // RTO24b1🤖 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 `@LiveObjects/Sources/AblyLiveObjects/Internal/ObjectsPool.swift` around lines 558 - 575, Add the queue-validated `nosync_isEmpty` property to `PathObjectSubscriptionRegister`, returning whether its subscriptions collection is empty. In `ObjectsPool.nosync_notifyPathSubscriptions`, check this property and return before calling `nosync_getFullPaths`, while preserving the existing path notification behavior when subscriptions exist.LiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultLiveMapInstance.swift (1)
233-238: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThrow instead of trapping when the delegate cannot create objects.
set(key:value:)reachesobjectCreatorfor anyLiveMap/LiveCounterblueprint value.preconditionFailurethen terminates the process. Any conformer ofInternalRealtimeObjectsProtocolother thanInternalDefaultRealtimeObjects(for example a test double, or a future conformer) turns a normal write into a crash.internalValue(from:...)is alreadythrows(ARTErrorInfo), so an error is the cheaper failure mode.♻️ Proposed change
- private static func objectCreator(_ realtimeObjects: any InternalRealtimeObjectsProtocol) -> InternalDefaultRealtimeObjects { + private static func objectCreator(_ realtimeObjects: any InternalRealtimeObjectsProtocol) throws(ARTErrorInfo) -> InternalDefaultRealtimeObjects { guard let creator = realtimeObjects as? InternalDefaultRealtimeObjects else { - preconditionFailure("Setting a LiveMap/LiveCounter blueprint value requires the concrete InternalDefaultRealtimeObjects") + throw LiveObjectsError.other( + message: "Setting a LiveMap/LiveCounter blueprint value requires the concrete InternalDefaultRealtimeObjects", + ).toARTErrorInfo() } return creator }Adjust the two call sites to
try Self.objectCreator(realtimeObjects), and use the error case that matchesLiveObjectsError.🤖 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 `@LiveObjects/Sources/AblyLiveObjects/Path` Based API/Default/DefaultLiveMapInstance.swift around lines 233 - 238, Update objectCreator and both call sites in internalValue(from:...) to propagate an ARTErrorInfo-compatible LiveObjectsError instead of invoking preconditionFailure when the delegate is not InternalDefaultRealtimeObjects. Mark objectCreator as throwing, adjust callers to use try, and preserve the existing successful creator path.LiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultLiveCounterInstance.swift (1)
55-72: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDocument or fix the listener lifetime contract.
The listener captures
selfweakly.DefaultSubscriptionstores only theSubscribeResponse, so it does not retain this instance. If a caller keeps only the returnedSubscriptionand releases theLiveCounterInstance(for example when the instance comes from a temporaryInstance.from(...)result), the listener stops firing and no error is reported.The weak capture does break the
node→ listener →self→nodecycle, so it is defensible. Make the requirement explicit in the doc comment, or let the returned subscription retain the instance so delivery does not depend on caller-side retention.♻️ Option: document the retention requirement
`@discardableResult` internal func subscribe(listener: `@escaping` InstanceSubscriptionCallback) throws(ARTErrorInfo) -> any Subscription { + // Lifetime: the listener holds `self` weakly to avoid a `node` -> listener -> `self` -> `node` + // cycle. The caller must keep this instance alive for as long as it expects events; retaining + // only the returned Subscription is not sufficient. // RTINS16b (the node's subscribe runs the RTO25b check), RTINS16d -> RTLO4b🤖 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 `@LiveObjects/Sources/AblyLiveObjects/Path` Based API/Default/DefaultLiveCounterInstance.swift around lines 55 - 72, Document the retention contract for subscribe in DefaultLiveCounterInstance: state that callers must retain the LiveCounterInstance while relying on the returned Subscription because the listener captures self weakly and delivery stops once the instance is released. Keep the existing weak capture and subscription behavior unchanged.LiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultPathObject.swift (1)
143-160: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument or make path resolution atomic.
resolveValueAtCurrentPath()reads the root on one internal-queue block, then eachmapNode.get(key:coreSDK:delegate:)locks independently viaInternalDefaultLiveMap.get. Multi-segment reads can be built from different graph states between the root read, key lookup, and any intermediate object states. If this best-effort behavior is intentional, document it nearresolveValueAtCurrentPath(); otherwise perform the entire walk in one internal-queue block using a queue-confined map accessor.🤖 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 `@LiveObjects/Sources/AblyLiveObjects/Path` Based API/Default/DefaultPathObject.swift around lines 143 - 160, Make path resolution in resolveValueAtCurrentPath atomic by executing the root read and entire PathSegments.parseStored(path) traversal within one internalQueue block, using a queue-confined map accessor instead of independently locking through mapNode.get. Preserve the existing empty-path, non-map, missing-key, and successful-value behaviors; if atomic traversal is not intended, document the best-effort consistency semantics near the method instead.LiveObjects/Tests/AblyLiveObjectsTests/UTS/ObjectsUTSHelpers.swift (1)
165-196: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deduping the publish holder and node factories.
The header comment (lines 12-17) states that the UTS ports now live in the
AblyLiveObjectsTeststarget, and that the trivial duplicates were folded intoTestLogger/MockSimpleClock. Two duplicates remain againstDefaultInstanceTests.swift:
ObjectsUTSPublished(lines 169-180) duplicates the privatePublishedholder (DefaultInstanceTests.swiftlines 470-480).ObjectsUTS.makeCounter/ObjectsUTS.makeMap(lines 190-196) duplicateDefaultInstanceTests.makeCounter/makeMap(lines 16-22).Since the import barrier no longer applies, you can promote one copy of each to an internal helper and delete the other. This is optional; the current duplication is small and harmless.
🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/ObjectsUTSHelpers.swift` around lines 165 - 196, Deduplicate the shared publish holder and node factories across the UTS helpers and DefaultInstanceTests. Promote one copy of ObjectsUTSPublished/Published and ObjectsUTS.makeCounter/makeMap to an internal shared helper, update both call sites to use it, and remove the redundant private definitions while preserving their current behavior.LiveObjects/Tests/AblyLiveObjectsTests/DefaultInstanceTests.swift (1)
267-270: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the stale self-reference note.
The comment states that a direct self-reference re-enters the map mutex and is "a pre-existing engine limitation". This PR fixes that path.
InternalDefaultLiveMapTests.selfReferencingEntryIsTreatedAsNormalEntry(lines 528-579) asserts thatentries(),keys(),values()andget()now treat a direct self-reference as a normal entry. Keep the indirect A→B→A fixture if you prefer it, but remove the claim that a direct self-reference crashes.📝 Proposed comment update
- // An indirect cycle A -> B -> A. (A direct self-reference is not used: the internal engine's - // `entries()` -> `nosync_isEntryTombstoned` re-enters the same map's mutex for a self-referencing - // entry, an exclusive-access conflict — a pre-existing engine limitation, unrelated to the - // compactJson cycle handling exercised here.) + // An indirect cycle A -> B -> A. (A direct self-reference is covered separately by + // `InternalDefaultLiveMapTests.selfReferencingEntryIsTreatedAsNormalEntry`; this case targets + // the multi-hop revisit branch of the compactJson walk.)🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/DefaultInstanceTests.swift` around lines 267 - 270, Update the cycle-handling comment near the indirect A → B → A fixture to remove the claim that direct self-reference re-enters the mutex or remains a pre-existing engine limitation. Reflect that direct self-referencing entries are now supported and treated as normal entries, while retaining the indirect-cycle explanation if still relevant.LiveObjects/Tests/AblyLiveObjectsTests/UTS/RealtimeObjectUTSTests.swift (1)
239-239: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe lint directives are inert.
Both lines carry
swiftformat:disable:thisandswiftlint:disable:thisdirectives. The repository does not run SwiftLint or SwiftFormat, so neither directive has an effect. You can drop them and keep the plain#expect(counter.count == 0). This is optional.As per coding guidelines: "Maintain EditorConfig compliance only (no SwiftLint/SwiftFormat)".
Also applies to: 363-363
🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/RealtimeObjectUTSTests.swift` at line 239, Remove the inert swiftformat and swiftlint directives from the `#expect` assertions in the affected test cases, leaving plain `#expect`(counter.count == 0) assertions at both locations.Source: Coding guidelines
LiveObjects/Tests/AblyLiveObjectsTests/PublicRealtimeObjectTests.swift (1)
176-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed sleep with a deterministic queue drain.
The test waits 100 ms and then asserts that no callback ran. The engine dispatches status callbacks to
userCallbackQueue, which is.mainhere. One awaited hop on.mainguarantees that every already-scheduled callback has run. That removes the timing dependence and the fixed delay.RealtimeObjectUTSTestsalready uses this pattern indrainMain()(lines 109-113) for the same assertion inRTO19_off_deregisters.♻️ Proposed change
Self.driveToSynced(proxied, on: internalQueue) - // Allow any (erroneously) scheduled callback to be delivered on the callback queue. - try await Task.sleep(nanoseconds: 100_000_000) + // Drain the callback queue so any (erroneously) scheduled callback has been delivered. + await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in + DispatchQueue.main.async { continuation.resume() } + } `#expect`(counter.isEmpty)🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/PublicRealtimeObjectTests.swift` around lines 176 - 180, In the test around Self.driveToSynced, replace the fixed Task.sleep delay with the existing deterministic main-queue drain pattern used by RealtimeObjectUTSTests.drainMain(). Await one hop on the main callback queue before asserting counter.isEmpty, preserving the assertion that no callback was delivered.LiveObjects/Tests/AblyLiveObjectsTests/UTS/PathObjectSubscribeUTSTests.swift (1)
296-311: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exact event counts where dispatch is deterministic.
Three assertions use
>=:
- Line 310:
events.events.count >= 3after three drained applies.- Line 438:
events.events.count >= 1after one MAP_CLEAR.- Line 454:
events.events.count >= 2after two drained applies.Each stimulus is applied through
applyAndDrain, so delivery is complete before the assertion. An exact count would catch a duplicate-dispatch or extra-candidate regression that>=accepts. If the MAP_CLEAR fan-out count is intentionally unpinned, keep>=there and add a short note stating why.Note: the
>=assertions on the control collectors (lines 362, 383, 292, 343) are quiescence barriers for negative assertions. Those should stay as they are.Also applies to: 426-455
🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/PathObjectSubscribeUTSTests.swift` around lines 296 - 311, Replace the >= event-count assertions in RTPO19c1_unlimited_depth and the corresponding MAP_CLEAR and two-apply tests with exact expected counts, since applyAndDrain completes delivery deterministically. Preserve the control-collector >= assertions used as quiescence barriers; if MAP_CLEAR fan-out is intentionally variable, retain its lower-bound assertion and add a brief explanation.LiveObjects/Tests/AblyLiveObjectsTests/DefaultPathObjectTests.swift (2)
7-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the stale doc comment about path subscriptions.
The comment states that path subscriptions (RTPO19/RTO24) "still trap". This PR implements them, and
PathObjectSubscriptionRegisterplus dedicated subscription tests now exist. Remove or reword that clause so the file header matches the shipped behaviour.♻️ Proposed doc update
-/// subclasses, the ``ChannelConfigGuards`` state checks, and the extended error model. Path -/// *subscriptions* (RTPO19/RTO24) are part 2 and are not exercised here (they still trap). +/// subclasses, the ``ChannelConfigGuards`` state checks, and the extended error model. Path +/// *subscriptions* (RTPO19/RTO24) are covered separately by the path-subscription tests.🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/DefaultPathObjectTests.swift` around lines 7 - 12, Update the file-level documentation near the PathSegments and DefaultPathObject description to remove or reword the claim that path subscriptions RTPO19/RTO24 still trap, reflecting that PathObjectSubscriptionRegister and their tests are now implemented.
388-395: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider accumulating published messages in the double.
nosync_publishAndApplyreplaces_capturedon every call. A future test that performs two writes will silently observe only the last batch. Appending keeps the double usable for multi-write cases.♻️ Proposed change
- mutex.withLock { _captured = objectMessages } + mutex.withLock { _captured = (_captured ?? []) + objectMessages }🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/DefaultPathObjectTests.swift` around lines 388 - 395, Update nosync_publishAndApply to append objectMessages to the existing _captured collection rather than replacing it, while preserving the current mutex protection and success callback behavior.LiveObjects/Tests/AblyLiveObjectsTests/ParentReferencesTests.swift (1)
5-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReconcile this file with the UTS port that landed in the same PR.
The header states that the full UTS port of
objects/unit/parent_references.mdis "a separate follow-up". That port is included in this PR asLiveObjects/Tests/AblyLiveObjectsTests/UTS/ParentReferencesUTSTests.swift, and it covers add, remove, diamond, cycle, orphan and the RTO5c10 rebuild with stricter assertions. Update this header, and consider keeping only the cases the UTS port does not cover (for exampleclearParentReferences), to avoid maintaining two copies of the same behaviour.🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/ParentReferencesTests.swift` around lines 5 - 8, Reconcile the ParentReferencesTests header with the existing ParentReferencesUTSTests coverage: remove the claim that the UTS port is a separate follow-up, and retain only native smoke cases not covered by the UTS tests, such as clearParentReferences, to avoid duplicate behavior coverage.LiveObjects/Tests/AblyLiveObjectsTests/Mocks/MockRealtimeObjects.swift (1)
12-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider failing fast if the register is used.
The register is created on queues that no test object shares. If a test ever routes path-subscription dispatch through this mock, the queue mismatch produces an internal-queue assertion rather than a clear message.
nosync_objectsPoolalready usespreconditionFailurefor unsupported use. Applying the same pattern here keeps misuse obvious.🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/Mocks/MockRealtimeObjects.swift` around lines 12 - 25, The mock’s nosync_pathObjectSubscriptionRegister currently exposes an unusable register instead of failing clearly when accessed. Replace the getter implementation with preconditionFailure, matching the existing nosync_objectsPool unsupported-use pattern, and remove the unused _pathObjectSubscriptionRegister initialization and stored property.LiveObjects/Tests/AblyLiveObjectsTests/UTS/ObjectIdUTSTests.swift (1)
41-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the objectId parsing and align the padding assertion.
objectIdFormatCounterandhashIsBase64urlEncodedsplit the same objectId with two different idioms, and they assert padding differently: line 51 uses!contains("="), line 121 uses!hasSuffix("="). A single helper that returns(type, hash, timestamp)removes the duplication and lets both tests use the stricter!contains("=")check.♻️ Proposed helper
/// Splits `{type}:{hash}@{timestamp}` into its three parts. private static func parts(of objectId: String) throws -> (type: String, hash: String, timestamp: String) { let type = String(objectId.prefix { $0 != ":" }) let rest = String(objectId.dropFirst(type.count + 1)) let hash = String(rest.prefix { $0 != "@" }) return (type, hash, String(rest.dropFirst(hash.count + 1))) }Also applies to: 116-121
🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/ObjectIdUTSTests.swift` around lines 41 - 51, Extract the duplicated `{type}:{hash}@{timestamp}` parsing from `objectIdFormatCounter` and `hashIsBase64urlEncoded` into a shared `parts(of:)` helper returning type, hash, and timestamp. Update both tests to use the helper and change the padding assertion in `hashIsBase64urlEncoded` from `hasSuffix("=")` to rejecting any `=` with `contains`.LiveObjects/Tests/AblyLiveObjectsTests/UTS/ValueTypesUTSTests.swift (1)
59-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the inert lint directives.
The repository uses EditorConfig only and does not run SwiftLint or SwiftFormat. The
swiftformat:disable/swiftlint:disablecomments on Line 59 and Line 114 have no effect. Delete them to keep the assertions readable.As per coding guidelines: "Maintain EditorConfig compliance only (no SwiftLint/SwiftFormat)".
🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/ValueTypesUTSTests.swift` at line 59, Remove the inert swiftformat:disable and swiftlint:disable comments from the count assertions at the referenced test locations, including the assertion near vt.count and the corresponding one near line 114. Keep the assertions themselves unchanged and maintain EditorConfig-only formatting.Source: Coding guidelines
Package.swift (1)
117-117: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused
_AblyPluginSupportPrivatedependency from theUTStarget.No Swift file under
Test/UTSimports_AblyPluginSupportPrivate; the LiveObjects UTS imports are underLiveObjects/Tests/AblyLiveObjectsTests, so this dependency is unnecessary for the standalone UTS target.🤖 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 `@Package.swift` at line 117, Remove the `.target(name: "_AblyPluginSupportPrivate")` dependency from the `UTS` target definition in `Package.swift`, leaving the remaining UTS dependencies unchanged.
🤖 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
`@LiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift`:
- Around line 1183-1217: Update the public message conversion in the
action-handling flow around objectMessage.toPublicObjectMessage so emitted
ObjectMessage values never receive an empty channel. Ensure the real channel
name is supplied when constructing or converting the message, or omit the
channel field until a valid channel accessor is available; preserve the existing
path-subscription notification behavior.
In `@LiveObjects/Sources/AblyLiveObjects/Protocol/ObjectMessage.swift`:
- Around line 649-650: The extras size calculation in ObjectMessage must use the
UTF-8 byte count of the serialized JSON instead of utf16.count. Update the
relevant ObjectsMap entry-key size calculation as well if it participates in the
same published limit, ensuring RTO15d validates actual wire JSON size for
non-ASCII content.
In
`@LiveObjects/Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift`:
- Around line 2189-2219: Update clearOnDetachedEmitsNoUpdateEvents to drain the
main callback queue before checking subscriber invocations: after triggering
nosync_onChannelStateChanged, add the same main-queue synchronization/barrier
pattern used by emitsCorrectSyncEvents, then retain the existing empty
assertions for rootSubscriber, mapSubscriber, and counterSubscriber.
In `@LiveObjects/Tests/AblyLiveObjectsTests/ParentReferencesTests.swift`:
- Around line 147-155: Correct the cycle-suppression comment to match the parent
relationship created by nosync_addParentReference, then strengthen the mapL
cycle test around testsOnly_getFullPaths by asserting the exact expected path
count, following the corresponding UTS/ParentReferencesUTSTests behavior. Remove
the redundant non-empty assertion while retaining the expected ["left"] path
check.
In `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/deviations.md`:
- Around line 262-263: Remove the leaked tool-artifact closing tags from all
affected documentation files: delete both closing tags at
LiveObjects/Tests/AblyLiveObjectsTests/UTS/deviations.md lines 262-263, and
delete the closing content tag at
LiveObjects/Tests/AblyLiveObjectsTests/UTS/README.md line 32 and
Test/UTS/deviations.md line 35.
- Around line 56-65: Update the test references in deviations.md to match the
actual Swift Testing method names: remove the test_ prefix from
DEV5_object_operation_action_seven_distinct_cases and
objectData_holds_typed_values, and correct the similarly prefixed
InstanceUTSTests.test_RTINS14a... reference on line 126. Preserve the
surrounding DEV-5 and DEV-6 descriptions.
---
Outside diff comments:
In `@LiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift`:
- Around line 781-796: Update the documentation for InternalDefaultLiveMap.apply
to describe its LiveObjectUpdate<DefaultLiveMapUpdate>? result and
optional/enriched-update behavior, matching the contract documented by the
public nosync_apply wrapper. Remove the stale Bool and RTLM15g skipped-operation
wording.
---
Nitpick comments:
In `@LiveObjects/Sources/AblyLiveObjects/Internal/ChannelConfigGuards.swift`:
- Around line 40-43: Track the missing channel-modes accessor gap in an issue
covering both object_subscribe and object_publish accessors, and reference that
issue from the TODO associated with throwIfMissingObjectSubscribeMode so the
intentional no-op stub is not left without follow-up.
In
`@LiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift`:
- Around line 566-569: Update the action switch in the LiveCounter operation
handling to remove the default clause. List each known map-only
ObjectOperationAction explicitly with the existing warning-and-discard behavior,
and keep case .unknown separate, so newly added actions require explicit
handling at compile time.
- Around line 320-328: Remove the default value from sourceObjectMessage in
nosync_apply so callers must provide it. Update all sync-originated and test
call sites to pass nil explicitly, while ensuring channel-originated callers
pass the source public message required for emitted updates.
In `@LiveObjects/Sources/AblyLiveObjects/Internal/ObjectsPool.swift`:
- Around line 558-575: Add the queue-validated `nosync_isEmpty` property to
`PathObjectSubscriptionRegister`, returning whether its subscriptions collection
is empty. In `ObjectsPool.nosync_notifyPathSubscriptions`, check this property
and return before calling `nosync_getFullPaths`, while preserving the existing
path notification behavior when subscriptions exist.
In `@LiveObjects/Sources/AblyLiveObjects/Path` Based
API/Default/DefaultLiveCounterInstance.swift:
- Around line 55-72: Document the retention contract for subscribe in
DefaultLiveCounterInstance: state that callers must retain the
LiveCounterInstance while relying on the returned Subscription because the
listener captures self weakly and delivery stops once the instance is released.
Keep the existing weak capture and subscription behavior unchanged.
In `@LiveObjects/Sources/AblyLiveObjects/Path` Based
API/Default/DefaultLiveMapInstance.swift:
- Around line 233-238: Update objectCreator and both call sites in
internalValue(from:...) to propagate an ARTErrorInfo-compatible LiveObjectsError
instead of invoking preconditionFailure when the delegate is not
InternalDefaultRealtimeObjects. Mark objectCreator as throwing, adjust callers
to use try, and preserve the existing successful creator path.
In `@LiveObjects/Sources/AblyLiveObjects/Path` Based
API/Default/DefaultPathObject.swift:
- Around line 143-160: Make path resolution in resolveValueAtCurrentPath atomic
by executing the root read and entire PathSegments.parseStored(path) traversal
within one internalQueue block, using a queue-confined map accessor instead of
independently locking through mapNode.get. Preserve the existing empty-path,
non-map, missing-key, and successful-value behaviors; if atomic traversal is not
intended, document the best-effort consistency semantics near the method
instead.
In `@LiveObjects/Sources/AblyLiveObjects/Path` Based
API/Default/DefaultPrimitiveInstance.swift:
- Around line 51-64: Rename nosync_checkAccessPreconditionsOnQueue to
checkAccessPreconditions, reflecting that it synchronizes onto internalQueue via
ably_syncNoDeadlock. Update both call sites in DefaultPrimitiveInstance while
preserving the existing channel-state validation behavior.
In `@LiveObjects/Tests/AblyLiveObjectsTests/DefaultInstanceTests.swift`:
- Around line 267-270: Update the cycle-handling comment near the indirect A → B
→ A fixture to remove the claim that direct self-reference re-enters the mutex
or remains a pre-existing engine limitation. Reflect that direct
self-referencing entries are now supported and treated as normal entries, while
retaining the indirect-cycle explanation if still relevant.
In `@LiveObjects/Tests/AblyLiveObjectsTests/DefaultPathObjectTests.swift`:
- Around line 7-12: Update the file-level documentation near the PathSegments
and DefaultPathObject description to remove or reword the claim that path
subscriptions RTPO19/RTO24 still trap, reflecting that
PathObjectSubscriptionRegister and their tests are now implemented.
- Around line 388-395: Update nosync_publishAndApply to append objectMessages to
the existing _captured collection rather than replacing it, while preserving the
current mutex protection and success callback behavior.
In `@LiveObjects/Tests/AblyLiveObjectsTests/Mocks/MockRealtimeObjects.swift`:
- Around line 12-25: The mock’s nosync_pathObjectSubscriptionRegister currently
exposes an unusable register instead of failing clearly when accessed. Replace
the getter implementation with preconditionFailure, matching the existing
nosync_objectsPool unsupported-use pattern, and remove the unused
_pathObjectSubscriptionRegister initialization and stored property.
In `@LiveObjects/Tests/AblyLiveObjectsTests/ParentReferencesTests.swift`:
- Around line 5-8: Reconcile the ParentReferencesTests header with the existing
ParentReferencesUTSTests coverage: remove the claim that the UTS port is a
separate follow-up, and retain only native smoke cases not covered by the UTS
tests, such as clearParentReferences, to avoid duplicate behavior coverage.
In `@LiveObjects/Tests/AblyLiveObjectsTests/PublicRealtimeObjectTests.swift`:
- Around line 176-180: In the test around Self.driveToSynced, replace the fixed
Task.sleep delay with the existing deterministic main-queue drain pattern used
by RealtimeObjectUTSTests.drainMain(). Await one hop on the main callback queue
before asserting counter.isEmpty, preserving the assertion that no callback was
delivered.
In `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/ObjectIdUTSTests.swift`:
- Around line 41-51: Extract the duplicated `{type}:{hash}@{timestamp}` parsing
from `objectIdFormatCounter` and `hashIsBase64urlEncoded` into a shared
`parts(of:)` helper returning type, hash, and timestamp. Update both tests to
use the helper and change the padding assertion in `hashIsBase64urlEncoded` from
`hasSuffix("=")` to rejecting any `=` with `contains`.
In `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/ObjectsUTSHelpers.swift`:
- Around line 165-196: Deduplicate the shared publish holder and node factories
across the UTS helpers and DefaultInstanceTests. Promote one copy of
ObjectsUTSPublished/Published and ObjectsUTS.makeCounter/makeMap to an internal
shared helper, update both call sites to use it, and remove the redundant
private definitions while preserving their current behavior.
In
`@LiveObjects/Tests/AblyLiveObjectsTests/UTS/PathObjectSubscribeUTSTests.swift`:
- Around line 296-311: Replace the >= event-count assertions in
RTPO19c1_unlimited_depth and the corresponding MAP_CLEAR and two-apply tests
with exact expected counts, since applyAndDrain completes delivery
deterministically. Preserve the control-collector >= assertions used as
quiescence barriers; if MAP_CLEAR fan-out is intentionally variable, retain its
lower-bound assertion and add a brief explanation.
In `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/RealtimeObjectUTSTests.swift`:
- Line 239: Remove the inert swiftformat and swiftlint directives from the
`#expect` assertions in the affected test cases, leaving plain
`#expect`(counter.count == 0) assertions at both locations.
In `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/ValueTypesUTSTests.swift`:
- Line 59: Remove the inert swiftformat:disable and swiftlint:disable comments
from the count assertions at the referenced test locations, including the
assertion near vt.count and the corresponding one near line 114. Keep the
assertions themselves unchanged and maintain EditorConfig-only formatting.
In `@Package.swift`:
- Line 117: Remove the `.target(name: "_AblyPluginSupportPrivate")` dependency
from the `UTS` target definition in `Package.swift`, leaving the remaining UTS
dependencies unchanged.
🪄 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: d96b3b49-2c17-436c-b200-e9b8c10610b3
📒 Files selected for processing (65)
LiveObjects/Sources/AblyLiveObjects/Internal/ChannelConfigGuards.swiftLiveObjects/Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swiftLiveObjects/Sources/AblyLiveObjects/Internal/DefaultLiveCounterUpdate.swiftLiveObjects/Sources/AblyLiveObjects/Internal/DefaultLiveMapUpdate.swiftLiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swiftLiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swiftLiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swiftLiveObjects/Sources/AblyLiveObjects/Internal/InternalLiveObject.swiftLiveObjects/Sources/AblyLiveObjects/Internal/InternalTypes.swiftLiveObjects/Sources/AblyLiveObjects/Internal/LiveObjectMutableState.swiftLiveObjects/Sources/AblyLiveObjects/Internal/LiveObjectUpdate.swiftLiveObjects/Sources/AblyLiveObjects/Internal/ObjectDiffHelpers.swiftLiveObjects/Sources/AblyLiveObjects/Internal/ObjectsPool.swiftLiveObjects/Sources/AblyLiveObjects/Internal/PathObjectSubscriptionRegister.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultLiveCounterInstance.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultLiveCounterPathObject.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultLiveMapInstance.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultLiveMapPathObject.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultPathObject.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultPrimitiveInstance.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultPrimitivePathObject.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultStatusSubscription.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Default/DefaultSubscription.swiftLiveObjects/Sources/AblyLiveObjects/Path Based API/Public/PathObject.swiftLiveObjects/Sources/AblyLiveObjects/Protocol/ObjectMessage.swiftLiveObjects/Sources/AblyLiveObjects/Protocol/SyncCursor.swiftLiveObjects/Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultRealtimeObject.swiftLiveObjects/Sources/AblyLiveObjects/Utility/Errors.swiftLiveObjects/Sources/AblyLiveObjects/Utility/PathSegments.swiftLiveObjects/Tests/AblyLiveObjectsTests/DefaultInstanceTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/DefaultPathObjectTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/Helpers/UTSTestPoolFactories.swiftLiveObjects/Tests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/Mocks/MockRealtimeObjects.swiftLiveObjects/Tests/AblyLiveObjectsTests/ObjectDiffHelpersTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/ObjectsPoolTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/ParentReferencesTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/PathObjectSubscriptionTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/PublicRealtimeObjectTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/SyncCursorTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/TestsOnlySeamsTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/InstanceUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/InternalLiveCounterApiUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/InternalLiveCounterUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/InternalLiveMapApiUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/InternalLiveMapParentReferencesUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/InternalLiveMapUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/LiveObjectSubscribeUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/ObjectIdUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/ObjectsPoolUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/ObjectsUTSHelpers.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/ParentReferencesUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/PathObjectMutationsUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/PathObjectSubscribeUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/PathObjectUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/PublicObjectMessageUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/README.mdLiveObjects/Tests/AblyLiveObjectsTests/UTS/RealtimeObjectUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/ValueTypesUTSTests.swiftLiveObjects/Tests/AblyLiveObjectsTests/UTS/deviations.mdLiveObjects/Tests/AblyLiveObjectsTests/WireObjectMessageSizeTests.swiftPackage.swiftTest/UTS/deviations.md
| // OM3d: the string length of the JSON representation of `extras` (ably-java: UTF-16 length). | ||
| let extrasSize = extras.map { JSONObjectOrArray.object($0).toJSONString.utf16.count } ?? 0 |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
ably-java WireObjectMessage size() extras String.length byteSize
💡 Result:
In the Ably Java SDK, the WireObjectMessage.size() method is used to estimate the size of a wire-formatted message for protocol compliance, such as adhering to maximum message size limits [1][2]. Historically, internal implementations (including WireObjectMessage.kt) have faced challenges with accurate sizing when using .length on JSON-serialized strings, as this counts characters rather than bytes [1]. This approach can undercount payloads containing non-ASCII (multi-byte) characters, potentially leading to violations of byte-based protocol limits [1]. To ensure accurate sizing consistent with Ably's protocol requirements, it is recommended to replace character-based calculations with UTF-8 byte length calculations [1]. For the extras field, specifically, this involves converting the JSON-serialized representation to a UTF-8 byte array and retrieving its size [1]: extrasSize = extras?.let { gson.toJson(it).toByteArray(StandardCharsets.UTF_8).size }?: 0 While the general Ably platform documentation defines the size of extras as the string length of its JSON representation for billing purposes [3], internal SDK message sizing for pre-publish checks requires accurate byte-level calculation to prevent rejection by the server [1][2]. Developers should use byteSize (or equivalent UTF-8 byte length calculations) for fields to maintain coherence and correctly handle multi-byte characters [1][4].
Citations:
- 1: [AIT-928] Added basic impl. for PathObject and Instance interfaces ably-java#1217
- 2: Implement pre-publish checks using Message/PresenceMessage/Annotation size ably-java#1100
- 3: https://ably.com/docs/platform/pricing/message-counting
- 4: [DTP-1118] Apply
ConnectionDetails.maxMessageSizelimit to state messages ably-js#1963
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate ObjectMessage.swift =="
fd -a 'ObjectMessage\.swift$' . | sed 's#^\./##'
echo "== relevant lines =="
file="$(fd 'ObjectMessage\.swift$' . | head -n1)"
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '620,725p' "$file" | cat -n
fi
echo "== search maxMessageSize / ObjectMessage.size references =="
rg -n "ObjectMessage|extrasSize|ObjectsMap|uuid|byteSize|maxMessageSize|RTO15d|utf16|utf8" -S .Repository: ably/ably-cocoa
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ObjectMessage.swift size algorithm =="
sed -n '624,785p' LiveObjects/Sources/AblyLiveObjects/Protocol/ObjectMessage.swift | cat -n
echo "== maxMessageSize enforcement references =="
rg -n "maxMessageSize|RTO15|RTO15d|messageSizeLimit|ObjectMessage.*size|client\.publish|publish\(" LiveObjects/Sources LiveObjects/Tests -S --max-count 120
echo "== behavioral comparison for sample strings =="
python3 - <<'PY'
samples = ["abc", "é", "￥", "😀"]
for s in samples:
utf8 = len(s.encode("utf-8"))
utf16 = len(s)
print(f"{s!r}: utf8_bytes={utf8} utf16_code_units={utf16} diff={utf8-utf16}")
PYRepository: ably/ably-cocoa
Length of output: 16767
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== WireObjectMessageSizeTests relevant non-ASCII cases =="
sed -n '1,260p' LiveObjects/Tests/AblyLiveObjectsTests/WireObjectMessageSizeTests.swift | cat -n
echo "== core/rto enforcement slice =="
sed -n '580,665p' LiveObjects/Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift | cat -nRepository: ably/ably-cocoa
Length of output: 17126
🌐 Web query:
"ObjectMessageSizeTest" "extras" "utf16" "String.length" ably-java WireObjectMessage size
💡 Result:
In the ably-java SDK, historical implementations of WireObjectMessage.size() incorrectly used the String.length property of JSON-serialized extras [1]. Because String.length in Java/Kotlin returns the number of 16-bit char (UTF-16 code units) rather than the number of bytes in a UTF-8 encoding, this approach undercounted the size of payloads containing non-ASCII characters [1]. To ensure compliance with byte-based protocol limits, the calculation must use the UTF-8 byte count of the serialized JSON string [1]. The corrected approach, as seen in ably-java development, involves converting the GSON-serialized string to a ByteArray using StandardCharsets.UTF_8 and then retrieving the size of that array [1]: // Corrected size calculation approach: val extrasSize = extras?.let { gson.toJson(it).toByteArray(StandardCharsets.UTF_8).size }?: 0 This ensures that the extras component—which is used for metadata and ancillary payloads in messages—is accurately accounted for against the protocol's message size limits [1][2][3]. Relying on String.length remains a known issue or pitfall in legacy or non-compliant implementations because it fails to accurately represent the actual byte-size consumed on the wire [1].
Citations:
- 1: [AIT-928] Added basic impl. for PathObject and Instance interfaces ably-java#1217
- 2: https://ably.com/docs/sdk/java/v1.2/io/ably/lib/liveobjects/message/ObjectMessage.html
- 3: https://ably.com/docs/api/rest-sdk/messages
Use UTF-8 byte length for the size gate.
extrasSize and ObjectsMap entry key sizes currently use utf16.count, which undercounts non-ASCII text. This can let the RTO15d check pass for a message whose wire JSON is larger than the checked size. Count the JSON-serialized extras UTF-8 bytes here, and use UTF-8 byte length for ObjectsMap entry keys if the published limit still needs parity.
🤖 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 `@LiveObjects/Sources/AblyLiveObjects/Protocol/ObjectMessage.swift` around
lines 649 - 650, The extras size calculation in ObjectMessage must use the UTF-8
byte count of the serialized JSON instead of utf16.count. Update the relevant
ObjectsMap entry-key size calculation as well if it participates in the same
published limit, ensuring RTO15d validates actual wire JSON size for non-ASCII
content.
| // RTO27a1 requires the clear to emit no LiveObjectUpdate events. | ||
| // @specPartial RTO27a1 - asserts the "without emitting any LiveObjectUpdate events" clause | ||
| @Test | ||
| func clearOnDetachedEmitsNoUpdateEvents() async throws { | ||
| let internalQueue = TestFactories.createInternalQueue() | ||
| let realtimeObjects = InternalDefaultRealtimeObjectsTests.createDefaultRealtimeObjects(internalQueue: internalQueue) | ||
| Self.seedPopulatedPool(realtimeObjects, internalQueue: internalQueue) | ||
|
|
||
| let pool = realtimeObjects.testsOnly_objectsPool | ||
| let coreSDK = MockCoreSDK(channelState: .attached, internalQueue: internalQueue) | ||
|
|
||
| // Subscribe to root, the nested map and the counter. | ||
| let rootSubscriber = Subscriber<DefaultLiveMapUpdate, SubscribeResponse>(callbackQueue: .main) | ||
| let mapSubscriber = Subscriber<DefaultLiveMapUpdate, SubscribeResponse>(callbackQueue: .main) | ||
| let counterSubscriber = Subscriber<DefaultLiveCounterUpdate, SubscribeResponse>(callbackQueue: .main) | ||
| let childMap = try #require(pool.entries["map:child@1"]?.mapValue) | ||
| let childCounter = try #require(pool.entries["counter:child@2"]?.counterValue) | ||
| try pool.root.subscribe(listener: rootSubscriber.createListener(), coreSDK: coreSDK) | ||
| try childMap.subscribe(listener: mapSubscriber.createListener(), coreSDK: coreSDK) | ||
| try childCounter.subscribe(listener: counterSubscriber.createListener(), coreSDK: coreSDK) | ||
|
|
||
| // When the channel enters DETACHED. | ||
| internalQueue.ably_syncNoDeadlock { | ||
| realtimeObjects.nosync_onChannelStateChanged(toState: .detached, reason: nil) | ||
| } | ||
|
|
||
| // RTO27a1: no update events are emitted by the clear. | ||
| #expect(await rootSubscriber.getInvocations().isEmpty) | ||
| #expect(await mapSubscriber.getInvocations().isEmpty) | ||
| #expect(await counterSubscriber.getInvocations().isEmpty) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Drain the user callback queue before the negative assertion.
Subscriber delivers on .main, and nosync_onChannelStateChanged is invoked on the internal queue. The assertions at lines 2216-2218 read the invocation lists without any barrier on .main. If the implementation did emit update events, the callbacks could still be pending, so the test would pass for the wrong reason. Add a main-queue barrier, as emitsCorrectSyncEvents does at line 1867.
💚 Proposed fix
// When the channel enters DETACHED.
internalQueue.ably_syncNoDeadlock {
realtimeObjects.nosync_onChannelStateChanged(toState: .detached, reason: nil)
}
+ // Give any (unwanted) callbacks a chance to run on the user callback queue.
+ await Task { `@MainActor` in }.value
+
// RTO27a1: no update events are emitted by the clear.
`#expect`(await rootSubscriber.getInvocations().isEmpty)📝 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.
| // RTO27a1 requires the clear to emit no LiveObjectUpdate events. | |
| // @specPartial RTO27a1 - asserts the "without emitting any LiveObjectUpdate events" clause | |
| @Test | |
| func clearOnDetachedEmitsNoUpdateEvents() async throws { | |
| let internalQueue = TestFactories.createInternalQueue() | |
| let realtimeObjects = InternalDefaultRealtimeObjectsTests.createDefaultRealtimeObjects(internalQueue: internalQueue) | |
| Self.seedPopulatedPool(realtimeObjects, internalQueue: internalQueue) | |
| let pool = realtimeObjects.testsOnly_objectsPool | |
| let coreSDK = MockCoreSDK(channelState: .attached, internalQueue: internalQueue) | |
| // Subscribe to root, the nested map and the counter. | |
| let rootSubscriber = Subscriber<DefaultLiveMapUpdate, SubscribeResponse>(callbackQueue: .main) | |
| let mapSubscriber = Subscriber<DefaultLiveMapUpdate, SubscribeResponse>(callbackQueue: .main) | |
| let counterSubscriber = Subscriber<DefaultLiveCounterUpdate, SubscribeResponse>(callbackQueue: .main) | |
| let childMap = try #require(pool.entries["map:child@1"]?.mapValue) | |
| let childCounter = try #require(pool.entries["counter:child@2"]?.counterValue) | |
| try pool.root.subscribe(listener: rootSubscriber.createListener(), coreSDK: coreSDK) | |
| try childMap.subscribe(listener: mapSubscriber.createListener(), coreSDK: coreSDK) | |
| try childCounter.subscribe(listener: counterSubscriber.createListener(), coreSDK: coreSDK) | |
| // When the channel enters DETACHED. | |
| internalQueue.ably_syncNoDeadlock { | |
| realtimeObjects.nosync_onChannelStateChanged(toState: .detached, reason: nil) | |
| } | |
| // RTO27a1: no update events are emitted by the clear. | |
| #expect(await rootSubscriber.getInvocations().isEmpty) | |
| #expect(await mapSubscriber.getInvocations().isEmpty) | |
| #expect(await counterSubscriber.getInvocations().isEmpty) | |
| } | |
| // RTO27a1 requires the clear to emit no LiveObjectUpdate events. | |
| // `@specPartial` RTO27a1 - asserts the "without emitting any LiveObjectUpdate events" clause | |
| `@Test` | |
| func clearOnDetachedEmitsNoUpdateEvents() async throws { | |
| let internalQueue = TestFactories.createInternalQueue() | |
| let realtimeObjects = InternalDefaultRealtimeObjectsTests.createDefaultRealtimeObjects(internalQueue: internalQueue) | |
| Self.seedPopulatedPool(realtimeObjects, internalQueue: internalQueue) | |
| let pool = realtimeObjects.testsOnly_objectsPool | |
| let coreSDK = MockCoreSDK(channelState: .attached, internalQueue: internalQueue) | |
| // Subscribe to root, the nested map and the counter. | |
| let rootSubscriber = Subscriber<DefaultLiveMapUpdate, SubscribeResponse>(callbackQueue: .main) | |
| let mapSubscriber = Subscriber<DefaultLiveMapUpdate, SubscribeResponse>(callbackQueue: .main) | |
| let counterSubscriber = Subscriber<DefaultLiveCounterUpdate, SubscribeResponse>(callbackQueue: .main) | |
| let childMap = try `#require`(pool.entries["map:child@1"]?.mapValue) | |
| let childCounter = try `#require`(pool.entries["counter:child@2"]?.counterValue) | |
| try pool.root.subscribe(listener: rootSubscriber.createListener(), coreSDK: coreSDK) | |
| try childMap.subscribe(listener: mapSubscriber.createListener(), coreSDK: coreSDK) | |
| try childCounter.subscribe(listener: counterSubscriber.createListener(), coreSDK: coreSDK) | |
| // When the channel enters DETACHED. | |
| internalQueue.ably_syncNoDeadlock { | |
| realtimeObjects.nosync_onChannelStateChanged(toState: .detached, reason: nil) | |
| } | |
| // Give any (unwanted) callbacks a chance to run on the user callback queue. | |
| await Task { `@MainActor` in }.value | |
| // RTO27a1: no update events are emitted by the clear. | |
| `#expect`(await rootSubscriber.getInvocations().isEmpty) | |
| `#expect`(await mapSubscriber.getInvocations().isEmpty) | |
| `#expect`(await counterSubscriber.getInvocations().isEmpty) | |
| } |
🤖 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
`@LiveObjects/Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift`
around lines 2189 - 2219, Update clearOnDetachedEmitsNoUpdateEvents to drain the
main callback queue before checking subscriber invocations: after triggering
nosync_onChannelStateChanged, add the same main-queue synchronization/barrier
pattern used by emitsCorrectSyncEvents, then retain the existing empty
assertions for rootSubscriber, mapSubscriber, and counterSubscriber.
| // Cycle suppression: add map:l as a parent of map:m via map:m, then create a back-edge | ||
| // map:l -> map:m so that the graph contains a cycle; getFullPaths must still terminate. | ||
| internalQueue.ably_syncNoDeadlock { | ||
| mapL.nosync_addParentReference(parentObjectID: "map:m@1000", key: "loop") | ||
| } | ||
| // map:l is reachable from root at ["left"]; the cycle back through map:m is suppressed. | ||
| let mapLPaths = mapL.testsOnly_getFullPaths(objectsPool: pool) | ||
| #expect(mapLPaths.contains(["left"])) | ||
| #expect(!mapLPaths.isEmpty) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Strengthen the cycle assertions and correct the comment.
The comment describes adding map:l as a parent of map:m, but line 150 adds map:m as a parent of map:l. The assertions also only prove termination: line 155 is implied by line 154, and neither rejects duplicate or extra paths. UTS/ParentReferencesUTSTests.swift (lines 348-354) asserts an exact path count for the same behaviour. Assert the count here too, so a regression that emits repeated paths fails.
💚 Proposed fix
- // Cycle suppression: add map:l as a parent of map:m via map:m, then create a back-edge
- // map:l -> map:m so that the graph contains a cycle; getFullPaths must still terminate.
+ // Cycle suppression: give map:l a back-edge parent of map:m, so the graph map:l -> map:m
+ // -> map:l contains a cycle; getFullPaths must terminate and emit each path once.
internalQueue.ably_syncNoDeadlock {
mapL.nosync_addParentReference(parentObjectID: "map:m@1000", key: "loop")
}
// map:l is reachable from root at ["left"]; the cycle back through map:m is suppressed.
let mapLPaths = mapL.testsOnly_getFullPaths(objectsPool: pool)
+ `#expect`(mapLPaths.count == 1)
`#expect`(mapLPaths.contains(["left"]))
- `#expect`(!mapLPaths.isEmpty)📝 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.
| // Cycle suppression: add map:l as a parent of map:m via map:m, then create a back-edge | |
| // map:l -> map:m so that the graph contains a cycle; getFullPaths must still terminate. | |
| internalQueue.ably_syncNoDeadlock { | |
| mapL.nosync_addParentReference(parentObjectID: "map:m@1000", key: "loop") | |
| } | |
| // map:l is reachable from root at ["left"]; the cycle back through map:m is suppressed. | |
| let mapLPaths = mapL.testsOnly_getFullPaths(objectsPool: pool) | |
| #expect(mapLPaths.contains(["left"])) | |
| #expect(!mapLPaths.isEmpty) | |
| // Cycle suppression: give map:l a back-edge parent of map:m, so the graph map:l -> map:m | |
| // -> map:l contains a cycle; getFullPaths must terminate and emit each path once. | |
| internalQueue.ably_syncNoDeadlock { | |
| mapL.nosync_addParentReference(parentObjectID: "map:m@1000", key: "loop") | |
| } | |
| // map:l is reachable from root at ["left"]; the cycle back through map:m is suppressed. | |
| let mapLPaths = mapL.testsOnly_getFullPaths(objectsPool: pool) | |
| `#expect`(mapLPaths.count == 1) | |
| `#expect`(mapLPaths.contains(["left"])) |
🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/ParentReferencesTests.swift` around
lines 147 - 155, Correct the cycle-suppression comment to match the parent
relationship created by nosync_addParentReference, then strengthen the mapL
cycle test around testsOnly_getFullPaths by asserting the exact expected path
count, following the corresponding UTS/ParentReferencesUTSTests behavior. Remove
the redundant non-empty assertion while retaining the expected ["left"] path
check.
| - **DEV-5** — Public enums drop `UNKNOWN`. `ObjectOperationAction` has exactly 7 cases (`.mapCreate`, | ||
| `.mapSet`, `.mapRemove`, `.counterCreate`, `.counterInc`, `.objectDelete`, `.mapClear`) and | ||
| `ObjectsMapSemantics` has only `.lww`. The spec's `action == "MAP_SET"` maps to `.mapSet`, etc. | ||
| Unknown wire codes are held internally by `WireEnum.unknown` and never surface publicly. Asserted by | ||
| `test_DEV5_object_operation_action_seven_distinct_cases`. | ||
| - **DEV-6** — `ObjectData` shape. The public `ObjectData` adds a Swift-only `encoding: String?` (no | ||
| wire/Java counterpart), exposes `json` as a raw `String?` (vs Java's parsed `JsonElement`), and uses | ||
| non-optional `Double` for `CounterCreate.count` / `CounterInc.number` (`number` on `ObjectData` is | ||
| `Double?`). Asserted by `test_ObjectData_holds_typed_values`, `test_PAOOP2_counter_inc_only_relevant_field` | ||
| and `test_PAOOP2_counter_create_with_count`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the referenced test-method names.
The document names tests with a test_ prefix, for example test_DEV5_object_operation_action_seven_distinct_cases and test_ObjectData_holds_typed_values. The Swift Testing methods in UTS/PublicObjectMessageUTSTests.swift are DEV5_object_operation_action_seven_distinct_cases and objectData_holds_typed_values. Line 126 has the same problem with InstanceUTSTests.test_RTINS14a.... Update the names so readers can locate the tests.
🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/deviations.md` around lines 56 -
65, Update the test references in deviations.md to match the actual Swift
Testing method names: remove the test_ prefix from
DEV5_object_operation_action_seven_distinct_cases and
objectData_holds_typed_values, and correct the similarly prefixed
InstanceUTSTests.test_RTINS14a... reference on line 126. Preserve the
surrounding DEV-5 and DEV-6 descriptions.
| </content> | ||
| </invoke> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Generated Markdown files contain leaked closing tags. Three documentation files end with tool artifacts instead of content.
LiveObjects/Tests/AblyLiveObjectsTests/UTS/deviations.md#L262-L263: delete the</content>and</invoke>lines.LiveObjects/Tests/AblyLiveObjectsTests/UTS/README.md#L32: delete the</content>line.Test/UTS/deviations.md#L35: delete the</content>line.
📍 Affects 3 files
LiveObjects/Tests/AblyLiveObjectsTests/UTS/deviations.md#L262-L263(this comment)LiveObjects/Tests/AblyLiveObjectsTests/UTS/README.md#L32-L32Test/UTS/deviations.md#L35-L35
🤖 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 `@LiveObjects/Tests/AblyLiveObjectsTests/UTS/deviations.md` around lines 262 -
263, Remove the leaked tool-artifact closing tags from all affected
documentation files: delete both closing tags at
LiveObjects/Tests/AblyLiveObjectsTests/UTS/deviations.md lines 262-263, and
delete the closing content tag at
LiveObjects/Tests/AblyLiveObjectsTests/UTS/README.md line 32 and
Test/UTS/deviations.md line 35.
…integration testing Implements the remaining core-accessor-gated functionality, ported from ably-java, after enabling the LiveObjects integration test suites: - RTL33 implicit attach: new plugin-API attach primitive (APPluginAPI nosync_attachChannel:) wired through CoreSDK into RealtimeObject.get() - full RTL33a/b/b1/c state handling - Real maxMessageSize on the RTO15d publish size gate (65536 fallback) - Real channel name threaded to the public ObjectMessage (PAOM3) - Channel-config guards fully implemented: object channel modes (40024), echoMessages, connection-active checks - no core-accessor stubs remain - Channel-release SPI (nosync_releaseChannel:) so releasing a channel fails in-flight waiters with a release-specific cause - Fix: engine deinit on the internal queue no longer trips the DispatchQueueMutex precondition (SIGTRAP during ARC-driven teardown) - Fix: siteCode from connectionDetails is now seeded into engines created after CONNECTED, restoring RTO20 local echo All changes are covered by new native tests. Verified end-to-end by the LiveObjects integration suites (17/17 against the Ably sandbox).
1 participant
Summary
This PR completes the LiveObjects implementation behind the previously shipped path-based public API, using ably-java's Kotlin implementation as the reference, and ports all 15 UTS
objects/unitspecs as its acceptance tests.The public API surface is byte-identical to what was already shipped — every change is additive behind the frozen protocols. Zero changes to the core SDK (
Source/),Cartfile, orAbly.podspec: the plugin remains SPM-only.What's implemented
Public surface (previously trapping via
notImplemented()— now zero trap sites)LiveMapInstance/LiveCounterInstance/PrimitiveInstance— identity-addressed,compactJson()with cycle markers (RTPO14b2), mutations via the RTO20 publish path, enriched subscription eventsPathObjectbase + typed path objects — stored dot-delimited path with per-call resolution (RTPO3),PathSegmentsescaping,exists()/type()/as*casts, reads and writes with 92005/92007 semanticsPathObjectSubscriptionRegisterwith RTO24 depth-windowed coverage ranking, driven bygetFullPaths(); both operation-driven and sync-originated updates fan out (nilmessageon sync, RTO4b2a)get()(guards → active-channel check → lost-wakeup-safe sync wait → rootLiveMapPathObject, RTO23),on()/off()status events (RTO17/18),dispose()lifecycleEngine additions
getFullPaths(), wired into all map mutation paths, RTO5c10 post-sync rebuild.ObjectMessage?and atombstoneflag; PAOM3 wire→public conversion; tombstone updates tear down instance subscriptions (RTLO4b4c3c).throwIf*, RTO25/26 — implementable subset; core-accessor gaps carryTODO(core accessor)markers) and the plugin-local error model (92005/92007/40024/40019/40003 as raw codes).Spec-compliance fixes (found by the ported spec tests)
14 pre-existing bugs fixed, including: RTLO4e10 root-tombstone protection; six Swift exclusive-access crash guards (self-referencing entries arriving over the wire could crash the client); RTLM24c equal-serial tie-break; RTLM22-consistent tombstone teardown diffs; empty/zero diffs collapsed to no-op (no spurious subscriber events); RTO9a2b/RTO9a3 checks hoisted before zero-value creation; RTO5a malformed sync-cursor handling (previously aborted the whole sync; now falls back to the RTO5a5 self-contained path).
Testing
objects/unitspecs ported toLiveObjects/Tests/AblyLiveObjectsTests/UTS/(16 files,<Name>UTSTestsnaming, per-case// @UTStraceability tags) — mirroring ably-java's consolidateduts/unit/layout. AREADME.mdanddeviations.mdaccompany them.test-librarygate; lint clean; availability annotations verified.Deviations & upstream notes
Every intentional divergence from ably-java or the spec is recorded — in the per-file test headers,
LiveObjects/Tests/AblyLiveObjectsTests/UTS/deviations.md, and the port's deviation register. Highlights for reviewers:createOperationIsMerged; RTLM23/RTLC16derivedFromprecedence) — kept spec-compliant; a spec clarification is recommended.getFullPaths, engine-level path fan-out) exist because Kotlin's in-object designs trip Swift's exclusive-access checker — observable behaviour is identical.maxMessageSize, channel modes/echo, attach initiation, channel-release dispose SPI) and are marked withTODO(core accessor)in code.Reviewer notes
Internal/LiveObjectMutableState+InternalLiveObject(graph) →LiveObjectUpdate/ObjectMessage(enrichment) →Path Based API/Default/(instances, path objects) →PathObjectSubscriptionRegister→PublicDefaultRealtimeObject.testsOnly_members are test seams (established convention);nosync_marks on-queue entry points.Test/UTStarget change is a trim only: objects deviations moved next to the consolidated tests; the shared rest/realtime harness is untouched.Summary by CodeRabbit