From 98c94b1cd297cc6b844b510a6870504e1572239c Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 21 Dec 2023 14:23:19 +0100 Subject: [PATCH 01/16] Added method for generating device's `id` and `secret` when needed (on activate) and initialising `clientId` at the same moment (RSH8b). Device's `id` and `secret` no longer are being generated lazily on the first call (RSH8a). I didn't touch any threading code here on purpose, since this PR is already complex enough. --- Source/ARTLocalDevice.m | 46 +++++++++++++------ Source/ARTPushActivationState.m | 3 ++ Source/ARTRest.m | 17 +++++-- .../Ably/ARTLocalDevice+Private.h | 3 +- Source/PrivateHeaders/Ably/ARTRest+Private.h | 1 + 5 files changed, 51 insertions(+), 19 deletions(-) diff --git a/Source/ARTLocalDevice.m b/Source/ARTLocalDevice.m index 873b751c3..c508a5dad 100644 --- a/Source/ARTLocalDevice.m +++ b/Source/ARTLocalDevice.m @@ -46,17 +46,16 @@ @interface ARTLocalDevice () @implementation ARTLocalDevice -- (instancetype)initWithClientId:(NSString *)clientId storage:(id)storage logger:(nullable ARTInternalLog *)logger { +- (instancetype)initWithStorage:(id)storage logger:(nullable ARTInternalLog *)logger { if (self = [super init]) { - self.clientId = clientId; self.storage = storage; _logger = logger; } return self; } -+ (ARTLocalDevice *)load:(NSString *)clientId storage:(id)storage logger:(nullable ARTInternalLog *)logger { - ARTLocalDevice *device = [[ARTLocalDevice alloc] initWithClientId:clientId storage:storage logger:logger]; ++ (instancetype)deviceWithStorage:(id)storage logger:(nullable ARTInternalLog *)logger { + ARTLocalDevice *device = [[ARTLocalDevice alloc] initWithStorage:storage logger:logger]; device.platform = ARTDevicePlatform; #if TARGET_OS_IOS switch ([[UIDevice currentDevice] userInterfaceIdiom]) { @@ -72,17 +71,9 @@ + (ARTLocalDevice *)load:(NSString *)clientId storage:(id)stor #endif device.push.recipient[@"transportType"] = ARTDevicePushTransportType; - NSString *deviceId = [storage objectForKey:ARTDeviceIdKey]; + NSString *deviceId = [storage objectForKey:ARTDeviceIdKey] ?: @""; // PCD2, not nullable NSString *deviceSecret = deviceId == nil ? nil : [storage secretForDevice:deviceId]; - if (deviceId == nil || deviceSecret == nil) { // generate both at the same time - deviceId = [self generateId]; - deviceSecret = [self generateSecret]; - - [storage setObject:deviceId forKey:ARTDeviceIdKey]; - [storage setSecret:deviceSecret forDevice:deviceId]; - } - device.id = deviceId; device.secret = deviceSecret; @@ -90,6 +81,8 @@ + (ARTLocalDevice *)load:(NSString *)clientId storage:(id)stor ARTDeviceIdentityTokenDetails *identityTokenDetails = [ARTDeviceIdentityTokenDetails unarchive:identityTokenDetailsInfo withLogger:logger]; device->_identityTokenDetails = identityTokenDetails; + device.clientId = identityTokenDetails.clientId; + NSArray *supportedTokenTypes = @[ ARTAPNSDeviceDefaultTokenType, ARTAPNSDeviceLocationTokenType @@ -102,6 +95,33 @@ + (ARTLocalDevice *)load:(NSString *)clientId storage:(id)stor return device; } +- (BOOL)setupDetailsWithClientId:(NSString *)clientId { + NSString *deviceId = self.id; + NSString *deviceSecret = self.secret; + + if ([deviceId isEqualToString:@""] || deviceSecret == nil) { // generate both at the same time + deviceId = [self.class generateId]; + deviceSecret = [self.class generateSecret]; + + [_storage setObject:deviceId forKey:ARTDeviceIdKey]; + [_storage setSecret:deviceSecret forDevice:deviceId]; + } + + self.id = deviceId; + self.secret = deviceSecret; + + NSString *localClientId = self.clientId; + + if (localClientId && clientId && ![localClientId isEqualToString:clientId]) { + ARTLogError(self.logger, @"ARTLocalDevice: `clientId`s don't match: %@ vs %@.", clientId, localClientId); + return NO; + } + if (clientId) { + self.clientId = clientId; + } + return YES; +} + + (NSString *)generateId { return [NSUUID new].UUIDString; } diff --git a/Source/ARTPushActivationState.m b/Source/ARTPushActivationState.m index b8d28649e..0c25fce11 100644 --- a/Source/ARTPushActivationState.m +++ b/Source/ARTPushActivationState.m @@ -117,6 +117,9 @@ - (ARTPushActivationState *)transition:(ARTPushActivationEvent *)event { } else if ([event isKindOfClass:[ARTPushActivationEventCalledActivate class]]) { [self.machine registerForAPNS]; +#if TARGET_OS_IOS + [self.machine.rest setupLocalDevice]; +#endif return validateAndSync(self.machine, event, self.logger); } return nil; diff --git a/Source/ARTRest.m b/Source/ARTRest.m index 731fe3745..37ad08006 100644 --- a/Source/ARTRest.m +++ b/Source/ARTRest.m @@ -744,10 +744,9 @@ - (ARTLocalDevice *)device { } - (ARTLocalDevice *)device_nosync { - NSString *clientId = self.auth.clientId_nosync; __block ARTLocalDevice *ret; - dispatch_sync(ARTRestInternal.deviceAccessQueue, ^{ - ret = [self deviceWithClientId_onlyCallOnDeviceAccessQueue:clientId]; + dispatch_sync([ARTRestInternal deviceAccessQueue], ^{ + ret = [self sharedDevice_onlyCallOnDeviceAccessQueue]; }); return ret; } @@ -765,7 +764,7 @@ + (dispatch_queue_t)deviceAccessQueue { static BOOL sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = YES; -- (ARTLocalDevice *)deviceWithClientId_onlyCallOnDeviceAccessQueue:(NSString *)clientId { +- (ARTLocalDevice *)sharedDevice_onlyCallOnDeviceAccessQueue { // The device is shared in a static variable because it's a reflection // of what's persisted. Having a device instance per ARTRest instance // could leave some instances in a stale state, if, through another @@ -776,12 +775,20 @@ - (ARTLocalDevice *)deviceWithClientId_onlyCallOnDeviceAccessQueue:(NSString *)c static id device; if (sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue) { - device = [ARTLocalDevice load:clientId storage:self.storage logger:self.logger]; + device = [ARTLocalDevice deviceWithStorage:self.storage logger:self.logger]; sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = NO; } return device; } +- (void)setupLocalDevice { + ARTLocalDevice *device = [self device_nosync]; + NSString *clientId = self.auth.clientId_nosync; + dispatch_sync([ARTRestInternal deviceAccessQueue], ^{ + [device setupDetailsWithClientId:clientId]; + }); +} + - (void)resetDeviceSingleton { dispatch_sync([ARTRestInternal deviceAccessQueue], ^{ sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = YES; diff --git a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h index 5c9518b41..2cde1df7a 100644 --- a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h +++ b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h @@ -19,13 +19,14 @@ NSString* ARTAPNSDeviceTokenKeyOfType(NSString * _Nullable tokenType); @property (nonatomic) id storage; -+ (ARTLocalDevice *)load:(NSString *)clientId storage:(id)storage logger:(nullable ARTInternalLog *)logger; ++ (instancetype)deviceWithStorage:(id)storage logger:(nullable ARTInternalLog *)logger; - (nullable NSString *)apnsDeviceToken; - (void)setAndPersistAPNSDeviceToken:(nullable NSString *)deviceToken tokenType:(NSString *)tokenType; - (void)setAndPersistAPNSDeviceToken:(nullable NSString *)deviceToken; - (void)setAndPersistIdentityTokenDetails:(nullable ARTDeviceIdentityTokenDetails *)tokenDetails; - (BOOL)isRegistered; - (void)clearIdentityTokenDetailsAndClientId; +- (BOOL)setupDetailsWithClientId:(nullable NSString *)clientId; + (NSString *)generateId; + (NSString *)generateSecret; diff --git a/Source/PrivateHeaders/Ably/ARTRest+Private.h b/Source/PrivateHeaders/Ably/ARTRest+Private.h index 1ffbff5ab..d25cef6e4 100644 --- a/Source/PrivateHeaders/Ably/ARTRest+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRest+Private.h @@ -76,6 +76,7 @@ NS_ASSUME_NONNULL_BEGIN - (nullable NSObject *)internetIsUp:(void (^)(BOOL isUp))cb; #if TARGET_OS_IOS +- (void)setupLocalDevice; // This is only intended to be called from test code. - (void)resetDeviceSingleton; From 94053aa804107afec4454def8c919f159285ae0c Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 21 Dec 2023 14:25:05 +0100 Subject: [PATCH 02/16] Added method for clearing all the device details (RSH3g2a). --- Source/ARTLocalDevice.m | 20 ++++++++++++++----- Source/ARTPushActivationState.m | 3 +-- Source/ARTRest.m | 7 +++++++ .../Ably/ARTLocalDevice+Private.h | 2 +- Source/PrivateHeaders/Ably/ARTRest+Private.h | 2 ++ 5 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Source/ARTLocalDevice.m b/Source/ARTLocalDevice.m index c508a5dad..013e964ee 100644 --- a/Source/ARTLocalDevice.m +++ b/Source/ARTLocalDevice.m @@ -122,6 +122,21 @@ - (BOOL)setupDetailsWithClientId:(NSString *)clientId { return YES; } +- (void)resetDetails { + self.id = @""; + self.secret = nil; + self.clientId = nil; + [_storage setObject:nil forKey:ARTDeviceIdKey]; + [self setAndPersistIdentityTokenDetails:nil]; + NSArray *supportedTokenTypes = @[ + ARTAPNSDeviceDefaultTokenType, + ARTAPNSDeviceLocationTokenType + ]; + for (NSString *tokenType in supportedTokenTypes) { + [self setAndPersistAPNSDeviceToken:nil tokenType:tokenType]; + } +} + + (NSString *)generateId { return [NSUUID new].UUIDString; } @@ -173,9 +188,4 @@ - (BOOL)isRegistered { return _identityTokenDetails != nil; } -- (void)clearIdentityTokenDetailsAndClientId { - [self setAndPersistIdentityTokenDetails:nil]; - self.clientId = nil; -} - @end diff --git a/Source/ARTPushActivationState.m b/Source/ARTPushActivationState.m index 0c25fce11..834c91822 100644 --- a/Source/ARTPushActivationState.m +++ b/Source/ARTPushActivationState.m @@ -277,8 +277,7 @@ - (ARTPushActivationState *)transition:(ARTPushActivationEvent *)event { } else if ([event isKindOfClass:[ARTPushActivationEventDeregistered class]]) { #if TARGET_OS_IOS - ARTLocalDevice *local = self.machine.rest.device_nosync; - [local clearIdentityTokenDetailsAndClientId]; + [self.machine.rest resetLocalDevice]; #endif [self.machine callDeactivatedCallback:nil]; return [ARTPushActivationStateNotActivated newWithMachine:self.machine logger:self.logger]; diff --git a/Source/ARTRest.m b/Source/ARTRest.m index 37ad08006..1282c1759 100644 --- a/Source/ARTRest.m +++ b/Source/ARTRest.m @@ -789,6 +789,13 @@ - (void)setupLocalDevice { }); } +- (void)resetLocalDevice { + ARTLocalDevice *device = [self device_nosync]; + dispatch_sync([ARTRestInternal deviceAccessQueue], ^{ + [device resetDetails]; + }); +} + - (void)resetDeviceSingleton { dispatch_sync([ARTRestInternal deviceAccessQueue], ^{ sharedDeviceNeedsLoading_onlyAccessOnDeviceAccessQueue = YES; diff --git a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h index 2cde1df7a..61aba7c2e 100644 --- a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h +++ b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h @@ -25,7 +25,7 @@ NSString* ARTAPNSDeviceTokenKeyOfType(NSString * _Nullable tokenType); - (void)setAndPersistAPNSDeviceToken:(nullable NSString *)deviceToken; - (void)setAndPersistIdentityTokenDetails:(nullable ARTDeviceIdentityTokenDetails *)tokenDetails; - (BOOL)isRegistered; -- (void)clearIdentityTokenDetailsAndClientId; +- (void)resetDetails; - (BOOL)setupDetailsWithClientId:(nullable NSString *)clientId; + (NSString *)generateId; diff --git a/Source/PrivateHeaders/Ably/ARTRest+Private.h b/Source/PrivateHeaders/Ably/ARTRest+Private.h index d25cef6e4..c7b2c260f 100644 --- a/Source/PrivateHeaders/Ably/ARTRest+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRest+Private.h @@ -77,6 +77,8 @@ NS_ASSUME_NONNULL_BEGIN #if TARGET_OS_IOS - (void)setupLocalDevice; +- (void)resetLocalDevice; + // This is only intended to be called from test code. - (void)resetDeviceSingleton; From a49061d9760aa961cfbcf7f08bd1ef7bb37aed19 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 21 Dec 2023 14:36:09 +0100 Subject: [PATCH 03/16] Updated tests for the new device details loading behavior. Many tests relied on device details were there by default, so the only way to supply the details there (without proper state machine activation sequence) is to artificially call the `setupLocalDevice()`. --- .../PushActivationStateMachineTests.swift | 31 ++++++++++++++----- Test/Tests/PushAdminTests.swift | 1 + Test/Tests/PushChannelTests.swift | 1 + 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/Test/Tests/PushActivationStateMachineTests.swift b/Test/Tests/PushActivationStateMachineTests.swift index 6c6c2a924..2bb0212cd 100644 --- a/Test/Tests/PushActivationStateMachineTests.swift +++ b/Test/Tests/PushActivationStateMachineTests.swift @@ -459,6 +459,7 @@ class PushActivationStateMachineTests: XCTestCase { storage = MockDeviceStorage(startWith: ARTPushActivationStateWaitingForDeviceRegistration(machine: initialStateMachine, logger: .init(core: MockInternalLogCore()))) rest.internal.storage = storage stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) + rest.internal.setupLocalDevice() } // RSH3c1 @@ -529,6 +530,7 @@ class PushActivationStateMachineTests: XCTestCase { storage = MockDeviceStorage(startWith: ARTPushActivationStateWaitingForNewPushDeviceDetails(machine: initialStateMachine, logger: .init(core: MockInternalLogCore()))) rest.internal.storage = storage stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) + rest.internal.setupLocalDevice() } // RSH3d1 @@ -951,6 +953,8 @@ class PushActivationStateMachineTests: XCTestCase { options.clientId = "deviceClient" let rest = ARTRest(options: options) rest.internal.storage = storage + rest.internal.setupLocalDevice() + XCTAssertEqual(rest.device.clientId, "deviceClient") let newOptions = ARTClientOptions(key: "xxxx:xxxx") @@ -1156,10 +1160,15 @@ class PushActivationStateMachineTests: XCTestCase { // RSH3d2b, RSH3d2c, RSH3d2d func test__should_fire_Deregistered_event_and_include_DeviceSecret_HTTP_header() throws { contextBeforeEach?() - + + rest.internal.setupLocalDevice() + let delegate = StateMachineDelegate() stateMachine.delegate = delegate - + + let deviceId = rest.device.id + let deviceSecret = rest.device.secret + waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) stateMachine.transitions = { event, _, currentState in @@ -1177,7 +1186,7 @@ class PushActivationStateMachineTests: XCTestCase { expect(stateMachine.current).to(beAKindOf(ARTPushActivationStateNotActivated.self)) XCTAssertEqual(httpExecutor.requests.count, 1) - let requests = httpExecutor.requests.compactMap { $0.url?.path }.filter { $0 == "/push/deviceRegistrations/\(rest.device.id)" } + let requests = httpExecutor.requests.compactMap { $0.url?.path }.filter { $0 == "/push/deviceRegistrations/\(deviceId)" } XCTAssertEqual(requests.count, 1) let request = try XCTUnwrap(httpExecutor.requests.first, "Should have a \"/push/deviceRegistrations\" request") @@ -1187,7 +1196,7 @@ class PushActivationStateMachineTests: XCTestCase { XCTAssertEqual(request.httpMethod, "DELETE") XCTAssertNotNil(request.allHTTPHeaderFields?["Authorization"]) let deviceAuthorization = request.allHTTPHeaderFields?["X-Ably-DeviceSecret"] - XCTAssertEqual(deviceAuthorization, rest.device.secret) + XCTAssertEqual(deviceAuthorization, deviceSecret) contextAfterEach?() } @@ -1195,7 +1204,9 @@ class PushActivationStateMachineTests: XCTestCase { // RSH3d2b, RSH3d2c, RSH3d2d func test__should_fire_Deregistered_event_and_include_DeviceIdentityToken_HTTP_header() throws { contextBeforeEach?() - + + rest.internal.setupLocalDevice() + let delegate = StateMachineDelegate() stateMachine.delegate = delegate @@ -1211,7 +1222,9 @@ class PushActivationStateMachineTests: XCTestCase { rest.device.setAndPersistIdentityTokenDetails(testIdentityTokenDetails) defer { rest.device.setAndPersistIdentityTokenDetails(nil) } XCTAssertNotNil(rest.device.identityTokenDetails) - + + let deviceId = rest.device.id + waitUntil(timeout: testTimeout) { done in let partialDone = AblyTests.splitDone(2, done: done) stateMachine.transitions = { event, _, currentState in @@ -1229,7 +1242,7 @@ class PushActivationStateMachineTests: XCTestCase { expect(stateMachine.current).to(beAKindOf(ARTPushActivationStateNotActivated.self)) XCTAssertEqual(httpExecutor.requests.count, 1) - let requests = httpExecutor.requests.compactMap { $0.url?.path }.filter { $0 == "/push/deviceRegistrations/\(rest.device.id)" } + let requests = httpExecutor.requests.compactMap { $0.url?.path }.filter { $0 == "/push/deviceRegistrations/\(deviceId)" } XCTAssertEqual(requests.count, 1) let request = try XCTUnwrap(httpExecutor.requests.first, "Should have a \"/push/deviceRegistrations\" request") @@ -1248,7 +1261,9 @@ class PushActivationStateMachineTests: XCTestCase { // RSH3d2c func test__should_fire_DeregistrationFailed_event() throws { contextBeforeEach?() - + + rest.internal.setupLocalDevice() + let delegate = StateMachineDelegate() stateMachine.delegate = delegate diff --git a/Test/Tests/PushAdminTests.swift b/Test/Tests/PushAdminTests.swift index 7fc3bdba8..c12c88210 100644 --- a/Test/Tests/PushAdminTests.swift +++ b/Test/Tests/PushAdminTests.swift @@ -188,6 +188,7 @@ class PushAdminTests: XCTestCase { rest.internal.httpExecutor = mockHttpExecutor storage = MockDeviceStorage() rest.internal.storage = storage + rest.internal.setupLocalDevice() localDevice = rest.device } diff --git a/Test/Tests/PushChannelTests.swift b/Test/Tests/PushChannelTests.swift index 59312ba43..0d2e37149 100644 --- a/Test/Tests/PushChannelTests.swift +++ b/Test/Tests/PushChannelTests.swift @@ -341,6 +341,7 @@ class PushChannelTests: XCTestCase { options.testOptions.channelNamePrefix = nil let rest = ARTRest(options: options) rest.internal.storage = MockDeviceStorage() + rest.internal.setupLocalDevice() // Activate device let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "") From be01fb37f841ca9025ddf471c969707d66512f33 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 21 Dec 2023 14:37:45 +0100 Subject: [PATCH 04/16] Tests for RSH8b, RSH3a2b, RSH3g2a. --- .../PushActivationStateMachineTests.swift | 89 +++++++++++++++++-- Test/Tests/PushTests.swift | 7 +- 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/Test/Tests/PushActivationStateMachineTests.swift b/Test/Tests/PushActivationStateMachineTests.swift index 2bb0212cd..69eebd84e 100644 --- a/Test/Tests/PushActivationStateMachineTests.swift +++ b/Test/Tests/PushActivationStateMachineTests.swift @@ -118,14 +118,27 @@ class PushActivationStateMachineTests: XCTestCase { func test__014__Activation_state_machine__State_NotActivated__on_Event_CalledActivate__local_device__should_have_a_generated_id() { beforeEach__Activation_state_machine__State_NotActivated() - - rest.internal.resetDeviceSingleton() + + let options = ARTClientOptions(key: "xxxx:xxxx") + let rest = ARTRest(options: options) + rest.internal.storage = storage + let stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) + + stateMachine.send(ARTPushActivationEventCalledActivate()) + XCTAssertEqual(rest.device.id.count, 36) } func test__015__Activation_state_machine__State_NotActivated__on_Event_CalledActivate__local_device__should_have_a_generated_secret() throws { beforeEach__Activation_state_machine__State_NotActivated() - + + let options = ARTClientOptions(key: "xxxx:xxxx") + let rest = ARTRest(options: options) + rest.internal.storage = storage + let stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) + + stateMachine.send(ARTPushActivationEventCalledActivate()) + let secret = try XCTUnwrap(rest.device.secret, "Device Secret should be available in storage") let data = try XCTUnwrap(Data(base64Encoded: secret), "Device Secret should be encoded with Base64") @@ -140,6 +153,11 @@ class PushActivationStateMachineTests: XCTestCase { options.clientId = "deviceClient" let rest = ARTRest(options: options) rest.internal.storage = storage + + let stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) + + XCTAssertNil(rest.device.clientId) + stateMachine.send(ARTPushActivationEventCalledActivate()) XCTAssertEqual(rest.device.clientId, "deviceClient") } @@ -832,6 +850,8 @@ class PushActivationStateMachineTests: XCTestCase { rest.internal.storage = storage stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) + rest.internal.setupLocalDevice() + XCTAssertEqual(stateMachine.rest.device.clientId, "client1") var deactivatedCallbackCalled = false @@ -840,17 +860,19 @@ class PushActivationStateMachineTests: XCTestCase { } defer { hook.remove() } - var clearIdentityTokenDetailsAndClientIdCalled = false - let hookDevice = stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("clearIdentityTokenDetailsAndClientId")) { - clearIdentityTokenDetailsAndClientIdCalled = true + var resetDetailsCalled = false + let hookDevice = stateMachine.rest.device.testSuite_injectIntoMethod(after: NSSelectorFromString("resetDetails")) { + resetDetailsCalled = true } defer { hookDevice.remove() } stateMachine.send(ARTPushActivationEventDeregistered()) expect(stateMachine.current).to(beAKindOf(ARTPushActivationStateNotActivated.self)) XCTAssertTrue(deactivatedCallbackCalled) - XCTAssertTrue(clearIdentityTokenDetailsAndClientIdCalled) + XCTAssertTrue(resetDetailsCalled) // RSH3g2a + XCTAssertEqual(stateMachine.rest.device.id, "") + XCTAssertNil(stateMachine.rest.device.secret) XCTAssertNil(stateMachine.rest.device.identityTokenDetails) XCTAssertNil(stateMachine.rest.device.clientId) } @@ -875,7 +897,60 @@ class PushActivationStateMachineTests: XCTestCase { expect(stateMachine.current).to(beAKindOf(ARTPushActivationStateWaitingForDeregistration.self)) XCTAssertTrue(deactivatedCallbackCalled) } + + // RSH8b, RSH3a2b, RSH3g2a + func test__056__Activation_state_machine__should_be_possible_to_activate_and_deactivate_and_then_activate_again_with_different_clientId() { + beforeEach__Activation_state_machine__State_NotActivated() + + let options1 = ARTClientOptions(key: "xxxx:xxxx") + options1.clientId = "client1" + let rest1 = ARTRest(options: options1) + httpExecutor = MockHTTPExecutor() + rest1.internal.httpExecutor = httpExecutor + rest1.internal.storage = storage + + let stateMachineDelegate = StateMachineDelegate() + let stateMachine1 = ARTPushActivationStateMachine(rest: rest1.internal, delegate: stateMachineDelegate, logger: .init(core: MockInternalLogCore())) + + let testDeviceToken = "xxxx-xxxx-xxxx-xxxx-xxxx" + stateMachine1.rest.device.setAndPersistAPNSDeviceToken(testDeviceToken) + defer { stateMachine1.rest.device.setAndPersistAPNSDeviceToken(nil) } + waitUntil(timeout: testTimeout) { done in + let partialDone = AblyTests.splitDone(3, done: done) + stateMachine1.transitions = { event, _, _ in + if event is ARTPushActivationEventCalledActivate { + XCTAssertEqual(rest1.internal.device_nosync.clientId, "client1") + partialDone() + } + if event is ARTPushActivationEventGotPushDeviceDetails { + partialDone() + stateMachine1.send(ARTPushActivationEventCalledDeactivate()) + } + if event is ARTPushActivationEventCalledDeactivate { + partialDone() + } + } + stateMachine1.send(ARTPushActivationEventCalledActivate()) + } + + XCTAssertNil(rest1.device.clientId) // after deactivation, RSH3g2a + + let options2 = ARTClientOptions(key: "xxxx:xxxx") + options2.clientId = "client2" + let rest2 = ARTRest(options: options2) + rest2.internal.storage = storage + rest2.internal.httpExecutor = httpExecutor + + XCTAssertNil(rest2.device.clientId) + + let stateMachine2 = ARTPushActivationStateMachine(rest: rest2.internal, delegate: stateMachineDelegate, logger: .init(core: MockInternalLogCore())) + stateMachine2.send(ARTPushActivationEventCalledActivate()) + + XCTAssertEqual(rest2.device.clientId, "client2") + XCTAssertTrue(rest1.device === rest2.device) + } + // RSH4 func test__005__Activation_state_machine__should_queue_event_that_has_no_transition_defined_for_it() throws { // Start with WaitingForDeregistration state diff --git a/Test/Tests/PushTests.swift b/Test/Tests/PushTests.swift index a98488ab4..3234257e6 100644 --- a/Test/Tests/PushTests.swift +++ b/Test/Tests/PushTests.swift @@ -234,11 +234,16 @@ class PushTests: XCTestCase { let rest = ARTRest(key: "fake:key") rest.internal.storage = storage + + storage.simulateOnNextRead(string: "testId", for: ARTDeviceIdKey) + storage.simulateOnNextRead(string: "testSecret", for: ARTDeviceSecretKey) storage.simulateOnNextRead(string: testToken, for: ARTAPNSDeviceTokenKey) storage.simulateOnNextRead(data: testIdentity.archive(withLogger: nil), for: ARTDeviceIdentityTokenKey) let device = rest.device + XCTAssertEqual(device.id, "testId") + XCTAssertEqual(device.secret, "testSecret") XCTAssertEqual(device.apnsDeviceToken(), testToken) XCTAssertEqual(device.identityTokenDetails?.token, testIdentity.token) } @@ -336,7 +341,7 @@ class PushTests: XCTestCase { storage.simulateOnNextRead(string: testDeviceToken, for: ARTAPNSDeviceTokenKey) storage.simulateOnNextRead(data: testDeviceIdentity.archive(withLogger: nil), for: ARTDeviceIdentityTokenKey) - XCTAssertNil(realtime.device.clientId) + XCTAssertEqual(realtime.device.clientId, testDeviceIdentity.clientId) waitUntil(timeout: testTimeout) { done in stateMachine.transitions = { event, _, _ in From 8381e102e261e302cb3e986c53f3971ecd9b2e5b Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 21 Dec 2023 19:59:47 +0100 Subject: [PATCH 05/16] Fix for `nil` check. --- Source/ARTLocalDevice.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/ARTLocalDevice.m b/Source/ARTLocalDevice.m index 013e964ee..b84a86cc2 100644 --- a/Source/ARTLocalDevice.m +++ b/Source/ARTLocalDevice.m @@ -71,10 +71,10 @@ + (instancetype)deviceWithStorage:(id)storage logger:(nullable #endif device.push.recipient[@"transportType"] = ARTDevicePushTransportType; - NSString *deviceId = [storage objectForKey:ARTDeviceIdKey] ?: @""; // PCD2, not nullable + NSString *deviceId = [storage objectForKey:ARTDeviceIdKey]; NSString *deviceSecret = deviceId == nil ? nil : [storage secretForDevice:deviceId]; - device.id = deviceId; + device.id = deviceId ?: @""; // PCD2, not nullable; device.secret = deviceSecret; id identityTokenDetailsInfo = [storage objectForKey:ARTDeviceIdentityTokenKey]; From 2f4b315f6c0eb639911f4e809e6e6ef0633040e1 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 4 Jan 2024 20:18:19 +0100 Subject: [PATCH 06/16] Updated func names. --- Source/ARTPushActivationState.m | 4 ++-- Source/ARTRest.m | 4 ++-- Source/PrivateHeaders/Ably/ARTRest+Private.h | 4 ++-- Test/Tests/PushActivationStateMachineTests.swift | 14 +++++++------- Test/Tests/PushAdminTests.swift | 2 +- Test/Tests/PushChannelTests.swift | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Source/ARTPushActivationState.m b/Source/ARTPushActivationState.m index 834c91822..289a82b48 100644 --- a/Source/ARTPushActivationState.m +++ b/Source/ARTPushActivationState.m @@ -118,7 +118,7 @@ - (ARTPushActivationState *)transition:(ARTPushActivationEvent *)event { else if ([event isKindOfClass:[ARTPushActivationEventCalledActivate class]]) { [self.machine registerForAPNS]; #if TARGET_OS_IOS - [self.machine.rest setupLocalDevice]; + [self.machine.rest setupLocalDevice_nosync]; #endif return validateAndSync(self.machine, event, self.logger); } @@ -277,7 +277,7 @@ - (ARTPushActivationState *)transition:(ARTPushActivationEvent *)event { } else if ([event isKindOfClass:[ARTPushActivationEventDeregistered class]]) { #if TARGET_OS_IOS - [self.machine.rest resetLocalDevice]; + [self.machine.rest resetLocalDevice_nosync]; #endif [self.machine callDeactivatedCallback:nil]; return [ARTPushActivationStateNotActivated newWithMachine:self.machine logger:self.logger]; diff --git a/Source/ARTRest.m b/Source/ARTRest.m index 1282c1759..36df56b52 100644 --- a/Source/ARTRest.m +++ b/Source/ARTRest.m @@ -781,7 +781,7 @@ - (ARTLocalDevice *)sharedDevice_onlyCallOnDeviceAccessQueue { return device; } -- (void)setupLocalDevice { +- (void)setupLocalDevice_nosync { ARTLocalDevice *device = [self device_nosync]; NSString *clientId = self.auth.clientId_nosync; dispatch_sync([ARTRestInternal deviceAccessQueue], ^{ @@ -789,7 +789,7 @@ - (void)setupLocalDevice { }); } -- (void)resetLocalDevice { +- (void)resetLocalDevice_nosync { ARTLocalDevice *device = [self device_nosync]; dispatch_sync([ARTRestInternal deviceAccessQueue], ^{ [device resetDetails]; diff --git a/Source/PrivateHeaders/Ably/ARTRest+Private.h b/Source/PrivateHeaders/Ably/ARTRest+Private.h index c7b2c260f..62ad582f9 100644 --- a/Source/PrivateHeaders/Ably/ARTRest+Private.h +++ b/Source/PrivateHeaders/Ably/ARTRest+Private.h @@ -76,8 +76,8 @@ NS_ASSUME_NONNULL_BEGIN - (nullable NSObject *)internetIsUp:(void (^)(BOOL isUp))cb; #if TARGET_OS_IOS -- (void)setupLocalDevice; -- (void)resetLocalDevice; +- (void)setupLocalDevice_nosync; +- (void)resetLocalDevice_nosync; // This is only intended to be called from test code. - (void)resetDeviceSingleton; diff --git a/Test/Tests/PushActivationStateMachineTests.swift b/Test/Tests/PushActivationStateMachineTests.swift index 69eebd84e..0e71446b0 100644 --- a/Test/Tests/PushActivationStateMachineTests.swift +++ b/Test/Tests/PushActivationStateMachineTests.swift @@ -477,7 +477,7 @@ class PushActivationStateMachineTests: XCTestCase { storage = MockDeviceStorage(startWith: ARTPushActivationStateWaitingForDeviceRegistration(machine: initialStateMachine, logger: .init(core: MockInternalLogCore()))) rest.internal.storage = storage stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) - rest.internal.setupLocalDevice() + rest.internal.setupLocalDevice_nosync() } // RSH3c1 @@ -548,7 +548,7 @@ class PushActivationStateMachineTests: XCTestCase { storage = MockDeviceStorage(startWith: ARTPushActivationStateWaitingForNewPushDeviceDetails(machine: initialStateMachine, logger: .init(core: MockInternalLogCore()))) rest.internal.storage = storage stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) - rest.internal.setupLocalDevice() + rest.internal.setupLocalDevice_nosync() } // RSH3d1 @@ -850,7 +850,7 @@ class PushActivationStateMachineTests: XCTestCase { rest.internal.storage = storage stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) - rest.internal.setupLocalDevice() + rest.internal.setupLocalDevice_nosync() XCTAssertEqual(stateMachine.rest.device.clientId, "client1") @@ -1028,7 +1028,7 @@ class PushActivationStateMachineTests: XCTestCase { options.clientId = "deviceClient" let rest = ARTRest(options: options) rest.internal.storage = storage - rest.internal.setupLocalDevice() + rest.internal.setupLocalDevice_nosync() XCTAssertEqual(rest.device.clientId, "deviceClient") @@ -1236,7 +1236,7 @@ class PushActivationStateMachineTests: XCTestCase { func test__should_fire_Deregistered_event_and_include_DeviceSecret_HTTP_header() throws { contextBeforeEach?() - rest.internal.setupLocalDevice() + rest.internal.setupLocalDevice_nosync() let delegate = StateMachineDelegate() stateMachine.delegate = delegate @@ -1280,7 +1280,7 @@ class PushActivationStateMachineTests: XCTestCase { func test__should_fire_Deregistered_event_and_include_DeviceIdentityToken_HTTP_header() throws { contextBeforeEach?() - rest.internal.setupLocalDevice() + rest.internal.setupLocalDevice_nosync() let delegate = StateMachineDelegate() stateMachine.delegate = delegate @@ -1337,7 +1337,7 @@ class PushActivationStateMachineTests: XCTestCase { func test__should_fire_DeregistrationFailed_event() throws { contextBeforeEach?() - rest.internal.setupLocalDevice() + rest.internal.setupLocalDevice_nosync() let delegate = StateMachineDelegate() stateMachine.delegate = delegate diff --git a/Test/Tests/PushAdminTests.swift b/Test/Tests/PushAdminTests.swift index c12c88210..852b45221 100644 --- a/Test/Tests/PushAdminTests.swift +++ b/Test/Tests/PushAdminTests.swift @@ -188,7 +188,7 @@ class PushAdminTests: XCTestCase { rest.internal.httpExecutor = mockHttpExecutor storage = MockDeviceStorage() rest.internal.storage = storage - rest.internal.setupLocalDevice() + rest.internal.setupLocalDevice_nosync() localDevice = rest.device } diff --git a/Test/Tests/PushChannelTests.swift b/Test/Tests/PushChannelTests.swift index 0d2e37149..6bfc04eca 100644 --- a/Test/Tests/PushChannelTests.swift +++ b/Test/Tests/PushChannelTests.swift @@ -341,7 +341,7 @@ class PushChannelTests: XCTestCase { options.testOptions.channelNamePrefix = nil let rest = ARTRest(options: options) rest.internal.storage = MockDeviceStorage() - rest.internal.setupLocalDevice() + rest.internal.setupLocalDevice_nosync() // Activate device let testIdentityTokenDetails = ARTDeviceIdentityTokenDetails(token: "xxxx-xxxx-xxx", issued: Date(), expires: Date.distantFuture, capability: "", clientId: "") From 12521f80be95e3aa5cc211861969c2f0ac87658b Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 4 Jan 2024 20:18:48 +0100 Subject: [PATCH 07/16] Fixed call order. --- Source/ARTPushActivationState.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ARTPushActivationState.m b/Source/ARTPushActivationState.m index 289a82b48..0dc0223fc 100644 --- a/Source/ARTPushActivationState.m +++ b/Source/ARTPushActivationState.m @@ -116,10 +116,10 @@ - (ARTPushActivationState *)transition:(ARTPushActivationEvent *)event { return self; } else if ([event isKindOfClass:[ARTPushActivationEventCalledActivate class]]) { - [self.machine registerForAPNS]; #if TARGET_OS_IOS [self.machine.rest setupLocalDevice_nosync]; #endif + [self.machine registerForAPNS]; return validateAndSync(self.machine, event, self.logger); } return nil; From f16491be1614c3464617a075f8d93f21d43ebcdf Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 7 Jan 2024 14:20:24 +0100 Subject: [PATCH 08/16] Fixed return type for function fetching APNs device token. --- Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h index 61aba7c2e..490b13e6e 100644 --- a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h +++ b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h @@ -31,7 +31,7 @@ NSString* ARTAPNSDeviceTokenKeyOfType(NSString * _Nullable tokenType); + (NSString *)generateId; + (NSString *)generateSecret; -+ (NSString *)apnsDeviceTokenOfType:(nullable NSString *)tokenType fromStorage:(id)storage; ++ (nullable NSString *)apnsDeviceTokenOfType:(nullable NSString *)tokenType fromStorage:(id)storage; @end From d5a9d823799941a7a2bbd72df02def17683251e1 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 7 Jan 2024 14:21:55 +0100 Subject: [PATCH 09/16] Updated RSH8b and RSH3g2a tests. --- Test/Tests/PushActivationStateMachineTests.swift | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Test/Tests/PushActivationStateMachineTests.swift b/Test/Tests/PushActivationStateMachineTests.swift index 0e71446b0..75ecee2ff 100644 --- a/Test/Tests/PushActivationStateMachineTests.swift +++ b/Test/Tests/PushActivationStateMachineTests.swift @@ -156,8 +156,15 @@ class PushActivationStateMachineTests: XCTestCase { let stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) + XCTAssertEqual(rest.device.id, "") + XCTAssertNil(rest.device.secret) XCTAssertNil(rest.device.clientId) + stateMachine.send(ARTPushActivationEventCalledActivate()) + + XCTAssertNotNil(rest.device.id) + XCTAssertNotEqual(rest.device.id, "") + XCTAssertNotNil(rest.device.secret) XCTAssertEqual(rest.device.clientId, "deviceClient") } @@ -870,11 +877,18 @@ class PushActivationStateMachineTests: XCTestCase { expect(stateMachine.current).to(beAKindOf(ARTPushActivationStateNotActivated.self)) XCTAssertTrue(deactivatedCallbackCalled) XCTAssertTrue(resetDetailsCalled) + // RSH3g2a XCTAssertEqual(stateMachine.rest.device.id, "") XCTAssertNil(stateMachine.rest.device.secret) XCTAssertNil(stateMachine.rest.device.identityTokenDetails) XCTAssertNil(stateMachine.rest.device.clientId) + XCTAssertNil(stateMachine.rest.device.push.recipient["push"]) + + XCTAssertNil(storage.object(forKey: ARTDeviceIdKey)) + XCTAssertNil(storage.object(forKey: ARTDeviceIdentityTokenKey)) + XCTAssertNil(ARTLocalDevice.apnsDeviceToken(ofType: ARTAPNSDeviceDefaultTokenType, from: storage)) + XCTAssertNil(ARTLocalDevice.apnsDeviceToken(ofType: ARTAPNSDeviceLocationTokenType, from: storage)) } // RSH3g3 From 10f1b075ddddfe993461cee50ad6d91ece1e1eb7 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 14 Jan 2024 20:48:50 +0100 Subject: [PATCH 10/16] Avoid resetting deviceId until the new one is needed together with the deviceSecret (due to wrong spec PCD2 which draws id as not nullable). --- Source/ARTLocalDevice.m | 7 +++---- Test/Tests/PushActivationStateMachineTests.swift | 3 --- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/Source/ARTLocalDevice.m b/Source/ARTLocalDevice.m index b84a86cc2..a988cfdad 100644 --- a/Source/ARTLocalDevice.m +++ b/Source/ARTLocalDevice.m @@ -74,7 +74,7 @@ + (instancetype)deviceWithStorage:(id)storage logger:(nullable NSString *deviceId = [storage objectForKey:ARTDeviceIdKey]; NSString *deviceSecret = deviceId == nil ? nil : [storage secretForDevice:deviceId]; - device.id = deviceId ?: @""; // PCD2, not nullable; + device.id = deviceId ?: [self.class generateId]; // temporarily ignore RSH8a/b, see https://github.com/ably/ably-cocoa/pull/1847#discussion_r1441954284 thread device.secret = deviceSecret; id identityTokenDetailsInfo = [storage objectForKey:ARTDeviceIdentityTokenKey]; @@ -99,7 +99,7 @@ - (BOOL)setupDetailsWithClientId:(NSString *)clientId { NSString *deviceId = self.id; NSString *deviceSecret = self.secret; - if ([deviceId isEqualToString:@""] || deviceSecret == nil) { // generate both at the same time + if (deviceId == nil || deviceSecret == nil) { // generate both at the same time deviceId = [self.class generateId]; deviceSecret = [self.class generateSecret]; @@ -123,10 +123,9 @@ - (BOOL)setupDetailsWithClientId:(NSString *)clientId { } - (void)resetDetails { - self.id = @""; self.secret = nil; self.clientId = nil; - [_storage setObject:nil forKey:ARTDeviceIdKey]; + [_storage setSecret:nil forDevice:self.id]; [self setAndPersistIdentityTokenDetails:nil]; NSArray *supportedTokenTypes = @[ ARTAPNSDeviceDefaultTokenType, diff --git a/Test/Tests/PushActivationStateMachineTests.swift b/Test/Tests/PushActivationStateMachineTests.swift index 75ecee2ff..730da5341 100644 --- a/Test/Tests/PushActivationStateMachineTests.swift +++ b/Test/Tests/PushActivationStateMachineTests.swift @@ -156,14 +156,12 @@ class PushActivationStateMachineTests: XCTestCase { let stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) - XCTAssertEqual(rest.device.id, "") XCTAssertNil(rest.device.secret) XCTAssertNil(rest.device.clientId) stateMachine.send(ARTPushActivationEventCalledActivate()) XCTAssertNotNil(rest.device.id) - XCTAssertNotEqual(rest.device.id, "") XCTAssertNotNil(rest.device.secret) XCTAssertEqual(rest.device.clientId, "deviceClient") } @@ -879,7 +877,6 @@ class PushActivationStateMachineTests: XCTestCase { XCTAssertTrue(resetDetailsCalled) // RSH3g2a - XCTAssertEqual(stateMachine.rest.device.id, "") XCTAssertNil(stateMachine.rest.device.secret) XCTAssertNil(stateMachine.rest.device.identityTokenDetails) XCTAssertNil(stateMachine.rest.device.clientId) From 988ecc7b508ec8c4fb705831866755145e26a070 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Fri, 12 Jan 2024 22:10:19 +0100 Subject: [PATCH 11/16] Removed unnecessary check for clientId. --- Source/ARTLocalDevice.m | 13 ++----------- Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h | 2 +- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/Source/ARTLocalDevice.m b/Source/ARTLocalDevice.m index a988cfdad..ac0ebd1f3 100644 --- a/Source/ARTLocalDevice.m +++ b/Source/ARTLocalDevice.m @@ -95,7 +95,7 @@ + (instancetype)deviceWithStorage:(id)storage logger:(nullable return device; } -- (BOOL)setupDetailsWithClientId:(NSString *)clientId { +- (void)setupDetailsWithClientId:(NSString *)clientId { NSString *deviceId = self.id; NSString *deviceSecret = self.secret; @@ -110,16 +110,7 @@ - (BOOL)setupDetailsWithClientId:(NSString *)clientId { self.id = deviceId; self.secret = deviceSecret; - NSString *localClientId = self.clientId; - - if (localClientId && clientId && ![localClientId isEqualToString:clientId]) { - ARTLogError(self.logger, @"ARTLocalDevice: `clientId`s don't match: %@ vs %@.", clientId, localClientId); - return NO; - } - if (clientId) { - self.clientId = clientId; - } - return YES; + self.clientId = clientId; } - (void)resetDetails { diff --git a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h index 490b13e6e..f2580a075 100644 --- a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h +++ b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h @@ -26,7 +26,7 @@ NSString* ARTAPNSDeviceTokenKeyOfType(NSString * _Nullable tokenType); - (void)setAndPersistIdentityTokenDetails:(nullable ARTDeviceIdentityTokenDetails *)tokenDetails; - (BOOL)isRegistered; - (void)resetDetails; -- (BOOL)setupDetailsWithClientId:(nullable NSString *)clientId; +- (void)setupDetailsWithClientId:(nullable NSString *)clientId; + (NSString *)generateId; + (NSString *)generateSecret; From e22d9b5728b9ec7ededb01e522745d35ac138eb5 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 14 Jan 2024 20:53:30 +0100 Subject: [PATCH 12/16] Save clientId in its own storage value. --- Source/ARTAuth.m | 3 +++ Source/ARTLocalDevice.m | 11 ++++++++++- .../Ably/ARTLocalDevice+Private.h | 1 + Test/Tests/PushTests.swift | 19 +++++++++++++++++-- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/Source/ARTAuth.m b/Source/ARTAuth.m index 16ab57a08..78a254f93 100644 --- a/Source/ARTAuth.m +++ b/Source/ARTAuth.m @@ -21,6 +21,8 @@ #import "ARTPushActivationState.h" #import "ARTFormEncode.h" #import "ARTInternalLog.h" +#import "ARTLocalDeviceStorage.h" +#import "ARTLocalDevice+Private.h" @implementation ARTAuth { ARTQueuedDealloc *_dealloc; @@ -824,6 +826,7 @@ - (void)setLocalDeviceClientId_nosync:(NSString *)clientId { return; } [_rest.device_nosync setClientId:clientId]; + [_rest.storage setObject:clientId forKey:ARTClientIdKey]; [_rest.push getActivationMachine:^(ARTPushActivationStateMachine *stateMachine) { if (![stateMachine.current_nosync isKindOfClass:[ARTPushActivationStateNotActivated class]]) { [stateMachine sendEvent:[[ARTPushActivationEventGotPushDeviceDetails alloc] init]]; diff --git a/Source/ARTLocalDevice.m b/Source/ARTLocalDevice.m index ac0ebd1f3..2153e3bbe 100644 --- a/Source/ARTLocalDevice.m +++ b/Source/ARTLocalDevice.m @@ -30,6 +30,7 @@ NSString *const ARTDeviceSecretKey = @"ARTDeviceSecret"; NSString *const ARTDeviceIdentityTokenKey = @"ARTDeviceIdentityToken"; NSString *const ARTAPNSDeviceTokenKey = @"ARTAPNSDeviceToken"; +NSString *const ARTClientIdKey = @"ARTClientId"; NSString *const ARTAPNSDeviceDefaultTokenType = @"default"; NSString *const ARTAPNSDeviceLocationTokenType = @"location"; @@ -81,7 +82,12 @@ + (instancetype)deviceWithStorage:(id)storage logger:(nullable ARTDeviceIdentityTokenDetails *identityTokenDetails = [ARTDeviceIdentityTokenDetails unarchive:identityTokenDetailsInfo withLogger:logger]; device->_identityTokenDetails = identityTokenDetails; - device.clientId = identityTokenDetails.clientId; + NSString *clientId = [storage objectForKey:ARTClientIdKey]; + if (clientId == nil && identityTokenDetails.clientId != nil) { + clientId = identityTokenDetails.clientId; + [storage setObject:clientId forKey:ARTClientIdKey]; + } + device.clientId = clientId; NSArray *supportedTokenTypes = @[ ARTAPNSDeviceDefaultTokenType, @@ -111,12 +117,14 @@ - (void)setupDetailsWithClientId:(NSString *)clientId { self.secret = deviceSecret; self.clientId = clientId; + [_storage setObject:clientId forKey:ARTClientIdKey]; } - (void)resetDetails { self.secret = nil; self.clientId = nil; [_storage setSecret:nil forDevice:self.id]; + [_storage setObject:nil forKey:ARTClientIdKey]; [self setAndPersistIdentityTokenDetails:nil]; NSArray *supportedTokenTypes = @[ ARTAPNSDeviceDefaultTokenType, @@ -171,6 +179,7 @@ - (void)setAndPersistIdentityTokenDetails:(ARTDeviceIdentityTokenDetails *)token _identityTokenDetails = tokenDetails; if (self.clientId == nil) { self.clientId = tokenDetails.clientId; + [self.storage setObject:tokenDetails.clientId forKey:ARTClientIdKey]; } } diff --git a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h index f2580a075..de9f0925c 100644 --- a/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h +++ b/Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h @@ -9,6 +9,7 @@ extern NSString *const ARTDeviceIdKey; extern NSString *const ARTDeviceSecretKey; extern NSString *const ARTDeviceIdentityTokenKey; extern NSString *const ARTAPNSDeviceTokenKey; +extern NSString *const ARTClientIdKey; extern NSString *const ARTAPNSDeviceDefaultTokenType; extern NSString *const ARTAPNSDeviceLocationTokenType; diff --git a/Test/Tests/PushTests.swift b/Test/Tests/PushTests.swift index 3234257e6..8470e28be 100644 --- a/Test/Tests/PushTests.swift +++ b/Test/Tests/PushTests.swift @@ -229,7 +229,7 @@ class PushTests: XCTestCase { issued: Date(), expires: Date.distantFuture, capability: "", - clientId: "" + clientId: "client1" ) let rest = ARTRest(key: "fake:key") @@ -244,6 +244,7 @@ class PushTests: XCTestCase { XCTAssertEqual(device.id, "testId") XCTAssertEqual(device.secret, "testSecret") + XCTAssertEqual(device.clientId, "client1") XCTAssertEqual(device.apnsDeviceToken(), testToken) XCTAssertEqual(device.identityTokenDetails?.token, testIdentity.token) } @@ -260,7 +261,11 @@ class PushTests: XCTestCase { } let realtime = ARTRealtime(options: options) + let storage = MockDeviceStorage() + realtime.internal.rest.storage = storage + XCTAssertNil(realtime.device.clientId) + XCTAssertNil(storage.keysWritten[ARTClientIdKey] as? String) waitUntil(timeout: testTimeout) { done in realtime.auth.authorize { _, _ in @@ -269,6 +274,7 @@ class PushTests: XCTestCase { } XCTAssertEqual(realtime.device.clientId, "testClient") + XCTAssertEqual(storage.keysWritten[ARTClientIdKey] as? String, "testClient") } // RSH8d @@ -279,8 +285,12 @@ class PushTests: XCTestCase { options.testOptions.transportFactory = TestProxyTransportFactory() let realtime = ARTRealtime(options: options) - XCTAssertNil(realtime.device.clientId) + let storage = MockDeviceStorage() + realtime.internal.rest.storage = storage + XCTAssertNil(realtime.device.clientId) + XCTAssertNil(storage.keysWritten[ARTClientIdKey] as? String) + waitUntil(timeout: testTimeout) { done in realtime.connection.once(.connected) { _ in done() @@ -293,6 +303,7 @@ class PushTests: XCTestCase { } XCTAssertEqual(realtime.device.clientId, "testClient") + XCTAssertEqual(storage.keysWritten[ARTClientIdKey] as? String, "testClient") } // RSH8e @@ -382,9 +393,12 @@ class PushTests: XCTestCase { clientId: expectedClientId ) } + let storage = MockDeviceStorage() + rest.internal.storage = storage rest.push.internal.activationMachine.delegate = stateMachineDelegate XCTAssertNil(rest.device.clientId) + XCTAssertNil(storage.keysWritten[ARTClientIdKey] as? String) waitUntil(timeout: testTimeout) { done in stateMachineDelegate.onDidActivateAblyPush = { _ in @@ -397,6 +411,7 @@ class PushTests: XCTestCase { } XCTAssertEqual(rest.device.clientId, expectedClientId) + XCTAssertEqual(storage.keysWritten[ARTClientIdKey] as? String, expectedClientId) } func test__014__Registerer_Delegate_option__a_successful_activation_should_call_the_correct_registerer_delegate_method() throws { From 1873d810b26da5145c08d8e22385ebfae06a00d4 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Sun, 14 Jan 2024 21:42:36 +0100 Subject: [PATCH 13/16] Setup device with the new clientId after validation. --- Source/ARTPushActivationState.m | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Source/ARTPushActivationState.m b/Source/ARTPushActivationState.m index 0dc0223fc..5b7ed7f26 100644 --- a/Source/ARTPushActivationState.m +++ b/Source/ARTPushActivationState.m @@ -102,6 +102,8 @@ @implementation ARTPushActivationPersistentState } else if ([local apnsDeviceToken]) { [machine sendEvent:[ARTPushActivationEventGotPushDeviceDetails new]]; } + [machine.rest setupLocalDevice_nosync]; + [machine registerForAPNS]; #endif return [ARTPushActivationStateWaitingForPushDeviceDetails newWithMachine:machine logger:logger]; @@ -116,10 +118,6 @@ - (ARTPushActivationState *)transition:(ARTPushActivationEvent *)event { return self; } else if ([event isKindOfClass:[ARTPushActivationEventCalledActivate class]]) { -#if TARGET_OS_IOS - [self.machine.rest setupLocalDevice_nosync]; -#endif - [self.machine registerForAPNS]; return validateAndSync(self.machine, event, self.logger); } return nil; From 79e741b269e7ab7486292fc0acc09b5146c2badf Mon Sep 17 00:00:00 2001 From: Marat Al Date: Wed, 17 Jan 2024 22:07:59 +0100 Subject: [PATCH 14/16] Generate device id/secret pair so that `id` always not `nil` unless issue https://github.com/ably/specification/issues/180 resolved. --- Source/ARTLocalDevice.m | 33 +++++++++++-------- .../PushActivationStateMachineTests.swift | 9 +++-- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Source/ARTLocalDevice.m b/Source/ARTLocalDevice.m index 2153e3bbe..c346bfbd8 100644 --- a/Source/ARTLocalDevice.m +++ b/Source/ARTLocalDevice.m @@ -55,6 +55,14 @@ - (instancetype)initWithStorage:(id)storage logger:(nullable A return self; } +- (void)generateAndPersistPairOfDeviceIdAndSecret { + self.id = [self.class generateId]; + self.secret = [self.class generateSecret]; + + [_storage setObject:self.id forKey:ARTDeviceIdKey]; + [_storage setSecret:self.secret forDevice:self.id]; +} + + (instancetype)deviceWithStorage:(id)storage logger:(nullable ARTInternalLog *)logger { ARTLocalDevice *device = [[ARTLocalDevice alloc] initWithStorage:storage logger:logger]; device.platform = ARTDevicePlatform; @@ -75,8 +83,13 @@ + (instancetype)deviceWithStorage:(id)storage logger:(nullable NSString *deviceId = [storage objectForKey:ARTDeviceIdKey]; NSString *deviceSecret = deviceId == nil ? nil : [storage secretForDevice:deviceId]; - device.id = deviceId ?: [self.class generateId]; // temporarily ignore RSH8a/b, see https://github.com/ably/ably-cocoa/pull/1847#discussion_r1441954284 thread - device.secret = deviceSecret; + if (deviceId == nil || deviceSecret == nil) { + [device generateAndPersistPairOfDeviceIdAndSecret]; // Should be removed later once spec issue #180 resolved. + } + else { + device.id = deviceId; + device.secret = deviceSecret; + } id identityTokenDetailsInfo = [storage objectForKey:ARTDeviceIdentityTokenKey]; ARTDeviceIdentityTokenDetails *identityTokenDetails = [ARTDeviceIdentityTokenDetails unarchive:identityTokenDetailsInfo withLogger:logger]; @@ -105,25 +118,19 @@ - (void)setupDetailsWithClientId:(NSString *)clientId { NSString *deviceId = self.id; NSString *deviceSecret = self.secret; - if (deviceId == nil || deviceSecret == nil) { // generate both at the same time - deviceId = [self.class generateId]; - deviceSecret = [self.class generateSecret]; - - [_storage setObject:deviceId forKey:ARTDeviceIdKey]; - [_storage setSecret:deviceSecret forDevice:deviceId]; + if (deviceId == nil || deviceSecret == nil) { + [self generateAndPersistPairOfDeviceIdAndSecret]; } - self.id = deviceId; - self.secret = deviceSecret; - self.clientId = clientId; [_storage setObject:clientId forKey:ARTClientIdKey]; } - (void)resetDetails { - self.secret = nil; + // Should be replaced later to resetting device's id/secret once spec issue #180 resolved. + [self generateAndPersistPairOfDeviceIdAndSecret]; + self.clientId = nil; - [_storage setSecret:nil forDevice:self.id]; [_storage setObject:nil forKey:ARTClientIdKey]; [self setAndPersistIdentityTokenDetails:nil]; NSArray *supportedTokenTypes = @[ diff --git a/Test/Tests/PushActivationStateMachineTests.swift b/Test/Tests/PushActivationStateMachineTests.swift index 730da5341..b47be792c 100644 --- a/Test/Tests/PushActivationStateMachineTests.swift +++ b/Test/Tests/PushActivationStateMachineTests.swift @@ -156,7 +156,6 @@ class PushActivationStateMachineTests: XCTestCase { let stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate(), logger: .init(core: MockInternalLogCore())) - XCTAssertNil(rest.device.secret) XCTAssertNil(rest.device.clientId) stateMachine.send(ARTPushActivationEventCalledActivate()) @@ -877,15 +876,19 @@ class PushActivationStateMachineTests: XCTestCase { XCTAssertTrue(resetDetailsCalled) // RSH3g2a - XCTAssertNil(stateMachine.rest.device.secret) XCTAssertNil(stateMachine.rest.device.identityTokenDetails) XCTAssertNil(stateMachine.rest.device.clientId) XCTAssertNil(stateMachine.rest.device.push.recipient["push"]) - XCTAssertNil(storage.object(forKey: ARTDeviceIdKey)) XCTAssertNil(storage.object(forKey: ARTDeviceIdentityTokenKey)) XCTAssertNil(ARTLocalDevice.apnsDeviceToken(ofType: ARTAPNSDeviceDefaultTokenType, from: storage)) XCTAssertNil(ARTLocalDevice.apnsDeviceToken(ofType: ARTAPNSDeviceLocationTokenType, from: storage)) + + // Should be replaced with `nil` checks after issue https://github.com/ably/specification/issues/180 resolved + XCTAssertNotNil(stateMachine.rest.device.id) + XCTAssertNotNil(stateMachine.rest.device.secret) + XCTAssertEqual(storage.keysWritten[ARTDeviceIdKey] as? String, stateMachine.rest.device.id) + XCTAssertEqual(storage.keysWritten[ARTDeviceSecretKey] as? String, stateMachine.rest.device.secret) } // RSH3g3 From e9cab44214014d599353bb8b740fd1c1e5ba77c6 Mon Sep 17 00:00:00 2001 From: Marat Al Date: Wed, 17 Jan 2024 22:08:22 +0100 Subject: [PATCH 15/16] Commentary added. --- Source/ARTLocalDevice.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ARTLocalDevice.m b/Source/ARTLocalDevice.m index c346bfbd8..4ad41cf2f 100644 --- a/Source/ARTLocalDevice.m +++ b/Source/ARTLocalDevice.m @@ -97,7 +97,7 @@ + (instancetype)deviceWithStorage:(id)storage logger:(nullable NSString *clientId = [storage objectForKey:ARTClientIdKey]; if (clientId == nil && identityTokenDetails.clientId != nil) { - clientId = identityTokenDetails.clientId; + clientId = identityTokenDetails.clientId; // pickup some value that exists for the updated installations with already registered device [storage setObject:clientId forKey:ARTClientIdKey]; } device.clientId = clientId; From f5b7359e604c0b703c41c0d6800d864ad33f7b5e Mon Sep 17 00:00:00 2001 From: Marat Al Date: Thu, 18 Jan 2024 17:14:01 +0100 Subject: [PATCH 16/16] Update Source/ARTLocalDevice.m Co-authored-by: Lawrence Forooghian <53756884+lawrence-forooghian@users.noreply.github.com> --- Source/ARTLocalDevice.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ARTLocalDevice.m b/Source/ARTLocalDevice.m index 4ad41cf2f..5a73a80ec 100644 --- a/Source/ARTLocalDevice.m +++ b/Source/ARTLocalDevice.m @@ -97,7 +97,7 @@ + (instancetype)deviceWithStorage:(id)storage logger:(nullable NSString *clientId = [storage objectForKey:ARTClientIdKey]; if (clientId == nil && identityTokenDetails.clientId != nil) { - clientId = identityTokenDetails.clientId; // pickup some value that exists for the updated installations with already registered device + clientId = identityTokenDetails.clientId; // Older versions of the SDK did not persist clientId, so as a fallback when loading data persisted by these versions we use the clientId of the stored identity token [storage setObject:clientId forKey:ARTClientIdKey]; } device.clientId = clientId;