From fff0b4db25e081330808bde2783c22c43fbd6181 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Fri, 1 Mar 2019 07:32:34 +0000 Subject: [PATCH 01/19] RTP17b --- Spec/RealtimeClientPresence.swift | 87 +++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/Spec/RealtimeClientPresence.swift b/Spec/RealtimeClientPresence.swift index f57979648..1e433f891 100644 --- a/Spec/RealtimeClientPresence.swift +++ b/Spec/RealtimeClientPresence.swift @@ -2990,6 +2990,93 @@ class RealtimeClientPresence: QuickSpec { } } + // RTP17b + it("should be applied to ENTER, PRESENT or UPDATE events with a connectionId that matches the current client’s connectionId and any LEAVE event with a connectionId that matches the current client’s connectionId and is not a synthesized") { + let options = AblyTests.commonAppSetup() + let client = ARTRealtime(options: options) + defer { client.dispose(); client.close() } + + let channel = client.channels.get("rtp17b") + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) + channel.presence.subscribe(.enter) { presence in + expect(presence.clientId).to(equal("one")) + channel.presence.unsubscribe() + partialDone() + } + channel.presence.enterClient("one", data: nil) { error in + expect(error).to(beNil()) + partialDone() + } + } + + guard let connectionId = client.connection.id else { + fail("connectionId is empty"); return + } + + expect(channel.presenceMap.localMembers).to(haveCount(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.presenceMap.localMembers.add(additionalMember) + done() + }) + expect(client.connection.state).to(equal(.suspended)) + + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) + channel.once(.attached) { stateChange in + expect(stateChange?.reason).to(beNil()) + partialDone() + } + // Reconnect + client.connect() + // Await Sync + channel.presenceMap.onceSyncEnds { _ in + partialDone() + } + } + + expect(channel.presenceMap.syncComplete).toEventually(beTrue(), timeout: testTimeout) + + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) + channel.presence.subscribe(.enter) { presence in + expect(presence.clientId).to(equal("two")) + channel.presence.unsubscribe() + partialDone() + } + if channel.presenceMap.syncComplete { + channel.sync { error in + expect(error).to(beNil()) + channel.presenceMap.onceSyncEnds { _ in + partialDone() + } + } + } + } + + waitUntil(timeout: testTimeout) { done in + channel.presence.get { presences, error in + expect(error).to(beNil()) + guard let presences = presences else { + fail("Presences is nil"); done(); return + } + expect(channel.presenceMap.syncComplete).to(beTrue()) + expect(presences).to(haveCount(2)) + expect(presences.map({$0.clientId})).to(contain(["one", "two"])) + done() + } + } + } + } // RTP15d From da88c56bf65dd390bafb4a70b4f8650557cb7b62 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 7 Mar 2019 11:14:34 +0000 Subject: [PATCH 02/19] Test Suite: HTTPURLResponse allHeaderFields is now case-sensitive Swift Regression https://bugs.swift.org/browse/SR-2429 That's why sometimes some tests were failing because the server responded differently: X-Ably-Errorcode or X-Ably-errorcode, etc. --- Source/ARTConstants.m | 4 ++-- Source/ARTHTTPPaginatedResponse+Private.h | 4 ++++ Source/ARTHTTPPaginatedResponse.h | 4 ++-- Source/ARTHTTPPaginatedResponse.m | 6 ++---- Spec/RestClient.swift | 14 ++++++------ Spec/TestUtilities.swift | 26 +++++++++++++++++++++++ 6 files changed, 43 insertions(+), 15 deletions(-) diff --git a/Source/ARTConstants.m b/Source/ARTConstants.m index c26109d46..438d3c074 100644 --- a/Source/ARTConstants.m +++ b/Source/ARTConstants.m @@ -8,5 +8,5 @@ #import "ARTConstants.h" -NSString *const ARTHttpHeaderFieldErrorCodeKey = @"X-Ably-Errorcode"; -NSString *const ARTHttpHeaderFieldErrorMessageKey = @"X-Ably-Errormessage"; +NSString *const ARTHttpHeaderFieldErrorCodeKey = @"x-ably-errorcode"; +NSString *const ARTHttpHeaderFieldErrorMessageKey = @"x-ably-errormessage"; diff --git a/Source/ARTHTTPPaginatedResponse+Private.h b/Source/ARTHTTPPaginatedResponse+Private.h index 6336f261a..e21760c13 100644 --- a/Source/ARTHTTPPaginatedResponse+Private.h +++ b/Source/ARTHTTPPaginatedResponse+Private.h @@ -8,12 +8,16 @@ #import +#import + @class ARTRest; NS_ASSUME_NONNULL_BEGIN @interface ARTHTTPPaginatedResponse () +@property (nonatomic, strong) NSHTTPURLResponse *response; + + (void)executePaginated:(ARTRest *)rest withRequest:(NSMutableURLRequest *)request andResponseProcessor:(ARTPaginatedResultResponseProcessor)responseProcessor callback:(void (^)(ARTPaginatedResult * _Nullable, ARTErrorInfo * _Nullable))callback UNAVAILABLE_ATTRIBUTE; + (void)executePaginated:(ARTRest *)rest withRequest:(NSMutableURLRequest *)request callback:(void (^)(ARTHTTPPaginatedResponse *_Nullable result, ARTErrorInfo *_Nullable error))callback; diff --git a/Source/ARTHTTPPaginatedResponse.h b/Source/ARTHTTPPaginatedResponse.h index c6de9f6eb..3408ee3ab 100644 --- a/Source/ARTHTTPPaginatedResponse.h +++ b/Source/ARTHTTPPaginatedResponse.h @@ -20,10 +20,10 @@ NS_ASSUME_NONNULL_BEGIN /// Returns true when the HTTP status code indicates success i.e. 200 <= statusCode < 300 @property (nonatomic, readonly) BOOL success; -/// Returns the error code if the X-Ably-Errorcode HTTP header is sent in the response +/// Returns the error code if the X-Ably-ErrorCode HTTP header is sent in the response @property (nonatomic, readonly) NSInteger errorCode; -/// Returns error message if the X-Ably-Errormessage HTTP header is sent in the response +/// Returns error message if the X-Ably-ErrorMessage HTTP header is sent in the response @property (nullable, nonatomic, readonly) NSString *errorMessage; /// Returns a dictionary containing all the HTTP header fields of the response header. diff --git a/Source/ARTHTTPPaginatedResponse.m b/Source/ARTHTTPPaginatedResponse.m index bfd99acca..a7b5d2657 100644 --- a/Source/ARTHTTPPaginatedResponse.m +++ b/Source/ARTHTTPPaginatedResponse.m @@ -6,7 +6,7 @@ // Copyright © 2018 Ably. All rights reserved. // -#import "ARTHTTPPaginatedResponse.h" +#import "ARTHTTPPaginatedResponse+Private.h" #import "ARTHttp.h" #import "ARTAuth.h" @@ -17,9 +17,7 @@ #import "ARTEncoder.h" #import "ARTConstants.h" -@implementation ARTHTTPPaginatedResponse { - NSHTTPURLResponse *_response; -} +@implementation ARTHTTPPaginatedResponse - (instancetype)initWithResponse:(NSHTTPURLResponse *)response items:(NSArray *)items diff --git a/Spec/RestClient.swift b/Spec/RestClient.swift index a64ae1084..1abaee3c9 100644 --- a/Spec/RestClient.swift +++ b/Spec/RestClient.swift @@ -423,7 +423,7 @@ class RestClient: QuickSpec { expect(error).to(beNil()) expect(result).toNot(beNil()) - guard let headerErrorCode = testHTTPExecutor.responses.first?.allHeaderFields["X-Ably-Errorcode"] as? String else { + guard let headerErrorCode = testHTTPExecutor.responses.first?.objc_allHeaderFields["X-Ably-Errorcode"] as? String else { fail("X-Ably-Errorcode not found"); done(); return } @@ -454,7 +454,7 @@ class RestClient: QuickSpec { expect(errorCode).to(equal(40160)) expect(result).to(beNil()) - guard let headerErrorCode = testHTTPExecutor.responses.first?.allHeaderFields["X-Ably-Errorcode"] as? String else { + guard let headerErrorCode = testHTTPExecutor.responses.first?.objc_allHeaderFields["X-Ably-Errorcode"] as? String else { fail("X-Ably-Errorcode not found"); done(); return } @@ -549,7 +549,7 @@ class RestClient: QuickSpec { delay(TimeInterval(truncating: tokenParams.ttl!)) { // [40140, 40150) - token expired and will not recover because authUrl is invalid publishTestMessage(rest) { error in - guard let errorCode = testHTTPExecutor.responses.first?.allHeaderFields["X-Ably-Errorcode"] as? String else { + guard let errorCode = testHTTPExecutor.responses.first?.objc_allHeaderFields["X-Ably-Errorcode"] as? String else { fail("expected X-Ably-Errorcode header in request") return } @@ -598,7 +598,7 @@ class RestClient: QuickSpec { delay(TimeInterval(truncating: tokenParams.ttl!)) { // [40140, 40150) - token expired and will not recover because authUrl is invalid publishTestMessage(rest) { error in - guard let errorCode = testHTTPExecutor.responses.first?.allHeaderFields["X-Ably-Errorcode"] as? String else { + guard let errorCode = testHTTPExecutor.responses.first?.objc_allHeaderFields["X-Ably-Errorcode"] as? String else { fail("expected X-Ably-Errorcode header in request") return } @@ -1617,7 +1617,7 @@ class RestClient: QuickSpec { } expect(response.statusCode) == httpPaginatedResponse.statusCode - expect(response.allHeaderFields as? [String: String]) == httpPaginatedResponse.headers + expect(response.allHeaderFields as NSDictionary) == httpPaginatedResponse.headers } it("should handle response failures") { @@ -1649,7 +1649,7 @@ class RestClient: QuickSpec { expect(paginatedResponse.errorCode) == 40400 expect(paginatedResponse.errorMessage).to(contain("Could not find path")) expect(paginatedResponse.headers).toNot(beEmpty()) - expect(paginatedResponse.headers["X-Ably-Errorcode"]).to(equal("40400")) + expect(paginatedResponse.headers["X-Ably-Errorcode"] as? String).to(equal("40400")) done() } } @@ -1665,7 +1665,7 @@ class RestClient: QuickSpec { } expect(response.statusCode) == 404 - expect(response.allHeaderFields["X-Ably-Errorcode"] as? String).to(equal("40400")) + expect(response.objc_allHeaderFields["X-Ably-Errorcode"] as? String).to(equal("40400")) } } diff --git a/Spec/TestUtilities.swift b/Spec/TestUtilities.swift index 1e8b7e2ea..10249a104 100644 --- a/Spec/TestUtilities.swift +++ b/Spec/TestUtilities.swift @@ -1434,3 +1434,29 @@ extension String { } } } + +extension HTTPURLResponse { + + /*! + @abstract Returns a dictionary containing all the HTTP header fields + of the receiver. + @discussion This is broken since Swift 3. The access is now case-sensitive. + Regression: HTTPURLResponse allHeaderFields is now case-sensitive + https://bugs.swift.org/browse/SR-2429 + @result A dictionary containing all the HTTP header fields of the + receiver. + */ + var objc_allHeaderFields: NSDictionary { + // Disables bridging and calls the Objective-C implementation of the private NSDictionary subclass in CFNetwork directly + return allHeaderFields as NSDictionary + } + +} + +extension ARTHTTPPaginatedResponse { + + var headers: NSDictionary { + return response.objc_allHeaderFields + } + +} From ed64c0195f01b6b9e5c50052999801c1f804c4cd Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 7 Mar 2019 14:56:34 +0000 Subject: [PATCH 03/19] Log with error convenience method --- Source/ARTLog.h | 1 + Source/ARTLog.m | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/Source/ARTLog.h b/Source/ARTLog.h index bfd21a59a..2c0efaf82 100644 --- a/Source/ARTLog.h +++ b/Source/ARTLog.h @@ -26,6 +26,7 @@ typedef NS_ENUM(NSUInteger, ARTLogLevel) { @property (nonatomic, assign) ARTLogLevel logLevel; - (void)log:(NSString *)message withLevel:(ARTLogLevel)level; +- (void)logWithError:(ARTErrorInfo *)error; - (ARTLog *)verboseMode; - (ARTLog *)debugMode; diff --git a/Source/ARTLog.m b/Source/ARTLog.m index b9cf2a7c1..3edeae8a4 100644 --- a/Source/ARTLog.m +++ b/Source/ARTLog.m @@ -149,6 +149,10 @@ - (void)log:(NSString *)message level:(ARTLogLevel)level { }); } +- (void)logWithError:(ARTErrorInfo *)error { + [self log:error.message withLevel:ARTLogLevelError]; +} + - (NSArray *)history { return _history; } From ec3a1062e25285127467906748fd488b04128760 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 7 Mar 2019 16:05:10 +0000 Subject: [PATCH 04/19] RTP17b implementation --- Source/ARTPresenceMap.m | 49 ++++------------------ Source/ARTPresenceMessage+Private.h | 9 +++++ Source/ARTPresenceMessage.h | 2 + Source/ARTPresenceMessage.m | 51 +++++++++++++++++++++++ Source/ARTRealtimeChannel+Private.h | 2 + Source/ARTRealtimeChannel.m | 63 ++++++++++++++++++++++++++--- 6 files changed, 129 insertions(+), 47 deletions(-) diff --git a/Source/ARTPresenceMap.m b/Source/ARTPresenceMap.m index f1b818e6a..db304e6c0 100644 --- a/Source/ARTPresenceMap.m +++ b/Source/ARTPresenceMap.m @@ -78,7 +78,7 @@ - (instancetype)initWithQueue:(_Nonnull dispatch_queue_t)queue logger:(ARTLog *) - (BOOL)add:(ARTPresenceMessage *)message { ARTPresenceMessage *latest = [_members objectForKey:message.memberKey]; - if ([self isNewestPresence:message comparingWith:latest]) { + if ([message isNewerThan:latest]) { ARTPresenceMessage *messageCopy = [message copy]; switch (message.action) { case ARTPresenceEnter: @@ -96,6 +96,7 @@ - (BOOL)add:(ARTPresenceMessage *)message { } return YES; } + [_logger debug:__FILE__ line:__LINE__ message:@"Presence member \"%@\" with action %@ has been ignored", message.memberKey, ARTPresenceActionToStr(message.action)]; latest.syncSessionId = _syncSessionId; return NO; } @@ -108,7 +109,7 @@ - (void)internalAdd:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sess message.syncSessionId = sessionId; [_members setObject:message forKey:message.memberKey]; // Local member - if ([message.connectionId isEqualToString:[self.delegate connectionId]]) { + if ([message.connectionId isEqualToString:self.delegate.connectionId]) { [_localMembers addObject:message]; [_logger debug:__FILE__ line:__LINE__ message:@"local member %@ added", message.memberKey]; } @@ -119,6 +120,10 @@ - (void)internalRemove:(ARTPresenceMessage *)message { } - (void)internalRemove:(ARTPresenceMessage *)message force:(BOOL)force { + if ([message.connectionId isEqualToString:self.delegate.connectionId] && !message.isSynthesized) { + [_localMembers removeObject:message]; + } + if (!force && self.syncInProgress) { message.action = ARTPresenceAbsent; // Should be removed after Sync ends @@ -130,46 +135,6 @@ - (void)internalRemove:(ARTPresenceMessage *)message force:(BOOL)force { } } -- (BOOL)isNewestPresence:(nonnull ARTPresenceMessage *)received comparingWith:(ARTPresenceMessage *)latest __attribute__((warn_unused_result)) { - if (latest == nil) { - return YES; - } - - NSArray *receivedMessageIdParts = [received.id componentsSeparatedByCharactersInSet:[NSCharacterSet characterSetWithCharactersInString:@":"]]; - if (receivedMessageIdParts.count != 3) { - [_logger error:@"Received presence message id is invalid %@", received.id]; - return !received.timestamp || - [latest.timestamp timeIntervalSince1970] <= [received.timestamp timeIntervalSince1970]; - } - NSString *receivedConnectionId = [receivedMessageIdParts objectAtIndex:0]; - NSInteger receivedMsgSerial = [[receivedMessageIdParts objectAtIndex:1] integerValue]; - NSInteger receivedIndex = [[receivedMessageIdParts objectAtIndex:2] integerValue]; - - if ([receivedConnectionId isEqualToString:received.connectionId]) { - NSArray *latestRegisteredIdParts = [latest.id componentsSeparatedByCharactersInSet:[NSCharacterSet characterSetWithCharactersInString:@":"]]; - if (latestRegisteredIdParts.count != 3) { - [_logger error:@"Latest registered presence message id is invalid %@", latest.id]; - return !received.timestamp || - [latest.timestamp timeIntervalSince1970] <= [received.timestamp timeIntervalSince1970]; - } - NSInteger latestRegisteredMsgSerial = [[latestRegisteredIdParts objectAtIndex:1] integerValue]; - NSInteger latestRegisteredIndex = [[latestRegisteredIdParts objectAtIndex:2] integerValue]; - - if (receivedMsgSerial > latestRegisteredMsgSerial) { - return YES; - } - else if (receivedMsgSerial == latestRegisteredMsgSerial && receivedIndex > latestRegisteredIndex) { - return YES; - } - - [_logger debug:__FILE__ line:__LINE__ message:@"Presence member \"%@\" with action %@ has been ignored", received.memberKey, ARTPresenceActionToStr(received.action)]; - return NO; - } - - return !received.timestamp || - [latest.timestamp timeIntervalSince1970] <= [received.timestamp timeIntervalSince1970]; -} - - (void)cleanUpAbsentMembers { NSSet *filteredMembers = [_members keysOfEntriesPassingTest:^BOOL(NSString *key, ARTPresenceMessage *message, BOOL *stop) { return message.action == ARTPresenceAbsent; diff --git a/Source/ARTPresenceMessage+Private.h b/Source/ARTPresenceMessage+Private.h index 78311c131..3fb93d5cc 100644 --- a/Source/ARTPresenceMessage+Private.h +++ b/Source/ARTPresenceMessage+Private.h @@ -12,4 +12,13 @@ @property (readwrite, assign, 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. + */ +- (BOOL)isSynthesized; + +- (nonnull NSArray *)parseId; +- (NSInteger)msgSerialFromId; +- (NSInteger)indexFromId; + @end diff --git a/Source/ARTPresenceMessage.h b/Source/ARTPresenceMessage.h index f009167bb..07a15cc26 100644 --- a/Source/ARTPresenceMessage.h +++ b/Source/ARTPresenceMessage.h @@ -31,6 +31,8 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)isEqualToPresenceMessage:(nonnull ARTPresenceMessage *)presence; +- (BOOL)isNewerThan:(ARTPresenceMessage *)latest __attribute__((warn_unused_result)); + @end #pragma mark - ARTEvent diff --git a/Source/ARTPresenceMessage.m b/Source/ARTPresenceMessage.m index 145ade075..432bb0092 100644 --- a/Source/ARTPresenceMessage.m +++ b/Source/ARTPresenceMessage.m @@ -8,6 +8,9 @@ #import "ARTPresenceMessage+Private.h" +NSString *const ARTPresenceMessageException = @"ARTPresenceMessageException"; +NSString *const ARTAblyMessageInvalidPresenceId = @"Received presence message id is invalid %@"; + @implementation ARTPresenceMessage - (instancetype)init { @@ -52,6 +55,54 @@ - (BOOL)isEqualToPresenceMessage:(ARTPresenceMessage *)presence { return haveEqualConnectionId && haveEqualCliendId; } +- (NSArray *)parseId { + if (!self.id) { + return nil; + } + NSArray *idParts = [self.id componentsSeparatedByCharactersInSet:[NSCharacterSet characterSetWithCharactersInString:@":"]]; + if (idParts.count != 3) { + [ARTException raise:ARTPresenceMessageException format:ARTAblyMessageInvalidPresenceId, self.id]; + } + return idParts; +} + +- (BOOL)isSynthesized { + NSString *connectionId = [[self parseId] objectAtIndex:0]; + return ![connectionId isEqualToString:self.connectionId]; +} + +- (NSInteger)msgSerialFromId { + NSInteger msgSerial = [[[self parseId] objectAtIndex:1] integerValue]; + return msgSerial; +} + +- (NSInteger)indexFromId { + NSInteger index = [[[self parseId] objectAtIndex:2] integerValue]; + return index; +} + +- (BOOL)isNewerThan:(ARTPresenceMessage *)latest { + if (latest == nil) { + return YES; + } + + if ([self isSynthesized] || [latest isSynthesized]) { + return !self.timestamp || [latest.timestamp timeIntervalSince1970] <= [self.timestamp timeIntervalSince1970]; + } + + NSInteger currentMsgSerial = [self msgSerialFromId]; + NSInteger currentIndex = [self indexFromId]; + NSInteger latestMsgSerial = [latest msgSerialFromId]; + NSInteger latestIndex = [latest indexFromId]; + + if (currentMsgSerial == latestMsgSerial) { + return currentIndex > latestIndex; + } + else { + return currentMsgSerial > latestMsgSerial; + } +} + #pragma mark - NSObject - (BOOL)isEqual:(id)object { diff --git a/Source/ARTRealtimeChannel+Private.h b/Source/ARTRealtimeChannel+Private.h index 526f8a397..6a98adcd8 100644 --- a/Source/ARTRealtimeChannel+Private.h +++ b/Source/ARTRealtimeChannel+Private.h @@ -76,6 +76,8 @@ NS_ASSUME_NONNULL_BEGIN - (void)broadcastPresence:(ARTPresenceMessage *)pm; - (void)detachChannel:(ARTStatus *)status; +- (void)sync; +- (void)sync:(nullable void (^)(ARTErrorInfo *_Nullable))callback; - (void)requestContinueSync; @end diff --git a/Source/ARTRealtimeChannel.m b/Source/ARTRealtimeChannel.m index bc614161d..8d1a82278 100644 --- a/Source/ARTRealtimeChannel.m +++ b/Source/ARTRealtimeChannel.m @@ -164,6 +164,59 @@ - (void)internalPostMessages:(id)data callback:(void (^)(ARTErrorInfo *__art_nul } ART_TRY_OR_MOVE_TO_FAILED_END } +- (void)sync { + [self sync:nil]; +} + +- (void)sync:(void (^)(ARTErrorInfo *__art_nullable error))callback { + if (callback) { + void (^userCallback)(ARTErrorInfo *__art_nullable error) = callback; + callback = ^(ARTErrorInfo *__art_nullable error) { + ART_EXITING_ABLY_CODE(self->_realtime.rest); + dispatch_async(self->_userQueue, ^{ + userCallback(error); + }); + }; + } + +ART_TRY_OR_MOVE_TO_FAILED_START(_realtime) { + switch (self.state) { + case ARTRealtimeChannelInitialized: + case ARTRealtimeChannelDetaching: + case ARTRealtimeChannelDetached: { + ARTErrorInfo *error = [ARTErrorInfo createWithCode:40000 message:@"unable to sync to channel; not attached"]; + [self.logger logWithError:error]; + if (callback) callback(error); + return; + } + default: + break; + } + + [self.logger verbose:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) requesting a sync operation", _realtime, self, self.name]; + + ARTProtocolMessage * msg = [[ARTProtocolMessage alloc] init]; + msg.action = ARTProtocolMessageSync; + msg.channel = self.name; + + if (self.presenceMap.syncMsgSerial) { + msg.channelSerial = self.presenceMap.syncChannelSerial; + msg.msgSerial = [NSNumber numberWithLongLong:self.presenceMap.syncMsgSerial]; + } + + [self.realtime send:msg sentCallback:^(ARTErrorInfo *error) { + if (error) { + [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) SYNC request failed with %@", self->_realtime, self, self.name, error]; + callback(error); + } + else { + [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) SYNC requested with success", self->_realtime, self, self.name]; + callback(nil); + } + } ackCallback:nil]; +} ART_TRY_OR_MOVE_TO_FAILED_END +} + - (void)requestContinueSync { ART_TRY_OR_MOVE_TO_FAILED_START(_realtime) { [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) requesting to continue sync operation after reconnect using msgSerial %lld and channelSerial %@", _realtime, self, self.name, self.presenceMap.syncMsgSerial, self.presenceMap.syncChannelSerial]; @@ -174,9 +227,9 @@ - (void)requestContinueSync { msg.channelSerial = self.presenceMap.syncChannelSerial; msg.channel = self.name; - [self.realtime send:msg sentCallback:nil ackCallback:^(ARTStatus *status) { - [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) continue sync, status is %@", self->_realtime, self, self.name, status]; - }]; + [self.realtime send:msg sentCallback:^(ARTErrorInfo *error) { + [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) continue sync, error is %@", self->_realtime, self, self.name, error]; + } ackCallback:nil]; } ART_TRY_OR_MOVE_TO_FAILED_END } @@ -770,11 +823,11 @@ - (void)onPresence:(ARTProtocolMessage *)message { [self.logger error:@"R:%p C:%p (%@) %@", _realtime, self, self.name, errorInfo.message]; } } - + if (!presence.timestamp) { presence.timestamp = message.timestamp; } - + if (!presence.id) { presence.id = [NSString stringWithFormat:@"%@:%d", message.id, i]; } From 8fb2e0c4616a57b7410e1cbcd2f446f905e7fd32 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 7 Mar 2019 16:06:53 +0000 Subject: [PATCH 05/19] Update tests because ARTPresenceMap.isNewestPresence:comparingWith is now ARTPresenceMessage.isNewerThan. --- Spec/RealtimeClientPresence.swift | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Spec/RealtimeClientPresence.swift b/Spec/RealtimeClientPresence.swift index 1e433f891..86d8a7ca1 100644 --- a/Spec/RealtimeClientPresence.swift +++ b/Spec/RealtimeClientPresence.swift @@ -1719,9 +1719,16 @@ class RealtimeClientPresence: QuickSpec { let channel = client.channels.get(channelName) waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) + channel.presence.subscribe { presence in + expect(presence.clientId).to(equal("tester")) + expect(presence.action).to(equal(.enter)) + channel.presence.unsubscribe() + partialDone() + } channel.presence.enterClient("tester", data: nil) { error in expect(error).to(beNil()) - done() + partialDone() } } @@ -1732,7 +1739,7 @@ class RealtimeClientPresence: QuickSpec { expect(intialPresenceMessage.memberKey()).to(equal("\(client.connection.id!):tester")) var compareForNewnessMethodCalls = 0 - let hook = channel.presenceMap.testSuite_injectIntoMethod(after: NSSelectorFromString("isNewestPresence:comparingWith:")) { + let hook = ARTPresenceMessage.testSuite_injectIntoMethod(after: NSSelectorFromString("isNewerThan:")) { compareForNewnessMethodCalls += 1 } defer { hook.remove() } @@ -1749,7 +1756,7 @@ class RealtimeClientPresence: QuickSpec { } expect(intialPresenceMessage.memberKey()).to(equal(updatedPresenceMessage.memberKey())) - expect(intialPresenceMessage.timestamp).toNot(equal(updatedPresenceMessage.timestamp)) + expect(intialPresenceMessage.timestamp).to(beLessThan(updatedPresenceMessage.timestamp)) expect(compareForNewnessMethodCalls) == 1 } From 3f1bf2d2460e3067a727ad403b10729964f5b576 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 7 Mar 2019 16:11:19 +0000 Subject: [PATCH 06/19] [fix] Lib is unnecessarily sending another ATTACH when channel is Suspended. Realtime sends an ATTACHED message after a successful resume. --- Source/ARTRealtime.m | 6 ------ Spec/RealtimeClientChannel.swift | 2 +- Spec/RealtimeClientPresence.swift | 5 +---- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/Source/ARTRealtime.m b/Source/ARTRealtime.m index 15f158606..624d0da06 100644 --- a/Source/ARTRealtime.m +++ b/Source/ARTRealtime.m @@ -550,12 +550,6 @@ - (ARTEventListener *)transitionSideEffects:(ARTConnectionStateChange *)stateCha if ([self shouldSendEvents]) { [self sendQueuedMessages]; - // For every Channel - for (ARTRealtimeChannel* channel in self.channels.nosyncIterable) { - if (channel.state_nosync == ARTRealtimeChannelSuspended) { - [channel _attach:nil]; - } - } } else if (![self shouldQueueEvents]) { ARTStatus *channelStatus = status; if (!channelStatus) { diff --git a/Spec/RealtimeClientChannel.swift b/Spec/RealtimeClientChannel.swift index 204e2fe33..47080b6c8 100644 --- a/Spec/RealtimeClientChannel.swift +++ b/Spec/RealtimeClientChannel.swift @@ -3344,7 +3344,7 @@ class RealtimeClientChannel: QuickSpec { // Check retry let start = NSDate() - channel.once(.attaching) { stateChange in + channel.once(.attached) { stateChange in let end = NSDate() expect(start).to(beCloseTo(end, within: 1.5)) expect(stateChange?.reason).to(beNil()) diff --git a/Spec/RealtimeClientPresence.swift b/Spec/RealtimeClientPresence.swift index 86d8a7ca1..30fb8cd1f 100644 --- a/Spec/RealtimeClientPresence.swift +++ b/Spec/RealtimeClientPresence.swift @@ -1104,15 +1104,12 @@ class RealtimeClientPresence: QuickSpec { expect(channel.presenceMap.localMembers).to(haveCount(1)) partialDone() } - channel.once(.attaching) { stateChange in + channel.once(.attached) { stateChange in expect(stateChange?.reason).to(beNil()) channel.presence.leave(nil) { error in expect(error).to(beNil()) partialDone() } - } - channel.once(.attached) { stateChange in - expect(stateChange?.reason).to(beNil()) partialDone() } channel.setSuspended(ARTStatus.state(.ok)) From f3933f7a0b282dbf110fa90e3eb7496157f74f46 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Fri, 8 Mar 2019 17:49:01 +0000 Subject: [PATCH 07/19] [fix] Check if response body has data Case: Push test was failing because lib was returning an error with 204 status code which shouldn't. --- Source/ARTHttp.m | 1 - Source/ARTRest.m | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/ARTHttp.m b/Source/ARTHttp.m index 1aaeac487..f03e441d4 100644 --- a/Source/ARTHttp.m +++ b/Source/ARTHttp.m @@ -46,7 +46,6 @@ - (void)executeRequest:(NSMutableURLRequest *)request completion:(void (^)(NSHTT [_urlSession get:request completion:^(NSHTTPURLResponse *response, NSData *data, NSError *error) { NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response; - if (error) { [self.logger error:@"%@ %@: error %@", request.HTTPMethod, request.URL.absoluteString, error]; } else { diff --git a/Source/ARTRest.m b/Source/ARTRest.m index a33818ed8..97596a277 100644 --- a/Source/ARTRest.m +++ b/Source/ARTRest.m @@ -270,7 +270,7 @@ - (void)executeRequest:(NSURLRequest *)request completion:(void (^)(NSHTTPURLRes [self.logger debug:__FILE__ line:__LINE__ message:@"RS:%p executing request %@", self, request]; [self.httpExecutor executeRequest:request completion:^(NSHTTPURLResponse *response, NSData *data, NSError *error) { // Error messages in plaintext and HTML format (only if the URL request is different than `options.authUrl` and we don't have an error already) - if (error == nil && data != nil && ![request.URL.host isEqualToString:[self.options.authUrl host]]) { + if (error == nil && data != nil && data.length != 0 && ![request.URL.host isEqualToString:[self.options.authUrl host]]) { NSString *contentType = [response.allHeaderFields objectForKey:@"Content-Type"]; BOOL validContentType = NO; From 8dbc03d9e1ec9776e4fbeb58a2d85469edc9182f Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Wed, 13 Mar 2019 10:41:27 +0000 Subject: [PATCH 08/19] Test Suite: fix [ARTPresenceMessage decodeWithEncoder:error:]: unrecognized selector sent to instance Because of 8fb2e0c4616a57b7410e1cbcd2f446f905e7fd32, there are 2 ARTBaseMessage (ARTPresenceMessage) class method swizzling and Aspects doesn't support that and that's why the error occurs. There are similar issues in the Aspects repo like this one: Aspects/issues/145. I tried the fix of that PR but it didn't work. So, I decided to change all of the class methods swizzling to fix this case and reduce future issues. --- Spec/RealtimeClientChannel.swift | 14 ++++++++++---- Spec/RealtimeClientPresence.swift | 26 ++++++++++++++++++-------- Spec/RestClientPresence.swift | 4 ++-- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/Spec/RealtimeClientChannel.swift b/Spec/RealtimeClientChannel.swift index 47080b6c8..dbad883b5 100644 --- a/Spec/RealtimeClientChannel.swift +++ b/Spec/RealtimeClientChannel.swift @@ -2844,14 +2844,20 @@ class RealtimeClientChannel: QuickSpec { let realtime = ARTRealtime(options: options) defer { realtime.close() } + let channelRest = rest.channels.get("test") + let channelRealtime = realtime.channels.get("test") + var restChannelHistoryMethodWasCalled = false - let hook = ARTRestChannel.testSuite_injectIntoClassMethod(#selector(ARTRestChannel.history(_:callback:))) { + + let hookRest = channelRest.testSuite_injectIntoMethod(after: #selector(ARTRestChannel.history(_:callback:))) { restChannelHistoryMethodWasCalled = true } - defer { hook?.remove() } + defer { hookRest.remove() } - let channelRest = rest.channels.get("test") - let channelRealtime = realtime.channels.get("test") + let hookRealtime = channelRealtime.testSuite_injectIntoMethod(after: #selector(ARTRestChannel.history(_:callback:))) { + restChannelHistoryMethodWasCalled = true + } + defer { hookRealtime.remove() } let queryRealtime = ARTRealtimeHistoryQuery() queryRealtime.start = NSDate() as Date diff --git a/Spec/RealtimeClientPresence.swift b/Spec/RealtimeClientPresence.swift index 30fb8cd1f..e9b1fcfc1 100644 --- a/Spec/RealtimeClientPresence.swift +++ b/Spec/RealtimeClientPresence.swift @@ -10,6 +10,7 @@ import Ably import Quick import Nimble import Foundation +import Aspects class RealtimeClientPresence: QuickSpec { @@ -1736,10 +1737,9 @@ class RealtimeClientPresence: QuickSpec { expect(intialPresenceMessage.memberKey()).to(equal("\(client.connection.id!):tester")) var compareForNewnessMethodCalls = 0 - let hook = ARTPresenceMessage.testSuite_injectIntoMethod(after: NSSelectorFromString("isNewerThan:")) { + let hook = ARTPresenceMessage.testSuite_injectIntoClassMethod(#selector(ARTPresenceMessage.isNewerThan(_:))) { compareForNewnessMethodCalls += 1 } - defer { hook.remove() } waitUntil(timeout: testTimeout) { done in channel.presence.enterClient("tester", data: nil) { error in @@ -1756,6 +1756,8 @@ class RealtimeClientPresence: QuickSpec { expect(intialPresenceMessage.timestamp).to(beLessThan(updatedPresenceMessage.timestamp)) expect(compareForNewnessMethodCalls) == 1 + + hook?.remove() } // RTP2b @@ -2194,9 +2196,10 @@ class RealtimeClientPresence: QuickSpec { fail("TestProxyTransport is not set"); return } + var hook: AspectToken? waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(3, done: done) - channel.presenceMap.testSuite_injectIntoMethod(after: #selector(ARTPresenceMap.startSync)) { + hook = channel.presenceMap.testSuite_injectIntoMethod(after: #selector(ARTPresenceMap.startSync)) { expect(channel.presenceMap.syncInProgress).to(beTrue()) AblyTests.extraQueue.async { @@ -2230,6 +2233,7 @@ class RealtimeClientPresence: QuickSpec { partialDone() } } + hook?.remove() // A single clientId may be present multiple times on the same channel via different client connections and that's way user11 is present because user11 presences messages were in distinct connections. expect(channel.presenceMap.members).to(haveCount(20)) @@ -3882,16 +3886,22 @@ class RealtimeClientPresence: QuickSpec { let rest = ARTRest(options: options) let realtime = ARTRealtime(options: options) - defer { realtime.close() } + defer { realtime.dispose(); realtime.close() } + + let channelRest = rest.channels.get("test") + let channelRealtime = realtime.channels.get("test") var restPresenceHistoryMethodWasCalled = false - var hook = ARTRestPresence.testSuite_injectIntoClassMethod(#selector(ARTRestPresence.history(_:callback:))) { + + var hookRest = channelRest.presence.testSuite_injectIntoMethod(after: #selector(ARTRestPresence.history(_:callback:))) { restPresenceHistoryMethodWasCalled = true } - defer { hook?.remove() } + defer { hookRest.remove() } - let channelRest = rest.channels.get("test") - let channelRealtime = realtime.channels.get("test") + var hookRealtime = channelRealtime.presence.testSuite_injectIntoMethod(after: #selector(ARTRestPresence.history(_:callback:))) { + restPresenceHistoryMethodWasCalled = true + } + defer { hookRealtime.remove() } let queryRealtime = ARTRealtimeHistoryQuery() queryRealtime.start = NSDate() as Date diff --git a/Spec/RestClientPresence.swift b/Spec/RestClientPresence.swift index b9560d462..6a7b57f5c 100644 --- a/Spec/RestClientPresence.swift +++ b/Spec/RestClientPresence.swift @@ -529,10 +529,10 @@ class RestClientPresence: QuickSpec { } var decodeNumberOfCalls = 0 - let hook = ARTBaseMessage.testSuite_injectIntoClassMethod(#selector(ARTBaseMessage.decode)) { + let hook = channel.dataEncoder.testSuite_injectIntoMethod(after: #selector(ARTDataEncoder.decode(_:encoding:))) { decodeNumberOfCalls += 1 } - defer { hook?.remove() } + defer { hook.remove() } waitUntil(timeout: testTimeout) { done in channel.history(checkReceivedMessage(done)) From b50cd73a07022fe1db761a5195111edb55427217 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Fri, 26 Apr 2019 11:47:35 +0100 Subject: [PATCH 09/19] Channel debug log small improvement --- Source/ARTRealtimeChannel.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ARTRealtimeChannel.m b/Source/ARTRealtimeChannel.m index 8d1a82278..89642f1a7 100644 --- a/Source/ARTRealtimeChannel.m +++ b/Source/ARTRealtimeChannel.m @@ -581,7 +581,7 @@ - (void)emit:(ARTChannelEvent)event with:(ARTChannelStateChange *)data { - (void)transition:(ARTRealtimeChannelState)state status:(ARTStatus *)status { ART_TRY_OR_MOVE_TO_FAILED_START(_realtime) { - [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) channel state transitions to %tu - %@", _realtime, self, self.name, state, ARTRealtimeChannelStateToStr(state)]; + [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) channel state transitions from %tu - %@ to %tu - %@", _realtime, self, self.name, self.state_nosync, ARTRealtimeChannelStateToStr(self.state_nosync), state, ARTRealtimeChannelStateToStr(state)]; ARTChannelStateChange *stateChange = [[ARTChannelStateChange alloc] initWithCurrent:state previous:self.state_nosync event:(ARTChannelEvent)state reason:status.errorInfo]; self.state = state; From dc84181a2dce29b47735e751597b509f9318f16d Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Fri, 26 Apr 2019 11:48:02 +0100 Subject: [PATCH 10/19] syncMsgSerial should only be used when requesting a continue sync --- Source/ARTRealtimeChannel.m | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Source/ARTRealtimeChannel.m b/Source/ARTRealtimeChannel.m index 89642f1a7..95a19725d 100644 --- a/Source/ARTRealtimeChannel.m +++ b/Source/ARTRealtimeChannel.m @@ -198,11 +198,7 @@ - (void)sync:(void (^)(ARTErrorInfo *__art_nullable error))callback { ARTProtocolMessage * msg = [[ARTProtocolMessage alloc] init]; msg.action = ARTProtocolMessageSync; msg.channel = self.name; - - if (self.presenceMap.syncMsgSerial) { - msg.channelSerial = self.presenceMap.syncChannelSerial; - msg.msgSerial = [NSNumber numberWithLongLong:self.presenceMap.syncMsgSerial]; - } + msg.channelSerial = self.presenceMap.syncChannelSerial; [self.realtime send:msg sentCallback:^(ARTErrorInfo *error) { if (error) { From ca32ef287b150251bb4972aa45658b4db71137a0 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Fri, 26 Apr 2019 11:48:45 +0100 Subject: [PATCH 11/19] Remove duplicated remove local member instruction --- Source/ARTPresenceMap.m | 1 - 1 file changed, 1 deletion(-) diff --git a/Source/ARTPresenceMap.m b/Source/ARTPresenceMap.m index db304e6c0..809fb4cb8 100644 --- a/Source/ARTPresenceMap.m +++ b/Source/ARTPresenceMap.m @@ -131,7 +131,6 @@ - (void)internalRemove:(ARTPresenceMessage *)message force:(BOOL)force { } else { [_members removeObjectForKey:message.memberKey]; - [_localMembers removeObject:message]; } } From 1ea0e4c01121544d1ac07610628feda13ab559fb Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Mon, 29 Apr 2019 09:34:16 +0100 Subject: [PATCH 12/19] Presense Message debug log small fix --- Source/ARTPresenceMap.m | 2 +- Source/ARTPresenceMessage.m | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/ARTPresenceMap.m b/Source/ARTPresenceMap.m index 809fb4cb8..4ceb5eb8a 100644 --- a/Source/ARTPresenceMap.m +++ b/Source/ARTPresenceMap.m @@ -111,7 +111,7 @@ - (void)internalAdd:(ARTPresenceMessage *)message withSessionId:(NSUInteger)sess // Local member if ([message.connectionId isEqualToString:self.delegate.connectionId]) { [_localMembers addObject:message]; - [_logger debug:__FILE__ line:__LINE__ message:@"local member %@ added", message.memberKey]; + [_logger debug:__FILE__ line:__LINE__ message:@"local member %@ with action %@ has been added", message.memberKey, ARTPresenceActionToStr(message.action).uppercaseString]; } } diff --git a/Source/ARTPresenceMessage.m b/Source/ARTPresenceMessage.m index 432bb0092..2d5719c8e 100644 --- a/Source/ARTPresenceMessage.m +++ b/Source/ARTPresenceMessage.m @@ -34,7 +34,7 @@ - (NSString *)description { NSMutableString *description = [[super description] mutableCopy]; [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:@" action: %lu,\n", (unsigned long)self.action]; [description appendFormat:@" syncSessionId: %lu\n", (unsigned long)self.syncSessionId]; [description appendFormat:@"}"]; return description; From 0c2d2f59ec1c22151915bacced82b4eed281a250 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Mon, 29 Apr 2019 11:03:55 +0100 Subject: [PATCH 13/19] fixup! RTP17b --- Spec/RealtimeClientPresence.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Spec/RealtimeClientPresence.swift b/Spec/RealtimeClientPresence.swift index e9b1fcfc1..27dad39d2 100644 --- a/Spec/RealtimeClientPresence.swift +++ b/Spec/RealtimeClientPresence.swift @@ -3038,6 +3038,8 @@ class RealtimeClientPresence: QuickSpec { }) expect(client.connection.state).to(equal(.suspended)) + expect(channel.presenceMap.localMembers).to(haveCount(2)) + waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) channel.once(.attached) { stateChange in @@ -3054,6 +3056,10 @@ class RealtimeClientPresence: QuickSpec { expect(channel.presenceMap.syncComplete).toEventually(beTrue(), timeout: testTimeout) + // 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. + expect(channel.presenceMap.localMembers).to(haveCount(1)) + waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) channel.presence.subscribe(.enter) { presence in From ec5a7f006c7cac808afc0ad9c99b8ce3f1441cf2 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Mon, 29 Apr 2019 11:05:59 +0100 Subject: [PATCH 14/19] [fix] Should change the sync state after calling sync method --- Source/ARTPresenceMap.m | 14 +++++--------- Source/ARTRealtimeChannel.m | 10 +++++++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Source/ARTPresenceMap.m b/Source/ARTPresenceMap.m index 4ceb5eb8a..589006440 100644 --- a/Source/ARTPresenceMap.m +++ b/Source/ARTPresenceMap.m @@ -170,18 +170,21 @@ - (void)reset { } - (void)startSync { + [_logger debug:__FILE__ line:__LINE__ message:@"%p PresenceMap sync started", self]; _syncSessionId++; _syncState = ARTPresenceSyncStarted; [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:_syncState] with:nil]; } - (void)endSync { + [_logger verbose:__FILE__ line:__LINE__ message:@"%p PresenceMap sync ending", self]; [self cleanUpAbsentMembers]; [self leaveMembersNotPresentInSync]; _syncState = ARTPresenceSyncEnded; [self reenterLocalMembersMissingFromSync]; [_syncEventEmitter emit:[ARTEvent newWithPresenceSyncState:ARTPresenceSyncEnded] with:[_members allValues]]; [_syncEventEmitter off]; + [_logger debug:__FILE__ line:__LINE__ message:@"%p PresenceMap sync ended", self]; } - (void)failsSync:(ARTErrorInfo *)error { @@ -192,18 +195,11 @@ - (void)failsSync:(ARTErrorInfo *)error { } - (void)onceSyncEnds:(void (^)(NSArray *))callback { - if (self.syncInProgress) { - [_syncEventEmitter once:[ARTEvent newWithPresenceSyncState:ARTPresenceSyncEnded] callback:callback]; - } - else { - callback([_members allValues]); - } + [_syncEventEmitter once:[ARTEvent newWithPresenceSyncState:ARTPresenceSyncEnded] callback:callback]; } - (void)onceSyncFails:(void (^)(ARTErrorInfo *))callback { - if (self.syncInProgress) { - [_syncEventEmitter once:[ARTEvent newWithPresenceSyncState:ARTPresenceSyncFailed] callback:callback]; - } + [_syncEventEmitter once:[ARTEvent newWithPresenceSyncState:ARTPresenceSyncFailed] callback:callback]; } - (BOOL)syncComplete { diff --git a/Source/ARTRealtimeChannel.m b/Source/ARTRealtimeChannel.m index 95a19725d..004f84709 100644 --- a/Source/ARTRealtimeChannel.m +++ b/Source/ARTRealtimeChannel.m @@ -195,14 +195,16 @@ - (void)sync:(void (^)(ARTErrorInfo *__art_nullable error))callback { [self.logger verbose:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) requesting a sync operation", _realtime, self, self.name]; - ARTProtocolMessage * msg = [[ARTProtocolMessage alloc] init]; + ARTProtocolMessage *msg = [[ARTProtocolMessage alloc] init]; msg.action = ARTProtocolMessageSync; msg.channel = self.name; msg.channelSerial = self.presenceMap.syncChannelSerial; + [self.presenceMap startSync]; [self.realtime send:msg sentCallback:^(ARTErrorInfo *error) { if (error) { [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) SYNC request failed with %@", self->_realtime, self, self.name, error]; + [self.presenceMap endSync]; callback(error); } else { @@ -223,8 +225,12 @@ - (void)requestContinueSync { msg.channelSerial = self.presenceMap.syncChannelSerial; msg.channel = self.name; + [self.presenceMap startSync]; [self.realtime send:msg sentCallback:^(ARTErrorInfo *error) { [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) continue sync, error is %@", self->_realtime, self, self.name, error]; + if (error) { + [self.presenceMap endSync]; + } } ackCallback:nil]; } ART_TRY_OR_MOVE_TO_FAILED_END } @@ -680,7 +686,6 @@ - (void)setAttached:(ARTProtocolMessage *)message { if (message.hasPresence) { [self.presenceMap startSync]; - [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) PresenceMap sync started", _realtime, self, self.name]; } else if ([self.presenceMap.members count] > 0 || [self.presenceMap.localMembers count] > 0) { if (!message.resumed) { @@ -844,7 +849,6 @@ - (void)onSync:(ARTProtocolMessage *)message { if (!self.presenceMap.syncInProgress) { [self.presenceMap startSync]; - [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) PresenceMap sync started", _realtime, self, self.name]; } for (int i=0; i<[message.presence count]; i++) { From 68b6967050a860af78ea0416a9f78ae48edd252f Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Mon, 29 Apr 2019 11:06:25 +0100 Subject: [PATCH 15/19] Remove redundant code --- Spec/RealtimeClientPresence.swift | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/Spec/RealtimeClientPresence.swift b/Spec/RealtimeClientPresence.swift index 27dad39d2..21958367f 100644 --- a/Spec/RealtimeClientPresence.swift +++ b/Spec/RealtimeClientPresence.swift @@ -201,7 +201,7 @@ class RealtimeClientPresence: QuickSpec { sync1Message.channel = channel.name sync1Message.channelSerial = "sequenceid:cursor" sync1Message.connectionSerial = lastConnectionSerial + 1 - sync1Message.timestamp = NSDate() as Date + 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"), @@ -214,7 +214,7 @@ class RealtimeClientPresence: QuickSpec { sync2Message.channel = channel.name sync2Message.channelSerial = "sequenceid:" //indicates SYNC is complete sync2Message.connectionSerial = lastConnectionSerial + 2 - sync2Message.timestamp = NSDate() as Date + sync2Message.timestamp = Date() sync2Message.presence = [ ARTPresenceMessage(clientId: "a", action: .leave, connectionId: "another", id: "another:1:0"), ] @@ -271,7 +271,7 @@ class RealtimeClientPresence: QuickSpec { syncMessage.action = .sync syncMessage.channel = channel.name syncMessage.connectionSerial = lastConnectionSerial + 1 - syncMessage.timestamp = NSDate() as Date + syncMessage.timestamp = Date() syncMessage.presence = [ ARTPresenceMessage(clientId: "a", action: .present, connectionId: "another", id: "another:0:0"), ARTPresenceMessage(clientId: "b", action: .present, connectionId: "another", id: "another:0:1"), @@ -783,7 +783,7 @@ class RealtimeClientPresence: QuickSpec { waitUntil(timeout: testTimeout*2) { done in let partialDone = AblyTests.splitDone(4, done: done) - let localMember = ARTPresenceMessage(clientId: "local1", action: .enter, connectionId: connectionId, id: "\(connectionId):1:1", timestamp: NSDate() as Date) + let localMember = ARTPresenceMessage(clientId: "local1", action: .enter, connectionId: connectionId, id: "\(connectionId):1:1", timestamp: Date()) channel.once(.attaching) { stateChange in // Local member @@ -971,7 +971,7 @@ class RealtimeClientPresence: QuickSpec { waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(3, done: done) - let localMember = ARTPresenceMessage(clientId: "local1", action: .enter, connectionId: connectionId, id: "\(connectionId):1:1", timestamp: NSDate() as Date) + let localMember = ARTPresenceMessage(clientId: "local1", action: .enter, connectionId: connectionId, id: "\(connectionId):1:1", timestamp: Date()) channel.once(.attaching) { stateChange in // Local member @@ -1804,7 +1804,7 @@ class RealtimeClientPresence: QuickSpec { presenceMessage.action = .presence presenceMessage.channel = protocolMessage.channel presenceMessage.connectionSerial = protocolMessage.connectionSerial + 1 - presenceMessage.timestamp = NSDate() as Date + presenceMessage.timestamp = Date() presenceMessage.presence = presenceData transport.receive(presenceMessage) @@ -1815,7 +1815,7 @@ class RealtimeClientPresence: QuickSpec { endSyncMessage.channel = protocolMessage.channel endSyncMessage.channelSerial = "validserialprefix:" //with no part after the `:` this indicates the end to the SYNC endSyncMessage.connectionSerial = protocolMessage.connectionSerial + 2 - endSyncMessage.timestamp = NSDate() as Date + endSyncMessage.timestamp = Date() transport.afterProcessingReceivedMessage = nil transport.receive(endSyncMessage) @@ -1888,7 +1888,7 @@ class RealtimeClientPresence: QuickSpec { presenceMessage.action = .presence presenceMessage.channel = protocolMessage.channel presenceMessage.connectionSerial = protocolMessage.connectionSerial + 1 - presenceMessage.timestamp = NSDate() as Date + presenceMessage.timestamp = Date() presenceMessage.presence = presenceData transport.receive(presenceMessage) @@ -1899,7 +1899,7 @@ class RealtimeClientPresence: QuickSpec { endSyncMessage.channel = protocolMessage.channel endSyncMessage.channelSerial = "validserialprefix:" //with no part after the `:` this indicates the end to the SYNC endSyncMessage.connectionSerial = protocolMessage.connectionSerial + 2 - endSyncMessage.timestamp = NSDate() as Date + endSyncMessage.timestamp = Date() transport.afterProcessingReceivedMessage = nil transport.receive(endSyncMessage) @@ -2025,7 +2025,7 @@ class RealtimeClientPresence: QuickSpec { injectLeave.action = .leave injectLeave.connectionId = membersConnectionId injectLeave.clientId = "user110" - injectLeave.timestamp = (NSDate() as Date) + 1 + injectLeave.timestamp = (Date()) + 1 protocolMessage.presence?.append(injectLeave) transport.beforeProcessingReceivedMessage = nil partialDone() @@ -3766,7 +3766,7 @@ class RealtimeClientPresence: QuickSpec { let presenceMessage = ARTProtocolMessage() presenceMessage.action = .presence presenceMessage.channel = channel.name - presenceMessage.timestamp = NSDate() as Date + presenceMessage.timestamp = Date() presenceMessage.presence = presenceData transport.receive(presenceMessage) @@ -3834,7 +3834,7 @@ class RealtimeClientPresence: QuickSpec { presenceMessage.action = .presence presenceMessage.channel = protocolMessage.channel presenceMessage.connectionSerial = protocolMessage.connectionSerial + 1 - presenceMessage.timestamp = NSDate() as Date + presenceMessage.timestamp = Date() presenceMessage.presence = presenceData transport.receive(presenceMessage) @@ -3845,7 +3845,7 @@ class RealtimeClientPresence: QuickSpec { endSyncMessage.channel = protocolMessage.channel endSyncMessage.channelSerial = "validserialprefix:" //with no part after the `:` this indicates the end to the SYNC endSyncMessage.connectionSerial = protocolMessage.connectionSerial + 2 - endSyncMessage.timestamp = NSDate() as Date + endSyncMessage.timestamp = Date() transport.afterProcessingReceivedMessage = nil transport.receive(endSyncMessage) @@ -3910,8 +3910,8 @@ class RealtimeClientPresence: QuickSpec { defer { hookRealtime.remove() } let queryRealtime = ARTRealtimeHistoryQuery() - queryRealtime.start = NSDate() as Date - queryRealtime.end = NSDate() as Date + queryRealtime.start = Date() + queryRealtime.end = Date() queryRealtime.direction = .forwards queryRealtime.limit = 50 From 46dd5e7dba21556c5da3cb9a5a64f6cd454fb05e Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 2 May 2019 10:46:10 +0100 Subject: [PATCH 16/19] fixup! RTP17b --- Spec/RealtimeClientPresence.swift | 192 +++++++++++++++++++----------- 1 file changed, 124 insertions(+), 68 deletions(-) diff --git a/Spec/RealtimeClientPresence.swift b/Spec/RealtimeClientPresence.swift index 21958367f..d950ea0bf 100644 --- a/Spec/RealtimeClientPresence.swift +++ b/Spec/RealtimeClientPresence.swift @@ -2999,96 +2999,152 @@ class RealtimeClientPresence: QuickSpec { } // RTP17b - it("should be applied to ENTER, PRESENT or UPDATE events with a connectionId that matches the current client’s connectionId and any LEAVE event with a connectionId that matches the current client’s connectionId and is not a synthesized") { - let options = AblyTests.commonAppSetup() - let client = ARTRealtime(options: options) - defer { client.dispose(); client.close() } + context("events applied to presence map") { - let channel = client.channels.get("rtp17b") - waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(2, done: done) - channel.presence.subscribe(.enter) { presence in - expect(presence.clientId).to(equal("one")) - channel.presence.unsubscribe() - partialDone() - } - channel.presence.enterClient("one", data: nil) { error in - expect(error).to(beNil()) - partialDone() + it("should be applied to ENTER, PRESENT or UPDATE events with a connectionId that matches the current client’s connectionId") { + let options = AblyTests.commonAppSetup() + let client = ARTRealtime(options: options) + defer { client.dispose(); client.close() } + + let channel = client.channels.get("foo") + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) + channel.presence.subscribe(.enter) { presence in + expect(presence.clientId).to(equal("one")) + channel.presence.unsubscribe() + partialDone() + } + channel.presence.enterClient("one", data: nil) { error in + expect(error).to(beNil()) + partialDone() + } } - } - guard let connectionId = client.connection.id else { - fail("connectionId is empty"); return - } + guard let connectionId = client.connection.id else { + fail("connectionId is empty"); return + } - expect(channel.presenceMap.localMembers).to(haveCount(1)) + expect(channel.presenceMap.localMembers).to(haveCount(1)) - let additionalMember = ARTPresenceMessage( - clientId: "two", - action: .enter, - connectionId: connectionId, - id: connectionId + ":0:0" - ) + 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.presenceMap.localMembers.add(additionalMember) - done() - }) - expect(client.connection.state).to(equal(.suspended)) + // Inject an additional member into the myMember set, then force a suspended state + client.simulateSuspended(beforeSuspension: { done in + channel.presenceMap.localMembers.add(additionalMember) + done() + }) + expect(client.connection.state).toEventually(equal(.suspended), timeout: testTimeout) - expect(channel.presenceMap.localMembers).to(haveCount(2)) + expect(channel.presenceMap.localMembers).to(haveCount(2)) - waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(2, done: done) - channel.once(.attached) { stateChange in - expect(stateChange?.reason).to(beNil()) - partialDone() - } - // Reconnect - client.connect() - // Await Sync - channel.presenceMap.onceSyncEnds { _ in - partialDone() + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) + channel.once(.attached) { stateChange in + expect(stateChange?.reason).to(beNil()) + partialDone() + } + // Reconnect + client.connect() + // Await Sync + channel.presenceMap.onceSyncEnds { _ in + partialDone() + } } - } - expect(channel.presenceMap.syncComplete).toEventually(beTrue(), timeout: testTimeout) + expect(channel.presenceMap.syncComplete).toEventually(beTrue(), timeout: testTimeout) - // 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. - expect(channel.presenceMap.localMembers).to(haveCount(1)) + // 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. + expect(channel.presenceMap.localMembers).to(haveCount(1)) - waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(2, done: done) - channel.presence.subscribe(.enter) { presence in - expect(presence.clientId).to(equal("two")) - channel.presence.unsubscribe() - partialDone() + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) + channel.presence.subscribe(.enter) { presence in + expect(presence.clientId).to(equal("two")) + channel.presence.unsubscribe() + partialDone() + } + if channel.presenceMap.syncComplete { + channel.sync { error in + expect(error).to(beNil()) + channel.presenceMap.onceSyncEnds { _ in + partialDone() + } + } + } } - if channel.presenceMap.syncComplete { - channel.sync { error in + + waitUntil(timeout: testTimeout) { done in + channel.presence.get { presences, error in expect(error).to(beNil()) - channel.presenceMap.onceSyncEnds { _ in - partialDone() + guard let presences = presences else { + fail("Presences is nil"); done(); return } + expect(channel.presenceMap.syncComplete).to(beTrue()) + expect(presences).to(haveCount(2)) + expect(presences.map({$0.clientId})).to(contain(["one", "two"])) + done() } } } - waitUntil(timeout: testTimeout) { done in - channel.presence.get { presences, error in - expect(error).to(beNil()) - guard let presences = presences else { - fail("Presences is nil"); done(); return + it("should be applied to any LEAVE event with a connectionId that matches the current client’s connectionId and is not a synthesized") { + let options = AblyTests.commonAppSetup() + let client = ARTRealtime(options: options) + client.suspendImmediateReconnection = true + defer { client.dispose(); client.close() } + + let channel = client.channels.get("foo") + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) + channel.presence.subscribe(.enter) { presence in + expect(presence.clientId).to(equal("one")) + channel.presence.unsubscribe() + partialDone() + } + channel.presence.enterClient("one", data: nil) { error in + expect(error).to(beNil()) + partialDone() + } + } + + waitUntil(timeout: 20) { done in + channel.presenceMap.onceSyncEnds { _ in + // Synthesized leave + expect(channel.presenceMap.localMembers).to(beEmpty()) + done() + } + client.onDisconnected() + } + + waitUntil(timeout: testTimeout) { done in + channel.presence.subscribe(.enter) { presence in + // Re-entering... + expect(presence.clientId).to(equal("one")) + channel.presence.unsubscribe() + done() + } + } + + waitUntil(timeout: testTimeout) { done in + channel.presence.get { presences, error in + expect(error).to(beNil()) + guard let presences = presences else { + fail("Presences is nil"); done(); return + } + expect(channel.presenceMap.syncComplete).to(beTrue()) + expect(presences).to(haveCount(1)) + expect(presences.map({$0.clientId})).to(contain(["one"])) + done() } - expect(channel.presenceMap.syncComplete).to(beTrue()) - expect(presences).to(haveCount(2)) - expect(presences.map({$0.clientId})).to(contain(["one", "two"])) - done() } } + } } From 782ab1ebf3da0267bf0f42592478d704d4e9932a Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Thu, 16 May 2019 18:12:04 +0100 Subject: [PATCH 17/19] [fix] Update Logging test which was improved --- Spec/Utilities.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Spec/Utilities.swift b/Spec/Utilities.swift index 8b6f57f26..a8ad398cb 100644 --- a/Spec/Utilities.swift +++ b/Spec/Utilities.swift @@ -426,7 +426,7 @@ class Utilities: QuickSpec { } expect(realtime.logger.history.count).toNot(beGreaterThan(100)) - expect(realtime.logger.history.map{ $0.message }.first).to(contain("channel state transitions to 2 - Attached")) + expect(realtime.logger.history.map{ $0.message }.first).to(contain("channel state transitions from 1 - Attaching to 2 - Attached")) expect(realtime.logger.history.filter{ $0.message.contains("realtime state transitions to 2 - Connected") }).to(haveCount(1)) } From 0d7cdc0ec60d4a31b19c1a1281dbfbf8a6273cdc Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Tue, 21 May 2019 18:53:32 +0100 Subject: [PATCH 18/19] Sync channelSerial should be unset when the SYNC succeeds. --- Source/ARTRealtimeChannel.m | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/ARTRealtimeChannel.m b/Source/ARTRealtimeChannel.m index f9d0a3bb3..f2d54c321 100644 --- a/Source/ARTRealtimeChannel.m +++ b/Source/ARTRealtimeChannel.m @@ -869,6 +869,7 @@ - (void)onSync:(ARTProtocolMessage *)message { if ([self isLastChannelSerial:message.channelSerial]) { [self.presenceMap endSync]; + self.presenceMap.syncChannelSerial = nil; [self.logger debug:__FILE__ line:__LINE__ message:@"R:%p C:%p (%@) PresenceMap sync ended", _realtime, self, self.name]; } } ART_TRY_OR_MOVE_TO_FAILED_END From 9d2ef9eebac8226d1865d504627fed80ca5bceb9 Mon Sep 17 00:00:00 2001 From: Ricardo Pereira Date: Tue, 21 May 2019 21:24:16 +0100 Subject: [PATCH 19/19] Test suite: avoid some race conditions --- Spec/RealtimeClientPresence.swift | 40 +++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/Spec/RealtimeClientPresence.swift b/Spec/RealtimeClientPresence.swift index d950ea0bf..a649b4a63 100644 --- a/Spec/RealtimeClientPresence.swift +++ b/Spec/RealtimeClientPresence.swift @@ -546,9 +546,15 @@ class RealtimeClientPresence: QuickSpec { defer { client.dispose(); client.close() } let channel = client.channels.get("test") waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) channel.presence.enterClient("user", data: nil) { error in expect(error).to(beNil()) - done() + partialDone() + } + channel.presence.subscribe { message in + expect(message.clientId).to(equal("user")) + channel.presence.unsubscribe() + partialDone() } } @@ -597,9 +603,15 @@ class RealtimeClientPresence: QuickSpec { defer { client.dispose(); client.close() } let channel = client.channels.get("test") waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) channel.presence.enterClient("user", data: nil) { error in expect(error).to(beNil()) - done() + partialDone() + } + channel.presence.subscribe { message in + expect(message.clientId).to(equal("user")) + channel.presence.unsubscribe() + partialDone() } } @@ -862,7 +874,7 @@ class RealtimeClientPresence: QuickSpec { let channel = client.channels.get(channelName) waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(2, done: done) + let partialDone = AblyTests.splitDone(3, done: done) channel.presence.get { members, error in expect(error).to(beNil()) expect(members).to(haveCount(2)) @@ -872,6 +884,12 @@ class RealtimeClientPresence: QuickSpec { expect(error).to(beNil()) partialDone() } + channel.presence.subscribe(.enter) { message in + if message.clientId == "local1" { + channel.presence.unsubscribe() + partialDone() + } + } } expect(channel.presenceMap.members).to(haveCount(3)) @@ -1081,7 +1099,7 @@ class RealtimeClientPresence: QuickSpec { defer { client.dispose(); client.close() } let channel = client.channels.get(channelName) waitUntil(timeout: testTimeout) { done in - let partialDone = AblyTests.splitDone(2, done: done) + let partialDone = AblyTests.splitDone(3, done: done) channel.presence.enter(nil) { error in expect(error).to(beNil()) partialDone() @@ -1091,6 +1109,12 @@ class RealtimeClientPresence: QuickSpec { expect(members).to(haveCount(3)) partialDone() } + channel.presence.subscribe(.enter) { message in + if message.clientId == "tester" { + channel.presence.unsubscribe() + partialDone() + } + } } waitUntil(timeout: testTimeout) { done in @@ -1355,9 +1379,15 @@ class RealtimeClientPresence: QuickSpec { let channel = client.channels.get("test") waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(2, done: done) channel.presence.enter("online") { error in expect(error).to(beNil()) - done() + partialDone() + } + channel.presence.subscribe { message in + expect(message.clientId).to(equal("john")) + channel.presence.unsubscribe() + partialDone() } }