[ECO-5386] Liveobjects serialization #1109
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Added LiveObjectSerializer interface consisting methods for json and msgpack serialization. 2. Added LiveObjectsHelper to initialize LiveObjectsPlugin and LiveObjectSerializer 3. Updated code to use LiveObjectSerializer and LiveObjectsHelper to serialize and initialize liveobjects
1. Implemented JsonSerializetion using gson library 2. Implemented MsgpackSerialization without external dependency 3. Annotated/updated ObjectMessage fields for gson serialization 4. Added msgpack dependency to liveobjects
1. Created dummy objectmessage fixture for different data types 2. Implemented unit tests with given fixture for various cases
WalkthroughThis change introduces a comprehensive serialization framework for LiveObjects, including both JSON and MessagePack support. It adds interfaces, helpers, serializers, and adapters for serializing and deserializing LiveObject-related data structures, integrates them into the core protocol message flow, and provides extensive unit tests and fixtures for correctness. Changes
Sequence Diagram(s)sequenceDiagram
participant AblyRealtime
participant LiveObjectsHelper
participant LiveObjectSerializer
participant ProtocolMessage
participant MsgPack/JSON
AblyRealtime->>LiveObjectsHelper: tryInitializeLiveObjectsPlugin(this)
LiveObjectsHelper->>LiveObjectSerializer: getLiveObjectSerializer()
ProtocolMessage->>LiveObjectsHelper: getLiveObjectSerializer()
ProtocolMessage->>LiveObjectSerializer: writeMsgpackArray(state, packer)
ProtocolMessage->>LiveObjectSerializer: asJsonArray(state)
MsgPack/JSON-->>ProtocolMessage: Data (state field serialized/deserialized)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related issues
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 (
|
sacOO7
commented
Jun 30, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
lib/src/main/java/io/ably/lib/objects/LiveObjectsHelper.java (1)
16-16: Consider renaming the anonymous class from "Adapter".The name "Adapter" is misleading as this is not implementing the adapter pattern. Consider using a more descriptive name or simply use the anonymous class without a name.
- LiveObjectsAdapter adapter = new Adapter(ablyRealtime); + LiveObjectsAdapter adapter = new LiveObjectsAdapter() { + // implementation would go here if it's an anonymous class + };live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSerializationTest.kt (1)
55-56: Fix incorrect comment.The comment incorrectly mentions "MsgPack format" when this is actually JSON serialization.
- // Serialize the ProtocolMessage containing ObjectMessages to MsgPack format + // Serialize the ProtocolMessage containing ObjectMessages to JSON format val serializedProtoMsg = ProtocolSerializer.writeJSON(protocolMessage).toString(Charsets.UTF_8)live-objects/src/main/kotlin/io/ably/lib/objects/serialization/JsonSerialization.kt (1)
37-40: Improve error handling for unknown enum codesThe current implementation throws a generic
NoSuchElementExceptionwhen an unknown enum code is encountered. Consider providing a more descriptive error message.override fun deserialize(json: JsonElement, typeOfT: Type, context: JsonDeserializationContext): T { val code = json.asInt - return enumValues.first { getCode(it) == code } + return enumValues.firstOrNull { getCode(it) == code } + ?: throw JsonParseException("Unknown enum code: $code for type ${typeOfT.typeName}") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
lib/src/main/java/io/ably/lib/objects/LiveObjectSerializer.java(1 hunks)lib/src/main/java/io/ably/lib/objects/LiveObjectsHelper.java(1 hunks)lib/src/main/java/io/ably/lib/objects/LiveObjectsJsonSerializer.java(1 hunks)lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java(2 hunks)lib/src/main/java/io/ably/lib/types/ProtocolMessage.java(5 hunks)lib/src/main/java/io/ably/lib/types/ProtocolSerializer.java(2 hunks)live-objects/build.gradle.kts(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/Helpers.kt(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/ObjectMessage.kt(5 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/Serialization.kt(0 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/serialization/DefaultSerialization.kt(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/serialization/JsonSerialization.kt(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt(1 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSerializationTest.kt(1 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.kt(4 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/unit/fixtures/ObjectMessageFixtures.kt(1 hunks)
💤 Files with no reviewable changes (1)
- live-objects/src/main/kotlin/io/ably/lib/objects/Serialization.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#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.
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#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#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.
live-objects/build.gradle.kts (3)
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.
Learnt from: sacOO7
PR: ably/ably-java#1095
File: live-objects/src/test/kotlin/io/ably/lib/objects/unit/LiveObjectTest.kt:1-6
Timestamp: 2025-06-05T10:24:28.789Z
Learning: In Kotlin, functions, classes, and other declarations within the same package are automatically accessible without explicit import statements. Do not suggest adding imports for functions/classes that are already in the same package.
Learnt from: sacOO7
PR: ably/ably-java#1095
File: gradle/libs.versions.toml:24-26
Timestamp: 2025-06-05T10:27:53.946Z
Learning: The ably-java project prefers to use the latest available versions of testing dependencies (including pre-release versions) when they contain relevant bug fixes, rather than sticking strictly to stable releases.
live-objects/src/main/kotlin/io/ably/lib/objects/Helpers.kt (1)
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.
lib/src/main/java/io/ably/lib/types/ProtocolSerializer.java (4)
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#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.
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.
lib/src/main/java/io/ably/lib/types/ProtocolMessage.java (5)
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#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.
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#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.
lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java (7)
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#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#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#1043
File: lib/src/main/java/io/ably/lib/realtime/Presence.java:318-320
Timestamp: 2024-10-16T10:05:29.809Z
Learning: In `lib/src/main/java/io/ably/lib/realtime/Presence.java`, when reviewing methods like `implicitAttachOnSubscribe()`, ensure that variables declared and initialized within conditional blocks (such as `errorString`) are not incorrectly flagged as potentially uninitialized when they are always initialized before use within their scope. In Java, variables declared within a block are scoped to that block, so they won't cause a `NullPointerException` if properly used.
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#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#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.
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/DefaultSerialization.kt (4)
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#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.
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.
lib/src/main/java/io/ably/lib/objects/LiveObjectsHelper.java (6)
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#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#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#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.
lib/src/main/java/io/ably/lib/objects/LiveObjectSerializer.java (6)
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#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#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.
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#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.
live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSerializationTest.kt (4)
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#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.
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.
live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.kt (5)
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#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#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#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.
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.
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/JsonSerialization.kt (4)
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#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/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.
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.
live-objects/src/test/kotlin/io/ably/lib/objects/unit/fixtures/ObjectMessageFixtures.kt (4)
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#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#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#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.
lib/src/main/java/io/ably/lib/objects/LiveObjectsJsonSerializer.java (5)
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#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#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#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.
live-objects/src/main/kotlin/io/ably/lib/objects/ObjectMessage.kt (4)
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#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.
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.
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt (3)
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#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.
🧬 Code Graph Analysis (3)
lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java (1)
lib/src/main/java/io/ably/lib/objects/LiveObjectsHelper.java (1)
LiveObjectsHelper(8-43)
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/DefaultSerialization.kt (1)
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt (1)
readObjectMessage(93-145)
lib/src/main/java/io/ably/lib/objects/LiveObjectSerializer.java (1)
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/DefaultSerialization.kt (3)
writeMsgpackArray(25-29)readFromJsonArray(31-36)asJsonArray(38-45)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: check-realtime-okhttp
- GitHub Check: check (24)
- GitHub Check: check-rest-okhttp
- GitHub Check: check (21)
- GitHub Check: check (19)
- GitHub Check: check (29)
- GitHub Check: check-realtime
- GitHub Check: check-rest
🔇 Additional comments (32)
live-objects/build.gradle.kts (1)
14-16: No duplicate coroutines dependency detectedThe
commonbundle inlibs.versions.tomlonly includesmsgpackandvcdiff-coreand does not transitively exportkotlinx-coroutines-core. There’s no overlap with your explicitimplementation(libs.coroutine.core), so no exclusion or further action is needed.live-objects/src/main/kotlin/io/ably/lib/objects/Helpers.kt (3)
42-42: LGTM! Excellent type safety improvement.The change from nullable to non-nullable
ByteArrayaligns with the design decision that calling sites ensure objects are never null, eliminating unnecessary null safety overhead.
46-51: Well-simplified equality and hash implementations.The removal of null checks and direct use of
contentEqualsandcontentHashCodeis correct and more efficient given the non-nullable data property.
55-55: Correctly updated extension function.The size calculation is now properly simplified to work with the non-nullable
dataproperty.live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSizeTest.kt (3)
3-3: Appropriate imports for updated serialization approach.The addition of
JsonObjectandtoByteArrayimports supports the transition to usingJsonObjectfor extras and the updatedBinaryclass usage.Also applies to: 21-21
37-40: Correct transition from Map to JsonObject for extras.The change preserves the same key-value pairs (
{"meta":"data","count":42}) while usingJsonObjectinstead of a KotlinMap, maintaining the 26-byte size calculation accuracy.
84-84: Updated Binary usage aligns with non-nullable changes.The change from
Binary("initial".toByteArray())toBinary("some-value".toByteArray())correctly uses the updated non-nullableBinaryconstructor and maintains the test's intent.lib/src/main/java/io/ably/lib/types/ProtocolSerializer.java (2)
31-31: Improved error handling with exception declaration.The method signature now properly declares that it throws
AblyException, making the error contract explicit for callers.
38-40: Better error propagation instead of returning null.The change from returning
nullon IOException to throwingAblyExceptionprovides better error information and prevents silent failures, improving debugging capabilities.lib/src/main/java/io/ably/lib/objects/LiveObjectSerializer.java (1)
14-51: Excellent interface design for LiveObjects serialization.The interface provides a clean abstraction with well-defined methods for both MessagePack and JSON serialization. Key strengths:
- Clear separation between read/write operations
- Consistent use of
@NotNullannotations aligning with type safety improvements- Comprehensive Javadoc documentation
- IOException handling for MessagePack operations where appropriate
The interface design enables modular serialization implementations while maintaining type safety.
lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java (2)
8-8: Appropriate import for centralized helper usage.Adding the
LiveObjectsHelperimport supports the architectural improvement of centralizing plugin initialization logic.
75-75: Excellent refactoring to centralize plugin initialization.Replacing direct reflection with
LiveObjectsHelper.tryInitializeLiveObjectsPlugin(this)provides several benefits:
- Removes complex reflection code from
AblyRealtime- Centralizes LiveObjects initialization logic in a dedicated helper
- Improves maintainability and testability
- Aligns with the broader LiveObjects serialization infrastructure
This is a clean architectural improvement that simplifies the main class while providing better separation of concerns.
lib/src/main/java/io/ably/lib/types/ProtocolMessage.java (3)
131-138: LGTM! Well-documented field declaration.The
statefield is properly annotated with@Nullableand uses the appropriate JSON adapter. The documentation clearly explains when this field might be null.
163-163: LGTM! Proper conditional serialization handling.The code correctly handles the conditional serialization of the
statefield, ensuring it's only included when both the field and serializer are available. The warning log provides good visibility when serialization is skipped.Also applies to: 203-211
274-282: LGTM! Consistent deserialization handling.The deserialization logic properly mirrors the serialization approach, with appropriate null checks and logging when the serializer is unavailable.
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/DefaultSerialization.kt (4)
20-23: LGTM! Clean msgpack array deserialization.The method correctly reads the array header and deserializes each element using
readObjectMessage.
25-29: LGTM! Intentional unsafe casting as per design.The unsafe cast is a deliberate design decision as documented in the learnings, allowing ClassCastException to occur naturally for type mismatches.
31-36: LGTM! Proper JSON array deserialization with error handling.The method correctly validates that each element is a JsonObject and provides a clear error message when encountering unexpected element types.
38-45: LGTM! Consistent implementation with msgpack serialization.The method follows the same pattern as
writeMsgpackArraywith intentional unsafe casting.lib/src/main/java/io/ably/lib/objects/LiveObjectsJsonSerializer.java (3)
14-17: LGTM! Proper caching of serializer instance.The serializer is obtained once during field initialization and cached, which is efficient.
19-28: LGTM! Robust deserialization with proper error handling.The method handles missing serializer gracefully and validates the JSON structure before processing.
31-37: LGTM! Consistent serialization handling.The method properly returns
JsonNull.INSTANCEwhen the serializer is unavailable, maintaining consistency with the deserialization approach.live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSerializationTest.kt (4)
30-47: LGTM! Comprehensive MsgPack serialization test.The test properly validates round-trip serialization/deserialization with element-by-element comparison.
68-101: LGTM! Thorough null field omission test.The test effectively verifies that null fields are properly omitted during both JSON and MsgPack serialization.
103-125: LGTM! Proper enum serialization validation.The test correctly verifies that enum values are serialized as their ordinal integers.
127-179: LGTM! Comprehensive null handling test coverage.The test thoroughly validates null handling at multiple levels, including protocol message state and nested fields within ObjectMessage.
live-objects/src/test/kotlin/io/ably/lib/objects/unit/fixtures/ObjectMessageFixtures.kt (1)
1-176: Well-structured test fixtures!The test fixtures provide comprehensive coverage for all
ObjectValuetypes and maintain consistency throughout the object hierarchy. The helper functions effectively demonstrate the propagation of different data types through the nested structures.live-objects/src/main/kotlin/io/ably/lib/objects/serialization/JsonSerialization.kt (1)
43-89: Correct implementation of polymorphic ObjectData serializationThe serializer properly handles all value types with appropriate encoding strategies:
- Base64 for binary data
- Special JSON encoding for JsonObject/JsonArray as specified in OD4c5 and OD5b3
- Proper null handling
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt (2)
112-112: Good use of string interning for performanceThe consistent use of
intern()on field names during deserialization is a good optimization that reduces memory usage and improves string comparison performance in switch statements.Also applies to: 226-226, 329-329, 439-439, 492-492, 548-548, 604-604, 684-684
1-723: Comprehensive MessagePack serialization implementationThe implementation demonstrates excellent practices:
- Proper nil value handling
- Unknown field skipping for forward compatibility
- Clear error messages for invalid enum codes
- Efficient field counting to minimize map header size
live-objects/src/main/kotlin/io/ably/lib/objects/ObjectMessage.kt (2)
318-318: Type safety improvement for extras fieldChanging the
extrasfield type fromAny?toJsonObject?improves type safety and ensures proper JSON serialization. This aligns with the spec requirement for JSON-encodable objects.
6-11: Well-integrated serialization annotationsThe Gson annotations are properly configured:
- Custom serializers for polymorphic types (ObjectData, Binary)
- Correct field naming with
@SerializedName("object")- Clear deprecation notice for
initialValueEncodingAlso applies to: 37-37, 221-221, 227-227, 334-334
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt (1)
152-153: objectId validation already addressedThe previous review comment about enforcing non-empty objectId is valid and addresses this concern.
🧹 Nitpick comments (1)
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt (1)
626-723: Consider refactoring ObjectData serialization for better maintainabilityThe serialization logic for
ObjectDatais complex with multiple conditional branches. Consider extracting the value serialization into separate functions for each type.Example refactoring approach:
private fun ObjectData.writeMsgpack(packer: MessagePacker) { var fieldCount = calculateFieldCount() packer.packMapHeader(fieldCount) objectId?.let { writeObjectId(packer, it) } value?.let { writeValue(packer, it) } } private fun writeValue(packer: MessagePacker, value: ObjectValue) { when (val v = value.value) { is Boolean -> writeBoolean(packer, v) is String -> writeString(packer, v) is Number -> writeNumber(packer, v) is Binary -> writeBinary(packer, v) is JsonObject, is JsonArray -> writeJson(packer, v) } } // Similar refactoring for the reader side
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/src/main/java/io/ably/lib/objects/LiveObjectsHelper.java(1 hunks)live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt(1 hunks)live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSerializationTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- live-objects/src/test/kotlin/io/ably/lib/objects/unit/ObjectMessageSerializationTest.kt
- lib/src/main/java/io/ably/lib/objects/LiveObjectsHelper.java
🧰 Additional context used
🧠 Learnings (2)
📓 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#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.
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#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#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.
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt (3)
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#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.
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: check (19)
- GitHub Check: check (21)
- GitHub Check: check (29)
- GitHub Check: check (24)
- GitHub Check: check-realtime-okhttp
- GitHub Check: check
- GitHub Check: check-rest-okhttp
- GitHub Check: check-liveobjects
- GitHub Check: check-rest
- GitHub Check: check-realtime
- GitHub Check: build
🔇 Additional comments (2)
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt (2)
417-428: Verify empty map serialization for ObjectCounterOpWhen
amountis null, this serializes as an empty MessagePack map{}. Is this intentional, or should it serialize as nil to save space?
526-537: Verify empty map serialization for ObjectCounterSimilar to
ObjectCounterOp, this serializes as an empty MessagePack map{}whencountis null. Confirm if this is the intended behavior.
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt
Show resolved
Hide resolved
live-objects/src/main/kotlin/io/ably/lib/objects/serialization/MsgpackSerialization.kt
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixed #1098
Decided to have separate serialization over just single gson serialization because =>
msgpack, needs two layers of serialization, first is always json and second is tomsgpack.binaryfield )ObjectMessageencoding and decoding specification#335 (comment)Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores