Merge pull request #994 from ably/ECO-4706/fix-push-corner-cases

ttypic · web-flow · commit ced191d081f2 · 2024-04-03T09:52:58.000+01:00
[ECO-4706] fix: push notifications corner cases
diff --git a/android/src/main/java/io/ably/lib/push/ActivationContext.java b/android/src/main/java/io/ably/lib/push/ActivationContext.java
@@ -4,13 +4,15 @@
 import android.content.SharedPreferences;
 import android.preference.PreferenceManager;
 
+import androidx.annotation.VisibleForTesting;
 import com.google.firebase.messaging.FirebaseMessaging;
 
 import java.util.WeakHashMap;
 
 import io.ably.lib.rest.AblyRest;
 import io.ably.lib.types.AblyException;
 import io.ably.lib.types.Callback;
+import io.ably.lib.types.ClientOptions;
 import io.ably.lib.types.ErrorInfo;
 import io.ably.lib.types.RegistrationToken;
 import io.ably.lib.util.Log;
@@ -63,6 +65,8 @@ AblyRest getAbly() throws AblyException {
             Log.v(TAG, "getAbly(): returning existing Ably instance");
             return ably;
         } else {
+            // In this case, we received a new FCM token while the app is offline,
+            // so we have to initialize the Ably client to send it to the server.
             Log.v(TAG, "getAbly(): creating new Ably instance");
         }
 
@@ -72,9 +76,21 @@ AblyRest getAbly() throws AblyException {
             throw AblyException.fromErrorInfo(new ErrorInfo("Unable to get Ably library instance; no device identity token", 40000, 400));
         }
         Log.v(TAG, "getAbly(): returning Ably instance using deviceIdentityToken");
+        // TODO: We need to persist Ably client options such as the environment with `deviceIdentityToken` and use these options during initialization.
         return (ably = new AblyRest(deviceIdentityToken));
     }
 
+    /**
+     * @return AblyRest instance with device identity token auth. We use this instance to perform
+     * deregistration calls in push activation flow.
+     */
+    AblyRest getDeviceIdentityTokenBasedAblyClient(String deviceIdentityToken) throws AblyException {
+        ClientOptions clientOptions = ably.options.copy();
+        clientOptions.clearAuthOptions();
+        clientOptions.token = deviceIdentityToken;
+        return new AblyRest(clientOptions);
+    }
+
     public boolean setClientId(String clientId, boolean propagateGotPushDeviceDetails) {
         Log.v(TAG, "setClientId(): clientId=" + clientId + ", propagateGotPushDeviceDetails=" + propagateGotPushDeviceDetails);
         boolean updated = !clientId.equals(this.clientId);
@@ -113,6 +129,10 @@ public void onNewRegistrationToken(RegistrationToken.Type type, String token) {
         getActivationStateMachine().handleEvent(new ActivationStateMachine.GotPushDeviceDetails());
     }
 
+    /**
+     * Should be used in tests only
+     */
+    @VisibleForTesting
     public void reset() {
         Log.v(TAG, "reset()");
 
diff --git a/android/src/main/java/io/ably/lib/push/ActivationStateMachine.java b/android/src/main/java/io/ably/lib/push/ActivationStateMachine.java
@@ -227,8 +227,18 @@ public String toString() {
 
         public ActivationStateMachine.State transition(ActivationStateMachine.Event event) {
             if (event instanceof ActivationStateMachine.CalledDeactivate) {
-                machine.callDeactivatedCallback(null);
-                return this;
+                LocalDevice device = machine.getDevice();
+
+                // RSH3a1c
+                if (device.isRegistered()) {
+                    machine.deregister();
+                    return new ActivationStateMachine.WaitingForDeregistration(machine, this);
+                // RSH3a1d
+                } else {
+                    device.reset();
+                    machine.callDeactivatedCallback(null);
+                    return this;
+                }
             } else if (event instanceof ActivationStateMachine.CalledActivate) {
                 LocalDevice device = machine.getDevice();
 
@@ -388,7 +398,6 @@ public ActivationStateMachine.State transition(ActivationStateMachine.Event even
                 machine.callActivatedCallback(null);
                 return this;
             } else if (event instanceof ActivationStateMachine.CalledDeactivate) {
-                LocalDevice device = machine.getDevice();
                 machine.deregister();
                 return new ActivationStateMachine.WaitingForDeregistration(machine, this);
             } else if (event instanceof ActivationStateMachine.GotPushDeviceDetails) {
@@ -742,7 +751,8 @@ private void deregister() {
         } else {
             final AblyRest ably;
             try {
-                ably = activationContext.getAbly();
+                // RSH3d2b: use `deviceIdentityToken` to perform request
+                ably = activationContext.getDeviceIdentityTokenBasedAblyClient(device.deviceIdentityToken);
             } catch(AblyException ae) {
                 ErrorInfo reason = ae.errorInfo;
                 Log.e(TAG, "exception registering " + device.id + ": " + reason.toString());
@@ -751,9 +761,11 @@ private void deregister() {
             }
             ably.http.request(new Http.Execute<Void>() {
                 @Override
-                public void execute(HttpScheduler http, Callback<Void> callback) throws AblyException {
+                public void execute(HttpScheduler http, Callback<Void> callback) {
                     Param[] params = ParamsUtils.enrichParams(new Param[0], ably.options);
-                    http.del("/push/deviceRegistrations/" + device.id, ably.push.pushRequestHeaders(true), params, null, true, callback);
+                    Param[] headers = HttpUtils.defaultAcceptHeaders(ably.options.useBinaryProtocol);
+                    final Param[] deviceIdentityHeaders = device.deviceIdentityHeaders();
+                    http.del("/push/deviceRegistrations/" + device.id, HttpUtils.mergeHeaders(headers, deviceIdentityHeaders), params, null, true, callback);
                 }
             }).async(new Callback<Void>() {
                 @Override
@@ -763,8 +775,14 @@ public void onSuccess(Void response) {
                 }
                 @Override
                 public void onError(ErrorInfo reason) {
-                    Log.e(TAG, "error deregistering " + device.id + ": " + reason.toString());
-                    handleEvent(new ActivationStateMachine.DeregistrationFailed(reason));
+                    // RSH3d2c1: ignore unauthorized or invalid credentials errors
+                    if (reason.statusCode == 401 || reason.code == 40005) {
+                        Log.w(TAG, "unauthorized error during deregistration " + device.id + ": " + reason);
+                        handleEvent(new ActivationStateMachine.Deregistered());
+                    } else {
+                        Log.e(TAG, "error deregistering " + device.id + ": " + reason);
+                        handleEvent(new ActivationStateMachine.DeregistrationFailed(reason));
+                    }
                 }
             });
         }
diff --git a/lib/src/main/java/io/ably/lib/debug/DebugOptions.java b/lib/src/main/java/io/ably/lib/debug/DebugOptions.java
@@ -31,4 +31,53 @@ public interface RawHttpListener {
     public RawProtocolListener protocolListener;
     public RawHttpListener httpListener;
     public ITransport.Factory transportFactory;
+
+    public DebugOptions copy() {
+        DebugOptions copied = new DebugOptions();
+        copied.protocolListener = protocolListener;
+        copied.httpListener = httpListener;
+        copied.transportFactory = transportFactory;
+        copied.clientId = clientId;
+        copied.logLevel = logLevel;
+        copied.logHandler = logHandler;
+        copied.tls = tls;
+        copied.restHost = restHost;
+        copied.realtimeHost = realtimeHost;
+        copied.port = port;
+        copied.tlsPort = tlsPort;
+        copied.autoConnect = autoConnect;
+        copied.useBinaryProtocol = useBinaryProtocol;
+        copied.queueMessages = queueMessages;
+        copied.echoMessages = echoMessages;
+        copied.recover = recover;
+        copied.proxy = proxy;
+        copied.environment = environment;
+        copied.idempotentRestPublishing = idempotentRestPublishing;
+        copied.httpOpenTimeout = httpOpenTimeout;
+        copied.httpRequestTimeout = httpRequestTimeout;
+        copied.httpMaxRetryDuration = httpMaxRetryDuration;
+        copied.httpMaxRetryCount = httpMaxRetryCount;
+        copied.realtimeRequestTimeout = realtimeRequestTimeout;
+        copied.disconnectedRetryTimeout = disconnectedRetryTimeout;
+        copied.suspendedRetryTimeout = suspendedRetryTimeout;
+        copied.fallbackHostsUseDefault = fallbackHostsUseDefault;
+        copied.fallbackRetryTimeout = fallbackRetryTimeout;
+        copied.defaultTokenParams = defaultTokenParams;
+        copied.channelRetryTimeout = channelRetryTimeout;
+        copied.asyncHttpThreadpoolSize = asyncHttpThreadpoolSize;
+        copied.pushFullWait = pushFullWait;
+        copied.localStorage = localStorage;
+        copied.addRequestIds = addRequestIds;
+        copied.authCallback = authCallback;
+        copied.authUrl = authUrl;
+        copied.authMethod = authMethod;
+        copied.key = key;
+        copied.token = token;
+        copied.tokenDetails = tokenDetails;
+        copied.authHeaders = authHeaders;
+        copied.authParams = authParams;
+        copied.queryTime = queryTime;
+        copied.useTokenAuth = useTokenAuth;
+        return copied;
+    }
 }
diff --git a/lib/src/main/java/io/ably/lib/types/ClientOptions.java b/lib/src/main/java/io/ably/lib/types/ClientOptions.java
@@ -11,7 +11,7 @@
 
 /**
  * Passes additional client-specific properties to the {@link io.ably.lib.rest.AblyRest} or the {@link io.ably.lib.realtime.AblyRealtime}.
- *
+ * <p>
  * Extends an {@link AuthOptions} object.
  * <p>
  * Spec: TO3j
@@ -25,6 +25,7 @@ public ClientOptions() {}
 
     /**
      * Creates a ClientOptions instance used to configure Rest and Realtime clients
+     *
      * @param key the key obtained from the application dashboard.
      * @throws AblyException if the key is not in a valid format
      */
@@ -322,4 +323,70 @@ public ClientOptions(String key) throws AblyException {
      * Spec: RSC7d6
      */
     public Map<String, String> agents;
+
+    /**
+     * Internal method
+     *
+     * @return copy of client options
+     */
+    public ClientOptions copy() {
+        ClientOptions copied = new ClientOptions();
+        copied.clientId = clientId;
+        copied.logLevel = logLevel;
+        copied.logHandler = logHandler;
+        copied.tls = tls;
+        copied.restHost = restHost;
+        copied.realtimeHost = realtimeHost;
+        copied.port = port;
+        copied.tlsPort = tlsPort;
+        copied.autoConnect = autoConnect;
+        copied.useBinaryProtocol = useBinaryProtocol;
+        copied.queueMessages = queueMessages;
+        copied.echoMessages = echoMessages;
+        copied.recover = recover;
+        copied.proxy = proxy;
+        copied.environment = environment;
+        copied.idempotentRestPublishing = idempotentRestPublishing;
+        copied.httpOpenTimeout = httpOpenTimeout;
+        copied.httpRequestTimeout = httpRequestTimeout;
+        copied.httpMaxRetryDuration = httpMaxRetryDuration;
+        copied.httpMaxRetryCount = httpMaxRetryCount;
+        copied.realtimeRequestTimeout = realtimeRequestTimeout;
+        copied.disconnectedRetryTimeout = disconnectedRetryTimeout;
+        copied.suspendedRetryTimeout = suspendedRetryTimeout;
+        copied.fallbackHostsUseDefault = fallbackHostsUseDefault;
+        copied.fallbackRetryTimeout = fallbackRetryTimeout;
+        copied.defaultTokenParams = defaultTokenParams;
+        copied.channelRetryTimeout = channelRetryTimeout;
+        copied.asyncHttpThreadpoolSize = asyncHttpThreadpoolSize;
+        copied.pushFullWait = pushFullWait;
+        copied.localStorage = localStorage;
+        copied.addRequestIds = addRequestIds;
+        copied.authCallback = authCallback;
+        copied.authUrl = authUrl;
+        copied.authMethod = authMethod;
+        copied.key = key;
+        copied.token = token;
+        copied.tokenDetails = tokenDetails;
+        copied.authHeaders = authHeaders;
+        copied.authParams = authParams;
+        copied.queryTime = queryTime;
+        copied.useTokenAuth = useTokenAuth;
+        return copied;
+    }
+
+    /**
+     * Internal method
+     * <p>
+     * clears all auth options
+     */
+    public void clearAuthOptions() {
+        key = null;
+        token = null;
+        tokenDetails = null;
+        authHeaders = null;
+        authParams = null;
+        queryTime = false;
+        useTokenAuth = false;
+    }
 }
