[AIT-322] Bump protocol version to v6 and add plugin compatibility checks#2191
[AIT-322] Bump protocol version to v6 and add plugin compatibility checks#2191lawrence-forooghian wants to merge 1 commit intointegration/protocol-v6from
Conversation
WalkthroughThis PR bumps the library API to version 6, pins the ably-cocoa-plugin-support dependency to a specific revision (with a TODO to unpin), adds Live Objects Protocol v6 compatibility checks and a version-based default idempotent REST publishing helper, and updates tests to expect API version 6. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
a907872 to
6ceffe4
Compare
6ceffe4 to
debb721
Compare
debb721 to
af5d05a
Compare
af5d05a to
6254e20
Compare
6254e20 to
409291c
Compare
409291c to
3bf6c74
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Package.swift (1)
22-23: Use the full commit SHA for the revision pin.Line 23 uses a short SHA (
e015e70). Prefer the full 40-char hash to avoid ambiguity in future Git resolution.♻️ Proposed change
- .package(name: "ably-cocoa-plugin-support", url: "https://github.com/ably/ably-cocoa-plugin-support", .revision("e015e70")) + .package(name: "ably-cocoa-plugin-support", url: "https://github.com/ably/ably-cocoa-plugin-support", .revision("e015e708b4c8f4eaf97958cb94e34167cd1ef6b3"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Package.swift` around lines 22 - 23, The package declaration pins the dependency "ably-cocoa-plugin-support" using a short commit SHA (.revision("e015e70")), which can be ambiguous; replace the short SHA with the full 40-character commit hash for the .revision value in the .package(...) entry so the dependency resolves deterministically (locate the .package(name: "ably-cocoa-plugin-support", url: "https://github.com/ably/ably-cocoa-plugin-support", .revision("e015e70")) line and update the .revision string to the full commit SHA).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Source/ARTClientOptions.m`:
- Around line 254-257: Replace the debug-only NSAssert with a runtime guard that
enforces plugin compatibility in all builds: after obtaining
id<APLiveObjectsInternalPluginProtocol> plugin = [publicPlugin internalPlugin],
check if (!plugin.compatibleWithProtocolV6) and then fail deterministically
(e.g., log the error with NSLog/processLogger and raise an NSException or call
abort()) so an incompatible plugin cannot silently pass in Release; keep the
same message string and reference plugin.compatibleWithProtocolV6 and
[publicPlugin internalPlugin] when implementing the check.
---
Nitpick comments:
In `@Package.swift`:
- Around line 22-23: The package declaration pins the dependency
"ably-cocoa-plugin-support" using a short commit SHA (.revision("e015e70")),
which can be ambiguous; replace the short SHA with the full 40-character commit
hash for the .revision value in the .package(...) entry so the dependency
resolves deterministically (locate the .package(name:
"ably-cocoa-plugin-support", url:
"https://github.com/ably/ably-cocoa-plugin-support", .revision("e015e70")) line
and update the .revision string to the full commit SHA).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
Package.resolvedPackage.swiftSource/ARTClientOptions.mSource/ARTDefault.mSource/ARTPluginAPI.mTest/AblyTests/Tests/ARTDefaultTests.swiftTest/AblyTests/Tests/PluginAPITests.swiftTest/AblyTests/Tests/RealtimeClientConnectionTests.swiftTest/AblyTests/Tests/RealtimeClientTests.swiftTest/AblyTests/Tests/RestClientTests.swift
This implements the protocol-version-bumping process proposed in the pinned plugin-support commit. Note that all of the v6 changes are LiveObjects-related, hence no other changes in ably-cocoa. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3bf6c74 to
e9cd9e1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Test/AblyTests/Tests/PluginAPITests.swift (1)
17-19: Add one negative-path test for protocol compatibility.This mock now hardcodes compatibility to true; please add a companion test with
compatibleWithProtocolV6 == falseto assert the expected failure path for the new compatibility check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Test/AblyTests/Tests/PluginAPITests.swift` around lines 17 - 19, Add a negative-path unit test alongside the existing positive case that uses a MockInternalLiveObjectsPlugin variant where compatibleWithProtocolV6 returns false; create a new test (e.g., testLiveObjectsPluginCompatibilityRejectsIncompatiblePlugin) that registers or injects this mock into the same code path under test and asserts the compatibility check fails/throws or causes the expected rejection behavior (matching the existing positive test’s assertions but inverted). Locate the mock class MockInternalLiveObjectsPlugin and either add a second subclass/instance with var compatibleWithProtocolV6: Bool { false } or parameterize the mock, then update PluginAPITests.swift to include the new test invoking the same initialization/registration logic used by the current success test and assert the negative outcome.Package.swift (1)
22-23: Use full commit SHA in.revision(...)for deterministic pinning.The short SHA
e015e70at line 23 is ambiguous in the remote repository—git ls-remotereturns 2 matching refs. Replace it with the full commit hash already present inPackage.resolved:Diff
- .package(name: "ably-cocoa-plugin-support", url: "https://github.com/ably/ably-cocoa-plugin-support", .revision("e015e70")) + .package(name: "ably-cocoa-plugin-support", url: "https://github.com/ably/ably-cocoa-plugin-support", .revision("e015e708b4c8f4eaf97958cb94e34167cd1ef6b3"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Package.swift` around lines 22 - 23, Replace the ambiguous short revision used in the Swift package declaration for "ably-cocoa-plugin-support" by using the full commit SHA from Package.resolved; locate the .package(...) entry for name "ably-cocoa-plugin-support" and update the .revision("e015e70") value to the complete 40-character commit hash so the dependency is deterministically pinned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Package.swift`:
- Around line 22-23: Replace the ambiguous short revision used in the Swift
package declaration for "ably-cocoa-plugin-support" by using the full commit SHA
from Package.resolved; locate the .package(...) entry for name
"ably-cocoa-plugin-support" and update the .revision("e015e70") value to the
complete 40-character commit hash so the dependency is deterministically pinned.
In `@Test/AblyTests/Tests/PluginAPITests.swift`:
- Around line 17-19: Add a negative-path unit test alongside the existing
positive case that uses a MockInternalLiveObjectsPlugin variant where
compatibleWithProtocolV6 returns false; create a new test (e.g.,
testLiveObjectsPluginCompatibilityRejectsIncompatiblePlugin) that registers or
injects this mock into the same code path under test and asserts the
compatibility check fails/throws or causes the expected rejection behavior
(matching the existing positive test’s assertions but inverted). Locate the mock
class MockInternalLiveObjectsPlugin and either add a second subclass/instance
with var compatibleWithProtocolV6: Bool { false } or parameterize the mock, then
update PluginAPITests.swift to include the new test invoking the same
initialization/registration logic used by the current success test and assert
the negative outcome.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
Package.resolvedPackage.swiftSource/ARTClientOptions.mSource/ARTDefault.mSource/ARTPluginAPI.mTest/AblyTests/Tests/ARTDefaultTests.swiftTest/AblyTests/Tests/PluginAPITests.swiftTest/AblyTests/Tests/RealtimeClientConnectionTests.swiftTest/AblyTests/Tests/RealtimeClientTests.swiftTest/AblyTests/Tests/RestClientTests.swift
🚧 Files skipped from review as they are similar to previous changes (4)
- Test/AblyTests/Tests/RestClientTests.swift
- Source/ARTClientOptions.m
- Test/AblyTests/Tests/RealtimeClientConnectionTests.swift
- Source/ARTPluginAPI.m
Note: This targets the
integration/protocol-v6branch.Summary
This implements the protocol-version-bumping process proposed in ably/ably-cocoa-plugin-support#10.
Note that all of the v6 changes are LiveObjects-related, hence no other changes in ably-cocoa.
Related PRs
ObjectMessagestructure ably-liveobjects-swift-plugin#114Summary by CodeRabbit
New Features
Improvements
Tests