From ee6b9bed185e098e50890ec478f008eff770e5a5 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 20 Nov 2024 16:17:01 -0300 Subject: [PATCH] =?UTF-8?q?wip=20only=20create=20contributors=20for=20stuf?= =?UTF-8?q?f=20that=20we=E2=80=99ll=20use?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TODO spec point TODO test that options are only specified if feature needs them (we can put them into an earlier test which we then split up) TODO test that contributors are only passed if feature is there (we can put this into an earlier test which we then split up) Resolves #105. --- Sources/AblyChat/Room.swift | 126 ++++++++++++++------- Tests/AblyChatTests/DefaultRoomTests.swift | 27 ++--- 2 files changed, 99 insertions(+), 54 deletions(-) diff --git a/Sources/AblyChat/Room.swift b/Sources/AblyChat/Room.swift index 49189ff2..c4596702 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,10 @@ 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) + // TODO: explain that this comes from features + 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 +143,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, @@ -104,26 +150,38 @@ internal actor DefaultRoom clientID: clientId ) - _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 { @@ -132,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 @@ -165,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 c1a4ac2c..cfc6fd34 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) @@ -54,7 +53,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) @@ -70,24 +68,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()) @@ -120,7 +120,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) @@ -157,7 +156,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) @@ -190,7 +188,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) @@ -217,7 +214,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) @@ -238,7 +234,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)