From 25e3f41b5756fd11635d8b62558faf121e414a41 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 4 Dec 2024 13:42:29 -0300 Subject: [PATCH 1/4] Rename `Message.latestAction` to `action` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Upon our integration tests starting to fail, we noticed that Realtime seems to have renamed the `latestAction` property to `action`. This change has not yet been reflected in the spec, but has been in JS [1]. I’ve created an issue to get the spec updated [2] but since this test failure is stopping us from merging anything else I am going to rename this property until we get a more definitive answer. Part of #169. [1] https://github.com/ably/ably-chat-js/pull/427 [2] https://github.com/ably/specification/issues/254 --- Example/AblyChatExample/Mocks/Misc.swift | 2 +- Example/AblyChatExample/Mocks/MockClients.swift | 4 ++-- Sources/AblyChat/ChatAPI.swift | 2 +- Sources/AblyChat/DefaultMessages.swift | 2 +- Sources/AblyChat/Message.swift | 8 ++++---- Tests/AblyChatTests/ChatAPITests.swift | 6 +++--- Tests/AblyChatTests/MessageSubscriptionTests.swift | 2 +- Tests/AblyChatTests/MessageTests.swift | 8 ++++---- Tests/AblyChatTests/Mocks/MockHTTPPaginatedResponse.swift | 4 ++-- 9 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Example/AblyChatExample/Mocks/Misc.swift b/Example/AblyChatExample/Mocks/Misc.swift index 9a0c2587..790b4e19 100644 --- a/Example/AblyChatExample/Mocks/Misc.swift +++ b/Example/AblyChatExample/Mocks/Misc.swift @@ -12,7 +12,7 @@ final class MockMessagesPaginatedResult: PaginatedResult { Array(repeating: 0, count: numberOfMockMessages).map { _ in Message( serial: "\(Date().timeIntervalSince1970)", - latestAction: .create, + action: .create, clientID: self.clientID, roomID: self.roomID, text: MockStrings.randomPhrase(), diff --git a/Example/AblyChatExample/Mocks/MockClients.swift b/Example/AblyChatExample/Mocks/MockClients.swift index 831d7044..1a950d65 100644 --- a/Example/AblyChatExample/Mocks/MockClients.swift +++ b/Example/AblyChatExample/Mocks/MockClients.swift @@ -104,7 +104,7 @@ actor MockMessages: Messages { let subscription = MockSubscription(randomElement: { Message( serial: "\(Date().timeIntervalSince1970)", - latestAction: .create, + action: .create, clientID: MockStrings.names.randomElement()!, roomID: self.roomID, text: MockStrings.randomPhrase(), @@ -130,7 +130,7 @@ actor MockMessages: Messages { func send(params: SendMessageParams) async throws -> Message { let message = Message( serial: "\(Date().timeIntervalSince1970)", - latestAction: .create, + action: .create, clientID: clientID, roomID: roomID, text: params.text, diff --git a/Sources/AblyChat/ChatAPI.swift b/Sources/AblyChat/ChatAPI.swift index 3792d0f8..9ea9978d 100644 --- a/Sources/AblyChat/ChatAPI.swift +++ b/Sources/AblyChat/ChatAPI.swift @@ -46,7 +46,7 @@ internal final class ChatAPI: Sendable { let message = Message( serial: response.serial, - latestAction: .create, + action: .create, clientID: clientId, roomID: roomId, text: params.text, diff --git a/Sources/AblyChat/DefaultMessages.swift b/Sources/AblyChat/DefaultMessages.swift index 5c2fea1b..03339058 100644 --- a/Sources/AblyChat/DefaultMessages.swift +++ b/Sources/AblyChat/DefaultMessages.swift @@ -83,7 +83,7 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities { let message = Message( serial: serial, - latestAction: action, + action: action, clientID: clientID, roomID: self.roomID, text: text, diff --git a/Sources/AblyChat/Message.swift b/Sources/AblyChat/Message.swift index a9053b7c..b781bad5 100644 --- a/Sources/AblyChat/Message.swift +++ b/Sources/AblyChat/Message.swift @@ -9,7 +9,7 @@ public struct Message: Sendable, Codable, Identifiable, Equatable { public var id: String { serial } public var serial: String - public var latestAction: MessageAction + public var action: MessageAction public var clientID: String public var roomID: String public var text: String @@ -17,9 +17,9 @@ public struct Message: Sendable, Codable, Identifiable, Equatable { public var metadata: MessageMetadata public var headers: MessageHeaders - public init(serial: String, latestAction: MessageAction, clientID: String, roomID: String, text: String, createdAt: Date?, metadata: MessageMetadata, headers: MessageHeaders) { + public init(serial: String, action: MessageAction, clientID: String, roomID: String, text: String, createdAt: Date?, metadata: MessageMetadata, headers: MessageHeaders) { self.serial = serial - self.latestAction = latestAction + self.action = action self.clientID = clientID self.roomID = roomID self.text = text @@ -30,7 +30,7 @@ public struct Message: Sendable, Codable, Identifiable, Equatable { internal enum CodingKeys: String, CodingKey { case serial - case latestAction + case action case clientID = "clientId" case roomID = "roomId" case text diff --git a/Tests/AblyChatTests/ChatAPITests.swift b/Tests/AblyChatTests/ChatAPITests.swift index 8564ed6f..e160d3ff 100644 --- a/Tests/AblyChatTests/ChatAPITests.swift +++ b/Tests/AblyChatTests/ChatAPITests.swift @@ -41,7 +41,7 @@ struct ChatAPITests { // Then let expectedMessage = Message( serial: "3446456", - latestAction: .create, + action: .create, clientID: "mockClientId", roomID: roomId, text: "hello", @@ -91,7 +91,7 @@ struct ChatAPITests { items: [ Message( serial: "3446456", - latestAction: .create, + action: .create, clientID: "random", roomID: roomId, text: "hello", @@ -101,7 +101,7 @@ struct ChatAPITests { ), Message( serial: "3446457", - latestAction: .create, + action: .create, clientID: "random", roomID: roomId, text: "hello response", diff --git a/Tests/AblyChatTests/MessageSubscriptionTests.swift b/Tests/AblyChatTests/MessageSubscriptionTests.swift index 7bbea980..c6d27ade 100644 --- a/Tests/AblyChatTests/MessageSubscriptionTests.swift +++ b/Tests/AblyChatTests/MessageSubscriptionTests.swift @@ -26,7 +26,7 @@ private final class MockPaginatedResult: PaginatedResult { struct MessageSubscriptionTests { let messages = ["First", "Second"].map { text in - Message(serial: "", latestAction: .create, clientID: "", roomID: "", text: text, createdAt: .init(), metadata: [:], headers: [:]) + Message(serial: "", action: .create, clientID: "", roomID: "", text: text, createdAt: .init(), metadata: [:], headers: [:]) } @Test diff --git a/Tests/AblyChatTests/MessageTests.swift b/Tests/AblyChatTests/MessageTests.swift index 115fdbd1..e085d72a 100644 --- a/Tests/AblyChatTests/MessageTests.swift +++ b/Tests/AblyChatTests/MessageTests.swift @@ -4,7 +4,7 @@ import Testing struct MessageTests { let earlierMessage = Message( serial: "ABC123@1631840000000-5:2", - latestAction: .create, + action: .create, clientID: "testClientID", roomID: "roomId", text: "hello", @@ -15,7 +15,7 @@ struct MessageTests { let laterMessage = Message( serial: "ABC123@1631840000001-5:2", - latestAction: .create, + action: .create, clientID: "testClientID", roomID: "roomId", text: "hello", @@ -26,7 +26,7 @@ struct MessageTests { let invalidMessage = Message( serial: "invalid", - latestAction: .create, + action: .create, clientID: "testClientID", roomID: "roomId", text: "hello", @@ -70,7 +70,7 @@ struct MessageTests { func isEqual_whenMessageIsEqual_ReturnsTrue() async throws { let duplicateOfEarlierMessage = Message( serial: "ABC123@1631840000000-5:2", - latestAction: .create, + action: .create, clientID: "random", roomID: "", text: "", diff --git a/Tests/AblyChatTests/Mocks/MockHTTPPaginatedResponse.swift b/Tests/AblyChatTests/Mocks/MockHTTPPaginatedResponse.swift index e31e2086..43587ae7 100644 --- a/Tests/AblyChatTests/Mocks/MockHTTPPaginatedResponse.swift +++ b/Tests/AblyChatTests/Mocks/MockHTTPPaginatedResponse.swift @@ -99,7 +99,7 @@ extension MockHTTPPaginatedResponse { [ "clientId": "random", "serial": "3446456", - "latestAction": "message.create", + "action": "message.create", "createdAt": 1_730_943_049_269, "roomId": "basketball::$chat::$chatMessages", "text": "hello", @@ -109,7 +109,7 @@ extension MockHTTPPaginatedResponse { [ "clientId": "random", "serial": "3446457", - "latestAction": "message.create", + "action": "message.create", "roomId": "basketball::$chat::$chatMessages", "text": "hello response", "metadata": [:], From 74cbbf408b8894e5a09f9eb50272e5a20f9c2ade Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 4 Dec 2024 12:10:07 -0300 Subject: [PATCH 2/4] =?UTF-8?q?Introduce=20a=20wait=20to=20work=20around?= =?UTF-8?q?=20history=20not=20being=20=E2=80=9Clive=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves #167. --- Tests/AblyChatTests/IntegrationTests.swift | 31 +++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/Tests/AblyChatTests/IntegrationTests.swift b/Tests/AblyChatTests/IntegrationTests.swift index abbfa2af..fa5b03d2 100644 --- a/Tests/AblyChatTests/IntegrationTests.swift +++ b/Tests/AblyChatTests/IntegrationTests.swift @@ -116,7 +116,36 @@ struct IntegrationTests { #expect(rxMessageFromSubscription == txMessageAfterRxSubscribe) // (7) Fetch historical messages from before subscribing, and check we get txMessageBeforeRxSubscribe - let rxMessagesBeforeSubscribing = try await rxMessageSubscription.getPreviousMessages(params: .init()) + + /* + TODO: This line should just be + + let messages = try await rxMessageSubscription.getPreviousMessages(params: .init()) + + but sometimes `messages.items` is coming back empty. Andy said in + https://ably-real-time.slack.com/archives/C03JDBVM5MY/p1733220395208909 + that + + > new materialised history system doesn’t currently support “live” + > history (realtime implementation detail) - so we’re approximating the + > behaviour + + and indicated that the right workaround for now is to introduce a + wait. So we retry the fetching of history until we get a non-empty + result. + + Revert this (https://github.com/ably/ably-chat-swift/issues/175) once it’s fixed in Realtime. + */ + let rxMessagesBeforeSubscribing = try await { + while true { + let messages = try await rxMessageSubscription.getPreviousMessages(params: .init()) + if !messages.items.isEmpty { + return messages + } + // Wait 1 second before retrying the history fetch + try await Task.sleep(nanoseconds: NSEC_PER_SEC) + } + }() try #require(rxMessagesBeforeSubscribing.items.count == 1) #expect(rxMessagesBeforeSubscribing.items[0] == txMessageBeforeRxSubscribe) From a8e63f2222251335a6cef2fa09249306e9754007 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 4 Dec 2024 14:15:09 -0300 Subject: [PATCH 3/4] Fix occupancy integration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is intermittently failing with > ✘ Test basicIntegrationTest() recorded an issue at IntegrationTests.swift:162:9: Expectation failed: (rxOccupancyEventFromSubscription.presenceMembers → 0) == 1 This is presumably because the arbitrary 2 second wait isn’t enough. So, instead, wait until the subscription gives us the presence count that we’re expecting. Resolves #149. --- Tests/AblyChatTests/IntegrationTests.swift | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Tests/AblyChatTests/IntegrationTests.swift b/Tests/AblyChatTests/IntegrationTests.swift index fa5b03d2..d801025c 100644 --- a/Tests/AblyChatTests/IntegrationTests.swift +++ b/Tests/AblyChatTests/IntegrationTests.swift @@ -178,18 +178,15 @@ struct IntegrationTests { // (4) Enter presence on the other client and check that we receive the updated occupancy on the subscription try await txRoom.presence.enter(data: nil) - // It can take a moment for the occupancy to update from the clients entering presence above, so we’ll wait 2 seconds here. - try await Task.sleep(nanoseconds: 2_000_000_000) + // (5) Check that we received an updated presence count on the subscription + _ = try #require(await rxOccupancySubscription.first { occupancyEvent in + occupancyEvent.presenceMembers == 1 // 1 for txClient entering presence + }) - // (5) Check that we received an updated presence count when getting the occupancy + // (6) Check that we received an updated presence count when getting the occupancy let updatedCurrentOccupancy = try await rxRoom.occupancy.get() #expect(updatedCurrentOccupancy.presenceMembers == 1) // 1 for txClient entering presence - // (6) Check that we received an updated presence count on the subscription - let rxOccupancyEventFromSubscription = try #require(await rxOccupancySubscription.first { _ in true }) - - #expect(rxOccupancyEventFromSubscription.presenceMembers == 1) // 1 for txClient entering presence - try await txRoom.presence.leave(data: nil) // It can take a moment for the occupancy to update from the clients leaving presence above, so we’ll wait 2 seconds here. Important for the occupancy tests below. From 80fe95be59bce67d37dbb60ded0a1756feb3170a Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 4 Dec 2024 14:26:39 -0300 Subject: [PATCH 4/4] Add integration test for occupancy after presence leave Noticed that this seemed to be half-done (the comment next to the `Task.sleep` suggested there was meant to be an occupancy test to follow the `leave`). --- Tests/AblyChatTests/IntegrationTests.swift | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Tests/AblyChatTests/IntegrationTests.swift b/Tests/AblyChatTests/IntegrationTests.swift index d801025c..a0104399 100644 --- a/Tests/AblyChatTests/IntegrationTests.swift +++ b/Tests/AblyChatTests/IntegrationTests.swift @@ -184,13 +184,20 @@ struct IntegrationTests { }) // (6) Check that we received an updated presence count when getting the occupancy - let updatedCurrentOccupancy = try await rxRoom.occupancy.get() - #expect(updatedCurrentOccupancy.presenceMembers == 1) // 1 for txClient entering presence + let rxOccupancyAfterTxEnter = try await rxRoom.occupancy.get() + #expect(rxOccupancyAfterTxEnter.presenceMembers == 1) // 1 for txClient entering presence + // (7) Leave presence on the other client and check that we receive the updated occupancy on the subscription try await txRoom.presence.leave(data: nil) - // It can take a moment for the occupancy to update from the clients leaving presence above, so we’ll wait 2 seconds here. Important for the occupancy tests below. - try await Task.sleep(nanoseconds: 2_000_000_000) + // (8) Check that we received an updated presence count on the subscription + _ = try #require(await rxOccupancySubscription.first { occupancyEvent in + occupancyEvent.presenceMembers == 0 // 0 for txClient leaving presence + }) + + // (9) Check that we received an updated presence count when getting the occupancy + let rxOccupancyAfterTxLeave = try await rxRoom.occupancy.get() + #expect(rxOccupancyAfterTxLeave.presenceMembers == 0) // 0 for txClient leaving presence // MARK: - Presence