Use server-sent GC grace period per spec

[lawrence-forooghian]

Skipped in 2167277 due to time constraints. Integration tests copied from JS at b2e5121.

The ably-cocoa changes are in ably/ably-cocoa#2150.

Resolves #32.

Summary by CodeRabbit

• Chores

  • Updated dependencies for improved compatibility and performance.

• Improvements

  • Enhanced garbage collection grace period handling with more flexible configuration options, allowing dynamic adjustment based on connection details while maintaining sensible defaults.

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

[coderabbitai]

Walkthrough

The PR implements dynamic garbage collection grace period configuration by reading server-provided values from connection details. It introduces a GracePeriod enum to distinguish between fixed and dynamic grace periods, adds lifecycle hooks to update the grace period on connection establishment, and includes integration tests to verify the behavior.

Changes

Cohort / File(s)	Change Summary
Dependency Updates AblyLiveObjects.xcworkspace/xcshareddata/swiftpm/Package.resolved, Package.resolved, Package.swift	Updated ably-cocoa and ably-cocoa-plugin-support dependency revisions and corresponding hash values.
GC Grace Period Infrastructure Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift	Introduced GracePeriod enum with fixed and dynamic cases, added mutableState property for grace period tracking, implemented nosync_setGarbageCollectionGracePeriod control method and testsOnly_gcGracePeriod accessor, updated GarbageCollectionOptions.gracePeriod to use the new enum type.
Plugin Integration Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift	Added computed garbageCollectionOptions property that derives grace period from plugin options or connection details, introduced nosync_onConnected hook to apply server-provided grace period on connection establishment.
Public API Proxy Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultRealtimeObjects.swift	Added testsOnly_gcGracePeriod computed property forwarding to proxied instance.
Test Support Tests/AblyLiveObjectsTests/Helpers/Ably+Concurrency.swift, Tests/AblyLiveObjectsTests/JS Integration Tests/TestProxyTransport.swift	Added onceAsync async/await helper for connection events, updated TestProxyTransport to include objectsGCGracePeriod field in connection details.
Integration Tests Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift	Added GC grace period initialization and dynamic update tests, updated tombstone GC scenarios to use new GracePeriod enum representation.

Sequence Diagram

sequenceDiagram
    participant Test
    participant Plugin as DefaultInternalPlugin
    participant Connection as ARTConnection
    participant InternalObjects as InternalDefaultRealtimeObjects
    participant GC as GC Scheduler

    Test->>Connection: Establish connection
    Connection-->>Plugin: Connection succeeded<br/>with ARTConnectionDetails
    Note over Connection: objectsGCGracePeriod provided<br/>in connection details

    Plugin->>Plugin: nosync_onConnected()<br/>Extract gracePeriod from details
    
    Plugin->>InternalObjects: nosync_setGarbageCollectionGracePeriod<br/>(dynamicGracePeriod)
    activate InternalObjects
    InternalObjects->>InternalObjects: Update mutableState<br/>.garbageCollectionGracePeriod<br/>to dynamic value
    deactivate InternalObjects

    Note over InternalObjects: GC operations now use<br/>server-provided grace period

    InternalObjects->>GC: performGarbageCollection()<br/>uses updated grace period
    GC->>GC: Schedule cleanup with<br/>server-provided timeout

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Type system changes: The introduction of GracePeriod enum and the refactoring of GarbageCollectionOptions.gracePeriod from TimeInterval requires careful review of all usage sites
• State management: The new mutableState.garbageCollectionGracePeriod field and its initialization/update lifecycle needs verification across connection establishment and GC operations
• Integration flow: Verify that nosync_onConnected in DefaultInternalPlugin correctly extracts and applies server-provided grace periods
• Test coverage alignment: Ensure ObjectsIntegrationTests adequately cover both fixed and dynamic grace period scenarios, and that deprecated hardcoded 24-hour assumptions are properly replaced

Poem

🐰 A grace period, dynamic and spry,
No more hardcoded—it floats from the sky!
Server whispers the timeout so true,
Our garbage now collects right on cue. ✨
From connection to cleanup, a graceful ballet!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 53.85% 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 clearly and concisely describes the main change: using server-sent GC grace period per specification instead of a hardcoded value.
Linked Issues check	✅ Passed	All coding requirements from issue #32 are met: server-provided gcGracePeriod is now used, plugin support for passing CONNECTED value is implemented, and dependency updates are included.
Out of Scope Changes check	✅ Passed	All changes directly support the objective of using server-sent GC grace periods. Dependency updates, grace period abstraction, connection details handling, and test infrastructure are all aligned with the PR objectives.

✨ 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 32-use-server-gcGracePeriod

---

📜 Recent 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 f35f293 and d647c46.

📒 Files selected for processing (9)

• AblyLiveObjects.xcworkspace/xcshareddata/swiftpm/Package.resolved (1 hunks)
• Package.resolved (1 hunks)
• Package.swift (1 hunks)
• Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift (2 hunks)
• Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (5 hunks)
• Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultRealtimeObjects.swift (1 hunks)
• Tests/AblyLiveObjectsTests/Helpers/Ably+Concurrency.swift (1 hunks)
• Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift (3 hunks)
• Tests/AblyLiveObjectsTests/JS Integration Tests/TestProxyTransport.swift (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.swift

📄 CodeRabbit inference engine (.cursor/rules/swift.mdc)

**/*.swift: Specify an explicit access control level (SwiftLint explicit_acl) for all declarations in Swift code (tests are exempt)
When extending a type, put the access level on the extension declaration rather than on each member (tests are exempt)
Prefer implicit .init(...) when the type can be inferred in initializer expressions
Prefer enum case shorthand (.caseName) when the type can be inferred
For JSONValue or WireValue, prefer using literal syntax via ExpressibleBy*Literal where possible
Prefer Swift raw string literals for JSON strings instead of escaping double quotes
When an array literal begins with an initializer expression, place the initializer on the line after the opening bracket

Files:

• Tests/AblyLiveObjectsTests/Helpers/Ably+Concurrency.swift
• Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift
• Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultRealtimeObjects.swift
• Tests/AblyLiveObjectsTests/JS Integration Tests/TestProxyTransport.swift
• Package.swift
• Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift
• Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift

Tests/**/*.swift

📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)

Tests/**/*.swift: Use the Swift Testing framework (import Testing), not XCTest, in test files
Do not use fatalError for expectation failures; prefer Swift Testing’s #require
Only add labels to test cases or suites when the label differs from the suite struct or test method name
Tag tests per CONTRIBUTING.md’s "Attributing tests to a spec point" with exact comment format; distinguish @spec vs @specPartial; do not repeat @spec for the same spec point
Add comments in tests to clarify when certain test data is irrelevant to the scenario
In tests, import Ably using import Ably
In tests, import AblyLiveObjects using @testable import AblyLiveObjects
In tests, import _AblyPluginSupportPrivate using import _AblyPluginSupportPrivate (do not use internal import)
When passing a logger to internal components in tests, use TestLogger()
When unwrapping optionals in tests, prefer #require over guard let

Files:

• Tests/AblyLiveObjectsTests/Helpers/Ably+Concurrency.swift
• Tests/AblyLiveObjectsTests/JS Integration Tests/TestProxyTransport.swift
• Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift

Sources/AblyLiveObjects/**/*.swift

📄 CodeRabbit inference engine (.cursor/rules/swift.mdc)

In AblyLiveObjects library (non-test) code, import modules as: Ably with import Ably, and _AblyPluginSupportPrivate with internal import _AblyPluginSupportPrivate

Files:

• Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift
• Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultRealtimeObjects.swift
• Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift

Sources/**/*.swift

📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)

For testsOnly_ property declarations, do not add generic explanatory comments (their meaning is understood)

Files:

• Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift
• Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultRealtimeObjects.swift
• Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift

🧬 Code graph analysis (3)

Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift (1)

Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (1)

• nosync_setGarbageCollectionGracePeriod (396-406)

Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (1)

Sources/AblyLiveObjects/Utility/DispatchQueueMutex.swift (2)

• withoutSync (43-47)
• withSync (25-35)

Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift (1)

Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (1)

• nosync_setGarbageCollectionGracePeriod (396-406)

⏰ 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). (13)

• GitHub Check: Example app, tvOS (Xcode 16.3)
• GitHub Check: Example app, macOS (Xcode 16.3)
• GitHub Check: Xcode, release configuration, tvOS (Xcode 16.3)
• GitHub Check: Example app, iOS (Xcode 16.3)
• GitHub Check: Xcode, release configuration, iOS (Xcode 16.3)
• GitHub Check: Xcode, tvOS (Xcode 16.3)
• GitHub Check: Xcode, macOS (Xcode 16.3)
• GitHub Check: Xcode, release configuration, macOS (Xcode 16.3)
• GitHub Check: SPM, release configuration (Xcode 16.3)
• GitHub Check: Xcode, iOS (Xcode 16.3)
• GitHub Check: SPM (Xcode 16.3)
• GitHub Check: Generate code coverage
• GitHub Check: check-documentation

🔇 Additional comments (14)

AblyLiveObjects.xcworkspace/xcshareddata/swiftpm/Package.resolved (1)

2-17: LGTM!

The lock file updates are consistent with the dependency revision changes in Package.swift for ably-cocoa and ably-cocoa-plugin-support.

Package.swift (1)

23-30: LGTM!

The dependency revisions are updated to include the objectsGCGracePeriod support from ably-cocoa. The existing TODO comments appropriately flag these for unpinning before release.

Package.resolved (1)

2-17: LGTM!

Lock file updates are consistent with Package.swift.

Tests/AblyLiveObjectsTests/Helpers/Ably+Concurrency.swift (1)

52-61: LGTM!

The async wrapper follows the established pattern in this file, correctly using withCheckedContinuation for the one-shot once() callback. The @discardableResult annotation is appropriate for cases where callers only need to await the event occurrence.

Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultRealtimeObjects.swift (1)

126-128: LGTM!

The test-only accessor correctly forwards to the internal implementation, following the established pattern for other testsOnly_* properties in this class. As per coding guidelines, no explanatory comment is needed.

Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (5)

30-53: Well-structured grace period abstraction.

The GracePeriod enum cleanly separates fixed (test-only) from dynamic (production) grace periods. The toTimeInterval helper consolidates the extraction logic.

---

132-132: LGTM!

The mutable state is correctly initialized with the grace period from garbageCollectionOptions.

---

373-380: LGTM!

Garbage collection now uses the mutable grace period via toTimeInterval, allowing it to reflect server-provided values when using the dynamic mode.

---

439-440: LGTM!

The MutableState correctly stores the grace period enum, enabling both fixed and dynamic modes.

---

391-412: Call sites already verified by dispatchPrecondition in withoutSync.

The method uses mutableStateMutex.withoutSync, which includes dispatchPrecondition(condition: .onQueue(dispatchQueue)) (DispatchQueueMutex.swift:44). This enforces at runtime that the method is called from the correct dispatch queue. The single production call site is from nosync_onConnected, which is part of LiveObjectsInternalPluginProtocol and is invoked by ably-cocoa from the internal queue. The test call site is explicitly wrapped with client.internal.queue.ably_syncNoDeadlock. No action required.

Tests/AblyLiveObjectsTests/JS Integration Tests/TestProxyTransport.swift (1)

401-408: Simulated connection details now carry objectsGCGracePeriod (looks good)

Wiring objectsGCGracePeriod into the ARTConnectionDetails used by simulateTransportSuccess keeps the test transport in line with the real connection details and enables the new GC grace-period tests without altering control flow.

Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift (1)

43-61: GC grace-period initialization and CONNECTED hook align with dynamic server configuration

The new garbageCollectionOptions computation and nosync_onConnected hook together give the desired behavior:

• Prefer user-specified options.garbageCollectionOptions.
• Otherwise, initialize GC with a .dynamic grace period from the latest connection details when available.
• On each CONNECTED, update the grace period via nosync_setGarbageCollectionGracePeriod, while respecting .fixed modes (no-op) inside InternalDefaultRealtimeObjects.

This looks consistent with the nosync_setGarbageCollectionGracePeriod semantics and avoids overriding fixed configuration.

Please double-check (in the ably-cocoa integration / specs) that:

• nosync_onConnected is only called for channels that have already been prepared (so pluginData exists), and
• objectsGCGracePeriod’s numeric units match what GarbageCollectionOptions.GracePeriod expects (i.e., any ms→seconds conversion happens in exactly one place and not twice).

Also applies to: 146-151

Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift (2)

3733-3788: GC grace-period tests clearly cover server and default behaviors

The two new tests:

• Verify initial GC grace period is sourced from connectionDetails.objectsGCGracePeriod, and
• Confirm that subsequent CONNECTED messages both update it when present and reset it to defaultGracePeriod when absent,

using TestProxyTransport and the internal queue to keep everything deterministic. This provides solid coverage for the new wiring without introducing race conditions.

---

3953-3974: Tombstone GC harness correctly adapted to GracePeriod enum

Switching the test GC configuration to gracePeriod: .fixed(0.25) and using garbageCollectionOptions.gracePeriod.toTimeInterval when computing timeUntilGracePeriodExpires is consistent with the new enum-based representation and ensures the tombstone-collection logic still uses the intended 0.25s grace period in a type-safe way.

---

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

[maratal]

LGTM