From 287477964bea7cf4fbc0c32f5ab8f20b208289b2 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 14 Apr 2024 15:05:31 +0200 Subject: [PATCH 01/40] Implement RTP18a (start of a new sync sequence and any previous in-flight sync is discarded). --- Source/ARTProtocolMessage.m | 20 ++++++++ Source/ARTRealtimePresence.m | 50 ++++++++++--------- .../Ably/ARTProtocolMessage+Private.h | 4 ++ 3 files changed, 51 insertions(+), 23 deletions(-) diff --git a/Source/ARTProtocolMessage.m b/Source/ARTProtocolMessage.m index e36d0bb1c..c667e9d85 100644 --- a/Source/ARTProtocolMessage.m +++ b/Source/ARTProtocolMessage.m @@ -35,6 +35,26 @@ - (NSString *)getConnectionKey { return _connectionKey; } +- (NSString *)getSyncIdentifierAtIndex:(NSInteger)index { + if (!_channelSerial || [_channelSerial isEqualToString:@""] || index > 1) { + return @""; + } + NSArray *a = [_channelSerial componentsSeparatedByString:@":"]; + return a.count > 1 ? a[index] : @""; +} + +- (NSString *)getSyncSequenceId { + return [self getSyncIdentifierAtIndex:0]; +} + +- (NSString *)getSyncCursor { + return [self getSyncIdentifierAtIndex:1]; +} + +- (BOOL)isEndOfSync { + return [[self getSyncCursor] isEqualToString:@""]; +} + - (NSString *)description { NSMutableString *description = [NSMutableString stringWithFormat:@"<%@: %p> {\n", self.class, self]; [description appendFormat:@" count: %d,\n", self.count]; diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 7d47bd006..7e6f4081c 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -183,6 +183,10 @@ @implementation ARTRealtimePresenceInternal { NSMutableDictionary *_internalMembers; // RTP17h NSMutableDictionary *_beforeSyncMembers; // RTP19 + + // RTP18a + NSString *_syncSequenceId; + NSMutableDictionary *_membersBackup; } - (instancetype)initWithChannel:(ARTRealtimeChannelInternal *)channel logger:(ARTInternalLog *)logger { @@ -690,21 +694,6 @@ - (void)broadcast:(ARTPresenceMessage *)pm { [_eventEmitter emit:[ARTEvent newWithPresenceAction:pm.action] with:pm]; } -/* - * Checks that a channelSerial is the final serial in a sequence of sync messages, - * by checking that there is nothing after the colon - RTP18b, RTP18c - */ -- (bool)isLastChannelSerial:(NSString *)channelSerial { - if (!channelSerial || [channelSerial isEqualToString:@""]) { - return true; - } - NSArray *a = [channelSerial componentsSeparatedByString:@":"]; - if (a.count > 1 && ![[a objectAtIndex:1] isEqualToString:@""]) { - return false; - } - return true; -} - - (void)onAttached:(ARTProtocolMessage *)message { [self startSync]; if (!message.hasPresence) { @@ -747,18 +736,31 @@ - (void)onMessage:(ARTProtocolMessage *)message { } } +- (BOOL)shoudRestartSyncWithSequenceId:(NSString *)sequenceId { + return _syncSequenceId && sequenceId && ![_syncSequenceId isEqualToString:sequenceId]; +} + +- (void)discardSync { + if (self.syncInProgress) { + _members = [_membersBackup mutableCopy]; + ARTLogDebug(_logger, @"%p PresenceMap sync with sequence id = %@ was discarded", self, _syncSequenceId); + } +} + - (void)onSync:(ARTProtocolMessage *)message { - if (!self.syncInProgress) { + NSString *sequenceId = [message getSyncSequenceId]; + if (!self.syncInProgress || [self shoudRestartSyncWithSequenceId:sequenceId]) { + [self discardSync]; // RTP18a + _syncSequenceId = sequenceId; [self startSync]; } else { - ARTLogDebug(self.logger, @"RT:%p C:%p (%@) PresenceMap sync is in progress", _realtime, _channel, _channel.name); + ARTLogDebug(self.logger, @"RT:%p C:%p (%@) PresenceMap sync is in progress (syncSequenceId = %@)", _realtime, _channel, _channel.name, _syncSequenceId); } [self onMessage:message]; - // TODO: RTP18a (previous in-flight sync should be discarded) - if ([self isLastChannelSerial:message.channelSerial]) { // RTP18b, RTP18c + if (message.isEndOfSync) { // RTP18b, RTP18c [self endSync]; ARTLogDebug(self.logger, @"RT:%p C:%p (%@) PresenceMap sync ended", _realtime, _channel, _channel.name); } @@ -941,22 +943,24 @@ - (void)reset { } - (void)startSync { - ARTLogDebug(_logger, @"%p PresenceMap sync started", self); + ARTLogDebug(_logger, @"%p PresenceMap sync started with sequence id = %@", self, _syncSequenceId); _beforeSyncMembers = [_members mutableCopy]; + _membersBackup = [_members mutableCopy]; _syncState = ARTPresenceSyncStarted; [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:_syncState] with:nil]; } - (void)endSync { - ARTLogVerbose(_logger, @"%p PresenceMap sync ending", self); + ARTLogVerbose(_logger, @"%p PresenceMap sync ending with sequence id = %@", self, _syncSequenceId); [self cleanUpAbsentMembers]; [self leaveMembersNotPresentInSync]; _syncState = ARTPresenceSyncEnded; _beforeSyncMembers = nil; - + _membersBackup = nil; [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:ARTPresenceSyncEnded] with:[_members allValues]]; [_syncEventEmitter off]; - ARTLogDebug(_logger, @"%p PresenceMap sync ended", self); + ARTLogDebug(_logger, @"%p PresenceMap sync with sequence id = %@ is ended", self, _syncSequenceId); + _syncSequenceId = nil; } - (void)failsSync:(ARTErrorInfo *)error { diff --git a/Source/PrivateHeaders/Ably/ARTProtocolMessage+Private.h b/Source/PrivateHeaders/Ably/ARTProtocolMessage+Private.h index 0722be468..f6b03ed96 100644 --- a/Source/PrivateHeaders/Ably/ARTProtocolMessage+Private.h +++ b/Source/PrivateHeaders/Ably/ARTProtocolMessage+Private.h @@ -24,6 +24,10 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)mergeFrom:(ARTProtocolMessage *)msg; +- (NSString *)getSyncSequenceId; +- (NSString *)getSyncCursor; +- (BOOL)isEndOfSync; + @end NS_ASSUME_NONNULL_END From 666906b25b816b539c46cbc152e35df857123afd Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 19 Apr 2024 16:48:00 +0200 Subject: [PATCH 02/40] Fixed `shoudRestartSyncForSequenceId` method name and updated test for RTP18a. --- Source/ARTRealtimePresence.m | 12 +- Test/Tests/RealtimeClientPresenceTests.swift | 116 +++++++++++-------- 2 files changed, 75 insertions(+), 53 deletions(-) diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 7e6f4081c..ea2908a54 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -736,20 +736,20 @@ - (void)onMessage:(ARTProtocolMessage *)message { } } -- (BOOL)shoudRestartSyncWithSequenceId:(NSString *)sequenceId { +- (BOOL)shoudRestartSyncForSequenceId:(NSString *)sequenceId { return _syncSequenceId && sequenceId && ![_syncSequenceId isEqualToString:sequenceId]; } - (void)discardSync { if (self.syncInProgress) { _members = [_membersBackup mutableCopy]; - ARTLogDebug(_logger, @"%p PresenceMap sync with sequence id = %@ was discarded", self, _syncSequenceId); + ARTLogDebug(_logger, @"%p PresenceMap sync with syncSequenceId = %@ was discarded", self, _syncSequenceId); } } - (void)onSync:(ARTProtocolMessage *)message { NSString *sequenceId = [message getSyncSequenceId]; - if (!self.syncInProgress || [self shoudRestartSyncWithSequenceId:sequenceId]) { + if (!self.syncInProgress || [self shoudRestartSyncForSequenceId:sequenceId]) { [self discardSync]; // RTP18a _syncSequenceId = sequenceId; [self startSync]; @@ -943,7 +943,7 @@ - (void)reset { } - (void)startSync { - ARTLogDebug(_logger, @"%p PresenceMap sync started with sequence id = %@", self, _syncSequenceId); + ARTLogDebug(_logger, @"%p PresenceMap sync started with syncSequenceId = %@", self, _syncSequenceId); _beforeSyncMembers = [_members mutableCopy]; _membersBackup = [_members mutableCopy]; _syncState = ARTPresenceSyncStarted; @@ -951,7 +951,7 @@ - (void)startSync { } - (void)endSync { - ARTLogVerbose(_logger, @"%p PresenceMap sync ending with sequence id = %@", self, _syncSequenceId); + ARTLogVerbose(_logger, @"%p PresenceMap sync ending with syncSequenceId = %@", self, _syncSequenceId); [self cleanUpAbsentMembers]; [self leaveMembersNotPresentInSync]; _syncState = ARTPresenceSyncEnded; @@ -959,7 +959,7 @@ - (void)endSync { _membersBackup = nil; [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:ARTPresenceSyncEnded] with:[_members allValues]]; [_syncEventEmitter off]; - ARTLogDebug(_logger, @"%p PresenceMap sync with sequence id = %@ is ended", self, _syncSequenceId); + ARTLogDebug(_logger, @"%p PresenceMap sync with syncSequenceId = %@ is ended", self, _syncSequenceId); _syncSequenceId = nil; } diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index d259446c2..e54d3ca3d 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -168,9 +168,8 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP18 - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP18a, RTP18b - func skipped__test__011__Presence__realtime_system_reserves_the_right_to_initiate_a_sync_of_the_presence_members_at_any_point_once_a_channel_is_attached__should_do_a_new_sync_whenever_a_SYNC_ProtocolMessage_is_received_with_a_channel_attribute_and_a_new_sync_sequence_identifier_in_the_channelSerial_attribute() throws { + func test__011__Presence__realtime_system_reserves_the_right_to_initiate_a_sync_of_the_presence_members_at_any_point_once_a_channel_is_attached__should_do_a_new_sync_whenever_a_SYNC_ProtocolMessage_is_received_with_a_channel_attribute_and_a_new_sync_sequence_identifier_in_the_channelSerial_attribute() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let client = AblyTests.newRealtime(options).client @@ -187,58 +186,81 @@ class RealtimeClientPresenceTests: XCTestCase { guard let transport = client.internal.transport as? TestProxyTransport else { fail("TestProxyTransport is not set"); return } + + // Add some initial members (user1, user2) + let _ = AblyTests.addMembersSequentiallyToChannel(channelName, members: 2, options: options) + + // Before starting artificial SYNC process we should wait for the initial one completed: + expect(channel.internal.presence.syncInProgress).toEventually(beFalse(), timeout: testTimeout) + XCTAssertEqual(channel.internal.presence.members.compactMap { $0.value.clientId }.sorted(), ["user1", "user2"]) + + // Inject a SYNC Presence message (first page) + let sync1Message = ARTProtocolMessage() + sync1Message.action = .sync + sync1Message.channel = channel.name + sync1Message.channelSerial = "sequenceid1:cursor1" + sync1Message.timestamp = Date() + sync1Message.presence = [ + ARTPresenceMessage(clientId: "a", action: .present, connectionId: "another", id: "another:0:0"), + ARTPresenceMessage(clientId: "b", action: .present, connectionId: "another", id: "another:0:1"), + ARTPresenceMessage(clientId: "c", action: .present, connectionId: "another", id: "another:0:2") + ] + transport.receive(sync1Message) - XCTAssertFalse(channel.internal.presence.syncInProgress) - expect(channel.internal.presence.members).to(beEmpty()) - - waitUntil(timeout: testTimeout) { done in - channel.presence.subscribe(.present) { msg in - if msg.clientId != "a" { - return - } - XCTAssertFalse(channel.presence.syncComplete) - var aClientHasLeft = false - channel.presence.subscribe(.leave) { _ in - if aClientHasLeft { - return - } - aClientHasLeft = true - done() - } - } + XCTAssertEqual( + channel.internal.presence.members.compactMap { $0.value.clientId }.sorted(), ["user1", "user2", "a", "b", "c"].sorted() + ) - // Inject a SYNC Presence message (first page) - let sync1Message = ARTProtocolMessage() - sync1Message.action = .sync - sync1Message.channel = channel.name - sync1Message.channelSerial = "sequenceid:cursor" - sync1Message.timestamp = Date() - sync1Message.presence = [ - ARTPresenceMessage(clientId: "a", action: .present, connectionId: "another", id: "another:0:0"), - ARTPresenceMessage(clientId: "b", action: .present, connectionId: "another", id: "another:0:1"), - ] - transport.receive(sync1Message) - - // Inject a SYNC Presence message (last page) - let sync2Message = ARTProtocolMessage() - sync2Message.action = .sync - sync2Message.channel = channel.name - sync2Message.channelSerial = "sequenceid:" // indicates SYNC is complete - sync2Message.timestamp = Date() - sync2Message.presence = [ - ARTPresenceMessage(clientId: "a", action: .leave, connectionId: "another", id: "another:1:0"), - ] - delay(0.5) { - transport.receive(sync2Message) - } + // Inject a SYNC Presence message (last page) + let sync2Message = ARTProtocolMessage() + sync2Message.action = .sync + sync2Message.channel = channel.name + sync2Message.channelSerial = "sequenceid1:cursor2" + sync2Message.timestamp = Date() + sync2Message.presence = [ + ARTPresenceMessage(clientId: "b", action: .leave, connectionId: "another", id: "another:1:1"), + ] + delay(0.1) { + transport.receive(sync2Message) } - - XCTAssertTrue(channel.presence.syncComplete) + + let sync3Message = ARTProtocolMessage() + sync3Message.action = .sync + sync3Message.channel = channel.name + sync3Message.channelSerial = "sequenceid2:cursor1" // sequence id is different, should discard 'sequenceid1' SYNC (RTP18a) + sync3Message.timestamp = Date() + sync3Message.presence = [ + ARTPresenceMessage(clientId: "a", action: .present, connectionId: "another", id: "another:2:0"), + ARTPresenceMessage(clientId: "b", action: .present, connectionId: "another", id: "another:2:1"), + ] + delay(0.2) { + transport.receive(sync3Message) + XCTAssertEqual( + channel.internal.presence.members.compactMap { $0.value.clientId }.sorted(), ["user1", "user2", "a", "b"].sorted() + ) + } + + let sync4Message = ARTProtocolMessage() + sync4Message.action = .sync + sync4Message.channel = channel.name + sync4Message.channelSerial = "sequenceid2:" // indicates end of SYNC + sync4Message.timestamp = Date() + sync4Message.presence = [ + ARTPresenceMessage(clientId: "a", action: .leave, connectionId: "another", id: "another:3:0"), + ] + delay(0.3) { + transport.receive(sync4Message) + // At this point initial members were removed (user1, user2), first sync discarded, "a" has left, so we have only "b" presented: + XCTAssertEqual(channel.internal.presence.members.compactMap { $0.value.clientId }, ["b"]) + } + + expect(channel.internal.presence.syncInProgress).toEventually(beFalse(), timeout: testTimeout) + waitUntil(timeout: testTimeout) { done in channel.presence.get { members, error in XCTAssertNil(error) guard let members = members, members.count == 1 else { - fail("Should at least have 1 member"); done(); return + fail("Should have 1 member"); done(); return } XCTAssertEqual(members[0].clientId, "b") done() From 342d57af9f5aa37594ea47758f10a163de6b17f8 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 22 Apr 2024 00:19:45 +0200 Subject: [PATCH 03/40] Test has no sense, because RTP1 says "ProtocolMessage may contain a HAS_PRESENCE bit flag indicating that it will perform a presence sync". It doesn't say that without members it will omit HAS_PRESENCE flag. --- Test/Tests/RealtimeClientPresenceTests.swift | 23 -------------------- 1 file changed, 23 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index e54d3ca3d..fe86c8eaa 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -105,29 +105,6 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP1 - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 - func skipped__test__009__Presence__ProtocolMessage_bit_flag__when_no_members_are_present() throws { - let test = Test() - let options = try AblyTests.commonAppSetup(for: test) - options.autoConnect = false - options.testOptions.transportFactory = TestProxyTransportFactory() - let client = ARTRealtime(options: options) - client.connect() - defer { client.dispose(); client.close() } - let channel = client.channels.get(test.uniqueChannelName()) - channel.attach() - - expect(channel.state).toEventually(equal(ARTRealtimeChannelState.attached), timeout: testTimeout) - - let transport = client.internal.transport as! TestProxyTransport - let attached = transport.protocolMessagesReceived.filter { $0.action == .attached }[0] - - XCTAssertEqual(attached.flags & 0x1, 0) - XCTAssertFalse(attached.hasPresence) - XCTAssertFalse(channel.presence.syncComplete) - XCTAssertFalse(channel.internal.presence.syncComplete) - } - func test__FLAKY__010__Presence__ProtocolMessage_bit_flag__when_members_are_present() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) From ee89bc502568f0965861316dd4da254ee0d0fd5f Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 22 Apr 2024 00:24:01 +0200 Subject: [PATCH 04/40] Improved commentary. --- Test/Tests/RealtimeClientPresenceTests.swift | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index fe86c8eaa..4e6afe6a3 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -188,7 +188,7 @@ class RealtimeClientPresenceTests: XCTestCase { channel.internal.presence.members.compactMap { $0.value.clientId }.sorted(), ["user1", "user2", "a", "b", "c"].sorted() ) - // Inject a SYNC Presence message (last page) + // Inject a SYNC Presence message (second page) let sync2Message = ARTProtocolMessage() sync2Message.action = .sync sync2Message.channel = channel.name @@ -201,10 +201,11 @@ class RealtimeClientPresenceTests: XCTestCase { transport.receive(sync2Message) } + // Inject another SYNC Presence message with a different sequence id to discard previous SYNC (RTP18a) let sync3Message = ARTProtocolMessage() sync3Message.action = .sync sync3Message.channel = channel.name - sync3Message.channelSerial = "sequenceid2:cursor1" // sequence id is different, should discard 'sequenceid1' SYNC (RTP18a) + sync3Message.channelSerial = "sequenceid2:cursor1" sync3Message.timestamp = Date() sync3Message.presence = [ ARTPresenceMessage(clientId: "a", action: .present, connectionId: "another", id: "another:2:0"), @@ -217,6 +218,7 @@ class RealtimeClientPresenceTests: XCTestCase { ) } + // Inject end of SYNC let sync4Message = ARTProtocolMessage() sync4Message.action = .sync sync4Message.channel = channel.name @@ -227,7 +229,7 @@ class RealtimeClientPresenceTests: XCTestCase { ] delay(0.3) { transport.receive(sync4Message) - // At this point initial members were removed (user1, user2), first sync discarded, "a" has left, so we have only "b" presented: + // At this point initial members were removed as not presented in sync (user1, user2), first sync discarded, "a" has left, so we have only "b" presented: XCTAssertEqual(channel.internal.presence.members.compactMap { $0.value.clientId }, ["b"]) } From ec8df5c259f1187298f39fc2de630b3aee68bbe8 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 22 Apr 2024 00:26:57 +0200 Subject: [PATCH 05/40] Fixed RTP18c test. --- Test/Tests/RealtimeClientPresenceTests.swift | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 4e6afe6a3..1528b0d1b 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -247,9 +247,8 @@ class RealtimeClientPresenceTests: XCTestCase { } } - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP18c, RTP18b - func skipped__test__012__Presence__realtime_system_reserves_the_right_to_initiate_a_sync_of_the_presence_members_at_any_point_once_a_channel_is_attached__when_a_SYNC_is_sent_with_no_channelSerial_attribute_then_the_sync_data_is_entirely_contained_within_that_ProtocolMessage() throws { + func test__012__Presence__realtime_system_reserves_the_right_to_initiate_a_sync_of_the_presence_members_at_any_point_once_a_channel_is_attached__when_a_SYNC_is_sent_with_no_channelSerial_attribute_then_the_sync_data_is_entirely_contained_within_that_ProtocolMessage() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let client = AblyTests.newRealtime(options).client @@ -267,7 +266,7 @@ class RealtimeClientPresenceTests: XCTestCase { fail("TestProxyTransport is not set"); return } - XCTAssertFalse(channel.internal.presence.syncInProgress) + expect(channel.internal.presence.syncInProgress).toEventually(beFalse(), timeout: testTimeout) expect(channel.internal.presence.members).to(beEmpty()) waitUntil(timeout: testTimeout) { done in From 150070f03a129285202d18c1f5bade7ebe1ff48f Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 22 Apr 2024 00:34:17 +0200 Subject: [PATCH 06/40] Fix test RTP19a - ensure incoming ATTACH message has no HAS_PRESENCE flag instead of requiring its absence (see RTP1). --- Test/Tests/RealtimeClientPresenceTests.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 1528b0d1b..cdba65dd2 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -368,9 +368,8 @@ class RealtimeClientPresenceTests: XCTestCase { } } - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP19a - func skipped__test__014__Presence__PresenceMap_has_existing_members_when_a_SYNC_is_started__should_emit_a_LEAVE_event_for_each_existing_member_if_the_PresenceMap_has_existing_members_when_an_ATTACHED_message_is_received_without_a_HAS_PRESENCE_flag() throws { + func test__014__Presence__PresenceMap_has_existing_members_when_a_SYNC_is_started__should_emit_a_LEAVE_event_for_each_existing_member_if_the_PresenceMap_has_existing_members_when_an_ATTACHED_message_is_received_without_a_HAS_PRESENCE_flag() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let client = AblyTests.newRealtime(options).client @@ -388,9 +387,10 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(4, done: done) - transport.setListenerAfterProcessingIncomingMessage { protocolMessage in + transport.setListenerBeforeProcessingIncomingMessage { protocolMessage in if protocolMessage.action == .attached { - XCTAssertFalse(protocolMessage.hasPresence) + // Ensure incoming ATTACH message has no HAS_PRESENCE flag + protocolMessage.flags = 0 partialDone() } } From 3011e008ad675c78b68c239fbe3b5340ca39ee61 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 26 Apr 2024 14:48:47 +0200 Subject: [PATCH 07/40] Fixed RTP2a test. --- Test/Tests/RealtimeClientPresenceTests.swift | 43 ++++++++++---------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index cdba65dd2..af13b7f0f 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1477,11 +1477,10 @@ class RealtimeClientPresenceTests: XCTestCase { } } - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP2 // RTP2a - func skipped__test__045__Presence__PresenceMap__all_incoming_presence_messages_must_be_compared_for_newness_with_the_matching_member_already_in_the_PresenceMap() throws { + func test__045__Presence__PresenceMap__all_incoming_presence_messages_must_be_compared_for_newness_with_the_matching_member_already_in_the_PresenceMap() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let client = ARTRealtime(options: options) @@ -1497,38 +1496,40 @@ class RealtimeClientPresenceTests: XCTestCase { channel.presence.unsubscribe() partialDone() } - channel.presence.enterClient("tester", data: nil) { error in + channel.presence.enterClient("tester", data: "existing") { error in XCTAssertNil(error) partialDone() } } - guard let intialPresenceMessage = channel.internal.presence.members["\(channel.internal.connectionId):tester"] else { - fail("Missing Presence message"); return - } - - XCTAssertEqual(intialPresenceMessage.memberKey(), "\(client.connection.id!):tester") - - var compareForNewnessMethodCalls = 0 - let hook = channel.internal.presence.testSuite_injectIntoMethod(after: #selector(ARTRealtimePresenceInternal.member(_:isNewerThan:))) { - compareForNewnessMethodCalls += 1 + var presence1: ARTPresenceMessage! + var presence2: ARTPresenceMessage! + + let selector = #selector(ARTRealtimePresenceInternal.member(_:isNewerThan:)) + + let hook = channel.internal.presence.testSuite_injectIntoMethod(after: selector) { + channel.internal.presence.testSuite_getArgument(from: selector, at: 1) { arg in + presence1 = arg as? ARTPresenceMessage + } + channel.internal.presence.testSuite_getArgument(from: selector, at: 0) { arg in + presence2 = arg as? ARTPresenceMessage + } } waitUntil(timeout: testTimeout) { done in - channel.presence.enterClient("tester", data: nil) { error in + channel.presence.enterClient("tester", data: "new") { error in XCTAssertNil(error) done() } } - guard let updatedPresenceMessage = channel.internal.presence.members["\(channel.internal.connectionId):tester"] else { - fail("Missing Presence message"); return - } - - XCTAssertEqual(intialPresenceMessage.memberKey(), updatedPresenceMessage.memberKey()) - expect(intialPresenceMessage.timestamp).to(beLessThan(updatedPresenceMessage.timestamp)) - - XCTAssertEqual(compareForNewnessMethodCalls, 1) + XCTAssertEqual(presence1.clientId, "tester") + XCTAssertEqual(presence2.clientId, "tester") + + XCTAssertEqual(presence1.data as! String, "existing") + XCTAssertEqual(presence2.data as! String, "new") + + expect(presence1.timestamp).to(beLessThan(presence2.timestamp)) hook.remove() } From 243c074831037f3d20154d45f43e71d0643b7110 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 26 Apr 2024 14:49:07 +0200 Subject: [PATCH 08/40] Fixed RTP2g test. --- Test/Tests/RealtimeClientPresenceTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index af13b7f0f..0ed61be22 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1996,11 +1996,11 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) - channel.presence.enterClient("tester", data: nil) { error in - XCTAssertNil(error) + channel.presence.subscribe(.enter) { _ in partialDone() } - channel.presence.subscribe(.enter) { _ in + channel.presence.enterClient("tester", data: nil) { error in + XCTAssertNil(error) partialDone() } } From b1320046a3131e6774e41071757caaa98abe5601 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 26 Apr 2024 15:43:05 +0200 Subject: [PATCH 09/40] Fixed RTP2c test. --- Test/Tests/RealtimeClientPresenceTests.swift | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 0ed61be22..6ee98e034 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1693,12 +1693,10 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP2c - - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 - func skipped__test__054__Presence__PresenceMap__all_presence_messages_from_a_SYNC_must_also_be_compared_for_newness_in_the_same_way_as_they_would_from_a_PRESENCE__discard_members_where_messages_have_arrived_before_the_SYNC() throws { + func test__054__Presence__PresenceMap__all_presence_messages_from_a_SYNC_must_also_be_compared_for_newness_in_the_same_way_as_they_would_from_a_PRESENCE__discard_members_where_messages_have_arrived_before_the_SYNC() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) - let timeBeforeSync = NSDate() + let timeBeforeSync = Date() let channelName = test.uniqueChannelName() var clientMembers: ARTRealtime? defer { clientMembers?.dispose(); clientMembers?.close() } @@ -1725,12 +1723,9 @@ class RealtimeClientPresenceTests: XCTestCase { let partialDone = AblyTests.splitDone(3, done: done) transport.setBeforeIncomingMessageModifier { protocolMessage in if protocolMessage.action == .sync { - let injectLeave = ARTPresenceMessage() - injectLeave.action = .leave - injectLeave.connectionId = membersConnectionId - injectLeave.clientId = "user110" - injectLeave.timestamp = timeBeforeSync as Date - protocolMessage.presence?.append(injectLeave) + protocolMessage.presence?.append( + ARTPresenceMessage(clientId: "user110", action: .leave, connectionId: membersConnectionId, id: "\(membersConnectionId):109:0", timestamp: timeBeforeSync) + ) transport.setBeforeIncomingMessageModifier(nil) partialDone() } @@ -1739,7 +1734,7 @@ class RealtimeClientPresenceTests: XCTestCase { channel.internal.presence.testSuite_injectIntoMethod(after: #selector(ARTRealtimePresenceInternal.endSync)) { XCTAssertFalse(channel.internal.presence.syncInProgress) XCTAssertEqual(channel.internal.presence.members.count, 120) - XCTAssertEqual(channel.internal.presence.members.filter { _, presence in presence.clientId == "user110" && presence.action == .present }.count, 1) + XCTAssertEqual(channel.internal.presence.members.filter { _, presence in presence.clientId == "user110" && presence.action == .present }.count, 1) // LEAVE for user110 is ignored, because it's timestamped before SYNC partialDone() } channel.attach { error in From 52c0a895352037e4517e8b19260821a34378b9fe Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 26 Apr 2024 16:35:23 +0200 Subject: [PATCH 10/40] Removed unnecessary test for RTP2c (added additional check to the first RTP2c test). Also reduced number of members to decrease test duration which was also unnecessary long. --- Test/Tests/RealtimeClientPresenceTests.swift | 74 ++++---------------- 1 file changed, 13 insertions(+), 61 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 6ee98e034..8a383fa85 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1700,7 +1700,7 @@ class RealtimeClientPresenceTests: XCTestCase { let channelName = test.uniqueChannelName() var clientMembers: ARTRealtime? defer { clientMembers?.dispose(); clientMembers?.close() } - clientMembers = AblyTests.addMembersSequentiallyToChannel(channelName, members: 120, options: options) + clientMembers = AblyTests.addMembersSequentiallyToChannel(channelName, members: 20, options: options) guard let membersConnectionId = clientMembers?.connection.id else { fail("Members client isn't connected"); return @@ -1715,70 +1715,21 @@ class RealtimeClientPresenceTests: XCTestCase { } channel.presence.subscribe(.leave) { leave in - XCTAssertEqual(leave.clientId, "user110") - fail("Should not fire Leave event for member `user110` because it's out of date") + if leave.clientId == "user10" { + fail("Should not fire Leave event for member `user10` because it's out of date") + } else { + XCTAssertEqual(leave.clientId, "user12") + } } waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(3, done: done) transport.setBeforeIncomingMessageModifier { protocolMessage in if protocolMessage.action == .sync { - protocolMessage.presence?.append( - ARTPresenceMessage(clientId: "user110", action: .leave, connectionId: membersConnectionId, id: "\(membersConnectionId):109:0", timestamp: timeBeforeSync) - ) - transport.setBeforeIncomingMessageModifier(nil) - partialDone() - } - return protocolMessage - } - channel.internal.presence.testSuite_injectIntoMethod(after: #selector(ARTRealtimePresenceInternal.endSync)) { - XCTAssertFalse(channel.internal.presence.syncInProgress) - XCTAssertEqual(channel.internal.presence.members.count, 120) - XCTAssertEqual(channel.internal.presence.members.filter { _, presence in presence.clientId == "user110" && presence.action == .present }.count, 1) // LEAVE for user110 is ignored, because it's timestamped before SYNC - partialDone() - } - channel.attach { error in - XCTAssertNil(error) - partialDone() - } - } - } - - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 - func skipped__test__055__Presence__PresenceMap__all_presence_messages_from_a_SYNC_must_also_be_compared_for_newness_in_the_same_way_as_they_would_from_a_PRESENCE__accept_members_where_message_have_arrived_after_the_SYNC() throws { - let test = Test() - let options = try AblyTests.commonAppSetup(for: test) - let channelName = test.uniqueChannelName() - var clientMembers: ARTRealtime? - defer { clientMembers?.dispose(); clientMembers?.close() } - clientMembers = AblyTests.addMembersSequentiallyToChannel(channelName, members: 120, options: options) - - guard let membersConnectionId = clientMembers?.connection.id else { - fail("Members client isn't connected"); return - } - - let client = AblyTests.newRealtime(options).client - defer { client.dispose(); client.close() } - let channel = client.channels.get(channelName) - - guard let transport = client.internal.transport as? TestProxyTransport else { - fail("TestProxyTransport is not set"); return - } - - waitUntil(timeout: testTimeout.multiplied(by: 2)) { done in - let partialDone = AblyTests.splitDone(4, done: done) - channel.presence.subscribe(.leave) { leave in - XCTAssertEqual(leave.clientId, "user110") - partialDone() - } - transport.setBeforeIncomingMessageModifier { protocolMessage in - if protocolMessage.action == .sync { - let injectLeave = ARTPresenceMessage() - injectLeave.action = .leave - injectLeave.connectionId = membersConnectionId - injectLeave.clientId = "user110" - injectLeave.timestamp = Date() + 1 - protocolMessage.presence?.append(injectLeave) + protocolMessage.presence?.append(contentsOf: [ + ARTPresenceMessage(clientId: "user10", action: .leave, connectionId: membersConnectionId, id: "synthesized:9:0", timestamp: timeBeforeSync), + ARTPresenceMessage(clientId: "user12", action: .leave, connectionId: membersConnectionId, id: "synthesized:11:0", timestamp: Date() + 1) + ]) transport.setBeforeIncomingMessageModifier(nil) partialDone() } @@ -1786,8 +1737,9 @@ class RealtimeClientPresenceTests: XCTestCase { } channel.internal.presence.testSuite_injectIntoMethod(after: #selector(ARTRealtimePresenceInternal.endSync)) { XCTAssertFalse(channel.internal.presence.syncInProgress) - XCTAssertEqual(channel.internal.presence.members.count, 119) - expect(channel.internal.presence.members.filter { _, presence in presence.clientId == "user110" }).to(beEmpty()) + XCTAssertEqual(channel.internal.presence.members.count, 19) + XCTAssertEqual(channel.internal.presence.members.filter { _, presence in presence.clientId == "user10" && presence.action == .present }.count, 1) // LEAVE for user10 is ignored, because it's timestamped before SYNC + XCTAssertEqual(channel.internal.presence.members.filter { _, presence in presence.clientId == "user12" && presence.action == .present }.count, 0) // LEAVE for user12 is not ignored, because it's timestamped after SYNC partialDone() } channel.attach { error in From 123283131c43fd1c9ceebb9f8a6d83e44dbb7cc8 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 26 Apr 2024 20:15:00 +0200 Subject: [PATCH 11/40] More fixes for RTP2a test: subscribe to ENTER only, since realtime sends PRESENT there as well. Compare connections of two messages. --- Test/Tests/RealtimeClientPresenceTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 8a383fa85..ecd5a0ade 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1490,9 +1490,8 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) - channel.presence.subscribe { presence in + channel.presence.subscribe(.enter) { presence in XCTAssertEqual(presence.clientId, "tester") - XCTAssertEqual(presence.action, .enter) channel.presence.unsubscribe() partialDone() } @@ -1525,6 +1524,7 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(presence1.clientId, "tester") XCTAssertEqual(presence2.clientId, "tester") + XCTAssertEqual(presence1.connectionId, presence2.connectionId) XCTAssertEqual(presence1.data as! String, "existing") XCTAssertEqual(presence2.data as! String, "new") From 02381fb98021f18c5cd511ca1eff96291fc0d478 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 26 Apr 2024 20:17:04 +0200 Subject: [PATCH 12/40] Fix for RTP2d test - update only after enter is done. --- Test/Tests/RealtimeClientPresenceTests.swift | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index ecd5a0ade..41a3f4c59 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1784,17 +1784,16 @@ class RealtimeClientPresenceTests: XCTestCase { let channel = client.channels.get(channelName) waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(3, done: done) + let partialDone = AblyTests.splitDone(2, done: done) channel.presence.subscribe(.update) { _ in partialDone() } channel.presence.enterClient("tester", data: nil) { error in XCTAssertNil(error) - partialDone() - } - channel.presence.updateClient("tester", data: nil) { error in - XCTAssertNil(error) - partialDone() + channel.presence.updateClient("tester", data: nil) { error in + XCTAssertNil(error) + partialDone() + } } } From dd4c7b54cdc8dabf0062d8e737b06fb02a1c9aab Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 26 Apr 2024 21:41:53 +0200 Subject: [PATCH 13/40] Removed unnecessary test for RTP2g (which is the same as for RTP2d for ENTER event). --- Test/Tests/RealtimeClientPresenceTests.swift | 27 +------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 41a3f4c59..2f96b1fbe 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1749,31 +1749,6 @@ class RealtimeClientPresenceTests: XCTestCase { } } - // RTP2d - func skipped__test__046__Presence__PresenceMap__if_action_of_ENTER_arrives__it_should_be_added_to_the_presence_map_with_the_action_set_to_PRESENT() throws { - let test = Test() - let options = try AblyTests.commonAppSetup(for: test) - let client = ARTRealtime(options: options) - defer { client.dispose(); client.close() } - let channelName = test.uniqueChannelName() - let channel = client.channels.get(channelName) - - waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(2, done: done) - channel.presence.subscribe(.enter) { _ in - partialDone() - } - channel.presence.enterClient("tester", data: nil) { error in - XCTAssertNil(error) - partialDone() - } - } - - XCTAssertEqual(channel.internal.presence.members.filter { _, presence in presence.action == .present }.count, 1) - expect(channel.internal.presence.members.filter { _, presence in presence.action == .enter }).to(beEmpty()) - } - - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP2d func test__FLAKY__047__Presence__PresenceMap__if_action_of_UPDATE_arrives__it_should_be_added_to_the_presence_map_with_the_action_set_to_PRESENT() throws { let test = Test() @@ -1931,7 +1906,7 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(channel.internal.presence.members.count, 20) } - // RTP2g + // RTP2d (ENTER), RTP2g func test__FLAKY__051__Presence__PresenceMap__any_incoming_presence_message_that_passes_the_newness_check_should_be_emitted_on_the_Presence_object__with_an_event_name_set_to_its_original_action() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) From e084f105f2471e954496ae607f593ce6a557f26f Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 28 Apr 2024 15:27:00 +0200 Subject: [PATCH 14/40] Fix for RTP17 test - sometimes realtime sends PRESENT before ENTER and test fails. --- Test/Tests/RealtimeClientPresenceTests.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 2f96b1fbe..6dbae6773 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -2510,10 +2510,9 @@ class RealtimeClientPresenceTests: XCTestCase { } } - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP17 - func skipped__test__080__Presence__private_and_internal_PresenceMap_containing_only_members_that_match_the_current_connectionId__any_ENTER__PRESENT__UPDATE_or_LEAVE_event_that_matches_the_current_connectionId_should_be_applied_to_this_object() throws { + func test__080__Presence__private_and_internal_PresenceMap_containing_only_members_that_match_the_current_connectionId__any_ENTER__PRESENT__UPDATE_or_LEAVE_event_that_matches_the_current_connectionId_should_be_applied_to_this_object() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let channelName = test.uniqueChannelName() @@ -2535,7 +2534,7 @@ class RealtimeClientPresenceTests: XCTestCase { guard let currentConnectionId = clientA.connection.id else { fail("ClientA should be connected"); partialDone(); return } - XCTAssertEqual(presence.action, ARTPresenceAction.enter) + XCTAssertTrue(presence.action == ARTPresenceAction.enter || presence.action == ARTPresenceAction.present) XCTAssertEqual(presence.connectionId, currentConnectionId) XCTAssertEqual(channelA.internal.presence.members.count, 1) XCTAssertEqual(channelA.internal.presence.internalMembers.count, 1) @@ -2546,7 +2545,7 @@ class RealtimeClientPresenceTests: XCTestCase { guard let currentConnectionId = clientB.connection.id else { fail("ClientB should be connected"); partialDone(); return } - expect(presence.action).to(equal(ARTPresenceAction.enter) || equal(ARTPresenceAction.present)) + XCTAssertTrue(presence.action == ARTPresenceAction.enter || presence.action == ARTPresenceAction.present) XCTAssertNotEqual(presence.connectionId, currentConnectionId) XCTAssertEqual(channelB.internal.presence.members.count, 1) XCTAssertEqual(channelB.internal.presence.internalMembers.count, 0) From 86e6ba28c5b210c8c03b74ff17256870da9ec0b1 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 28 Apr 2024 23:12:17 +0200 Subject: [PATCH 15/40] Added forgotten dispose() call which unsubscribes channels. Without this clients remain in memory and interacts with later executing tests sometimes failing them, like here for example - https://test-observability.herokuapp.com/repos/ably/ably-cocoa/test_cases/8eae564d-2dab-409c-a49d-65dddcd5b422?branches%5B%5D=fix%2F1848-refactoring-presence-looped Test 038 doesn't contain any "shouldn't be called" assertions. --- Test/Tests/RealtimeClientPresenceTests.swift | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 6dbae6773..a2e3f16dd 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -431,7 +431,7 @@ class RealtimeClientPresenceTests: XCTestCase { clientSource = AblyTests.addMembersSequentiallyToChannel(channelName, members: 250, options: options) let clientTarget = ARTRealtime(options: options) - defer { clientTarget.close() } + defer { clientTarget.dispose(); clientTarget.close() } let channel = clientTarget.channels.get(channelName) waitUntil(timeout: testTimeout) { done in @@ -880,13 +880,13 @@ class RealtimeClientPresenceTests: XCTestCase { options.clientId = "john" let client1 = ARTRealtime(options: options) - defer { client1.close() } + defer { client1.dispose(); client1.close() } let channelName = test.uniqueChannelName() let channel1 = client1.channels.get(channelName) let client2 = ARTRealtime(options: options) - defer { client2.close() } + defer { client2.dispose(); client2.close() } let channel2 = client2.channels.get(channelName) waitUntil(timeout: testTimeout) { done in @@ -1035,13 +1035,13 @@ class RealtimeClientPresenceTests: XCTestCase { options.clientId = "john" let client1 = ARTRealtime(options: options) - defer { client1.close() } + defer { client1.dispose(); client1.close() } let channelName = test.uniqueChannelName() let channel1 = client1.channels.get(channelName) let client2 = ARTRealtime(options: options) - defer { client2.close() } + defer { client2.dispose(); client2.close() } let channel2 = client2.channels.get(channelName) waitUntil(timeout: testTimeout) { done in @@ -1064,13 +1064,13 @@ class RealtimeClientPresenceTests: XCTestCase { options.clientId = "john" let client1 = ARTRealtime(options: options) - defer { client1.close() } + defer { client1.dispose(); client1.close() } let channelName = test.uniqueChannelName() let channel1 = client1.channels.get(channelName) let client2 = ARTRealtime(options: options) - defer { client2.close() } + defer { client2.dispose(); client2.close() } let channel2 = client2.channels.get(channelName) waitUntil(timeout: testTimeout) { done in From 0f12e3e9806eb2eb3e88305c12f43b83474b5e7a Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 28 Apr 2024 23:31:40 +0200 Subject: [PATCH 16/40] Removed broken FIXME link. --- Test/Tests/RealtimeClientPresenceTests.swift | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index a2e3f16dd..85ff6af2d 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -649,7 +649,6 @@ class RealtimeClientPresenceTests: XCTestCase { } } - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP5b func skipped__test__017__Presence__Channel_state_change_side_effects__if_a_channel_enters_the_ATTACHED_state_then_all_queued_presence_messages_will_be_sent_immediately_and_a_presence_SYNC_may_be_initiated() throws { let test = Test() @@ -1027,7 +1026,6 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP8 - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP8b func test__FLAKY__030__Presence__enter__optionally_a_callback_can_be_provided_that_is_called_for_success() throws { let test = Test() @@ -1056,7 +1054,6 @@ class RealtimeClientPresenceTests: XCTestCase { } } - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP8b func skipped__test__031__Presence__enter__optionally_a_callback_can_be_provided_that_is_called_for_failure() throws { let test = Test() @@ -1291,7 +1288,6 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP9 - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP9b func skipped__test__039__Presence__update__should_enter_current_client_into_the_channel_if_the_client_was_not_already_entered() throws { let test = Test() @@ -3280,7 +3276,6 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertTrue(ARTRealtimePresenceQuery().waitForSync) } - // FIXME: Fix flaky presence tests and re-enable. See https://ably-real-time.slack.com/archives/C030C5YLY/p1623172436085700 // RTP11a func test__FLAKY__100__Presence__get__should_return_a_list_of_current_members_on_the_channel() throws { let test = Test() From 065583c05d5fcc44ccfbfadb250703bf494f6701 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 28 Apr 2024 23:36:12 +0200 Subject: [PATCH 17/40] Removed unnecessary startSync() call. --- Test/Test Utilities/TestUtilities.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Test/Test Utilities/TestUtilities.swift b/Test/Test Utilities/TestUtilities.swift index 2349a2368..09bb0ddac 100644 --- a/Test/Test Utilities/TestUtilities.swift +++ b/Test/Test Utilities/TestUtilities.swift @@ -1709,7 +1709,6 @@ extension ARTRealtime { guard let transport = self.internal.transport as? TestProxyTransport else { fail("TestProxyTransport is not set"); return } - channel.internal.presence.startSync() transport.send(syncMessage) } } From 9ca4d6be13d665a3ead70c5343039c9bb78cd6ee Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 28 Apr 2024 23:39:11 +0200 Subject: [PATCH 18/40] Unskip tests that don't fail locally while being executed repeatedly (marked them FLAKY). --- Test/Tests/RealtimeClientPresenceTests.swift | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 85ff6af2d..ee306cb1e 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -650,7 +650,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP5b - func skipped__test__017__Presence__Channel_state_change_side_effects__if_a_channel_enters_the_ATTACHED_state_then_all_queued_presence_messages_will_be_sent_immediately_and_a_presence_SYNC_may_be_initiated() throws { + func test__FLAKY__017__Presence__Channel_state_change_side_effects__if_a_channel_enters_the_ATTACHED_state_then_all_queued_presence_messages_will_be_sent_immediately_and_a_presence_SYNC_may_be_initiated() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let client1 = AblyTests.newRealtime(options).client @@ -873,7 +873,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP8 // RTP8a - func skipped__test__024__Presence__enter__should_enter_the_current_client__optionally_with_the_data_provided() throws { + func test__FLAKY__024__Presence__enter__should_enter_the_current_client__optionally_with_the_data_provided() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1055,7 +1055,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP8b - func skipped__test__031__Presence__enter__optionally_a_callback_can_be_provided_that_is_called_for_failure() throws { + func test__FLAKY__031__Presence__enter__optionally_a_callback_can_be_provided_that_is_called_for_failure() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1261,7 +1261,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP9a - func skipped__test__038__Presence__update__should_update_the_data_for_the_present_member_with_null() throws { + func test__FLAKY__038__Presence__update__should_update_the_data_for_the_present_member_with_null() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1289,7 +1289,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP9 // RTP9b - func skipped__test__039__Presence__update__should_enter_current_client_into_the_channel_if_the_client_was_not_already_entered() throws { + func test__FLAKY__039__Presence__update__should_enter_current_client_into_the_channel_if_the_client_was_not_already_entered() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1384,7 +1384,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP10 // RTP10a - func skipped__test__043__Presence__leave__should_leave_the_current_client_from_the_channel_and_the_data_will_be_updated_with_the_value_provided() throws { + func test__FLAKY__043__Presence__leave__should_leave_the_current_client_from_the_channel_and_the_data_will_be_updated_with_the_value_provided() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -3508,7 +3508,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP11c // RTP11c1 - func skipped__test__110__Presence__get__Query__set_of_params___waitForSync_is_true__should_wait_until_SYNC_is_complete_before_returning_a_list_of_members() throws { + func test__FLAKY__110__Presence__get__Query__set_of_params___waitForSync_is_true__should_wait_until_SYNC_is_complete_before_returning_a_list_of_members() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) var clientSecondary: ARTRealtime! @@ -3856,7 +3856,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP13 - func skipped__test__008__Presence__Presence_syncComplete_returns_true_if_the_initial_SYNC_operation_has_completed() throws { + func test__FLAKY__008__Presence__Presence_syncComplete_returns_true_if_the_initial_SYNC_operation_has_completed() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) @@ -3893,7 +3893,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP14 // RTP14a, RTP14b, RTP14c, RTP14d - func skipped__test__116__Presence__enterClient__enters_into_presence_on_a_channel_on_behalf_of_another_clientId() throws { + func test__FLAKY__116__Presence__enterClient__enters_into_presence_on_a_channel_on_behalf_of_another_clientId() throws { let test = Test() let client = ARTRealtime(options: try AblyTests.commonAppSetup(for: test)) defer { client.dispose(); client.close() } From 1998a8456ce3ac432423b36f443e61e12ab42361 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 30 Apr 2024 00:33:36 +0200 Subject: [PATCH 19/40] Fixed RTP19 test (removed leave.clientId == internalMember.clientId assertion, bc realtime sometimes send LEAVE for other members too, not sure why it does that). --- Test/Tests/RealtimeClientPresenceTests.swift | 22 +++++++++----------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index ee306cb1e..29f0d29a7 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -331,27 +331,27 @@ class RealtimeClientPresenceTests: XCTestCase { } XCTAssertEqual(channel.internal.presence.members.count, 2) - // Inject a local member - let internalMember = ARTPresenceMessage(clientId: NSUUID().uuidString, action: .enter, connectionId: "another", id: "another:0:0") + // Inject a internal member + let internalMember = ARTPresenceMessage(clientId: "internal-member", action: .enter, connectionId: channel.internal.connectionId, id: "\(channel.internal.connectionId):0:0") channel.internal.presence.processMember(internalMember) XCTAssertEqual(channel.internal.presence.members.count, 3) + XCTAssertEqual(channel.internal.presence.internalMembers.count, 1) XCTAssertEqual(channel.internal.presence.members.filter { memberKey, _ in memberKey.contains(internalMember.clientId!) }.count, 1) waitUntil(timeout: testTimeout) { done in channel.presence.get { members, error in XCTAssertNil(error) - guard let members = members, members.count == 3 else { - fail("Should at least have 3 members"); done(); return - } - XCTAssertEqual(members.filter { $0.clientId == internalMember.clientId }.count, 1) + XCTAssertEqual(members?.count, 3) + XCTAssertEqual(members?.filter { $0.clientId == internalMember.clientId }.count, 1) done() } } waitUntil(timeout: testTimeout) { done in channel.presence.subscribe(.leave) { leave in - XCTAssertEqual(leave.clientId, internalMember.clientId) - done() + if leave.clientId == internalMember.clientId { + done() + } } client.requestPresenceSyncForChannel(channel) } @@ -359,10 +359,8 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in channel.presence.get { members, error in XCTAssertNil(error) - guard let members = members, members.count == 2 else { - fail("Should at least have 2 members"); done(); return - } - expect(members.filter { $0.clientId == internalMember.clientId }).to(beEmpty()) + XCTAssertEqual(members?.count, 2) + expect(members?.filter { $0.clientId == internalMember.clientId }).to(beEmpty()) done() } } From f169e1bbcd483e444222ea9b0efcbb57dddd6d58 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 30 Apr 2024 00:48:17 +0200 Subject: [PATCH 20/40] Added forgotten resetting setListenerBeforeProcessingIncomingMessage call for transport. Might cause tests interfere with each other, like here f.e. - https://test-observability.herokuapp.com/repos/ably/ably-cocoa/test_cases/7a5e904a-faaa-4a62-b4a7-01fdee993289?branches%5B%5D=fix%2F1889-unskip-presense-tests-looped Line reported #3884 is from completely different test! (test__FLAKY__008__Presence__Presence_syncComplete_returns_true_if_the_initial_SYNC_operation_has_completed which also fails and have setListenerBeforeProcessingIncomingMessage in use) --- Test/Tests/RealtimeClientPresenceTests.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 29f0d29a7..33fa3030b 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -408,6 +408,7 @@ class RealtimeClientPresenceTests: XCTestCase { partialDone() } } + transport.setListenerBeforeProcessingIncomingMessage(nil) waitUntil(timeout: testTimeout) { done in channel.presence.get { members, error in @@ -3522,10 +3523,12 @@ class RealtimeClientPresenceTests: XCTestCase { let query = ARTRealtimePresenceQuery() XCTAssertTrue(query.waitForSync) + var transport = client.internal.transport as! TestProxyTransport + waitUntil(timeout: testTimeout) { done in channel.attach { error in XCTAssertNil(error) - let transport = client.internal.transport as! TestProxyTransport + transport = client.internal.transport as! TestProxyTransport transport.setListenerBeforeProcessingIncomingMessage { protocolMessage in if protocolMessage.action == .sync { XCTAssertEqual(protocolMessage.presence!.count, 100) @@ -3543,6 +3546,7 @@ class RealtimeClientPresenceTests: XCTestCase { } } } + transport.setListenerBeforeProcessingIncomingMessage(nil) } // RTP11c1 @@ -3883,7 +3887,7 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertFalse(channel.presence.internal.syncComplete_nosync()) } } - + transport.setListenerBeforeProcessingIncomingMessage(nil) expect(channel.presence.syncComplete).toEventually(beTrue(), timeout: testTimeout) XCTAssertEqual(transport.protocolMessagesReceived.filter { $0.action == .sync }.count, 3) } From 85de5f51242220e80160d50403ddf4c8fe4c74e8 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 30 Apr 2024 17:42:32 +0200 Subject: [PATCH 21/40] Realtime sends PRESENT among ENTER every other time and it fails test because decodeNumberOfCalls == 2 in such case. --- Test/Tests/RealtimeClientPresenceTests.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 33fa3030b..e4dae860e 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -3904,15 +3904,15 @@ class RealtimeClientPresenceTests: XCTestCase { let expectedData = ["test": 1] - var encodeNumberOfCalls = 0 + var encodeWasCalled = false let hookEncode = channel.internal.dataEncoder.testSuite_injectIntoMethod(after: #selector(ARTDataEncoder.encode(_:))) { - encodeNumberOfCalls += 1 + encodeWasCalled = true } defer { hookEncode.remove() } - var decodeNumberOfCalls = 0 + var decodeWasCalled = false let hookDecode = channel.internal.dataEncoder.testSuite_injectIntoMethod(after: #selector(ARTDataEncoder.decode(_:encoding:))) { - decodeNumberOfCalls += 1 + decodeWasCalled = true } defer { hookDecode.remove() } @@ -3940,8 +3940,8 @@ class RealtimeClientPresenceTests: XCTestCase { } } - XCTAssertEqual(encodeNumberOfCalls, 1) - XCTAssertEqual(decodeNumberOfCalls, 1) + XCTAssertTrue(encodeWasCalled) + XCTAssertTrue(decodeWasCalled) } // RTP14d From f73ae68c39823bfaee013fa521fe5c67dcc6690b Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 30 Apr 2024 19:06:21 +0200 Subject: [PATCH 22/40] Removed FLAKY label since some of these tests seems to be failing not more than other tests and some not failing at all. --- Test/Tests/RealtimeClientPresenceTests.swift | 40 ++++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index e4dae860e..4c9f22ebb 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -105,7 +105,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP1 - func test__FLAKY__010__Presence__ProtocolMessage_bit_flag__when_members_are_present() throws { + func test__010__Presence__ProtocolMessage_bit_flag__when_members_are_present() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) @@ -307,7 +307,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP19 - func test__FLAKY__013__Presence__PresenceMap_has_existing_members_when_a_SYNC_is_started__should_ensure_that_members_no_longer_present_on_the_channel_are_removed_from_the_local_PresenceMap_once_the_sync_is_complete() throws { + func test__013__Presence__PresenceMap_has_existing_members_when_a_SYNC_is_started__should_ensure_that_members_no_longer_present_on_the_channel_are_removed_from_the_local_PresenceMap_once_the_sync_is_complete() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let channelName = test.uniqueChannelName() @@ -420,7 +420,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP4 - func test__FLAKY__002__Presence__should_receive_all_250_members() throws { + func test__002__Presence__should_receive_all_250_members() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) var clientSource: ARTRealtime! @@ -551,7 +551,7 @@ class RealtimeClientPresenceTests: XCTestCase { } } - func test__FLAKY__019__Presence__Channel_state_change_side_effects__if_the_channel_enters_the_FAILED_state__should_clear_the_PresenceMap_including_local_members_and_does_not_emit_any_presence_events() throws { + func test__019__Presence__Channel_state_change_side_effects__if_the_channel_enters_the_FAILED_state__should_clear_the_PresenceMap_including_local_members_and_does_not_emit_any_presence_events() throws { let test = Test() let client = ARTRealtime(options: try AblyTests.commonAppSetup(for: test)) defer { client.dispose(); client.close() } @@ -649,7 +649,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP5b - func test__FLAKY__017__Presence__Channel_state_change_side_effects__if_a_channel_enters_the_ATTACHED_state_then_all_queued_presence_messages_will_be_sent_immediately_and_a_presence_SYNC_may_be_initiated() throws { + func test__017__Presence__Channel_state_change_side_effects__if_a_channel_enters_the_ATTACHED_state_then_all_queued_presence_messages_will_be_sent_immediately_and_a_presence_SYNC_may_be_initiated() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let client1 = AblyTests.newRealtime(options).client @@ -872,7 +872,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP8 // RTP8a - func test__FLAKY__024__Presence__enter__should_enter_the_current_client__optionally_with_the_data_provided() throws { + func test__024__Presence__enter__should_enter_the_current_client__optionally_with_the_data_provided() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1026,7 +1026,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP8 // RTP8b - func test__FLAKY__030__Presence__enter__optionally_a_callback_can_be_provided_that_is_called_for_success() throws { + func test__030__Presence__enter__optionally_a_callback_can_be_provided_that_is_called_for_success() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1054,7 +1054,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP8b - func test__FLAKY__031__Presence__enter__optionally_a_callback_can_be_provided_that_is_called_for_failure() throws { + func test__031__Presence__enter__optionally_a_callback_can_be_provided_that_is_called_for_failure() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1260,7 +1260,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP9a - func test__FLAKY__038__Presence__update__should_update_the_data_for_the_present_member_with_null() throws { + func test__038__Presence__update__should_update_the_data_for_the_present_member_with_null() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1288,7 +1288,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP9 // RTP9b - func test__FLAKY__039__Presence__update__should_enter_current_client_into_the_channel_if_the_client_was_not_already_entered() throws { + func test__039__Presence__update__should_enter_current_client_into_the_channel_if_the_client_was_not_already_entered() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1383,7 +1383,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP10 // RTP10a - func test__FLAKY__043__Presence__leave__should_leave_the_current_client_from_the_channel_and_the_data_will_be_updated_with_the_value_provided() throws { + func test__043__Presence__leave__should_leave_the_current_client_from_the_channel_and_the_data_will_be_updated_with_the_value_provided() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -1439,7 +1439,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP2 - func test__FLAKY__003__Presence__should_be_used_a_PresenceMap_to_maintain_a_list_of_members() throws { + func test__003__Presence__should_be_used_a_PresenceMap_to_maintain_a_list_of_members() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) var clientSecondary: ARTRealtime! @@ -1745,7 +1745,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP2d - func test__FLAKY__047__Presence__PresenceMap__if_action_of_UPDATE_arrives__it_should_be_added_to_the_presence_map_with_the_action_set_to_PRESENT() throws { + func test__047__Presence__PresenceMap__if_action_of_UPDATE_arrives__it_should_be_added_to_the_presence_map_with_the_action_set_to_PRESENT() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let client = ARTRealtime(options: options) @@ -1801,7 +1801,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP2e - func test__FLAKY__049__Presence__PresenceMap__if_a_SYNC_is_not_in_progress__then_when_a_presence_message_with_an_action_of_LEAVE_arrives__that_memberKey_should_be_deleted_from_the_presence_map__if_present() throws { + func test__049__Presence__PresenceMap__if_a_SYNC_is_not_in_progress__then_when_a_presence_message_with_an_action_of_LEAVE_arrives__that_memberKey_should_be_deleted_from_the_presence_map__if_present() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) @@ -1902,7 +1902,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP2d (ENTER), RTP2g - func test__FLAKY__051__Presence__PresenceMap__any_incoming_presence_message_that_passes_the_newness_check_should_be_emitted_on_the_Presence_object__with_an_event_name_set_to_its_original_action() throws { + func test__051__Presence__PresenceMap__any_incoming_presence_message_that_passes_the_newness_check_should_be_emitted_on_the_Presence_object__with_an_event_name_set_to_its_original_action() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let client = ARTRealtime(options: options) @@ -2639,7 +2639,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP17a - func test__FLAKY__081__Presence__private_and_internal_PresenceMap_containing_only_members_that_match_the_current_connectionId__all_members_belonging_to_the_current_connection_are_published_as_a_PresenceMessage_on_the_Channel_by_the_server_irrespective_of_whether_the_client_has_permission_to_subscribe_or_the_Channel_is_configured_to_publish_presence_events() throws { + func test__081__Presence__private_and_internal_PresenceMap_containing_only_members_that_match_the_current_connectionId__all_members_belonging_to_the_current_connection_are_published_as_a_PresenceMessage_on_the_Channel_by_the_server_irrespective_of_whether_the_client_has_permission_to_subscribe_or_the_Channel_is_configured_to_publish_presence_events() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let channelName = test.uniqueChannelName() @@ -3276,7 +3276,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP11a - func test__FLAKY__100__Presence__get__should_return_a_list_of_current_members_on_the_channel() throws { + func test__100__Presence__get__should_return_a_list_of_current_members_on_the_channel() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) @@ -3507,7 +3507,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP11c // RTP11c1 - func test__FLAKY__110__Presence__get__Query__set_of_params___waitForSync_is_true__should_wait_until_SYNC_is_complete_before_returning_a_list_of_members() throws { + func test__110__Presence__get__Query__set_of_params___waitForSync_is_true__should_wait_until_SYNC_is_complete_before_returning_a_list_of_members() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) var clientSecondary: ARTRealtime! @@ -3858,7 +3858,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP13 - func test__FLAKY__008__Presence__Presence_syncComplete_returns_true_if_the_initial_SYNC_operation_has_completed() throws { + func test__008__Presence__Presence_syncComplete_returns_true_if_the_initial_SYNC_operation_has_completed() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) @@ -3895,7 +3895,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP14 // RTP14a, RTP14b, RTP14c, RTP14d - func test__FLAKY__116__Presence__enterClient__enters_into_presence_on_a_channel_on_behalf_of_another_clientId() throws { + func test__116__Presence__enterClient__enters_into_presence_on_a_channel_on_behalf_of_another_clientId() throws { let test = Test() let client = ARTRealtime(options: try AblyTests.commonAppSetup(for: test)) defer { client.dispose(); client.close() } From 1acc1a9981b89752a570c8c23bac8b3f0ac1f081 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 30 Apr 2024 22:42:26 +0200 Subject: [PATCH 23/40] Added post-check of leave events received, since realtime might send those events for other members too. --- Test/Tests/RealtimeClientPresenceTests.swift | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 4c9f22ebb..9d8a5fcb1 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1708,13 +1708,10 @@ class RealtimeClientPresenceTests: XCTestCase { guard let transport = client.internal.transport as? TestProxyTransport else { fail("TestProxyTransport is not set"); return } - - channel.presence.subscribe(.leave) { leave in - if leave.clientId == "user10" { - fail("Should not fire Leave event for member `user10` because it's out of date") - } else { - XCTAssertEqual(leave.clientId, "user12") - } + + var leaveEvents = [ARTPresenceMessage]() + channel.presence.subscribe(.leave) { message in + leaveEvents.append(message) } waitUntil(timeout: testTimeout) { done in @@ -1742,6 +1739,8 @@ class RealtimeClientPresenceTests: XCTestCase { partialDone() } } + XCTAssertTrue(leaveEvents.contains(where: { $0.clientId == "user12" })) + XCTAssertFalse(leaveEvents.contains(where: { $0.clientId == "user10" })) } // RTP2d From c71b54630882aaf5f785ba1c9c70709ef41efb53 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 30 Apr 2024 23:28:43 +0200 Subject: [PATCH 24/40] Realtime sometimes sends 50. --- Test/Tests/RealtimeClientPresenceTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 9d8a5fcb1..4b3379d00 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -3530,7 +3530,6 @@ class RealtimeClientPresenceTests: XCTestCase { transport = client.internal.transport as! TestProxyTransport transport.setListenerBeforeProcessingIncomingMessage { protocolMessage in if protocolMessage.action == .sync { - XCTAssertEqual(protocolMessage.presence!.count, 100) channel.presence.get(query) { members, error in XCTAssertNil(error) if let members { From 3ed152d80e992e855c808b222260222475996bb3 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Wed, 1 May 2024 00:19:53 +0200 Subject: [PATCH 25/40] Spec RTP5b: "If a channel enters the ATTACHED state then all queued presence messages will be sent immediately. A presence SYNC may be initiated per RTP1". "may be initiated" doesn't mean "will be". So this statement periodically fails the test, so I'm removing it. --- Test/Tests/RealtimeClientPresenceTests.swift | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 4b3379d00..0cf5dc75d 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -691,13 +691,7 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertFalse(channel2.presence.syncComplete) XCTAssertEqual(channel2.internal.presence.members.count, 0) } - - guard let transport = client2.internal.transport as? TestProxyTransport else { - fail("Transport should be a test proxy"); return - } - - XCTAssertEqual(transport.protocolMessagesReceived.filter { $0.action == .sync }.count, 1) - + expect(channel2.presence.syncComplete).toEventually(beTrue(), timeout: testTimeout) XCTAssertEqual(channel2.internal.presence.members.count, 2) } From 30d49a75277a06e0fbc11d9f98df3e9415122b6b Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 3 May 2024 02:31:00 +0200 Subject: [PATCH 26/40] Revert "Implement RTP18a (start of a new sync sequence and any previous in-flight sync is discarded)." This reverts commits: 666906b, 2874779. --- Source/ARTProtocolMessage.m | 20 --- Source/ARTRealtimePresence.m | 50 ++++---- .../Ably/ARTProtocolMessage+Private.h | 4 - Test/Tests/RealtimeClientPresenceTests.swift | 117 +++++++----------- 4 files changed, 69 insertions(+), 122 deletions(-) diff --git a/Source/ARTProtocolMessage.m b/Source/ARTProtocolMessage.m index c667e9d85..e36d0bb1c 100644 --- a/Source/ARTProtocolMessage.m +++ b/Source/ARTProtocolMessage.m @@ -35,26 +35,6 @@ - (NSString *)getConnectionKey { return _connectionKey; } -- (NSString *)getSyncIdentifierAtIndex:(NSInteger)index { - if (!_channelSerial || [_channelSerial isEqualToString:@""] || index > 1) { - return @""; - } - NSArray *a = [_channelSerial componentsSeparatedByString:@":"]; - return a.count > 1 ? a[index] : @""; -} - -- (NSString *)getSyncSequenceId { - return [self getSyncIdentifierAtIndex:0]; -} - -- (NSString *)getSyncCursor { - return [self getSyncIdentifierAtIndex:1]; -} - -- (BOOL)isEndOfSync { - return [[self getSyncCursor] isEqualToString:@""]; -} - - (NSString *)description { NSMutableString *description = [NSMutableString stringWithFormat:@"<%@: %p> {\n", self.class, self]; [description appendFormat:@" count: %d,\n", self.count]; diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index ea2908a54..7d47bd006 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -183,10 +183,6 @@ @implementation ARTRealtimePresenceInternal { NSMutableDictionary *_internalMembers; // RTP17h NSMutableDictionary *_beforeSyncMembers; // RTP19 - - // RTP18a - NSString *_syncSequenceId; - NSMutableDictionary *_membersBackup; } - (instancetype)initWithChannel:(ARTRealtimeChannelInternal *)channel logger:(ARTInternalLog *)logger { @@ -694,6 +690,21 @@ - (void)broadcast:(ARTPresenceMessage *)pm { [_eventEmitter emit:[ARTEvent newWithPresenceAction:pm.action] with:pm]; } +/* + * Checks that a channelSerial is the final serial in a sequence of sync messages, + * by checking that there is nothing after the colon - RTP18b, RTP18c + */ +- (bool)isLastChannelSerial:(NSString *)channelSerial { + if (!channelSerial || [channelSerial isEqualToString:@""]) { + return true; + } + NSArray *a = [channelSerial componentsSeparatedByString:@":"]; + if (a.count > 1 && ![[a objectAtIndex:1] isEqualToString:@""]) { + return false; + } + return true; +} + - (void)onAttached:(ARTProtocolMessage *)message { [self startSync]; if (!message.hasPresence) { @@ -736,31 +747,18 @@ - (void)onMessage:(ARTProtocolMessage *)message { } } -- (BOOL)shoudRestartSyncForSequenceId:(NSString *)sequenceId { - return _syncSequenceId && sequenceId && ![_syncSequenceId isEqualToString:sequenceId]; -} - -- (void)discardSync { - if (self.syncInProgress) { - _members = [_membersBackup mutableCopy]; - ARTLogDebug(_logger, @"%p PresenceMap sync with syncSequenceId = %@ was discarded", self, _syncSequenceId); - } -} - - (void)onSync:(ARTProtocolMessage *)message { - NSString *sequenceId = [message getSyncSequenceId]; - if (!self.syncInProgress || [self shoudRestartSyncForSequenceId:sequenceId]) { - [self discardSync]; // RTP18a - _syncSequenceId = sequenceId; + if (!self.syncInProgress) { [self startSync]; } else { - ARTLogDebug(self.logger, @"RT:%p C:%p (%@) PresenceMap sync is in progress (syncSequenceId = %@)", _realtime, _channel, _channel.name, _syncSequenceId); + ARTLogDebug(self.logger, @"RT:%p C:%p (%@) PresenceMap sync is in progress", _realtime, _channel, _channel.name); } [self onMessage:message]; - if (message.isEndOfSync) { // RTP18b, RTP18c + // TODO: RTP18a (previous in-flight sync should be discarded) + if ([self isLastChannelSerial:message.channelSerial]) { // RTP18b, RTP18c [self endSync]; ARTLogDebug(self.logger, @"RT:%p C:%p (%@) PresenceMap sync ended", _realtime, _channel, _channel.name); } @@ -943,24 +941,22 @@ - (void)reset { } - (void)startSync { - ARTLogDebug(_logger, @"%p PresenceMap sync started with syncSequenceId = %@", self, _syncSequenceId); + ARTLogDebug(_logger, @"%p PresenceMap sync started", self); _beforeSyncMembers = [_members mutableCopy]; - _membersBackup = [_members mutableCopy]; _syncState = ARTPresenceSyncStarted; [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:_syncState] with:nil]; } - (void)endSync { - ARTLogVerbose(_logger, @"%p PresenceMap sync ending with syncSequenceId = %@", self, _syncSequenceId); + ARTLogVerbose(_logger, @"%p PresenceMap sync ending", self); [self cleanUpAbsentMembers]; [self leaveMembersNotPresentInSync]; _syncState = ARTPresenceSyncEnded; _beforeSyncMembers = nil; - _membersBackup = nil; + [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:ARTPresenceSyncEnded] with:[_members allValues]]; [_syncEventEmitter off]; - ARTLogDebug(_logger, @"%p PresenceMap sync with syncSequenceId = %@ is ended", self, _syncSequenceId); - _syncSequenceId = nil; + ARTLogDebug(_logger, @"%p PresenceMap sync ended", self); } - (void)failsSync:(ARTErrorInfo *)error { diff --git a/Source/PrivateHeaders/Ably/ARTProtocolMessage+Private.h b/Source/PrivateHeaders/Ably/ARTProtocolMessage+Private.h index f6b03ed96..0722be468 100644 --- a/Source/PrivateHeaders/Ably/ARTProtocolMessage+Private.h +++ b/Source/PrivateHeaders/Ably/ARTProtocolMessage+Private.h @@ -24,10 +24,6 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)mergeFrom:(ARTProtocolMessage *)msg; -- (NSString *)getSyncSequenceId; -- (NSString *)getSyncCursor; -- (BOOL)isEndOfSync; - @end NS_ASSUME_NONNULL_END diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 0cf5dc75d..8024b802f 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -145,7 +145,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP18 - // RTP18a, RTP18b + // RTP18b func test__011__Presence__realtime_system_reserves_the_right_to_initiate_a_sync_of_the_presence_members_at_any_point_once_a_channel_is_attached__should_do_a_new_sync_whenever_a_SYNC_ProtocolMessage_is_received_with_a_channel_attribute_and_a_new_sync_sequence_identifier_in_the_channelSerial_attribute() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) @@ -163,83 +163,58 @@ class RealtimeClientPresenceTests: XCTestCase { guard let transport = client.internal.transport as? TestProxyTransport else { fail("TestProxyTransport is not set"); return } - - // Add some initial members (user1, user2) - let _ = AblyTests.addMembersSequentiallyToChannel(channelName, members: 2, options: options) - - // Before starting artificial SYNC process we should wait for the initial one completed: - expect(channel.internal.presence.syncInProgress).toEventually(beFalse(), timeout: testTimeout) - XCTAssertEqual(channel.internal.presence.members.compactMap { $0.value.clientId }.sorted(), ["user1", "user2"]) - - // Inject a SYNC Presence message (first page) - let sync1Message = ARTProtocolMessage() - sync1Message.action = .sync - sync1Message.channel = channel.name - sync1Message.channelSerial = "sequenceid1:cursor1" - sync1Message.timestamp = Date() - sync1Message.presence = [ - ARTPresenceMessage(clientId: "a", action: .present, connectionId: "another", id: "another:0:0"), - ARTPresenceMessage(clientId: "b", action: .present, connectionId: "another", id: "another:0:1"), - ARTPresenceMessage(clientId: "c", action: .present, connectionId: "another", id: "another:0:2") - ] - transport.receive(sync1Message) - XCTAssertEqual( - channel.internal.presence.members.compactMap { $0.value.clientId }.sorted(), ["user1", "user2", "a", "b", "c"].sorted() - ) + XCTAssertFalse(channel.internal.presence.syncInProgress) + expect(channel.internal.presence.members).to(beEmpty()) - // Inject a SYNC Presence message (second page) - let sync2Message = ARTProtocolMessage() - sync2Message.action = .sync - sync2Message.channel = channel.name - sync2Message.channelSerial = "sequenceid1:cursor2" - sync2Message.timestamp = Date() - sync2Message.presence = [ - ARTPresenceMessage(clientId: "b", action: .leave, connectionId: "another", id: "another:1:1"), - ] - delay(0.1) { - transport.receive(sync2Message) - } - - // Inject another SYNC Presence message with a different sequence id to discard previous SYNC (RTP18a) - let sync3Message = ARTProtocolMessage() - sync3Message.action = .sync - sync3Message.channel = channel.name - sync3Message.channelSerial = "sequenceid2:cursor1" - sync3Message.timestamp = Date() - sync3Message.presence = [ - ARTPresenceMessage(clientId: "a", action: .present, connectionId: "another", id: "another:2:0"), - ARTPresenceMessage(clientId: "b", action: .present, connectionId: "another", id: "another:2:1"), - ] - delay(0.2) { - transport.receive(sync3Message) - XCTAssertEqual( - channel.internal.presence.members.compactMap { $0.value.clientId }.sorted(), ["user1", "user2", "a", "b"].sorted() - ) - } - - // Inject end of SYNC - let sync4Message = ARTProtocolMessage() - sync4Message.action = .sync - sync4Message.channel = channel.name - sync4Message.channelSerial = "sequenceid2:" // indicates end of SYNC - sync4Message.timestamp = Date() - sync4Message.presence = [ - ARTPresenceMessage(clientId: "a", action: .leave, connectionId: "another", id: "another:3:0"), - ] - delay(0.3) { - transport.receive(sync4Message) - // At this point initial members were removed as not presented in sync (user1, user2), first sync discarded, "a" has left, so we have only "b" presented: - XCTAssertEqual(channel.internal.presence.members.compactMap { $0.value.clientId }, ["b"]) + waitUntil(timeout: testTimeout) { done in + channel.presence.subscribe(.present) { msg in + if msg.clientId != "a" { + return + } + XCTAssertFalse(channel.presence.syncComplete) + var aClientHasLeft = false + channel.presence.subscribe(.leave) { _ in + if aClientHasLeft { + return + } + aClientHasLeft = true + done() + } + } + + // Inject a SYNC Presence message (first page) + let sync1Message = ARTProtocolMessage() + sync1Message.action = .sync + sync1Message.channel = channel.name + sync1Message.channelSerial = "sequenceid:cursor" + sync1Message.timestamp = Date() + sync1Message.presence = [ + ARTPresenceMessage(clientId: "a", action: .present, connectionId: "another", id: "another:0:0"), + ARTPresenceMessage(clientId: "b", action: .present, connectionId: "another", id: "another:0:1"), + ] + transport.receive(sync1Message) + + // Inject a SYNC Presence message (last page) + let sync2Message = ARTProtocolMessage() + sync2Message.action = .sync + sync2Message.channel = channel.name + sync2Message.channelSerial = "sequenceid:" // indicates SYNC is complete + sync2Message.timestamp = Date() + sync2Message.presence = [ + ARTPresenceMessage(clientId: "a", action: .leave, connectionId: "another", id: "another:1:0"), + ] + delay(0.5) { + transport.receive(sync2Message) + } } - - expect(channel.internal.presence.syncInProgress).toEventually(beFalse(), timeout: testTimeout) - + + XCTAssertTrue(channel.presence.syncComplete) waitUntil(timeout: testTimeout) { done in channel.presence.get { members, error in XCTAssertNil(error) guard let members = members, members.count == 1 else { - fail("Should have 1 member"); done(); return + fail("Should at least have 1 member"); done(); return } XCTAssertEqual(members[0].clientId, "b") done() From 7f7a466ec48daba951f262fcec69225b15bae914 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 3 May 2024 02:32:26 +0200 Subject: [PATCH 27/40] Fixed RTP18b test. --- Test/Tests/RealtimeClientPresenceTests.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 8024b802f..88b8d7322 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -163,8 +163,9 @@ class RealtimeClientPresenceTests: XCTestCase { guard let transport = client.internal.transport as? TestProxyTransport else { fail("TestProxyTransport is not set"); return } - - XCTAssertFalse(channel.internal.presence.syncInProgress) + + // Before starting artificial SYNC process we should wait for the initial one completed: + expect(channel.internal.presence.syncInProgress).toEventually(beFalse(), timeout: testTimeout) expect(channel.internal.presence.members).to(beEmpty()) waitUntil(timeout: testTimeout) { done in From dd75e84ab5cc89f3eba679e71b94d93f0f54317e Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 5 May 2024 17:43:52 +0200 Subject: [PATCH 28/40] Ensure connections for this test are closed and completion closure only called in case of no errors. Otherwise it affects execution of the "test_111..." below - https://test-observability.herokuapp.com/repos/ably/ably-cocoa/test_cases/ee46a8e6-e4ed-489c-83c7-648774930f92?branches%5B%5D=fix%2F1889-unskip-presense-tests-looped-4 --- Test/Tests/RealtimeClientPresenceTests.swift | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 88b8d7322..816faba9c 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -3479,14 +3479,15 @@ class RealtimeClientPresenceTests: XCTestCase { func test__110__Presence__get__Query__set_of_params___waitForSync_is_true__should_wait_until_SYNC_is_complete_before_returning_a_list_of_members() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) - var clientSecondary: ARTRealtime! - defer { clientSecondary.dispose(); clientSecondary.close() } - let channelName = test.uniqueChannelName() - clientSecondary = AblyTests.addMembersSequentiallyToChannel(channelName, members: 150, options: options) - + let clientSecondary = AblyTests.addMembersSequentiallyToChannel(channelName, members: 150, options: options) let client = AblyTests.newRealtime(options).client - defer { client.dispose(); client.close() } + defer { + clientSecondary.dispose(); clientSecondary.close() + client.dispose(); client.close() + expect(clientSecondary.connection.state).toEventually(equal(.closed), timeout: testTimeout) + expect(client.connection.state).toEventually(equal(.closed), timeout: testTimeout) + } let channel = client.channels.get(channelName) let query = ARTRealtimePresenceQuery() @@ -3504,10 +3505,10 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertNil(error) if let members { XCTAssertEqual(members.count, 150) + done() } else { XCTFail("Expected members to be non-nil") } - done() } transport.setListenerBeforeProcessingIncomingMessage(nil) } From 5460d1d2ca704737e3ca811647b3d596c427752b Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 5 May 2024 18:00:58 +0200 Subject: [PATCH 29/40] Revert fix for RTP2a since it works bad either. --- Test/Tests/RealtimeClientPresenceTests.swift | 44 ++++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 816faba9c..b709e2585 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1455,46 +1455,44 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) - channel.presence.subscribe(.enter) { presence in + channel.presence.subscribe { presence in XCTAssertEqual(presence.clientId, "tester") + XCTAssertEqual(presence.action, .enter) channel.presence.unsubscribe() partialDone() } - channel.presence.enterClient("tester", data: "existing") { error in + channel.presence.enterClient("tester", data: nil) { error in XCTAssertNil(error) partialDone() } } - var presence1: ARTPresenceMessage! - var presence2: ARTPresenceMessage! - - let selector = #selector(ARTRealtimePresenceInternal.member(_:isNewerThan:)) - - let hook = channel.internal.presence.testSuite_injectIntoMethod(after: selector) { - channel.internal.presence.testSuite_getArgument(from: selector, at: 1) { arg in - presence1 = arg as? ARTPresenceMessage - } - channel.internal.presence.testSuite_getArgument(from: selector, at: 0) { arg in - presence2 = arg as? ARTPresenceMessage - } + guard let intialPresenceMessage = channel.internal.presence.members["\(channel.internal.connectionId):tester"] else { + fail("Missing Presence message"); return + } + + XCTAssertEqual(intialPresenceMessage.memberKey(), "\(client.connection.id!):tester") + + var compareForNewnessMethodCalls = 0 + let hook = channel.internal.presence.testSuite_injectIntoMethod(after: #selector(ARTRealtimePresenceInternal.member(_:isNewerThan:))) { + compareForNewnessMethodCalls += 1 } waitUntil(timeout: testTimeout) { done in - channel.presence.enterClient("tester", data: "new") { error in + channel.presence.enterClient("tester", data: nil) { error in XCTAssertNil(error) done() } } - XCTAssertEqual(presence1.clientId, "tester") - XCTAssertEqual(presence2.clientId, "tester") - XCTAssertEqual(presence1.connectionId, presence2.connectionId) - - XCTAssertEqual(presence1.data as! String, "existing") - XCTAssertEqual(presence2.data as! String, "new") - - expect(presence1.timestamp).to(beLessThan(presence2.timestamp)) + guard let updatedPresenceMessage = channel.internal.presence.members["\(channel.internal.connectionId):tester"] else { + fail("Missing Presence message"); return + } + + XCTAssertEqual(intialPresenceMessage.memberKey(), updatedPresenceMessage.memberKey()) + expect(intialPresenceMessage.timestamp).to(beLessThan(updatedPresenceMessage.timestamp)) + + XCTAssertEqual(compareForNewnessMethodCalls, 1) hook.remove() } From 83f3d92aa386b262050caddf306bc36de59b1b5b Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 5 May 2024 18:05:22 +0200 Subject: [PATCH 30/40] New fix for RTP2a similar to 85de5f51. --- Test/Tests/RealtimeClientPresenceTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index b709e2585..26735cc0d 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1473,9 +1473,9 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(intialPresenceMessage.memberKey(), "\(client.connection.id!):tester") - var compareForNewnessMethodCalls = 0 + var compareForNewnessMethodCalled = false let hook = channel.internal.presence.testSuite_injectIntoMethod(after: #selector(ARTRealtimePresenceInternal.member(_:isNewerThan:))) { - compareForNewnessMethodCalls += 1 + compareForNewnessMethodCalled = true } waitUntil(timeout: testTimeout) { done in @@ -1492,7 +1492,7 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(intialPresenceMessage.memberKey(), updatedPresenceMessage.memberKey()) expect(intialPresenceMessage.timestamp).to(beLessThan(updatedPresenceMessage.timestamp)) - XCTAssertEqual(compareForNewnessMethodCalls, 1) + XCTAssertTrue(compareForNewnessMethodCalled) hook.remove() } From 1de053873ad8ab9dccefd024d9aeda5669d5b125 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 5 May 2024 18:35:43 +0200 Subject: [PATCH 31/40] Ensure connections for this test are closed. Otherwise it affects execution of the "test__032..." below - https://test-observability.herokuapp.com/repos/ably/ably-cocoa/test_cases/ac44f09c-f521-4bfc-a3d7-58d4f7428c74?branches%5B%5D=fix%2F1889-unskip-presense-tests-looped-4 Also reducing error delay since often callback on ENTER called faster than 0.1 (at least this may be the reason why "shouldn't be called" assertion is being fired every other time). --- Test/Tests/RealtimeClientPresenceTests.swift | 22 +++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 26735cc0d..7aab59453 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1028,17 +1028,19 @@ class RealtimeClientPresenceTests: XCTestCase { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" - - let client1 = ARTRealtime(options: options) - defer { client1.dispose(); client1.close() } - let channelName = test.uniqueChannelName() - let channel1 = client1.channels.get(channelName) - + let client1 = ARTRealtime(options: options) let client2 = ARTRealtime(options: options) - defer { client2.dispose(); client2.close() } + let channel1 = client1.channels.get(channelName) let channel2 = client2.channels.get(channelName) - + + defer { + client1.dispose(); client1.close() + client2.dispose(); client2.close() + expect(client1.connection.state).toEventually(equal(.closed), timeout: testTimeout) + expect(client2.connection.state).toEventually(equal(.closed), timeout: testTimeout) + } + waitUntil(timeout: testTimeout) { done in channel1.presence.subscribe(.enter) { _ in fail("shouldn't be called") @@ -1048,8 +1050,8 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertTrue(error === protocolError.error) done() } - delay(0.1) { - channel2.internal.onError(protocolError) + channel2.internalAsync { _internal in + _internal.onError(protocolError) } } } From 0c149c47b8349e01a5d56a9a71065d2af240b800 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 6 May 2024 00:57:21 +0200 Subject: [PATCH 32/40] Removing "test__83" either, since it's from protocol v1. Currently upon disconnect internal presence map is kept and upon re-connect no synthesized LEAVE event appears to be sent by realtime. --- Test/Tests/RealtimeClientPresenceTests.swift | 53 -------------------- 1 file changed, 53 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 7aab59453..6ffe45b16 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -2826,59 +2826,6 @@ class RealtimeClientPresenceTests: XCTestCase { expect(client1PresenceMessage.data as? String).to(equal(firstClientData)) expect(client2PresenceMessage.data as? String).to(equal(secondClientData)) } - - func skipped__test__083__Presence__private_and_internal_PresenceMap_containing_only_members_that_match_the_current_connectionId__events_applied_to_presence_map__should_be_applied_to_any_LEAVE_event_with_a_connectionId_that_matches_the_current_client_s_connectionId_and_is_not_a_synthesized() throws { - let test = Test() - let options = try AblyTests.commonAppSetup(for: test) - let client = ARTRealtime(options: options) - client.internal.shouldImmediatelyReconnect = false - defer { client.dispose(); client.close() } - - let channel = client.channels.get(test.uniqueChannelName()) - waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(2, done: done) - channel.presence.subscribe(.enter) { presence in - XCTAssertEqual(presence.clientId, "one") - channel.presence.unsubscribe() - partialDone() - } - channel.presence.enterClient("one", data: nil) { error in - XCTAssertNil(error) - partialDone() - } - } - - waitUntil(timeout: .seconds(20)) { done in - channel.internal.presence.onceSyncEnds { _ in - // Synthesized leave - expect(channel.internal.presence.internalMembers).to(beEmpty()) - done() - } - client.internal.onDisconnected() - } - - waitUntil(timeout: testTimeout) { done in - channel.presence.subscribe(.enter) { presence in - // Re-entering... - XCTAssertEqual(presence.clientId, "one") - channel.presence.unsubscribe() - done() - } - } - - waitUntil(timeout: testTimeout) { done in - channel.presence.get { presences, error in - XCTAssertNil(error) - guard let presences = presences else { - fail("Presences is nil"); done(); return - } - XCTAssertTrue(channel.internal.presence.syncComplete) - XCTAssertEqual(presences.count, 1) - expect(presences.map { $0.clientId }).to(contain(["one"])) - done() - } - } - } // RTP15d func test__004__Presence__callback_can_be_provided_that_will_be_called_upon_success() throws { From 9b77978592f99b804305dd88b24ec98eb037ef13 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 6 May 2024 19:13:51 +0200 Subject: [PATCH 33/40] Removing "test__082.." since it's purpose is unclear, it claims in the title what is already done in test 080, but does cryptic things in its body. For example the comment "// Should remove the "two" member that was added manually because the connectionId // doesn't match and it's not synthesized, it will be re-entered." doesn't make sense to me, because the member added manually shouldn't be removed. Yes, connection after re-connect is different, but spec doesn't says to remove anything (only in case of channel FAILED or DETACHED should reset both presence maps). And the next comment line says that it will be re-entered. Yes, it will be, so it should be kept then, right? I've marked test 080 as testing spec RTP17b. --- Test/Tests/RealtimeClientPresenceTests.swift | 102 +------------------ 1 file changed, 2 insertions(+), 100 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 6ffe45b16..dc72bba80 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -2476,6 +2476,7 @@ class RealtimeClientPresenceTests: XCTestCase { // RTP17 + // RTP17b func test__080__Presence__private_and_internal_PresenceMap_containing_only_members_that_match_the_current_connectionId__any_ENTER__PRESENT__UPDATE_or_LEAVE_event_that_matches_the_current_connectionId_should_be_applied_to_this_object() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) @@ -2599,7 +2600,7 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(presence.data as? String, "bye") XCTAssertEqual(presence.connectionId, currentConnectionId) XCTAssertEqual(channelB.internal.presence.members.count, 1) - XCTAssertEqual(channelB.internal.presence.internalMembers.count, 0) + XCTAssertEqual(channelB.internal.presence.internalMembers.count, 0) // was removed bc not a 'synthesized leave' (RTP17b) channelB.presence.unsubscribe() partialDone() } @@ -2644,105 +2645,6 @@ class RealtimeClientPresenceTests: XCTestCase { } } } - - // RTP17b - - func skipped__test__082__Presence__private_and_internal_PresenceMap_containing_only_members_that_match_the_current_connectionId__events_applied_to_presence_map__should_be_applied_to_ENTER__PRESENT_or_UPDATE_events_with_a_connectionId_that_matches_the_current_client_s_connectionId() throws { - let test = Test() - let options = try AblyTests.commonAppSetup(for: test) - let client = AblyTests.newRealtime(options).client - defer { client.dispose(); client.close() } - - let channel = client.channels.get(test.uniqueChannelName()) - waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(2, done: done) - channel.presence.subscribe(.enter) { presence in - XCTAssertEqual(presence.clientId, "one") - channel.presence.unsubscribe() - partialDone() - } - channel.presence.enterClient("one", data: nil) { error in - XCTAssertNil(error) - partialDone() - } - } - - guard let connectionId = client.connection.id else { - fail("connectionId is empty"); return - } - - channel.internalSync { _internal in - XCTAssertEqual(_internal.presence.internalMembers.count, 1) - } - - let additionalMember = ARTPresenceMessage( - clientId: "two", - action: .enter, - connectionId: connectionId, - id: connectionId + ":0:0" - ) - - // Inject an additional member into the myMember set, then force a suspended state - client.simulateSuspended(beforeSuspension: { done in - channel.internal.presence.internalMembers[additionalMember.clientId!] = additionalMember - done() - }) - expect(client.connection.state).toEventually(equal(.suspended), timeout: testTimeout) - - XCTAssertEqual(channel.internal.presence.internalMembers.count, 2) - - channel.internalSync { _internal in - XCTAssertEqual(_internal.presence.internalMembers.count, 2) - } - - waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(3, done: done) - - channel.once(.attached) { stateChange in - XCTAssertNil(stateChange.reason) - partialDone() - } - - // Await Sync - channel.internal.presence.onceSyncEnds { _ in - // Should remove the "two" member that was added manually because the connectionId - // doesn't match and it's not synthesized, it will be re-entered. - XCTAssertEqual(channel.internal.presence.internalMembers.count, 1) - - partialDone() - } - - channel.presence.subscribe(.enter) { presence in - XCTAssertEqual(presence.clientId, "two") - channel.presence.unsubscribe() - partialDone() - } - - // Reconnect - client.connect() - } - - // Wait for server - waitUntil(timeout: testTimeout) { done in - delay(1, closure: done) - } - - client.requestPresenceSyncForChannel(channel) - - XCTAssertFalse(channel.presence.syncComplete) - waitUntil(timeout: testTimeout) { done in - channel.presence.get { presences, error in - XCTAssertNil(error) - guard let presences = presences else { - fail("Presences is nil"); done(); return - } - XCTAssertTrue(channel.presence.syncComplete) - XCTAssertEqual(presences.count, 2) - expect(presences.map { $0.clientId }).to(contain(["one", "two"])) - done() - } - } - } // RTP17i, RTP17g func test__200__Presence__PresenceMap_should_perform_re_entry_whenever_a_channel_moves_into_the_attached_state_and_presence_message_consists_of_enter_action_with_client_id_and_data() throws { From 9dac73ef18ff5088975244ae83f9233231531ae7 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 6 May 2024 23:44:08 +0200 Subject: [PATCH 34/40] Another fix for RTP2a (ENTER/PRESENT ambiguity). --- Test/Tests/RealtimeClientPresenceTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index dc72bba80..6c9ea0b54 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1459,7 +1459,7 @@ class RealtimeClientPresenceTests: XCTestCase { let partialDone = AblyTests.splitDone(2, done: done) channel.presence.subscribe { presence in XCTAssertEqual(presence.clientId, "tester") - XCTAssertEqual(presence.action, .enter) + XCTAssertTrue(presence.action == .enter || presence.action == .present) channel.presence.unsubscribe() partialDone() } From d0ee62c1965d6f8383c23b1a947153e715b20599 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 9 May 2024 16:48:22 +0200 Subject: [PATCH 35/40] Fixed test__080... and test__050... to properly check amount of members (syncInProgress vs not syncInProgress). --- Test/Tests/RealtimeClientPresenceTests.swift | 21 ++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 6c9ea0b54..d83a099e1 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1839,8 +1839,11 @@ class RealtimeClientPresenceTests: XCTestCase { channel.presence.subscribe(.leave) { leave in XCTAssertEqual(leave.clientId, "user11") let absentMember = channel.internal.presence.members.first { _, m in m.clientId == "user11" }.map { $0.value } - XCTAssertTrue(channel.internal.presence.syncInProgress) - XCTAssertEqual(absentMember?.action, .absent) + if channel.internal.presence.syncInProgress { + XCTAssertEqual(absentMember?.action, .absent) + } else { + XCTAssertEqual(absentMember?.action, .leave) + } partialDone() } @@ -2587,7 +2590,12 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(presence.action, ARTPresenceAction.leave) XCTAssertEqual(presence.data as? String, "bye") XCTAssertNotEqual(presence.connectionId, currentConnectionId) - XCTAssertEqual(channelA.internal.presence.members.count, 1) + if channelA.internal.presence.syncInProgress { + XCTAssertEqual(channelA.internal.presence.members.filter({ $0.value.action != .present }).count, 1) + XCTAssertEqual(channelA.internal.presence.members.filter({ $0.value.action != .absent }).count, 1) + } else { + XCTAssertEqual(channelA.internal.presence.members.count, 1) + } XCTAssertEqual(channelA.internal.presence.internalMembers.count, 1) channelA.presence.unsubscribe() partialDone() @@ -2599,7 +2607,12 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(presence.action, ARTPresenceAction.leave) XCTAssertEqual(presence.data as? String, "bye") XCTAssertEqual(presence.connectionId, currentConnectionId) - XCTAssertEqual(channelB.internal.presence.members.count, 1) + if channelB.internal.presence.syncInProgress { + XCTAssertEqual(channelB.internal.presence.members.filter({ $0.value.action != .present }).count, 1) + XCTAssertEqual(channelB.internal.presence.members.filter({ $0.value.action != .absent }).count, 1) + } else { + XCTAssertEqual(channelB.internal.presence.members.count, 1) + } XCTAssertEqual(channelB.internal.presence.internalMembers.count, 0) // was removed bc not a 'synthesized leave' (RTP17b) channelB.presence.unsubscribe() partialDone() From 96eea55905c6c3454f2bd22d1f6674297540ecce Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 9 May 2024 18:16:40 +0200 Subject: [PATCH 36/40] Removed extra attach, because according to RTL4i it happens after connection goes to CONNECTED (also it's called inside members `get` too). Also removed interception of a SYNC message, since it's not needed here - we just check that sync process was started before calling members `get` and then check if it was finished upon `get` callback (at least that's what the name of the test claims should be happening). --- Test/Tests/RealtimeClientPresenceTests.swift | 34 +++++++------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index d83a099e1..8bcf064ba 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -3351,33 +3351,23 @@ class RealtimeClientPresenceTests: XCTestCase { expect(client.connection.state).toEventually(equal(.closed), timeout: testTimeout) } let channel = client.channels.get(channelName) - - let query = ARTRealtimePresenceQuery() - XCTAssertTrue(query.waitForSync) - - var transport = client.internal.transport as! TestProxyTransport - + expect(channel.internal.presence.syncInProgress).toEventually(beTrue(), timeout: testTimeout) + waitUntil(timeout: testTimeout) { done in - channel.attach { error in + let query = ARTRealtimePresenceQuery() + XCTAssertTrue(query.waitForSync) + XCTAssertEqual(channel.internal.presence.syncInProgress, true) + channel.presence.get(query) { members, error in XCTAssertNil(error) - transport = client.internal.transport as! TestProxyTransport - transport.setListenerBeforeProcessingIncomingMessage { protocolMessage in - if protocolMessage.action == .sync { - channel.presence.get(query) { members, error in - XCTAssertNil(error) - if let members { - XCTAssertEqual(members.count, 150) - done() - } else { - XCTFail("Expected members to be non-nil") - } - } - transport.setListenerBeforeProcessingIncomingMessage(nil) - } + if let members { + XCTAssertEqual(members.count, 150) + XCTAssertEqual(channel.internal.presence.syncInProgress, false) + done() + } else { + XCTFail("Expected members to be non-nil") } } } - transport.setListenerBeforeProcessingIncomingMessage(nil) } // RTP11c1 From c6fdecac31e70565130fdabd73c346b78f0bd7d9 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 10 May 2024 01:00:27 +0200 Subject: [PATCH 37/40] Fix queue to receive fake message. --- Test/Tests/RealtimeClientPresenceTests.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 8bcf064ba..bbca9b79a 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1859,7 +1859,9 @@ class RealtimeClientPresenceTests: XCTestCase { leaveMessage.presence = [ ARTPresenceMessage(clientId: "user11", action: .leave, connectionId: "another", id: "another:123:0", timestamp: Date()), ] - transport.receive(leaveMessage) + channel.internalAsync { _ in + transport.receive(leaveMessage) + } partialDone() } } From 33af3c8ab17011946e03d853200e041815c167c4 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 13 May 2024 02:11:59 +0200 Subject: [PATCH 38/40] Removed semaphore in favour of an atomic property for synchronised access. --- Source/ARTGCD.m | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/Source/ARTGCD.m b/Source/ARTGCD.m index 26a17eca9..5781699b7 100644 --- a/Source/ARTGCD.m +++ b/Source/ARTGCD.m @@ -1,8 +1,13 @@ #import "ARTGCD.h" +@interface ARTScheduledBlockHandle () + +// Mark this as `atomic` to syncronize access to it from `_scheduledBlock` and `cancel`. +@property (atomic, copy, nullable) dispatch_block_t block; + +@end + @implementation ARTScheduledBlockHandle { - dispatch_semaphore_t _semaphore; - dispatch_block_t _block; dispatch_block_t _scheduledBlock; } @@ -11,21 +16,13 @@ - (instancetype)initWithDelay:(NSTimeInterval)delay queue:(dispatch_queue_t)queu if (self == nil) return nil; - // Use a sempaphore to coorindate state. We use a reference here to decouple it when creating a block we'll schedule. - dispatch_semaphore_t semaphore = dispatch_semaphore_create(1); - __weak ARTScheduledBlockHandle *weakSelf = self; _scheduledBlock = dispatch_block_create(0, ^{ dispatch_block_t copiedBlock = nil; - dispatch_semaphore_wait(semaphore, DISPATCH_TIME_FOREVER); - { - // Get a strong reference to self within our semaphore to avoid potential race conditions - ARTScheduledBlockHandle *strongSelf = weakSelf; - if (strongSelf != nil) { - copiedBlock = strongSelf->_block; // copied below - } + ARTScheduledBlockHandle *strongSelf = weakSelf; + if (strongSelf != nil) { + copiedBlock = strongSelf.block; // copied below } - dispatch_semaphore_signal(semaphore); // If our block is non-nil, our scheduled block was still valid by the time this was invoked if (copiedBlock != nil) { @@ -33,8 +30,7 @@ - (instancetype)initWithDelay:(NSTimeInterval)delay queue:(dispatch_queue_t)queu } }); - _block = [block copy]; - _semaphore = semaphore; + self.block = block; // copied block dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(NSEC_PER_SEC * delay)), queue, _scheduledBlock); @@ -42,13 +38,8 @@ - (instancetype)initWithDelay:(NSTimeInterval)delay queue:(dispatch_queue_t)queu } - (void)cancel { - // Cancel within our semaphore for predictable behavior if our block is invoked while we're cancelling - dispatch_semaphore_wait(_semaphore, DISPATCH_TIME_FOREVER); - { - dispatch_block_cancel(_scheduledBlock); - _block = nil; - } - dispatch_semaphore_signal(_semaphore); + self.block = nil; + dispatch_block_cancel(_scheduledBlock); } - (void)dealloc { From a480152681795fb1f19993184390b62de00f7229 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 19 May 2024 16:04:29 +0200 Subject: [PATCH 39/40] Got back `resumed` property in `ARTChannelStateChangeParams` along with the removed test (for RTL2f). --- Source/ARTRealtimeChannel.m | 3 +- .../Ably/ARTChannelStateChangeParams.h | 5 ++ Test/Tests/RealtimeClientChannelTests.swift | 48 ++++++++++++++++++- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/Source/ARTRealtimeChannel.m b/Source/ARTRealtimeChannel.m index 7b226c6b7..ed860c3ea 100644 --- a/Source/ARTRealtimeChannel.m +++ b/Source/ARTRealtimeChannel.m @@ -545,7 +545,7 @@ - (void)emit:(ARTChannelEvent)event with:(ARTChannelStateChange *)data { - (void)performTransitionToState:(ARTRealtimeChannelState)state withParams:(ARTChannelStateChangeParams *)params { ARTLogDebug(self.logger, @"RT:%p C:%p (%@) channel state transitions from %tu - %@ to %tu - %@%@", _realtime, self, self.name, self.state_nosync, ARTRealtimeChannelStateToStr(self.state_nosync), state, ARTRealtimeChannelStateToStr(state), params.retryAttempt ? [NSString stringWithFormat: @" (result of %@)", params.retryAttempt.id] : @""); - ARTChannelStateChange *stateChange = [[ARTChannelStateChange alloc] initWithCurrent:state previous:self.state_nosync event:(ARTChannelEvent)state reason:params.errorInfo resumed:NO retryAttempt:params.retryAttempt]; + ARTChannelStateChange *stateChange = [[ARTChannelStateChange alloc] initWithCurrent:state previous:self.state_nosync event:(ARTChannelEvent)state reason:params.errorInfo resumed:params.resumed retryAttempt:params.retryAttempt]; self.state = state; if (params.storeErrorInfo) { @@ -680,6 +680,7 @@ - (void)setAttached:(ARTProtocolMessage *)message { } else { params = [[ARTChannelStateChangeParams alloc] initWithState:ARTStateOk]; } + params.resumed = message.resumed; [self performTransitionToState:ARTRealtimeChannelAttached withParams:params]; [self.presence onAttached:message]; [_attachedEventEmitter emit:nil with:nil]; diff --git a/Source/PrivateHeaders/Ably/ARTChannelStateChangeParams.h b/Source/PrivateHeaders/Ably/ARTChannelStateChangeParams.h index d5a24128a..3e3171d30 100644 --- a/Source/PrivateHeaders/Ably/ARTChannelStateChangeParams.h +++ b/Source/PrivateHeaders/Ably/ARTChannelStateChangeParams.h @@ -31,6 +31,11 @@ NS_SWIFT_NAME(ChannelStateChangeParams) @property (nullable, nonatomic, readonly) ARTRetryAttempt *retryAttempt; +/** + The `resumed` value of the `ARTProtocolMessage` that triggered this state change. + */ +@property (nonatomic) BOOL resumed; + - (instancetype)init NS_UNAVAILABLE; /** diff --git a/Test/Tests/RealtimeClientChannelTests.swift b/Test/Tests/RealtimeClientChannelTests.swift index 92254672e..fe9b769c4 100644 --- a/Test/Tests/RealtimeClientChannelTests.swift +++ b/Test/Tests/RealtimeClientChannelTests.swift @@ -497,8 +497,54 @@ class RealtimeClientChannelTests: XCTestCase { } } - // TODO: RTL2f + // RTL2f (connection resumption case) + func test__011a__Channel__EventEmitter__channel_states_and_events__ChannelStateChange_will_contain_a_resumed_boolean_attribute_with_value__true__if_the_bit_flag_RESUMED_was_included() throws { + let test = Test() + let options = try AblyTests.commonAppSetup(for: test) + options.tokenDetails = try getTestTokenDetails(for: test, ttl: 5.0) + let client = ARTRealtime(options: options) + defer { client.dispose(); client.close() } + let channel = client.channels.get(test.uniqueChannelName()) + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(4, done: done) + channel.on { stateChange in + switch stateChange.current { + case .attached: + XCTAssertFalse(stateChange.resumed) + partialDone() + default: + XCTAssertFalse(stateChange.resumed) + } + } + client.connection.once(.disconnected) { stateChange in + channel.off() + guard let error = stateChange.reason else { + fail("Error is nil"); done(); return + } + XCTAssertEqual(error.code, ARTErrorCode.tokenExpired.intValue) + XCTAssertEqual(channel.state, ARTRealtimeChannelState.attached) + client.connection.once(.connected) { stateChange in + XCTAssertEqual(channel.state, ARTRealtimeChannelState.attaching) + partialDone() + } + channel.on { stateChange in + switch stateChange.current { + case .attached: + XCTAssertTrue(stateChange.resumed) + partialDone() + default: + XCTAssertFalse(stateChange.resumed) + } + } + partialDone() + } + channel.attach() + } + } + + // TODO: RTL2f (connection recovery case) + // RTL3 // RTL3a From c8524f07266ea7b36f63163bb24ef42ac21be3d3 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 19 May 2024 21:44:41 +0200 Subject: [PATCH 40/40] Added second test for RTL2f for connection recovery case (issue #1812). --- Test/Tests/RealtimeClientChannelTests.swift | 43 ++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/Test/Tests/RealtimeClientChannelTests.swift b/Test/Tests/RealtimeClientChannelTests.swift index fe9b769c4..9727094f5 100644 --- a/Test/Tests/RealtimeClientChannelTests.swift +++ b/Test/Tests/RealtimeClientChannelTests.swift @@ -543,7 +543,48 @@ class RealtimeClientChannelTests: XCTestCase { } } - // TODO: RTL2f (connection recovery case) + // RTL2f (connection recovery case) + func test__011b__Channel__EventEmitter__channel_states_and_events__ChannelStateChange_will_contain_a_resumed_boolean_attribute_with_value__true__if_the_bit_flag_RESUMED_was_included_for_recovered_connection() throws { + let test = Test() + let options = try AblyTests.commonAppSetup(for: test) + let client = ARTRealtime(options: options) + defer { client.dispose(); client.close() } + + let channelName = test.uniqueChannelName() + let channel = client.channels.get(channelName) + + waitUntil(timeout: testTimeout) { done in + channel.on { stateChange in + switch stateChange.current { + case .attached: + XCTAssertFalse(stateChange.resumed) + done() + default: + XCTAssertFalse(stateChange.resumed) + } + } + channel.publish(nil, data: "A message") + } + + options.recover = client.connection.createRecoveryKey() + + let recoveredClient = ARTRealtime(options: options) + defer { recoveredClient.dispose(); recoveredClient.close() } + + let recoveredChannel = recoveredClient.channels.get(channelName) + + waitUntil(timeout: testTimeout) { done in + recoveredChannel.on { stateChange in + switch stateChange.current { + case .attached: + XCTAssertTrue(stateChange.resumed) + done() + default: + XCTAssertFalse(stateChange.resumed) + } + } + } + } // RTL3