Push activation state machine gets stuck in WaitingForDeregistration state after failing to deregister for push

[lawrence-forooghian]

What's the issue?

RSG3g3b says that a push activation state machine in WaitingForDeregistration should, upon receiving a DeregistrationFailed event, transition back to its previous state. However, as can be seen in the code, ably-cocoa remains in the WaitingForDeregistration state upon receiving this event.

This means that the library is stuck in this state, and there is no way to transition out of it.

Failing unit tests demonstrating the issue

From 16a68e46f048b6e6e3f9cbbafd806a7265fd169f Mon Sep 17 00:00:00 2001
From: Lawrence Forooghian <[email protected]>
Date: Thu, 20 Jan 2022 15:20:19 -0300
Subject: [PATCH] =?UTF-8?q?Add=20tests=20demonstrating=20that=20RSH3g3b=20?=
 =?UTF-8?q?isn=E2=80=99t=20respected?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../PushActivationStateMachineTests.swift     | 57 ++++++++++++++++---
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/Spec/Tests/PushActivationStateMachineTests.swift b/Spec/Tests/PushActivationStateMachineTests.swift
index 97cd981b..8c0b1a58 100644
--- a/Spec/Tests/PushActivationStateMachineTests.swift
+++ b/Spec/Tests/PushActivationStateMachineTests.swift
@@ -835,10 +835,31 @@ class PushActivationStateMachineTests: XCTestCase {
 
     // RSH3g
 
-    func beforeEach__Activation_state_machine__State_WaitingForDeregistration() {
-        storage = MockDeviceStorage(startWith: ARTPushActivationStateWaitingForDeregistration(machine: initialStateMachine))
-        rest.internal.storage = storage
-        stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate())
+    func beforeEach__Activation_state_machine__State_WaitingForDeregistration<T: ARTPushActivationState>(withPreviousStateType previousStateType: T.Type? = nil) {
+        if let previousStateType = previousStateType {
+            storage = MockDeviceStorage(startWith: previousStateType.init(machine: initialStateMachine))
+            rest.internal.storage = storage
+            stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate())
+            
+            // This dance here is to start off in our "previous state" and then poke
+            // the state machine to make it transition into WaitingForDeregistration;
+            // there might be a neater way to do this
+            
+            // WaitingForNewPushDeviceDetails or AfterRegistrationSyncFailed -> WaitingForDeregistration needs CalledDeactivate and a fake deregisterCallback (RSH3d2, RSH3f2)
+            let delegate = StateMachineDelegateCustomCallbacks()
+            delegate.onPushCustomDeregister = { _, _ -> NSError? in
+                print("onPushCustomDeregister")
+                return nil
+            }
+            stateMachine.delegate = delegate
+            stateMachine.send(ARTPushActivationEventCalledDeactivate())
+            
+            expect(stateMachine.current).to(beAKindOf(ARTPushActivationStateWaitingForDeregistration.self), description: "Expected these steps to have transitioned the state machine to our desired initial state of WaitingForDeregistration")
+        } else {
+            storage = MockDeviceStorage(startWith: ARTPushActivationStateWaitingForDeregistration(machine: initialStateMachine))
+            rest.internal.storage = storage
+            stateMachine = ARTPushActivationStateMachine(rest: rest.internal, delegate: StateMachineDelegate())
+        }
     }
 
     // RSH3g1
@@ -880,8 +901,8 @@ class PushActivationStateMachineTests: XCTestCase {
         expect(stateMachine.rest.device.storage.object(forKey: ARTDeviceIdentityTokenKey)).to(beNil())
     }
 
-    // RSH3g3
-    func test__055__Activation_state_machine__State_WaitingForDeregistration__on_Event_DeregistrationFailed() {
+    // RSH3g3a
+    func test__055__Activation_state_machine__State_WaitingForDeregistration__on_Event_DeregistrationFailed__callsCallbackWithError() {
         beforeEach__Activation_state_machine__State_WaitingForDeregistration()
 
         let expectedError = ARTErrorInfo(domain: ARTAblyErrorDomain, code: 1234, userInfo: nil)
@@ -897,9 +918,31 @@ class PushActivationStateMachineTests: XCTestCase {
         defer { hook.remove() }
 
         stateMachine.send(ARTPushActivationEventDeregistrationFailed(error: expectedError))
-        expect(stateMachine.current).to(beAKindOf(ARTPushActivationStateWaitingForDeregistration.self))
+        // We need to maintain this call to stateMachine.current, since it
+        // appears that the delay that its dispatch_sync contains is needed
+        // else the deactivate callback hasn’t been called by the time the
+        // assertion executes.
+        // TODO change this to an expectation that fulfills when the callback is called
+        let _ = stateMachine.current
         expect(deactivatedCallbackCalled).to(beTrue())
     }
+    
+    // RSH3g3b
+    func test__Activation_state_machine__State_WaitingForDeregistration__on_Event_DeregistrationFailed__transitionsBackToPreviousState<T: ARTPushActivationState>(withPreviousStateType previousStateType: T.Type? = nil) {
+        beforeEach__Activation_state_machine__State_WaitingForDeregistration(withPreviousStateType: previousStateType)
+
+        let error = ARTErrorInfo(domain: ARTAblyErrorDomain, code: 1234, userInfo: nil)
+        stateMachine.send(ARTPushActivationEventDeregistrationFailed(error: error))
+        expect(stateMachine.current).to(beAKindOf(T.self))
+    }
+    
+    func test__Activation_state_machine__State_WaitingForDeregistration__withPreviousState__WaitingForNewPushDeviceDetails__on_Event_DeregistrationFailed__transitionsBackToPreviousState() {
+        test__Activation_state_machine__State_WaitingForDeregistration__on_Event_DeregistrationFailed__transitionsBackToPreviousState(withPreviousStateType: ARTPushActivationStateWaitingForNewPushDeviceDetails.self)
+    }
+    
+    func test__Activation_state_machine__State_WaitingForDeregistration__withPreviousState__AfterRegistrationSyncFailed__on_Event_DeregistrationFailed__transitionsBackToPreviousState() {
+        test__Activation_state_machine__State_WaitingForDeregistration__on_Event_DeregistrationFailed__transitionsBackToPreviousState(withPreviousStateType: ARTPushActivationStateAfterRegistrationSyncFailed.self)
+    }
 
     // RSH4
     func test__005__Activation_state_machine__should_queue_event_that_has_no_transition_defined_for_it() {
-- 
2.34.1

┆Issue is synchronized with this Jira Bug by Unito