[ECO-5482][LiveObjects] Implement realtime write API#1135
[ECO-5482][LiveObjects] Implement realtime write API#1135sacOO7 wants to merge 13 commits intofeature/server-provided-tombstone-serialfrom
Conversation
WalkthroughThis change set introduces a comprehensive implementation of the realtime write API for LiveObjects, including LiveMap and LiveCounter. It adds type-safe value handling via a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant LiveObjects
participant Adapter
participant AblyRealtime
participant Channel
Client->>LiveObjects: createMap()/createCounter()
LiveObjects->>Adapter: getTime(), getClientOptions()
Adapter->>AblyRealtime: time()
LiveObjects->>LiveObjects: generateNonce()
LiveObjects->>LiveObjects: create ObjectId (hash of value+nonce+timestamp)
LiveObjects->>Channel: publish(ObjectMessage)
Channel-->>LiveObjects: Ack/Result
LiveObjects-->>Client: LiveMap/LiveCounter instance
sequenceDiagram
participant Client
participant LiveMap
participant LiveObjects
participant Channel
Client->>LiveMap: set(key, LiveMapValue)
LiveMap->>LiveObjects: publish(MapSet ObjectMessage)
LiveObjects->>Channel: send(ObjectMessage)
Channel-->>LiveObjects: Ack/Result
LiveMap-->>Client: (operation complete)
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. Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
sacOO7
force-pushed
the
feature/liveobjects-realtime-write-api
branch
from
8803630 to
430151c
Compare
sacOO7
force-pushed
the
feature/liveobjects-realtime-write-api
branch
from
430151c to
55c38ef
Compare
- Implemented LiveMapValue as a separate type representing map values - Updated interface method signatures - Updated doc for createMap method with example
sacOO7
force-pushed
the
feature/liveobjects-realtime-write-api
branch
from
55c38ef to
6bfc6ea
Compare
sacOO7
changed the title
- Updated `increment`,`incrementAsync` method to take `number` as param - Updated `decrement`, `decrementAsync` method to take `number` as param
…liveobjects-realtime-write-api # Conflicts: # live-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt
sacOO7
force-pushed
the
feature/liveobjects-realtime-write-api
branch
from
f22fb02 to
b69cb81
Compare
…LiveMapValue - Updated implementation for the same to return LiveMapValue - Fixed related integration tests with the same
sacOO7
force-pushed
the
feature/liveobjects-realtime-write-api
branch
from
b69cb81 to
25f37c8
Compare
- Added separate object class for getting current server time - Added thread safety to return current server time
sacOO7
force-pushed
the
feature/server-provided-tombstone-serial
branch
from
332ac0a to
97f55f7
Compare
…liveobjects-realtime-write-api # Conflicts: # lib/src/main/java/io/ably/lib/objects/LiveObjects.java # lib/src/main/java/io/ably/lib/objects/type/counter/LiveCounter.java # lib/src/main/java/io/ably/lib/objects/type/map/LiveMap.java # live-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt # live-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt # live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt # live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.kt # live-objects/src/test/kotlin/io/ably/lib/objects/unit/UtilsTest.kt
sacOO7
force-pushed
the
feature/liveobjects-realtime-write-api
branch
from
380d9fb to
010a4e3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.kt (1)
220-337: New test validates realtime API, but consider reducing duplication.The new test properly exercises the realtime write API for LiveMaps using
createMap(),set(), andremove()operations. However, there's significant duplication with the REST API test above.Consider extracting the common assertion logic into a shared helper method that both tests can use, reducing maintenance burden.
lib/src/main/java/io/ably/lib/objects/type/map/LiveMapValue.java (1)
266-443: Well-structured concrete implementations with good encapsulation.The private static final classes provide clean, focused implementations for each type. The immutability design is good, though note that
BinaryValueholds a mutable byte array (a common Java limitation).Consider documenting that callers should not modify byte arrays returned by
getAsBinary()or passed toof(byte[]).live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt (2)
96-98: Consider the performance implications of runBlockingUsing
runBlockingwill block the calling thread until the async operation completes. This could cause performance issues if called from the main thread or in high-throughput scenarios. Consider documenting this behavior in the method's JavaDoc to warn users about the blocking nature.
145-145: Fix typo in spec referenceThe comment contains a typo: "RTLM21cm" should be "RTLM21c,".
- // RTLM21b, RTLM21cm RTLM21d - Validate write API configuration + // RTLM21b, RTLM21c, RTLM21d - Validate write API configuration
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
lib/src/main/java/io/ably/lib/objects/Adapter.java(2 hunks)lib/src/main/java/io/ably/lib/objects/LiveObjects.java(5 hunks)lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java(2 hunks)lib/src/main/java/io/ably/lib/objects/type/counter/LiveCounter.java(1 hunks)lib/src/main/java/io/ably/lib/objects/type/map/LiveMap.java(4 hunks)lib/src/main/java/io/ably/lib/objects/type/map/LiveMapValue.java(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt(3 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/ErrorCodes.kt(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/Helpers.kt(5 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/ObjectId.kt(2 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/ServerTime.kt(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/Utils.kt(3 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt(0 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/type/BaseLiveObject.kt(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt(4 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt(7 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.kt(3 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.kt(5 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.kt(10 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.kt(4 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectIdTest.kt(1 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.kt(1 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/unit/UtilsTest.kt(1 hunks)
💤 Files with no reviewable changes (1)
- live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: sacOO7
PR: ably/ably-java#1106
File: live-objects/src/main/kotlin/io/ably/lib/objects/serialization/Serialization.kt:25-29
Timestamp: 2025-06-23T14:18:25.315Z
Learning: In the ably-java codebase, the DefaultLiveObjectSerializer class methods like writeMsgpackArray will have their type signatures updated to be non-nullable since the calling sites ensure objects are never null when passed to these methods.
Learnt from: sacOO7
PR: ably/ably-java#1106
File: live-objects/src/main/kotlin/io/ably/lib/objects/serialization/Serialization.kt:38-46
Timestamp: 2025-06-23T14:14:17.847Z
Learning: In the ably-java codebase, the DefaultLiveObjectSerializer class uses intentional unsafe casting (`objects as Array<ObjectMessage>`) without type validation in both writeMsgpackArray and asJsonArray methods. This is a deliberate design decision to keep the dynamically-loaded class simple and let ClassCastException occur naturally for type mismatches.
Learnt from: sacOO7
PR: ably/ably-java#1087
File: lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java:32-34
Timestamp: 2025-05-27T12:11:25.084Z
Learning: In LiveObjects implementation (lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java), the send method intentionally hardcodes queueEvents to true rather than respecting ably.options.queueMessages. This is because LiveObjects requires reliable message delivery to ensure proper state synchronization and acknowledgment, unlike other realtime components that may allow configurable queuing behavior.
Learnt from: sacOO7
PR: ably/ably-java#1092
File: live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt:21-32
Timestamp: 2025-06-03T09:15:15.338Z
Learning: In test utility code for the Ably Java Live Objects module, the team prefers to keep reflection-based field access utilities simple without additional error handling, allowing tests to fail fast if incorrect field names are used.
Learnt from: sacOO7
PR: ably/ably-java#1130
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.kt:241-241
Timestamp: 2025-08-01T10:30:27.049Z
Learning: In DefaultLiveMapTest.kt integration tests, operations are performed sequentially one after another, and subscription callback list updates happen one at a time, so thread-safe collections are not needed for collecting updates in test scenarios.
Learnt from: sacOO7
PR: ably/ably-java#1085
File: lib/src/main/java/io/ably/lib/objects/LiveObjects.java:0-0
Timestamp: 2025-05-20T13:12:19.013Z
Learning: The LiveObjects interface does not currently include public API methods for resource management (dispose) or change listeners, as these features are not yet implemented.
Learnt from: sacOO7
PR: ably/ably-java#1120
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.kt:0-0
Timestamp: 2025-08-01T09:53:16.730Z
Learning: In the ably-java LiveObjects test code, extension properties with capital letter names (like `State`, `ObjectId`) are defined in live-objects/src/test/kotlin/io/ably/lib/objects/integration/helpers/Utils.kt to provide access to internal fields of concrete implementations through their public interfaces. For example, `LiveObjects.State` casts to `DefaultLiveObjects` to access the internal `state` field for testing purposes.
Learnt from: sacOO7
PR: ably/ably-java#1120
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.kt:0-0
Timestamp: 2025-08-01T09:53:16.730Z
Learning: In the ably-java LiveObjects test code, an extension property `State` (capital S) is defined on the `LiveObjects` interface in live-objects/src/test/kotlin/io/ably/lib/objects/integration/helpers/Utils.kt to provide access to the internal `state` field by casting to `DefaultLiveObjects`. This allows tests to access internal state for verification purposes.
Learnt from: sacOO7
PR: ably/ably-java#1113
File: live-objects/src/test/kotlin/io/ably/lib/objects/unit/type/livemap/LiveMapManagerTest.kt:16-16
Timestamp: 2025-08-01T05:50:33.039Z
Learning: In LiveMapManagerTest.kt, the private field `livemapManager` is used extensively in the `shouldCalculateMapDifferenceCorrectly` test method to test the `calculateUpdateFromDataDiff` functionality across multiple test scenarios, so it should not be removed as unused.
Learnt from: sacOO7
PR: ably/ably-java#1092
File: live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt:48-61
Timestamp: 2025-06-03T09:15:18.827Z
Learning: User sacOO7 prefers simple test utilities without extensive error handling, believing tests should fail fast if incorrect field/method names are used rather than having defensive programming.
📚 Learning: in the ably-java liveobjects test code, extension properties with capital letter names (like `state`...
Learnt from: sacOO7
PR: ably/ably-java#1120
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.kt:0-0
Timestamp: 2025-08-01T09:53:16.730Z
Learning: In the ably-java LiveObjects test code, extension properties with capital letter names (like `State`, `ObjectId`) are defined in live-objects/src/test/kotlin/io/ably/lib/objects/integration/helpers/Utils.kt to provide access to internal fields of concrete implementations through their public interfaces. For example, `LiveObjects.State` casts to `DefaultLiveObjects` to access the internal `state` field for testing purposes.
Applied to files:
live-objects/src/test/kotlin/io/ably/lib/objects/unit/UtilsTest.ktlib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.javalive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/ServerTime.ktlib/src/main/java/io/ably/lib/objects/type/map/LiveMap.javalib/src/main/java/io/ably/lib/objects/type/map/LiveMapValue.javalive-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectIdTest.ktlib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/main/kotlin/io/ably/lib/objects/Helpers.ktlive-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/ObjectId.ktlive-objects/src/main/kotlin/io/ably/lib/objects/Utils.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/BaseLiveObject.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt
📚 Learning: in livemapmanagertest.kt, the private field `livemapmanager` is used extensively in the `shouldcalcu...
Learnt from: sacOO7
PR: ably/ably-java#1113
File: live-objects/src/test/kotlin/io/ably/lib/objects/unit/type/livemap/LiveMapManagerTest.kt:16-16
Timestamp: 2025-08-01T05:50:33.039Z
Learning: In LiveMapManagerTest.kt, the private field `livemapManager` is used extensively in the `shouldCalculateMapDifferenceCorrectly` test method to test the `calculateUpdateFromDataDiff` functionality across multiple test scenarios, so it should not be removed as unused.
Applied to files:
live-objects/src/test/kotlin/io/ably/lib/objects/unit/UtilsTest.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.ktlib/src/main/java/io/ably/lib/objects/type/map/LiveMap.javalib/src/main/java/io/ably/lib/objects/type/map/LiveMapValue.javalib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt
📚 Learning: in livemapmanagertest.kt, the private field `livemapmanager` is used in the `shouldcalculatemapdiffe...
Learnt from: sacOO7
PR: ably/ably-java#1113
File: live-objects/src/test/kotlin/io/ably/lib/objects/unit/type/livemap/LiveMapManagerTest.kt:16-16
Timestamp: 2025-08-01T05:50:33.039Z
Learning: In LiveMapManagerTest.kt, the private field `livemapManager` is used in the `shouldCalculateMapDifferenceCorrectly` test method to test the `calculateUpdateFromDataDiff` functionality, so it should not be removed as unused.
Applied to files:
live-objects/src/test/kotlin/io/ably/lib/objects/unit/UtilsTest.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.ktlib/src/main/java/io/ably/lib/objects/type/map/LiveMap.javalib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt
📚 Learning: in liveobjects implementation (lib/src/main/java/io/ably/lib/objects/liveobjectsadapter.java), the s...
Learnt from: sacOO7
PR: ably/ably-java#1087
File: lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java:32-34
Timestamp: 2025-05-27T12:11:25.084Z
Learning: In LiveObjects implementation (lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java), the send method intentionally hardcodes queueEvents to true rather than respecting ably.options.queueMessages. This is because LiveObjects requires reliable message delivery to ensure proper state synchronization and acknowledgment, unlike other realtime components that may allow configurable queuing behavior.
Applied to files:
lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.javalive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.ktlib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/main/kotlin/io/ably/lib/objects/Helpers.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt
📚 Learning: in the ably-java codebase, the defaultliveobjectserializer class methods like writemsgpackarray will...
Learnt from: sacOO7
PR: ably/ably-java#1106
File: live-objects/src/main/kotlin/io/ably/lib/objects/serialization/Serialization.kt:25-29
Timestamp: 2025-06-23T14:18:25.315Z
Learning: In the ably-java codebase, the DefaultLiveObjectSerializer class methods like writeMsgpackArray will have their type signatures updated to be non-nullable since the calling sites ensure objects are never null when passed to these methods.
Applied to files:
lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.javalive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.ktlib/src/main/java/io/ably/lib/objects/type/map/LiveMap.javalib/src/main/java/io/ably/lib/objects/type/map/LiveMapValue.javalive-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectIdTest.ktlib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/main/kotlin/io/ably/lib/objects/Helpers.ktlib/src/main/java/io/ably/lib/objects/Adapter.javalive-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/Utils.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/BaseLiveObject.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt
📚 Learning: the liveobjects interface does not currently include public api methods for resource management (dis...
Learnt from: sacOO7
PR: ably/ably-java#1085
File: lib/src/main/java/io/ably/lib/objects/LiveObjects.java:0-0
Timestamp: 2025-05-20T13:12:19.013Z
Learning: The LiveObjects interface does not currently include public API methods for resource management (dispose) or change listeners, as these features are not yet implemented.
Applied to files:
lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.javalive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.ktlib/src/main/java/io/ably/lib/objects/type/map/LiveMap.javalib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt
📚 Learning: in test utility code for the ably java live objects module, the team prefers to keep reflection-base...
Learnt from: sacOO7
PR: ably/ably-java#1092
File: live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt:21-32
Timestamp: 2025-06-03T09:15:15.338Z
Learning: In test utility code for the Ably Java Live Objects module, the team prefers to keep reflection-based field access utilities simple without additional error handling, allowing tests to fail fast if incorrect field names are used.
Applied to files:
lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.javalive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/ServerTime.ktlib/src/main/java/io/ably/lib/objects/type/map/LiveMap.javalib/src/main/java/io/ably/lib/objects/type/map/LiveMapValue.javalive-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectIdTest.ktlib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/main/kotlin/io/ably/lib/objects/Helpers.ktlive-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/Utils.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/BaseLiveObject.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt
📚 Learning: in the ably-java liveobjects test code, an extension property `state` (capital s) is defined on the ...
Learnt from: sacOO7
PR: ably/ably-java#1120
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.kt:0-0
Timestamp: 2025-08-01T09:53:16.730Z
Learning: In the ably-java LiveObjects test code, an extension property `State` (capital S) is defined on the `LiveObjects` interface in live-objects/src/test/kotlin/io/ably/lib/objects/integration/helpers/Utils.kt to provide access to the internal `state` field by casting to `DefaultLiveObjects`. This allows tests to access internal state for verification purposes.
Applied to files:
lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.javalive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.ktlib/src/main/java/io/ably/lib/objects/type/map/LiveMap.javalib/src/main/java/io/ably/lib/objects/type/map/LiveMapValue.javalive-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectIdTest.ktlib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/main/kotlin/io/ably/lib/objects/Helpers.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/BaseLiveObject.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt
📚 Learning: in defaultlivemaptest.kt integration tests, operations are performed sequentially one after another,...
Learnt from: sacOO7
PR: ably/ably-java#1130
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.kt:241-241
Timestamp: 2025-08-01T10:30:27.049Z
Learning: In DefaultLiveMapTest.kt integration tests, operations are performed sequentially one after another, and subscription callback list updates happen one at a time, so thread-safe collections are not needed for collecting updates in test scenarios.
Applied to files:
live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.ktlib/src/main/java/io/ably/lib/objects/type/map/LiveMap.javalib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.ktlive-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt
📚 Learning: in the ably-java codebase, the defaultliveobjectserializer class uses intentional unsafe casting (`o...
Learnt from: sacOO7
PR: ably/ably-java#1106
File: live-objects/src/main/kotlin/io/ably/lib/objects/serialization/Serialization.kt:38-46
Timestamp: 2025-06-23T14:14:17.847Z
Learning: In the ably-java codebase, the DefaultLiveObjectSerializer class uses intentional unsafe casting (`objects as Array<ObjectMessage>`) without type validation in both writeMsgpackArray and asJsonArray methods. This is a deliberate design decision to keep the dynamically-loaded class simple and let ClassCastException occur naturally for type mismatches.
Applied to files:
live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.ktlib/src/main/java/io/ably/lib/objects/type/map/LiveMapValue.javalib/src/main/java/io/ably/lib/objects/LiveObjects.javalive-objects/src/main/kotlin/io/ably/lib/objects/Helpers.ktlive-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt
📚 Learning: in the ably-java codebase test files, junit's assert.assertequals method is used which has the signa...
Learnt from: sacOO7
PR: ably/ably-java#1113
File: live-objects/src/test/kotlin/io/ably/lib/objects/unit/type/livemap/LiveMapManagerTest.kt:640-640
Timestamp: 2025-08-01T05:54:07.024Z
Learning: In the ably-java codebase test files, JUnit's Assert.assertEquals method is used which has the signature assertEquals(String message, Object expected, Object actual) where the message parameter comes first, not last like in Kotlin test's assertEquals method.
Applied to files:
live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.ktlive-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.kt
📚 Learning: user sacoo7 prefers simple test utilities without extensive error handling, believing tests should f...
Learnt from: sacOO7
PR: ably/ably-java#1092
File: live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt:48-61
Timestamp: 2025-06-03T09:15:18.827Z
Learning: User sacOO7 prefers simple test utilities without extensive error handling, believing tests should fail fast if incorrect field/method names are used rather than having defensive programming.
Applied to files:
live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.kt
📚 Learning: the sandbox.kt file in ably-java live-objects module already has comprehensive http retry mechanism ...
Learnt from: sacOO7
PR: ably/ably-java#1095
File: live-objects/src/test/kotlin/io/ably/lib/objects/integration/setup/IntegrationTest.kt:87-89
Timestamp: 2025-06-06T09:28:12.298Z
Learning: The Sandbox.kt file in ably-java live-objects module already has comprehensive HTTP retry mechanism using HttpRequestRetry with 5 retries, exponential backoff, and automatic retry on non-success responses and timeout exceptions.
Applied to files:
live-objects/src/main/kotlin/io/ably/lib/objects/ServerTime.ktlive-objects/src/main/kotlin/io/ably/lib/objects/Helpers.ktlive-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.ktlive-objects/src/main/kotlin/io/ably/lib/objects/Utils.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt
📚 Learning: in the ably java liveobjects messagepack deserialization code, the `action` field in objectoperation...
Learnt from: sacOO7
PR: ably/ably-java#1106
File: live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt:207-211
Timestamp: 2025-06-23T14:28:23.301Z
Learning: In the Ably Java LiveObjects MessagePack deserialization code, the `action` field in ObjectOperation is guaranteed to always be present in the protocol, so using a default value during deserialization is acceptable and won't mask real protocol errors.
Applied to files:
live-objects/src/main/kotlin/io/ably/lib/objects/Helpers.ktlive-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt
📚 Learning: in kotlin/java codebases, when referencing types (classes, enums, interfaces) that are defined in th...
Learnt from: sacOO7
PR: ably/ably-java#1087
File: live-objects/src/main/kotlin/io/ably/lib/objects/ObjectMessage.kt:198-198
Timestamp: 2025-05-27T12:12:10.782Z
Learning: In Kotlin/Java codebases, when referencing types (classes, enums, interfaces) that are defined in the same package, no import statement is required as they are automatically accessible due to package-private visibility.
Applied to files:
live-objects/src/main/kotlin/io/ably/lib/objects/type/BaseLiveObject.kt
🧬 Code Graph Analysis (6)
live-objects/src/test/kotlin/io/ably/lib/objects/unit/UtilsTest.kt (1)
live-objects/src/main/kotlin/io/ably/lib/objects/Utils.kt (1)
generateNonce(114-117)
live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt (1)
live-objects/src/main/kotlin/io/ably/lib/objects/Utils.kt (1)
invalidInputError(45-47)
lib/src/main/java/io/ably/lib/objects/type/counter/LiveCounter.java (2)
live-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt (5)
increment(44-44)decrement(46-46)incrementAsync(48-50)incrementAsync(72-92)decrementAsync(52-54)live-objects/src/main/kotlin/io/ably/lib/objects/ObjectMessage.kt (1)
amount(90-96)
live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.kt (2)
live-objects/src/test/kotlin/io/ably/lib/objects/integration/setup/IntegrationTest.kt (2)
generateChannelName(61-63)getRealtimeChannel(39-55)live-objects/src/test/kotlin/io/ably/lib/objects/TestUtils.kt (1)
assertWaiter(11-20)
live-objects/src/main/kotlin/io/ably/lib/objects/Helpers.kt (1)
live-objects/src/main/kotlin/io/ably/lib/objects/Utils.kt (3)
ablyException(10-18)ablyException(20-23)clientError(37-37)
live-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt (1)
live-objects/src/main/kotlin/io/ably/lib/objects/Utils.kt (1)
invalidInputError(45-47)
⏰ 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). (5)
- GitHub Check: check-rest-okhttp
- GitHub Check: check-realtime-okhttp
- GitHub Check: check-realtime
- GitHub Check: check-rest
- GitHub Check: check (24)
🔇 Additional comments (54)
live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.kt (1)
171-171: LGTM! Corrected exception message to use plural form.The change from "ObjectMessage" to "ObjectMessages" accurately reflects that multiple messages are being validated, aligning with the test scenario where an array of ObjectMessages is passed to
ensureMessageSizeWithinLimit.live-objects/src/main/kotlin/io/ably/lib/objects/ErrorCodes.kt (1)
9-9: LGTM! Improved error code semantics.Renaming
MapKeyShouldBeStringtoInvalidInputParamsmakes this error code more general-purpose and reusable for various input validation scenarios, while maintaining the same numeric code (40003) for backward compatibility.live-objects/src/test/kotlin/io/ably/lib/objects/unit/UtilsTest.kt (1)
15-31: LGTM! Comprehensive test coverage for nonce generation.The test thoroughly validates the
generateNonce()function by checking:
- Correct length (16 characters)
- Randomness between multiple calls
- Character set validation (alphanumeric only)
This provides good coverage for the nonce generation utility used in object ID creation.
live-objects/src/main/kotlin/io/ably/lib/objects/type/BaseLiveObject.kt (1)
30-30: LGTM! Appropriate visibility adjustment for internal module access.Changing
objectTypefromprivatetointernalallows other components within the live objects module to access this property while maintaining proper encapsulation from external users. This supports the enhanced internal integration described in the PR objectives.live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectIdTest.kt (1)
56-74: LGTM! Well-structured test for ObjectId creation from initial values.The test comprehensively validates the
ObjectId.fromInitialValuemethod by:
- Verifying the correct string format (
type:hash@timestamp)- Asserting the expected hash value for deterministic testing
- Ensuring proper timestamp inclusion
The use of a specific expected hash value provides deterministic test results while validating the hashing algorithm's correctness.
lib/src/main/java/io/ably/lib/objects/LiveObjectsAdapter.java (1)
60-84: LGTM! Clean interface extension with proper documentation.The new method declarations are well-documented with clear JavaDoc comments explaining their purpose. The
@Blockingannotation ongetTime()correctly indicates this is a synchronous operation, and the spec reference (RTO16) provides good traceability.live-objects/src/main/kotlin/io/ably/lib/objects/Utils.kt (3)
45-47: LGTM! Consistent error handling pattern.The
invalidInputErrorfunction follows the same pattern as other error creation functions in the file, providing a consistent approach to creating validation-related exceptions.
55-55: Good improvement to use explicit charset.Using
StandardCharsets.UTF_8instead of the default charset makes the encoding explicit and ensures consistent behavior across different environments.
114-117: LGTM! Efficient nonce generation implementation.The function generates a 16-character random alphanumeric string efficiently. The hardcoded character string avoids range calculations and the implementation is straightforward and correct.
live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveObjectsTest.kt (1)
76-152: Excellent type safety improvements!The migration from unsafe casting to safe nullable property accessors (like
?.asLiveCounter,?.asLiveMap, etc.) significantly improves type safety. The use ofcontentEquals()for byte array comparison and the.sizeproperty instead of method calls are also good improvements that align with Kotlin idioms.lib/src/main/java/io/ably/lib/objects/Adapter.java (1)
71-84: LGTM! Clean adapter implementation.The new methods correctly implement the interface by delegating to the underlying
AblyRealtimeinstance. The implementation follows the adapter pattern appropriately and maintains consistency with existing methods in the class.live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/LiveMapEntry.kt (2)
45-59: Excellent type safety improvement with LiveMapValue.The change from
Any?toLiveMapValue?provides much better type safety and API clarity. The updated logic consistently returns wrappedLiveMapValueinstances, which aligns perfectly with the new typed API design.
69-85: Well-organized helper functions for type conversion.The
fromObjectValueandfromLiveObjecthelper functions provide clean, comprehensive conversion logic that covers all supported LiveMap value types. The pattern matching approach ensures all cases are handled correctly and makes the code easy to understand and maintain.live-objects/src/main/kotlin/io/ably/lib/objects/ServerTime.kt (2)
14-17: Well-designed singleton with appropriate thread safety.The use of
@VolatileforserverTimeOffsetand a coroutineMutexprovides proper thread safety for lazy initialization of the server time offset.
23-34: Correct implementation of double-checked locking with coroutines.The double-checked locking pattern is properly implemented:
- First check outside the lock (line 24)
- Second check inside the lock (line 26)
- Switching to IO dispatcher for the adapter call is good practice
- The non-null assertion on line 33 is safe due to the locking pattern
live-objects/src/main/kotlin/io/ably/lib/objects/ObjectId.kt (3)
4-6: Appropriate imports for cryptographic operations.The imports for UTF-8 charset, SHA-256 message digest, and Base64 encoding are correctly chosen for the hashing and encoding operations.
26-33: Secure implementation of ObjectId generation from initial value.The implementation correctly follows the specification:
- Uses SHA-256 for secure hashing (RTO14b)
- Concatenates initial value and nonce with colon separator for uniqueness
- Uses URL-safe Base64 encoding without padding, appropriate for IDs
- UTF-8 encoding ensures consistent byte representation
38-38: Appropriate visibility change to internal.Changing
fromStringfrom public to internal is consistent withObjectIdbeing an internal class and maintains proper encapsulation.lib/src/main/java/io/ably/lib/objects/type/counter/LiveCounter.java (3)
17-27: Enhanced flexibility with Number parameter type.The change from fixed increment amounts to accepting
Numberparameters significantly improves the API flexibility. The documentation clearly explains the operation and includes proper specification references.
29-37: Well-documented decrement as increment alias.The documentation clearly explains that
decrementis an alias forincrementwith a negative amount, which simplifies the implementation while providing a clear API surface.
67-71: Appropriate return type change to Double.Changing the return type from
LongtoDoubleenables support for fractional counter values, which aligns with the implementation that usesDoubleinternally for operations.live-objects/src/main/kotlin/io/ably/lib/objects/Helpers.kt (4)
56-60: Comprehensive write API validation.The validation properly checks for echo messages, channel modes, and channel states that would prevent successful write operations. The inclusion of
suspendedstate in the validation is appropriate for write operations.
62-67: Appropriate connection and channel state validation.The function correctly validates both connection manager state and channel state before allowing publish operations, ensuring reliable message delivery.
85-89: Essential echo messages validation for LiveObjects.This validation is crucial for LiveObjects functionality since they rely on receiving echoed messages to maintain proper state synchronization. The error message clearly explains the requirement.
107-113: Simple and appropriate payload data classes.These data classes provide clean encapsulation for counter and map creation payloads, following a consistent pattern and serving their purpose well.
live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveCounterTest.kt (3)
31-32: Improved type safety with nullable accessors.The migration from unsafe casting to safe nullable accessors (
.asLiveCounter,.asLiveMap) followed byassertNotNullsignificantly improves type safety and prevents potentialClassCastExceptions.Also applies to: 37-38, 42-43, 47-48, 52-53, 57-58, 62-63, 67-68, 73-74, 78-79, 83-84, 88-89
208-297: Comprehensive realtime API test coverage.This new test method provides excellent coverage of the LiveCounter realtime API:
- Tests creation, increment, and decrement operations directly through the realtime API
- Uses proper
LiveMapValue.of()for type-safe value setting- Follows consistent assertion patterns with
assertWaiterfor async operations- Covers various increment/decrement amounts matching the new flexible API
132-133: Consistent application of safe accessor pattern.The pattern of using nullable accessors followed by
assertNotNullis consistently applied throughout all test methods, providing uniform type safety improvements.Also applies to: 203-206, 223-224, 294-295, 308-309, 311-312
lib/src/main/java/io/ably/lib/objects/type/map/LiveMap.java (5)
32-33: LGTM! Type-safe return value improves API clarity.The change from
ObjecttoLiveMapValueprovides better type safety and makes the API more explicit about what values can be stored in a LiveMap.
43-43: Consistent type safety for map entries.The entries() method now returns properly typed entries with LiveMapValue, maintaining consistency with the type-safe API design.
63-63: Type-safe values collection.The values() method now returns LiveMapValue instances, completing the type-safe API for all collection methods.
71-77: Type-safe set operations with proper spec references.Both synchronous and asynchronous set methods now enforce LiveMapValue type safety at compile time, preventing invalid value types from being set. The RTLM20 spec references provide good traceability.
Also applies to: 108-115
85-85: Good addition of spec references for remove operations.Adding RTLM21 spec references to both remove methods improves documentation consistency and traceability.
Also applies to: 123-123
live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.kt (3)
34-98: Excellent improvement in test type safety.The refactoring from unsafe casts to safe nullable access with explicit type conversions (
.asLiveMap,.asLiveCounter,.asString, etc.) prevents potential ClassCastException and makes the tests more robust. The consistent pattern ofget()?.asTypeproperly handles the nullable return values.
126-218: Type-safe conversions with improved value filtering.The consistent application of safe type conversions continues throughout this test. The new value filtering logic (lines 214-218) elegantly demonstrates the use of LiveMapValue's type checking capabilities by filtering for string values only.
349-415: Consistent type safety in subscription tests.The subscription test has been properly updated with the same type-safe conversion pattern, ensuring consistency across all test methods.
live-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt (3)
44-54: Clean implementation of increment/decrement operations.Good design choice to implement decrement as increment with negative amount, reducing code duplication. The synchronous methods properly delegate to async implementations using
runBlocking.
72-92: Well-structured increment implementation with proper validation.The implementation correctly follows the spec requirements (RTLC12b-f) with appropriate validation for NaN/Infinite values and proper ObjectMessage construction. The error handling provides clear feedback.
128-137: Useful helper for counter initialization.The
initialValuemethod provides a clean way to create counter payloads with initial values, properly handling Number to Double conversion. Good spec reference (RTO12f2).lib/src/main/java/io/ably/lib/objects/type/map/LiveMapValue.java (3)
9-82: Excellent union type implementation following established patterns.The abstract class design with type-checking methods follows the proven pattern used by Gson's JsonElement. The spec reference (RTO11a1) clearly indicates the supported types, and the implementation provides comprehensive type safety.
84-172: Type-safe getter implementation with clear error messages.The getter methods properly enforce type safety by throwing
IllegalStateExceptionwith descriptive messages when called on the wrong type. The@NotNullannotations correctly indicate that these methods never return null.
174-264: Clean factory method API using idiomatic Java patterns.The
of()factory methods follow Java conventions (similar toOptional.of(),List.of()) and provide a type-safe way to create LiveMapValue instances. All methods are properly annotated for null-safety.live-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt (5)
63-85: Consistent delegation pattern for synchronous methods.All synchronous methods properly delegate to their asynchronous counterparts using
runBlocking, maintaining a clean and consistent API implementation.
100-134: Comprehensive map creation implementation following spec requirements.The implementation correctly follows all spec requirements (RTO11c-h) with proper validation, ID generation, and pool management. The empty key check is a good defensive measure.
136-170: Counter creation mirrors map pattern with appropriate numeric validation.The implementation properly validates numeric inputs (NaN/Infinite) and follows the same robust pattern as map creation, adhering to spec requirements RTO12c-h.
172-193: Well-structured helper methods with proper validation.The object ID generation correctly uses server time (RTO16) and nonce. The publish method properly validates channel state (RTO15b, RTL6c) and message size before constructing and sending the protocol message according to spec (RTO15e-g).
55-55: Async scope visibility change enables consistent async handling.Changing
asyncScopefrom private to internal allowsDefaultLiveCounterandDefaultLiveMapto use the same coroutine scope for their async operations, ensuring consistent error handling and cancellation behavior across the module.live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt (4)
49-58: Type-safe API enhancement looks goodThe return type change from
Any?toLiveMapValue?aligns with the new type-safe API design, providing better compile-time safety.
60-71: Efficient implementation of entries() with type safetyGood use of sequences for lazy evaluation and filtering out null values. The immutable entry wrapper ensures thread safety.
119-142: Well-structured implementation of setAsyncThe method correctly implements the spec requirements (RTLM20b-f) with proper validation and error handling. Good separation of concerns by delegating value conversion to
fromLiveMapValue.
218-248: Comprehensive type conversion implementationThe
fromLiveMapValuemethod correctly handles all supported LiveMapValue types with appropriate conversions to ObjectData. Good use of when expression for exhaustive type checking.lib/src/main/java/io/ably/lib/objects/LiveObjects.java (3)
48-76: Excellent API design with comprehensive documentationThe simplified API with LiveMapValue provides type safety while maintaining flexibility. The JavaDoc example clearly demonstrates all supported value types, making it easy for developers to understand usage.
78-103: Good generalization of counter initial value typeChanging from
LongtoNumberprovides better flexibility for initializing counters with different numeric types while maintaining backward compatibility.
116-165: Consistent async API designThe async methods properly mirror the synchronous API changes with appropriate callback handling. The API remains clean and intuitive.
sacOO7
force-pushed
the
feature/liveobjects-realtime-write-api
branch
from
010a4e3 to
fcbea10
Compare
sacOO7
force-pushed
the
feature/liveobjects-realtime-write-api
branch
from
fcbea10 to
c4dba39
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt (1)
221-248: Review LiveMapValue conversion method for completenessThe fromLiveMapValue method handles most LiveMapValue types correctly, but there are a few considerations:
- Type coverage: Covers all major types (Boolean, Binary, Number, String, JsonObject, JsonArray, LiveMap, LiveCounter)
- Error handling: Throws IllegalArgumentException for unsupported types
- Nested objects: Properly handles LiveMap and LiveCounter as object references
However, the error message could be more descriptive.
Improve the error message for better debugging:
- else -> { - throw IllegalArgumentException("Unsupported value type") - } + else -> { + throw IllegalArgumentException("Unsupported LiveMapValue type: ${value.javaClass.simpleName}") + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
lib/src/main/java/io/ably/lib/objects/LiveObjects.java(5 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt(3 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/ErrorCodes.kt(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/ObjectId.kt(2 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/ServerTime.kt(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/Utils.kt(3 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt(4 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt(7 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.kt(10 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/integration/helpers/PayloadBuilder.kt(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- live-objects/src/main/kotlin/io/ably/lib/objects/ErrorCodes.kt
- live-objects/src/test/kotlin/io/ably/lib/objects/integration/helpers/PayloadBuilder.kt
🚧 Files skipped from review as they are similar to previous changes (6)
- live-objects/src/main/kotlin/io/ably/lib/objects/ObjectId.kt
- live-objects/src/main/kotlin/io/ably/lib/objects/ServerTime.kt
- live-objects/src/main/kotlin/io/ably/lib/objects/Utils.kt
- live-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt
- lib/src/main/java/io/ably/lib/objects/LiveObjects.java
- live-objects/src/test/kotlin/io/ably/lib/objects/integration/DefaultLiveMapTest.kt
👮 Files not reviewed due to content moderation or server errors (2)
- live-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt
- live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt (1)
live-objects/src/main/kotlin/io/ably/lib/objects/Utils.kt (1)
invalidInputError(45-47)
⏰ 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). (3)
- GitHub Check: check
- GitHub Check: check-realtime-okhttp
- GitHub Check: check-realtime
🔇 Additional comments (41)
live-objects/src/main/kotlin/io/ably/lib/objects/DefaultLiveObjects.kt (21)
3-11: LGTM! Import statements properly support new functionality.The added imports are necessary for the realtime write API implementation, including JSON serialization, type-safe map values, and concrete live object implementations.
55-55: Appropriate visibility change for module-level access.Changing
asyncScopefrom private to internal is justified to support callback operations inDefaultLiveMapwhile maintaining proper encapsulation within the module.
63-69: LGTM! Consistent delegation pattern for synchronous methods.The synchronous methods properly delegate to their asynchronous counterparts using
runBlocking, with appropriate default values (empty map, zero counter). This maintains API consistency.
75-85: LGTM! Consistent callback delegation pattern.The asynchronous callback methods properly use
asyncScope.launchWithCallbackand delegate to appropriate default values, maintaining consistency with the synchronous API.
100-134: Well-structured map creation implementation.The implementation correctly follows RTO11 specifications with proper validation, object ID generation, and message publishing. The use of
sequentialScope.coroutineContextensures thread-safe pool operations.
136-170: LGTM! Robust counter creation implementation.The implementation correctly follows RTO12 specifications with appropriate validation for NaN/infinite values, proper object ID generation, and consistent message publishing pattern.
172-193: Excellent helper method implementations.Both
getObjectIdStringWithNonceandpublishmethods are well-structured with proper validation, spec compliance (RTO15, RTO16), and appropriate error handling. The separation of concerns enhances maintainability.
3-3: LGTM: Import addition for gson serializationThe addition of the gson import is appropriate for JSON serialization of initial values in the creation methods.
6-11: LGTM: Import additions for type safetyThe new imports for ObjectType, DefaultLiveCounter, DefaultLiveMap, and LiveMapValue support the type-safe implementation of the write API.
63-69: LGTM: Synchronous method implementationsThe synchronous createMap and createCounter methods correctly delegate to their async counterparts using runBlocking, maintaining API consistency.
75-85: LGTM: Async callback method implementationsThe async callback methods properly delegate to the internal async implementations using the asyncScope for safe callback execution.
175-179: LGTM: Object ID generation helper methodThe getObjectIdStringWithNonce method properly implements the specification for ID generation with nonce and server time.
184-193: LGTM: Message publishing implementationThe publish method correctly implements RTO15 specification with proper validation, protocol message construction, and async sending.
55-55: asyncScope internal visibility is appropriateThe
asyncScopeproperty is only accessed within the module (in DefaultLiveMap.kt and DefaultLiveCounter.kt) and test code directly instantiatesObjectsAsyncScoperather than reaching intoliveObjects.asyncScope. No public API surface is exposed, so encapsulation remains intact.– Used in:
• live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt
• live-objects/src/main/kotlin/io/ably/lib/objects/type/livecounter/DefaultLiveCounter.kt
3-3: LGTM!The new imports are appropriate and necessary for the implemented functionality - gson serialization, type definitions, and concrete implementations for live objects.
Also applies to: 6-6, 8-11
55-55: LGTM!Changing
asyncScopevisibility to internal is appropriate to allow other classes in the module (likeDefaultLiveMap) to access it for async operations.
63-63: LGTM!The synchronous methods follow a clean pattern of delegating to their async counterparts using
runBlocking, with appropriate default parameter handling. This avoids code duplication while maintaining the expected API.Also applies to: 65-65, 67-67, 69-69
75-75: LGTM!The async callback methods follow a consistent pattern with proper default parameter handling and use
asyncScope.launchWithCallbackfor safe callback execution with error handling.Also applies to: 77-79, 81-81, 83-85
136-170: LGTM!The
createCounterAsyncimplementation follows the RTO12 specifications comprehensively with proper validation for numeric values (NaN/Infinite check), consistent object creation patterns, and appropriate error handling. The implementation mirrors the solid pattern established increateMapAsync.
175-179: LGTM!The utility methods are well-implemented:
getObjectIdStringWithNonceproperly generates nonces and uses server time per RTO11f8/RTO12f6publishmethod follows RTO15 specification with proper validation, ProtocolMessage construction, and async sending- Good separation of concerns with appropriate visibility modifiers
Also applies to: 184-193
100-134: Utility methods verified – implementation is correctAll utility functions used in
createMapAsynchave been inspected and behave as specified:
getObjectIdStringWithNonce(DefaultLiveObjects.kt:175–179) correctly generates a 16-char nonce, obtains server time viaServerTime.getCurrentTime, and returns the object ID and nonce.generateNonce(Utils.kt:114–117) produces a secure, 16-character alphanumeric string.ServerTime.getCurrentTime(ServerTime.kt:23–33) fetches and caches the server time offset with proper thread safety.No further changes are needed. Approving as-is.
live-objects/src/main/kotlin/io/ably/lib/objects/type/livemap/DefaultLiveMap.kt (20)
14-14: LGTM! Proper imports and scope access for new functionality.The added imports support type-safe operations and synchronous method delegation. The
asyncScopeaccess aligns with the visibility change inDefaultLiveObjects.Also applies to: 17-17, 47-47
49-49: Excellent type safety improvements with LiveMapValue.The migration from
AnytoLiveMapValueprovides compile-time type safety as specified in RTO11. This change is consistent across all value-related methods and aligns with the PR objectives.Also applies to: 60-63, 82-82, 96-96, 100-101
96-98: Consistent delegation pattern for synchronous operations.The synchronous
setandremovemethods correctly delegate to their asynchronous counterparts usingrunBlocking, maintaining API consistency with the established pattern.Also applies to: 105-106
119-164: Well-implemented async write operations.Both
setAsyncandremoveAsyncmethods properly follow RTLM20/RTLM21 specifications with appropriate validation, message construction, and publishing. The use ofinvalidInputErrorfor empty key validation is consistent with the utility function inUtils.kt.
200-248: Comprehensive value conversion implementation.The companion methods properly handle all
LiveMapValuetypes with appropriateObjectDataconversion. TheinitialValuemethod correctly implements RTO11f4 specification.Consider enhancing the error message in line 245 to specify which value type was unsupported:
- throw IllegalArgumentException("Unsupported value type") + throw IllegalArgumentException("Unsupported value type: ${value::class.simpleName}")
14-14: LGTM: Import addition for LiveMapValueThe import addition supports the type-safe refactoring from Any to LiveMapValue.
17-17: LGTM: Import addition for runBlockingThe runBlocking import is needed for the synchronous methods that delegate to async implementations.
47-47: LGTM: AsyncScope access propertyThe asyncScope property provides consistent access to the async execution scope from DefaultLiveObjects.
49-58: LGTM: Type-safe get methodThe method signature change from returning Any? to LiveMapValue? provides better type safety while maintaining the same logic.
60-71: LGTM: Type-safe entries methodThe entries method now returns Map.Entry<String, LiveMapValue> instead of Map.Entry<String, Any>, providing better type safety.
82-89: LGTM: Type-safe values methodThe values method now returns Iterable instead of Iterable, maintaining type consistency.
96-98: LGTM: Synchronous method implementationsThe synchronous set and remove methods properly delegate to their async counterparts using runBlocking.
100-106: LGTM: Async callback method implementationsThe setAsync and removeAsync callback methods properly delegate to internal async implementations using asyncScope.
119-142: LGTM: SetAsync implementation with proper validationThe setAsync method implementation is well-structured:
- Validation: Proper write API configuration and input parameter validation
- Message construction: Correct ObjectMessage with MAP_SET operation
- Publishing: Uses the liveObjects publish method
- Error handling: Validates empty key names with appropriate error
The implementation follows the RTLM20 specification correctly.
144-164: LGTM: RemoveAsync implementation with proper validationThe removeAsync method follows the same pattern as setAsync with appropriate validation and message construction for MAP_REMOVE operations. Follows RTLM21 specification correctly.
200-216: LGTM: Initial value creation methodThe initialValue companion method properly creates MapCreatePayload with the correct structure for map initialization following RTO11f4 specification.
14-14: LGTM!The changes establish strong type safety by replacing
AnywithLiveMapValuethroughout the API. The synchronous methods properly delegate to async counterparts usingrunBlocking, maintaining consistency with the overall async-first design pattern.Also applies to: 17-17, 47-47, 49-49, 60-60, 63-63, 82-82, 96-96, 98-98, 100-102, 104-106
119-142: LGTM!The
setAsyncimplementation properly follows RTLM20 specifications with comprehensive validation, correct ObjectMessage construction, and appropriate error handling. The use offromLiveMapValueensures type-safe conversion to ObjectData.
144-164: LGTM!The
removeAsyncimplementation consistently follows RTLM21 specifications with proper validation and ObjectMessage construction. The ObjectMapOp correctly omits data for removal operations, maintaining consistency with thesetAsyncpattern.
200-248: LGTM!The companion object methods comprehensively implement the required functionality:
initialValueproperly creates MapCreatePayload per RTO11f4fromLiveMapValuehandles all supported LiveMapValue types per RTLM20e5- Correct distinction between object references and primitive values
- Appropriate error handling for unsupported types
The type conversion logic is thorough and maintains the integrity of the LiveMapValue type system.
|
@coderabbitai resolve |
✅ Actions performedComments resolved. |
|
closing in favor of #1141 |
1 participant
LiveMapValueas a concrete type representing a map valueReason to choose
of()methods return immutable objectsSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Refactor
Chores