Skip to content

Commit

Permalink
Moved the rest of the presence code into presence object.
Browse files Browse the repository at this point in the history
I've split presence `onAttached:` into two parts, because:

with this variance:

```
[_attachedEventEmitter emit:nil with:nil];
[self.presence onAttached:message fromState:state];
```

the test "test__105__Presence__get..." fails because it needs sync to begin (within `presence onAttached:`) before attach event emitted, otherwise the logic inside `channel.presence.get { ... }` takes another path and calls callback without an error.

with opposite order:

```
[self.presence onAttached:message fromState:state];
[_attachedEventEmitter emit:nil with:nil];
```
the test "test__020__Presence__Channel..." fails because it sends pending presence messages before the `detach` call has an oportunity to fail those messages.

So those tests are kinda mutually exclusive.
  • Loading branch information
maratal committed Feb 25, 2024
1 parent ddf12a4 commit 1664da5
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 53 deletions.
40 changes: 7 additions & 33 deletions Source/ARTRealtimeChannel.m
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,8 @@ - (void)onChannelMessage:(ARTProtocolMessage *)message {
}

- (void)setAttached:(ARTProtocolMessage *)message {
switch (self.state_nosync) {
ARTRealtimeChannelState state = self.state_nosync;
switch (state) {
case ARTRealtimeChannelDetaching:
case ARTRealtimeChannelFailed:
// Ignore
Expand All @@ -673,24 +674,15 @@ - (void)setAttached:(ARTProtocolMessage *)message {
self.serial = message.channelSerial;
}

if (message.hasPresence) {
[self.presence startSync];
}
else {
// RTP1 - when an ATTACHED message is received without a HAS_PRESENCE flag, reset PresenceMap
[self.presence startSync];
[self.presence endSync];
ARTLogDebug(self.logger, @"R:%p C:%p (%@) PresenceMap has been reset", _realtime, self, self.name);
}

if (self.state_nosync == ARTRealtimeChannelAttached) {
[self.presence beforeAttached:message];

if (state == ARTRealtimeChannelAttached) {
if (!message.resumed) { // RTL12
if (message.error != nil) {
_errorReason = message.error;
}
ARTChannelStateChange *stateChange = [[ARTChannelStateChange alloc] initWithCurrent:self.state_nosync previous:self.state_nosync event:ARTChannelEventUpdate reason:message.error resumed:message.resumed];
ARTChannelStateChange *stateChange = [[ARTChannelStateChange alloc] initWithCurrent:state previous:state event:ARTChannelEventUpdate reason:message.error resumed:message.resumed];
[self emit:stateChange.event with:stateChange];
[self reenterLocalMembers]; // RTP17i
}
return;
}
Expand All @@ -704,8 +696,7 @@ - (void)setAttached:(ARTProtocolMessage *)message {
[self performTransitionToState:ARTRealtimeChannelAttached withParams:params];
[_attachedEventEmitter emit:nil with:nil];

[self.presence sendPendingPresence];
[self reenterLocalMembers]; // RTP17i
[self.presence afterAttached:message fromState:state];
}

- (void)setDetached:(ARTProtocolMessage *)message {
Expand Down Expand Up @@ -1084,23 +1075,6 @@ - (void)startDecodeFailureRecoveryWithErrorInfo:(ARTErrorInfo *)error {
} withParams:params];
}

- (void)reenterLocalMembers {
[self.presence reenterLocalMembersWithMemberCallback:^(ARTPresenceMessage *member, ARTErrorInfo *error) {
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];
ARTChannelStateChange *stateChange = [[ARTChannelStateChange alloc] initWithCurrent:self.state_nosync previous:self.state_nosync event:ARTChannelEventUpdate reason:reenterError resumed:true]; // RTP17e

[self emit:stateChange.event with:stateChange];

ARTLogWarn(self.logger, @"RT:%p C:%p (%@) Re-entering member \"%@\" is failed with code %ld (%@)", self->_realtime, self, self.name, member.memberKey, (long)error.code, error.message);
}
else {
ARTLogDebug(self.logger, @"RT:%p C:%p (%@) re-entered local member \"%@\"", self->_realtime, self, self.name, member.memberKey);
}
}];
}

- (NSString *)connectionId {
return _realtime.connection.id_nosync;
}
Expand Down
66 changes: 49 additions & 17 deletions Source/ARTRealtimePresence.m
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#import "ARTEventEmitter+Private.h"
#import "ARTDataEncoder.h"
#import "ARTBaseMessage+Private.h"
#import "ARTProtocolMessage+Private.h"
#import "ARTEventEmitter+Private.h"

#pragma mark - ARTRealtimePresenceQuery

Expand Down Expand Up @@ -662,17 +664,6 @@ - (void)broadcast:(ARTPresenceMessage *)pm {
[_eventEmitter emit:[ARTEvent newWithPresenceAction:pm.action] with:pm];
}

- (void)reenterLocalMembersWithMemberCallback:(ARTPresenceMessageErrorCallback)callback {
ARTLogDebug(self.logger, @"%p reentering local members", self);
for (ARTPresenceMessage *member in [self.localMembers allValues]) {
[self enterWithPresenceMessageId:member.id clientId:member.clientId data:member.data callback:^(ARTErrorInfo *error) {
callback(member, error);
}];
ARTLogDebug(self.logger, @"RT:%p C:%p (%@) re-entering local member \"%@\"", _realtime, _channel, _channel.name, member.memberKey);
}
[self cleanUpAbsentMembers];
}

- (void)sync {
[self sync:nil];
}
Expand Down Expand Up @@ -722,6 +713,30 @@ - (void)sync:(ARTCallback)callback {
} ackCallback:nil];
}

- (void)beforeAttached:(ARTProtocolMessage *)message {
if (message.hasPresence) {
[self startSync];
}
else {
// RTP1 - when an ATTACHED message is received without a HAS_PRESENCE flag, reset PresenceMap
[self startSync];
[self endSync];
ARTLogDebug(self.logger, @"R:%p C:%p (%@) PresenceMap has been reset", _realtime, self, _channel.name);
}
}

- (void)afterAttached:(ARTProtocolMessage *)message fromState:(ARTRealtimeChannelState)state {
if (state == ARTRealtimeChannelAttached) {

This comment has been minimized.

Copy link
@sacOO7

sacOO7 Feb 27, 2024

Contributor

if (state == ARTRealtimeChannelAttached) will always be true in case of afterAttached. Can we implement it like we do in ably-go. I don;t think we need to maintain two different methods beforeAttached and afterAttached as such.

This comment has been minimized.

Copy link
@sacOO7

sacOO7 Feb 27, 2024

Contributor

I feel you can just copy the logic as is from ably-go code.

This comment has been minimized.

Copy link
@maratal

maratal Feb 28, 2024

Author Collaborator

if (state == ARTRealtimeChannelAttached) will always be true in case of afterAttached

No, it's fromState, which means previous state.

if (!message.resumed) { // RTL12
[self reenterLocalMembers]; // RTP17i
}
}
else {
[self sendPendingPresence];
[self reenterLocalMembers]; // RTP17i
}
}

- (void)onMessage:(ARTProtocolMessage *)message {
int i = 0;
for (ARTPresenceMessage *p in message.presence) {
Expand Down Expand Up @@ -762,12 +777,7 @@ - (void)onSync:(ARTProtocolMessage *)message {
ARTLogDebug(self.logger, @"RT:%p C:%p (%@) PresenceMap sync is in progress", _realtime, _channel, _channel.name);
}

for (int i = 0; i < [message.presence count]; i++) {
ARTPresenceMessage *presence = [message.presence objectAtIndex:i];
if ([self add:presence]) {
[self broadcast:presence];
}
}
[self onMessage:message];

if ([_channel isLastChannelSerial:message.channelSerial]) {
[self endSync];
Expand All @@ -788,6 +798,28 @@ - (void)didRemovedMemberNoLongerPresent:(ARTPresenceMessage *)pm {
ARTLogDebug(self.logger, @"RT:%p C:%p (%@) member \"%@\" no longer present", _realtime, _channel, _channel.name, pm.memberKey);
}

- (void)reenterLocalMembers {
ARTLogDebug(self.logger, @"%p reentering local members", self);
for (ARTPresenceMessage *member in [self.localMembers allValues]) {
[self enterWithPresenceMessageId:member.id clientId:member.clientId data:member.data callback:^(ARTErrorInfo *error) {
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];
ARTChannelStateChange *stateChange = [[ARTChannelStateChange alloc] initWithCurrent:self->_channel.state_nosync previous:self->_channel.state_nosync event:ARTChannelEventUpdate reason:reenterError resumed:true]; // RTP17e

[self->_channel emit:stateChange.event with:stateChange];

ARTLogWarn(self.logger, @"RT:%p C:%p (%@) Re-entering member \"%@\" is failed with code %ld (%@)", self->_realtime, self->_channel, self->_channel.name, member.memberKey, (long)error.code, error.message);
}
else {
ARTLogDebug(self.logger, @"RT:%p C:%p (%@) re-entered local member \"%@\"", self->_realtime, self->_channel, self->_channel.name, member.memberKey);
}
}];
ARTLogDebug(self.logger, @"RT:%p C:%p (%@) re-entering local member \"%@\"", _realtime, _channel, _channel.name, member.memberKey);
}
[self cleanUpAbsentMembers];
}

#pragma mark - Presence Map

- (NSDictionary<NSString *, ARTPresenceMessage *> *)members {
Expand Down
2 changes: 2 additions & 0 deletions Source/PrivateHeaders/Ably/ARTRealtimeChannel+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ NS_ASSUME_NONNULL_BEGIN

- (void)detachChannel:(ARTChannelStateChangeParams *)params;

- (void)emit:(ARTChannelEvent)event with:(ARTChannelStateChange *)data;

@end

@interface ARTRealtimeChannel ()
Expand Down
6 changes: 3 additions & 3 deletions Source/PrivateHeaders/Ably/ARTRealtimePresence+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ NS_ASSUME_NONNULL_BEGIN
- (void)_unsubscribe;
- (BOOL)syncComplete_nosync;

- (void)sendPendingPresence;
- (void)failPendingPresence:(ARTStatus *)status;

- (void)broadcast:(ARTPresenceMessage *)pm;
- (void)reenterLocalMembersWithMemberCallback:(ARTPresenceMessageErrorCallback)callback;

- (void)sync;
- (void)sync:(nullable ARTCallback)callback;

- (void)onMessage:(ARTProtocolMessage *)message;
- (void)onSync:(ARTProtocolMessage *)message;

- (void)beforeAttached:(ARTProtocolMessage *)message;

This comment has been minimized.

Copy link
@sacOO7

sacOO7 Feb 27, 2024

Contributor

No need to maintain two different methods. Just one onAttached method should be enough.

This comment has been minimized.

Copy link
@maratal

maratal Feb 28, 2024

Author Collaborator

Tests are very sensitive to the order of calls in setAttached: in the channel object. This is the best setup I was able to come up with so far.

- (void)afterAttached:(ARTProtocolMessage *)message fromState:(ARTRealtimeChannelState)state;

@property (nonatomic) dispatch_queue_t queue;
@property (readwrite, nonatomic) ARTPresenceAction lastPresenceAction;
@property (readonly, nonatomic) NSMutableArray<ARTQueuedMessage *> *pendingPresence;
Expand Down

0 comments on commit 1664da5

Please sign in to comment.