Change ObjectOperation.initialValue to be a JSON encoded string

[VeskeR]

This updates ably-js SDK to use simpler initialValue API added to realtime in [1], based on the LiveObjects DR [2]

[1] https://github.com/ably/realtime/pull/7549
[2] https://ably.atlassian.net/wiki/x/GID-9Q

Summary by CodeRabbit

• Refactor
  • Initial values for counters and maps now use JSON string serialization instead of binary encoding.
  • Object identifiers and creation messages are generated from the JSON initial value.
  • Encoding APIs simplified and related method names clarified to reflect operation-based initial values.
• Tests
  • Updated tests to use JSON-string initial values and to match the new encoding behavior.

[coderabbitai]

Walkthrough

Initial-value handling for live objects is moved from binary buffers to JSON strings. Signatures and helpers across LiveCounter, LiveMap, ObjectId, and ObjectMessage were renamed/adjusted to produce and consume JSON-stringified initialValue; encoding types and binary initial-value APIs were removed. Tests updated accordingly.

Changes

Cohort / File(s)	Change Summary
LiveCounter initial-value update src/plugins/objects/livecounter.ts	Replaced binary initial-value encoding with JSON string path: uses createInitialValueOperation and createInitialValueJSONString; createCounterCreateMessage now sets initialValue to a JSON string and uses it for ObjectId.fromInitialValue.
LiveMap initial-value update src/plugins/objects/livemap.ts	Same refactor as LiveCounter: createInitialValueOperation + createInitialValueJSONString used; initial map state serialized to JSON string and used for ObjectId and object create message.
ObjectId initial-value API src/plugins/objects/objectid.ts	fromInitialValue now accepts initialValue: string (JSON) instead of a buffer-like value and UTF-8 encodes it internally before hashing.
ObjectMessage serialization overhaul src/plugins/objects/objectmessage.ts	Removed binary initial-value encoding and related types; initialValue on operations is now `string
Tests test/realtime/objects.test.js	Updated test case to provide operation.initialValue as a JSON string and removed initialValueEncoding.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant LiveObject as LiveCounter/LiveMap
    participant ObjectMessage
    participant ObjectId

    Client->>LiveObject: request create (e.g. count or entries)
    LiveObject->>ObjectMessage: build operation (createInitialValueOperation)
    LiveObject->>ObjectMessage: createInitialValueJSONString(operation, client)
    ObjectMessage-->>LiveObject: initialValueJSONString
    LiveObject->>ObjectId: fromInitialValue(platform, type, initialValueJSONString, nonce, ts)
    ObjectId-->>LiveObject: objectId
    LiveObject-->>Client: ObjectMessage { initialValue: initialValueJSONString, objectId, ... }

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

I nibble on bytes and hop through the trees,
Turning buffers to JSON with elegant ease.
Counters and maps now sing in a string,
ObjectIds hashed, ready to spring.
A rabbit-approved patch — hop, commit, please! 🐇✨

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.38.6)

test/realtime/objects.test.js

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch initialvalue-jsonstring

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[sacOO7]

LGTM

[mschristensen]

LGTM

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

src/plugins/objects/objectmessage.ts (3)

146-155: initialValue should be treated as write-only; strip on decode and document construction path

Good addition and clear spec notes. Two follow-ups recommended:

• Runtime: prevent leaking initialValue back to app code if it ever arrives from the server (spec says “must not be read by the client”).
• Docs: nudge SDK users and internal callsites to always build this via createInitialValueJSONString(...) to ensure proper canonicalization and bytes handling.

Apply these small changes:

   /**
    * The initial value of the object, represented as a JSON string.
    * Used along with the nonce and timestamp to create the object ID.
    *
    * This field must be set by the client for MAP_CREATE and COUNTER_CREATE operations.
    * The server uses it to verify the object ID, and after verification, the JSON string
    * is decoded into the initial value for new Map or Counter objects.
    *
-   * This field must not be read by the client if received from the server.
+   * This field must not be read by the client if received from the server.
+   *
+   * Important: Construct this string via {@link createInitialValueJSONString} to ensure
+   * consistent JSON encoding (including proper bytes/base64 handling).
    */

And (outside this hunk) strip it on decode before returning to user code:

// place just before `return result;` in WireObjectMessage.decode()
if (result.operation && 'initialValue' in result.operation) {
  delete (result.operation as any).initialValue;
}

I can open a follow-up PR to add a unit test that asserts initialValue is absent on decoded messages.

---

241-246: Trivial pass-through helper — consider inlining (optional)

encodeObjectData simply forwards to encodeFn. You could inline the call sites for less indirection, unless you prefer the semantic name for clarity.

---

247-276: createInitialValueJSONString: defensively exclude stale nested initialValue and sanitize input

Great approach: encode as a wire operation first, then stringify for JSON. Two low-risk hardening tweaks:

• Don’t accidentally embed a stale operation.initialValue inside itself if a caller reuses an ObjectOperation object.
• (Optional) Validate the action is one of the *_CREATE actions to catch misuse early.

Apply this minimal diff:

 export function createInitialValueJSONString(
   operation: Partial<ObjectOperation<ObjectData>>,
   client: BaseClient,
 ): string {
+  // Avoid embedding a stale `initialValue` inside itself if present on the input
+  // (e.g. when reusing a previously prepared operation object).
+  const { initialValue: _ignored, ...opWithoutInitialValue } = operation as any;

   // the object operation may contain user-provided data that requires encoding.
   // for example, buffers must be encoded since the initial value will be represented as a JSON string.
   // we can use ObjectMessage methods to encode the object operation.
   const msg = ObjectMessage.fromValues(
-    { operation: operation as ObjectOperation<ObjectData> },
+    { operation: opWithoutInitialValue as ObjectOperation<ObjectData> },
     client.Utils,
     client.MessageEncoding,
   );
   const wireMsg = msg.encode(client);

   // get the encoded operation that is safe to be sent over the wire as a JSON string.
   const { operation: encodedOperation } = wireMsg.encodeForWire(client.Utils.Format.json);

   // finally, initialValue is the JSON string representation of the encoded operation.
   return JSON.stringify(encodedOperation);
 }

If you want to enforce the *_CREATE-only precondition, we can add a lightweight guard that logs via client.Logger when action is neither MAP_CREATE nor COUNTER_CREATE.

I can add unit tests that cover: (1) buffers inside map entries are base64 in the output JSON, (2) nested initialValue is not included, (3) non-create actions are rejected/logged.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ad89800 and c5c8076.

📒 Files selected for processing (5)

• src/plugins/objects/livecounter.ts (3 hunks)
• src/plugins/objects/livemap.ts (3 hunks)
• src/plugins/objects/objectid.ts (1 hunks)
• src/plugins/objects/objectmessage.ts (8 hunks)
• test/realtime/objects.test.js (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

• src/plugins/objects/livemap.ts
• test/realtime/objects.test.js
• src/plugins/objects/objectid.ts
• src/plugins/objects/livecounter.ts

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: VeskeR
PR: ably/ably-js#1924
File: src/plugins/liveobjects/statemessage.ts:220-224
Timestamp: 2024-11-22T17:15:25.075Z
Learning: In `src/plugins/liveobjects/statemessage.ts`, using `JSON.parse(JSON.stringify(...))` for deep copying is acceptable when buffer values are preserved in subsequent code.

⏰ 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). (7)

• GitHub Check: test-browser (webkit)
• GitHub Check: test-browser (firefox)
• GitHub Check: test-node (16.x)
• GitHub Check: test-node (18.x)
• GitHub Check: test-browser (chromium)
• GitHub Check: test-npm-package
• GitHub Check: test-node (20.x)

🔇 Additional comments (6)

src/plugins/objects/objectmessage.ts (6)

7-7: Broadened encoder input type — LGTM

Allowing EncodeObjectDataFunction to accept either ObjectData or WireObjectData simplifies reuse across pre-wire and wire-encoding paths.

---

187-193: encode(...) signature update — LGTM

Switching to a single EncodeObjectDataFunction keeps the API narrow and avoids the need for a separate “initial value” encoder.

---

223-226: encodeMapEntries signature simplification — LGTM

Consistent use of EncodeObjectDataFunction across helpers improves readability.

---

389-410: ObjectMessage.encode: bytes/JSON encoding path — LGTM

Encoding covers Bufferlike, primitives, and JSON objects correctly, deferring bytes Base64 to wire encoding. This aligns with our previous learning about deep copying with JSON while preserving original buffers for encoding.

---

551-552: toJSON delegates to instance encodeForWire — LGTM

Nice containment; removing private fields via destructuring is clean.

---

473-486: No remaining static calls or encoding metadata usages found

Both verification steps confirmed that:

• There are no static invocations of WireObjectMessage.encodeForWire anywhere in the codebase.
• No consumers reference an encoding field on object payloads, so dropping the encoding metadata is safe.

All concerns have been addressed.