Skip to content

Commit

Permalink
Close mozilla-mobile#8846: Re-subscribe FxA push subscription on subs…
Browse files Browse the repository at this point in the history
…criptionExpired

This is a workaround fix to re-enable Send Tab when we are notified
about the subscriptionExpired flag. Until mozilla-mobile#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.
  • Loading branch information
jonalmeida committed Nov 3, 2020
1 parent 4c7ff86 commit 3339328
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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())
}
Expand Down Expand Up @@ -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)
Expand All @@ -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}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,31 @@
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
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)

Expand All @@ -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)

Expand All @@ -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)
Expand All @@ -74,4 +80,21 @@ class ConstellationObserverTest {
verify(push).renewRegistration()
verify(verifier).increment()
}
}

/**
* 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()
}
}
1 change: 1 addition & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 3339328

Please sign in to comment.