Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Close #8846: Re-subscribe FxA push subscription on subscriptionExpired
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jonalmeida
jonalmeida committed Nov 17, 2020
1 parent f482878 commit f323cf6
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 19 deletions.
92 changes: 79 additions & 13 deletions ...unts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
* 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/. */

@file:Suppress("LongParameterList")

package mozilla.components.feature.accounts.push

import android.content.Context
Expand All @@ -11,7 +13,8 @@ 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.base.crash.Breadcrumb
import mozilla.components.concept.base.crash.CrashReporting
import mozilla.components.concept.sync.AuthType
import mozilla.components.concept.sync.ConstellationState
import mozilla.components.concept.sync.Device
Expand Down Expand Up @@ -42,6 +45,7 @@ internal const val PREF_FXA_SCOPE = "fxa_push_scope"
* @param context The application Android context.
* @param accountManager The FxaAccountManager.
* @param pushFeature The [AutoPushFeature] if that is setup for observing push events.
* @param crashReporter Instance of `CrashReporting` to record unexpected caught exceptions.
* @param owner the lifecycle owner for the observer. Defaults to [ProcessLifecycleOwner].
* @param autoPause whether to stop notifying the observer during onPause lifecycle events.
* Defaults to false so that observers are always notified.
Expand All @@ -50,6 +54,7 @@ class FxaPushSupportFeature(
private val context: Context,
accountManager: FxaAccountManager,
pushFeature: AutoPushFeature,
private val crashReporter: CrashReporting? = null,
owner: LifecycleOwner = ProcessLifecycleOwner.get(),
autoPause: Boolean = false
) {
Expand Down Expand Up @@ -85,6 +90,7 @@ class FxaPushSupportFeature(
context,
pushFeature,
fxaPushScope,
crashReporter,
owner,
autoPause
)
Expand All @@ -107,25 +113,42 @@ internal class AccountObserver(
private val context: Context,
private val push: AutoPushFeature,
private val fxaPushScope: String,
private val crashReporter: CrashReporting?,
private val lifecycleOwner: LifecycleOwner,
private val autoPause: Boolean
) : SyncAccountObserver {

private val logger = Logger(AccountObserver::class.java.simpleName)
private val verificationDelegate = VerificationDelegate(context, push.config.disableRateLimit)
private val constellationObserver = ConstellationObserver(push, verificationDelegate)

override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {

val constellationObserver = ConstellationObserver(
context = context,
push = push,
scope = fxaPushScope,
account = account,
verifier = verificationDelegate,
crashReporter = crashReporter
)

// We need a new subscription only when we have a new account.
// The subscription is removed when an account logs out.
if (authType != AuthType.Existing && authType != AuthType.Recovered) {
logger.debug("Subscribing for FxaPushScope ($fxaPushScope) events.")

push.subscribe(fxaPushScope) { subscription ->
CoroutineScope(Dispatchers.Main).launch {
account.deviceConstellation().setDevicePushSubscription(subscription.into())
}
}
push.subscribe(
scope = fxaPushScope,
onSubscribeError = { e ->
crashReporter?.recordCrashBreadcrumb(Breadcrumb("Subscribing to FxA push failed at login."))
logger.info("Subscribing to FxA push failed at login.", e)
},
onSubscribe = { subscription ->
logger.info("Created a new subscription: $subscription")
CoroutineScope(Dispatchers.Main).launch {
account.deviceConstellation().setDevicePushSubscription(subscription.into())
}
})
}

// NB: can we just expose registerDeviceObserver on account manager?
Expand All @@ -152,8 +175,12 @@ 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),
private val crashReporter: CrashReporting?
) : DeviceConstellationObserver {

private val logger = Logger(ConstellationObserver::class.java.simpleName)
Expand All @@ -166,21 +193,59 @@ internal class ConstellationObserver(
// If our last check was recent (see: PERIODIC_INTERVAL_MILLISECONDS), we do nothing.
val allowedToRenew = verifier.allowedToRenew()
if (!updateSubscription || !allowedToRenew) {
logger.info("Short-circuiting onDevicesUpdate: " +
"updateSubscription($updateSubscription), allowedToRenew($allowedToRenew)")
logger.info(
"Short-circuiting onDevicesUpdate: " +
"updateSubscription($updateSubscription), allowedToRenew($allowedToRenew)"
)
return
} else {
logger.info("Proceeding to renew registration")
}

logger.info("Renewing registration")
push.renewRegistration()
logger.info("We have been notified that our push subscription has expired; re-subscribing.")
push.subscribe(
scope = scope,
onSubscribeError = ::onSubscribeError,
onSubscribe = { onSubscribe(constellation, it) }
)

logger.info("Incrementing verifier")
logger.info("Verifier state before: timestamp=${verifier.innerTimestamp}, count=${verifier.innerCount}")
verifier.increment()
logger.info("Verifier state after: timestamp=${verifier.innerTimestamp}, count=${verifier.innerCount}")
}

internal fun onSubscribe(constellation: ConstellationState, subscription: AutoPushSubscription) {

logger.info("Created a new subscription: $subscription")

val oldEndpoint = constellation.currentDevice?.subscription?.endpoint
if (subscription.endpoint == oldEndpoint) {
val exception = IllegalStateException(
"New push endpoint matches existing one",
Throwable("New endpoint: ${subscription.endpoint}\nOld endpoint: $oldEndpoint")
)

crashReporter?.submitCaughtException(exception)

logger.warn("Push endpoints match!", exception)
}

CoroutineScope(Dispatchers.Main).launch {
account.deviceConstellation().setDevicePushSubscription(subscription.into())
}
}

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()
)
)
crashReporter?.recordCrashBreadcrumb(e.toBreadcrumb())
}
}

/**
Expand Down Expand Up @@ -261,6 +326,7 @@ internal class VerificationDelegate(

@VisibleForTesting
internal var innerCount: Int = 0

@VisibleForTesting
internal var innerTimestamp: Long = System.currentTimeMillis()

Expand Down
40 changes: 40 additions & 0 deletions ...counts-push/src/test/java/mozilla/components/feature/accounts/push/AccountObserverTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package mozilla.components.feature.accounts.push
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleOwner
import kotlinx.coroutines.runBlocking
import mozilla.components.concept.base.crash.CrashReporting
import mozilla.components.concept.sync.AuthType
import mozilla.components.concept.sync.DeviceConstellation
import mozilla.components.concept.sync.OAuthAccount
Expand Down Expand Up @@ -42,6 +43,7 @@ class AccountObserverTest {
private val account: OAuthAccount = mock()
private val constellation: DeviceConstellation = mock()
private val config: PushConfig = mock()
private val crashReporter: CrashReporting = mock()

@Before
fun setup() {
Expand All @@ -58,6 +60,7 @@ class AccountObserverTest {
testContext,
pushFeature,
pushScope,
crashReporter,
lifecycleOwner,
false
)
Expand All @@ -81,6 +84,7 @@ class AccountObserverTest {
testContext,
pushFeature,
pushScope,
crashReporter,
mock(),
false
)
Expand All @@ -104,6 +108,7 @@ class AccountObserverTest {
testContext,
pushFeature,
pushScope,
crashReporter,
mock(),
false
)
Expand All @@ -123,6 +128,7 @@ class AccountObserverTest {
testContext,
pushFeature,
pushScope,
crashReporter,
mock(),
false
)
Expand All @@ -140,6 +146,7 @@ class AccountObserverTest {
testContext,
pushFeature,
pushScope,
crashReporter,
mock(),
false
)
Expand All @@ -161,6 +168,7 @@ class AccountObserverTest {
testContext,
pushFeature,
pushScope,
crashReporter,
mock(),
false
)
Expand All @@ -173,12 +181,32 @@ class AccountObserverTest {
Unit
}

@Test
fun `notify crash reporter on subscription error`() = runBlocking {
val observer = AccountObserver(
testContext,
pushFeature,
pushScope,
crashReporter,
mock(),
false
)

whenSubscribeError()

observer.onAuthenticated(account, AuthType.Signin)

verify(crashReporter).recordCrashBreadcrumb(any())
Unit
}

@Test
fun `feature and service invoked on logout`() {
val observer = AccountObserver(
testContext,
pushFeature,
pushScope,
crashReporter,
mock(),
false
)
Expand All @@ -194,6 +222,7 @@ class AccountObserverTest {
testContext,
pushFeature,
pushScope,
crashReporter,
mock(),
false
)
Expand Down Expand Up @@ -221,4 +250,15 @@ class AccountObserverTest {
)
}
}

@Suppress("UNCHECKED_CAST")
private fun whenSubscribeError(): OngoingStubbing<Unit>? {
return `when`(pushFeature.subscribe(any(), nullable(), any(), any())).thenAnswer {

// Invoke the `onSubscribe` lambda with a fake subscription.
(it.arguments[2] as ((Exception) -> Unit)).invoke(
IllegalStateException("test")
)
}
}
}
Loading

0 comments on commit f323cf6

Please sign in to comment.