Closes #6194: Re-subscribe for push on subscription change events

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

@@ -17,6 +17,8 @@ import mozilla.components.concept.sync.DeviceConstellationObserver
 import mozilla.components.concept.sync.DevicePushSubscription
 import mozilla.components.concept.sync.OAuthAccount
 import mozilla.components.feature.push.AutoPushFeature
+import mozilla.components.feature.push.AutoPushSubscription
+import mozilla.components.feature.push.PushScope
 import mozilla.components.concept.sync.AccountObserver as SyncAccountObserver
 import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.service.fxa.manager.ext.withConstellation
@@ -74,7 +76,7 @@ class FxaPushSupportFeature(
     }
 
     init {
-        val autoPushObserver = AutoPushObserver(accountManager, fxaPushScope)
+        val autoPushObserver = AutoPushObserver(accountManager, pushFeature, fxaPushScope)
 
         val accountObserver = AccountObserver(
             context,
@@ -120,13 +122,7 @@ internal class AccountObserver(
             logger.debug("Subscribing for FxaPushScope ($fxaPushScope) events.")
 
             push.subscribe(fxaPushScope) { subscription ->
-                account.deviceConstellation().setDevicePushSubscriptionAsync(
-                    DevicePushSubscription(
-                        endpoint = subscription.endpoint,
-                        publicKey = subscription.publicKey,
-                        authKey = subscription.authKey
-                    )
-                )
+                account.deviceConstellation().setDevicePushSubscriptionAsync(subscription.into())
             }
         }
 
@@ -179,18 +175,18 @@ internal class ConstellationObserver(
  */
 internal class AutoPushObserver(
     private val accountManager: FxaAccountManager,
+    private val pushFeature: AutoPushFeature,
     private val fxaPushScope: String
 ) : AutoPushFeature.Observer {
     private val logger = Logger(AutoPushObserver::class.java.simpleName)
 
     override fun onMessageReceived(scope: String, message: ByteArray?) {
-        logger.info("Received new push message for $scope")
-
-        // Ignore messages that are not meant for us.
         if (scope != fxaPushScope) {
             return
         }
 
+        logger.info("Received new push message for $scope")
+
         // Ignore push messages that do not have data.
         val rawEvent = message ?: return
 
@@ -198,6 +194,25 @@ internal class AutoPushObserver(
             it.processRawEventAsync(String(rawEvent))
         }
     }
+
+    override fun onSubscriptionChanged(scope: PushScope) {
+        if (scope != fxaPushScope) {
+            return
+        }
+
+        logger.info("Our sync push scope ($scope) has expired. Re-subscribing..")
+
+        pushFeature.subscribe(fxaPushScope) { subscription ->
+            val account = accountManager.authenticatedAccount()
+
+            if (account == null) {
+                logger.info("We don't have any account to pass the push subscription to.")
+                return@subscribe
+            }
+
+            account.deviceConstellation().setDevicePushSubscriptionAsync(subscription.into())
+        }
+    }
 }
 
 /**
@@ -310,13 +325,7 @@ class OneTimeFxaPushReset(
 
         pushFeature.unsubscribe(pushScope)
         pushFeature.subscribe(newPushScope) { subscription ->
-            account.deviceConstellation().setDevicePushSubscriptionAsync(
-                DevicePushSubscription(
-                    endpoint = subscription.endpoint,
-                    publicKey = subscription.publicKey,
-                    authKey = subscription.authKey
-                )
-            )
+            account.deviceConstellation().setDevicePushSubscriptionAsync(subscription.into())
         }
 
         preference(context).edit().putString(PREF_FXA_SCOPE, newPushScope).apply()
@@ -327,3 +336,9 @@ internal data class VerificationState(val timestamp: Long, val totalCount: Int)
 
 @VisibleForTesting
 internal fun preference(context: Context) = context.getSharedPreferences(PREFERENCE_NAME, Context.MODE_PRIVATE)
+
+internal fun AutoPushSubscription.into() = DevicePushSubscription(
+    endpoint = this.endpoint,
+    publicKey = this.publicKey,
+    authKey = this.authKey
+)

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

@@ -11,6 +11,7 @@ import mozilla.components.concept.sync.AuthType
 import mozilla.components.concept.sync.DeviceConstellation
 import mozilla.components.concept.sync.OAuthAccount
 import mozilla.components.feature.push.AutoPushFeature
+import mozilla.components.feature.push.AutoPushSubscription
 import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.support.test.any
 import mozilla.components.support.test.eq
@@ -28,6 +29,7 @@ import org.mockito.Mockito.reset
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.verifyNoMoreInteractions
 import org.mockito.Mockito.verifyZeroInteractions
+import org.mockito.stubbing.OngoingStubbing
 import org.robolectric.RobolectricTestRunner
 
 @RunWith(RobolectricTestRunner::class)
@@ -148,6 +150,23 @@ class AccountObserverTest {
         verifyZeroInteractions(pushFeature)
     }
 
+    @Test
+    fun `notify account of new subscriptions`() {
+        val observer = AccountObserver(
+            testContext,
+            pushFeature,
+            pushScope,
+            mock(),
+            false
+        )
+
+        whenSubscribe()
+
+        observer.onAuthenticated(account, AuthType.Signin)
+
+        verify(constellation).setDevicePushSubscriptionAsync(any())
+    }
+
     @Test
     fun `feature and service invoked on logout`() {
         val observer = AccountObserver(
@@ -180,4 +199,21 @@ class AccountObserverTest {
 
         verifyNoMoreInteractions(pushFeature)
     }
+
+    @Suppress("UNCHECKED_CAST")
+    private fun whenSubscribe(): OngoingStubbing<Unit>? {
+        return `when`(pushFeature.subscribe(any(), nullable(), any(), any())).thenAnswer {
+
+            // Invoke the `onSubscribe` lambda with a fake subscription.
+            (it.arguments[3] as ((AutoPushSubscription) -> Unit)).invoke(
+                AutoPushSubscription(
+                    scope = "test",
+                    endpoint = "https://foo",
+                    publicKey = "p256dh",
+                    authKey = "auth",
+                    appServerKey = null
+                )
+            )
+        }
+    }
 }
\ No newline at end of file

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

@@ -6,22 +6,28 @@ package mozilla.components.feature.accounts.push
 
 import mozilla.components.concept.sync.DeviceConstellation
 import mozilla.components.concept.sync.OAuthAccount
+import mozilla.components.feature.push.AutoPushFeature
+import mozilla.components.feature.push.AutoPushSubscription
 import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
+import mozilla.components.support.test.nullable
 import org.junit.Test
 import org.mockito.Mockito.`when`
 import org.mockito.Mockito.never
 import org.mockito.Mockito.verify
+import org.mockito.Mockito.verifyZeroInteractions
+import org.mockito.stubbing.OngoingStubbing
 
 class AutoPushObserverTest {
+    private val manager: FxaAccountManager = mock()
+    private val account: OAuthAccount = mock()
+    private val constellation: DeviceConstellation = mock()
+    private val pushFeature: AutoPushFeature = mock()
 
     @Test
     fun `messages are forwarded to account manager`() {
-        val manager: FxaAccountManager = mock()
-        val account: OAuthAccount = mock()
-        val constellation: DeviceConstellation = mock()
-        val observer = AutoPushObserver(manager, "test")
+        val observer = AutoPushObserver(manager, mock(), "test")
 
         `when`(manager.authenticatedAccount()).thenReturn(account)
         `when`(account.deviceConstellation()).thenReturn(constellation)
@@ -33,9 +39,7 @@ class AutoPushObserverTest {
 
     @Test
     fun `account manager is not invoked if no account is available`() {
-        val manager: FxaAccountManager = mock()
-        val constellation: DeviceConstellation = mock()
-        val observer = AutoPushObserver(manager, "test")
+        val observer = AutoPushObserver(manager, mock(), "test")
 
         observer.onMessageReceived("test", "foobar".toByteArray())
 
@@ -45,16 +49,65 @@ class AutoPushObserverTest {
 
     @Test
     fun `messages are not forwarded to account manager if they are for a different scope`() {
-        val manager: FxaAccountManager = mock()
-        val account: OAuthAccount = mock()
-        val constellation: DeviceConstellation = mock()
-        val observer = AutoPushObserver(manager, "fake")
+        val observer = AutoPushObserver(manager, mock(), "fake")
+
+        observer.onMessageReceived("test", "foobar".toByteArray())
+
+        verify(constellation, never()).processRawEventAsync(any())
+    }
+
+    @Test
+    fun `subscription changes are forwarded to account manager`() {
+        val observer = AutoPushObserver(manager, pushFeature, "test")
+
+        whenSubscribe()
 
         `when`(manager.authenticatedAccount()).thenReturn(account)
         `when`(account.deviceConstellation()).thenReturn(constellation)
 
-        observer.onMessageReceived("test", "foobar".toByteArray())
+        observer.onSubscriptionChanged("test")
 
-        verify(constellation, never()).processRawEventAsync(any())
+        verify(constellation).setDevicePushSubscriptionAsync(any())
+    }
+
+    @Test
+    fun `do nothing if there is no account manager`() {
+        val observer = AutoPushObserver(manager, pushFeature, "test")
+
+        whenSubscribe()
+
+        observer.onSubscriptionChanged("test")
+
+        verify(constellation, never()).setDevicePushSubscriptionAsync(any())
+    }
+
+    @Test
+    fun `subscription changes are not forwarded to account manager if they are for a different scope`() {
+        val observer = AutoPushObserver(manager, mock(), "fake")
+
+        `when`(manager.authenticatedAccount()).thenReturn(account)
+        `when`(account.deviceConstellation()).thenReturn(constellation)
+
+        observer.onSubscriptionChanged("test")
+
+        verify(constellation, never()).setDevicePushSubscriptionAsync(any())
+        verifyZeroInteractions(pushFeature)
+    }
+
+    @Suppress("UNCHECKED_CAST")
+    private fun whenSubscribe(): OngoingStubbing<Unit>? {
+        return `when`(pushFeature.subscribe(any(), nullable(), any(), any())).thenAnswer {
+
+            // Invoke the `onSubscribe` lambda with a fake subscription.
+            (it.arguments[3] as ((AutoPushSubscription) -> Unit)).invoke(
+                AutoPushSubscription(
+                    scope = "test",
+                    endpoint = "https://foo",
+                    publicKey = "p256dh",
+                    authKey = "auth",
+                    appServerKey = null
+                )
+            )
+        }
     }
 }

docs/changelog.md

@@ -15,8 +15,11 @@ permalink: /changelog/
 * **service-accounts-push**
   * Fixed a bug where the push subscription was incorrectly cached and caused some `GeneralError`s.
 
-  * **feature-addons**
-    * Added `DefaultAddonUpdater.UpdateAttemptStorage` allows to query the last known state for a previous attempt to update an add-on.
+* **feature-addons**
+  * Added `DefaultAddonUpdater.UpdateAttemptStorage` allows to query the last known state for a previous attempt to update an add-on.
+
+* **feature-accounts-push**
+  * Re-subscribe for Sync push support when notified by `onSubscriptionChanged` events.
 
 # 37.0.0