Add onAttach callback variants to annotations subscribe

[lawrence-forooghian]

Commit d7c8b45 introduced RealtimeAnnotations#subscribe but omitted the onAttach callback variants that RealtimeChannel#subscribe provides. Per RTAN4d, annotations subscribe must have the same return value and callback semantics as channel subscribe (RTL7g), which includes the optional attach callback.

This adds subscribeWithAttachCallback:callback: and subscribe:onAttach:callback: to ARTRealtimeAnnotations, mirroring the existing channel API. The internal _subscribe:onAttach:callback: method already accepted an onAttach parameter but the public API was not wiring it through.

Also fixes a pre-existing bug in _subscribe:onAttach:callback: where the onAttach callback was not being wrapped to dispatch to the user queue, unlike the channel's equivalent. The channel wraps onAttach to dispatch to the user queue so that it is called on the correct queue per the public API contract.

Tests are copied from the RTL7g/RTL7h channel subscribe tests in RealtimeClientChannelTests.swift, adapted to use annotations subscribe.

Summary by CodeRabbit

• New Features

  • Added attach-callback support to annotation subscriptions, including per-type subscription variants.
  • Attach callbacks are now invoked on the user's execution queue for correct asynchronous behavior.

• Tests

  • Added comprehensive tests covering annotation subscription behavior, channel attach scenarios, and error/state handling during subscribe operations.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6431b137-c6ca-4a86-bf03-70a3d1aaaf7a

📥 Commits

Reviewing files that changed from the base of the PR and between 6ee0b84 and 5ca4c4a.

📒 Files selected for processing (4)

• Source/ARTRealtimeAnnotations.m
• Source/ARTWrapperSDKProxyRealtimeAnnotations.m
• Source/include/Ably/ARTRealtimeAnnotations.h
• Test/AblyTests/Tests/RealtimeAnnotationsTests.swift

Disabled knowledge base sources:

• Jira integration is disabled

You can enable these sources in your CodeRabbit configuration.

---

Walkthrough

Adds optional attach-callback variants to annotation subscription APIs, routes them through internal subscription methods, and ensures provided onAttach callbacks are dispatched asynchronously on the user queue. Proxy and header declarations updated; tests added (duplicated in file).

Changes

Cohort / File(s)	Summary
API Header Definition Source/include/Ably/ARTRealtimeAnnotations.h	Replaced prior subscribe signatures with new variants that accept an optional onAttach callback: subscribeWithAttachCallback:onAttach:callback: and subscribe:type onAttach:callback:.
Core Implementation Source/ARTRealtimeAnnotations.m	Added public subscribe variants routing to internal _subscribe paths and introduced internal subscribe...onAttach... overloads; when onAttach is provided it is wrapped to dispatch asynchronously onto the user queue before invoking the user callback.
Internal Routing Source/...ARTRealtimeAnnotations.m (internal classes)	Added matching internal ARTRealtimeAnnotationsInternal methods mirroring the public APIs to forward subscription behavior and apply the onAttach user-queue dispatch.
Proxy Wrapper Source/ARTWrapperSDKProxyRealtimeAnnotations.m	Added proxy methods that delegate the new attach-callback subscription variants to the underlying realtime annotations instance.
Tests Test/AblyTests/Tests/RealtimeAnnotationsTests.swift	Added four test cases covering attach-on-subscribe true/false and failed-channel behaviors; the new test blocks appear duplicated in the file.

Sequence Diagram(s)

sequenceDiagram
participant Caller
participant ARTRealtimeAnnotations
participant ARTRealtimeAnnotationsInternal
participant UserQueue

Caller->>ARTRealtimeAnnotations: subscribeWithAttachCallback(onAttach, callback)
ARTRealtimeAnnotations->>ARTRealtimeAnnotationsInternal: subscribe...onAttach...(onAttach, callback)
alt onAttach provided
ARTRealtimeAnnotationsInternal->>UserQueue: dispatch async wrapper for onAttach
UserQueue-->>ARTRealtimeAnnotationsInternal: wrapped onAttach calls original onAttach
end
ARTRealtimeAnnotationsInternal->>ARTRealtimeAnnotations: return ARTEventListener
ARTRealtimeAnnotations-->>Caller: ARTEventListener

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I hopped in code, with callbacks bright,
Wrapped onAttach to keep threads right,
I dispatch softly to the user's queue,
Subscriptions bloom — a rabbit's review. 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add onAttach callback variants to annotations subscribe' clearly and concisely describes the main change: introducing new onAttach callback method variants for the annotations subscribe API.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch annotations-onattach

📝 Coding Plan

• Generate coding plan for human review comments

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

Test/AblyTests/Tests/RealtimeAnnotationsTests.swift (1)

277-282: Prefer deterministic assertions over fixed delays in negative-callback tests.

delay(1) can introduce CI flakiness; inverted expectations (or equivalent) will make these checks more stable.

Also applies to: 324-329

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Test/AblyTests/Tests/RealtimeAnnotationsTests.swift` around lines 277 - 282,
Replace the non-deterministic delay(1) inside the waitUntil block with a
deterministic Nimble eventual/inverted expectation: instead of using delay to
wait, assert channel.state reaches ARTRealtimeChannelState.initialized with
expect(channel.state).toEventually(equal(ARTRealtimeChannelState.initialized),
timeout: testTimeout) (or use an inverted eventual expectation if testing a
negative callback), and remove the delay/done pairing; apply the same change for
the other occurrence that asserts channel.state in this file.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@Test/AblyTests/Tests/RealtimeAnnotationsTests.swift`:
- Around line 277-282: Replace the non-deterministic delay(1) inside the
waitUntil block with a deterministic Nimble eventual/inverted expectation:
instead of using delay to wait, assert channel.state reaches
ARTRealtimeChannelState.initialized with
expect(channel.state).toEventually(equal(ARTRealtimeChannelState.initialized),
timeout: testTimeout) (or use an inverted eventual expectation if testing a
negative callback), and remove the delay/done pairing; apply the same change for
the other occurrence that asserts channel.state in this file.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5dc2920b-608f-48e0-88f3-a7390409c521

📥 Commits

Reviewing files that changed from the base of the PR and between 83bb536 and 6ee0b84.

📒 Files selected for processing (4)

• Source/ARTRealtimeAnnotations.m
• Source/ARTWrapperSDKProxyRealtimeAnnotations.m
• Source/include/Ably/ARTRealtimeAnnotations.h
• Test/AblyTests/Tests/RealtimeAnnotationsTests.swift

[maratal]

LGTM (two small formatting issues)