Closes mozilla-mobile#3173: Check push subscriptions on interval

jonalmeida · jonalmeida · commit c8c342402c12 · 2019-10-25T10:34:04.000-04:00
diff --git a/components/feature/push/README.md b/components/feature/push/README.md
@@ -4,7 +4,7 @@ A component that implements push notifications with a supported push service.
 
 ## Usage
 
-Add a supported push service for providing the encrypted messages (for example Firebase Cloud Messaging via `lib-push-firebase`):
+Add a supported push service for providing the encrypted messages (for example, Firebase Cloud Messaging via `lib-push-firebase`):
 ```kotlin
 class FirebasePush : AbstractFirebasePushService()
 ```
@@ -14,7 +14,7 @@ Create a push configuration with the project info and also place the required se
 ```kotlin
 PushConfig(
   senderId = "push-test-f408f",
-  serverHost = "push.service.mozilla.com",
+  serverHost = "updates.push.services.mozilla.com",
   serviceType = ServiceType.FCM,
   protocol = Protocol.HTTPS
 )
@@ -62,8 +62,25 @@ Use Gradle to download the library from [maven.mozilla.org](https://maven.mozill
 implementation "org.mozilla.components:feature-push:{latest-version}"
 ```
 
+## Implementation Notes
+
+Why do we need to verify connections, and what happens when we do?
+- Various services may need to communicate with us via push messages. Examples: FxA events (send tab, etc), WebPush (a web app receives a push message from its server).
+- To send these push messages, services (FxA, random internet servers talking to their web apps) post an HTTP request to a "push endpoint" maintained by [Mozilla's Autopush service][0]. This push endpoint is specific to its recipient - so one instance of an app may have many endpoints associated with it: one for the current FxA device, a few for web apps, etc.
+- Important point here: servers (FxA, services behind web apps, etc.) need to be told about subscription info we get from Autopush.
+- Here is where things start to get complicated: client (us) and server (Autopush) may disagree on which channels are associated with the current UAID (remember: our subscriptions are per-channel). Channels may expire (TTL'd) or may be deleted by some server's Cron job if they're unused. For example, if this happens, services that use this subscription info (e.g. FxA servers) to communication with their clients (FxA devices) will fail to deliver push messages.
+- So the client needs to be able to find out that this is the case, re-create channel subscriptions on Autopush, and update any dependent services with new subscription info (e.g. update the FxA device record for `PushType.Services`, or notify the JS code with a `pushsubscriptionchanged` event for WebPush).
+- The Autopush side of this is `verify_connection` API - we're expected to call this periodically, and that library will compare channel registrations that the server knows about vs those that the client knows about.
+- If those are misaligned, we need to re-register affected (or, all?) channels, and notify related services so that they may update their own server-side records.
+- For FxA, this means that we need to have an instance of the rust FirefoxAccount object around in order to call `setDevicePushSubscriptionAsync` once we re-generate our push subscription.
+- For consumers such as Fenix, easiest way to access that method is via an `account manager`.
+- However, neither account object itself, nor the account manager, aren't available from within a Worker. It's possible to "re-hydrate" (instantiate rust object from the locally persisted state) a FirefoxAccount instance, but that's a separate can of worms, and needs to be carefully considered.
+- Similarly for WebPush (in the future), we will need to have Gecko around in order to fire `pushsubscriptionchanged` javascript events.
+
 ## License
 
     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/
+
+[0]: https://github.com/mozilla-services/autopush
diff --git a/components/feature/push/src/main/java/mozilla/components/feature/push/AutoPushFeature.kt b/components/feature/push/src/main/java/mozilla/components/feature/push/AutoPushFeature.kt
@@ -6,6 +6,7 @@ package mozilla.components.feature.push
 
 import android.content.Context
 import android.content.SharedPreferences
+import androidx.annotation.VisibleForTesting
 import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.ProcessLifecycleOwner
 import kotlinx.coroutines.CoroutineScope
@@ -61,7 +62,7 @@ import mozilla.appservices.push.PushError as RustPushError
  * </code>
  *
  */
-@Suppress("TooManyFunctions")
+@Suppress("TooManyFunctions", "LargeClass")
 class AutoPushFeature(
     private val context: Context,
     private val service: PushService,
@@ -82,6 +83,9 @@ class AutoPushFeature(
     // The preference that stores new registration tokens.
     private val prefToken: String?
         get() = preferences(context).getString(PREF_TOKEN, null)
+    private var prefLastVerified: Long
+        get() = preferences(context).getLong(LAST_VERIFIED, System.currentTimeMillis())
+        set(value) = preferences(context).edit().putLong(LAST_VERIFIED, value).apply()
 
     internal var job: Job = SupervisorJob()
     private val scope = CoroutineScope(coroutineContext) + job
@@ -98,12 +102,18 @@ class AutoPushFeature(
     }
 
     /**
-     * Starts the push service provided.
+     * Starts the push feature and initialization work needed.
      */
-    override fun initialize() { service.start(context) }
+    override fun initialize() {
+        // Starts the push feature.
+        service.start(context)
+
+        // Starts verification of push subscription endpoints.
+        tryVerifySubscriptions()
+    }
 
     /**
-     * Un-subscribes from all push message channels and stops the push service.
+     * Un-subscribes from all push message channels, stops the push service, and stops periodic verifications.
      * This should only be done on an account logout or app data deletion.
      */
     override fun shutdown() {
@@ -118,6 +128,9 @@ class AutoPushFeature(
                 job.cancel()
             }
         }
+
+        // Reset the push subscription check.
+        prefLastVerified = 0L
     }
 
     /**
@@ -239,11 +252,6 @@ class AutoPushFeature(
     /**
      * Deletes the registration token locally so that it forces the service to get a new one the
      * next time hits it's messaging server.
-     *
-     * Implementation notes: This shouldn't need to be used unless we're certain. When we introduce
-     * [a polling service][0] to check if endpoints are expired, we would invoke this.
-     *
-     * [0]: https://github.com/mozilla-mobile/android-components/issues/3173
      */
     override fun renewRegistration() {
         logger.warn("Forcing registration renewal by deleting our (cached) token.")
@@ -259,6 +267,38 @@ class AutoPushFeature(
         service.start(context)
     }
 
+    /**
+     * Verifies status (active, expired) of the push subscriptions and then notifies observers.
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    internal fun verifyActiveSubscriptions() {
+        DeliveryManager.with(connection) {
+            scope.launchAndTry {
+                val notifyObservers = connection.verifyConnection()
+
+                if (notifyObservers) {
+                    logger.info("Subscriptions have changed; notifying observers..")
+
+                    PushType.values().forEach { type ->
+                        val sub = subscribe(type.toChannelId()).toPushSubscription()
+                        subscriptionObservers.notifyObservers { onSubscriptionAvailable(sub) }
+                    }
+                }
+            }
+        }
+    }
+
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    internal fun tryVerifySubscriptions() {
+        logger.info("Checking validity of push subscriptions.")
+
+        if (shouldVerifyNow()) {
+            verifyActiveSubscriptions()
+
+            prefLastVerified = System.currentTimeMillis()
+        }
+    }
+
     private fun CoroutineScope.launchAndTry(block: suspend CoroutineScope.() -> Unit) {
         job = launch {
             try {
@@ -280,10 +320,20 @@ class AutoPushFeature(
     private fun preferences(context: Context): SharedPreferences =
         context.getSharedPreferences(PREFERENCE_NAME, Context.MODE_PRIVATE)
 
+    /**
+     * We should verify if it's been [PERIODIC_INTERVAL_MILLISECONDS] since our last attempt (successful or not).
+     */
+    private fun shouldVerifyNow(): Boolean {
+        return (System.currentTimeMillis() - prefLastVerified) >= PERIODIC_INTERVAL_MILLISECONDS
+    }
+
     companion object {
         internal const val PREFERENCE_NAME = "mozac_feature_push"
         internal const val PREF_TOKEN = "token"
         internal const val DB_NAME = "push.sqlite"
+
+        internal const val LAST_VERIFIED = "last_verified_push_connection"
+        internal const val PERIODIC_INTERVAL_MILLISECONDS = 24 * 60 * 60 * 1000L // 24 hours
     }
 }
 
diff --git a/components/feature/push/src/test/java/mozilla/components/feature/push/AutoPushFeatureTest.kt b/components/feature/push/src/test/java/mozilla/components/feature/push/AutoPushFeatureTest.kt
@@ -17,6 +17,8 @@ import mozilla.appservices.push.SubscriptionResponse
 import mozilla.components.concept.push.Bus
 import mozilla.components.concept.push.EncryptedPushMessage
 import mozilla.components.concept.push.PushService
+import mozilla.components.feature.push.AutoPushFeature.Companion.LAST_VERIFIED
+import mozilla.components.feature.push.AutoPushFeature.Companion.PERIODIC_INTERVAL_MILLISECONDS
 import mozilla.components.feature.push.AutoPushFeature.Companion.PREFERENCE_NAME
 import mozilla.components.feature.push.AutoPushFeature.Companion.PREF_TOKEN
 import mozilla.components.support.test.any
@@ -31,32 +33,37 @@ import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.ArgumentMatchers.anyString
+import org.mockito.Mockito.`when`
 import org.mockito.Mockito.never
 import org.mockito.Mockito.spy
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
+import org.mockito.Mockito.verifyNoMoreInteractions
 
 @ExperimentalCoroutinesApi
 @RunWith(AndroidJUnit4::class)
 class AutoPushFeatureTest {
 
+    var lastVerified: Long
+        get() = preference(testContext).getLong(LAST_VERIFIED, System.currentTimeMillis())
+        set(value) = preference(testContext).edit().putLong(LAST_VERIFIED, value).apply()
+
     @Before
     fun setup() {
-        preference(testContext)
-            .edit()
-            .clear()
-            .apply()
+        lastVerified = 0L
     }
 
     @Test
     fun `initialize starts push service`() {
         val service: PushService = mock()
         val config = PushConfig("push-test")
-        val feature = AutoPushFeature(testContext, service, config)
+        val feature = spy(AutoPushFeature(testContext, service, config))
 
         feature.initialize()
 
         verify(service).start(testContext)
+
+        verifyNoMoreInteractions(service)
     }
 
     @Test
@@ -74,8 +81,10 @@ class AutoPushFeatureTest {
 
         preference(testContext).edit().putString(PREF_TOKEN, "token").apply()
 
-        AutoPushFeature(testContext, mock(), mock(), connection = connection,
-            coroutineContext = coroutineContext)
+        AutoPushFeature(
+            testContext, mock(), mock(), connection = connection,
+            coroutineContext = coroutineContext
+        )
 
         verify(connection).updateToken("token")
     }
@@ -213,14 +222,107 @@ class AutoPushFeatureTest {
         assertNull(pref)
     }
 
-    private fun preference(context: Context): SharedPreferences {
-        return context.getSharedPreferences(PREFERENCE_NAME, Context.MODE_PRIVATE)
+    @Test
+    fun `verifyActiveSubscriptions notifies subscribers`() = runBlockingTest {
+        val connection: PushConnection = spy(TestPushConnection(true))
+        val owner: LifecycleOwner = mock()
+        val lifecycle: Lifecycle = mock()
+        val observers: PushSubscriptionObserver = mock()
+        val feature = spy(AutoPushFeature(testContext, mock(), mock(), coroutineContext, connection))
+        whenever(owner.lifecycle).thenReturn(lifecycle)
+        whenever(lifecycle.currentState).thenReturn(Lifecycle.State.STARTED)
+
+        feature.registerForSubscriptions(observers)
+
+        // When there are NO subscription updates, observers should not be notified.
+        feature.verifyActiveSubscriptions()
+
+        verify(observers, never()).onSubscriptionAvailable(any())
+
+        // When there are subscription updates, observers should not be notified.
+        whenever(connection.verifyConnection()).thenReturn(true)
+        feature.verifyActiveSubscriptions()
+
+        verify(observers, times(2)).onSubscriptionAvailable(any())
+    }
+
+    @Test
+    fun `initialize executes verifyActiveSubscriptions after interval`() = runBlockingTest {
+        val feature = spy(
+            AutoPushFeature(
+                context = testContext,
+                service = mock(),
+                config = mock(),
+                coroutineContext = coroutineContext,
+                connection = mock()
+            )
+        )
+
+        lastVerified = System.currentTimeMillis() - VERIFY_NOW
+
+        feature.initialize()
+
+        verify(feature).tryVerifySubscriptions()
+    }
+
+    @Test
+    fun `initialize does not execute verifyActiveSubscription before interval`() = runBlockingTest {
+        val feature = spy(
+            AutoPushFeature(
+                context = testContext,
+                service = mock(),
+                config = mock(),
+                coroutineContext = coroutineContext,
+                connection = mock()
+            )
+        )
+
+        lastVerified = System.currentTimeMillis() - SKIP_INTERVAL
+
+        feature.initialize()
+
+        verify(feature, never()).verifyActiveSubscriptions()
+    }
+
+    @Test
+    fun `verifySubscriptions notifies observers`() = runBlockingTest {
+        val owner: LifecycleOwner = mock()
+        val lifecycle: Lifecycle = mock()
+        val native: PushConnection = TestPushConnection(true)
+        val feature = spy(
+            AutoPushFeature(
+                context = testContext,
+                service = mock(),
+                config = mock(),
+                coroutineContext = coroutineContext,
+                connection = native
+            )
+        )
+        `when`(owner.lifecycle).thenReturn(lifecycle)
+        `when`(lifecycle.currentState).thenReturn(Lifecycle.State.STARTED)
+
+        feature.registerForSubscriptions(object : PushSubscriptionObserver {
+            override fun onSubscriptionAvailable(subscription: AutoPushSubscription) {
+                assertEquals("https://fool", subscription.endpoint)
+            }
+        }, owner, false)
+
+        feature.verifyActiveSubscriptions()
+    }
+
+    companion object {
+        private fun preference(context: Context): SharedPreferences {
+            return context.getSharedPreferences(PREFERENCE_NAME, Context.MODE_PRIVATE)
+        }
+
+        private const val SKIP_INTERVAL = 23 * 60 * 60 * 1000L // 23 hours; less than interval
+        private const val VERIFY_NOW = PERIODIC_INTERVAL_MILLISECONDS + (10 * 60 * 1000) // interval + 10 mins
     }
 
     class TestPushConnection(private val init: Boolean = false) : PushConnection {
         override suspend fun subscribe(channelId: String, scope: String) =
             SubscriptionResponse(
-                "992a0f0542383f1ea5ef51b7cf4ae6c4",
+                channelId,
                 SubscriptionInfo("https://foo", KeyInfo("auth", "p256dh"))
             )
 
@@ -230,9 +332,7 @@ class AutoPushFeatureTest {
 
         override suspend fun updateToken(token: String) = true
 
-        override suspend fun verifyConnection(): Boolean {
-            TODO("not implemented")
-        }
+        override suspend fun verifyConnection(): Boolean = false
 
         override fun decrypt(
             channelId: String,
