[ECO-5634] Messages update, delete and versions

[maratal]

Summary by CodeRabbit

Release Notes

• New Features
  • Added message update capability on channels to modify previously published messages with operation metadata.
  • Added message deletion capability on channels to remove previously published messages.
  • Added ability to retrieve individual messages by serial identifier.
  • Added ability to fetch version history for messages.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR implements the mutable messages write API for the Ably iOS SDK, introducing support for updating, deleting, and retrieving message versions across REST and realtime channels. It adds a new ARTMessageOperation class to represent operation metadata, extends channel APIs with four new message manipulation methods, and includes comprehensive test coverage across REST and realtime environments.

Changes

Cohort / File(s)	Summary
Message Operation Model Source/include/Ably/ARTMessageOperation.h, Source/ARTMessageOperation.m	New public ARTMessageOperation class with clientId, descriptionText, and metadata properties; includes copy semantics and dictionary serialization support.
Protocol & Serialization Source/PrivateHeaders/Ably/ARTMessageOperation+Private.h, Source/include/Ably/ARTDictionarySerializable.h	New private category extending ARTMessageOperation to conform to ARTDictionarySerializable protocol; replaces prior dependency on ARTSummaryTypes+Private.
Channel Public API Definitions Source/PrivateHeaders/Ably/ARTChannel.h, Source/include/Ably/ARTChannelProtocol.h	Added four new abstract methods for message operations: updateMessage:operation:params:wrapperSDKAgents:callback:, deleteMessage:operation:params:wrapperSDKAgents:callback:, getMessageWithSerial:wrapperSDKAgents:callback:, and getMessageVersionsWithSerial:wrapperSDKAgents:callback:.
Channel Implementations Source/ARTChannel.m, Source/ARTRealtimeChannel.m, Source/ARTWrapperSDKProxyRealtimeChannel.m	Added public method overloads (without wrapperSDKAgents) that delegate to internal implementations; introduced assertions in ARTChannel base class to enforce override requirements.
REST Channel Internal Logic Source/ARTRestChannel.m	Implemented message operations via new internal helper _updateMessage:isDelete:operation:params:wrapperSDKAgents:callback:; added HTTP PATCH (update), POST (delete), and GET (retrieve) request handling with parameter serialization and wrapperSDKAgents support.
Type System Additions Source/include/Ably/ARTTypes.h	New public callback typedef ARTMessageErrorCallback for message-result callbacks.
Module & Build Configuration Ably.xcodeproj/project.pbxproj, Source/Ably.modulemap, Source/include/module.modulemap, Source/include/Ably/AblyPublic.h	Updated Xcode project to reference new files; added ARTMessageOperation.h and ARTDictionarySerializable.h to public/private module exports; replaced ARTSummaryTypes+Private.h with new headers.
Import Updates Source/ARTSummaryTypes.m	Replaced ARTSummaryTypes+Private.h import with ARTDictionarySerializable.h.
Test Suite Test/AblyTests/Tests/MessageUpdatesDeletesTests.swift	Comprehensive test coverage for message update (RSL12), delete (RSL13), retrieve (RSL11), and version history (RSL14) operations across both REST and realtime channels; includes HTTP request/response validation and state assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Areas requiring attention:
  • Logic in ARTRestChannel.m for HTTP request construction, parameter serialization, and payload encoding for update/delete operations
  • Consistency of delegate patterns across ARTRealtimeChannel.m, ARTChannel.m, and ARTWrapperSDKProxyRealtimeChannel.m
  • Test coverage in MessageUpdatesDeletesTests.swift for both REST and realtime code paths, including HTTP assertion validation
  • Module map and project configuration correctness to ensure proper header visibility

Poem

🐰 Mutable messages hopping in the light,
Updates and deletes, now working just right!
From REST to Realtime, operations take flight,
Versions preserved through each edit's might.
A rabbit's delight—messages bending to sight! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main changes: implementing message update, delete, and version retrieval functionality for ably-cocoa.
Linked Issues check	✅ Passed	The PR implements mutable message write APIs including update, delete, and version retrieval operations across REST and Realtime channels, aligning with ECO-5634 requirements.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing message update, delete, and version operations. Project file updates, new classes, method additions, and test coverage are all in scope.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ECO-5634-message-updates

---

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[lawrence-forooghian]

I know this one's in draft but I'm going to start taking a look at it now, because next week I'll need to build the Realtime equivalent on top of this.

[maratal]

I know this one's in draft but I'm going to start taking a look at it now, because next week I'll need to build the Realtime equivalent on top of this.

You mean swift version?

[lawrence-forooghian]

You mean swift version?

No, I mean that the ARTRealtimeChannel versions of these methods are going to be replaced by sending a message over the Realtime connection instead of making a REST request.

[lawrence-forooghian]

Of course I agree that current implementation is far form optimized

My question though is whether this is how all of our existing REST calls work; do we really not have any common helper methods for doing this sort of thing?

Also shouldn't at this point such initiatives all go to the ably-swift effort?

I don't think so. We don't have imminent plans for releasing ably-swift (for example, I'm currently not working on it) and it'll be a while before it's at the same level of maturity as ably-cocoa. And we'll be continuing to add new features to ably-cocoa for the foreseeable future. We need to ensure that ably-cocoa remains maintainable.

[maratal]

My question though is whether this is how all of our existing REST calls work; do we really not have any common helper methods for doing this sort of thing?

Yes, there is a lot of duplication re REST requests in ably-cocoa. There is also common helper method executeRequest:, but before you call it you should do a lot of other work which might or might not be a subject for such an optimization.

[lawrence-forooghian]

Yes, there is a lot of duplication re REST requests in ably-cocoa. There is also common helper method executeRequest:, but before you call it you should do a lot of other work which might or might not be a subject for such an optimization.

Do you think we should have an issue for improving this?

[lawrence-forooghian]

Yes, there is a lot of duplication re REST requests in ably-cocoa. There is also common helper method executeRequest:, but before you call it you should do a lot of other work which might or might not be a subject for such an optimization.

Do you think we should have an issue for improving this?

That is; how much of an implementation burden does the current state of affairs cause? Was it a lot of effort to add these new REST calls? How confident are you that the methods you've implemented are doing all the things that they should be doing?

[maratal]

Yes, there is a lot of duplication re REST requests in ably-cocoa. There is also common helper method executeRequest:, but before you call it you should do a lot of other work which might or might not be a subject for such an optimization.

Do you think we should have an issue for improving this?

That is; how much of an implementation burden does the current state of affairs cause? Was it a lot of effort to add these new REST calls?

The implementation is done by Claude (from the second attempt tho). Architectural part was heavily edited/guided by me.

How confident are you that the methods you've implemented are doing all the things that they should be doing?

Well, I've reviewed them, tests here resembles JS tests and they are passing.

[maratal]

Yes, there is a lot of duplication re REST requests in ably-cocoa. There is also common helper method executeRequest:, but before you call it you should do a lot of other work which might or might not be a subject for such an optimization.

Do you think we should have an issue for improving this?

I think yes, auditing all the rest requests and trying to find common places for optimization would be a proper thing to do.

[lawrence-forooghian]

Yes, there is a lot of duplication re REST requests in ably-cocoa. There is also common helper method executeRequest:, but before you call it you should do a lot of other work which might or might not be a subject for such an optimization.

Do you think we should have an issue for improving this?

I think yes, auditing all the rest requests and trying to find common places for optimization would be a proper thing to do.

Please can you make an issue for this? I worry about the amount of extra code added here

[maratal]

Yes, there is a lot of duplication re REST requests in ably-cocoa. There is also common helper method executeRequest:, but before you call it you should do a lot of other work which might or might not be a subject for such an optimization.

Do you think we should have an issue for improving this?

I think yes, auditing all the rest requests and trying to find common places for optimization would be a proper thing to do.

Please can you make an issue for this? I worry about the amount of extra code added here

#2156

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (6)

Source/PrivateHeaders/Ably/ARTMessageOperation+Private.h (1)

1-3: Prefer #import <Foundation/Foundation.h> and verify ARTMessageOperation.h visibility

To keep this private header friendly to all toolchains (including the static analysis configuration that reported an error), it would be safer and more consistent with typical Ably Cocoa headers to use a standard import:

-@import Foundation;
-#import <Ably/ARTMessageOperation.h>
-#import "ARTDictionarySerializable.h"
+#import <Foundation/Foundation.h>
+#import <Ably/ARTMessageOperation.h>
+#import "ARTDictionarySerializable.h"

Also, given the analyzer warning 'Ably/ARTMessageOperation.h' file not found, please double‑check that ARTMessageOperation.h is added to the appropriate header phase and exposed in all targets that include this private header so <Ably/…> resolves correctly.

Source/include/Ably/ARTChannelProtocol.h (1)

4-5: New channel message operation APIs look coherent and well-typed

• Importing ARTStringifiable and forward‑declaring ARTMessageOperation are appropriate for the new signatures.
• updateMessage:…params:callback: and deleteMessage:…params:callback: cleanly model operation metadata plus optional stringifiable params, while relying on the existing ARTCallback pattern.
• getMessageWithSerial:callback: and getMessageVersionsWithSerial:callback: correctly use the new ARTMessageErrorCallback and existing ARTPaginatedMessagesCallback, matching their intended behaviors.

From a public API perspective, these additions are consistent with the surrounding channel methods and with the callback typedefs defined in ARTTypes.h.

If you plan further polish, consider expanding the docstrings slightly (for example, clarifying what kinds of values are expected in params and how message.serial is obtained) to make the new operations self‑explanatory to SDK consumers.

Also applies to: 9-10, 73-113

Source/include/Ably/ARTMessageOperation.h (1)

3-22: Consider copy semantics for properties and trimming unused imports

For an immutable, NS_SWIFT_SENDABLE type it would be safer to declare the string/dictionary properties as copy and copy the inputs in the initializer, to avoid external mutation via NSMutableString/NSMutableDictionary instances passed in.

Also, ARTStringifiable.h isn’t used in this header; if nothing transitively relies on it being re‑exported, you can drop that import to keep the public surface narrower.

Source/ARTMessageOperation.m (2)

16-21: Copy currently shares underlying objects; consider defensive copies

copyWithZone: assigns clientId, descriptionText, and metadata by reference:

operation->_clientId = self.clientId;
operation->_descriptionText = self.descriptionText;
operation->_metadata = self.metadata;

If callers ever pass mutable instances into the initializer, both the original and the copy will see subsequent mutations. If you want ARTMessageOperation to behave immutably, prefer copying here (and in the initializer) instead.

---

45-53: Unimplemented dictionary factories may fail silently in release

createFromDictionary: and initWithDictionary: currently NSAssert(false) and return nil. In release builds this just returns nil with no signal if someone accidentally wires these up via ARTDictionarySerializable.

If these aren’t needed yet, consider either:

• Explicitly documenting them as unsupported (e.g. comment + ARTLogWarn), or
• Implementing them now using the same keys as writeToDictionary:.

This avoids surprising nil in future refactors.

Source/ARTRestChannel.m (1)

104-118: REST message update/delete/get/version wiring looks correct and consistent

The new ARTRestChannel surface methods correctly forward to the internal counterparts with wrapperSDKAgents:nil, and ARTRestChannelInternal now centralises update/delete in _updateMessage:isDelete:operation:params:wrapperSDKAgents:callback: with:

• Proper serial presence validation for RSL12/RSL13.
• Request bodies that match the spec (serial, optional operation/name/data/encoding/extras) and reuse of dataEncoder for the message payload.
• Correct endpoint/method selection for update (PATCH /messages/{serial}) vs delete (POST /messages/{serial}/delete), with params encoded into the query string.
• Callbacks consistently marshalled back onto _userQueue, and wrapperSDKAgents threaded through to executeRequest / executePaginated.

Given there’s already a follow-up issue to de-duplicate REST request construction, this structure looks good for now.

Also applies to: 408-662

📜 Review details

Configuration used: CodeRabbit 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 44b2199 and c6474a3.

📒 Files selected for processing (16)

• Ably.xcodeproj/project.pbxproj (16 hunks)
• Source/ARTChannel.m (1 hunks)
• Source/ARTMessageOperation.m (1 hunks)
• Source/ARTRealtimeChannel.m (2 hunks)
• Source/ARTRestChannel.m (3 hunks)
• Source/ARTSummaryTypes.m (1 hunks)
• Source/ARTWrapperSDKProxyRealtimeChannel.m (1 hunks)
• Source/Ably.modulemap (1 hunks)
• Source/PrivateHeaders/Ably/ARTChannel.h (2 hunks)
• Source/PrivateHeaders/Ably/ARTMessageOperation+Private.h (1 hunks)
• Source/include/Ably/ARTChannelProtocol.h (2 hunks)
• Source/include/Ably/ARTMessageOperation.h (1 hunks)
• Source/include/Ably/ARTTypes.h (1 hunks)
• Source/include/Ably/AblyPublic.h (1 hunks)
• Source/include/module.modulemap (1 hunks)
• Test/AblyTests/Tests/MessageUpdatesDeletesTests.swift (1 hunks)

🧰 Additional context used

🪛 Clang (14.0.6)

Source/PrivateHeaders/Ably/ARTMessageOperation+Private.h

[error] 1-1: expected identifier or '('

(clang-diagnostic-error)

---

[error] 2-2: 'Ably/ARTMessageOperation.h' file not found

(clang-diagnostic-error)

Source/include/Ably/ARTMessageOperation.h

[error] 1-1: 'Foundation/Foundation.h' file not found

(clang-diagnostic-error)

🪛 SwiftLint (0.57.0)

Test/AblyTests/Tests/MessageUpdatesDeletesTests.swift

[Warning] 5-5: Imports should be unique

(duplicate_imports)

---

[Error] 126-126: Force casts should be avoided

(force_cast)

---

[Error] 252-252: Force casts should be avoided

(force_cast)

🔇 Additional comments (11)

Ably.xcodeproj/project.pbxproj (2)

384-392: ARTMessageOperation is correctly wired across targets and build phases

The new ARTMessageOperation files (public header, private header, and .m) are added consistently to:

• PBXFileReference and the Types group,
• all three frameworks’ Headers phases with appropriate Public/Private attributes,
• all three frameworks’ Sources phases.

This matches the existing pattern for other message-related types (e.g., ARTMessageVersion) and should make the new type available on iOS, macOS, and tvOS without build issues.

Also applies to: 1241-1243, 1866-1868, 3081-3082

---

393-395: Confirm whether ARTDictionarySerializable is intentionally header‑only

ARTDictionarySerializable.h is added as:

• a PBXFileReference under PrivateHeaders/Ably,
• a member of the Types group,
• a Private header in the iOS/macOS/tvOS Headers build phases.

There is no ARTDictionarySerializable.m referenced anywhere in this project file. If the type is meant to be header‑only (e.g. a protocol or category with no implementation), this is fine; if an implementation file exists, it still needs a PBXFileReference, PBXBuildFile, and Sources entries for each target to actually compile.

Please double‑check whether an .m is expected here and wire it up if so.

Also applies to: 1244-1244, 1935-1935, 2169-2171, 2501-2501, 2712-2712

Source/ARTSummaryTypes.m (1)

2-2: Importing ARTDictionarySerializable is appropriate here

The new import cleanly reflects that the summary types conform to ARTDictionarySerializable, without changing runtime behavior. Looks correct and consistent with the protocol usage throughout this file.

Source/include/Ably/ARTTypes.h (1)

504-505: New ARTMessageErrorCallback typedef is consistent with existing patterns

The message+error callback shape and nullability mirror other result/error callbacks in this header and are suitable for single‑message lookup APIs.

Source/include/Ably/AblyPublic.h (1)

48-48: Exposing ARTMessageOperation via the public umbrella is correct

Including ARTMessageOperation.h here makes the new message operation API available to SDK consumers in the expected way alongside other message types.

Source/include/module.modulemap (1)

142-143: SPM Private module now correctly exposes new internal headers

Adding ARTDictionarySerializable.h and ARTMessageOperation+Private.h to the Ably.Private explicit module keeps the SPM module map aligned with the new internal types and matches the Xcode module map changes.

Source/ARTChannel.m (1)

156-182: New abstract channel methods are wired consistently with existing design

The four new methods for update/delete/lookups follow the existing pattern for abstract operations on ARTChannel (assert‑only bodies to enforce subclass overrides) and align with the public protocol variants that don’t take wrapperSDKAgents. As long as ARTRestChannel and ARTRealtimeChannel override these, this is a clean extension of the existing design.

Please confirm both concrete channel implementations override all four methods so these code paths are never hit in production.

Source/Ably.modulemap (1)

142-143: Xcode module map correctly updated for new private headers

Including ARTDictionarySerializable.h and ARTMessageOperation+Private.h in the Ably.Private module keeps the Xcode module map consistent with SPM and ensures internal code using import Ably.Private can see these types.

Source/ARTWrapperSDKProxyRealtimeChannel.m (1)

107-136: Wrapper forwarding for message operations looks consistent

The new getMessageWithSerial, updateMessage, deleteMessage, and getMessageVersionsWithSerial methods correctly delegate to underlyingChannel.internal and pass through proxyOptions.agents, matching the internal API shape and keeping wrapperSDK agent propagation consistent.

Source/PrivateHeaders/Ably/ARTChannel.h (1)

48-66: Channel-level message APIs align with internal implementations

The added updateMessage, deleteMessage, getMessageWithSerial, and getMessageVersionsWithSerial signatures match the corresponding internal/rest implementations (including ARTStringifiable params and NSStringDictionary *wrapperSDKAgents), which should keep the public/internal surfaces in sync.

Source/ARTRealtimeChannel.m (1)

147-161: Realtime message operation APIs are wired cleanly through to REST

The new updateMessage, deleteMessage, getMessageWithSerial, and getMessageVersionsWithSerial overloads on ARTRealtimeChannel and ARTRealtimeChannelInternal consistently delegate to the REST channel, threading wrapperSDKAgents where provided and defaulting to nil otherwise. This keeps the public realtime API thin while reusing the REST implementation for edits/deletes/versions.

Also applies to: 470-492

[lawrence-forooghian]

why do we need a channel prefix?

[lawrence-forooghian]

This looks flaky. why can't we wait to receive it on a Realtime connection?

[lawrence-forooghian]

why do you need to switch this? can't you just call get on the result and let it bubble up out of the test as an error?

[lawrence-forooghian]

where do you disconnect the realtime client for these realtime tests? doesn't it just leak?

[lawrence-forooghian]

I would suggest not using implicitly unwrapped optionals (ditto force-unwrapping) unless we're sure that we're exiting early if they're nil, else we're going to crash the test