Skip to content

Commit

Permalink
Revert 26d9bf7
Browse files Browse the repository at this point in the history
That change, which moved the `presence` property from the
ARTRealtimeChannel class to ARTRealtimeChannelProtocol, changed the type
of the property from ARTRealtimePresence to
id<ARTRealtimePresenceProtocol>. This is a breaking change for anybody
who was relying on the property returning the concrete type, one such
example being our Asset Tracking SDK. So, we undo it.

The change made in 26d9bf7 was triggered by us noticing, whilst working
on the Chat SDK — which does its best to avoid referring to ably-cocoa's
concrete types — that when you have an ably-cocoa concrete type
ConcreteA which implements a protocol ProtocolA, and where ConcreteA has
a public method or property `someMethod` which returns an instance of
another concrete type ConcreteB which happens to implement a protocol
ConcreteB, then `someMethod` is always defined directly on ConcreteA,
not on ProtocolA. We wondered "why can't these methods be on ProtocolA
instead?", hence 26d9bf7. But the first reason is that we can't because
in order to do this it makes sense for you to also change the the return
type of `someMethod` from ConcreteB to ProtocolB, which is a breaking
change as we've seen here. But also I'm not sure whether it'd work even
if we accepted the breaking change; for example, I don't know if you can
mark a protocol as Sendable in Objective-C (haven't looked into it since
we currently have no intention of making the breaking change, but one to
investigate if we consider it in the future).

Resolves #2003.
  • Loading branch information
lawrence-forooghian committed Jan 16, 2025
1 parent 5ba7809 commit e216ae5
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 138 deletions.
30 changes: 15 additions & 15 deletions Source/ARTRealtimeChannel.m
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ - (ARTErrorInfo *)errorReason {
return _internal.errorReason;
}

- (id<ARTRealtimePresenceProtocol>)presence {
return [[ARTRealtimePresence alloc] initWithInternal:_internal.internalPresence queuedDealloc:_dealloc];
- (ARTRealtimePresence *)presence {
return [[ARTRealtimePresence alloc] initWithInternal:_internal.presence queuedDealloc:_dealloc];
}

#if TARGET_OS_IPHONE
Expand Down Expand Up @@ -224,7 +224,7 @@ - (void)setOptions:(ARTRealtimeChannelOptions *_Nullable)options callback:(nulla
@end

@interface ARTRealtimeChannelInternal () {
ARTRealtimePresenceInternal *_internalPresence;
ARTRealtimePresenceInternal *_realtimePresence;
#if TARGET_OS_IPHONE
ARTPushChannelInternal *_pushChannel;
#endif
Expand Down Expand Up @@ -263,7 +263,7 @@ - (instancetype)initWithRealtime:(ARTRealtimeInternal *)realtime andName:(NSStri
_restChannel = [_realtime.rest.channels _getChannel:self.name options:options addPrefix:true];
_state = ARTRealtimeChannelInitialized;
_attachSerial = nil;
_internalPresence = [[ARTRealtimePresenceInternal alloc] initWithChannel:self logger:self.logger];
_realtimePresence = [[ARTRealtimePresenceInternal alloc] initWithChannel:self logger:self.logger];
_statesEventEmitter = [[ARTPublicEventEmitter alloc] initWithRest:_realtime.rest logger:logger];
_messagesEventEmitter = [[ARTInternalEventEmitter alloc] initWithQueues:_queue userQueue:_userQueue];
_attachedEventEmitter = [[ARTInternalEventEmitter alloc] initWithQueue:_queue];
Expand Down Expand Up @@ -324,8 +324,8 @@ - (ARTErrorInfo *)errorReason_nosync {
return _errorReason;
}

- (id<ARTRealtimePresenceProtocol>)presence {
return _internalPresence;
- (ARTRealtimePresenceInternal *)presence {
return _realtimePresence;
}

#if TARGET_OS_IPHONE
Expand Down Expand Up @@ -567,14 +567,14 @@ - (void)performTransitionToState:(ARTRealtimeChannelState)state withParams:(ARTC
break;
case ARTRealtimeChannelDetached:
self.channelSerial = nil; // RTP5a1
[self.internalPresence failsSync:params.errorInfo]; // RTP5a
[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.internalPresence failsSync:params.errorInfo]; // RTP5a
[self.presence failsSync:params.errorInfo]; // RTP5a
break;
default:
break;
Expand Down Expand Up @@ -658,7 +658,7 @@ - (void)setAttached:(ARTProtocolMessage *)message {
}
ARTChannelStateChange *stateChange = [[ARTChannelStateChange alloc] initWithCurrent:state previous:state event:ARTChannelEventUpdate reason:message.error resumed:message.resumed];
[self emit:stateChange.event with:stateChange];
[self.internalPresence onAttached:message];
[self.presence onAttached:message];
}
return;
}
Expand All @@ -671,7 +671,7 @@ - (void)setAttached:(ARTProtocolMessage *)message {
}
params.resumed = message.resumed;
[self performTransitionToState:ARTRealtimeChannelAttached withParams:params];
[self.internalPresence onAttached:message];
[self.presence onAttached:message];
[_attachedEventEmitter emit:nil with:nil];
}

Expand Down Expand Up @@ -709,7 +709,7 @@ - (void)setDetached:(ARTProtocolMessage *)message {

- (void)failPendingPresenceWithState:(ARTState)state info:(nullable ARTErrorInfo *)info {
ARTStatus *const status = [ARTStatus state:state info:info];
[self.internalPresence failPendingPresence:status];
[self.presence failPendingPresence:status];
}

- (void)detachChannel:(ARTChannelStateChangeParams *)params {
Expand Down Expand Up @@ -804,11 +804,11 @@ - (void)onPresence:(ARTProtocolMessage *)message {
if (message.channelSerial) {
self.channelSerial = message.channelSerial;
}
[self.internalPresence onMessage:message];
[self.presence onMessage:message];
}

- (void)onSync:(ARTProtocolMessage *)message {
[self.internalPresence onSync:message];
[self.presence onSync:message];
}

- (void)onError:(ARTProtocolMessage *)msg {
Expand Down Expand Up @@ -1026,8 +1026,8 @@ - (void)detachAfterChecks {
[self->_detachedEventEmitter emit:nil with:errorInfo];
}] startTimer];

if (self.internalPresence.syncInProgress_nosync) {
[self.internalPresence failsSync:[ARTErrorInfo createWithCode:ARTErrorChannelOperationFailed message:@"channel is being DETACHED"]];
if (self.presence.syncInProgress_nosync) {
[self.presence failsSync:[ARTErrorInfo createWithCode:ARTErrorChannelOperationFailed message:@"channel is being DETACHED"]];
}
}

Expand Down
2 changes: 1 addition & 1 deletion Source/ARTRealtimeChannels.m
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ - (void)release:(NSString *)name callback:(ARTCallback)cb {
[channel _detach:^(ARTErrorInfo *errorInfo) {
[channel off_nosync];
[channel _unsubscribe];
[channel.internalPresence _unsubscribe];
[channel.presence _unsubscribe];

// Only release if the stored channel now is the same as whne.
// Otherwise, subsequent calls to this release method race, and
Expand Down
2 changes: 1 addition & 1 deletion Source/PrivateHeaders/Ably/ARTRealtimeChannel+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ NS_ASSUME_NONNULL_BEGIN

@interface ARTRealtimeChannelInternal : ARTChannel <ARTRealtimeChannelProtocol>

@property (readonly) ARTRealtimePresenceInternal *internalPresence;
@property (readonly) ARTRealtimePresenceInternal *presence;
#if TARGET_OS_IPHONE
@property (readonly) ARTPushChannelInternal *push;
#endif
Expand Down
10 changes: 5 additions & 5 deletions Source/include/Ably/ARTRealtimeChannel.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

NS_ASSUME_NONNULL_BEGIN

@class ARTRealtimePresence;
@class ARTRealtimeChannelOptions;
@class ARTChannelProperties;
#if TARGET_OS_IPHONE
Expand Down Expand Up @@ -38,11 +39,6 @@ NS_ASSUME_NONNULL_BEGIN
/// :nodoc: TODO: docstring
@property (readonly, nullable, getter=getOptions) ARTRealtimeChannelOptions *options;

/**
* An `ARTRealtimePresence` object.
*/
@property (readonly) id<ARTRealtimePresenceProtocol> presence;

/**
* A shortcut for the `-[ARTRealtimeChannelProtocol attach:]` method.
*/
Expand Down Expand Up @@ -184,6 +180,10 @@ ART_EMBED_INTERFACE_EVENT_EMITTER(ARTChannelEvent, ARTChannelStateChange *)
NS_SWIFT_SENDABLE
@interface ARTRealtimeChannel : NSObject <ARTRealtimeChannelProtocol>

/**
* An `ARTRealtimePresence` object.
*/
@property (readonly) ARTRealtimePresence *presence;
#if TARGET_OS_IPHONE
/**
* An `ARTPushChannel` object.
Expand Down
1 change: 1 addition & 0 deletions Source/include/Ably/ARTRealtimePresence.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#import <Ably/ARTRestPresence.h>
#import <Ably/ARTDataQuery.h>
#import <Ably/ARTEventEmitter.h>
#import <Ably/ARTRealtimeChannel.h>

NS_ASSUME_NONNULL_BEGIN

Expand Down
22 changes: 11 additions & 11 deletions Test/Tests/RealtimeClientChannelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,14 @@ class RealtimeClientChannelTests: XCTestCase {
channel2.attach()

XCTAssertFalse(channel2.presence.syncComplete)
XCTAssertEqual(channel1.internal.internalPresence.members.count, 1)
XCTAssertEqual(channel2.internal.internalPresence.members.count, 0)
XCTAssertEqual(channel1.internal.presence.members.count, 1)
XCTAssertEqual(channel2.internal.presence.members.count, 0)
}

expect(channel2.presence.syncComplete).toEventually(beTrue(), timeout: testTimeout)

XCTAssertEqual(channel1.internal.internalPresence.members.count, 1)
expect(channel2.internal.internalPresence.members).toEventually(haveCount(1), timeout: testTimeout)
XCTAssertEqual(channel1.internal.presence.members.count, 1)
expect(channel2.internal.presence.members).toEventually(haveCount(1), timeout: testTimeout)

waitUntil(timeout: testTimeout) { done in
channel1.publish("message", data: nil) { errorInfo in
Expand All @@ -212,14 +212,14 @@ class RealtimeClientChannelTests: XCTestCase {
}
}

expect(channel1.internal.internalPresence.members).toEventually(haveCount(2), timeout: testTimeout)
expect(channel1.internal.internalPresence.members.keys).to(allPass { $0.hasPrefix("\(channel1.internal.connectionId):Client") || $0.hasPrefix("\(channel2.internal.connectionId):Client") })
expect(channel1.internal.internalPresence.members.values).to(allPass { $0.action == .present })
expect(channel1.internal.presence.members).toEventually(haveCount(2), timeout: testTimeout)
expect(channel1.internal.presence.members.keys).to(allPass { $0.hasPrefix("\(channel1.internal.connectionId):Client") || $0.hasPrefix("\(channel2.internal.connectionId):Client") })
expect(channel1.internal.presence.members.values).to(allPass { $0.action == .present })

expect(channel2.internal.internalPresence.members).toEventually(haveCount(2), timeout: testTimeout)
expect(channel2.internal.internalPresence.members.keys).to(allPass { $0.hasPrefix("\(channel1.internal.connectionId):Client") || $0.hasPrefix("\(channel2.internal.connectionId):Client") })
XCTAssertEqual(channel2.internal.internalPresence.members["\(channel1.internal.connectionId):Client 1"]!.action, ARTPresenceAction.present)
XCTAssertEqual(channel2.internal.internalPresence.members["\(channel2.internal.connectionId):Client 2"]!.action, ARTPresenceAction.present)
expect(channel2.internal.presence.members).toEventually(haveCount(2), timeout: testTimeout)
expect(channel2.internal.presence.members.keys).to(allPass { $0.hasPrefix("\(channel1.internal.connectionId):Client") || $0.hasPrefix("\(channel2.internal.connectionId):Client") })
XCTAssertEqual(channel2.internal.presence.members["\(channel1.internal.connectionId):Client 1"]!.action, ARTPresenceAction.present)
XCTAssertEqual(channel2.internal.presence.members["\(channel2.internal.connectionId):Client 2"]!.action, ARTPresenceAction.present)
}

// RTL2
Expand Down
Loading

0 comments on commit e216ae5

Please sign in to comment.