Close #9011: Unsubscribe before subscribing to fix FxA push

components/feature/accounts-push/build.gradle

@@ -48,6 +48,7 @@ dependencies {
     testImplementation project(':support-test')
 
     testImplementation Dependencies.androidx_test_core
+    testImplementation Dependencies.androidx_test_junit
     testImplementation Dependencies.testing_junit
     testImplementation Dependencies.testing_mockito
     testImplementation Dependencies.testing_robolectric

components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt

@@ -22,6 +22,7 @@ import mozilla.components.concept.sync.DeviceConstellation
 import mozilla.components.concept.sync.DeviceConstellationObserver
 import mozilla.components.concept.sync.DevicePushSubscription
 import mozilla.components.concept.sync.OAuthAccount
+import mozilla.components.feature.accounts.push.ext.redactPartialUri
 import mozilla.components.feature.push.AutoPushFeature
 import mozilla.components.feature.push.AutoPushSubscription
 import mozilla.components.feature.push.PushScope
@@ -203,6 +204,11 @@ internal class ConstellationObserver(
         }
 
         logger.info("We have been notified that our push subscription has expired; re-subscribing.")
+        push.unsubscribe(
+            scope = scope,
+            onUnsubscribeError = ::onUnsubscribeError,
+            onUnsubscribe = ::onUnsubscribeResult
+        )
         push.subscribe(
             scope = scope,
             onSubscribeError = ::onSubscribeError,
@@ -223,12 +229,15 @@ internal class ConstellationObserver(
         if (subscription.endpoint == oldEndpoint) {
             val exception = IllegalStateException(
                 "New push endpoint matches existing one",
-                Throwable("New endpoint: ${subscription.endpoint}\nOld endpoint: $oldEndpoint")
+                Throwable(
+                    "New endpoint: ${subscription.endpoint.redactPartialUri()}\n" +
+                    "Old endpoint: ${oldEndpoint.redactPartialUri()}"
+                )
             )
 
-            crashReporter?.submitCaughtException(exception)
-
             logger.warn("Push endpoints match!", exception)
+
+            crashReporter?.submitCaughtException(exception)
         }
 
         CoroutineScope(Dispatchers.Main).launch {
@@ -237,14 +246,42 @@ internal class ConstellationObserver(
     }
 
     internal fun onSubscribeError(e: Exception) {
-        fun Exception.toBreadcrumb() = Breadcrumb(
-            message = "Re-subscribing to FxA push failed after subscriptionExpired",
-            data = mapOf(
-                "exception" to javaClass.name,
-                "message" to message.orEmpty()
+        logger.warn("Re-subscribing failed; FxA push events will not be received.", e)
+        breadcrumb(
+            exception = e,
+            message = "Re-subscribing to failed FxA push after subscriptionExpired"
+        )
+    }
+
+    internal fun onUnsubscribeError(e: Exception) {
+        logger.warn("Un-subscribing failed; subscribe may return the same endpoint", e)
+        breadcrumb(
+            exception = e,
+            message = "Un-subscribing to failed FxA push after subscriptionExpired"
+        )
+    }
+
+    internal fun onUnsubscribeResult(success: Boolean) {
+        logger.info("Un-subscribing successful: $success")
+        if (success) {
+            logger.info("Subscribe call should give you a new endpoint.")
+        }
+    }
+
+    private fun breadcrumb(
+        exception: Exception,
+        message: String
+    ) {
+        crashReporter?.recordCrashBreadcrumb(
+            Breadcrumb(
+                category = ConstellationObserver::class.java.simpleName,
+                message = message,
+                data = mapOf(
+                    "exception" to exception.javaClass.name,
+                    "message" to exception.message.orEmpty()
+                )
             )
         )
-        crashReporter?.recordCrashBreadcrumb(e.toBreadcrumb())
     }
 }
 

components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/ext/String.kt

@@ -0,0 +1,34 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.feature.accounts.push.ext
+
+import android.net.Uri
+
+/**
+ * Removes all expect the [lastSegmentToTake] of the [Uri.getLastPathSegment] result.
+ *
+ * Example:
+ * ```
+ * val url = https://mozilla.org/firefox
+ * val redactedUrl = url.redactPartialUri(3, "redacted...")
+ *
+ * assertEquals("https://mozilla.org/redacted...fox", redactedUrl)
+ * ```
+ */
+fun String.redactPartialUri(lastSegmentToTake: Int = 20, shortForm: String = "redacted..."): String {
+    val uri = Uri.parse(this)
+    val end = shortForm + uri.lastPathSegment?.takeLast(lastSegmentToTake)
+    val path = uri
+        .pathSegments
+        .take(uri.pathSegments.size - 1)
+        .joinToString("/")
+
+    return uri
+        .buildUpon()
+        .path(path)
+        .appendPath(end)
+        .build()
+        .toString()
+}

components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ConstellationObserverTest.kt

@@ -7,6 +7,7 @@
 package mozilla.components.feature.accounts.push
 
 import android.content.Context
+import androidx.test.ext.junit.runners.AndroidJUnit4
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.test.TestCoroutineDispatcher
 import kotlinx.coroutines.test.setMain
@@ -26,10 +27,12 @@ import org.junit.Before
 import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
+import org.junit.runner.RunWith
 import org.mockito.Mockito.`when`
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.verifyZeroInteractions
 
+@RunWith(AndroidJUnit4::class)
 class ConstellationObserverTest {
 
     private val push: AutoPushFeature = mock()
@@ -114,6 +117,7 @@ class ConstellationObserverTest {
 
         observer.onDevicesUpdate(state)
 
+        verify(push).unsubscribe(eq("testScope"), any(), any())
         verify(push).subscribe(eq("testScope"), any(), any(), any())
         verify(verifier).increment()
     }
@@ -129,7 +133,7 @@ class ConstellationObserverTest {
         `when`(account.deviceConstellation()).thenReturn(constellation)
         `when`(state.currentDevice).thenReturn(device)
         `when`(device.subscription).thenReturn(subscription)
-        `when`(subscription.endpoint).thenReturn("https://example.com")
+        `when`(subscription.endpoint).thenReturn("https://example.com/foobar")
 
         observer.onSubscribe(state, testSubscription())
 
@@ -145,5 +149,20 @@ class ConstellationObserverTest {
         verify(crashReporter).recordCrashBreadcrumb(any())
     }
 
-    private fun testSubscription() = AutoPushSubscription(scope = "testScope", endpoint = "https://example.com", publicKey = "", authKey = "", appServerKey = null)
+    @Test
+    fun `notify crash reporter if un-subscribe error occurs`() {
+        val observer = ConstellationObserver(context, push, "testScope", account, verifier, crashReporter)
+
+        observer.onUnsubscribeError(mock())
+
+        verify(crashReporter).recordCrashBreadcrumb(any())
+    }
+
+    private fun testSubscription() = AutoPushSubscription(
+        scope = "testScope",
+        endpoint = "https://example.com/foobar",
+        publicKey = "",
+        authKey = "",
+        appServerKey = null
+    )
 }

components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ext/StringKtTest.kt

@@ -0,0 +1,25 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ *  License, v. 2.0. If a copy of the MPL was not distributed with this
+ *  file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+package mozilla.components.feature.accounts.push.ext
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+class StringKtTest {
+
+    @Test
+    fun `redacted endpoint contains short form in it`() {
+        val endpoint =
+            "https://updates.push.services.mozilla.com/wpush/v1/gAAAAABfAL4OBMRjP3O66lugUrcHT8kk4ENnJP4SE67US" +
+                "kmH9NdIz__-_3PtC_V79-KwG73Y3mZye1qtnYzoJETaGQidjgbiJdXzB7u0T9BViE2b7O3oqsFJpnwvmO-CiFqKKP14vitH"
+
+        assertTrue(endpoint.redactPartialUri().contains("redacted..."))
+    }
+}