[ECO-5435] Implement the write spec #51
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change implements the full write API for LiveObjects in Swift, including creation, mutation, and message publishing for live counters and maps. It introduces new helper utilities for object creation, updates internal and public APIs to support the new flows, refines protocol encoding/decoding, and adds comprehensive unit and integration tests for all major write operations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PublicRealtimeObjects
participant InternalRealtimeObjects
participant ObjectCreationHelpers
participant CoreSDK
participant ObjectsPool
Client->>PublicRealtimeObjects: createMap(entries)
PublicRealtimeObjects->>InternalRealtimeObjects: createMap(entries, coreSDK)
InternalRealtimeObjects->>ObjectCreationHelpers: creationOperationForLiveMap(entries, timestamp)
ObjectCreationHelpers-->>InternalRealtimeObjects: MapCreationOperation (objectID, op, message)
InternalRealtimeObjects->>CoreSDK: publish([OutboundObjectMessage])
CoreSDK-->>InternalRealtimeObjects: (async)
InternalRealtimeObjects->>ObjectsPool: getOrCreateMap(MapCreationOperation, ...)
ObjectsPool-->>InternalRealtimeObjects: InternalDefaultLiveMap
InternalRealtimeObjects-->>PublicRealtimeObjects: InternalDefaultLiveMap
PublicRealtimeObjects->>Client: PublicDefaultLiveMap
sequenceDiagram
participant Client
participant PublicDefaultLiveMap
participant InternalDefaultLiveMap
participant CoreSDK
Client->>PublicDefaultLiveMap: set(key, value)
PublicDefaultLiveMap->>InternalDefaultLiveMap: set(key, InternalLiveMapValue, coreSDK)
InternalDefaultLiveMap->>CoreSDK: publish([OutboundObjectMessage])
CoreSDK-->>InternalDefaultLiveMap: (async)
InternalDefaultLiveMap-->>PublicDefaultLiveMap: (success or error)
PublicDefaultLiveMap-->>Client: (success or error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
6859050 to
073b104
Compare
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
lawrence-forooghian
changed the base branch from
44-JSON-map-entries-support
to
ECO-5461-deletions
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
073b104 to
77e1072
Compare
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
77e1072 to
e0edc33
Compare
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
e0edc33 to
6d189a3
Compare
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
6d189a3 to
fea84af
Compare
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
fea84af to
cd8514e
Compare
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
lawrence-forooghian
force-pushed
the
ECO-5461-deletions
branch
from
1badcec to
fe7503c
Compare
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
cd8514e to
00a9b11
Compare
lawrence-forooghian
changed the title
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
00a9b11 to
c9b2a82
Compare
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
c9b2a82 to
48e6937
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
github-actions
bot
temporarily deployed
to
staging/pull/51/AblyLiveObjects
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
Sources/AblyLiveObjects/Internal/ObjectCreationHelpers.swift (1)
218-222: Consider safer nonce generation implementation.While the current implementation works, the force unwrap could be avoided for better safety:
- return String((0 ..< 16).map { _ in letters.randomElement()! }) + return String((0 ..< 16).compactMap { _ in letters.randomElement() })Also, the TODO comment indicates specification clarification is needed for nonce requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.cursor/rules/swift.mdc(1 hunks)Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift(1 hunks)Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift(1 hunks)Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift(1 hunks)Sources/AblyLiveObjects/Internal/InternalLiveMapValue.swift(1 hunks)Sources/AblyLiveObjects/Internal/InternalObjectsMapEntry.swift(1 hunks)Sources/AblyLiveObjects/Internal/ObjectCreationHelpers.swift(1 hunks)Sources/AblyLiveObjects/Internal/ObjectsPool.swift(1 hunks)Sources/AblyLiveObjects/Protocol/ObjectMessage.swift(7 hunks)Sources/AblyLiveObjects/Protocol/WireObjectMessage.swift(6 hunks)Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveCounter.swift(1 hunks)Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift(1 hunks)Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultRealtimeObjects.swift(1 hunks)Sources/AblyLiveObjects/Utility/Errors.swift(2 hunks)Sources/AblyLiveObjects/Utility/JSONValue.swift(1 hunks)Tests/AblyLiveObjectsTests/AblyLiveObjectsTests.swift(1 hunks)Tests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swift(1 hunks)Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift(1 hunks)Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift(2 hunks)Tests/AblyLiveObjectsTests/Mocks/MockCoreSDK.swift(2 hunks)Tests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swift(1 hunks)
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/specification.mdc:0-0
Timestamp: 2025-07-29T08:07:37.861Z
Learning: The LiveObjects functionality is referred to in the Specification simply as 'Objects'.
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to AblyLiveObjects/**/!(*Test|*Tests).swift : When importing the Ably module inside the AblyLiveObjects library code (non-test code), use `import Ably`.
📚 Learning: applies to **/*.swift : when writing jsonvalue or wirevalue types, favour using the literal syntax e...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to **/*.swift : When writing JSONValue or WireValue types, favour using the literal syntax enabled by their conformance to the `ExpressibleBy*Literal` protocols where possible.
Applied to files:
.cursor/rules/swift.mdcSources/AblyLiveObjects/Utility/JSONValue.swiftSources/AblyLiveObjects/Protocol/WireObjectMessage.swift
📚 Learning: applies to **/!(*test|*tests).swift : satisfy swiftlint's `explicit_acl` rule: all declarations shou...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to **/!(*Test|*Tests).swift : Satisfy SwiftLint's `explicit_acl` rule: all declarations should specify Access Control Level keywords explicitly.
Applied to files:
.cursor/rules/swift.mdc
📚 Learning: applies to **/*.swift : when writing an array literal that starts with an initializer expression, st...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to **/*.swift : When writing an array literal that starts with an initializer expression, start the initializer expression on the line after the opening square bracket of the array literal.
Applied to files:
.cursor/rules/swift.mdc
📚 Learning: applies to ablyliveobjects/**/!(*test|*tests).swift : when importing the ably module inside the ably...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to AblyLiveObjects/**/!(*Test|*Tests).swift : When importing the Ably module inside the AblyLiveObjects library code (non-test code), use `import Ably`.
Applied to files:
.cursor/rules/swift.mdcSources/AblyLiveObjects/Internal/InternalObjectsMapEntry.swiftSources/AblyLiveObjects/Utility/Errors.swiftSources/AblyLiveObjects/Internal/InternalLiveMapValue.swiftTests/AblyLiveObjectsTests/AblyLiveObjectsTests.swiftSources/AblyLiveObjects/Protocol/ObjectMessage.swiftSources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swiftTests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swiftTests/AblyLiveObjectsTests/Mocks/MockCoreSDK.swiftSources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swiftTests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftSources/AblyLiveObjects/Internal/ObjectCreationHelpers.swift
📚 Learning: applies to **/tests/**/*.swift : when you need to import the following modules in the tests, do so i...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:07:58.385Z
Learning: Applies to **/Tests/**/*.swift : When you need to import the following modules in the tests, do so in the following way: Ably: use `import Ably`; AblyLiveObjects: use `@testable import AblyLiveObjects`; AblyPlugin: use `import AblyPlugin`; do not do `internal import`.
Applied to files:
.cursor/rules/swift.mdcSources/AblyLiveObjects/Internal/InternalObjectsMapEntry.swiftSources/AblyLiveObjects/Internal/InternalLiveMapValue.swiftTests/AblyLiveObjectsTests/AblyLiveObjectsTests.swiftSources/AblyLiveObjects/Protocol/ObjectMessage.swiftTests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swiftTests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftSources/AblyLiveObjects/Internal/ObjectCreationHelpers.swift
📚 Learning: applies to ablyliveobjects/**/!(*test|*tests).swift : when importing the ablyplugin module inside th...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to AblyLiveObjects/**/!(*Test|*Tests).swift : When importing the AblyPlugin module inside the AblyLiveObjects library code (non-test code), use `internal import AblyPlugin`.
Applied to files:
.cursor/rules/swift.mdcSources/AblyLiveObjects/Internal/InternalObjectsMapEntry.swiftSources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swiftSources/AblyLiveObjects/Internal/InternalLiveMapValue.swiftTests/AblyLiveObjectsTests/AblyLiveObjectsTests.swiftSources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swiftSources/AblyLiveObjects/Protocol/ObjectMessage.swiftSources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swiftSources/AblyLiveObjects/Internal/ObjectsPool.swiftTests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swiftTests/AblyLiveObjectsTests/Mocks/MockCoreSDK.swiftSources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swiftTests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftSources/AblyLiveObjects/Internal/ObjectCreationHelpers.swift
📚 Learning: applies to **/*.swift : when writing initializer expressions, when the type that is being initialize...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to **/*.swift : When writing initializer expressions, when the type that is being initialized can be inferred, favour using the implicit `.init(…)` form instead of explicitly writing the type name.
Applied to files:
.cursor/rules/swift.mdc
📚 Learning: applies to **/*.swift : when writing enum value expressions, when the type that is being initialized...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to **/*.swift : When writing enum value expressions, when the type that is being initialized can be inferred, favour using the implicit `.caseName` form instead of explicitly writing the type name.
Applied to files:
.cursor/rules/swift.mdc
📚 Learning: applies to **/!(*test|*tests).swift : when writing an `extension` of a type, favour placing the acce...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-07-29T08:07:47.875Z
Learning: Applies to **/!(*Test|*Tests).swift : When writing an `extension` of a type, favour placing the access level on the declaration of the extension rather than each of its individual members.
Applied to files:
.cursor/rules/swift.mdc
📚 Learning: applies to **/tests/**/*.swift : when you need to unwrap an optional value in the tests, favour usin...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:07:58.385Z
Learning: Applies to **/Tests/**/*.swift : When you need to unwrap an optional value in the tests, favour using `#require` instead of `guard let`.
Applied to files:
.cursor/rules/swift.mdcTests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swiftTests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift
📚 Learning: applies to **/tests/**/*.swift : when creating `testsonly_` property declarations, do not write gene...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : When creating `testsOnly_` property declarations, do not write generic comments of the form 'Test-only access to the private createOperationIsMerged property'; the meaning of these properties is already well understood.
Applied to files:
Tests/AblyLiveObjectsTests/AblyLiveObjectsTests.swiftTests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swiftTests/AblyLiveObjectsTests/Mocks/MockCoreSDK.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swiftTests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift
📚 Learning: applies to **/tests/**/*.swift : use the swift testing framework (`import testing`), not xctest....
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : Use the Swift Testing framework (`import Testing`), not XCTest.
Applied to files:
Tests/AblyLiveObjectsTests/AblyLiveObjectsTests.swift
📚 Learning: applies to **/tests/**/*.swift : do not use `fatalerror` in response to a test expectation failure. ...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : Do not use `fatalError` in response to a test expectation failure. Favour the usage of Swift Testing's `#require` macro.
Applied to files:
Tests/AblyLiveObjectsTests/AblyLiveObjectsTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swiftTests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift
📚 Learning: the liveobjects functionality is referred to in the specification simply as 'objects'....
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/specification.mdc:0-0
Timestamp: 2025-07-29T08:07:37.861Z
Learning: The LiveObjects functionality is referred to in the Specification simply as 'Objects'.
Applied to files:
Sources/AblyLiveObjects/Protocol/ObjectMessage.swiftSources/AblyLiveObjects/Internal/ObjectCreationHelpers.swift
📚 Learning: applies to **/tests/**/*.swift : follow the guidelines given under 'attributing tests to a spec poin...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : Follow the guidelines given under 'Attributing tests to a spec point' in the file `CONTRIBUTING.md` in order to tag the unit tests with the relevant specification points. Make sure to follow the exact format of the comments as described in that file. Pay particular attention to the difference between the meaning of `@spec` and `@specPartial` and be sure not to write `@spec` multiple times for the same specification point.
Applied to files:
Tests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift
📚 Learning: applies to **/tests/**/*.swift : when writing tests, follow the guidelines given under 'attributing ...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:07:58.385Z
Learning: Applies to **/Tests/**/*.swift : When writing tests, follow the guidelines given under 'Attributing tests to a spec point' in the file `CONTRIBUTING.md` in order to tag the unit tests with the relevant specification points. Make sure to follow the exact format of the comments as described in that file. Pay particular attention to the difference between the meaning of `@spec` and `@specPartial` and be sure not to write `@spec` multiple times for the same specification point.
Applied to files:
Tests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift
📚 Learning: applies to **/tests/**/*.swift : when writing tests, make sure to add comments that explain when som...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:07:58.385Z
Learning: Applies to **/Tests/**/*.swift : When writing tests, make sure to add comments that explain when some piece of test data is not important for the scenario being tested.
Applied to files:
Tests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swiftTests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift
📚 Learning: applies to **/tests/**/*.swift : only add labels to test cases or suites when the label is different...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : Only add labels to test cases or suites when the label is different to the name of the suite `struct` or test method.
Applied to files:
Tests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swift
📚 Learning: applies to **/tests/**/*.swift : add comments that explain when some piece of test data is not impor...
Learnt from: CR
PR: ably/ably-cocoa-liveobjects-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-29T08:08:00.588Z
Learning: Applies to **/Tests/**/*.swift : Add comments that explain when some piece of test data is not important for the scenario being tested.
Applied to files:
Tests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swiftTests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift
🧬 Code Graph Analysis (5)
Sources/AblyLiveObjects/Utility/Errors.swift (1)
Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift (1)
value(82-90)
Sources/AblyLiveObjects/Internal/InternalLiveMapValue.swift (1)
Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift (1)
value(82-90)
Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift (3)
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveCounter.swift (2)
increment(29-31)decrement(33-35)Sources/AblyLiveObjects/Internal/CoreSDK.swift (2)
validateChannelState(56-68)publish(32-39)Tests/AblyLiveObjectsTests/Helpers/TestFactories.swift (1)
counterOp(386-388)
Tests/AblyLiveObjectsTests/Mocks/MockCoreSDK.swift (2)
Sources/AblyLiveObjects/Internal/CoreSDK.swift (1)
publish(32-39)Tests/AblyLiveObjectsTests/Helpers/Assertions.swift (1)
protocolRequirementNotImplemented(2-7)
Sources/AblyLiveObjects/Internal/ObjectCreationHelpers.swift (1)
Tests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swift (1)
createObjectID(201-214)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Example app, iOS (Xcode 16.3)
- GitHub Check: Example app, tvOS (Xcode 16.3)
- GitHub Check: Example app, macOS (Xcode 16.3)
- GitHub Check: Xcode,
releaseconfiguration, macOS (Xcode 16.3) - GitHub Check: Xcode,
releaseconfiguration, tvOS (Xcode 16.3) - GitHub Check: Xcode,
releaseconfiguration, iOS (Xcode 16.3) - GitHub Check: Xcode, tvOS (Xcode 16.3)
- GitHub Check: Xcode, macOS (Xcode 16.3)
- GitHub Check: Xcode, iOS (Xcode 16.3)
- GitHub Check: SPM,
releaseconfiguration (Xcode 16.3) - GitHub Check: SPM (Xcode 16.3)
- GitHub Check: Generate code coverage
🔇 Additional comments (56)
.cursor/rules/swift.mdc (1)
14-14: LGTM! Excellent coding guideline addition.Using Swift raw string literals for JSON strings improves readability and reduces escaping errors, which is particularly valuable for the LiveObjects implementation that involves extensive JSON handling.
Sources/AblyLiveObjects/Internal/InternalObjectsMapEntry.swift (1)
4-4: LGTM! Appropriate protocol conformance addition.Adding
Equatableconformance enables equality comparisons forInternalObjectsMapEntryinstances, which is beneficial for the LiveObjects write operations. Swift will automatically synthesize the implementation since all properties areEquatable.Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveCounter.swift (2)
30-30: LGTM! Correct API alignment.The addition of the
coreSDKparameter correctly aligns with the updated internal method signature for write operations.
34-34: LGTM! Correct API alignment.The addition of the
coreSDKparameter correctly aligns with the updated internal method signature for write operations.Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift (2)
79-81: LGTM! Correct implementation of public-to-internal value conversion.The conversion from public
LiveMapValuetoInternalLiveMapValueand the addition of thecoreSDKparameter correctly enable write operations while maintaining API separation.
85-85: LGTM! Correct API alignment.The addition of the
coreSDKparameter correctly aligns with the updated internal method signature for write operations.Sources/AblyLiveObjects/Utility/JSONValue.swift (1)
181-197: LGTM! Well-implemented convenience accessors.The new computed properties provide clean, convenient access to associated values following standard Swift patterns. The implementation correctly returns the associated value or
nilusing pattern matching, improving code readability for JSON handling operations.Sources/AblyLiveObjects/Utility/Errors.swift (4)
9-10: LGTM! Well-structured error cases for counter validation.The new error cases follow the established pattern and provide clear validation feedback for invalid counter values and increment amounts.
17-19: Good adherence to specification requirements.The error code mapping correctly references the specification requirements (RTO12f1, RTLC12e1) and uses the appropriate
.invalidParameterValueerror code.
26-29: Consistent HTTP status code mapping.The 400 status code is appropriate for invalid parameter validation errors and maintains consistency with the existing error case.
38-41: Clear and descriptive error messages.The error messages clearly indicate the validation requirement (finite numbers) and include the invalid value for debugging purposes.
Sources/AblyLiveObjects/Internal/InternalLiveMapValue.swift (2)
14-31: LGTM! Conversion logic is sound despite runtime checks.The initializer correctly converts public types to internal representations. While the runtime type checks aren't ideal, they're appropriately acknowledged with TODO comments referencing issue #37 for future improvement.
36-61: Excellent specification compliance for protocol conversion.The
toObjectDataproperty correctly implements the protocol requirements (RTO11f4 and RTLM20e4) for converting internal values to wire format. The handling of primitive types and object references is thorough and accurate.Tests/AblyLiveObjectsTests/Mocks/MockCoreSDK.swift (3)
9-9: LGTM! Consistent with existing mutable state patterns.The new
_publishHandlerproperty follows the samenonisolated(unsafe)pattern as the existing_channelStateproperty.
15-21: Good fallback behavior for testing flexibility.The publish method appropriately uses the injected handler when available and falls back to the existing
protocolRequirementNotImplemented()behavior, maintaining backwards compatibility.
36-41: Proper thread-safe handler injection.The
setPublishHandlermethod correctly uses mutex protection to safely set the handler, consistent with the existing thread-safety patterns in this mock class.Tests/AblyLiveObjectsTests/AblyLiveObjectsTests.swift (1)
126-187: Excellent comprehensive smoke test of the write API.This test effectively exercises the complete public API workflow including:
- Object creation with initial values
- Subscription to updates
- Various mutation operations (set, increment, remove)
- Verification of both state changes and update events
The test structure is well-organized and properly validates the asynchronous, event-driven nature of the LiveObjects system.
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultRealtimeObjects.swift (4)
38-50: LGTM! Proper conversion and wrapping for map creation with entries.The method correctly converts public
LiveMapValueentries to internal representations using the new initializer, passes thecoreSDKparameter, and wraps the result with the appropriate public proxy.
52-63: Consistent pattern for parameterless map creation.The method follows the same pattern as the entries version, maintaining consistency in the API implementation.
65-75: Proper counter creation with initial value.The method correctly passes the count parameter and
coreSDKto the internal implementation and wraps the result appropriately.
77-87: Complete and consistent counter creation pattern.The parameterless counter creation follows the established pattern, completing the set of creation methods with consistent implementation.
Tests/AblyLiveObjectsTests/InternalDefaultLiveCounterTests.swift (5)
426-444: LGTM! Comprehensive channel state validation testing.The test correctly validates that
incrementthrows appropriate errors for invalid channel states (.detached, .failed, .suspended) as specified in RTLC12c. The error code validation (90001) and status code (400) are accurate.
446-466: LGTM! Thorough validation of invalid amount handling.The test properly covers RTLC12e1 by testing all non-finite values (NaN, infinity, -infinity) and validates the correct error code (40003) and status code (400).
468-497: LGTM! Comprehensive outbound message validation.The test effectively validates all aspects of the published message per RTLC12e2-e4 and RTLC12f:
- Correct action (.counterInc)
- Correct objectId propagation
- Correct amount encoding as NSNumber
- Single message published
The use of MockCoreSDK with custom publish handler is well implemented.
499-517: LGTM! Proper error propagation testing.The test correctly validates that publish failures are properly converted and propagated as ARTErrorInfo, ensuring robust error handling in the increment operation.
521-540: LGTM! Appropriate smoke test for decrement operation.The test correctly validates RTLC13b by confirming that
decrementbehaves as the negation ofincrement. The assertion that the published amount is-10.5(negated from the input10.5) effectively demonstrates the opposite behavior. The approach of treating this as a smoke test is sensible since the underlying increment logic is tested elsewhere.Sources/AblyLiveObjects/Protocol/WireObjectMessage.swift (4)
160-160: LGTM! Accurate comment update.The comment correctly reflects that
PartialWireObjectOperationnow excludes bothactionandobjectIdproperties, which aligns with its usage for encoding initial values.
232-270: LGTM! Well-structured refactoring of action handling.The separation of concerns is well-implemented:
WireObjectOperationnow explicitly handlesactionandobjectIddecoding/encoding- Proper delegation to
PartialWireObjectOperationfor common fields- Clean separation maintains functionality while improving code organization
The encoding logic correctly adds the action field after encoding the partial operation.
355-355: LGTM! Appropriate Equatable conformance.Adding
Equatableconformance toWireObjectsCounterOpenables equality comparisons which are valuable for testing and validation purposes.
409-409: LGTM! Appropriate Equatable conformance.Adding
Equatableconformance toWireObjectsCounterenables equality comparisons which are valuable for testing and validation purposes.Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift (1)
126-129: LGTM! Clean delegation to increment.The decrement method correctly implements RTLC13b by delegating to increment with the negated amount. This is an elegant and correct approach.
Sources/AblyLiveObjects/Protocol/ObjectMessage.swift (3)
21-21: LGTM! Good addition of Equatable conformances.Adding
Equatableconformance to these protocol message structures is beneficial for testing and debugging, allowing for clear equality comparisons between expected and actual message structures.Also applies to: 46-46, 57-57, 66-66, 71-71, 78-78, 83-83
34-34: LGTM! Documentation correctly updated.The documentation now accurately reflects that
PartialObjectOperationexcludes bothactionandobjectIdproperties, which aligns with the code changes in this file.
150-152: LGTM! Clean separation of concerns in encoding/decoding.The refactored encoding and decoding logic properly separates the responsibilities -
ObjectOperationnow explicitly handlesactionandobjectIdwhile delegating other properties toPartialObjectOperation. This makes the code more maintainable and the separation of concerns clearer.Also applies to: 190-193
Sources/AblyLiveObjects/Internal/ObjectsPool.swift (2)
317-351: LGTM! Comprehensive implementation of RTO12h1.The
getOrCreateCountermethod correctly implements the "find or create zero-value" behavior specified in RTO12h1. The type checking withpreconditionFailureis appropriate for catching programming errors, and the method properly handles both existing and new counter creation scenarios.
361-396: LGTM! Comprehensive implementation of RTO11h1.The
getOrCreateMapmethod correctly implements the "find or create zero-value" behavior specified in RTO11h1. It properly handles map-specific concerns including semantics and passing theobjectsPoolfor nested merges during initial value merging.Tests/AblyLiveObjectsTests/ObjectCreationHelpersTests.swift (3)
22-143: LGTM! Comprehensive test coverage for map creation.This test thoroughly validates the map creation operation including:
- All supported entry types (object references and primitives)
- JSON serialization correctness per RTO13
- Object ID format validation per RTO14
- Nonce generation per RTO11f11
- Proper denormalization of properties
The test coverage is excellent and follows the specification attribution guidelines correctly.
151-192: LGTM! Thorough counter creation test.The counter creation test appropriately validates:
- Initial value serialization per RTO13
- Counter-specific properties per RTO12f2a
- Object ID format per RTO14
- Action type and nonce generation
The test is well-structured and covers all essential aspects of counter creation operations.
201-214: LGTM! Clever edge case testing for object ID generation.This test excellently validates RTO14 object ID generation with a particularly good choice of test data. The nonce "arbitraryNonceABC" is specifically chosen to produce Base64 characters (+, /, =) that must be converted to Base64URL format (-, _, removed), ensuring the encoding implementation is correct.
Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift (1)
1365-1365: Missing @test annotation on test function.The
publishesCorrectObjectMessagefunction won't be executed by the test runner without the @test annotation.Apply this diff to add the missing annotation:
+ @Test func publishesCorrectObjectMessage() async throws {⛔ Skipped due to learnings
Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/testing.mdc:0-0 Timestamp: 2025-07-29T08:08:00.588Z Learning: Applies to **/Tests/**/*.swift : Follow the guidelines given under 'Attributing tests to a spec point' in the file `CONTRIBUTING.md` in order to tag the unit tests with the relevant specification points. Make sure to follow the exact format of the comments as described in that file. Pay particular attention to the difference between the meaning of `@spec` and `@specPartial` and be sure not to write `@spec` multiple times for the same specification point.Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/testing.mdc:0-0 Timestamp: 2025-07-29T08:07:58.385Z Learning: Applies to **/Tests/**/*.swift : When writing tests, make sure to add comments that explain when some piece of test data is not important for the scenario being tested.Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/testing.mdc:0-0 Timestamp: 2025-07-29T08:07:58.385Z Learning: Applies to **/Tests/**/*.swift : When writing tests, follow the guidelines given under 'Attributing tests to a spec point' in the file `CONTRIBUTING.md` in order to tag the unit tests with the relevant specification points. Make sure to follow the exact format of the comments as described in that file. Pay particular attention to the difference between the meaning of `@spec` and `@specPartial` and be sure not to write `@spec` multiple times for the same specification point.Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/swift.mdc:0-0 Timestamp: 2025-07-29T08:07:47.875Z Learning: Applies to AblyLiveObjects/**/!(*Test|*Tests).swift : When importing the Ably module inside the AblyLiveObjects library code (non-test code), use `import Ably`.Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/testing.mdc:0-0 Timestamp: 2025-07-29T08:07:58.385Z Learning: Applies to **/Tests/**/*.swift : When you need to import the following modules in the tests, do so in the following way: Ably: use `import Ably`; AblyLiveObjects: use `@testable import AblyLiveObjects`; AblyPlugin: use `import AblyPlugin`; do not do `internal import`.Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/testing.mdc:0-0 Timestamp: 2025-07-29T08:08:00.588Z Learning: Applies to **/Tests/**/*.swift : When you need to import the following modules in the tests, do so in the following way: Ably: use `import Ably`; AblyLiveObjects: use `@testable import AblyLiveObjects`; AblyPlugin: use `import AblyPlugin`; do not do `internal import`.Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/testing.mdc:0-0 Timestamp: 2025-07-29T08:08:00.588Z Learning: Applies to **/Tests/**/*.swift : When creating `testsOnly_` property declarations, do not write generic comments of the form 'Test-only access to the private createOperationIsMerged property'; the meaning of these properties is already well understood.Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/testing.mdc:0-0 Timestamp: 2025-07-29T08:07:58.385Z Learning: Applies to **/Tests/**/*.swift : When creating `testsOnly_` property declarations, do not write generic comments of the form 'Test-only access to the private createOperationIsMerged property'; the meaning of these properties is already well understood.Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/testing.mdc:0-0 Timestamp: 2025-07-29T08:08:00.588Z Learning: Applies to **/Tests/**/*.swift : Add comments that explain when some piece of test data is not important for the scenario being tested.Learnt from: CR PR: ably/ably-cocoa-liveobjects-plugin#0 File: .cursor/rules/swift.mdc:0-0 Timestamp: 2025-07-29T08:07:47.875Z Learning: Applies to AblyLiveObjects/**/!(*Test|*Tests).swift : When importing the AblyPlugin module inside the AblyLiveObjects library code (non-test code), use `internal import AblyPlugin`.Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift (6)
10-14: LGTM! Good test enhancement for time-dependent functionality.Adding the optional
clockparameter with a default value is a clean way to support deterministic timestamp testing while maintaining backward compatibility with existing tests.
1047-1061: Well-structured channel state validation test.Good use of parameterized testing to cover all invalid channel states per RTO11d specification.
1067-1105: Comprehensive test coverage for map creation flow.The test properly validates message publishing (RTO11g), initial value merging (RTO11h3a), and pool management (RTO11h3b). The mock clock approach for deterministic timestamp testing is well implemented.
Note: The TODO comment on line 1095 indicates a future enhancement to use server time.
1109-1132: Good edge case coverage for empty map creation.Test properly validates RTO11f4b specification for creating maps without initial entries.
1136-1173: Excellent test for concurrent object creation scenario.This sophisticated test properly validates RTO11h2 by simulating a race condition where an object appears in the pool before the create operation completes. The use of reference equality (
===) correctly verifies that the existing object is reused rather than replaced.
1198-1298: Well-structured counter creation tests.The remaining counter tests properly validate message publishing (RTO12g), initial value handling (RTO12h3a), pool management (RTO12h3b), zero value creation (RTO12f2a), and existing object reuse (RTO12h2). Good consistency with the map tests structure.
Sources/AblyLiveObjects/Internal/ObjectCreationHelpers.swift (6)
1-3: Correct module imports.The use of
internal import AblyPluginfollows the established pattern for library code, and the other imports are appropriate for the functionality implemented.
10-44: Well-designed creation operation structs.Good API design separating the non-nil
objectIDandoperationproperties from the nullable fields inobjectMessage. The documentation clearly explains the rationale.
51-96: Correct implementation of counter creation per RTO12f.The method properly implements all specification steps from RTO12f2 through RTO12f12, including initial value creation, JSON serialization, nonce generation, and object ID construction.
103-160: Correct implementation of map creation per RTO11f.The method properly implements all specification steps from RTO11f4 through RTO11f13. The use of LWW semantics is consistent throughout, and the entry conversion is handled correctly.
165-179: Proper implementation of RTO13 JSON encoding.The method correctly implements RTO13b (OM4 encoding) and RTO13c (JSON string conversion). The use of
preconditionFailurefor conversion errors is appropriate since this indicates a programmer error.
182-215: Correct implementation of object ID generation per RTO14.The method properly implements:
- RTO14b1: SHA-256 digest generation
- RTO14b2: Base64URL encoding (correctly replacing
+→-,/→_, removing=)- RTO14c: Proper ID format
[type]:[hash]@[timestamp]The timestamp conversion to milliseconds is correctly implemented.
Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (4)
160-192: LGTM! Well-structured implementation of the map creation flow.The implementation correctly validates channel state, constructs the creation operation, publishes it, and manages the object pool access with proper thread safety. The error handling with type conversions is appropriate.
Note: There's a TODO comment about switching from local clock to server time per RTO11f5, tracked in issue #50.
194-197: LGTM! Clean convenience method implementation.Correctly delegates to the main
createMapmethod with empty entries as per RTO11f4b.
199-237: Well-implemented counter creation with proper validation.The implementation correctly validates the initial count value for finiteness, handles channel state validation, and manages the object pool with thread safety. Good error handling throughout.
Note: There's a TODO comment about switching from local clock to server time per RTO12f5, tracked in issue #50.
239-242: LGTM! Consistent convenience method implementation.Correctly delegates to the main
createCountermethod with a default count of 0 as per RTO12f2a.
Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift
Outdated
Show resolved
Hide resolved
Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift
Outdated
Show resolved
Hide resolved
lawrence-forooghian
force-pushed
the
ECO-5461-deletions
branch
2 times, most recently
from
e334349 to
2167277
Compare
Should have done this in b701b46.
Based on [1] at cb11ba8. Internal interfaces by me, implementation and tests by Cursor (both with some tweaking by me). Haven't implemented: - the RTO11e etc echoMessages check — deferred to #49 - the RTO11c etc channel mode checking - same reason as 392fae3 - using server time for generating object ID — deferred to #50 [1] ably/specification#353
Based on [1] at cb11ba8. Code by me, tests by Cursor and tidied up by me. Channel mode checking and echoMessages check omitted as in e65643a. [1] ably/specification#353
Just to convince myself that the write API is somewhat working. Will get further confidence upon porting across more of the JS integration tests in the future.
lawrence-forooghian
force-pushed
the
ECO-5435-write-API
branch
from
ca7a1b7 to
4e8e076
Compare
Note: This PR is based on top of #33; please review that one first.
Implements ably/specification#353. See commit messages for more details.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Documentation
Style
Equatableconformance to multiple internal data structures for easier comparison and testing.