[ECO-4982] Implement room release #107
Conversation
We want to be able to mock out the upcoming `Room.release()` method.
|
Warning Rate limit exceeded@lawrence-forooghian has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (16)
Tests/AblyChatTests/Mocks/MockRoomFactory.swift (2)
1-5: LGTM! Consider adding a type alias for arguments tuple.The actor declaration and properties are well-structured. The use of an actor ensures thread-safe access to the mock's state, which is essential for testing concurrent code.
Consider adding a type alias to improve readability:
+typealias RoomArguments = (realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: any InternalLogger) actor MockRoomFactory: RoomFactory { private var room: MockRoom? - private(set) var createRoomArguments: (realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: any InternalLogger)? + private(set) var createRoomArguments: RoomArguments?
15-23: Enhance the fatal error message with more context.The implementation correctly captures arguments and handles the room creation flow. However, the error message could be more specific to help with debugging test failures.
Consider enhancing the error message:
- fatalError("MockRoomFactory.createRoom called, but the mock factory has not been set up with a room to return") + fatalError("MockRoomFactory.createRoom called for roomID: \(roomID), but the mock factory has not been set up with a room to return. Ensure setRoom() is called before createRoom().")Tests/AblyChatTests/DefaultChatClientTests.swift (1)
26-27: Consider strengthening the equality assertionsThe current assertions verify object identity for realtime (
===) and value equality for options. This looks good, but consider adding assertions to verify that the factory is properly configured.Add a test helper to verify factory configuration:
extension DefaultRooms { var testsOnly_factory: Factory { factory } }Then enhance the test:
#expect(defaultRooms.testsOnly_realtime === realtime) #expect(defaultRooms.clientOptions.isEqualForTestPurposes(options)) +#expect(defaultRooms.testsOnly_factory is DefaultRoomFactory)Tests/AblyChatTests/Mocks/MockChannels.swift (1)
36-41: Consider defer and evaluate array copying necessityTwo suggestions for improvement:
- Use
deferfor safer mutex handling- Consider if array copying is necessary for this test mock
var releaseArguments: [String] { - let result: [String] mutex.lock() - result = _releaseArguments - mutex.unlock() - return result + defer { mutex.unlock() } + return _releaseArguments }Tests/AblyChatTests/Mocks/MockRoom.swift (3)
3-11: Add documentation to explain mock usage and configurationThe mock implementation is well-structured, but would benefit from documentation explaining:
- Purpose of the mock
- How to configure the
releaseImplementation- Example usage in tests
Add documentation like this:
+/// A mock implementation of `InternalRoom` that tracks release calls and allows custom release behavior. +/// +/// Example usage: +/// ```swift +/// let mockRoom = MockRoom(options: options) { +/// // Custom release implementation +/// } +/// await mockRoom.release() +/// XCTAssertEqual(await mockRoom.releaseCallCount, 1) +/// ``` actor MockRoom: InternalRoom {
13-35: Improve error messages and consider future test requirementsThe error messages could be more descriptive to help debugging test failures. Also, consider implementing getters that might be needed for future tests instead of using
fatalError.Example improvement:
nonisolated var roomID: String { - fatalError("Not implemented") + fatalError("MockRoom.roomID getter not implemented. Configure this property if needed for your test.") }
41-59: Consider tracking attach/detach calls for comprehensive testingWhile
release()is well implemented with call tracking, consider adding similar tracking forattach()anddetach()methods as they might be needed for testing the complete room lifecycle.Example implementation:
+ private(set) var attachCallCount = 0 + private(set) var detachCallCount = 0 + let attachImplementation: (@Sendable () async throws -> Void)? + let detachImplementation: (@Sendable () async throws -> Void)? + func attach() async throws { - fatalError("Not implemented") + attachCallCount += 1 + try await attachImplementation?() } func detach() async throws { - fatalError("Not implemented") + detachCallCount += 1 + try await detachImplementation?() }Sources/AblyChat/ChatClient.swift (1)
23-24: Consider injecting the room factory as a dependencyThe current implementation creates a hardcoded instance of
DefaultRoomFactory. Consider injecting the factory through the initializer to improve testability and flexibility.Here's a suggested implementation:
- public init(realtime: RealtimeClient, clientOptions: ClientOptions?) { + public init( + realtime: RealtimeClient, + clientOptions: ClientOptions?, + roomFactory: RoomFactory = DefaultRoomFactory() + ) { self.realtime = realtime self.clientOptions = clientOptions ?? .init() logger = DefaultInternalLogger(logHandler: self.clientOptions.logHandler, logLevel: self.clientOptions.logLevel) - let roomFactory = DefaultRoomFactory() rooms = DefaultRooms(realtime: realtime, clientOptions: self.clientOptions, logger: logger, roomFactory: roomFactory) }Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
36-38: Consider aligning release operation with existing patterns.While the implementation is functional, it deviates from the established patterns used in
performAttachOperationandperformDetachOperation. Consider these improvements:
- Add a
releaseResultparameter for consistency with other operations- Add documentation explaining the method's purpose
- Add guard/fatal error check like other operations
Here's a suggested implementation:
- private(set) var releaseCallCount = 0 + private let releaseResult: Result<Void, ARTErrorInfo>? + private(set) var releaseCallCount = 0 init(attachResult: Result<Void, ARTErrorInfo>? = nil, detachResult: Result<Void, ARTErrorInfo>? = nil, + releaseResult: Result<Void, ARTErrorInfo>? = nil, roomStatus: RoomStatus? = nil) { self.attachResult = attachResult self.detachResult = detachResult + self.releaseResult = releaseResult _roomStatus = roomStatus } - func performReleaseOperation() async { - releaseCallCount += 1 - } + /// Performs the release operation and throws if the configured result is a failure + func performReleaseOperation() async throws { + releaseCallCount += 1 + guard let releaseResult else { + fatalError("In order to call performReleaseOperation, releaseResult must be passed to the initializer") + } + try releaseResult.get() + }Tests/AblyChatTests/IntegrationTests.swift (3)
81-83: Consider enhancing status verification robustness.The status verification is good but could be more comprehensive.
Consider these enhancements:
- Add timeout to prevent test hanging if status never changes
- Verify no unexpected intermediate states occur between DETACHED and RELEASED
// (13) Check that we received a RELEASED status change as a result of releasing the room -_ = try #require(await rxRoomStatusSubscription.first { $0.current == .released }) +let statusChanges = try await rxRoomStatusSubscription.prefix { $0.current != .released }.reduce(into: [RoomStatus]()) { $0.append($1.current) } +#expect(statusChanges.isEmpty, "Unexpected intermediate states: \(statusChanges)") + +// Verify final status #expect(await rxRoom.status == .released)
85-87: Consider verifying initial state of new room instance.While checking for a new instance is good, consider verifying the new room's initial state.
Add verification of the new room's state:
let postReleaseRxRoom = try await rxClient.rooms.get(roomID: roomID, options: .init()) #expect(postReleaseRxRoom !== rxRoom) +#expect(await postReleaseRxRoom.status == .initialized)
Line range hint
6-8: Address the TODO comment about implementing test timeouts.The comment indicates a need for timeout functionality to prevent tests from hanging indefinitely. This is particularly important for integration tests that interact with external services.
Would you like me to help implement a timeout wrapper function that works across all supported iOS versions? This would help prevent test hangs and provide better error messages when operations take too long.
Tests/AblyChatTests/DefaultRoomsTests.swift (1)
91-105: Consider extracting test setup helpersThe test setup is quite complex with multiple components. Consider extracting helper methods to improve readability and reusability:
- Stream setup
- Room creation with state capture
Example helper method:
private func createRoomWithStateCapture( options: RoomOptions = .init(), stateCapture: @escaping () async -> Void ) -> (room: MockRoom, factory: MockRoomFactory) { let room = MockRoom(options: options, onRelease: stateCapture) let factory = MockRoomFactory() return (room, factory) }Tests/AblyChatTests/DefaultRoomTests.swift (1)
99-120: Consider strengthening test assertions.The test effectively verifies the basic release functionality. Consider adding these additional assertions to make the test more robust:
// Then: It: // 1. calls `performReleaseOperation()` on the room lifecycle manager // 2. calls `channels.release()` with the name of each of the features' channels #expect(await lifecycleManager.releaseCallCount == 1) #expect(Set(channels.releaseArguments) == Set(channelsList.map(\.name))) +// Verify no other lifecycle manager methods were called +#expect(await lifecycleManager.attachCallCount == 0) +#expect(await lifecycleManager.detachCallCount == 0) +// Verify channels were released exactly once +#expect(channels.releaseCallCount == channelsList.count)Sources/AblyChat/Room.swift (2)
37-41: Simplify the associated type constraint inRoomFactoryIn the
RoomFactoryprotocol, the associated typeRoomis constrained with a module prefix. SinceInternalRoomis defined within the same module, the module qualification may be unnecessary.Consider updating the associated type constraint to:
associatedtype Room: InternalRoomThis simplifies the code and improves readability unless there's a specific need for the module prefix.
139-146: Confirm error handling when releasing channelsIn the
release()method, therealtime.channels.release(channel.name)call may benefit from error handling if it can fail or throw exceptions.Consider checking if
releasecan throw errors:
- If it can throw, add a
trystatement and handle potential errors.- If it is asynchronous, use
await.For example:
for channel in channels.values { try realtime.channels.release(channel.name) }Ensure that all resources are properly released and any exceptions are managed to prevent resource leaks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
Sources/AblyChat/ChatClient.swift(1 hunks)Sources/AblyChat/Room.swift(5 hunks)Sources/AblyChat/RoomLifecycleManager.swift(2 hunks)Sources/AblyChat/Rooms.swift(3 hunks)Tests/AblyChatTests/DefaultChatClientTests.swift(1 hunks)Tests/AblyChatTests/DefaultRoomTests.swift(1 hunks)Tests/AblyChatTests/DefaultRoomsTests.swift(4 hunks)Tests/AblyChatTests/IntegrationTests.swift(1 hunks)Tests/AblyChatTests/Mocks/MockChannels.swift(2 hunks)Tests/AblyChatTests/Mocks/MockRoom.swift(1 hunks)Tests/AblyChatTests/Mocks/MockRoomFactory.swift(1 hunks)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift(2 hunks)
🧰 Additional context used
📓 Learnings (3)
Sources/AblyChat/RoomLifecycleManager.swift (1)
Learnt from: lawrence-forooghian
PR: ably-labs/ably-chat-swift#85
File: Sources/AblyChat/RoomLifecycleManager.swift:713-713
Timestamp: 2024-11-10T17:12:32.395Z
Learning: In `RoomLifecycleManager.performReleaseOperation` in `Sources/AblyChat/RoomLifecycleManager.swift`, the use of `try!` with `waitForCompletionOfOperationWithID` is intentional due to the reasons explained in the preceding comment.
Tests/AblyChatTests/DefaultChatClientTests.swift (2)
Learnt from: lawrence-forooghian
PR: ably-labs/ably-chat-swift#61
File: Tests/AblyChatTests/DefaultRoomTests.swift:33-34
Timestamp: 2024-11-10T17:12:32.395Z
Learning: In Swift Testing, using `#expect` with `await` may result in less informative failure messages when debugging asynchronous tests.
Learnt from: lawrence-forooghian
PR: ably-labs/ably-chat-swift#61
File: Tests/AblyChatTests/DefaultRoomTests.swift:33-34
Timestamp: 2024-11-10T17:12:36.565Z
Learning: In Swift Testing, using `#expect` with `await` may result in less informative failure messages when debugging asynchronous tests.
Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (4)
Learnt from: lawrence-forooghian
PR: ably-labs/ably-chat-swift#54
File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:499-504
Timestamp: 2024-11-10T17:12:32.395Z
Learning: In test closures, parameters like `callCount` may be used to track invocation counts correctly across invocations.
Learnt from: lawrence-forooghian
PR: ably-labs/ably-chat-swift#54
File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:499-504
Timestamp: 2024-11-10T17:12:32.395Z
Learning: In test closures, parameters like `callCount` may be used to track invocation counts correctly across invocations.
Learnt from: lawrence-forooghian
PR: ably-labs/ably-chat-swift#54
File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:627-631
Timestamp: 2024-11-10T17:12:32.395Z
Learning: The closure `detachImpl` in `RoomLifecycleManagerTests.swift` functions as intended without external state variables to track invocation count.
Learnt from: lawrence-forooghian
PR: ably-labs/ably-chat-swift#54
File: Tests/AblyChatTests/RoomLifecycleManagerTests.swift:627-631
Timestamp: 2024-11-10T17:12:36.565Z
Learning: The closure `detachImpl` in `RoomLifecycleManagerTests.swift` functions as intended without external state variables to track invocation count.
🔇 Additional comments (17)
Tests/AblyChatTests/Mocks/MockRoomFactory.swift (2)
7-9: LGTM! Clean and flexible initializer.
The initializer is well-designed with an optional parameter, providing flexibility in test setup.
11-13: LGTM! Clear and purposeful setter.
The setter provides a clean way to configure the mock during test setup.
Tests/AblyChatTests/DefaultChatClientTests.swift (1)
25-27: Verify test coverage for the new room factory pattern
While the type assertion correctly validates the new DefaultRoomFactory implementation, consider adding test cases to verify the room release functionality mentioned in PR objectives (ECO-4982).
Let's check if there are dedicated tests for the release functionality:
Consider structuring the tests to cover:
- Successful room release
- Release of non-existent rooms
- Multiple releases of the same room
- Concurrent release operations
Tests/AblyChatTests/Mocks/MockChannels.swift (2)
6-8: LGTM! Thread safety implementation is solid.
Good use of NSLock and documentation. The private(set) modifier ensures thread-safe access control.
4-4: Verify thread safety of channels property
The class is marked as @unchecked Sendable, but the channels array is not protected by the mutex. If the array is immutable after initialization, this is fine. Otherwise, consider protecting it with the mutex as well.
✅ Verification successful
Thread safety of channels property is properly implemented
Looking at the implementation, the channels property is:
- Declared as private and immutable (let)
- Only set once during initialization
- Only used for read-only operations (first)
The @unchecked Sendable marking is appropriate as the class properly handles thread safety:
- Immutable
channelsarray ensures thread-safe read access - Mutable state (
_releaseArguments) is protected by the mutex
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if channels array is modified anywhere in the codebase
# Look for any assignments to channels property or array mutations
ast-grep --pattern 'class MockChannels {
$$$
channels = $_
$$$
}'
ast-grep --pattern 'class MockChannels {
$$$
channels.$_($_)
$$$
}'
Length of output: 149
Script:
#!/bin/bash
# Let's try a different approach to find the channels property and its usage
# First, let's see the content of the MockChannels file
cat Tests/AblyChatTests/Mocks/MockChannels.swift
# Also search for any modifications to channels using ripgrep
rg "channels\." Tests/AblyChatTests/Mocks/MockChannels.swift -A 2
Length of output: 1406
Sources/AblyChat/ChatClient.swift (1)
23-24: Verify memory management with room releases
Since this change is part of implementing room release functionality, we should verify that the new factory properly handles memory management during room creation and release cycles.
✅ Verification successful
Based on the shell script results, I can now generate the final response:
Memory management implementation verified for room releases
The codebase shows proper memory management implementation for room releases:
- Appropriate
deinithandling inRoomLifecycleManagerthat cancels ongoing tasks - Proper weak self usage in closures to avoid retain cycles
- Clear state transitions through
.releasingto.releasedstates - Proper cleanup of channels and resources during release
- Test coverage verifying memory management scenarios including:
- Release state transitions
- Channel cleanup
- Resource deallocation
- Edge cases like multiple releases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for memory management related code in room implementations
echo "Searching for room release and deinit implementations..."
rg -A 5 "deinit|release" --type swift
# Look for potential retain cycles
echo "Checking for potential retain cycles in closures..."
ast-grep --pattern 'weak\s+self'
ast-grep --pattern '[^weak]\s+self'
Length of output: 34633
Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
9-9: LGTM! Property follows established patterns.
The releaseCallCount property maintains consistency with existing count tracking properties and has appropriate access control.
Tests/AblyChatTests/IntegrationTests.swift (1)
78-79: LGTM! Room release operation is well-placed.
The release operation correctly follows the room detachment, maintaining proper lifecycle order.
Tests/AblyChatTests/DefaultRoomsTests.swift (1)
Line range hint 6-82: Well-structured test refactoring using mocks!
The modifications to the room retrieval tests demonstrate good testing practices:
- Clear separation of concerns using mock objects
- Comprehensive test coverage for success and error scenarios
- Strong assertions verifying both object identity and property values
Tests/AblyChatTests/DefaultRoomTests.swift (1)
97-97: Consider adding order verification test.
While the current approach is pragmatic given time constraints, the order of operations could be critical for correctness. The spec point CHA-RL3h seems to indicate that performReleaseOperation() must complete before channel release.
Consider adding a follow-up test using async/await mechanisms to verify the operation order. I can help implement this if needed.
Sources/AblyChat/RoomLifecycleManager.swift (3)
868-870: LGTM! Public method follows established patterns.
The implementation of performReleaseOperation() aligns with other lifecycle operations and correctly delegates to the internal implementation.
872-874: LGTM! Test support maintains consistency.
The test-only variant with operation ID forcing maintains consistency with other lifecycle operations and supports proper testing of operation sequencing.
Line range hint 880-884: Verify non-throwing implementation approach.
The release operation is implemented as non-throwing, unlike ATTACH and DETACH operations. This appears intentional as the operation handles all error cases internally through state transitions.
Let's verify this design choice by checking other release-related implementations:
✅ Verification successful
Based on the comprehensive code analysis, I can now provide a definitive response about the non-throwing implementation of the release operation.
The non-throwing implementation is verified as intentional and consistent with the codebase design.
The evidence shows that:
- The release operation is consistently implemented as non-throwing across all layers:
RoomLifecycleManagerprotocol definesperformReleaseOperation()as non-throwingDefaultRoomLifecycleManagerimplements release operations without throws- The implementation handles all error cases internally through state transitions
- The design is deliberate as shown by:
- Release operation retries detach operations until success (CHA-RL3f)
- Failed contributors are skipped rather than causing errors (CHA-RL3e)
- State transitions handle all error cases internally
- This contrasts with attach/detach operations which are explicitly throwing to propagate errors to callers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the non-throwing pattern is consistent across release implementations
# Check for any release method implementations
ast-grep --pattern 'func $$$release$$$'
# Check for release error handling patterns
rg -A 5 'throw.*release'
Length of output: 68
Script:
#!/bin/bash
# Let's try a more comprehensive search for release-related implementations and error handling
# Search for release-related method implementations with context
rg -U -A 10 -B 5 'func.*[Rr]elease' --type swift
# Search for any throw statements in files containing 'release'
rg -l 'release' --type swift | xargs rg -A 5 'throw'
# Search specifically for the RoomLifecycleManager class implementation
fd RoomLifecycleManager.swift --exec cat {}
Length of output: 165798
Sources/AblyChat/Rooms.swift (2)
52-56: Approved: Addition of testsOnly_hasExistingRoomWithID(_:) method
The testsOnly_hasExistingRoomWithID(_:) method is a valuable addition for testing purposes. It is appropriately enclosed within #if DEBUG directives, ensuring it does not affect the production code.
65-68: Verify proper cleanup in room.release()
After removing the room from the rooms dictionary, await room.release() is called. Please ensure that the release() method within the room correctly handles all necessary cleanup operations, such as unsubscribing from channels and releasing resources, to prevent memory leaks or unintended behavior.
If you'd like, we can review the room.release() implementation to confirm it handles all required cleanup tasks.
Sources/AblyChat/Room.swift (2)
22-25: Introduction of InternalRoom protocol is appropriate
The creation of the InternalRoom protocol extending Room with an asynchronous release() method enhances internal room management within the SDK.
69-69: Verify the use of existential types with the any keyword
The declaration of channels uses the any keyword with a protocol type:
private let channels: [RoomFeature: any RealtimeChannelProtocol]Ensure that the use of any is necessary and compatible with your project's Swift version and coding standards. If the RealtimeChannelProtocol does not contain Self or associated type requirements, you might omit any.
Run the following script to check for any usage across the codebase:
This will help verify consistent usage of existential types.
✅ Verification successful
Let me gather more information about the RealtimeChannelProtocol to determine if it has any Self or associated type requirements.
Let me check the base protocol ARTRealtimeChannelProtocol to ensure we have a complete understanding of the protocol hierarchy.
Let me check for imports to understand where ARTRealtimeChannelProtocol comes from.
Based on the evidence gathered, I can now provide the final response:
The use of any keyword is necessary and correct
The RealtimeChannelProtocol inherits from ARTRealtimeChannelProtocol which is from the Ably SDK. The use of any is consistent across the codebase as shown by multiple similar declarations:
public typealias RealtimeClient = any RealtimeClientProtocolprivate let underlyingChannel: any RealtimeChannelProtocolfunc getChannel(_ name: String, opts: ARTRealtimeChannelOptions? = nil) -> any RealtimeChannelProtocol
This consistent usage pattern and the fact that it's working with an external SDK protocol suggests the any keyword is required here for proper type erasure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all occurrences of 'any' keyword usage with protocols.
# Search for 'any' keyword usage with protocols.
rg '\bany\s+[A-Z][A-Za-z0-9]+Protocol\b'
Length of output: 620
Script:
#!/bin/bash
# Search for the definition of RealtimeChannelProtocol
rg "protocol\s+RealtimeChannelProtocol" -A 10
Length of output: 1030
Script:
#!/bin/bash
# Search for ARTRealtimeChannelProtocol definition
rg "protocol\s+ARTRealtimeChannelProtocol" -A 10
# Also check if this is imported from a framework
rg "import.*ARTRealtimeChannelProtocol"
Length of output: 92
Script:
#!/bin/bash
# Search for imports in the relevant files
rg "^import" Sources/AblyChat/Dependencies.swift Sources/AblyChat/Room.swift
# Also check if there's any Ably related imports
rg "import.*Ably"
Length of output: 4835
lawrence-forooghian
force-pushed
the
47-implement-release
branch
from
fb0f60c
to
dc12be6
Compare
We use the implementation of the RELEASE operation provided by the room lifecycle manager, and implement the spec points relating to room map bookkeeping and releasing the underlying realtime channels. Based on [1] at 6f0740a. I have not implemented the spec points that relate to making sure that a room fetch waits for any previous room with the same ID to finish releasing; this is a part of the spec which is in flux (currently implemented via the INITIALIZING status, which was added to the spec after we started implementing the room lifecycle manager and hasn’t been implemented in this SDK yet, and soon to be further changed in the spec by making room-getting async). We can look at the current state of things when we come to do #66. Part of #47. [1] ably/specification#200
lawrence-forooghian
force-pushed
the
47-implement-release
branch
from
dc12be6
to
8daa191
Compare
Note: This PR is based on top of #106; please review that one first.
This implements the
DefaultRooms.release(roomID:)method. See commit messages for more details.Part of #47.
Summary by CodeRabbit
Release Notes
New Features
InternalRoomprotocol with enhanced room management capabilities.release()method for proper cleanup of rooms.MockRoomFactoryfor easier testing of room creation.Bug Fixes
Tests
release()method and integration tests for room lifecycle management.