[AIT-99] feat: protocol v5 + message appends #1182
Conversation
WalkthroughThe pull request updates message edit and delete APIs to return Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
github-actions
bot
temporarily deployed
to
staging/pull/1182/features
ttypic
force-pushed
the
AIT-99/message-appends
branch
from
9153b20 to
44c71e5
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: 2
🧹 Nitpick comments (3)
lib/src/main/java/io/ably/lib/types/MessageSerializer.java (1)
77-85: Silent null return on IOException.The
write(Message)method returnsnullonIOException, which follows the existing pattern inwriteMsgpackArray. However,asSingleMsgpackRequest(line 51) passes this directly toByteArrayRequestBody, which could cause issues downstream if serialization fails.Consider throwing
AblyExceptioninstead for consistency with other methods that usethrows AblyException:🔎 Suggested improvement:
-public static byte[] write(Message message) { +public static byte[] write(Message message) throws AblyException { try { ByteArrayOutputStream out = new ByteArrayOutputStream(); MessagePacker packer = Serialisation.msgpackPackerConfig.newPacker(out); message.writeMsgpack(packer); packer.flush(); return out.toByteArray(); - } catch(IOException e) { return null; } + } catch(IOException e) { + throw AblyException.fromThrowable(e); + } }lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
20-22: Specify UTF-8 charset explicitly.
new String(packed)uses platform default charset. JSON should be decoded as UTF-8 for consistency and portability.🔎 Suggested fix:
+import java.nio.charset.StandardCharsets; + public static UpdateDeleteResult readFromJson(byte[] packed) throws MessageDecodeException { - return Serialisation.gson.fromJson(new String(packed), UpdateDeleteResult.class); + return Serialisation.gson.fromJson(new String(packed, StandardCharsets.UTF_8), UpdateDeleteResult.class); }lib/src/main/java/io/ably/lib/rest/MessageEditsMixin.java (1)
143-165: Minor Javadoc clarification suggestion.The Javadoc states "Appends message text to the end of the message" but the
Message.datafield can contain other types (byte arrays, JSON objects, etc.). Consider updating to "Appends data to the message" for accuracy, or clarify if only text appending is supported.🔎 Suggested Javadoc update:
/** - * Appends message text to the end of the message. + * Appends data to the end of the message. * * @param message A {@link Message} object containing the serial identifier and data to append.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java(5 hunks)lib/src/main/java/io/ably/lib/rest/ChannelBase.java(5 hunks)lib/src/main/java/io/ably/lib/rest/MessageEditsMixin.java(3 hunks)lib/src/main/java/io/ably/lib/transport/Defaults.java(1 hunks)lib/src/main/java/io/ably/lib/types/MessageAction.java(1 hunks)lib/src/main/java/io/ably/lib/types/MessageOperationSerializer.java(0 hunks)lib/src/main/java/io/ably/lib/types/MessageSerializer.java(3 hunks)lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java(1 hunks)lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java(1 hunks)lib/src/test/java/io/ably/lib/test/realtime/RealtimeHttpHeaderTest.java(1 hunks)lib/src/test/java/io/ably/lib/test/rest/HttpHeaderTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- lib/src/main/java/io/ably/lib/types/MessageOperationSerializer.java
🧰 Additional context used
🧬 Code graph analysis (5)
lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java (1)
liveobjects/src/main/kotlin/io/ably/lib/objects/ObjectMessage.kt (1)
id(272-345)
lib/src/main/java/io/ably/lib/types/MessageSerializer.java (1)
lib/src/main/java/io/ably/lib/util/Serialisation.java (1)
Serialisation(40-299)
lib/src/main/java/io/ably/lib/rest/MessageEditsMixin.java (6)
lib/src/main/java/io/ably/lib/types/MessageOperation.java (1)
MessageOperation(14-100)lib/src/main/java/io/ably/lib/types/MessageSerializer.java (1)
MessageSerializer(23-242)lib/src/main/java/io/ably/lib/types/MessageVersion.java (1)
MessageVersion(24-232)lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
UpdateDeleteResult(10-71)lib/src/main/java/io/ably/lib/util/Crypto.java (1)
Crypto(25-410)lib/src/main/java/io/ably/lib/http/HttpUtils.java (1)
HttpUtils(28-280)
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (1)
lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
UpdateDeleteResult(10-71)
lib/src/main/java/io/ably/lib/rest/ChannelBase.java (1)
lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
UpdateDeleteResult(10-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: check
- GitHub Check: check (19)
- GitHub Check: check (29)
- GitHub Check: check (29)
- GitHub Check: check (24)
- GitHub Check: check-liveobjects
- GitHub Check: check-realtime-httpurlconnection
- GitHub Check: check-rest-okhttp
- GitHub Check: check (21)
- GitHub Check: check-rest-httpurlconnection
- GitHub Check: check-realtime-okhttp
- GitHub Check: build
🔇 Additional comments (15)
lib/src/test/java/io/ably/lib/test/rest/HttpHeaderTest.java (1)
84-84: LGTM!The test expectation correctly updated to match the protocol version bump to "5".
lib/src/test/java/io/ably/lib/test/realtime/RealtimeHttpHeaderTest.java (1)
83-84: LGTM!The WebSocket version parameter test expectation correctly updated to match protocol version "5".
lib/src/main/java/io/ably/lib/types/MessageAction.java (1)
8-9: New message action types added.The new
MESSAGE_SUMMARY(ordinal 4) andMESSAGE_APPEND(ordinal 5) actions are correctly added. Note that ordinal values are significant for protocol serialization—ensure these match the server-side protocol specification for version 5.lib/src/main/java/io/ably/lib/types/MessageSerializer.java (2)
50-52: LGTM!The
asSingleMsgpackRequestmethod correctly serializes a single message for the new message operations API.
151-153: LGTM!The
asSingleJsonRequestmethod correctly provides JSON serialization for single messages, complementing the existing array-basedasJsonRequest.lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (2)
10-18: LGTM!The class structure with immutable
versionSerialfield is clean and appropriate for representing update/delete/append operation results.
51-70: LGTM!The body handler implementation follows the established pattern in the codebase for handling both JSON and MessagePack content types.
lib/src/main/java/io/ably/lib/transport/Defaults.java (1)
15-15: Verify that Ably service supports protocol version 5 before deployment.This change declares support for protocol version 5, but current Ably SDKs (ably-java and ably-js) only show support up to protocol version 4. Confirm with the Ably team that version 5 is available on their service and document the deployment coordination required.
lib/src/main/java/io/ably/lib/rest/MessageEditsMixin.java (2)
167-205: LGTM! Well-structured shared implementation for update/delete/append operations.The consolidation of update, delete, and append into a single
updateMessageImplmethod with an action parameter is a clean design. The method properly:
- Validates the serial identifier
- Constructs the request with action and version metadata
- Handles both binary and JSON protocols
- Returns the
UpdateDeleteResultwith proper error handling
175-183: LGTM! Proper construction of the augmented message with version metadata.The
MessageVersionis correctly initialized with operation metadata only. Theserialandtimestampfields remain unset as they are populated by the server in the response.lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (3)
1218-1262: LGTM! Consistent API surface for update operations.The update methods properly delegate to
messageEditsMixinand maintain consistent overload patterns for both sync and async variants.
1274-1318: LGTM! Delete operations follow the same consistent pattern.
1320-1366: LGTM! Append operations correctly mirror update/delete patterns.The new append methods are consistent with the existing update and delete API surface, properly delegating to
messageEditsMixin.lib/src/main/java/io/ably/lib/rest/ChannelBase.java (2)
356-504: LGTM! REST ChannelBase maintains API consistency with Realtime ChannelBase.All update, delete, and append operations are properly implemented with:
- Correct return types (
UpdateDeleteResult)- Consistent async callback patterns (
Callback<UpdateDeleteResult>)- Proper delegation to
MessageEditsMixin- Matching overload patterns for optional
MessageOperationparameter
519-532: LGTM! Message versioning API properly exposes paginated history.The
getMessageVersionsandgetMessageVersionsAsyncmethods correctly delegate to the mixin for retrieving historical message versions.
| public static UpdateDeleteResult readMsgpack(MessageUnpacker unpacker) throws IOException { | ||
| int fieldCount = unpacker.unpackMapHeader(); | ||
| String versionSerial = null; | ||
| for(int i = 0; i < fieldCount; i++) { | ||
| String fieldName = unpacker.unpackString().intern(); | ||
| MessageFormat fieldFormat = unpacker.getNextFormat(); | ||
| if(fieldFormat.equals(MessageFormat.NIL)) { | ||
| unpacker.unpackNil(); | ||
| continue; | ||
| } | ||
|
|
||
| if(fieldName.equals(VERSION_SERIAL)) { | ||
| versionSerial = unpacker.unpackString(); | ||
| } | ||
| } | ||
| return new UpdateDeleteResult(versionSerial); |
There was a problem hiding this comment.
Missing skip for unknown fields corrupts MessagePack stream.
When an unknown field is encountered, the code does not consume (skip) its value before continuing to the next field. This will cause incorrect parsing of subsequent fields or an IOException.
🔎 Apply this diff to fix the issue:
public static UpdateDeleteResult readMsgpack(MessageUnpacker unpacker) throws IOException {
int fieldCount = unpacker.unpackMapHeader();
String versionSerial = null;
for(int i = 0; i < fieldCount; i++) {
String fieldName = unpacker.unpackString().intern();
MessageFormat fieldFormat = unpacker.getNextFormat();
if(fieldFormat.equals(MessageFormat.NIL)) {
unpacker.unpackNil();
continue;
}
if(fieldName.equals(VERSION_SERIAL)) {
versionSerial = unpacker.unpackString();
+ } else {
+ unpacker.skipValue();
}
}
return new UpdateDeleteResult(versionSerial);
}🤖 Prompt for AI Agents
In lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java around lines 33
to 48, the reader doesn't skip values for unknown fields which leaves the
MessageUnpacker positioned incorrectly and corrupts parsing; update the loop so
that when a field name is not VERSION_SERIAL (and after handling nil), you call
the MessageUnpacker skip method to consume the field's value (e.g.,
unpacker.skipValue() or the appropriate skip API) before continuing, ensuring
all unknown field payloads are fully skipped and the stream remains in sync.
| Message msg1 = new Message("test_event", "hi there"); | ||
| msg1.id = "msg1"; | ||
| channel.publish(msg1); | ||
| Message msg2 = new Message("test_event1", "hi there1"); | ||
| msg2.id = "msg1"; | ||
| channel.publish(msg2); | ||
| Message msg3 = new Message("test_event2", "hi there2"); | ||
| msg3.id = "msg1"; | ||
| channel.publish(msg3); | ||
| Message msg4 = new Message("test_event4", "hi there4"); | ||
| msg4.id = "msg4"; | ||
| channel.publish(msg4); | ||
| // Expecting two msg: one from the wildcard subscription and one from test_event subscription | ||
| Exception conditionError = new Helpers.ConditionalWaiter(). | ||
| wait(() -> receivedMsg.size() == 2, 5000); | ||
| assertNull(conditionError); | ||
|
|
||
| receivedMsg.clear(); | ||
| channel.publish("test_event1", "hi there"); | ||
| // Expecting two msg: one from the wildcard subscription and one from test_event1 subscription | ||
| conditionError = new Helpers.ConditionalWaiter(). | ||
| wait(() -> receivedMsg.size() == 2, 5000); | ||
| assertNull(conditionError); | ||
|
|
||
| receivedMsg.clear(); | ||
| channel.publish("test_event2", "hi there"); | ||
| // Expecting two msg: one from the wildcard subscription and one from test_event2 subscription | ||
| conditionError = new Helpers.ConditionalWaiter(). | ||
| wait(() -> receivedMsg.size() == 2, 5000); | ||
| assertEquals("msg1", receivedMsg.get(0)); | ||
| assertEquals("msg4", receivedMsg.get(1)); | ||
| assertNull(conditionError); |
There was a problem hiding this comment.
Multiple issues in test logic.
-
Duplicate message IDs (copy-paste error):
msg2.idandmsg3.idare both set to"msg1"instead of"msg2"and"msg3". -
Inconsistent test logic: The test has only one wildcard subscription (line 437) but expects exactly 2 messages. A wildcard subscription should receive all 4 published messages, not 2. The comment mentions "one from the wildcard subscription and one from test_event subscription" but no
test_eventsubscription exists in this test. -
Assertion mismatch: The assertions expect
"msg1"and"msg4", but with a single wildcard subscription, the expected behavior is unclear.
🔎 Suggested fix for IDs (minimum fix):
Message msg2 = new Message("test_event1", "hi there1");
-msg2.id = "msg1";
+msg2.id = "msg2";
channel.publish(msg2);
Message msg3 = new Message("test_event2", "hi there2");
-msg3.id = "msg1";
+msg3.id = "msg3";
channel.publish(msg3);Please clarify the intended test behavior—if the goal is to test filtering, you may need to add a specific event subscription or revise the expected message count.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
33-48: Missing skip for unknown fields corrupts MessagePack stream.This issue was flagged in a previous review but doesn't appear to be fixed. When an unknown field is encountered, the code does not consume (skip) its value before continuing to the next field. This will cause incorrect parsing of subsequent fields or an
IOException.🔎 Apply this diff to fix the issue:
public static UpdateDeleteResult readMsgpack(MessageUnpacker unpacker) throws IOException { int fieldCount = unpacker.unpackMapHeader(); String versionSerial = null; for(int i = 0; i < fieldCount; i++) { String fieldName = unpacker.unpackString().intern(); MessageFormat fieldFormat = unpacker.getNextFormat(); if(fieldFormat.equals(MessageFormat.NIL)) { unpacker.unpackNil(); continue; } if(fieldName.equals(VERSION_SERIAL)) { versionSerial = unpacker.unpackString(); + } else { + unpacker.skipValue(); } } return new UpdateDeleteResult(versionSerial); }
🧹 Nitpick comments (5)
lib/src/main/java/io/ably/lib/types/MessageSerializer.java (1)
50-52: Potential null propagation if serialization fails.The
asSingleMsgpackRequestmethod passes the result ofwrite(message)directly toByteArrayRequestBody. Ifwrite()fails due to anIOException, it returnsnull, which would create a request body with a null byte array.This follows the existing pattern in
writeMsgpackArray(line 65), but consider whether this silent failure is intentional or should propagate the exception.🔎 Consider propagating the exception:
- public static HttpCore.RequestBody asSingleMsgpackRequest(Message message) throws AblyException { - return new HttpUtils.ByteArrayRequestBody(write(message), "application/x-msgpack"); + public static HttpCore.RequestBody asSingleMsgpackRequest(Message message) throws AblyException { + byte[] packed = write(message); + if (packed == null) { + throw AblyException.fromThrowable(new IOException("Failed to serialize message")); + } + return new HttpUtils.ByteArrayRequestBody(packed, "application/x-msgpack"); }lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
55-70: Consider handling unknown content types explicitly.If the
contentTypeis neitherapplication/jsonnorapplication/x-msgpack, the method returns[null]. This matches the pattern inMessageBodyHandler, but callers likeMessageEditsMixin.updateMessageImplwill then throw due to missingversionSerial. Consider adding an explicit check for clarity.🔎 Optional improvement for explicit handling:
public UpdateDeleteResult[] handleResponseBody(String contentType, byte[] body) throws AblyException { try { UpdateDeleteResult updateDeleteResult = null; if("application/json".equals(contentType)) updateDeleteResult = readFromJson(body); else if("application/x-msgpack".equals(contentType)) updateDeleteResult = readMsgpack(body); + else + throw AblyException.fromThrowable(new Exception("Unknown content type: " + contentType)); return new UpdateDeleteResult[] { updateDeleteResult }; } catch (MessageDecodeException e) { throw AblyException.fromThrowable(e); } }lib/src/main/java/io/ably/lib/rest/ChannelBase.java (1)
458-504: Consider clarifying the Javadoc for data type flexibility.The Javadoc states "Appends message text to the end of the message" but the
Messageobject can contain various data types, not just text. Consider updating the documentation to be more accurate about what data types are supported for append operations.🔎 Suggested documentation update:
/** - * Appends message text to the end of the message. + * Appends data to the end of the message. * * @param message A {@link Message} object containing the serial identifier and data to append.lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (2)
1320-1366: Same documentation nit as REST ChannelBase.The Javadoc describes "Appends message text" but the operation supports various data types. Consider updating for consistency with the REST channel.
🔎 Suggested documentation update:
/** - * Appends message text to the end of the message. + * Appends data to the end of the message. * * @param message A {@link Message} object containing the serial identifier and data to append.
1580-1581: Redundant initialization ofmessageEditsMixin.The constructor calls
this.setOptions(options)at line 1567, which already initializesmessageEditsMixinat line 1504. The assignment at line 1580 is therefore redundant and can be removed.🔎 Suggested fix:
this.annotations = new RealtimeAnnotations( this, new RestAnnotations(name, ably.http, ably.options, options) ); - this.messageEditsMixin = new MessageEditsMixin(basePath, ably.options, options, ably.auth);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java(5 hunks)lib/src/main/java/io/ably/lib/rest/ChannelBase.java(5 hunks)lib/src/main/java/io/ably/lib/rest/MessageEditsMixin.java(3 hunks)lib/src/main/java/io/ably/lib/transport/Defaults.java(1 hunks)lib/src/main/java/io/ably/lib/types/MessageAction.java(1 hunks)lib/src/main/java/io/ably/lib/types/MessageOperationSerializer.java(0 hunks)lib/src/main/java/io/ably/lib/types/MessageSerializer.java(3 hunks)lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java(1 hunks)lib/src/test/java/io/ably/lib/test/realtime/RealtimeHttpHeaderTest.java(1 hunks)lib/src/test/java/io/ably/lib/test/rest/HttpHeaderTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- lib/src/main/java/io/ably/lib/types/MessageOperationSerializer.java
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/src/main/java/io/ably/lib/types/MessageAction.java
- lib/src/main/java/io/ably/lib/transport/Defaults.java
🧰 Additional context used
🧬 Code graph analysis (5)
lib/src/main/java/io/ably/lib/types/MessageSerializer.java (1)
lib/src/main/java/io/ably/lib/util/Serialisation.java (1)
Serialisation(40-299)
lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (3)
lib/src/main/java/io/ably/lib/http/HttpCore.java (1)
HttpCore(39-583)lib/src/main/java/io/ably/lib/util/Serialisation.java (1)
Serialisation(40-299)lib/src/main/java/io/ably/lib/types/AblyException.java (1)
AblyException(12-67)
lib/src/main/java/io/ably/lib/rest/ChannelBase.java (1)
lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
UpdateDeleteResult(10-71)
lib/src/main/java/io/ably/lib/rest/MessageEditsMixin.java (6)
lib/src/main/java/io/ably/lib/types/MessageOperation.java (1)
MessageOperation(14-100)lib/src/main/java/io/ably/lib/types/MessageSerializer.java (1)
MessageSerializer(23-242)lib/src/main/java/io/ably/lib/types/MessageVersion.java (1)
MessageVersion(24-232)lib/src/main/java/io/ably/lib/types/Param.java (1)
Param(6-88)lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
UpdateDeleteResult(10-71)lib/src/main/java/io/ably/lib/http/HttpUtils.java (1)
HttpUtils(28-280)
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (1)
lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
UpdateDeleteResult(10-71)
⏰ 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: check-liveobjects
- GitHub Check: check (29)
- GitHub Check: check (21)
- GitHub Check: check (19)
- GitHub Check: check (24)
- GitHub Check: check (29)
- GitHub Check: check
🔇 Additional comments (16)
lib/src/test/java/io/ably/lib/test/rest/HttpHeaderTest.java (1)
84-84: LGTM!The test assertion correctly updated to expect protocol version "5", aligning with the protocol version bump in
Defaults.java.lib/src/test/java/io/ably/lib/test/realtime/RealtimeHttpHeaderTest.java (1)
83-84: LGTM!The WebSocket version parameter assertion correctly updated to "5", consistent with the protocol version bump.
lib/src/main/java/io/ably/lib/types/MessageSerializer.java (2)
77-85: LGTM with note on existing pattern.The
write(Message)method correctly mirrors the existingwriteMsgpackArraypattern. The silentnullreturn onIOExceptionis consistent with the codebase style, though it's a known trade-off.
151-153: LGTM!Clean single-message JSON serialization using Gson, consistent with the existing
asJsonRequest(Message[])pattern.lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
10-18: LGTM!Clean immutable result type with public final field for
versionSerial.lib/src/main/java/io/ably/lib/rest/MessageEditsMixin.java (4)
143-165: LGTM! New append functionality correctly implemented.The
appendMessageandappendMessageAsyncmethods are cleanly implemented, reusing the sharedupdateMessageImplwithMessageAction.MESSAGE_APPEND. The Javadoc is clear about the operation's purpose.
175-183: LGTM! Message augmentation logic is sound.The implementation correctly:
- Creates a new
Messagewith copied fields to avoid mutating the input- Sets the appropriate
actionfor the operation type- Populates
MessageVersionwith operation metadata (server will setserial/timestamp)- Encodes the message before serialization
196-201: LGTM! Error handling is appropriate.The response handler correctly validates the result and throws a descriptive error if
versionSerialis missing from the response.
99-141: LGTM! API changes are consistent and well-documented.The transition from
void/CompletionListenertoUpdateDeleteResult/Callback<UpdateDeleteResult>is applied consistently acrossupdateMessage,deleteMessage, and their async variants. The Javadoc correctly documents the new return types.lib/src/main/java/io/ably/lib/rest/ChannelBase.java (3)
20-20: LGTM!The import for
UpdateDeleteResultis correctly added to support the new return types for update/delete/append operations.
356-400: LGTM!The
updateMessagemethods are correctly updated to returnUpdateDeleteResultand useCallback<UpdateDeleteResult>for async variants. The delegation tomessageEditsMixinis consistent, and the Javadoc accurately reflects the new return type.
412-456: LGTM!The
deleteMessagemethods follow the same pattern asupdateMessage, correctly returningUpdateDeleteResultand using callbacks for async variants.lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (4)
46-46: LGTM!The import for
UpdateDeleteResultis correctly added.
106-106: LGTM!The
volatilemodifier ensures thread-safe visibility whenmessageEditsMixinis reassigned insetOptions(). Since the mixin is treated as immutable after construction and only replaced atomically, this is appropriate.
1218-1262: LGTM!The
updateMessagemethods are correctly updated, consistent with the RESTChannelBaseimplementation.
1274-1318: LGTM!The
deleteMessagemethods follow the same pattern and are correctly implemented.
Adds message appends and protocol v5 upgrade
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.