[AIT-1009] Implement Json and MsgPack serializers for path-based LiveObjects#1218
Conversation
WalkthroughIntroduces a ChangesLiveObjects Wire Serialization and Plugin Interface
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
Note over AblyRealtime,LiveObjectsPlugin: Plugin initialization
end
AblyRealtime->>LiveObjectsPlugin.Factory: tryInitialize(ablyRealtime)
LiveObjectsPlugin.Factory->>DefaultLiveObjectsPlugin: Class.forName + constructor(AblyClientAdapter)
DefaultLiveObjectsPlugin-->>LiveObjectsPlugin.Factory: instance or null
LiveObjectsPlugin.Factory-->>AblyRealtime: LiveObjectsPlugin or null
rect rgba(60, 179, 113, 0.5)
Note over Gson,DefaultObjectsSerializer: Wire message (de)serialization
end
Gson->>ObjectJsonSerializer: deserialize(JsonElement, Object[])
ObjectJsonSerializer->>ObjectSerializer.Holder: tryGet()
ObjectSerializer.Holder->>DefaultObjectsSerializer: reflective load + cache
DefaultObjectsSerializer-->>ObjectSerializer.Holder: ObjectSerializer
ObjectSerializer.Holder-->>ObjectJsonSerializer: ObjectSerializer
ObjectJsonSerializer->>DefaultObjectsSerializer: readFromJsonArray(json)
DefaultObjectsSerializer->>JsonSerialization: toObjectMessage() per element
JsonSerialization-->>DefaultObjectsSerializer: WireObjectMessage[]
DefaultObjectsSerializer-->>ObjectJsonSerializer: Object[]
ObjectJsonSerializer-->>Gson: Object[]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Implemented JsonSerializer annotation for better json handling
Point the JSON and MsgPack serializers in io.ably.lib.object.serialization
at the new WireObjectMessage wire model instead of the legacy
io.ably.lib.objects.ObjectMessage, so the new `object` package has no
dependency on the legacy `objects` package.
- DefaultSerialization: implement the new ObjectSerializer interface and
(de)serialize WireObjectMessage arrays (reflectively loaded via
ObjectSerializer.Holder).
- Json/MsgpackSerialization: bind the Wire* types; replace legacy
objectError with the object package's objectStateError (same 500/92000).
- WireObjectMessage: restore the gson annotations required for wire-format
fidelity - @SerializedName("object") on objectState and
@JsonAdapter(WireObjectDataJsonSerializer) on WireObjectData.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixes checkstyle AvoidStarImport violation on com.google.gson.*. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b69d950 to
bfa574f
Compare
There was a problem hiding this comment.
Pull request overview
Adds the wire-format serialization layer for the path-based LiveObjects API (io.ably.lib.object), providing JSON and MessagePack codecs for the Wire* object-message model while keeping the core lib module decoupled from the optional liveobjects plugin via reflective loading.
Changes:
- Introduces
ObjectSerializer(+ObjectJsonSerializer) inlib, with a lazy reflectively-loaded plugin implementation. - Adds
DefaultObjectsSerializerplus JSON/MsgPack encoding/decoding forWireObjectMessagearrays inliveobjects. - Updates the
Wire*model with Gson annotations needed for wire-format fidelity (e.g.,"object"field name andWireObjectDataJSON-as-string handling).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| liveobjects/src/main/kotlin/io/ably/lib/object/serialization/MsgpackSerialization.kt | MessagePack (de)serialization for the path-based Wire* object model. |
| liveobjects/src/main/kotlin/io/ably/lib/object/serialization/JsonSerialization.kt | Gson configuration + enum-by-code adapters + WireObjectData JSON-as-string adapter. |
| liveobjects/src/main/kotlin/io/ably/lib/object/serialization/DefaultSerialization.kt | Plugin-side ObjectSerializer implementation for JSON/MsgPack arrays of WireObjectMessage. |
| liveobjects/src/main/kotlin/io/ably/lib/object/message/WireObjectMessage.kt | Wire model updates: Gson annotations for "object" and WireObjectData adapter. |
| lib/src/main/java/io/ably/lib/object/serialization/ObjectSerializer.java | Core-side serializer interface + reflective singleton loader for plugin implementation. |
| lib/src/main/java/io/ably/lib/object/serialization/ObjectJsonSerializer.java | Gson serializer/deserializer for the protocol state field via ObjectSerializer.tryGet(). |
| lib/src/main/java/io/ably/lib/object/LiveObjectsPlugin.java | Core-side reflective plugin initializer interface for LiveObjects functionality. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
liveobjects/src/main/kotlin/io/ably/lib/object/serialization/DefaultSerialization.kt (1)
21-36: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winValidate element types before casting object arrays.
Line 22 and Line 35 use unchecked casts from
AnytoWireObjectMessage; malformed upstream arrays will fail with opaqueClassCastException. A typed check gives deterministic, actionable failures.Suggested patch
override fun writeMsgpackArray(objects: Array<out Any>, packer: MessagePacker) { - val objectMessages = objects.map { it as WireObjectMessage } + val objectMessages = objects.mapIndexed { index, value -> + value as? WireObjectMessage + ?: throw IllegalArgumentException("Expected WireObjectMessage at index $index, got ${value::class.java.name}") + } packer.packArrayHeader(objectMessages.size) objectMessages.forEach { it.writeMsgpack(packer) } } @@ override fun asJsonArray(objects: Array<out Any>): JsonArray { - val objectMessages = objects.map { it as WireObjectMessage } + val objectMessages = objects.mapIndexed { index, value -> + value as? WireObjectMessage + ?: throw IllegalArgumentException("Expected WireObjectMessage at index $index, got ${value::class.java.name}") + } val jsonArray = JsonArray() for (objectMessage in objectMessages) { jsonArray.add(objectMessage.toJsonObject()) } return jsonArray }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@liveobjects/src/main/kotlin/io/ably/lib/object/serialization/DefaultSerialization.kt` around lines 21 - 36, The methods writeMsgpackArray and asJsonArray both contain unchecked casts to WireObjectMessage without validating the input types, which can result in opaque ClassCastException errors. Add type validation before casting in both methods by checking if each element is an instance of WireObjectMessage using the is operator or as? operator, and throw a descriptive exception with the actual element type if validation fails, similar to the pattern already implemented in readFromJsonArray where you check isJsonObject and throw JsonParseException.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/src/main/java/io/ably/lib/object/LiveObjectsPlugin.java`:
- Around line 96-105: The catch block in the plugin initialization code (around
the getDeclaredConstructor call and cast to LiveObjectsPlugin) does not handle
ClassCastException and LinkageError, which can be thrown during reflection and
type loading if the implementation exists but is incompatible. This causes the
optional LiveObjects feature to fail hard instead of gracefully degrading.
Extend the catch clause to also catch ClassCastException and LinkageError
alongside the existing exception types (ClassNotFoundException,
InstantiationException, IllegalAccessException, NoSuchMethodException,
InvocationTargetException) so the method returns null when any of these errors
occur, maintaining the resilience of this optional plugin.
In `@lib/src/main/java/io/ably/lib/object/serialization/ObjectSerializer.java`:
- Around line 83-90: The reflective loading of the ObjectSerializer class on
lines 84-85 can throw ClassCastException or LinkageError when the class exists
but is incompatible or incompletely linked, but these exceptions are not
currently caught in the catch block. Add ClassCastException and LinkageError to
the multi-catch exception handler in the ObjectSerializer reflective loading
section so these failures are gracefully handled the same way as other expected
exceptions, allowing the optional plugin to be disabled instead of crashing the
runtime message handling.
In
`@liveobjects/src/main/kotlin/io/ably/lib/object/serialization/MsgpackSerialization.kt`:
- Around line 356-407: Add validation for the required fields `siteTimeserials`
and `tombstone` in the readObjectState function. Currently these fields have
default values (empty map and false respectively), which prevents detection of
missing required fields per the WireObjectState spec. Introduce boolean flags to
track whether these fields were encountered during unpacking, then add
validation checks after the loop (similar to the existing objectId validation)
to throw an objectStateError if either siteTimeserials or tombstone is missing
from the wire payload.
---
Nitpick comments:
In
`@liveobjects/src/main/kotlin/io/ably/lib/object/serialization/DefaultSerialization.kt`:
- Around line 21-36: The methods writeMsgpackArray and asJsonArray both contain
unchecked casts to WireObjectMessage without validating the input types, which
can result in opaque ClassCastException errors. Add type validation before
casting in both methods by checking if each element is an instance of
WireObjectMessage using the is operator or as? operator, and throw a descriptive
exception with the actual element type if validation fails, similar to the
pattern already implemented in readFromJsonArray where you check isJsonObject
and throw JsonParseException.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c3a8978-b914-4dc6-ac31-9091b13905f6
📒 Files selected for processing (7)
lib/src/main/java/io/ably/lib/object/LiveObjectsPlugin.javalib/src/main/java/io/ably/lib/object/serialization/ObjectJsonSerializer.javalib/src/main/java/io/ably/lib/object/serialization/ObjectSerializer.javaliveobjects/src/main/kotlin/io/ably/lib/object/message/WireObjectMessage.ktliveobjects/src/main/kotlin/io/ably/lib/object/serialization/DefaultSerialization.ktliveobjects/src/main/kotlin/io/ably/lib/object/serialization/JsonSerialization.ktliveobjects/src/main/kotlin/io/ably/lib/object/serialization/MsgpackSerialization.kt
Summary
Adds the serialization layer for the path-based LiveObjects API (
io.ably.lib.object), supporting both JSON and MessagePack encoding/decoding of object messages. This is the wire-format plumbing that sits underneath the path-based public API.The design keeps the core
libfree of any hard dependency on the optional LiveObjects plugin: the core declares interfaces and reflectively loads the implementation from theliveobjectsmodule at runtime, returningnullwhen the plugin is absent.Core (
lib)ObjectSerializer— public interface declaring the four codec entry points (readMsgpackArray/writeMsgpackArray/readFromJsonArray/asJsonArray). The implementation is loaded reflectively and cached as a lazy singleton via a nestedHolder(double-checked locking; retries until the plugin is present), exposed through the staticObjectSerializer.tryGet().ObjectJsonSerializer— GsonJsonSerializer/JsonDeserializerfor the protocol-messagestatefield; delegates to the loadedObjectSerializer, and no-ops gracefully (logs +JsonNull) when the plugin is unavailable.Plugin (
liveobjects)DefaultObjectsSerializer— implementsObjectSerializer, (de)serializing arrays of object messages for JSON and MessagePack.JsonSerialization— Gson setup: enum-by-code adapters andWireObjectDataJsonSerializer(handles thejson-as-string encoding per OD4c5 and the "at least one value field" validation).MsgpackSerialization— hand-rolled MessagePack pack/unpack for the full object model (operations, state, map/counter payloads, map entries, object data).Wire model decoupling
The serializers operate on the
WireObjectMessagemodel inio.ably.lib.object.message, so the newobjectpackage carries no dependency on the legacyio.ably.lib.objectspackage.WireObjectMessagecarries the Gson annotations needed for wire-format fidelity —@SerializedName("object")onobjectState(OM2g) and@JsonAdapter(WireObjectDataJsonSerializer)onWireObjectData.Notes
objectpackage'sobjectStateError(ErrorInfo 500 / 92000).Wire*model are tracked as a follow-up.Spec: OM*, OOP*, OD*, OMP*, OCN*, OST*, MCR*, MST*, MRM*, CCR*, CIN*, ODE*, MCL*
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
LiveObjectsPlugininterface to enable channel-specific live object management with lifecycle and event-handling capabilities