From aeef1330940e06146c099301c370103709851d1e Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 17 Mar 2024 23:23:04 +0100 Subject: [PATCH 01/16] Made method names more obvious, fixed non-existing member update. --- Source/ARTPresenceMessage.m | 22 +++--- Source/ARTRealtimePresence.m | 73 ++++++++++--------- .../Ably/ARTRealtimePresence+Private.h | 7 +- Source/include/Ably/ARTPresenceMessage.h | 22 +++--- Test/Tests/RealtimeClientPresenceTests.swift | 10 +-- 5 files changed, 70 insertions(+), 64 deletions(-) diff --git a/Source/ARTPresenceMessage.m b/Source/ARTPresenceMessage.m index e2eac708b..6c07f5176 100644 --- a/Source/ARTPresenceMessage.m +++ b/Source/ARTPresenceMessage.m @@ -73,25 +73,25 @@ - (NSInteger)indexFromId { return index; } -- (BOOL)isNewerThan:(ARTPresenceMessage *)latest { - if (latest == nil) { +- (BOOL)isNewerThan:(ARTPresenceMessage *)existing { + if (existing == nil) { return YES; } - if ([self isSynthesized] || [latest isSynthesized]) { - return !self.timestamp || [latest.timestamp timeIntervalSince1970] <= [self.timestamp timeIntervalSince1970]; + if ([self isSynthesized] || [existing isSynthesized]) { + return !self.timestamp || [existing.timestamp timeIntervalSince1970] <= [self.timestamp timeIntervalSince1970]; } - NSInteger currentMsgSerial = [self msgSerialFromId]; - NSInteger currentIndex = [self indexFromId]; - NSInteger latestMsgSerial = [latest msgSerialFromId]; - NSInteger latestIndex = [latest indexFromId]; + NSInteger msgSerial = [self msgSerialFromId]; + NSInteger index = [self indexFromId]; + NSInteger existingMsgSerial = [existing msgSerialFromId]; + NSInteger existingIndex = [existing indexFromId]; - if (currentMsgSerial == latestMsgSerial) { - return currentIndex > latestIndex; + if (msgSerial == existingMsgSerial) { + return index > existingIndex; } else { - return currentMsgSerial > latestMsgSerial; + return msgSerial > existingMsgSerial; } } diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 50e9a87d8..b41ddcf01 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -691,31 +691,29 @@ - (void)onAttached:(ARTProtocolMessage *)message { - (void)onMessage:(ARTProtocolMessage *)message { int i = 0; for (ARTPresenceMessage *p in message.presence) { - ARTPresenceMessage *presence = p; - if (presence.data && _dataEncoder) { + ARTPresenceMessage *member = p; + if (member.data && _dataEncoder) { NSError *decodeError = nil; - presence = [p decodeWithEncoder:_dataEncoder error:&decodeError]; + member = [p decodeWithEncoder:_dataEncoder error:&decodeError]; if (decodeError != nil) { ARTErrorInfo *errorInfo = [ARTErrorInfo wrap:[ARTErrorInfo createWithCode:ARTErrorUnableToDecodeMessage message:decodeError.localizedFailureReason] prepend:@"Failed to decode data: "]; ARTLogError(self.logger, @"RT:%p C:%p (%@) %@", _realtime, _channel, _channel.name, errorInfo.message); } } - if (!presence.timestamp) { - presence.timestamp = message.timestamp; + if (!member.timestamp) { + member.timestamp = message.timestamp; } - if (!presence.id) { - presence.id = [NSString stringWithFormat:@"%@:%d", message.id, i]; + if (!member.id) { + member.id = [NSString stringWithFormat:@"%@:%d", message.id, i]; } - if (!presence.connectionId) { - presence.connectionId = message.connectionId; + if (!member.connectionId) { + member.connectionId = message.connectionId; } - if ([self add:presence]) { - [self broadcast:presence]; - } + [self processMember:member]; ++i; } @@ -780,50 +778,57 @@ - (void)reenterInternalMembers { return _internalMembers; } -- (BOOL)add:(ARTPresenceMessage *)message { - ARTPresenceMessage *latest = [_members objectForKey:message.memberKey]; - if ([message isNewerThan:latest]) { +- (void)processMember:(ARTPresenceMessage *)message { + BOOL memberUpdated = false; + ARTPresenceMessage *existing = [_members objectForKey:message.clientId]; + if ([message isNewerThan:existing]) { ARTPresenceMessage *messageCopy = [message copy]; switch (message.action) { case ARTPresenceEnter: case ARTPresenceUpdate: - messageCopy.action = ARTPresencePresent; - // intentional fallthrough case ARTPresencePresent: - [self internalAdd:messageCopy]; + [self addMember:messageCopy]; + memberUpdated = true; break; case ARTPresenceLeave: - [self internalRemove:messageCopy]; + if (existing) { + [self removeMember:messageCopy]; + memberUpdated = true; + } break; default: break; } - return YES; } - ARTLogDebug(_logger, @"Presence member \"%@\" with action %@ has been ignored", message.memberKey, ARTPresenceActionToStr(message.action)); - latest.syncSessionId = _syncSessionId; - return NO; + if (memberUpdated) { + [self broadcast:message]; + } + else { + existing.syncSessionId = _syncSessionId; + ARTLogDebug(_logger, @"Presence member \"%@\" with action %@ has been ignored", message.memberKey, ARTPresenceActionToStr(message.action)); + } } -- (void)internalAdd:(ARTPresenceMessage *)message { - [self internalAdd:message withSessionId:_syncSessionId]; +- (void)addMember:(ARTPresenceMessage *)message { + [self addMember:message withSessionId:_syncSessionId]; } -- (void)internalAdd:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId { +- (void)addMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId { + message.action = ARTPresencePresent; message.syncSessionId = sessionId; - [_members setObject:message forKey:message.memberKey]; - // Local member + _members[message.memberKey] = message; + // Internal member if ([message.connectionId isEqualToString:self.connectionId]) { _internalMembers[message.clientId] = message; ARTLogDebug(_logger, @"local member %@ with action %@ has been added", message.memberKey, ARTPresenceActionToStr(message.action).uppercaseString); } } -- (void)internalRemove:(ARTPresenceMessage *)message { - [self internalRemove:message force:false]; +- (void)removeMember:(ARTPresenceMessage *)message { + [self removeMember:message force:false]; } -- (void)internalRemove:(ARTPresenceMessage *)message force:(BOOL)force { +- (void)removeMember:(ARTPresenceMessage *)message force:(BOOL)force { if ([message.connectionId isEqualToString:self.connectionId] && !message.isSynthesized) { [_internalMembers removeObjectForKey:message.clientId]; } @@ -833,7 +838,7 @@ - (void)internalRemove:(ARTPresenceMessage *)message force:(BOOL)force { ARTLogDebug(_logger, @"%p \"%@\" should be removed after sync ends (syncInProgress=%d)", self, message.clientId, syncInProgress); message.action = ARTPresenceAbsent; // Should be removed after Sync ends - [self internalAdd:message withSessionId:message.syncSessionId]; + [self addMember:message withSessionId:message.syncSessionId]; } else { [_members removeObjectForKey:message.memberKey]; @@ -846,7 +851,7 @@ - (void)cleanUpAbsentMembers { return message.action == ARTPresenceAbsent; }]; for (NSString *key in filteredMembers) { - [self internalRemove:[_members objectForKey:key] force:true]; + [self removeMember:[_members objectForKey:key] force:true]; } } @@ -856,7 +861,7 @@ - (void)leaveMembersNotPresentInSync { if (member.syncSessionId != _syncSessionId) { // Handle members that have not been added or updated in the PresenceMap during the sync process ARTPresenceMessage *leave = [member copy]; - [self internalRemove:member force:true]; + [self removeMember:member force:true]; [self didRemovedMemberNoLongerPresent:leave]; } } diff --git a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h index e3a09847e..a1b5a6f2d 100644 --- a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h @@ -50,7 +50,7 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init UNAVAILABLE_ATTRIBUTE; - (instancetype)initWithQueue:(_Nonnull dispatch_queue_t)queue logger:(ARTInternalLog *)logger; -- (BOOL)add:(ARTPresenceMessage *)message; +- (void)processMember:(ARTPresenceMessage *)message; - (void)reset; - (void)startSync; @@ -60,8 +60,9 @@ NS_ASSUME_NONNULL_BEGIN - (void)onceSyncEnds:(void (^)(NSArray *))callback; - (void)onceSyncFails:(ARTCallback)callback; -- (void)internalAdd:(ARTPresenceMessage *)message; -- (void)internalAdd:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId; +- (void)addMember:(ARTPresenceMessage *)message; +- (void)addMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId; +- (void)removeMember:(ARTPresenceMessage *)message; - (void)cleanUpAbsentMembers; diff --git a/Source/include/Ably/ARTPresenceMessage.h b/Source/include/Ably/ARTPresenceMessage.h index cb3b1dfd7..b82ed5f4d 100644 --- a/Source/include/Ably/ARTPresenceMessage.h +++ b/Source/include/Ably/ARTPresenceMessage.h @@ -6,24 +6,24 @@ */ typedef NS_ENUM(NSUInteger, ARTPresenceAction) { /** - * A member is not present in the channel. - */ + * A member is not present in the channel. + */ ARTPresenceAbsent, /** - * When subscribing to presence events on a channel that already has members present, this event is emitted for every member already present on the channel before the subscribe listener was registered. - */ + * When subscribing to presence events on a channel that already has members present, this event is emitted for every member already present on the channel before the subscribe listener was registered. + */ ARTPresencePresent, /** - * A new member has entered the channel. - */ + * A new member has entered the channel. + */ ARTPresenceEnter, /** - * A member who was present has now left the channel. This may be a result of an explicit request to leave or implicitly when detaching from the channel. Alternatively, if a member's connection is abruptly disconnected and they do not resume their connection within a minute, Ably treats this as a leave event as the client is no longer present. - */ + * A member who was present has now left the channel. This may be a result of an explicit request to leave or implicitly when detaching from the channel. Alternatively, if a member's connection is abruptly disconnected and they do not resume their connection within a minute, Ably treats this as a leave event as the client is no longer present. + */ ARTPresenceLeave, /** - * An already present member has updated their member data. Being notified of member data updates can be very useful, for example, it can be used to update the status of a user when they are typing a message. - */ + * An already present member has updated their member data. Being notified of member data updates can be very useful, for example, it can be used to update the status of a user when they are typing a message. + */ ARTPresenceUpdate }; @@ -53,7 +53,7 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)isEqualToPresenceMessage:(nonnull ARTPresenceMessage *)presence; /// :nodoc: -- (BOOL)isNewerThan:(ARTPresenceMessage *)latest __attribute__((warn_unused_result)); +- (BOOL)isNewerThan:(nullable ARTPresenceMessage *)latest __attribute__((warn_unused_result)); @end diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index f345d08b5..0f42f52e9 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -333,7 +333,7 @@ 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") - channel.internal.presence.add(internalMember) + channel.internal.presence.processMember(internalMember) XCTAssertEqual(channel.internal.presence.members.count, 3) XCTAssertEqual(channel.internal.presence.members.filter { memberKey, _ in memberKey.contains(internalMember.clientId!) }.count, 1) @@ -379,8 +379,8 @@ class RealtimeClientPresenceTests: XCTestCase { let channel = client.channels.get(channelName) // Inject local members - channel.internal.presence.add(ARTPresenceMessage(clientId: "tester1", action: .enter, connectionId: "another", id: "another:0:0")) - channel.internal.presence.add(ARTPresenceMessage(clientId: "tester2", action: .enter, connectionId: "another", id: "another:0:1")) + channel.internal.presence.processMember(ARTPresenceMessage(clientId: "tester1", action: .enter, connectionId: "another", id: "another:0:0")) + channel.internal.presence.processMember(ARTPresenceMessage(clientId: "tester2", action: .enter, connectionId: "another", id: "another:0:1")) guard let transport = client.internal.transport as? TestProxyTransport else { fail("TestProxyTransport is not set"); return @@ -1955,7 +1955,7 @@ class RealtimeClientPresenceTests: XCTestCase { } hook = channel.internal.presence.testSuite_getArgument( - from: #selector(ARTRealtimePresenceInternal.internalAdd(_:withSessionId:)), + from: #selector(ARTRealtimePresenceInternal.addMember(_:withSessionId:)), at: 0 ) { arg in let m = arg as? ARTPresenceMessage @@ -3578,7 +3578,7 @@ class RealtimeClientPresenceTests: XCTestCase { for i in 0 ..< 3 { let msg = ARTPresenceMessage(clientId: "client\(i)", action: .present, connectionId: "foo", id: "foo:0:0") msgs[msg.clientId!] = msg - channel.internal.presence.internalAdd(msg) + channel.internal.presence.addMember(msg) } channel.presence.get(getParams) { result, err in From 515db0ff8cfae18daad5e1e50fb40bc06fd493b6 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 18 Mar 2024 00:37:10 +0100 Subject: [PATCH 02/16] Removed `syncSessionId` from presence object. --- Source/ARTRealtimePresence.m | 23 +++++++++++-------- .../Ably/ARTRealtimePresence+Private.h | 1 - 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index b41ddcf01..284b69548 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -175,7 +175,6 @@ @implementation ARTRealtimePresenceInternal { ARTEventEmitter *_eventEmitter; ARTDataEncoder *_dataEncoder; - NSUInteger _syncSessionId; ARTPresenceSyncState _syncState; ARTEventEmitter *_syncEventEmitter; @@ -196,7 +195,6 @@ - (instancetype)initWithChannel:(ARTRealtimeChannelInternal *)channel logger:(AR _dataEncoder = _channel.dataEncoder; _members = [NSMutableDictionary new]; _internalMembers = [NSMutableDictionary new]; - _syncSessionId = 0; _syncState = ARTPresenceSyncInitialized; _syncEventEmitter = [[ARTInternalEventEmitter alloc] initWithQueue:_queue]; } @@ -804,13 +802,12 @@ - (void)processMember:(ARTPresenceMessage *)message { [self broadcast:message]; } else { - existing.syncSessionId = _syncSessionId; ARTLogDebug(_logger, @"Presence member \"%@\" with action %@ has been ignored", message.memberKey, ARTPresenceActionToStr(message.action)); } } - (void)addMember:(ARTPresenceMessage *)message { - [self addMember:message withSessionId:_syncSessionId]; + [self addMember:message withSessionId:1]; } - (void)addMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId { @@ -836,9 +833,9 @@ - (void)removeMember:(ARTPresenceMessage *)message force:(BOOL)force { const BOOL syncInProgress = self.syncInProgress; if (!force && syncInProgress) { ARTLogDebug(_logger, @"%p \"%@\" should be removed after sync ends (syncInProgress=%d)", self, message.clientId, syncInProgress); - message.action = ARTPresenceAbsent; // Should be removed after Sync ends - [self addMember:message withSessionId:message.syncSessionId]; + [self addMember:message withSessionId:0]; + message.action = ARTPresenceAbsent; } else { [_members removeObjectForKey:message.memberKey]; @@ -846,7 +843,7 @@ - (void)removeMember:(ARTPresenceMessage *)message force:(BOOL)force { } - (void)cleanUpAbsentMembers { - ARTLogDebug(_logger, @"%p cleaning up absent members (syncSessionId=%lu)", self, (unsigned long)_syncSessionId); + ARTLogDebug(_logger, @"%p cleaning up absent members...", self); NSSet *filteredMembers = [_members keysOfEntriesPassingTest:^BOOL(NSString *key, ARTPresenceMessage *message, BOOL *stop) { return message.action == ARTPresenceAbsent; }]; @@ -855,10 +852,16 @@ - (void)cleanUpAbsentMembers { } } +- (void)prepareMembersForSync { + for (ARTPresenceMessage *member in [_members allValues]) { + member.syncSessionId = 0; + } +} + - (void)leaveMembersNotPresentInSync { - ARTLogDebug(_logger, @"%p leaving members not present in sync (syncSessionId=%lu)", self, (unsigned long)_syncSessionId); + ARTLogDebug(_logger, @"%p leaving members not present in sync...", self); for (ARTPresenceMessage *member in [_members allValues]) { - if (member.syncSessionId != _syncSessionId) { + if (member.syncSessionId == 0) { // Handle members that have not been added or updated in the PresenceMap during the sync process ARTPresenceMessage *leave = [member copy]; [self removeMember:member force:true]; @@ -874,7 +877,7 @@ - (void)reset { - (void)startSync { ARTLogDebug(_logger, @"%p PresenceMap sync started", self); - _syncSessionId++; + [self prepareMembersForSync]; _syncState = ARTPresenceSyncStarted; [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:_syncState] with:nil]; } diff --git a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h index a1b5a6f2d..9b7a222a5 100644 --- a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h @@ -43,7 +43,6 @@ NS_ASSUME_NONNULL_BEGIN /// The key is the clientId and the value is the latest relevant ARTPresenceMessage for that clientId. @property (readonly, atomic) NSMutableDictionary *internalMembers; -@property (readonly, nonatomic) NSUInteger syncSessionId; @property (readonly, nonatomic) BOOL syncComplete; @property (readonly, nonatomic) BOOL syncInProgress; From 11a239b9d7323928fb3aaaf85bddc6fb70184940 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Mon, 18 Mar 2024 01:54:28 +0100 Subject: [PATCH 03/16] Fix for message key. --- Source/ARTRealtimePresence.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 284b69548..d535de173 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -778,7 +778,7 @@ - (void)reenterInternalMembers { - (void)processMember:(ARTPresenceMessage *)message { BOOL memberUpdated = false; - ARTPresenceMessage *existing = [_members objectForKey:message.clientId]; + ARTPresenceMessage *existing = [_members objectForKey:message.memberKey]; if ([message isNewerThan:existing]) { ARTPresenceMessage *messageCopy = [message copy]; switch (message.action) { From 2ee0ef719ea4fbcfd3b9ffd0d6f55d738929f5b2 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 19 Mar 2024 01:08:36 +0100 Subject: [PATCH 04/16] Refactoring to ably-go impl. --- Source/ARTPresenceMessage.m | 4 - Source/ARTRealtimePresence.m | 94 +++++++++++-------- .../Ably/ARTRealtimePresence+Private.h | 8 +- Source/include/Ably/ARTPresenceMessage.h | 2 +- Test/Tests/RealtimeClientPresenceTests.swift | 4 +- 5 files changed, 64 insertions(+), 48 deletions(-) diff --git a/Source/ARTPresenceMessage.m b/Source/ARTPresenceMessage.m index 6c07f5176..b0b0d7ad3 100644 --- a/Source/ARTPresenceMessage.m +++ b/Source/ARTPresenceMessage.m @@ -74,10 +74,6 @@ - (NSInteger)indexFromId { } - (BOOL)isNewerThan:(ARTPresenceMessage *)existing { - if (existing == nil) { - return YES; - } - if ([self isSynthesized] || [existing isSynthesized]) { return !self.timestamp || [existing.timestamp timeIntervalSince1970] <= [self.timestamp timeIntervalSince1970]; } diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index d535de173..4d9ebf4cd 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -777,27 +777,42 @@ - (void)reenterInternalMembers { } - (void)processMember:(ARTPresenceMessage *)message { - BOOL memberUpdated = false; - ARTPresenceMessage *existing = [_members objectForKey:message.memberKey]; - if ([message isNewerThan:existing]) { - ARTPresenceMessage *messageCopy = [message copy]; + ARTPresenceMessage *messageCopy = [message copy]; + // Internal member + if ([message.connectionId isEqualToString:self.connectionId]) { switch (message.action) { case ARTPresenceEnter: case ARTPresenceUpdate: case ARTPresencePresent: - [self addMember:messageCopy]; - memberUpdated = true; + messageCopy.action = ARTPresencePresent; + [self addInternalMember:messageCopy withSessionId:1]; break; case ARTPresenceLeave: - if (existing) { - [self removeMember:messageCopy]; - memberUpdated = true; + if (!message.isSynthesized) { + [self removeInternalMember:messageCopy]; } break; default: break; } } + + BOOL memberUpdated = false; + switch (message.action) { + case ARTPresenceEnter: + case ARTPresenceUpdate: + case ARTPresencePresent: + messageCopy.action = ARTPresencePresent; + memberUpdated = [self addMember:messageCopy withSessionId:1]; + break; + case ARTPresenceLeave: + messageCopy.action = ARTPresenceAbsent; // RTP2f + memberUpdated = [self removeMember:messageCopy]; + break; + default: + break; + } + if (memberUpdated) { [self broadcast:message]; } @@ -806,49 +821,52 @@ - (void)processMember:(ARTPresenceMessage *)message { } } -- (void)addMember:(ARTPresenceMessage *)message { - [self addMember:message withSessionId:1]; -} - -- (void)addMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId { - message.action = ARTPresencePresent; +- (BOOL)addMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId { + ARTPresenceMessage *existing = [_members objectForKey:message.memberKey]; + if (existing && [existing isNewerThan:message]) { + return false; + } message.syncSessionId = sessionId; _members[message.memberKey] = message; - // Internal member - if ([message.connectionId isEqualToString:self.connectionId]) { - _internalMembers[message.clientId] = message; - ARTLogDebug(_logger, @"local member %@ with action %@ has been added", message.memberKey, ARTPresenceActionToStr(message.action).uppercaseString); - } + return true; } -- (void)removeMember:(ARTPresenceMessage *)message { - [self removeMember:message force:false]; +- (BOOL)addInternalMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId { + ARTPresenceMessage *existing = [_internalMembers objectForKey:message.clientId]; + if (existing && [existing isNewerThan:message]) { + return false; + } + message.syncSessionId = sessionId; + _internalMembers[message.clientId] = message; + ARTLogDebug(_logger, @"local member %@ with action %@ has been added", message.clientId, ARTPresenceActionToStr(message.action).uppercaseString); + return true; } -- (void)removeMember:(ARTPresenceMessage *)message force:(BOOL)force { - if ([message.connectionId isEqualToString:self.connectionId] && !message.isSynthesized) { - [_internalMembers removeObjectForKey:message.clientId]; +- (BOOL)removeMember:(ARTPresenceMessage *)message { + ARTPresenceMessage *existing = [_members objectForKey:message.memberKey]; + if (existing && [existing isNewerThan:message]) { + return false; } + [_members removeObjectForKey:message.memberKey]; + return true; +} - const BOOL syncInProgress = self.syncInProgress; - if (!force && syncInProgress) { - ARTLogDebug(_logger, @"%p \"%@\" should be removed after sync ends (syncInProgress=%d)", self, message.clientId, syncInProgress); - // Should be removed after Sync ends - [self addMember:message withSessionId:0]; - message.action = ARTPresenceAbsent; - } - else { - [_members removeObjectForKey:message.memberKey]; +- (BOOL)removeInternalMember:(ARTPresenceMessage *)message { + ARTPresenceMessage *existing = [_internalMembers objectForKey:message.clientId]; + if (existing && [existing isNewerThan:message]) { + return false; } + [_internalMembers removeObjectForKey:message.clientId]; + return true; } - (void)cleanUpAbsentMembers { ARTLogDebug(_logger, @"%p cleaning up absent members...", self); - NSSet *filteredMembers = [_members keysOfEntriesPassingTest:^BOOL(NSString *key, ARTPresenceMessage *message, BOOL *stop) { + NSSet *absentMembers = [_members keysOfEntriesPassingTest:^BOOL(NSString *key, ARTPresenceMessage *message, BOOL *stop) { return message.action == ARTPresenceAbsent; }]; - for (NSString *key in filteredMembers) { - [self removeMember:[_members objectForKey:key] force:true]; + for (NSString *key in absentMembers) { + [self removeMember:[_members objectForKey:key]]; } } @@ -864,7 +882,7 @@ - (void)leaveMembersNotPresentInSync { if (member.syncSessionId == 0) { // Handle members that have not been added or updated in the PresenceMap during the sync process ARTPresenceMessage *leave = [member copy]; - [self removeMember:member force:true]; + [self removeMember:member]; [self didRemovedMemberNoLongerPresent:leave]; } } diff --git a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h index 9b7a222a5..2083cac6c 100644 --- a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h @@ -59,9 +59,11 @@ NS_ASSUME_NONNULL_BEGIN - (void)onceSyncEnds:(void (^)(NSArray *))callback; - (void)onceSyncFails:(ARTCallback)callback; -- (void)addMember:(ARTPresenceMessage *)message; -- (void)addMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId; -- (void)removeMember:(ARTPresenceMessage *)message; +- (BOOL)addMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId; +- (BOOL)addInternalMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId; + +- (BOOL)removeMember:(ARTPresenceMessage *)message; +- (BOOL)removeInternalMember:(ARTPresenceMessage *)message; - (void)cleanUpAbsentMembers; diff --git a/Source/include/Ably/ARTPresenceMessage.h b/Source/include/Ably/ARTPresenceMessage.h index b82ed5f4d..92906248b 100644 --- a/Source/include/Ably/ARTPresenceMessage.h +++ b/Source/include/Ably/ARTPresenceMessage.h @@ -53,7 +53,7 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)isEqualToPresenceMessage:(nonnull ARTPresenceMessage *)presence; /// :nodoc: -- (BOOL)isNewerThan:(nullable ARTPresenceMessage *)latest __attribute__((warn_unused_result)); +- (BOOL)isNewerThan:(ARTPresenceMessage *)latest __attribute__((warn_unused_result)); @end diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 0f42f52e9..8c1f12e08 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1955,7 +1955,7 @@ class RealtimeClientPresenceTests: XCTestCase { } hook = channel.internal.presence.testSuite_getArgument( - from: #selector(ARTRealtimePresenceInternal.addMember(_:withSessionId:)), + from: #selector(ARTRealtimePresenceInternal.processMember(_:)), at: 0 ) { arg in let m = arg as? ARTPresenceMessage @@ -3578,7 +3578,7 @@ class RealtimeClientPresenceTests: XCTestCase { for i in 0 ..< 3 { let msg = ARTPresenceMessage(clientId: "client\(i)", action: .present, connectionId: "foo", id: "foo:0:0") msgs[msg.clientId!] = msg - channel.internal.presence.addMember(msg) + channel.internal.presence.processMember(msg) } channel.presence.get(getParams) { result, err in From 4d01584946a8fc1b22fbb5715950ddc36acf9118 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 19 Mar 2024 13:28:13 +0100 Subject: [PATCH 05/16] Fixed naming. --- Source/ARTPresenceMessage.m | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Source/ARTPresenceMessage.m b/Source/ARTPresenceMessage.m index b0b0d7ad3..519658664 100644 --- a/Source/ARTPresenceMessage.m +++ b/Source/ARTPresenceMessage.m @@ -73,21 +73,21 @@ - (NSInteger)indexFromId { return index; } -- (BOOL)isNewerThan:(ARTPresenceMessage *)existing { - if ([self isSynthesized] || [existing isSynthesized]) { - return !self.timestamp || [existing.timestamp timeIntervalSince1970] <= [self.timestamp timeIntervalSince1970]; +- (BOOL)isNewerThan:(ARTPresenceMessage *)other { + if ([self isSynthesized] || [other isSynthesized]) { + return !self.timestamp || [other.timestamp timeIntervalSince1970] <= [self.timestamp timeIntervalSince1970]; } NSInteger msgSerial = [self msgSerialFromId]; NSInteger index = [self indexFromId]; - NSInteger existingMsgSerial = [existing msgSerialFromId]; - NSInteger existingIndex = [existing indexFromId]; + NSInteger otherMsgSerial = [other msgSerialFromId]; + NSInteger otherIndex = [other indexFromId]; - if (msgSerial == existingMsgSerial) { - return index > existingIndex; + if (msgSerial == otherMsgSerial) { + return index > otherIndex; } else { - return msgSerial > existingMsgSerial; + return msgSerial > otherMsgSerial; } } From b98d6b35af7750f12884347b09507af6fa413966 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Tue, 19 Mar 2024 13:56:11 +0100 Subject: [PATCH 06/16] Removed using syncSessionId. --- Source/ARTPresenceMessage.m | 3 -- Source/ARTRealtimePresence.m | 33 ++++++++----------- .../Ably/ARTPresenceMessage+Private.h | 2 -- .../Ably/ARTRealtimePresence+Private.h | 4 +-- 4 files changed, 15 insertions(+), 27 deletions(-) diff --git a/Source/ARTPresenceMessage.m b/Source/ARTPresenceMessage.m index 519658664..9309bba0e 100644 --- a/Source/ARTPresenceMessage.m +++ b/Source/ARTPresenceMessage.m @@ -10,7 +10,6 @@ - (instancetype)init { if (self) { // Default _action = ARTPresenceEnter; - _syncSessionId = 0; } return self; } @@ -18,7 +17,6 @@ - (instancetype)init { - (id)copyWithZone:(NSZone *)zone { ARTPresenceMessage *message = [super copyWithZone:zone]; message->_action = self.action; - message->_syncSessionId = self.syncSessionId; return message; } @@ -27,7 +25,6 @@ - (NSString *)description { [description deleteCharactersInRange:NSMakeRange(description.length - (description.length>2 ? 2:0), 2)]; [description appendFormat:@",\n"]; [description appendFormat:@" action: %lu,\n", (unsigned long)self.action]; - [description appendFormat:@" syncSessionId: %lu\n", (unsigned long)self.syncSessionId]; [description appendFormat:@"}"]; return description; } diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 4d9ebf4cd..38c597e60 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -180,6 +180,8 @@ @implementation ARTRealtimePresenceInternal { NSMutableDictionary *_members; NSMutableDictionary *_internalMembers; // RTP17h + + NSMutableDictionary *_beforeSyncMembers; // RTP19 } - (instancetype)initWithChannel:(ARTRealtimeChannelInternal *)channel logger:(ARTInternalLog *)logger { @@ -785,7 +787,7 @@ - (void)processMember:(ARTPresenceMessage *)message { case ARTPresenceUpdate: case ARTPresencePresent: messageCopy.action = ARTPresencePresent; - [self addInternalMember:messageCopy withSessionId:1]; + [self addInternalMember:messageCopy]; break; case ARTPresenceLeave: if (!message.isSynthesized) { @@ -803,7 +805,7 @@ - (void)processMember:(ARTPresenceMessage *)message { case ARTPresenceUpdate: case ARTPresencePresent: messageCopy.action = ARTPresencePresent; - memberUpdated = [self addMember:messageCopy withSessionId:1]; + memberUpdated = [self addMember:messageCopy]; break; case ARTPresenceLeave: messageCopy.action = ARTPresenceAbsent; // RTP2f @@ -814,6 +816,7 @@ - (void)processMember:(ARTPresenceMessage *)message { } if (memberUpdated) { + [_beforeSyncMembers removeObjectForKey:message.memberKey]; // RTP19 [self broadcast:message]; } else { @@ -821,22 +824,20 @@ - (void)processMember:(ARTPresenceMessage *)message { } } -- (BOOL)addMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId { +- (BOOL)addMember:(ARTPresenceMessage *)message { ARTPresenceMessage *existing = [_members objectForKey:message.memberKey]; if (existing && [existing isNewerThan:message]) { return false; } - message.syncSessionId = sessionId; _members[message.memberKey] = message; return true; } -- (BOOL)addInternalMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId { +- (BOOL)addInternalMember:(ARTPresenceMessage *)message { ARTPresenceMessage *existing = [_internalMembers objectForKey:message.clientId]; if (existing && [existing isNewerThan:message]) { return false; } - message.syncSessionId = sessionId; _internalMembers[message.clientId] = message; ARTLogDebug(_logger, @"local member %@ with action %@ has been added", message.clientId, ARTPresenceActionToStr(message.action).uppercaseString); return true; @@ -870,21 +871,13 @@ - (void)cleanUpAbsentMembers { } } -- (void)prepareMembersForSync { - for (ARTPresenceMessage *member in [_members allValues]) { - member.syncSessionId = 0; - } -} - - (void)leaveMembersNotPresentInSync { ARTLogDebug(_logger, @"%p leaving members not present in sync...", self); - for (ARTPresenceMessage *member in [_members allValues]) { - if (member.syncSessionId == 0) { - // Handle members that have not been added or updated in the PresenceMap during the sync process - ARTPresenceMessage *leave = [member copy]; - [self removeMember:member]; - [self didRemovedMemberNoLongerPresent:leave]; - } + for (ARTPresenceMessage *member in [_beforeSyncMembers allValues]) { + // Handle members that have not been added or updated in the PresenceMap during the sync process + ARTPresenceMessage *leave = [member copy]; + [self removeMember:member]; + [self didRemovedMemberNoLongerPresent:leave]; } } @@ -895,7 +888,7 @@ - (void)reset { - (void)startSync { ARTLogDebug(_logger, @"%p PresenceMap sync started", self); - [self prepareMembersForSync]; + _beforeSyncMembers = [_members mutableCopy]; _syncState = ARTPresenceSyncStarted; [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:_syncState] with:nil]; } diff --git a/Source/PrivateHeaders/Ably/ARTPresenceMessage+Private.h b/Source/PrivateHeaders/Ably/ARTPresenceMessage+Private.h index e2a3c2c7f..ca1f97307 100644 --- a/Source/PrivateHeaders/Ably/ARTPresenceMessage+Private.h +++ b/Source/PrivateHeaders/Ably/ARTPresenceMessage+Private.h @@ -2,8 +2,6 @@ @interface ARTPresenceMessage () -@property (readwrite, nonatomic) NSUInteger syncSessionId; - /** Returns whether this presenceMessage is synthesized, i.e. was not actually sent by the connection (usually means a leave event sent 15s after a disconnection). This is useful because synthesized messages cannot be compared for newness by id lexicographically - RTP2b1. */ diff --git a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h index 2083cac6c..16bbf76a4 100644 --- a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h @@ -59,8 +59,8 @@ NS_ASSUME_NONNULL_BEGIN - (void)onceSyncEnds:(void (^)(NSArray *))callback; - (void)onceSyncFails:(ARTCallback)callback; -- (BOOL)addMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId; -- (BOOL)addInternalMember:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sessionId; +- (BOOL)addMember:(ARTPresenceMessage *)message; +- (BOOL)addInternalMember:(ARTPresenceMessage *)message; - (BOOL)removeMember:(ARTPresenceMessage *)message; - (BOOL)removeInternalMember:(ARTPresenceMessage *)message; From 508806a95ba88ed432b4781cadf396580fe2e923 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Wed, 20 Mar 2024 22:10:17 +0100 Subject: [PATCH 07/16] A few fixes. --- Source/ARTPresenceMessage.m | 2 +- Source/ARTRealtimePresence.m | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Source/ARTPresenceMessage.m b/Source/ARTPresenceMessage.m index 9309bba0e..fb51fa443 100644 --- a/Source/ARTPresenceMessage.m +++ b/Source/ARTPresenceMessage.m @@ -81,7 +81,7 @@ - (BOOL)isNewerThan:(ARTPresenceMessage *)other { NSInteger otherIndex = [other indexFromId]; if (msgSerial == otherMsgSerial) { - return index > otherIndex; + return index >= otherIndex; } else { return msgSerial > otherMsgSerial; diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 38c597e60..202acb27c 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -816,12 +816,12 @@ - (void)processMember:(ARTPresenceMessage *)message { } if (memberUpdated) { - [_beforeSyncMembers removeObjectForKey:message.memberKey]; // RTP19 [self broadcast:message]; } else { ARTLogDebug(_logger, @"Presence member \"%@\" with action %@ has been ignored", message.memberKey, ARTPresenceActionToStr(message.action)); } + [_beforeSyncMembers removeObjectForKey:message.memberKey]; // RTP19 } - (BOOL)addMember:(ARTPresenceMessage *)message { @@ -849,7 +849,7 @@ - (BOOL)removeMember:(ARTPresenceMessage *)message { return false; } [_members removeObjectForKey:message.memberKey]; - return true; + return existing.action != ARTPresenceAbsent; } - (BOOL)removeInternalMember:(ARTPresenceMessage *)message { @@ -876,7 +876,7 @@ - (void)leaveMembersNotPresentInSync { for (ARTPresenceMessage *member in [_beforeSyncMembers allValues]) { // Handle members that have not been added or updated in the PresenceMap during the sync process ARTPresenceMessage *leave = [member copy]; - [self removeMember:member]; + [_members removeObjectForKey:leave.memberKey]; [self didRemovedMemberNoLongerPresent:leave]; } } @@ -898,6 +898,7 @@ - (void)endSync { [self cleanUpAbsentMembers]; [self leaveMembersNotPresentInSync]; _syncState = ARTPresenceSyncEnded; + _beforeSyncMembers = nil; [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:ARTPresenceSyncEnded] with:[_members allValues]]; [_syncEventEmitter off]; From 1d2f015ff007537fd66febc6b28f79c7e2e37468 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 21 Mar 2024 00:08:20 +0100 Subject: [PATCH 08/16] RTP2f implementation. --- Source/ARTRealtimePresence.m | 10 ++++++--- Test/Tests/RealtimeClientPresenceTests.swift | 22 +++++++------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 202acb27c..ec01ce06b 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -808,8 +808,12 @@ - (void)processMember:(ARTPresenceMessage *)message { memberUpdated = [self addMember:messageCopy]; break; case ARTPresenceLeave: - messageCopy.action = ARTPresenceAbsent; // RTP2f - memberUpdated = [self removeMember:messageCopy]; + if (self.syncInProgress) { + messageCopy.action = ARTPresenceAbsent; // RTP2f + memberUpdated = [self addMember:messageCopy]; + } else { + memberUpdated = [self removeMember:messageCopy]; + } break; default: break; @@ -867,7 +871,7 @@ - (void)cleanUpAbsentMembers { return message.action == ARTPresenceAbsent; }]; for (NSString *key in absentMembers) { - [self removeMember:[_members objectForKey:key]]; + [_members removeObjectForKey:key]; } } diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 8c1f12e08..5e737d4cd 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1928,7 +1928,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP2f - func test__FLAKY__050__Presence__PresenceMap__if_a_SYNC_is_in_progress__then_when_a_presence_message_with_an_action_of_LEAVE_arrives__it_should_be_stored_in_the_presence_map_with_the_action_set_to_ABSENT() throws { + func test__050__Presence__PresenceMap__if_a_SYNC_is_in_progress__then_when_a_presence_message_with_an_action_of_LEAVE_arrives__it_should_be_stored_in_the_presence_map_with_the_action_set_to_ABSENT() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let channelName = test.uniqueChannelName() @@ -1944,26 +1944,18 @@ class RealtimeClientPresenceTests: XCTestCase { guard let transport = client.internal.transport as? TestProxyTransport else { fail("TestProxyTransport is not set"); return } - - var hook: AspectToken? + waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) 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) partialDone() } - hook = channel.internal.presence.testSuite_getArgument( - from: #selector(ARTRealtimePresenceInternal.processMember(_:)), - at: 0 - ) { arg in - let m = arg as? ARTPresenceMessage - if m?.clientId == "user11", m?.action == .absent { - partialDone() - } - } - channel.attach { error in XCTAssertNil(error) XCTAssertTrue(channel.internal.presence.syncInProgress) @@ -1977,11 +1969,11 @@ class RealtimeClientPresenceTests: XCTestCase { ARTPresenceMessage(clientId: "user11", action: .leave, connectionId: "another", id: "another:123:0", timestamp: Date()), ] transport.receive(leaveMessage) + partialDone() } } - hook?.remove() channel.presence.unsubscribe() - + expect(channel.internal.presence.syncInProgress).toEventually(beFalse(), timeout: testTimeout) expect(channel.internal.presence.members.filter { _, presence in presence.action == .leave }).to(beEmpty()) expect(channel.internal.presence.members.filter { _, presence in presence.action == .absent }).to(beEmpty()) From 7ae70b3a8efd93ae93643657d7de0c2abdf4a2a1 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sat, 30 Mar 2024 16:19:58 +0100 Subject: [PATCH 09/16] Moved `isNewerThan:` to presence to avoid further confusion of what newer than what. Changed `addMember:` and `removeMember:` to resemble ably-go version. --- Source/ARTPresenceMessage.m | 18 ------ Source/ARTRealtimePresence.m | 58 +++++++++++++------ .../Ably/ARTRealtimePresence+Private.h | 6 +- Source/include/Ably/ARTPresenceMessage.h | 3 - Test/Tests/RealtimeClientPresenceTests.swift | 4 +- 5 files changed, 45 insertions(+), 44 deletions(-) diff --git a/Source/ARTPresenceMessage.m b/Source/ARTPresenceMessage.m index fb51fa443..7d2ded38a 100644 --- a/Source/ARTPresenceMessage.m +++ b/Source/ARTPresenceMessage.m @@ -70,24 +70,6 @@ - (NSInteger)indexFromId { return index; } -- (BOOL)isNewerThan:(ARTPresenceMessage *)other { - if ([self isSynthesized] || [other isSynthesized]) { - return !self.timestamp || [other.timestamp timeIntervalSince1970] <= [self.timestamp timeIntervalSince1970]; - } - - NSInteger msgSerial = [self msgSerialFromId]; - NSInteger index = [self indexFromId]; - NSInteger otherMsgSerial = [other msgSerialFromId]; - NSInteger otherIndex = [other indexFromId]; - - if (msgSerial == otherMsgSerial) { - return index >= otherIndex; - } - else { - return msgSerial > otherMsgSerial; - } -} - #pragma mark - NSObject - (BOOL)isEqual:(id)object { diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index ec01ce06b..b30c37aa6 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -828,41 +828,61 @@ - (void)processMember:(ARTPresenceMessage *)message { [_beforeSyncMembers removeObjectForKey:message.memberKey]; // RTP19 } +- (BOOL)member:(ARTPresenceMessage *)msg1 isNewerThan:(ARTPresenceMessage *)msg2 { + if ([msg1 isSynthesized] || [msg2 isSynthesized]) { + return !msg1.timestamp || msg1.timestamp.timeIntervalSince1970 >= msg2.timestamp.timeIntervalSince1970; + } + + NSInteger msg1Serial = [msg1 msgSerialFromId]; + NSInteger msg1Index = [msg1 indexFromId]; + NSInteger msg2Serial = [msg2 msgSerialFromId]; + NSInteger msg2Index = [msg2 indexFromId]; + + if (msg1Serial == msg2Serial) { + return msg1Index > msg2Index; + } + else { + return msg1Serial > msg2Serial; + } +} + - (BOOL)addMember:(ARTPresenceMessage *)message { ARTPresenceMessage *existing = [_members objectForKey:message.memberKey]; - if (existing && [existing isNewerThan:message]) { + if (existing) { + if ([self member:message isNewerThan:existing]) { + _members[message.memberKey] = message; + return true; + } return false; } _members[message.memberKey] = message; return true; } -- (BOOL)addInternalMember:(ARTPresenceMessage *)message { - ARTPresenceMessage *existing = [_internalMembers objectForKey:message.clientId]; - if (existing && [existing isNewerThan:message]) { - return false; +- (BOOL)removeMember:(ARTPresenceMessage *)message { + ARTPresenceMessage *existing = [_members objectForKey:message.memberKey]; + if (existing) { + if ([self member:message isNewerThan:existing]) { + [_members removeObjectForKey:message.memberKey]; + return existing.action != ARTPresenceAbsent; + } } - _internalMembers[message.clientId] = message; - ARTLogDebug(_logger, @"local member %@ with action %@ has been added", message.clientId, ARTPresenceActionToStr(message.action).uppercaseString); - return true; + return false; } -- (BOOL)removeMember:(ARTPresenceMessage *)message { - ARTPresenceMessage *existing = [_members objectForKey:message.memberKey]; - if (existing && [existing isNewerThan:message]) { - return false; +- (void)addInternalMember:(ARTPresenceMessage *)message { + ARTPresenceMessage *existing = [_internalMembers objectForKey:message.clientId]; + if (!existing || [self member:message isNewerThan:existing]) { + _internalMembers[message.clientId] = message; + ARTLogDebug(_logger, @"local member %@ with action %@ has been added", message.clientId, ARTPresenceActionToStr(message.action).uppercaseString); } - [_members removeObjectForKey:message.memberKey]; - return existing.action != ARTPresenceAbsent; } -- (BOOL)removeInternalMember:(ARTPresenceMessage *)message { +- (void)removeInternalMember:(ARTPresenceMessage *)message { ARTPresenceMessage *existing = [_internalMembers objectForKey:message.clientId]; - if (existing && [existing isNewerThan:message]) { - return false; + if (!existing || [self member:message isNewerThan:existing]) { + [_internalMembers removeObjectForKey:message.clientId]; } - [_internalMembers removeObjectForKey:message.clientId]; - return true; } - (void)cleanUpAbsentMembers { diff --git a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h index 16bbf76a4..d0bf7517d 100644 --- a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h @@ -60,13 +60,15 @@ NS_ASSUME_NONNULL_BEGIN - (void)onceSyncFails:(ARTCallback)callback; - (BOOL)addMember:(ARTPresenceMessage *)message; -- (BOOL)addInternalMember:(ARTPresenceMessage *)message; +- (void)addInternalMember:(ARTPresenceMessage *)message; - (BOOL)removeMember:(ARTPresenceMessage *)message; -- (BOOL)removeInternalMember:(ARTPresenceMessage *)message; +- (void)removeInternalMember:(ARTPresenceMessage *)message; - (void)cleanUpAbsentMembers; +- (BOOL)member:(ARTPresenceMessage *)msg1 isNewerThan:(ARTPresenceMessage *)msg2 __attribute__((warn_unused_result)); + @end NS_ASSUME_NONNULL_END diff --git a/Source/include/Ably/ARTPresenceMessage.h b/Source/include/Ably/ARTPresenceMessage.h index 92906248b..a97411136 100644 --- a/Source/include/Ably/ARTPresenceMessage.h +++ b/Source/include/Ably/ARTPresenceMessage.h @@ -52,9 +52,6 @@ NS_ASSUME_NONNULL_BEGIN /// :nodoc: - (BOOL)isEqualToPresenceMessage:(nonnull ARTPresenceMessage *)presence; -/// :nodoc: -- (BOOL)isNewerThan:(ARTPresenceMessage *)latest __attribute__((warn_unused_result)); - @end #pragma mark - ARTEvent diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 5e737d4cd..1dd61eb7b 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1508,7 +1508,7 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(intialPresenceMessage.memberKey(), "\(client.connection.id!):tester") var compareForNewnessMethodCalls = 0 - let hook = ARTPresenceMessage.testSuite_injectIntoClassMethod(#selector(ARTPresenceMessage.isNewerThan(_:))) { + let hook = channel.internal.presence.testSuite_injectIntoMethod(after: #selector(ARTRealtimePresenceInternal.member(_:isNewerThan:))) { compareForNewnessMethodCalls += 1 } @@ -1528,7 +1528,7 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(compareForNewnessMethodCalls, 1) - hook?.remove() + hook.remove() } // RTP2b From 00b04a0b020c6515d1d99dfcd8237f4d0430abe9 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sat, 6 Apr 2024 14:37:52 +0200 Subject: [PATCH 10/16] Put `_beforeSyncMembers` removal in the proper place. --- Source/ARTRealtimePresence.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index b30c37aa6..4ac6b85d6 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -804,6 +804,7 @@ - (void)processMember:(ARTPresenceMessage *)message { case ARTPresenceEnter: case ARTPresenceUpdate: case ARTPresencePresent: + [_beforeSyncMembers removeObjectForKey:message.memberKey]; // RTP19 messageCopy.action = ARTPresencePresent; memberUpdated = [self addMember:messageCopy]; break; @@ -825,7 +826,6 @@ - (void)processMember:(ARTPresenceMessage *)message { else { ARTLogDebug(_logger, @"Presence member \"%@\" with action %@ has been ignored", message.memberKey, ARTPresenceActionToStr(message.action)); } - [_beforeSyncMembers removeObjectForKey:message.memberKey]; // RTP19 } - (BOOL)member:(ARTPresenceMessage *)msg1 isNewerThan:(ARTPresenceMessage *)msg2 { From 56529cca6ebe2f6829c3ac4d4734c2562a3c38f7 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sat, 6 Apr 2024 14:38:14 +0200 Subject: [PATCH 11/16] Marked tests properly. --- 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 1dd61eb7b..63060df74 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -704,7 +704,7 @@ class RealtimeClientPresenceTests: XCTestCase { XCTAssertEqual(channel2.internal.presence.members.count, 2) } - // RTP5f + // RTP5a func test__022__Presence__Channel_state_change_side_effects__channel_enters_the_SUSPENDED_state__all_queued_presence_messages_should_fail_immediately() throws { let test = Test() @@ -740,6 +740,8 @@ class RealtimeClientPresenceTests: XCTestCase { } } + // RTP5f + func test__023__Presence__Channel_state_change_side_effects__channel_enters_the_SUSPENDED_state__members_map_is_preserved_and_only_members_that_changed_between_ATTACHED_states_should_result_in_presence_events() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) From 23b92b5516567dfe211e60d6cdd870be4e5f90c7 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sat, 6 Apr 2024 15:05:09 +0200 Subject: [PATCH 12/16] Update Source/ARTRealtimePresence.m Co-authored-by: sachin shinde --- Source/ARTRealtimePresence.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 4ac6b85d6..a1c2c42ec 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -880,7 +880,7 @@ - (void)addInternalMember:(ARTPresenceMessage *)message { - (void)removeInternalMember:(ARTPresenceMessage *)message { ARTPresenceMessage *existing = [_internalMembers objectForKey:message.clientId]; - if (!existing || [self member:message isNewerThan:existing]) { + if (existing && [self member:message isNewerThan:existing]) { [_internalMembers removeObjectForKey:message.clientId]; } } From 1c4186b7e35c4ac47268b7490f2c8962b06260fa Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 7 Apr 2024 23:35:15 +0200 Subject: [PATCH 13/16] Removed unused property. --- Source/ARTRealtimePresence.m | 3 --- Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h | 1 - 2 files changed, 4 deletions(-) diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index a1c2c42ec..2532ee15b 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -191,7 +191,6 @@ - (instancetype)initWithChannel:(ARTRealtimeChannelInternal *)channel logger:(AR _userQueue = _realtime.rest.userQueue; _queue = _realtime.rest.queue; _pendingPresence = [NSMutableArray array]; - _lastPresenceAction = ARTPresenceAbsent; _logger = logger; _eventEmitter = [[ARTInternalEventEmitter alloc] initWithQueue:_queue]; _dataEncoder = _channel.dataEncoder; @@ -580,8 +579,6 @@ - (void)publishPresence:(ARTPresenceMessage *)msg callback:(ARTCallback)callback return; } - _lastPresenceAction = msg.action; - if (msg.data && _channel.dataEncoder) { ARTDataEncoderOutput *encoded = [_channel.dataEncoder encode:msg.data]; if (encoded.errorInfo) { diff --git a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h index d0bf7517d..e7a4b6c63 100644 --- a/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h @@ -20,7 +20,6 @@ NS_ASSUME_NONNULL_BEGIN - (void)onAttached:(ARTProtocolMessage *)message; @property (nonatomic) dispatch_queue_t queue; -@property (readwrite, nonatomic) ARTPresenceAction lastPresenceAction; @property (readonly, nonatomic) NSMutableArray *pendingPresence; @end From 5fedf5af43055884335eb56e29e9593cd70e11c4 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 7 Apr 2024 23:41:07 +0200 Subject: [PATCH 14/16] Put spec comment for all presence code. --- Source/ARTRealtimeChannel.m | 10 +++---- Source/ARTRealtimePresence.m | 54 ++++++++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/Source/ARTRealtimeChannel.m b/Source/ARTRealtimeChannel.m index fad5bc603..7b226c6b7 100644 --- a/Source/ARTRealtimeChannel.m +++ b/Source/ARTRealtimeChannel.m @@ -578,14 +578,14 @@ - (void)performTransitionToState:(ARTRealtimeChannelState)state withParams:(ARTC break; case ARTRealtimeChannelDetached: self.channelSerial = nil; // RTP5a1 - [self.presence failsSync:params.errorInfo]; + [self.presence failsSync:params.errorInfo]; // RTP5a break; case ARTRealtimeChannelFailed: self.channelSerial = nil; // RTP5a1 self.attachResume = false; [_attachedEventEmitter emit:nil with:params.errorInfo]; [_detachedEventEmitter emit:nil with:params.errorInfo]; - [self.presence failsSync:params.errorInfo]; + [self.presence failsSync:params.errorInfo]; // RTP5a break; default: break; @@ -726,17 +726,17 @@ - (void)detachChannel:(ARTChannelStateChangeParams *)params { if (self.state_nosync == ARTRealtimeChannelDetached) { return; } - [self failPendingPresenceWithState:params.state info:params.errorInfo]; + [self failPendingPresenceWithState:params.state info:params.errorInfo]; // RTP5a [self performTransitionToState:ARTRealtimeChannelDetached withParams:params]; } - (void)setFailed:(ARTChannelStateChangeParams *)params { - [self failPendingPresenceWithState:params.state info:params.errorInfo]; + [self failPendingPresenceWithState:params.state info:params.errorInfo]; // RTP5a [self performTransitionToState:ARTRealtimeChannelFailed withParams:params]; } - (void)setSuspended:(ARTChannelStateChangeParams *)params { - [self failPendingPresenceWithState:params.state info:params.errorInfo]; + [self failPendingPresenceWithState:params.state info:params.errorInfo]; // RTP5f [self performTransitionToState:ARTRealtimeChannelSuspended withParams:params]; } diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 2532ee15b..47e4898ab 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -178,7 +178,7 @@ @implementation ARTRealtimePresenceInternal { ARTPresenceSyncState _syncState; ARTEventEmitter *_syncEventEmitter; - NSMutableDictionary *_members; + NSMutableDictionary *_members; // RTP2 NSMutableDictionary *_internalMembers; // RTP17h NSMutableDictionary *_beforeSyncMembers; // RTP19 @@ -202,6 +202,8 @@ - (instancetype)initWithChannel:(ARTRealtimeChannelInternal *)channel logger:(AR return self; } +// RTP11 + - (void)get:(ARTPresenceMessagesCallback)callback { [self get:[[ARTRealtimePresenceQuery alloc] init] callback:callback]; } @@ -223,7 +225,7 @@ - (void)get:(ARTRealtimePresenceQuery *)query callback:(ARTPresenceMessagesCallb if (callback) callback(nil, [ARTErrorInfo createWithCode:ARTErrorChannelOperationFailedInvalidState message:[NSString stringWithFormat:@"unable to return the list of current members (incompatible channel state: %@)", ARTRealtimeChannelStateToStr(self->_channel.state_nosync)]]); return; case ARTRealtimeChannelSuspended: - if (query && !query.waitForSync) { + if (query && !query.waitForSync) { // RTP11d if (callback) callback(self->_members.allValues, nil); return; } @@ -233,12 +235,13 @@ - (void)get:(ARTRealtimePresenceQuery *)query callback:(ARTPresenceMessagesCallb break; } + // RTP11c BOOL (^filterMemberBlock)(ARTPresenceMessage *message) = ^BOOL(ARTPresenceMessage *message) { return (query.clientId == nil || [message.clientId isEqualToString:query.clientId]) && (query.connectionId == nil || [message.connectionId isEqualToString:query.connectionId]); }; - [self->_channel _attach:^(ARTErrorInfo *error) { + [self->_channel _attach:^(ARTErrorInfo *error) { // RTP11b if (error) { callback(nil, error); return; @@ -263,6 +266,8 @@ - (void)get:(ARTRealtimePresenceQuery *)query callback:(ARTPresenceMessagesCallb }); } +// RTP12 + - (void)history:(ARTPaginatedPresenceCallback)callback { [self history:[[ARTRealtimeHistoryQuery alloc] init] callback:callback error:nil]; } @@ -272,6 +277,8 @@ - (BOOL)history:(ARTRealtimeHistoryQuery *)query callback:(ARTPaginatedPresenceC return [_channel.restChannel.presence history:query callback:callback error:errorPtr]; } +// RTP8 + - (void)enter:(id)data { [self enter:data callback:nil]; } @@ -291,6 +298,8 @@ - (void)enter:(id)data callback:(ARTCallback)cb { }); } +// RTP14, RTP15 + - (void)enterClient:(NSString *)clientId data:(id)data { [self enterClient:clientId data:data callback:nil]; } @@ -324,6 +333,8 @@ - (void)enterWithPresenceMessageId:(NSString *)messageId clientId:(NSString *)cl }); } +// RTP9 + - (void)update:(id)data { [self update:data callback:nil]; } @@ -343,6 +354,8 @@ - (void)update:(id)data callback:(ARTCallback)cb { }); } +// RTP15 + - (void)updateClient:(NSString *)clientId data:(id)data { [self updateClient:clientId data:data callback:nil]; } @@ -386,6 +399,8 @@ - (void)enterOrUpdateAfterChecks:(ARTPresenceAction)action messageId:(NSString * [self publishPresence:msg callback:cb]; } +// RTP10 + - (void)leave:(id)data { [self leave:data callback:nil]; } @@ -413,6 +428,8 @@ - (void)leave:(id)data callback:(ARTCallback)cb { } } +// RTP15 + - (void)leaveClient:(NSString *)clientId data:(id)data { [self leaveClient:clientId data:data callback:nil]; } @@ -449,10 +466,14 @@ - (BOOL)syncComplete { return ret; } +// RTP13 + - (BOOL)syncComplete_nosync { return _syncState == ARTPresenceSyncEnded || _syncState == ARTPresenceSyncFailed; } +// RTP6 + - (ARTEventListener *)subscribe:(ARTPresenceMessageCallback)callback { return [self subscribeWithAttachCallback:nil callback:callback]; } @@ -523,6 +544,8 @@ - (ARTEventListener *)subscribe:(ARTPresenceAction)action onAttach:(ARTCallback) return listener; } +// RTP7 + - (void)unsubscribe { dispatch_sync(_queue, ^{ [self _unsubscribe]; @@ -555,9 +578,9 @@ - (void)addPendingPresence:(ARTProtocolMessage *)msg callback:(ARTStatusCallback - (void)publishPresence:(ARTPresenceMessage *)msg callback:(ARTCallback)callback { if (msg.clientId == nil) { - NSString *authClientId = _realtime.auth.clientId_nosync; + NSString *authClientId = _realtime.auth.clientId_nosync; // RTP8c BOOL connected = _realtime.connection.state_nosync == ARTRealtimeConnected; - if (connected && (authClientId == nil || [authClientId isEqualToString:@"*"])) { + if (connected && (authClientId == nil || [authClientId isEqualToString:@"*"])) { // RTP8j if (callback) { callback([ARTErrorInfo createWithCode:ARTStateNoClientId message:@"Invalid attempt to publish presence message without clientId."]); } @@ -664,7 +687,7 @@ - (void)broadcast:(ARTPresenceMessage *)pm { * by checking that there is nothing after the colon - RTP18b, RTP18c */ - (bool)isLastChannelSerial:(NSString *)channelSerial { - if ([channelSerial isEqualToString:@""]) { + if (!channelSerial || [channelSerial isEqualToString:@""]) { return true; } NSArray *a = [channelSerial componentsSeparatedByString:@":"]; @@ -677,11 +700,11 @@ - (bool)isLastChannelSerial:(NSString *)channelSerial { - (void)onAttached:(ARTProtocolMessage *)message { [self startSync]; if (!message.hasPresence) { - // RTP1 - when an ATTACHED message is received without a HAS_PRESENCE flag, reset PresenceMap + // RTP1 - when an ATTACHED message is received without a HAS_PRESENCE flag, reset PresenceMap (also RTP19a) [self endSync]; ARTLogDebug(self.logger, @"R:%p C:%p (%@) PresenceMap has been reset", _realtime, self, _channel.name); } - [self sendPendingPresence]; + [self sendPendingPresence]; // RTP5b [self reenterInternalMembers]; // RTP17i } @@ -726,7 +749,7 @@ - (void)onSync:(ARTProtocolMessage *)message { [self onMessage:message]; - if ([self isLastChannelSerial:message.channelSerial]) { + if ([self isLastChannelSerial:message.channelSerial]) { // RTP18b, RTP18c [self endSync]; ARTLogDebug(self.logger, @"RT:%p C:%p (%@) PresenceMap sync ended", _realtime, _channel, _channel.name); } @@ -747,7 +770,7 @@ - (void)didRemovedMemberNoLongerPresent:(ARTPresenceMessage *)pm { - (void)reenterInternalMembers { ARTLogDebug(self.logger, @"%p reentering local members", self); for (ARTPresenceMessage *member in [self.internalMembers allValues]) { - [self enterWithPresenceMessageId:member.id clientId:member.clientId data:member.data callback:^(ARTErrorInfo *error) { + [self enterWithPresenceMessageId:member.id clientId:member.clientId data:member.data callback:^(ARTErrorInfo *error) { // RTP17g if (error != nil) { NSString *message = [NSString stringWithFormat:@"Re-entering member \"%@\" is failed with code %ld (%@)", member.memberKey, (long)error.code, error.message]; ARTErrorInfo *reenterError = [ARTErrorInfo createWithCode:ARTErrorUnableToAutomaticallyReEnterPresenceChannel message:message]; @@ -778,7 +801,7 @@ - (void)reenterInternalMembers { - (void)processMember:(ARTPresenceMessage *)message { ARTPresenceMessage *messageCopy = [message copy]; // Internal member - if ([message.connectionId isEqualToString:self.connectionId]) { + if ([message.connectionId isEqualToString:self.connectionId]) { // RTP17b switch (message.action) { case ARTPresenceEnter: case ARTPresenceUpdate: @@ -802,7 +825,7 @@ - (void)processMember:(ARTPresenceMessage *)message { case ARTPresenceUpdate: case ARTPresencePresent: [_beforeSyncMembers removeObjectForKey:message.memberKey]; // RTP19 - messageCopy.action = ARTPresencePresent; + messageCopy.action = ARTPresencePresent; // RTP2d memberUpdated = [self addMember:messageCopy]; break; case ARTPresenceLeave: @@ -810,7 +833,7 @@ - (void)processMember:(ARTPresenceMessage *)message { messageCopy.action = ARTPresenceAbsent; // RTP2f memberUpdated = [self addMember:messageCopy]; } else { - memberUpdated = [self removeMember:messageCopy]; + memberUpdated = [self removeMember:messageCopy]; // RTP2e } break; default: @@ -818,7 +841,7 @@ - (void)processMember:(ARTPresenceMessage *)message { } if (memberUpdated) { - [self broadcast:message]; + [self broadcast:message]; // RTP2g (original action) } else { ARTLogDebug(_logger, @"Presence member \"%@\" with action %@ has been ignored", message.memberKey, ARTPresenceActionToStr(message.action)); @@ -826,7 +849,7 @@ - (void)processMember:(ARTPresenceMessage *)message { } - (BOOL)member:(ARTPresenceMessage *)msg1 isNewerThan:(ARTPresenceMessage *)msg2 { - if ([msg1 isSynthesized] || [msg2 isSynthesized]) { + if ([msg1 isSynthesized] || [msg2 isSynthesized]) { // RTP2b1 return !msg1.timestamp || msg1.timestamp.timeIntervalSince1970 >= msg2.timestamp.timeIntervalSince1970; } @@ -835,6 +858,7 @@ - (BOOL)member:(ARTPresenceMessage *)msg1 isNewerThan:(ARTPresenceMessage *)msg2 NSInteger msg2Serial = [msg2 msgSerialFromId]; NSInteger msg2Index = [msg2 indexFromId]; + // RTP2b2 if (msg1Serial == msg2Serial) { return msg1Index > msg2Index; } From 5f70c2ae735f21a855291dd624e3316a98c0bd94 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 7 Apr 2024 23:42:51 +0200 Subject: [PATCH 15/16] Highlighted with TODO everything that was not implemented according to spec. --- Source/ARTRealtimePresence.m | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index 47e4898ab..f60e48812 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -452,6 +452,7 @@ - (void)leaveClient:(NSString *)clientId data:(id)data callback:(ARTCallback)cb - (void)leaveAfterChecks:(NSString *_Nullable)clientId data:(id)data callback:(ARTCallback)cb { ARTPresenceMessage *msg = [[ARTPresenceMessage alloc] init]; msg.action = ARTPresenceLeave; + // TODO: RTP10a (if the language permits the data argument to be omitted, then the previously set data value will be sent as a convenience) msg.data = data; msg.clientId = clientId; msg.connectionId = _realtime.connection.id_nosync; @@ -619,9 +620,9 @@ - (void)publishPresence:(ARTPresenceMessage *)msg callback:(ARTCallback)callback ARTRealtimeChannelState channelState = _channel.state_nosync; switch (channelState) { case ARTRealtimeChannelInitialized: - case ARTRealtimeChannelDetached: + case ARTRealtimeChannelDetached: // TODO: RTP16b (should fail if DETACHED) [_channel _attach:nil]; - case ARTRealtimeChannelAttaching: { + case ARTRealtimeChannelAttaching: { // TODO: RTP16a (should check `ARTClientOptions.queueMessages`) [self addPendingPresence:pm callback:^(ARTStatus *status) { if (callback) { callback(status.errorInfo); @@ -749,6 +750,7 @@ - (void)onSync:(ARTProtocolMessage *)message { [self onMessage:message]; + // 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); From 81ef883360665f84b3a594551de78bc913b6e062 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 12 Apr 2024 17:15:30 +0200 Subject: [PATCH 16/16] Fail publish presence according to RTP16b/RTP16c (Removed RTP10a TODO as it is not needed, since realtime already sends previously set data value). --- Source/ARTRealtimePresence.m | 39 ++++++++++++-------- Test/Tests/RealtimeClientPresenceTests.swift | 16 ++++---- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/Source/ARTRealtimePresence.m b/Source/ARTRealtimePresence.m index f60e48812..7d47bd006 100644 --- a/Source/ARTRealtimePresence.m +++ b/Source/ARTRealtimePresence.m @@ -15,6 +15,7 @@ #import "ARTBaseMessage+Private.h" #import "ARTProtocolMessage+Private.h" #import "ARTEventEmitter+Private.h" +#import "ARTClientOptions.h" #pragma mark - ARTRealtimePresenceQuery @@ -380,7 +381,7 @@ - (void)enterOrUpdateAfterChecks:(ARTPresenceAction)action messageId:(NSString * case ARTRealtimeChannelDetached: case ARTRealtimeChannelFailed: { if (cb) { - ARTErrorInfo *channelError = [ARTErrorInfo createWithCode:ARTErrorChannelOperationFailedInvalidState message:[NSString stringWithFormat:@"unable to enter presence channel (incompatible channel state: %@)", ARTRealtimeChannelStateToStr(_channel.state_nosync)]]; + ARTErrorInfo *channelError = [ARTErrorInfo createWithCode:ARTErrorUnableToEnterPresenceChannelInvalidState message:[NSString stringWithFormat:@"unable to enter presence channel (incompatible channel state: %@)", ARTRealtimeChannelStateToStr(_channel.state_nosync)]]; cb(channelError); } return; @@ -449,10 +450,9 @@ - (void)leaveClient:(NSString *)clientId data:(id)data callback:(ARTCallback)cb }); } -- (void)leaveAfterChecks:(NSString *_Nullable)clientId data:(id)data callback:(ARTCallback)cb { +- (void)leaveAfterChecks:(NSString *_Nullable)clientId data:(id _Nullable)data callback:(ARTCallback)cb { ARTPresenceMessage *msg = [[ARTPresenceMessage alloc] init]; msg.action = ARTPresenceLeave; - // TODO: RTP10a (if the language permits the data argument to be omitted, then the previously set data value will be sent as a convenience) msg.data = data; msg.clientId = clientId; msg.connectionId = _realtime.connection.id_nosync; @@ -619,28 +619,35 @@ - (void)publishPresence:(ARTPresenceMessage *)msg callback:(ARTCallback)callback ARTRealtimeChannelState channelState = _channel.state_nosync; switch (channelState) { - case ARTRealtimeChannelInitialized: - case ARTRealtimeChannelDetached: // TODO: RTP16b (should fail if DETACHED) - [_channel _attach:nil]; - case ARTRealtimeChannelAttaching: { // TODO: RTP16a (should check `ARTClientOptions.queueMessages`) - [self addPendingPresence:pm callback:^(ARTStatus *status) { - if (callback) { - callback(status.errorInfo); - } - }]; - break; - } case ARTRealtimeChannelAttached: { - [_realtime send:pm sentCallback:nil ackCallback:^(ARTStatus *status) { + [_realtime send:pm sentCallback:nil ackCallback:^(ARTStatus *status) { // RTP16a if (callback) callback(status.errorInfo); }]; break; } + case ARTRealtimeChannelInitialized: + if (_realtime.options.queueMessages) { // RTP16b + [_channel _attach:nil]; + } + // fallthrough + case ARTRealtimeChannelAttaching: { + if (_realtime.options.queueMessages) { // RTP16b + [self addPendingPresence:pm callback:^(ARTStatus *status) { + if (callback) { + callback(status.errorInfo); + } + }]; + break; + } + // else fallthrough + } + // RTP16c case ARTRealtimeChannelSuspended: case ARTRealtimeChannelDetaching: + case ARTRealtimeChannelDetached: case ARTRealtimeChannelFailed: { if (callback) { - ARTErrorInfo *invalidChannelError = [ARTErrorInfo createWithCode:ARTErrorChannelOperationFailedInvalidState message:[NSString stringWithFormat:@"channel operation failed (invalid channel state: %@)", ARTRealtimeChannelStateToStr(channelState)]]; + ARTErrorInfo *invalidChannelError = [ARTErrorInfo createWithCode:ARTErrorUnableToEnterPresenceChannelInvalidState message:[NSString stringWithFormat:@"channel operation failed (invalid channel state: %@)", ARTRealtimeChannelStateToStr(channelState)]]; callback(invalidChannelError); } break; diff --git a/Test/Tests/RealtimeClientPresenceTests.swift b/Test/Tests/RealtimeClientPresenceTests.swift index 63060df74..d259446c2 100644 --- a/Test/Tests/RealtimeClientPresenceTests.swift +++ b/Test/Tests/RealtimeClientPresenceTests.swift @@ -1182,7 +1182,7 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in channel.presence.enter(nil) { error in - XCTAssertEqual(error?.code, ARTErrorCode.channelOperationFailedInvalidState.intValue) + XCTAssertEqual(error?.code, ARTErrorCode.unableToEnterPresenceChannelInvalidState.intValue) done() } } @@ -1208,7 +1208,7 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in channel.presence.enter(nil) { error in - XCTAssertEqual(error?.code, ARTErrorCode.channelOperationFailedInvalidState.intValue) + XCTAssertEqual(error?.code, ARTErrorCode.unableToEnterPresenceChannelInvalidState.intValue) done() } } @@ -1418,7 +1418,7 @@ class RealtimeClientPresenceTests: XCTestCase { } // RTP10a - func skipped__test__044__Presence__leave__should_leave_the_current_client_with_no_data() throws { + func test__044__Presence__leave__should_leave_the_current_client_with_no_data() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) options.clientId = "john" @@ -2065,7 +2065,7 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in channel.presence.update(nil) { error in - XCTAssertEqual(error?.code, ARTErrorCode.channelOperationFailedInvalidState.intValue) + XCTAssertEqual(error?.code, ARTErrorCode.unableToEnterPresenceChannelInvalidState.intValue) done() } } @@ -2086,7 +2086,7 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in channel.presence.update(nil) { error in - XCTAssertEqual(error?.code, ARTErrorCode.channelOperationFailedInvalidState.intValue) + XCTAssertEqual(error?.code, ARTErrorCode.unableToEnterPresenceChannelInvalidState.intValue) done() } } @@ -2298,7 +2298,7 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in channel.presence.enter("online") { error in - XCTAssertEqual(error?.code, ARTErrorCode.channelOperationFailedInvalidState.intValue) + XCTAssertEqual(error?.code, ARTErrorCode.unableToEnterPresenceChannelInvalidState.intValue) done() } } @@ -2330,7 +2330,7 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in channel.presence.enter("online") { error in - XCTAssertEqual(error?.code, ARTErrorCode.channelOperationFailedInvalidState.intValue) + XCTAssertEqual(error?.code, ARTErrorCode.unableToEnterPresenceChannelInvalidState.intValue) done() } } @@ -3103,7 +3103,7 @@ class RealtimeClientPresenceTests: XCTestCase { waitUntil(timeout: testTimeout) { done in // Call: enterClient, updateClient and leaveClient performMethod(channel.presence) { error in - XCTAssertEqual(error?.code, ARTErrorCode.channelOperationFailedInvalidState.intValue) + XCTAssertEqual(error?.code, ARTErrorCode.unableToEnterPresenceChannelInvalidState.intValue) XCTAssertEqual(channel.state, ARTRealtimeChannelState.failed) guard let reason = channel.errorReason else { fail("Reason is empty"); done(); return