From 07bb1011702d81dc449e63e4c28c78afe72259a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toni=20C=C3=A1rdenas?= Date: Mon, 13 Jan 2020 12:02:25 +0100 Subject: [PATCH 1/3] If waiting for push device details and got them persisted, re-emit them. While once #966 is fixed new apps won't get into that "illegal" persisted state, we still need to recover from it for old apps. This also serves as a workaround, so that the "proper" fix for #966 is not urgent. --- Source/ARTPushActivationStateMachine+Private.h | 1 + Source/ARTPushActivationStateMachine.m | 13 +++++++++++++ Spec/PushActivationStateMachine.swift | 17 +++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/Source/ARTPushActivationStateMachine+Private.h b/Source/ARTPushActivationStateMachine+Private.h index 9f24efd3b..387053baf 100644 --- a/Source/ARTPushActivationStateMachine+Private.h +++ b/Source/ARTPushActivationStateMachine+Private.h @@ -19,6 +19,7 @@ extern NSString *const ARTPushActivationPendingEventsKey; @property (nonatomic, strong) ARTRestInternal *rest; - (instancetype)init:(ARTRestInternal *)rest; +- (instancetype)init:(ARTRestInternal *)rest delegate:(nullable id)delegate; @property (weak, nonatomic) id delegate; // weak because delegates outlive their counterpart @property (nonatomic, copy, nullable) void (^transitions)(ARTPushActivationEvent *event, ARTPushActivationState *from, ARTPushActivationState *to); diff --git a/Source/ARTPushActivationStateMachine.m b/Source/ARTPushActivationStateMachine.m index d2f2ec9a9..88f96cc5f 100644 --- a/Source/ARTPushActivationStateMachine.m +++ b/Source/ARTPushActivationStateMachine.m @@ -35,8 +35,13 @@ @implementation ARTPushActivationStateMachine { } - (instancetype)init:(ARTRestInternal *)rest { + return [self init:rest delegate:nil]; +} + +- (instancetype)init:(ARTRestInternal *)rest delegate:(id)delegate { if (self = [super init]) { _rest = rest; + _delegate = delegate; _queue = _rest.queue; _userQueue = _rest.userQueue; // Unarchiving @@ -52,6 +57,14 @@ - (instancetype)init:(ARTRestInternal *)rest { if (!_pendingEvents) { _pendingEvents = [NSMutableArray array]; } + + // Due to bug #966, old versions of the library might have led us to an illegal + // persisted state: we have a deviceToken, but the persisted push state is WaitingForPushDeviceDetails. + // So we need to re-emit the GotPushDeviceDetails event that led us there. + if ([_current isKindOfClass:[ARTPushActivationStateWaitingForPushDeviceDetails class]] && rest.device_nosync.deviceToken != nil) { + [rest.logger debug:@"ARTPush: re-emitting stored device details for stuck state machine"]; + [self handleEvent:[ARTPushActivationEventGotPushDeviceDetails new]]; + } } return self; } diff --git a/Spec/PushActivationStateMachine.swift b/Spec/PushActivationStateMachine.swift index 905813f08..183e8cbb4 100644 --- a/Spec/PushActivationStateMachine.swift +++ b/Spec/PushActivationStateMachine.swift @@ -409,7 +409,24 @@ class PushActivationStateMachine : QuickSpec { } } + + // https://github.com/ably/ably-cocoa/issues/966 + it("when initializing from persistent state with a deviceToken, GotPushDeviceDetails should be re-emitted") { + storage = MockDeviceStorage(startWith: ARTPushActivationStateWaitingForPushDeviceDetails(machine: initialStateMachine)) + rest.internal.storage = storage + rest.device.setAndPersistDeviceToken("foo") + + var registered = false + + let delegate = StateMachineDelegateCustomCallbacks() + stateMachine = ARTPushActivationStateMachine(rest.internal, delegate: delegate) + delegate.onPushCustomRegister = { error, deviceDetails in + registered = true + return nil + } + expect(registered).toEventually(beTrue()) + } } // RSH3c From 2573e03f39a8a86df743770c230ffcc639085bfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toni=20C=C3=A1rdenas?= Date: Mon, 13 Jan 2020 14:59:21 +0100 Subject: [PATCH 2/3] Ensure activation machine gets delegate at initialization time. --- Source/ARTPush.m | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/Source/ARTPush.m b/Source/ARTPush.m index e4573d0ee..6798dbe5f 100644 --- a/Source/ARTPush.m +++ b/Source/ARTPush.m @@ -106,7 +106,17 @@ - (ARTPushActivationStateMachine *)activationMachine { activationMachine_once_token = &once; static id activationMachineInstance; dispatch_once(&once, ^{ - activationMachineInstance = [[ARTPushActivationStateMachine alloc] init:self->_rest]; + // -[UIApplication delegate] is an UI API call, so needs to be called from main thread. + __block id delegate = nil; + if ([NSThread isMainThread]) { + delegate = UIApplication.sharedApplication.delegate; + } else { + dispatch_sync(dispatch_get_main_queue(), ^{ + delegate = UIApplication.sharedApplication.delegate; + }); + } + + activationMachineInstance = [[ARTPushActivationStateMachine alloc] init:self->_rest delegate:delegate]; }); return activationMachineInstance; } @@ -169,29 +179,11 @@ + (void)didFailToRegisterForRemoteNotificationsWithError:(NSError *)error rest:( } - (void)activate { - if (!self.activationMachine.delegate) { - dispatch_async(dispatch_get_main_queue(), ^{ - // -[UIApplication delegate] is an UI API call - self.activationMachine.delegate = UIApplication.sharedApplication.delegate; - [self.activationMachine sendEvent:[ARTPushActivationEventCalledActivate new]]; - }); - } - else { - [self.activationMachine sendEvent:[ARTPushActivationEventCalledActivate new]]; - } + [self.activationMachine sendEvent:[ARTPushActivationEventCalledActivate new]]; } - (void)deactivate { - if (!self.activationMachine.delegate) { - dispatch_async(dispatch_get_main_queue(), ^{ - // -[UIApplication delegate] is an UI API call - self.activationMachine.delegate = UIApplication.sharedApplication.delegate; - [self.activationMachine sendEvent:[ARTPushActivationEventCalledDeactivate new]]; - }); - } - else { - [self.activationMachine sendEvent:[ARTPushActivationEventCalledDeactivate new]]; - } + [self.activationMachine sendEvent:[ARTPushActivationEventCalledDeactivate new]]; } #endif From 285a3fb8292ae803d49062d9ae66e803bfda265b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toni=20C=C3=A1rdenas?= Date: Mon, 13 Jan 2020 16:02:20 +0100 Subject: [PATCH 3/3] Reset token to nil to avoid affecting other tests via shared rest. Even though the storage gets refreshed before each test, the token is cached in the rest instance, so other tests see it. --- Spec/PushActivationStateMachine.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Spec/PushActivationStateMachine.swift b/Spec/PushActivationStateMachine.swift index 183e8cbb4..c50d9b00e 100644 --- a/Spec/PushActivationStateMachine.swift +++ b/Spec/PushActivationStateMachine.swift @@ -415,6 +415,7 @@ class PushActivationStateMachine : QuickSpec { storage = MockDeviceStorage(startWith: ARTPushActivationStateWaitingForPushDeviceDetails(machine: initialStateMachine)) rest.internal.storage = storage rest.device.setAndPersistDeviceToken("foo") + defer { rest.device.setAndPersistDeviceToken(nil) } var registered = false