[AIT-98] feat: realtime edits and deletes #1183
Conversation
WalkthroughThis PR implements message edit/delete functionality for Java realtime channels by introducing a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Channel as ChannelBase
participant Validation as updateDeleteImpl
participant ConnMgr as ConnectionManager
participant Protocol as ProtocolMessage
participant Server
rect rgb(200, 220, 240)
Note over Client,Server: Message Update/Delete Flow
end
Client->>Channel: updateMessage(message, operation, listener)
activate Channel
Channel->>Validation: updateDeleteImpl(message, operation, listener)
activate Validation
Validation->>Validation: Validate message.serial
Validation->>Validation: Check connection state
Validation->>Validation: Encode message with action/version
rect rgb(230, 240, 200)
Note over Validation: Wrap UpdateDeleteResult callback<br/>to PublishResult callback
end
Validation->>ConnMgr: send(protocolMsg, listener)
activate ConnMgr
ConnMgr->>ConnMgr: Queue or send immediately
ConnMgr->>Server: Send protocol message
deactivate ConnMgr
Server-->>ConnMgr: ACK with results
activate ConnMgr
rect rgb(240, 220, 200)
Note over ConnMgr: Extract per-message<br/>PublishResult from array
end
ConnMgr->>ConnMgr: onAck(message.res)
ConnMgr->>Channel: listener.onSuccess(publishResult)
deactivate ConnMgr
deactivate Validation
Channel->>Client: Callback with result
deactivate Channel
rect rgb(240, 200, 200)
Note over Server,Client: Error Path
end
Server-->>ConnMgr: NACK/Error
ConnMgr->>Channel: listener.onError(error)
Channel->>Client: Error callback triggered
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI Agents
In @lib/src/main/java/io/ably/lib/realtime/ChannelBase.java:
- Around line 1357-1396: The ProtocolMessage is sending the original Message
object instead of the constructed updatedMessage, so the action, version and
encoded data are lost; change the send call to use updatedMessage (i.e., set
msg.messages = new Message[] { updatedMessage }) so the ProtocolMessage includes
the modified action/version/encoded payload before connectionManager.send(...).
- Around line 1374-1381: The code in ChannelBase constructs an updatedMessage
(setting action and version fields) but then sends the original message, and
updatedMessage also never copies the original message.serial; fix by sending the
updatedMessage and ensuring its serial is copied from the original message:
populate updatedMessage.serial = message.serial after building updatedMessage,
and replace msg.messages = new Message[] { message } with msg.messages = new
Message[] { updatedMessage } (or remove updatedMessage construction if you
intend to send the original message instead).
In @lib/src/main/java/io/ably/lib/types/PublishResult.java:
- Around line 52-63: In PublishResult msgpack deserialization the inner loop
declares j but mistakenly tests and increments i, corrupting the outer loop and
leaving serials mostly unset; change the inner loop to iterate on j (e.g., for
(int j = 0; j < count; j++) ) and use j++ so unpacker results are stored into
serials[j] while leaving the outer variable i untouched, keeping the existing
nil check/unpack logic and final return new PublishResult(serials).
In
@lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelMessageEditTest.java:
- Around line 309-324: The test getMessageVersions_async currently only
publishes and reads history synchronously; update it to call the asynchronous
API getMessageVersionsAsync for the published message (using the
publishedMessage.serial from waitForMessageAppearInHistory) and wait for the
async result (future/callback) to complete with a timeout; then assert that the
returned versions array is non-empty and contains an entry whose serial equals
publishedMessage.serial (and any other expected fields). Ensure you handle
errors by failing the test on exception and cleanly await the async completion
before finishing the test.
- Around line 39-45: Add an @After cleanup method (e.g., tearDownAfter) to close
the AblyRealtime instance created in setUpBefore: implement a method annotated
with @After that checks if the field ably is non-null, calls ably.close() (or
the appropriate shutdown method on AblyRealtime), catches/logs any exceptions,
and sets ably to null to avoid resource leaks between tests.
📜 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 (8)
lib/src/main/java/io/ably/lib/realtime/ChannelBase.javalib/src/main/java/io/ably/lib/realtime/Presence.javalib/src/main/java/io/ably/lib/transport/ConnectionManager.javalib/src/main/java/io/ably/lib/types/ProtocolMessage.javalib/src/main/java/io/ably/lib/types/PublishResult.javalib/src/main/java/io/ably/lib/util/Listeners.javalib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelMessageEditTest.javalib/src/test/java/io/ably/lib/test/realtime/RealtimeConnectFailTest.java
🧰 Additional context used
🧬 Code graph analysis (8)
lib/src/main/java/io/ably/lib/types/ProtocolMessage.java (2)
lib/src/main/java/io/ably/lib/types/PublishResult.java (1)
PublishResult(15-129)liveobjects/src/main/kotlin/io/ably/lib/objects/serialization/DefaultSerialization.kt (1)
writeMsgpackArray(23-27)
lib/src/main/java/io/ably/lib/types/PublishResult.java (2)
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/test/java/io/ably/lib/test/realtime/RealtimeChannelMessageEditTest.java (9)
lib/src/main/java/io/ably/lib/realtime/AblyRealtime.java (1)
AblyRealtime(31-319)lib/src/main/java/io/ably/lib/types/ChannelOptions.java (1)
ChannelOptions(13-131)lib/src/main/java/io/ably/lib/types/ClientOptions.java (1)
ClientOptions(19-392)lib/src/main/java/io/ably/lib/types/ErrorInfo.java (1)
ErrorInfo(17-197)lib/src/main/java/io/ably/lib/types/Message.java (1)
Message(25-443)lib/src/main/java/io/ably/lib/types/MessageOperation.java (1)
MessageOperation(14-100)lib/src/main/java/io/ably/lib/types/Param.java (1)
Param(6-88)lib/src/main/java/io/ably/lib/util/Crypto.java (1)
Crypto(25-410)lib/src/main/java/io/ably/lib/util/Listeners.java (1)
Listeners(9-72)
lib/src/main/java/io/ably/lib/transport/ConnectionManager.java (2)
lib/src/main/java/io/ably/lib/types/ProtocolMessage.java (1)
ProtocolMessage(34-375)lib/src/main/java/io/ably/lib/types/PublishResult.java (1)
PublishResult(15-129)
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (2)
lib/src/main/java/io/ably/lib/util/Listeners.java (1)
Listeners(9-72)lib/src/main/java/io/ably/lib/types/AblyException.java (1)
AblyException(12-67)
lib/src/main/java/io/ably/lib/util/Listeners.java (2)
lib/src/main/java/io/ably/lib/types/PublishResult.java (1)
PublishResult(15-129)lib/src/main/java/io/ably/lib/types/UpdateDeleteResult.java (1)
UpdateDeleteResult(14-81)
lib/src/test/java/io/ably/lib/test/realtime/RealtimeConnectFailTest.java (1)
lib/src/main/java/io/ably/lib/types/PublishResult.java (1)
PublishResult(15-129)
lib/src/main/java/io/ably/lib/realtime/Presence.java (2)
lib/src/main/java/io/ably/lib/types/PublishResult.java (1)
PublishResult(15-129)lib/src/main/java/io/ably/lib/util/Listeners.java (1)
Listeners(9-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: check (21)
- GitHub Check: check (29)
- GitHub Check: check (24)
- GitHub Check: check (19)
- GitHub Check: check (29)
🔇 Additional comments (24)
lib/src/test/java/io/ably/lib/test/realtime/RealtimeConnectFailTest.java (1)
397-407: LGTM!The test correctly adapts to the new
Callback<PublishResult>interface for theconnectionManager.send()method. The callback signature change from parameterlessonSuccess()toonSuccess(PublishResult result)aligns with the broader refactoring to support per-message publish results.lib/src/main/java/io/ably/lib/realtime/Presence.java (3)
125-130: LGTM!The method signature change to accept
Callback<PublishResult>and the use ofListeners.unwrap(listener)to extract the underlyingCompletionListenerfor storage maintains backward compatibility with the existingQueuedPresencestructure while adapting to the new callback model.
768-768: LGTM!Correctly wraps the
CompletionListenerusingListeners.fromCompletionListener(listener)to match the updatedconnectionManager.send()signature.
897-897: LGTM!Consistent with the pattern used in
updatePresence- correctly wraps the listener for the send operation.lib/src/main/java/io/ably/lib/util/Listeners.java (2)
15-25: LGTM!The new adapter methods are well-designed:
toPublishResultListenerbridges theUpdateDeleteResultcallback toPublishResultcallbackunwrapsafely extracts the underlyingCompletionListenerwith proper null handling
49-71: LGTM!The
UpdateResultToPublishAdaptercorrectly:
- Handles null
listener,result,result.serials, and empty arrays- Extracts only the first serial for
UpdateDeleteResult(appropriate for single-message operations)- Properly delegates error handling
lib/src/main/java/io/ably/lib/types/ProtocolMessage.java (3)
140-141: LGTM!The new
resfield follows the established pattern for optional array fields inProtocolMessageand is properly annotated with@Nullable.
215-218: LGTM!The msgpack serialization for the
resfield correctly follows the pattern used for other array fields (e.g.,messages,annotations).
290-292: LGTM!The deserialization case for the
resfield is correctly integrated into the existing switch statement.lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelMessageEditTest.java (4)
51-74: LGTM!The
getMessage_retrieveBySerialtest correctly validates the publish-and-retrieve-by-serial flow.
79-111: LGTM!The
updateMessage_updateDatatest covers the core update flow with proper assertions for data, name, and action.
154-185: LGTM!The async update test correctly uses
Listeners.fromCompletionListenerand validates the callback-based flow.
386-426: LGTM!Comprehensive workflow test covering publish → update → verify → delete → verify deletion → verify version history. Well-structured end-to-end test.
lib/src/main/java/io/ably/lib/types/PublishResult.java (3)
15-27: LGTM!The class structure and
serialsfield documentation clearly explain the 1:1 correspondence with published messages and the null case for conflation.
71-90: LGTM!The array serialization/deserialization methods follow standard patterns used elsewhere in the codebase.
113-128: LGTM!The body handler correctly handles both JSON and msgpack content types and wraps decode exceptions appropriately.
lib/src/main/java/io/ably/lib/realtime/ChannelBase.java (3)
1128-1128: LGTM!The
publishmethod correctly usesListeners.fromCompletionListener(listener)to adapt to the new callback model.
1217-1257: LGTM!The
updateMessagemethod overloads properly delegate to the centralized implementation, maintaining a clean API surface.
1714-1714: LGTM!Consistent with the pattern used in
publish- correctly wraps the listener for the send operation.lib/src/main/java/io/ably/lib/transport/ConnectionManager.java (5)
31-42: LGTM!The added imports are appropriate for the new
Callback<PublishResult>type and null safety annotations.
1408-1410: LGTM!The
message.res(per-message publish results) is correctly propagated to the ack handler.
1728-1735: LGTM!The
QueuedMessageclass correctly updated to useCallback<PublishResult>. Note that this is a breaking change for the public class, but the AI summary indicates that callers have been updated to use adapter wrappers.
1737-1751: LGTM!The
sendmethod signature correctly updated to acceptCallback<PublishResult>. This is a breaking public API change that enables per-message publish result handling.
1831-1885: LGTM!The
ackmethod implementation correctly handles per-message results:
- Defensive bounds checking on the
resultsarray- Index-based iteration properly correlates results with acknowledged messages
- Callbacks are invoked outside the synchronized block, avoiding potential deadlocks
The design gracefully handles cases where
resultsis null or shorter than expected by passingnullto the callback. TheCallback<PublishResult>interface supports null values (no @nonnull annotation), and the codebase already establishes this pattern in implementations likeListeners.javaandCompletionListener.javathat handle null defensively.
| private void updateDeleteImpl( | ||
| Message message, | ||
| MessageOperation operation, | ||
| MessageAction action, | ||
| Callback<UpdateDeleteResult> listener | ||
| ) throws AblyException { | ||
| if (message.serial == null || message.serial.isEmpty()) { | ||
| throw AblyException.fromErrorInfo(new ErrorInfo("Message serial cannot be empty", 400, 40003)); | ||
| } | ||
| ConnectionManager connectionManager = ably.connection.connectionManager; | ||
| ConnectionManager.State connectionState = connectionManager.getConnectionState(); | ||
| boolean queueMessages = ably.options.queueMessages; | ||
| if (!connectionManager.isActive() || (connectionState.queueEvents && !queueMessages)) { | ||
| throw AblyException.fromErrorInfo(connectionState.defaultErrorInfo); | ||
| } | ||
| boolean connected = (connectionState.sendEvents); | ||
|
|
||
| Message updatedMessage = new Message(message.name, message.data, message.extras); | ||
| updatedMessage.action = action; | ||
| updatedMessage.version = new MessageVersion(); | ||
| if (operation != null) { | ||
| updatedMessage.version.clientId = operation.clientId; | ||
| updatedMessage.version.description = operation.description; | ||
| updatedMessage.version.metadata = operation.metadata; | ||
| } | ||
|
|
||
| try { | ||
| ably.auth.checkClientId(message, true, connected); | ||
| updatedMessage.encode(options); | ||
| } catch (AblyException e) { | ||
| if (listener != null) { | ||
| listener.onError(e.errorInfo); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| ProtocolMessage msg = new ProtocolMessage(Action.message, this.name); | ||
| msg.messages = new Message[] { message }; | ||
| connectionManager.send(msg, queueMessages, Listeners.toPublishResultListener(listener)); | ||
| } |
There was a problem hiding this comment.
Critical bug: Wrong message object used in ProtocolMessage.
At line 1394, the ProtocolMessage uses the original message instead of updatedMessage. This means:
- The
action(UPDATE/DELETE/APPEND) is not sent - The
versionmetadata (clientId, description, metadata) is not sent - The encoded message data is not sent
The server will receive the unmodified original message instead of the properly constructed update/delete message.
🔎 Proposed fix
ProtocolMessage msg = new ProtocolMessage(Action.message, this.name);
- msg.messages = new Message[] { message };
+ msg.messages = new Message[] { updatedMessage };
connectionManager.send(msg, queueMessages, Listeners.toPublishResultListener(listener));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void updateDeleteImpl( | |
| Message message, | |
| MessageOperation operation, | |
| MessageAction action, | |
| Callback<UpdateDeleteResult> listener | |
| ) throws AblyException { | |
| if (message.serial == null || message.serial.isEmpty()) { | |
| throw AblyException.fromErrorInfo(new ErrorInfo("Message serial cannot be empty", 400, 40003)); | |
| } | |
| ConnectionManager connectionManager = ably.connection.connectionManager; | |
| ConnectionManager.State connectionState = connectionManager.getConnectionState(); | |
| boolean queueMessages = ably.options.queueMessages; | |
| if (!connectionManager.isActive() || (connectionState.queueEvents && !queueMessages)) { | |
| throw AblyException.fromErrorInfo(connectionState.defaultErrorInfo); | |
| } | |
| boolean connected = (connectionState.sendEvents); | |
| Message updatedMessage = new Message(message.name, message.data, message.extras); | |
| updatedMessage.action = action; | |
| updatedMessage.version = new MessageVersion(); | |
| if (operation != null) { | |
| updatedMessage.version.clientId = operation.clientId; | |
| updatedMessage.version.description = operation.description; | |
| updatedMessage.version.metadata = operation.metadata; | |
| } | |
| try { | |
| ably.auth.checkClientId(message, true, connected); | |
| updatedMessage.encode(options); | |
| } catch (AblyException e) { | |
| if (listener != null) { | |
| listener.onError(e.errorInfo); | |
| } | |
| return; | |
| } | |
| ProtocolMessage msg = new ProtocolMessage(Action.message, this.name); | |
| msg.messages = new Message[] { message }; | |
| connectionManager.send(msg, queueMessages, Listeners.toPublishResultListener(listener)); | |
| } | |
| private void updateDeleteImpl( | |
| Message message, | |
| MessageOperation operation, | |
| MessageAction action, | |
| Callback<UpdateDeleteResult> listener | |
| ) throws AblyException { | |
| if (message.serial == null || message.serial.isEmpty()) { | |
| throw AblyException.fromErrorInfo(new ErrorInfo("Message serial cannot be empty", 400, 40003)); | |
| } | |
| ConnectionManager connectionManager = ably.connection.connectionManager; | |
| ConnectionManager.State connectionState = connectionManager.getConnectionState(); | |
| boolean queueMessages = ably.options.queueMessages; | |
| if (!connectionManager.isActive() || (connectionState.queueEvents && !queueMessages)) { | |
| throw AblyException.fromErrorInfo(connectionState.defaultErrorInfo); | |
| } | |
| boolean connected = (connectionState.sendEvents); | |
| Message updatedMessage = new Message(message.name, message.data, message.extras); | |
| updatedMessage.action = action; | |
| updatedMessage.version = new MessageVersion(); | |
| if (operation != null) { | |
| updatedMessage.version.clientId = operation.clientId; | |
| updatedMessage.version.description = operation.description; | |
| updatedMessage.version.metadata = operation.metadata; | |
| } | |
| try { | |
| ably.auth.checkClientId(message, true, connected); | |
| updatedMessage.encode(options); | |
| } catch (AblyException e) { | |
| if (listener != null) { | |
| listener.onError(e.errorInfo); | |
| } | |
| return; | |
| } | |
| ProtocolMessage msg = new ProtocolMessage(Action.message, this.name); | |
| msg.messages = new Message[] { updatedMessage }; | |
| connectionManager.send(msg, queueMessages, Listeners.toPublishResultListener(listener)); | |
| } |
🤖 Prompt for AI Agents
In @lib/src/main/java/io/ably/lib/realtime/ChannelBase.java around lines 1357 -
1396, The ProtocolMessage is sending the original Message object instead of the
constructed updatedMessage, so the action, version and encoded data are lost;
change the send call to use updatedMessage (i.e., set msg.messages = new
Message[] { updatedMessage }) so the ProtocolMessage includes the modified
action/version/encoded payload before connectionManager.send(...).
| Message updatedMessage = new Message(message.name, message.data, message.extras); | ||
| updatedMessage.action = action; | ||
| updatedMessage.version = new MessageVersion(); | ||
| if (operation != null) { | ||
| updatedMessage.version.clientId = operation.clientId; | ||
| updatedMessage.version.description = operation.description; | ||
| updatedMessage.version.metadata = operation.metadata; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Message class structure and serial field usage
echo "=== Checking Message.java for serial field ==="
find . -name "Message.java" -path "*/types/*" -type f | head -5Repository: ably/ably-java
Length of output: 155
🏁 Script executed:
#!/bin/bash
# Search for serial field in Message class
rg -n -A 3 -B 1 'serial' lib/src/main/java/io/ably/lib/types/Message.java | head -50Repository: ably/ably-java
Length of output: 2091
🏁 Script executed:
#!/bin/bash
# Check the test file for how serial is used in update operations
rg -n 'serial' lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelMessageEditTest.javaRepository: ably/ably-java
Length of output: 3966
🏁 Script executed:
#!/bin/bash
# Check the full context of lines 1374-1381 in ChannelBase.java
sed -n '1360,1395p' lib/src/main/java/io/ably/lib/realtime/ChannelBase.javaRepository: ably/ably-java
Length of output: 1766
🏁 Script executed:
#!/bin/bash
# Get more context around the message sending to understand if updatedMessage should be used
sed -n '1358,1405p' lib/src/main/java/io/ably/lib/realtime/ChannelBase.javaRepository: ably/ably-java
Length of output: 2195
🏁 Script executed:
#!/bin/bash
# Check if there are other places where updatedMessage is used after its creation
sed -n '1374,1420p' lib/src/main/java/io/ably/lib/realtime/ChannelBase.javaRepository: ably/ably-java
Length of output: 2174
Critical issue: Wrong message object being sent, and updatedMessage missing the serial field.
The code creates and configures updatedMessage with the action and version metadata (lines 1374-1380), but then sends the original message object instead (line 1400), which lacks this metadata. Additionally, if updatedMessage were to be sent, it would need the serial field copied from the original message for the server to identify which message to update/delete. Either msg.messages = new Message[] { updatedMessage }; should be used, or the entire updatedMessage construction should be removed if the original message is the intended payload.
🤖 Prompt for AI Agents
In @lib/src/main/java/io/ably/lib/realtime/ChannelBase.java around lines 1374 -
1381, The code in ChannelBase constructs an updatedMessage (setting action and
version fields) but then sends the original message, and updatedMessage also
never copies the original message.serial; fix by sending the updatedMessage and
ensuring its serial is copied from the original message: populate
updatedMessage.serial = message.serial after building updatedMessage, and
replace msg.messages = new Message[] { message } with msg.messages = new
Message[] { updatedMessage } (or remove updatedMessage construction if you
intend to send the original message instead).
| if (fieldName.equals(SERIALS)) { | ||
| int count = unpacker.unpackArrayHeader(); | ||
| String[] serials = new String[count]; | ||
| for (int j = 0; i < count; i++) { | ||
| if (unpacker.getNextFormat().equals(MessageFormat.NIL)) { | ||
| unpacker.unpackNil(); | ||
| serials[j] = null; | ||
| } else { | ||
| serials[j] = unpacker.unpackString(); | ||
| } | ||
| } | ||
| return new PublishResult(serials); |
There was a problem hiding this comment.
Critical bug: Loop variable confusion in msgpack deserialization.
The inner loop at line 55 initializes j but conditions and increments on i (the outer loop variable). This causes:
- The outer loop counter
ito be corrupted - The array index
jto never increment (always 0) - Only
serials[0]will ever be set, regardless of array size
🔎 Proposed fix
if (fieldName.equals(SERIALS)) {
int count = unpacker.unpackArrayHeader();
String[] serials = new String[count];
- for (int j = 0; i < count; i++) {
+ for (int j = 0; j < count; j++) {
if (unpacker.getNextFormat().equals(MessageFormat.NIL)) {
unpacker.unpackNil();
serials[j] = null;
} else {
serials[j] = unpacker.unpackString();
}
}
return new PublishResult(serials);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (fieldName.equals(SERIALS)) { | |
| int count = unpacker.unpackArrayHeader(); | |
| String[] serials = new String[count]; | |
| for (int j = 0; i < count; i++) { | |
| if (unpacker.getNextFormat().equals(MessageFormat.NIL)) { | |
| unpacker.unpackNil(); | |
| serials[j] = null; | |
| } else { | |
| serials[j] = unpacker.unpackString(); | |
| } | |
| } | |
| return new PublishResult(serials); | |
| if (fieldName.equals(SERIALS)) { | |
| int count = unpacker.unpackArrayHeader(); | |
| String[] serials = new String[count]; | |
| for (int j = 0; j < count; j++) { | |
| if (unpacker.getNextFormat().equals(MessageFormat.NIL)) { | |
| unpacker.unpackNil(); | |
| serials[j] = null; | |
| } else { | |
| serials[j] = unpacker.unpackString(); | |
| } | |
| } | |
| return new PublishResult(serials); |
🤖 Prompt for AI Agents
In @lib/src/main/java/io/ably/lib/types/PublishResult.java around lines 52 - 63,
In PublishResult msgpack deserialization the inner loop declares j but
mistakenly tests and increments i, corrupting the outer loop and leaving serials
mostly unset; change the inner loop to iterate on j (e.g., for (int j = 0; j <
count; j++) ) and use j++ so unpacker results are stored into serials[j] while
leaving the outer variable i untouched, keeping the existing nil check/unpack
logic and final return new PublishResult(serials).
| private AblyRealtime ably; | ||
|
|
||
| @Before | ||
| public void setUpBefore() throws Exception { | ||
| ClientOptions opts = createOptions(testVars.keys[0].keyStr); | ||
| ably = new AblyRealtime(opts); | ||
| } |
There was a problem hiding this comment.
Missing @After cleanup for AblyRealtime resource.
The ably instance is created in @Before but never closed, which can lead to resource leaks (open connections, threads) across test runs.
🔎 Proposed fix
+import org.junit.After;
...
private AblyRealtime ably;
@Before
public void setUpBefore() throws Exception {
ClientOptions opts = createOptions(testVars.keys[0].keyStr);
ably = new AblyRealtime(opts);
}
+
+ @After
+ public void tearDown() {
+ if (ably != null) {
+ ably.close();
+ }
+ }🤖 Prompt for AI Agents
In
@lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelMessageEditTest.java
around lines 39 - 45, Add an @After cleanup method (e.g., tearDownAfter) to
close the AblyRealtime instance created in setUpBefore: implement a method
annotated with @After that checks if the field ably is non-null, calls
ably.close() (or the appropriate shutdown method on AblyRealtime), catches/logs
any exceptions, and sets ably to null to avoid resource leaks between tests.
| @Test | ||
| public void getMessageVersions_async() throws Exception { | ||
| String channelName = "mutable:message_versions_async_" + UUID.randomUUID() + "_" + testParams.name; | ||
| Channel channel = ably.channels.get(channelName); | ||
|
|
||
| // Publish a message | ||
| channel.publish("test_event", "Original data"); | ||
|
|
||
| // Get the message from history | ||
| PaginatedResult<Message> history = waitForMessageAppearInHistory(channel); | ||
| assertNotNull("Expected non-null history", history); | ||
| assertEquals(1, history.items().length); | ||
|
|
||
| final Message publishedMessage = history.items()[0]; | ||
| assertNotNull("Expected message to have a serial", publishedMessage.serial); | ||
| } |
There was a problem hiding this comment.
Incomplete test: getMessageVersions_async doesn't test async retrieval.
The test only publishes a message and retrieves history synchronously. It doesn't actually test asynchronous version retrieval via getMessageVersionsAsync.
Would you like me to generate the complete async test implementation?
🤖 Prompt for AI Agents
In
@lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelMessageEditTest.java
around lines 309 - 324, The test getMessageVersions_async currently only
publishes and reads history synchronously; update it to call the asynchronous
API getMessageVersionsAsync for the published message (using the
publishedMessage.serial from waitForMessageAppearInHistory) and wait for the
async result (future/callback) to complete with a timeout; then assert that the
returned versions array is non-empty and contains an entry whose serial equals
publishedMessage.serial (and any other expected fields). Ensure you handle
errors by failing the test on exception and cleanly await the async completion
before finishing the test.
Replaced rest message edits and deletes with realtime for realtime client
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.