From 33393280c6e60d93da0d115999e73fad5ae52d8b Mon Sep 17 00:00:00 2001 From: Jonathan Almeida Date: Tue, 3 Nov 2020 12:33:15 -0500 Subject: [PATCH] Close #8846: Re-subscribe FxA push subscription on subscriptionExpired This is a workaround fix to re-enable Send Tab when we are notified about the subscriptionExpired flag. Until #7143 is fixed, we need to avoid calling `registrationRenewal` to avoid going into an unfixable state. In the context of the FxaPushSupportFeature, we know that our end goal is to call `push.subscribe(fxaPushScope)`, so we can by pass the `verifyConnection` call **assuming** that our native push client is always has the latest push token and knows how to invalidate the endpoint with the Autopush server. --- .../accounts/push/FxaPushSupportFeature.kt | 22 ++++++++---- .../push/ConstellationObserverTest.kt | 35 +++++++++++++++---- docs/changelog.md | 1 + 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt b/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt index 2e6368af3ee..356eafcdeaa 100644 --- a/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt +++ b/components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt @@ -11,7 +11,6 @@ import androidx.lifecycle.ProcessLifecycleOwner import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch -import mozilla.components.concept.push.PushProcessor import mozilla.components.concept.sync.AuthType import mozilla.components.concept.sync.ConstellationState import mozilla.components.concept.sync.Device @@ -113,11 +112,12 @@ internal class AccountObserver( private val logger = Logger(AccountObserver::class.java.simpleName) private val verificationDelegate = VerificationDelegate(context, push.config.disableRateLimit) - private val constellationObserver = ConstellationObserver(push, verificationDelegate) private val pushReset = OneTimeFxaPushReset(context, push) override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { + val constellationObserver = ConstellationObserver(context, push, fxaPushScope, account, verificationDelegate) + pushReset.resetSubscriptionIfNeeded(account) // We need a new subscription only when we have a new account. @@ -126,6 +126,7 @@ internal class AccountObserver( logger.debug("Subscribing for FxaPushScope ($fxaPushScope) events.") push.subscribe(fxaPushScope) { subscription -> + logger.info("Created a new subscription: $subscription") CoroutineScope(Dispatchers.Main).launch { account.deviceConstellation().setDevicePushSubscription(subscription.into()) } @@ -156,8 +157,11 @@ internal class AccountObserver( * when notified by the FxA server. See [Device.subscriptionExpired]. */ internal class ConstellationObserver( - private val push: PushProcessor, - private val verifier: VerificationDelegate + context: Context, + private val push: AutoPushFeature, + private val scope: String, + private val account: OAuthAccount, + private val verifier: VerificationDelegate = VerificationDelegate(context) ) : DeviceConstellationObserver { private val logger = Logger(ConstellationObserver::class.java.simpleName) @@ -177,8 +181,14 @@ internal class ConstellationObserver( logger.info("Proceeding to renew registration") } - logger.info("Renewing registration") - push.renewRegistration() + logger.info("We have been notified that our push subscription has expired; renewing registration.") + push.subscribe(scope) { subscription -> + logger.info("Created a new subscription: $subscription") + + CoroutineScope(Dispatchers.Main).launch { + account.deviceConstellation().setDevicePushSubscription(subscription.into()) + } + } logger.info("Incrementing verifier") logger.info("Verifier state before: timestamp=${verifier.innerTimestamp}, count=${verifier.innerCount}") diff --git a/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ConstellationObserverTest.kt b/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ConstellationObserverTest.kt index 7cefcef13e0..beeb70ea97a 100644 --- a/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ConstellationObserverTest.kt +++ b/components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/ConstellationObserverTest.kt @@ -7,10 +7,14 @@ package mozilla.components.feature.accounts.push import android.content.Context -import mozilla.components.concept.push.PushProcessor import mozilla.components.concept.sync.ConstellationState import mozilla.components.concept.sync.Device +import mozilla.components.concept.sync.OAuthAccount +import mozilla.components.feature.push.AutoPushFeature +import mozilla.components.support.test.any +import mozilla.components.support.test.eq import mozilla.components.support.test.mock +import org.junit.Ignore import org.junit.Test import org.mockito.Mockito.`when` import org.mockito.Mockito.verify @@ -18,15 +22,16 @@ import org.mockito.Mockito.verifyZeroInteractions class ConstellationObserverTest { - private val push: PushProcessor = mock() + private val push: AutoPushFeature = mock() private val verifier: VerificationDelegate = mock() private val state: ConstellationState = mock() private val device: Device = mock() private val context: Context = mock() + private val account: OAuthAccount = mock() @Test fun `do nothing if subscription has not expired`() { - val observer = ConstellationObserver(push, verifier) + val observer = ConstellationObserver(context, push, "testScope", account, verifier) observer.onDevicesUpdate(state) @@ -42,7 +47,7 @@ class ConstellationObserverTest { @Test fun `do nothing if verifier is false`() { - val observer = ConstellationObserver(push, verifier) + val observer = ConstellationObserver(context, push, "testScope", account, verifier) observer.onDevicesUpdate(state) @@ -62,8 +67,9 @@ class ConstellationObserverTest { } @Test + @Ignore("Disabling the test until we revert the changes from #8846 and fix #7143") fun `invoke registration renewal`() { - val observer = ConstellationObserver(push, verifier) + val observer = ConstellationObserver(context, push, "testScope", account, verifier) `when`(state.currentDevice).thenReturn(device) `when`(device.subscriptionExpired).thenReturn(true) @@ -74,4 +80,21 @@ class ConstellationObserverTest { verify(push).renewRegistration() verify(verifier).increment() } -} \ No newline at end of file + + /** + * Remove this test in the future. See [invoke registration renewal] test. + */ + @Test + fun `re-subscribe for push in onDevicesUpdate`() { + val observer = ConstellationObserver(context, push, "testScope", account, verifier) + + `when`(state.currentDevice).thenReturn(device) + `when`(device.subscriptionExpired).thenReturn(true) + `when`(verifier.allowedToRenew()).thenReturn(true) + + observer.onDevicesUpdate(state) + + verify(push).subscribe(eq("testScope"), any(), any(), any()) + verify(verifier).increment() + } +} diff --git a/docs/changelog.md b/docs/changelog.md index a0ce9294e99..037930726ed 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -30,6 +30,7 @@ permalink: /changelog/ * **feature-accounts-push** * Made `FxaPushSupportFeature` aware of the `PushConfig.disableRateLimit` flag. + * ⚠️ `FxaPushSupportFeature` now re-subscribes to push instead of triggering the registration renewal process - this is a temporary workaround and will be removed in the future. # 64.0.0