From c5a55874877570598a9b72f1248db016f10b5c1e Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 20 Nov 2024 16:17:01 -0300 Subject: [PATCH] Only fetch channels and create contributors for enabled features Resolves #105. [1] https://github.com/ably/specification/pull/237 --- Sources/AblyChat/Room.swift | 125 ++++++++++++++------- Tests/AblyChatTests/DefaultRoomTests.swift | 53 +++++---- 2 files changed, 112 insertions(+), 66 deletions(-) diff --git a/Sources/AblyChat/Room.swift b/Sources/AblyChat/Room.swift index c7ba88db..f250dd19 100644 --- a/Sources/AblyChat/Room.swift +++ b/Sources/AblyChat/Room.swift @@ -73,6 +73,51 @@ internal actor DefaultRoom private let logger: InternalLogger + private enum RoomFeatureWithOptions { + case messages + case presence(PresenceOptions) + case typing(TypingOptions) + case reactions(RoomReactionsOptions) + case occupancy(OccupancyOptions) + + var toRoomFeature: RoomFeature { + switch self { + case .messages: + .messages + case .presence: + .presence + case .typing: + .typing + case .reactions: + .reactions + case .occupancy: + .occupancy + } + } + + static func fromRoomOptions(_ roomOptions: RoomOptions) -> [Self] { + var result: [Self] = [.messages] + + if let presenceOptions = roomOptions.presence { + result.append(.presence(presenceOptions)) + } + + if let typingOptions = roomOptions.typing { + result.append(.typing(typingOptions)) + } + + if let reactionsOptions = roomOptions.reactions { + result.append(.reactions(reactionsOptions)) + } + + if let occupancyOptions = roomOptions.occupancy { + result.append(.occupancy(occupancyOptions)) + } + + return result + } + } + internal init(realtime: RealtimeClient, chatAPI: ChatAPI, roomID: String, options: RoomOptions, logger: InternalLogger, lifecycleManagerFactory: LifecycleManagerFactory) async throws { self.realtime = realtime self.roomID = roomID @@ -84,7 +129,9 @@ internal actor DefaultRoom throw ARTErrorInfo.create(withCode: 40000, message: "Ensure your Realtime instance is initialized with a clientId.") } - let featureChannelPartialDependencies = Self.createFeatureChannelPartialDependencies(roomID: roomID, roomOptions: options, realtime: realtime) + let featuresWithOptions = RoomFeatureWithOptions.fromRoomOptions(options) + + let featureChannelPartialDependencies = Self.createFeatureChannelPartialDependencies(roomID: roomID, featuresWithOptions: featuresWithOptions, realtime: realtime) channels = featureChannelPartialDependencies.mapValues(\.channel) let contributors = featureChannelPartialDependencies.values.map(\.contributor) @@ -95,8 +142,6 @@ internal actor DefaultRoom let featureChannels = Self.createFeatureChannels(partialDependencies: featureChannelPartialDependencies, lifecycleManager: lifecycleManager) - // TODO: Address force unwrapping of `channels` within feature initialisation below: https://github.com/ably-labs/ably-chat-swift/issues/105 - messages = await DefaultMessages( featureChannel: featureChannels[.messages]!, chatAPI: chatAPI, @@ -105,26 +150,38 @@ internal actor DefaultRoom logger: logger ) - _reactions = options.reactions != nil ? await DefaultRoomReactions( - featureChannel: featureChannels[.reactions]!, - clientID: clientId, - roomID: roomID, - logger: logger - ) : nil + _reactions = if let featureChannel = featureChannels[.reactions] { + await DefaultRoomReactions( + featureChannel: featureChannel, + clientID: clientId, + roomID: roomID, + logger: logger + ) + } else { + nil + } - _presence = options.presence != nil ? await DefaultPresence( - featureChannel: featureChannels[.presence]!, - roomID: roomID, - clientID: clientId, - logger: logger - ) : nil + _presence = if let featureChannel = featureChannels[.presence] { + await DefaultPresence( + featureChannel: featureChannel, + roomID: roomID, + clientID: clientId, + logger: logger + ) + } else { + nil + } - _occupancy = options.occupancy != nil ? DefaultOccupancy( - featureChannel: featureChannels[.occupancy]!, - chatAPI: chatAPI, - roomID: roomID, - logger: logger - ) : nil + _occupancy = if let featureChannel = featureChannels[.occupancy] { + DefaultOccupancy( + featureChannel: featureChannel, + chatAPI: chatAPI, + roomID: roomID, + logger: logger + ) + } else { + nil + } } private struct FeatureChannelPartialDependencies { @@ -133,32 +190,30 @@ internal actor DefaultRoom } /// The returned dictionary is guaranteed to have an entry for each element of `features`. - private static func createChannelsForFeatures(_ features: [RoomFeature], roomID: String, roomOptions _: RoomOptions, realtime: RealtimeClient) -> [RoomFeature: RealtimeChannelProtocol] { + private static func createChannelsForFeaturesWithOptions(_ featuresWithOptions: [RoomFeatureWithOptions], roomID: String, realtime: RealtimeClient) -> [RoomFeature: RealtimeChannelProtocol] { // CHA-RC3a // Multiple features can share a realtime channel. We fetch each realtime channel exactly once, merging the channel options for the various features that use this channel. - let featuresGroupedByChannelName = Dictionary(grouping: features) { $0.channelNameForRoomID(roomID) } + let featuresGroupedByChannelName = Dictionary(grouping: featuresWithOptions) { $0.toRoomFeature.channelNameForRoomID(roomID) } let pairsOfFeatureAndChannel = featuresGroupedByChannelName.flatMap { channelName, features in var channelOptions = RealtimeChannelOptions() // channel setup for presence and occupancy for feature in features { - if feature == .presence { + if case /* let */ .presence /* (presenceOptions) */ = feature { // TODO: Restore this code once we understand weird Realtime behaviour and spec points (https://github.com/ably-labs/ably-chat-swift/issues/133) /* - let presenceOptions = roomOptions.presence - - if presenceOptions?.enter ?? false { + if presenceOptions.enter { channelOptions.modes.insert(.presence) } - if presenceOptions?.subscribe ?? false { + if presenceOptions.subscribe { channelOptions.modes.insert(.presenceSubscribe) } */ - } else if feature == .occupancy { + } else if case .occupancy = feature { var params: [String: String] = channelOptions.params ?? [:] params["occupancy"] = "metrics" channelOptions.params = params @@ -166,20 +221,14 @@ internal actor DefaultRoom } let channel = realtime.getChannel(channelName, opts: channelOptions) - return features.map { ($0, channel) } + return features.map { ($0.toRoomFeature, channel) } } return Dictionary(uniqueKeysWithValues: pairsOfFeatureAndChannel) } - private static func createFeatureChannelPartialDependencies(roomID: String, roomOptions: RoomOptions, realtime: RealtimeClient) -> [RoomFeature: FeatureChannelPartialDependencies] { - let features: [RoomFeature] = [ - .messages, - .reactions, - .presence, - .occupancy, - ] - let channelsByFeature = createChannelsForFeatures(features, roomID: roomID, roomOptions: roomOptions, realtime: realtime) + private static func createFeatureChannelPartialDependencies(roomID: String, featuresWithOptions: [RoomFeatureWithOptions], realtime: RealtimeClient) -> [RoomFeature: FeatureChannelPartialDependencies] { + let channelsByFeature = createChannelsForFeaturesWithOptions(featuresWithOptions, roomID: roomID, realtime: realtime) return .init(uniqueKeysWithValues: channelsByFeature.map { feature, channel in let contributor = DefaultRoomLifecycleContributor(channel: .init(underlyingChannel: channel), feature: feature) diff --git a/Tests/AblyChatTests/DefaultRoomTests.swift b/Tests/AblyChatTests/DefaultRoomTests.swift index e69494a0..a4b4b22b 100644 --- a/Tests/AblyChatTests/DefaultRoomTests.swift +++ b/Tests/AblyChatTests/DefaultRoomTests.swift @@ -11,7 +11,6 @@ struct DefaultRoomTests { // Given: A DefaultRoom instance let channelsList = [ MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), ] let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels) @@ -29,7 +28,6 @@ struct DefaultRoomTests { // Given: A DefaultRoom instance, configured to use presence and occupancy let channelsList = [ MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), ] let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels) @@ -38,7 +36,7 @@ struct DefaultRoomTests { // Then: It fetches the …$chatMessages channel (which is used by messages, presence, and occupancy) only once, and the options with which it does so are the result of merging the options used by the presence feature and those used by the occupancy feature let channelsGetArguments = channels.getArguments - #expect(channelsGetArguments.map(\.name).sorted() == ["basketball::$chat::$chatMessages", "basketball::$chat::$reactions"]) + #expect(channelsGetArguments.map(\.name).sorted() == ["basketball::$chat::$chatMessages"]) let chatMessagesChannelGetOptions = try #require(channelsGetArguments.first { $0.name == "basketball::$chat::$chatMessages" }?.options) #expect(chatMessagesChannelGetOptions.params?["occupancy"] == "metrics") @@ -54,7 +52,6 @@ struct DefaultRoomTests { // Given: a DefaultRoom instance let channelsList = [ MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), // required as DefaultRoom attaches reactions implicitly for now ] let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels) @@ -64,32 +61,35 @@ struct DefaultRoomTests { #expect(room.messages.channel.name == "basketball::$chat::$chatMessages") } - // TODO: Only create contributors for features user has enabled (https://github.com/ably-labs/ably-chat-swift/issues/105) - // TODO: Only fetch channel for features user has enabled (https://github.com/ably-labs/ably-chat-swift/issues/105) + // @spec CHA-RC2c + // @spec CHA-RC2d + // @spec CHA-RC2f @Test - func fetchesChannelAndCreatesLifecycleContributorForEachFeature() async throws { - // Given: a DefaultRoom instance + func fetchesChannelAndCreatesLifecycleContributorForEnabledFeatures() async throws { + // Given: a DefaultRoom instance, initialized with options that request that the room use a strict subset of the possible features let channelsList = [ MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), ] let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels) + let roomOptions = RoomOptions( + presence: .init() + ) let lifecycleManagerFactory = MockRoomLifecycleManagerFactory() - _ = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: .init(), logger: TestLogger(), lifecycleManagerFactory: lifecycleManagerFactory) + _ = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: roomOptions, logger: TestLogger(), lifecycleManagerFactory: lifecycleManagerFactory) // Then: It: - // - fetches the channel that corresponds to each feature (well, each one that we’ve currently implemented) - // - initializes the RoomLifecycleManager with a contributor for each feature (well, each one that we’ve currently implemented) + // - fetches the channel that corresponds to each feature requested by the room options, plus the messages feature + // - initializes the RoomLifecycleManager with a contributor for each feature requested by the room options, plus the messages feature let lifecycleManagerCreationArguments = try #require(await lifecycleManagerFactory.createManagerArguments.first) - let expectedFeatures: [RoomFeature] = [.messages, .occupancy, .reactions, .presence] + let expectedFeatures: [RoomFeature] = [.messages, .presence] // messages is implicitly always turned on #expect(lifecycleManagerCreationArguments.contributors.count == expectedFeatures.count) #expect(Set(lifecycleManagerCreationArguments.contributors.map(\.feature)) == Set(expectedFeatures)) let channelsGetArguments = channels.getArguments + // This is the channel used by the messages and presence features let expectedFetchedChannelNames = [ "basketball::$chat::$chatMessages", - "basketball::$chat::$reactions", ] #expect(channelsGetArguments.count == expectedFetchedChannelNames.count) #expect(Set(channelsGetArguments.map(\.name)) == Set(expectedFetchedChannelNames)) @@ -101,24 +101,26 @@ struct DefaultRoomTests { @Test(arguments: [.messages, .presence, .reactions, .occupancy] as[RoomFeature]) func whenFeatureEnabled_propertyGetterReturns(feature: RoomFeature) async throws { // Given: A RoomOptions with the (test argument `feature`) feature enabled in the room options - let roomOptions: RoomOptions = switch feature { + let roomOptions: RoomOptions + var namesOfChannelsToMock = ["basketball::$chat::$chatMessages"] + switch feature { case .messages: // Messages should always be enabled without needing any special options - .init() + roomOptions = .init() case .presence: - .init(presence: .init()) + roomOptions = .init(presence: .init()) case .reactions: - .init(reactions: .init()) + roomOptions = .init(reactions: .init()) + namesOfChannelsToMock.append("basketball::$chat::$reactions") case .occupancy: - .init(occupancy: .init()) + roomOptions = .init(occupancy: .init()) default: fatalError("Unexpected feature \(feature)") } - let channelsList = [ - MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), - ] + let channelsList = namesOfChannelsToMock.map { name in + MockRealtimeChannel(name: name) + } let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels) let room = try await DefaultRoom(realtime: realtime, chatAPI: ChatAPI(realtime: realtime), roomID: "basketball", options: roomOptions, logger: TestLogger(), lifecycleManagerFactory: MockRoomLifecycleManagerFactory()) @@ -151,7 +153,6 @@ struct DefaultRoomTests { // Given: a DefaultRoom instance let channelsList = [ MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), // required as DefaultRoom attaches reactions implicitly for now ] let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels) @@ -188,7 +189,6 @@ struct DefaultRoomTests { // Given: a DefaultRoom instance let channelsList = [ MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), // required as DefaultRoom attaches reactions implicitly for now ] let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels) @@ -221,7 +221,6 @@ struct DefaultRoomTests { // Given: a DefaultRoom instance let channelsList = [ MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), // required as DefaultRoom attaches reactions implicitly for now ] let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels) @@ -248,7 +247,6 @@ struct DefaultRoomTests { // Given: a DefaultRoom instance let channelsList = [ MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), // required as DefaultRoom attaches reactions implicitly for now ] let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels) @@ -269,7 +267,6 @@ struct DefaultRoomTests { // Given: a DefaultRoom instance let channelsList = [ MockRealtimeChannel(name: "basketball::$chat::$chatMessages"), - MockRealtimeChannel(name: "basketball::$chat::$reactions"), // required as DefaultRoom attaches reactions implicitly for now ] let channels = MockChannels(channels: channelsList) let realtime = MockRealtime.create(channels: channels)