From fa0344c7394fd2c9ad0536750eb14849f965c4a5 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Mon, 29 Jul 2019 16:07:47 -0700 Subject: [PATCH] Part 3: Rust SyncManager integration Co-authored-by: Arturo Mejia --- buildSrc/src/main/java/Dependencies.kt | 1 + .../browser/storage/sync/Connection.kt | 10 + .../storage/sync/PlacesBookmarksStorage.kt | 7 + .../storage/sync/PlacesHistoryStorage.kt | 7 + .../storage/sync/PlacesHistoryStorageTest.kt | 20 ++ .../components/concept/sync/Devices.kt | 4 +- .../mozilla/components/concept/sync/Sync.kt | 5 + .../feature/accounts/FxaWebChannelFeature.kt | 18 +- .../accounts/FxaWebChannelFeatureTest.kt | 12 +- components/service/firefox-accounts/README.md | 5 + .../service/firefox-accounts/build.gradle | 3 + .../mozilla/components/service/fxa/Config.kt | 11 +- .../service/fxa/FxaDeviceConstellation.kt | 4 +- .../service/fxa/FxaDeviceSettingsCache.kt | 62 ++++ .../service/fxa/SharedPreferencesCache.kt | 56 ++++ .../service/fxa/SyncAuthInfoCache.kt | 79 ++--- .../mozilla/components/service/fxa/Types.kt | 25 +- .../service/fxa/manager/FxaAccountManager.kt | 64 ++++- .../service/fxa/manager/SyncEnginesStorage.kt | 40 +++ .../service/fxa/sync/SyncManager.kt | 30 +- .../components/service/fxa/sync/Types.kt | 61 ++++ .../fxa/sync/WorkManagerSyncManager.kt | 269 ++++++++++++++---- .../service/fxa/FxaAccountManagerTest.kt | 97 +++++-- .../service/fxa/FxaDeviceConstellationTest.kt | 5 +- .../service/sync/logins/AsyncLoginsStorage.kt | 5 + .../sync/telemetry/BaseGleanSyncPing.kt | 6 +- .../samples/sync/logins/MainActivity.kt | 7 +- .../org/mozilla/samples/sync/MainActivity.kt | 9 +- 28 files changed, 747 insertions(+), 175 deletions(-) create mode 100644 components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceSettingsCache.kt create mode 100644 components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SharedPreferencesCache.kt create mode 100644 components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/SyncEnginesStorage.kt create mode 100644 components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/Types.kt diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt index 85a69c93ddf..5e1780de146 100644 --- a/buildSrc/src/main/java/Dependencies.kt +++ b/buildSrc/src/main/java/Dependencies.kt @@ -118,6 +118,7 @@ object Dependencies { const val mozilla_sync_logins = "org.mozilla.appservices:logins:${Versions.mozilla_appservices}" const val mozilla_places = "org.mozilla.appservices:places:${Versions.mozilla_appservices}" + const val mozilla_sync_manager = "org.mozilla.appservices:syncmanager:${Versions.mozilla_appservices}" const val mozilla_push = "org.mozilla.appservices:push:${Versions.mozilla_appservices}" diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Connection.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Connection.kt index 0a97505b1d5..90d13edd712 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Connection.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Connection.kt @@ -25,6 +25,11 @@ const val DB_NAME = "places.sqlite" * Writer is always the same, as guaranteed by [PlacesApi]. */ internal interface Connection : Closeable { + /** + * @return raw handle that could be used for referencing underlying [PlacesApi]. + */ + fun getHandle(): Long + fun reader(): PlacesReaderConnection fun writer(): PlacesWriterConnection @@ -57,6 +62,11 @@ internal object RustPlacesConnection : Connection { cachedReader = api!!.openReader() } + override fun getHandle(): Long { + check(api != null) { "must call init first" } + return api!!.getHandle() + } + override fun reader(): PlacesReaderConnection = synchronized(this) { check(cachedReader != null) { "must call init first" } return cachedReader!! diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorage.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorage.kt index ab7d82dfa70..eccbb0d5547 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorage.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorage.kt @@ -184,4 +184,11 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo } } } + + /** + * TODO + */ + override fun getHandle(): Long { + return places.getHandle() + } } diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesHistoryStorage.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesHistoryStorage.kt index 5427fa166ff..ad784380715 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesHistoryStorage.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesHistoryStorage.kt @@ -177,4 +177,11 @@ open class PlacesHistoryStorage(context: Context) : PlacesStorage(context), Hist } } } + + /** + * @return raw handle that could be used for referencing underlying [PlacesApi]. + */ + override fun getHandle(): Long { + return places.getHandle() + } } diff --git a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt index 30cac725a4c..ea956f534e5 100644 --- a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt +++ b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt @@ -501,6 +501,11 @@ class PlacesHistoryStorageTest { fail() } + override fun getHandle(): Long { + fail() + return 0L + } + override fun close() { fail() } @@ -533,6 +538,11 @@ class PlacesHistoryStorageTest { override fun syncBookmarks(syncInfo: SyncAuthInfo) {} + override fun getHandle(): Long { + fail() + return 0L + } + override fun close() { fail() } @@ -565,6 +575,11 @@ class PlacesHistoryStorageTest { fail() } + override fun getHandle(): Long { + fail() + return 0L + } + override fun close() { fail() } @@ -601,6 +616,11 @@ class PlacesHistoryStorageTest { fail() } + override fun getHandle(): Long { + fail() + return 0L + } + override fun close() { fail() } diff --git a/components/concept/sync/src/main/java/mozilla/components/concept/sync/Devices.kt b/components/concept/sync/src/main/java/mozilla/components/concept/sync/Devices.kt index 486a6e16812..c62b5b7503d 100644 --- a/components/concept/sync/src/main/java/mozilla/components/concept/sync/Devices.kt +++ b/components/concept/sync/src/main/java/mozilla/components/concept/sync/Devices.kt @@ -4,6 +4,7 @@ @file:SuppressWarnings("TooManyFunctions") package mozilla.components.concept.sync +import android.content.Context import androidx.lifecycle.LifecycleOwner import kotlinx.coroutines.Deferred import mozilla.components.support.base.observer.Observable @@ -51,9 +52,10 @@ interface DeviceConstellation : Observable { /** * Set name of the current device. * @param name New device name. + * @param context An application context, used for updating internal caches. * @return A [Deferred] that will be resolved with a success flag once operation is complete. */ - fun setDeviceNameAsync(name: String): Deferred + fun setDeviceNameAsync(name: String, context: Context): Deferred /** * Set a [DevicePushSubscription] for the current device. diff --git a/components/concept/sync/src/main/java/mozilla/components/concept/sync/Sync.kt b/components/concept/sync/src/main/java/mozilla/components/concept/sync/Sync.kt index 397e646be12..c4a9378e023 100644 --- a/components/concept/sync/src/main/java/mozilla/components/concept/sync/Sync.kt +++ b/components/concept/sync/src/main/java/mozilla/components/concept/sync/Sync.kt @@ -53,6 +53,11 @@ interface SyncableStore { * @return [SyncStatus] A status object describing how sync went. */ suspend fun sync(authInfo: SyncAuthInfo): SyncStatus + + /** + * TODO docs also, correct location? + */ + fun getHandle(): Long } /** diff --git a/components/feature/accounts/src/main/java/mozilla/components/feature/accounts/FxaWebChannelFeature.kt b/components/feature/accounts/src/main/java/mozilla/components/feature/accounts/FxaWebChannelFeature.kt index c703f88f9c8..952cda1dfbd 100644 --- a/components/feature/accounts/src/main/java/mozilla/components/feature/accounts/FxaWebChannelFeature.kt +++ b/components/feature/accounts/src/main/java/mozilla/components/feature/accounts/FxaWebChannelFeature.kt @@ -17,6 +17,7 @@ import mozilla.components.concept.sync.AuthType import mozilla.components.service.fxa.FxaAuthData import mozilla.components.service.fxa.SyncEngine import mozilla.components.service.fxa.manager.FxaAccountManager +import mozilla.components.service.fxa.sync.toSyncEngines import mozilla.components.service.fxa.toAuthType import mozilla.components.support.base.feature.LifecycleAwareFeature import mozilla.components.support.base.log.logger.Logger @@ -248,6 +249,14 @@ class FxaWebChannelFeature( return status } + fun JSONArray.toStringList(): List { + val result = mutableListOf() + for (i in 0 until this.length()) { + result.add(this.getString(i)) + } + return result + } + /** * Handles the [COMMAND_OAUTH_LOGIN] event from the web-channel. */ @@ -255,12 +264,18 @@ class FxaWebChannelFeature( val authType: AuthType val code: String val state: String + val declinedEngines: List? try { val data = payload.getJSONObject("data") authType = data.getString("action").toAuthType() code = data.getString("code") state = data.getString("state") + declinedEngines = try { + data.getJSONArray("declinedSyncEngines").toStringList() + } catch (e: JSONException) { + null + } } catch (e: JSONException) { // TODO ideally, this should log to Sentry. logger.error("Error while processing WebChannel oauth-login command", e) @@ -270,7 +285,8 @@ class FxaWebChannelFeature( accountManager.finishAuthenticationAsync(FxaAuthData( authType = authType, code = code, - state = state + state = state, + declinedEngines = declinedEngines?.toSyncEngines() )) return null diff --git a/components/feature/accounts/src/test/java/mozilla/components/feature/accounts/FxaWebChannelFeatureTest.kt b/components/feature/accounts/src/test/java/mozilla/components/feature/accounts/FxaWebChannelFeatureTest.kt index 2064ca886f9..254a9e30d77 100644 --- a/components/feature/accounts/src/test/java/mozilla/components/feature/accounts/FxaWebChannelFeatureTest.kt +++ b/components/feature/accounts/src/test/java/mozilla/components/feature/accounts/FxaWebChannelFeatureTest.kt @@ -201,7 +201,7 @@ class FxaWebChannelFeatureTest { val messageHandler = argumentCaptor() val responseToTheWebChannel = argumentCaptor() val port = mock() - val expectedEngines = setOf(SyncEngine.HISTORY) + val expectedEngines = setOf(SyncEngine.History) WebExtensionController.installedExtensions[FxaWebChannelFeature.WEB_CHANNEL_EXTENSION_ID] = ext @@ -250,7 +250,7 @@ class FxaWebChannelFeatureTest { val messageHandler = argumentCaptor() val responseToTheWebChannel = argumentCaptor() val port = mock() - val expectedEngines = setOf(SyncEngine.HISTORY, SyncEngine.BOOKMARKS, SyncEngine.PASSWORDS) + val expectedEngines = setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords) WebExtensionController.installedExtensions[FxaWebChannelFeature.WEB_CHANNEL_EXTENSION_ID] = ext @@ -301,7 +301,7 @@ class FxaWebChannelFeatureTest { val messageHandler = argumentCaptor() val responseToTheWebChannel = argumentCaptor() val port = mock() - val expectedEngines = setOf(SyncEngine.HISTORY, SyncEngine.BOOKMARKS, SyncEngine.PASSWORDS) + val expectedEngines = setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords) val logoutDeferred = CompletableDeferred() WebExtensionController.installedExtensions[FxaWebChannelFeature.WEB_CHANNEL_EXTENSION_ID] = ext @@ -425,7 +425,7 @@ class FxaWebChannelFeatureTest { val messageHandler = argumentCaptor() val responseToTheWebChannel = argumentCaptor() val port = mock() - val expectedEngines = setOf(SyncEngine.HISTORY, SyncEngine.BOOKMARKS, SyncEngine.PASSWORDS) + val expectedEngines = setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords) WebExtensionController.installedExtensions[FxaWebChannelFeature.WEB_CHANNEL_EXTENSION_ID] = ext @@ -477,7 +477,7 @@ class FxaWebChannelFeatureTest { val messageHandler = argumentCaptor() val responseToTheWebChannel = argumentCaptor() val port = mock() - val expectedEngines = setOf(SyncEngine.HISTORY, SyncEngine.BOOKMARKS, SyncEngine.PASSWORDS) + val expectedEngines = setOf(SyncEngine.History, SyncEngine.Bookmarks, SyncEngine.Passwords) WebExtensionController.installedExtensions[FxaWebChannelFeature.WEB_CHANNEL_EXTENSION_ID] = ext @@ -648,7 +648,7 @@ class FxaWebChannelFeatureTest { val messageHandler = argumentCaptor() val jsonFromWebChannel = argumentCaptor() val port = mock() - val expectedEngines = setOf(SyncEngine.HISTORY, SyncEngine.BOOKMARKS) + val expectedEngines = setOf(SyncEngine.History, SyncEngine.Bookmarks) WebExtensionController.installedExtensions[FxaWebChannelFeature.WEB_CHANNEL_EXTENSION_ID] = ext diff --git a/components/service/firefox-accounts/README.md b/components/service/firefox-accounts/README.md index a7a2cd17961..6647ced60a0 100644 --- a/components/service/firefox-accounts/README.md +++ b/components/service/firefox-accounts/README.md @@ -26,6 +26,11 @@ Useful companion components: * [feature-accounts](https://github.com/mozilla-mobile/android-components/tree/master/components/feature/accounts), provides a `tabs` integration on top of `FxaAccountManager`, to handle display of web sign-in UI. * [browser-storage-sync](https://github.com/mozilla-mobile/android-components/tree/master/components/browser/storage-sync), provides data storage layers compatible with Firefox Sync. +## Before using this component +Products sending telemetry and using this component *must request* a data-review following [this process](https://wiki.mozilla.org/Firefox/Data_Collection). +This component provides data collection using the [Glean SDK](https://mozilla.github.io/glean/book/index.html). +The list of metrics being collected is available in the [metrics documentation](../../support/telemetry/docs/metrics.md). + ## Usage ### Setting up the dependency diff --git a/components/service/firefox-accounts/build.gradle b/components/service/firefox-accounts/build.gradle index 741de0f57d0..f4905b92fd4 100644 --- a/components/service/firefox-accounts/build.gradle +++ b/components/service/firefox-accounts/build.gradle @@ -33,9 +33,12 @@ dependencies { // Parts of this dependency are typealiase'd or are otherwise part of this module's public API. api Dependencies.mozilla_fxa + implementation Dependencies.mozilla_sync_manager // Observable is part of public API of the FxaAccountManager. api project(':support-base') + implementation project(':support-sync-telemetry') + implementation project(':service-glean') implementation project(':support-ktx') implementation Dependencies.kotlin_stdlib diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Config.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Config.kt index 7f508e9ae1b..d9357419077 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Config.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Config.kt @@ -50,8 +50,11 @@ data class SyncConfig( * robust integrations. We do not expose "unknown" engines via our public API, but do handle them * internally (by persisting their enabled/disabled status). */ -enum class SyncEngine(val nativeName: String) { - HISTORY("history"), - BOOKMARKS("bookmarks"), - PASSWORDS("passwords"), +sealed class SyncEngine(val nativeName: String) { + object History : SyncEngine("history") + object Bookmarks : SyncEngine("bookmarks") + object Passwords : SyncEngine("passwords") + data class Other(val name: String) : SyncEngine(name) + + internal object Forms : SyncEngine("forms") } diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceConstellation.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceConstellation.kt index 2456cacdb2b..1cf260fbd0e 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceConstellation.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceConstellation.kt @@ -4,6 +4,7 @@ package mozilla.components.service.fxa +import android.content.Context import androidx.annotation.GuardedBy import androidx.lifecycle.LifecycleOwner import kotlinx.coroutines.CoroutineScope @@ -81,11 +82,12 @@ class FxaDeviceConstellation( deviceObserverRegistry.register(observer, owner, autoPause) } - override fun setDeviceNameAsync(name: String): Deferred { + override fun setDeviceNameAsync(name: String, context: Context): Deferred { return scope.async { val rename = handleFxaExceptions(logger, "changing device name") { account.setDeviceDisplayName(name) } + FxaDeviceSettingsCache(context).updateCachedName(name) // See the latest device (name) changes after changing it. val refreshDevices = refreshDevicesAsync().await() diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceSettingsCache.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceSettingsCache.kt new file mode 100644 index 00000000000..90d314bb199 --- /dev/null +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FxaDeviceSettingsCache.kt @@ -0,0 +1,62 @@ +/* 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/. */ +package mozilla.components.service.fxa + +import android.content.Context +import android.content.SharedPreferences +import mozilla.appservices.syncmanager.DeviceSettings +import mozilla.appservices.syncmanager.DeviceType +import mozilla.components.support.base.log.logger.Logger +import org.json.JSONObject +import java.lang.IllegalArgumentException +import java.lang.IllegalStateException + +private const val CACHE_NAME = "FxaDeviceSettingsCache" +private const val CACHE_KEY = CACHE_NAME +private const val KEY_FXA_DEVICE_ID = "kid" +private const val KEY_DEVICE_NAME = "syncKey" +private const val KEY_DEVICE_TYPE = "tokenServerUrl" + +/** + * A thin wrapper around [SharedPreferences] which knows how to serialize/deserialize [DeviceSettings]. + * + * This class exists to provide background sync workers with access to [DeviceSettings]. + */ +class FxaDeviceSettingsCache(context: Context) : SharedPreferencesCache(context) { + override val logger = Logger("SyncAuthInfoCache") + override val cacheKey = CACHE_KEY + override val cacheName = CACHE_NAME + + override fun DeviceSettings.toJSON(): JSONObject { + return JSONObject().also { + it.put(KEY_FXA_DEVICE_ID, this.fxaDeviceId) + it.put(KEY_DEVICE_NAME, this.name) + it.put(KEY_DEVICE_TYPE, this.type.name) + } + } + + override fun fromJSON(obj: JSONObject): DeviceSettings { + return DeviceSettings( + fxaDeviceId = obj.getString(KEY_FXA_DEVICE_ID), + name = obj.getString(KEY_DEVICE_NAME), + type = obj.getString(KEY_DEVICE_TYPE).toDeviceType() + ) + } + + fun updateCachedName(name: String) { + val cached = getCached() ?: throw IllegalStateException("Trying to update cached value in an empty cache") + setToCache(cached.copy(name = name)) + } + + private fun String.toDeviceType(): DeviceType { + return when (this) { + "DESKTOP" -> DeviceType.DESKTOP + "MOBILE" -> DeviceType.MOBILE + "TABLET" -> DeviceType.TABLET + "VR" -> DeviceType.VR + "TV" -> DeviceType.TV + else -> throw IllegalArgumentException("Unknown device type in cached string: $this") + } + } +} diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SharedPreferencesCache.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SharedPreferencesCache.kt new file mode 100644 index 00000000000..dcad31cd98c --- /dev/null +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SharedPreferencesCache.kt @@ -0,0 +1,56 @@ +/* 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/. */ +package mozilla.components.service.fxa + +import android.content.Context +import android.content.SharedPreferences +import mozilla.components.support.base.log.logger.Logger +import org.json.JSONException +import org.json.JSONObject + +/** + * An abstract wrapper around [SharedPreferences] which facilitates caching of [T] objects. + */ +abstract class SharedPreferencesCache(val context: Context) { + internal abstract val logger: Logger + internal abstract val cacheKey: String + internal abstract val cacheName: String + internal abstract fun T.toJSON(): JSONObject + internal abstract fun fromJSON(obj: JSONObject): T + + internal fun setToCache(obj: T) { + // JSONObject swallows any 'JSONException' thrown in 'toString', and simply returns 'null'. + // An error happened while converting a JSONObject into a string. Let's fail loudly and + // see if this actually happens in the wild. An alternative is to swallow this error and + // log an error message, but we're very unlikely to notice any problems in that case. + val s = obj.toJSON().toString() as String? ?: throw IllegalStateException("Failed to stringify") + cache().edit().putString(cacheKey, s).apply() + } + + internal fun getCached(): T? { + val s = cache().getString(cacheKey, null) ?: return null + return try { + fromJSON(JSONObject(s)) + } catch (e: JSONException) { + logger.error("Failed to parse cached value", e) + null + } + } + + internal fun clear() { + cache().edit().clear().apply() + } + + internal fun stringifyJSON(o: JSONObject): String { + // JSONObject swallows any JSONExceptions thrown during 'toString', and simply returns 'null'. + // An error happened while converting a JSONObject into a string. Let's fail loudly and + // see if this actually happens in the wild. An alternative is to swallow this error and + // log an error message, but we're very unlikely to notice any problems in that case. + return o.toString() as String? ?: throw IllegalStateException("Failed to stringify") + } + + private fun cache(): SharedPreferences { + return context.getSharedPreferences(cacheName, Context.MODE_PRIVATE) + } +} diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SyncAuthInfoCache.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SyncAuthInfoCache.kt index 1ac01804ab6..3130d69b2e8 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SyncAuthInfoCache.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SyncAuthInfoCache.kt @@ -7,7 +7,6 @@ import android.content.Context import android.content.SharedPreferences import mozilla.components.concept.sync.SyncAuthInfo import mozilla.components.support.base.log.logger.Logger -import org.json.JSONException import org.json.JSONObject private const val CACHE_NAME = "SyncAuthInfoCache" @@ -20,64 +19,36 @@ private const val KEY_TOKEN_SERVER_URL = "tokenServerUrl" /** * A thin wrapper around [SharedPreferences] which knows how to serialize/deserialize [SyncAuthInfo]. + * + * This class exists to provide background sync workers with access to [SyncAuthInfo]. */ -class SyncAuthInfoCache(val context: Context) { - private val logger = Logger("SyncAuthInfoCache") - - fun setToCache(authInfo: SyncAuthInfo) { - cache().edit().putString(CACHE_KEY, authInfo.toCacheString()).apply() +class SyncAuthInfoCache(context: Context) : SharedPreferencesCache(context) { + override val logger = Logger("SyncAuthInfoCache") + override val cacheKey = CACHE_KEY + override val cacheName = CACHE_NAME + + override fun SyncAuthInfo.toJSON(): JSONObject { + return JSONObject().also { + it.put(KEY_KID, this.kid) + it.put(KEY_FXA_ACCESS_TOKEN, this.fxaAccessToken) + it.put(KEY_FXA_ACCESS_TOKEN_EXPIRES_AT, this.fxaAccessTokenExpiresAt) + it.put(KEY_SYNC_KEY, this.syncKey) + it.put(KEY_TOKEN_SERVER_URL, this.tokenServerUrl) + } } - fun getCached(): SyncAuthInfo? { - val s = cache().getString(CACHE_KEY, null) ?: return null - return fromCacheString(s) + override fun fromJSON(obj: JSONObject): SyncAuthInfo { + return SyncAuthInfo( + kid = obj.getString(KEY_KID), + fxaAccessToken = obj.getString(KEY_FXA_ACCESS_TOKEN), + fxaAccessTokenExpiresAt = obj.getLong(KEY_FXA_ACCESS_TOKEN_EXPIRES_AT), + syncKey = obj.getString(KEY_SYNC_KEY), + tokenServerUrl = obj.getString(KEY_TOKEN_SERVER_URL) + ) } fun expired(): Boolean { - val s = cache().getString(CACHE_KEY, null) ?: return true - val cached = fromCacheString(s) ?: return true - return cached.fxaAccessTokenExpiresAt <= System.currentTimeMillis() - } - - fun clear() { - cache().edit().clear().apply() - } - - private fun cache(): SharedPreferences { - return context.getSharedPreferences(CACHE_NAME, Context.MODE_PRIVATE) - } - - private fun SyncAuthInfo.toCacheString(): String? { - val o = JSONObject() - o.put(KEY_KID, this.kid) - o.put(KEY_FXA_ACCESS_TOKEN, this.fxaAccessToken) - o.put(KEY_FXA_ACCESS_TOKEN_EXPIRES_AT, this.fxaAccessTokenExpiresAt) - o.put(KEY_SYNC_KEY, this.syncKey) - o.put(KEY_TOKEN_SERVER_URL, this.tokenServerUrl) - // JSONObject swallows any JSONExceptions thrown during 'toString', and simply returns 'null'. - val asString: String? = o.toString() - if (asString == null) { - // An error happened while converting a JSONObject into a string. Let's fail loudly and - // see if this actually happens in the wild. An alternative is to swallow this error and - // log an error message, but we're very unlikely to notice any problems in that case. - throw IllegalStateException("Failed to stringify SyncAuthInfo") - } - return asString - } - - private fun fromCacheString(s: String): SyncAuthInfo? { - return try { - val o = JSONObject(s) - SyncAuthInfo( - kid = o.getString(KEY_KID), - fxaAccessToken = o.getString(KEY_FXA_ACCESS_TOKEN), - fxaAccessTokenExpiresAt = o.getLong(KEY_FXA_ACCESS_TOKEN_EXPIRES_AT), - syncKey = o.getString(KEY_SYNC_KEY), - tokenServerUrl = o.getString(KEY_TOKEN_SERVER_URL) - ) - } catch (e: JSONException) { - logger.error("Failed to parse cached value", e) - null - } + val expiresAt = getCached()?.fxaAccessTokenExpiresAt ?: return true + return expiresAt <= System.currentTimeMillis() } } diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Types.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Types.kt index f04e12cb513..d6c044fd577 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Types.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/Types.kt @@ -39,8 +39,14 @@ fun String?.toAuthType(): AuthType { * @property authType Type of authentication which caused this object to be created. * @property code OAuth code. * @property state OAuth state. + * @property declinedEngines An optional list of [SyncEngine]s that user declined to sync. */ -data class FxaAuthData(val authType: AuthType, val code: String, val state: String) +data class FxaAuthData( + val authType: AuthType, + val code: String, + val state: String, + val declinedEngines: Set? = null +) // The rest of this file describes translations between fxaclient's internal type definitions and analogous // types defined by concept-sync. It's a little tedious, but ensures decoupling between abstract @@ -93,7 +99,7 @@ fun Profile.into(): mozilla.components.concept.sync.Profile { ) } -fun Device.Type.into(): mozilla.components.concept.sync.DeviceType { +fun Device.Type.into(): DeviceType { return when (this) { Device.Type.DESKTOP -> DeviceType.DESKTOP Device.Type.MOBILE -> DeviceType.MOBILE @@ -115,6 +121,21 @@ fun DeviceType.into(): Device.Type { } } +/** + * FxA and Sync libraries both define a "DeviceType", so we get to have even more cruft. + */ +fun DeviceType.intoSyncType(): mozilla.appservices.syncmanager.DeviceType { + return when (this) { + DeviceType.DESKTOP -> mozilla.appservices.syncmanager.DeviceType.DESKTOP + DeviceType.MOBILE -> mozilla.appservices.syncmanager.DeviceType.MOBILE + DeviceType.TABLET -> mozilla.appservices.syncmanager.DeviceType.TABLET + DeviceType.TV -> mozilla.appservices.syncmanager.DeviceType.TV + DeviceType.VR -> mozilla.appservices.syncmanager.DeviceType.VR + // There's not a corresponding syncmanager type, so we pick a default for simplicity's sake. + DeviceType.UNKNOWN -> mozilla.appservices.syncmanager.DeviceType.MOBILE + } +} + fun DeviceCapability.into(): Device.Capability { return when (this) { DeviceCapability.SEND_TAB -> Device.Capability.SEND_TAB diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt index 4ef326ad2a2..7e3108bf20f 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/FxaAccountManager.kt @@ -16,6 +16,7 @@ import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.async import kotlinx.coroutines.launch import kotlinx.coroutines.cancel +import mozilla.appservices.syncmanager.DeviceSettings import mozilla.components.concept.sync.AccountObserver import mozilla.components.concept.sync.AuthException import mozilla.components.concept.sync.DeviceCapability @@ -28,6 +29,7 @@ import mozilla.components.concept.sync.Profile import mozilla.components.concept.sync.StatePersistenceCallback import mozilla.components.service.fxa.AccountStorage import mozilla.components.service.fxa.DeviceConfig +import mozilla.components.service.fxa.FxaDeviceSettingsCache import mozilla.components.service.fxa.FirefoxAccount import mozilla.components.service.fxa.FxaAuthData import mozilla.components.service.fxa.FxaException @@ -38,9 +40,11 @@ import mozilla.components.service.fxa.SyncAuthInfoCache import mozilla.components.service.fxa.SyncConfig import mozilla.components.service.fxa.SyncEngine import mozilla.components.service.fxa.asSyncAuthInfo +import mozilla.components.service.fxa.intoSyncType import mozilla.components.service.fxa.sharing.AccountSharing import mozilla.components.service.fxa.sharing.ShareableAccount import mozilla.components.service.fxa.sync.SyncManager +import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.service.fxa.sync.SyncStatusObserver import mozilla.components.service.fxa.sync.WorkManagerSyncManager import mozilla.components.support.base.log.logger.Logger @@ -55,8 +59,8 @@ import java.util.concurrent.Executors import kotlin.coroutines.CoroutineContext /** - * A global registry for propagating [AuthException] errors. Components such as [SyncManager] and - * [FxaDeviceRefreshManager] may encounter authentication problems during their normal operation, and + * A global registry for propagating [AuthException] errors. Components such as [SyncManager] may + * encounter authentication problems during their normal operation, and * this registry is how they inform [FxaAccountManager] that these errors happened. * * [FxaAccountManager] monitors this registry, adjusts internal state accordingly, and notifies @@ -318,7 +322,13 @@ open class FxaAccountManager( CoroutineScope(coroutineContext).launch { // Make sure auth-info cache is populated before starting sync manager. maybeUpdateSyncAuthInfoCache() - manager.start() + + // So this is a bit of a wrinkle. We enabled syncing for an authenticated account. + // How do we know if this is going to be a first sync for an account? + // We can make two assumptions, that are probably roughly correct most of the time: + // - we know what the user data choices are - they were presented with CWTS during + // sign-up (here or on some other client), or we're enabling sync for an existing account + manager.start(SyncReason.FirstSync) result.complete(Unit) } } else { @@ -353,11 +363,11 @@ open class FxaAccountManager( /** * Request an immediate synchronization, as configured according to [syncConfig]. * - * @param startup Boolean flag indicating if sync is being requested in a startup situation. + * @param reason A [SyncReason] indicating why this sync is being requested. * @param debounce Boolean flag indicating if this sync may be debounced (in case another sync executed recently). */ fun syncNowAsync( - startup: Boolean = false, + reason: SyncReason, debounce: Boolean = false ): Deferred = CoroutineScope(coroutineContext).async { // Make sure auth cache is populated before we try to sync. @@ -370,7 +380,7 @@ open class FxaAccountManager( "Sync is not configured. Construct this class with a 'syncConfig' or use 'setSyncConfig'" ) } - syncManager?.now(startup, debounce) + syncManager?.now(reason, debounce) } Unit } @@ -639,7 +649,7 @@ open class FxaAccountManager( deviceConfig.name, deviceConfig.type, deviceConfig.capabilities ).await() - postAuthenticated(via.authData.authType) + postAuthenticated(via.authData.authType, via.authData.declinedEngines) Event.FetchProfile } @@ -820,13 +830,41 @@ open class FxaAccountManager( return null } - private suspend fun postAuthenticated(authType: AuthType) { + private suspend fun postAuthenticated(authType: AuthType, declinedEngines: Set? = null) { + // Sync may not be configured at all (e.g. won't run), but if we received a + // list of declined engines, that indicates user intent, so we preserve it + // within SyncEnginesStorage regardless. If sync becomes enabled later on, + // we will be respecting user choice around what to sync. + declinedEngines?.let { + val enginesStorage = SyncEnginesStorage(context) + declinedEngines.forEach { declinedEngine -> + enginesStorage.setStatus(declinedEngine, false) + } + + // Enable all engines that were not explicitly disabled. Only do this in + // the presence of a "declinedEngines" list, since that indicates user + // intent. Absence of that list means that user was not presented with a + // choice during authentication, and so we can't assume an enabled state + // for any of the engines. + syncConfig?.supportedEngines?.forEach { supportedEngine -> + if (!declinedEngines.contains(supportedEngine)) { + enginesStorage.setStatus(supportedEngine, true) + } + } + } + // Before any sync workers have a chance to access it, make sure our SyncAuthInfo cache is hot. maybeUpdateSyncAuthInfoCache() // Notify our internal (sync) and external (app logic) observers. notifyObservers { onAuthenticated(account, authType) } + FxaDeviceSettingsCache(context).setToCache(DeviceSettings( + fxaDeviceId = account.getCurrentDeviceId()!!, + name = deviceConfig.name, + type = deviceConfig.type.intoSyncType() + )) + // If device supports SEND_TAB... if (deviceConfig.capabilities.contains(DeviceCapability.SEND_TAB)) { // ... update constellation state, and poll for any pending device events. @@ -869,7 +907,8 @@ open class FxaAccountManager( /** * Account status events flowing into the sync manager. */ - private class AccountsToSyncIntegration( + @VisibleForTesting + internal class AccountsToSyncIntegration( private val syncManager: SyncManager ) : AccountObserver { override fun onLoggedOut() { @@ -877,7 +916,12 @@ open class FxaAccountManager( } override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { - syncManager.start() + val reason = when (authType) { + is AuthType.OtherExternal, AuthType.Signin, AuthType.Signup, AuthType.Shared, AuthType.Pairing + -> SyncReason.FirstSync + AuthType.Existing, AuthType.Recovered -> SyncReason.Startup + } + syncManager.start(reason) } override fun onProfileUpdated(profile: Profile) { diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/SyncEnginesStorage.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/SyncEnginesStorage.kt new file mode 100644 index 00000000000..9093a9613de --- /dev/null +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/manager/SyncEnginesStorage.kt @@ -0,0 +1,40 @@ +/* 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/. */ + +package mozilla.components.service.fxa.manager + +import android.content.Context +import mozilla.components.service.fxa.SyncEngine +import mozilla.components.service.fxa.sync.toSyncEngine + +/** + * TODO DOCS + */ +class SyncEnginesStorage(val context: Context) { + companion object { + const val SYNC_ENGINES_KEY = "syncEngines" + } + + fun getStatus(): Map { + val resultMap = mutableMapOf() + // TODO think through changes in the Engine list... default value? initial value? + + // this needs to be reversed: go through what we have in local storage, and populate result map based on that. + // reason: we may have "other" engines. + // this will be empty if `setStatus` was never called. + storage().all.forEach { + if (it.value is Boolean) { + resultMap[it.key.toSyncEngine()] = it.value as Boolean + } + } + + return resultMap + } + + fun setStatus(engine: SyncEngine, status: Boolean) { + storage().edit().putBoolean(engine.nativeName, status).apply() // todo consider commit() + } + + private fun storage() = context.getSharedPreferences(SYNC_ENGINES_KEY, Context.MODE_PRIVATE) +} diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/SyncManager.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/SyncManager.kt index 985ead0ffca..882ccbd75ea 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/SyncManager.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/SyncManager.kt @@ -7,6 +7,7 @@ package mozilla.components.service.fxa.sync import mozilla.components.concept.sync.SyncableStore import mozilla.components.service.fxa.SyncConfig import mozilla.components.service.fxa.SyncEngine +import mozilla.components.service.fxa.manager.SyncEnginesStorage import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.base.observer.Observable import mozilla.components.support.base.observer.ObserverRegistry @@ -25,6 +26,8 @@ interface SyncStatusObserver { /** * Gets called at the end of a sync, after every configured syncable has been synchronized. + * A set of enabled [SyncEngine]s could have changed, so observers are expected to query + * [SyncEnginesStorage.getStatus]. */ fun onIdle() @@ -59,12 +62,20 @@ object GlobalSyncableStoreProvider { */ interface SyncDispatcher : Closeable, Observable { fun isSyncActive(): Boolean - fun syncNow(startup: Boolean = false, debounce: Boolean = false) + fun syncNow(reason: SyncReason, debounce: Boolean = false) fun startPeriodicSync(unit: TimeUnit, period: Long) fun stopPeriodicSync() fun workersStateChanged(isRunning: Boolean) } +sealed class SyncReason { + object Startup : SyncReason() + object User : SyncReason() + object EngineChange : SyncReason() + internal object FirstSync : SyncReason() + internal object Scheduled : SyncReason() +} + /** * A base sync manager implementation. * @param syncConfig A [SyncConfig] object describing how sync should behave. @@ -120,24 +131,25 @@ abstract class SyncManager( /** * Request an immediate synchronization of all configured stores. * - * @param startup Boolean flag indicating if sync is being requested in a startup situation. + * @param reason A [SyncReason] indicating why this sync is being requested. + * @param debounce Whether or not this sync should debounced. */ - internal fun now(startup: Boolean = false, debounce: Boolean = false) = synchronized(this) { + internal fun now(reason: SyncReason, debounce: Boolean = false) = synchronized(this) { if (syncDispatcher == null) { logger.info("Sync is not enabled. Ignoring 'sync now' request.") } syncDispatcher?.let { - logger.debug("Requesting immediate sync") - it.syncNow(startup, debounce) + logger.debug("Requesting immediate sync, reason: $reason, debounce: $debounce") + it.syncNow(reason, debounce) } } /** * Enables synchronization, with behaviour described in [syncConfig]. */ - internal fun start() = synchronized(this) { + internal fun start(reason: SyncReason) = synchronized(this) { logger.debug("Enabling...") - syncDispatcher = initDispatcher(newDispatcher(syncDispatcher, syncConfig.supportedEngines)) + syncDispatcher = initDispatcher(newDispatcher(syncDispatcher, syncConfig.supportedEngines), reason) logger.debug("set and initialized new dispatcher: $syncDispatcher") } @@ -170,9 +182,9 @@ abstract class SyncManager( return createDispatcher(supportedEngines) } - private fun initDispatcher(dispatcher: SyncDispatcher): SyncDispatcher { + private fun initDispatcher(dispatcher: SyncDispatcher, reason: SyncReason): SyncDispatcher { dispatcher.register(dispatcherStatusObserver) - dispatcher.syncNow() + dispatcher.syncNow(reason) if (syncConfig.syncPeriodInMinutes != null) { dispatcher.startPeriodicSync(SYNC_PERIOD_UNIT, syncConfig.syncPeriodInMinutes) } diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/Types.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/Types.kt new file mode 100644 index 00000000000..8a4518fdf94 --- /dev/null +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/Types.kt @@ -0,0 +1,61 @@ +package mozilla.components.service.fxa.sync + +import mozilla.appservices.syncmanager.SyncAuthInfo +import mozilla.components.service.fxa.SyncEngine + +/** + * Conversion from our SyncAuthInfo into its "native" version used at the interface boundary. + */ +internal fun mozilla.components.concept.sync.SyncAuthInfo.toNative(): SyncAuthInfo { + return SyncAuthInfo( + kid = this.kid, + fxaAccessToken = this.fxaAccessToken, + syncKey = this.syncKey, + tokenserverURL = this.tokenServerUrl + ) +} + +fun String.toSyncEngine(): SyncEngine { + return when (this) { + "history" -> SyncEngine.History + "bookmarks" -> SyncEngine.Bookmarks + "passwords" -> SyncEngine.Passwords + // This handles a case of engines that we don't yet model in SyncEngine. + else -> SyncEngine.Other(this) + } +} + +fun List.toSyncEngines(): Set { + return this.map { it.toSyncEngine() }.toSet() +} + +fun SyncReason.toRustSyncReason(): mozilla.appservices.syncmanager.SyncReason { + return when (this) { + SyncReason.Startup -> mozilla.appservices.syncmanager.SyncReason.STARTUP + SyncReason.User -> mozilla.appservices.syncmanager.SyncReason.USER + SyncReason.Scheduled -> mozilla.appservices.syncmanager.SyncReason.SCHEDULED + SyncReason.EngineChange -> mozilla.appservices.syncmanager.SyncReason.ENABLED_CHANGE + SyncReason.FirstSync -> mozilla.appservices.syncmanager.SyncReason.ENABLED_CHANGE + } +} + +fun SyncReason.asString(): String { + return when (this) { + SyncReason.FirstSync -> "first_sync" + SyncReason.Scheduled -> "scheduled" + SyncReason.EngineChange -> "engine_change" + SyncReason.User -> "user" + SyncReason.Startup -> "startup" + } +} + +fun String.toSyncReason(): SyncReason { + return when (this) { + "startup" -> SyncReason.Startup + "user" -> SyncReason.User + "engine_change" -> SyncReason.EngineChange + "scheduled" -> SyncReason.Scheduled + "first_sync" -> SyncReason.FirstSync + else -> throw IllegalStateException("Invalid SyncReason: $this") + } +} diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt index f0680bf48df..aef5beaf02b 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt @@ -6,6 +6,7 @@ package mozilla.components.service.fxa.sync import android.content.Context import androidx.annotation.UiThread +import androidx.work.BackoffPolicy import androidx.work.Constraints import androidx.work.CoroutineWorker import androidx.work.Data @@ -19,15 +20,21 @@ import androidx.work.PeriodicWorkRequestBuilder import androidx.work.WorkInfo import androidx.work.WorkManager import androidx.work.WorkerParameters +import mozilla.appservices.syncmanager.SyncParams +import mozilla.appservices.syncmanager.SyncServiceStatus +import mozilla.appservices.syncmanager.SyncManager as RustSyncManager import mozilla.components.concept.sync.AuthException -import mozilla.components.concept.sync.SyncStatus +import mozilla.components.concept.sync.AuthExceptionType +import mozilla.components.service.fxa.FxaDeviceSettingsCache import mozilla.components.service.fxa.SyncAuthInfoCache import mozilla.components.service.fxa.SyncConfig import mozilla.components.service.fxa.SyncEngine +import mozilla.components.service.fxa.manager.SyncEnginesStorage import mozilla.components.service.fxa.manager.authErrorRegistry import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.base.observer.Observable import mozilla.components.support.base.observer.ObserverRegistry +import mozilla.components.support.sync.telemetry.SyncTelemetry import java.io.Closeable import java.util.concurrent.TimeUnit @@ -43,6 +50,9 @@ private enum class SyncWorkerName { } private const val KEY_DATA_STORES = "stores" +private const val KEY_REASON = "reason" + +private const val SYNC_WORKER_BACKOFF_DELAY_MINUTES = 3L /** * A [SyncManager] implementation which uses WorkManager APIs to schedule sync tasks. @@ -136,9 +146,9 @@ class WorkManagerSyncDispatcher( return isSyncActive } - override fun syncNow(startup: Boolean, debounce: Boolean) { - logger.debug("Immediate sync requested, startup = $startup") - val delayMs = if (startup) { + override fun syncNow(reason: SyncReason, debounce: Boolean) { + logger.debug("Immediate sync requested, reason = $reason, debounce = $debounce") + val delayMs = if (reason == SyncReason.Startup) { // Startup delay is there to avoid SQLITE_BUSY crashes, since we currently do a poor job // of managing database connections, and we expect there to be database writes at the start. // We've done bunch of work to make this better (see https://github.com/mozilla-mobile/android-components/issues/1369), @@ -152,7 +162,7 @@ class WorkManagerSyncDispatcher( // Use the 'keep' policy to minimize overhead from multiple "sync now" operations coming in // at the same time. ExistingWorkPolicy.KEEP, - regularSyncWorkRequest(delayMs, debounce) + regularSyncWorkRequest(reason, delayMs, debounce) ).enqueue() } @@ -185,7 +195,7 @@ class WorkManagerSyncDispatcher( } private fun periodicSyncWorkRequest(unit: TimeUnit, period: Long): PeriodicWorkRequest { - val data = getWorkerData() + val data = getWorkerData(SyncReason.Scheduled) // Periodic interval must be at least PeriodicWorkRequest.MIN_PERIODIC_INTERVAL_MILLIS, // e.g. not more frequently than 15 minutes. return PeriodicWorkRequestBuilder(period, unit) @@ -197,11 +207,20 @@ class WorkManagerSyncDispatcher( .setInputData(data) .addTag(SyncWorkerTag.Common.name) .addTag(SyncWorkerTag.Debounce.name) + .setBackoffCriteria( + BackoffPolicy.EXPONENTIAL, + SYNC_WORKER_BACKOFF_DELAY_MINUTES, + TimeUnit.MINUTES + ) .build() } - private fun regularSyncWorkRequest(delayMs: Long = 0L, debounce: Boolean = false): OneTimeWorkRequest { - val data = getWorkerData() + private fun regularSyncWorkRequest( + reason: SyncReason, + delayMs: Long = 0L, + debounce: Boolean = false + ): OneTimeWorkRequest { + val data = getWorkerData(reason) return OneTimeWorkRequestBuilder() .setConstraints( Constraints.Builder() @@ -212,15 +231,19 @@ class WorkManagerSyncDispatcher( .addTag(SyncWorkerTag.Common.name) .addTag(if (debounce) SyncWorkerTag.Debounce.name else SyncWorkerTag.Immediate.name) .setInitialDelay(delayMs, TimeUnit.MILLISECONDS) + .setBackoffCriteria( + BackoffPolicy.EXPONENTIAL, + SYNC_WORKER_BACKOFF_DELAY_MINUTES, + TimeUnit.MINUTES + ) .build() } - private fun getWorkerData(): Data { - val dataBuilder = Data.Builder().putStringArray( - KEY_DATA_STORES, supportedEngines.map { it.nativeName }.toTypedArray() - ) - - return dataBuilder.build() + private fun getWorkerData(reason: SyncReason): Data { + return Data.Builder() + .putStringArray(KEY_DATA_STORES, supportedEngines.map { it.nativeName }.toTypedArray()) + .putString(KEY_REASON, reason.asString()) + .build() } } @@ -239,7 +262,7 @@ class WorkManagerSyncWorker( return lastSyncedTs != 0L && (System.currentTimeMillis() - lastSyncedTs) < SYNC_STAGGER_BUFFER_MS } - @Suppress("ReturnCount") + @Suppress("ReturnCount", "LongMethod", "ComplexMethod") override suspend fun doWork(): Result { logger.debug("Starting sync... Tagged as: ${params.tags}") @@ -248,62 +271,190 @@ class WorkManagerSyncWorker( return Result.success() } - // Otherwise, proceed as normal. - // In order to sync, we'll need: - // - a list of SyncableStores... + // Otherwise, proceed as normal and start preparing to sync. + + // We will need a list of SyncableStores. val syncableStores = params.inputData.getStringArray(KEY_DATA_STORES)!!.associate { - it to GlobalSyncableStoreProvider.getStore(it)!! + // Convert from a string back to our SyncEngine type. + val engine = when (it) { + SyncEngine.History.nativeName -> SyncEngine.History + SyncEngine.Bookmarks.nativeName -> SyncEngine.Bookmarks + SyncEngine.Passwords.nativeName -> SyncEngine.Passwords + else -> throw IllegalStateException("Invalid syncable store: $it") + } + engine to GlobalSyncableStoreProvider.getStore(engine.nativeName)!! }.ifEmpty { // Short-circuit if there are no configured stores. // Don't update the "last-synced" timestamp because we haven't actually synced anything. return Result.success() + }.also { stores -> + // We need to tell RustSyncManager about instances of supported stores ('places' and 'logins'). + stores.entries.forEach { + // We're assuming all syncable stores live in Rust. + // Currently `RustSyncManager` doesn't support non-Rust sync engines. + when (it.key) { + // NB: History and Bookmarks will have the same handle. + SyncEngine.History -> RustSyncManager.setPlaces(it.value.getHandle()) + SyncEngine.Bookmarks -> RustSyncManager.setPlaces(it.value.getHandle()) + SyncEngine.Passwords -> RustSyncManager.setLogins(it.value.getHandle()) + } + } } - // - and a cached "sync auth info" object. - val syncAuthInfo = SyncAuthInfoCache(context).getCached() ?: return Result.failure() + // We need to know the reason we're syncing. + val reason = params.inputData.getString(KEY_REASON)!!.toSyncReason() - // Sync! - val syncResult = StorageSync(syncableStores).sync(syncAuthInfo) + // We need a cached "sync auth info" object. + val syncAuthInfo = SyncAuthInfoCache(context).getCached() ?: return Result.failure() - val resultBuilder = Data.Builder() - syncResult.forEach { - when (val status = it.value.status) { - SyncStatus.Ok -> { - logger.info("Synchronized store ${it.key}") - resultBuilder.putBoolean(it.key, true) - } - is SyncStatus.Error -> { - val exception = status.exception - when (exception) { - // Notify auth error observers that we saw an auth-related error while syncing. - is AuthException -> { - authErrorRegistry.notifyObservers { - onAuthErrorAsync(exception) - } - } - } - - logger.error("Failed to synchronize store ${it.key}", exception) - resultBuilder.putBoolean(it.key, false) + // We need any persisted state that we received from RustSyncManager in the past. + // We should be able to pass a `null` value, but currently the library will crash. + // See https://github.com/mozilla/application-services/issues/1829 + val currentSyncState = getSyncState(context) ?: "" + + // We need to tell RustSyncManager about our local "which engines are enabled/disabled" state. + // This is a global state, shared by every sync client for this account. State that we will + // pass here will overwrite current global state and may be propagated to every sync client. + // This should be empty if there have been no changes to this state. + // We pass this state if user changed it (EngineChange) or if we're in a first sync situation. + // A "first sync" will happen after signing up or signing in. + // In both cases, user may have been asked which engines they'd like to sync. + val enabledChanges = when (reason) { + SyncReason.EngineChange, SyncReason.FirstSync -> { + val engineMap = SyncEnginesStorage(context).getStatus().toMutableMap() + // For historical reasons, a "history engine" really means two sync collections: history and forms. + // Underlying library doesn't manage this for us, and other clients will get confused + // if we modify just "history" without also modifying "forms", and vice versa. + // So: whenever a change to the "history" engine happens locally, inject the same "forms" change. + // This should be handled by RustSyncManager. See https://github.com/mozilla/application-services/issues/1832 + engineMap[SyncEngine.History]?.let { + engineMap[SyncEngine.Forms] = it } + engineMap.mapKeys { it.key.nativeName } } + else -> emptyMap() + } + + // We need to tell RustSyncManager which engines to sync. 'null' means "sync all", which is an + // intersection of stores for which we've set a 'handle' and those that are enabled. + // This should be an empty list if we only want to sync metadata (e.g. which engines are enabled). + val enginesToSync = null + + // We need to tell RustSyncManager about our current FxA device. It needs that information + // in order to sync the 'clients' collection. + // We're depending on cache being populated. An alternative to using a "cache" is to + // configure the worker with the values stored in it: device name, type and fxaDeviceID. + // While device type and fxaDeviceID are stable, users are free to change device name whenever. + // We need to reflect these changes during a sync. Our options are then: provide a global cache, + // or re-configure our workers every time a change is made to the device name. + // We have the same basic story already with syncAuthInfo cache, and a cache is much easier + // to implement/reason about than worker reconfiguration. + val deviceSettings = FxaDeviceSettingsCache(context).getCached()!! + + // We're now ready to sync. + val syncParams = SyncParams( + reason = reason.toRustSyncReason(), + engines = enginesToSync, + authInfo = syncAuthInfo.toNative(), + enabledChanges = enabledChanges, + persistedState = currentSyncState, + deviceSettings = deviceSettings + ) + val syncResult = RustSyncManager.sync(syncParams) + + // Persist the sync state; it may have changed during a sync, and RustSyncManager relies on us + // to store it. + setSyncState(context, syncResult.persistedState) + + // Log the results. + syncResult.failures.entries.forEach { + logger.error("Failed to sync ${it.key}, reason: ${it.value}") + } + syncResult.successful.forEach { + logger.info("Successfully synced $it") } - // Worker should set the "last-synced" timestamp, and since we have a single timestamp, - // it's not clear if a single failure should prevent its update. That's the current behaviour - // in Fennec, but for very specific reasons that aren't relevant here. We could have - // a timestamp per store, or whatever we want here really. - // For now, we just update it every time we attempt to sync, regardless of the results. - setLastSynced(context, System.currentTimeMillis()) + // Process any changes to the list of declined/accepted engines. + // NB: We may have received engine state information about an engine we're unfamiliar with. + // `SyncEngine.Other(string)` stands in for unknown engines. + val declinedEngines = syncResult.declined?.map { it.toSyncEngine() }?.toSet() ?: emptySet() + // We synthesize the 'accepted' list ourselves: engines we know about - declined engines. + // This assumes that "engines we know about" is a subset of engines RustSyncManager knows about. + // RustSyncManager will handle this, eventually. + // See: https://github.com/mozilla/application-services/issues/1685 + val acceptedEngines = syncableStores.keys.filter { !declinedEngines.contains(it) } + + // Persist engine state changes. + with(SyncEnginesStorage(context)) { + declinedEngines.forEach { setStatus(it, status = false) } + acceptedEngines.forEach { setStatus(it, status = true) } + } + + // Process telemetry. + syncResult.telemetry?.let { + // TODO in case of errors, will this not duplicate error reporting? + SyncTelemetry.processBookmarksPing(it) + SyncTelemetry.processHistoryPing(it) + // TODO file a follow-up for passwords ping + } - // Always return 'success' here. In the future, we could inspect exceptions in SyncError - // and request a 'retry' if we see temporary problems such as network errors. - return Result.success(resultBuilder.build()) + // Finally, declare success, failure or request a retry based on 'sync status'. + return when (syncResult.status) { + // Happy case. + SyncServiceStatus.OK -> { + logger.error("All good") + // TODO think about error reporting. + // Worker should set the "last-synced" timestamp, and since we have a single timestamp, + // it's not clear if a single failure should prevent its update. That's the current behaviour + // in Fennec, but for very specific reasons that aren't relevant here. We could have + // a timestamp per store, or whatever we want here really. + // For now, we just update it every time we succeed to sync. + setLastSynced(context, System.currentTimeMillis()) + Result.success() + } + + // Retry cases. + // NB: retry doesn't mean "immediate retry". It means "retry, but respecting this worker's + // backoff policy, as configured during worker's creation. + // TODO FOR ALL retries: look at workerParams.mRunAttemptCount, don't retry after a certain number. + SyncServiceStatus.NETWORK_ERROR -> { + logger.error("Network error") + Result.retry() + } + SyncServiceStatus.BACKED_OFF -> { + logger.error("Backed-off error") + // As part of `syncResult`, we get back `nextSyncAllowedAt`. Ideally, we should not retry + // before that passes. However, we can not reconfigure back-off policy for an already + // created Worker. So, we just rely on a sensible default. `RustSyncManager` will fail + // to sync with a BACKED_OFF error without hitting the server if we don't respect + // `nextSyncAllowedAt`, so we should be good either way. + Result.retry() + } + + // Failure cases. + SyncServiceStatus.AUTH_ERROR -> { + logger.error("Auth error") + authErrorRegistry.notifyObservers { + // TODO simplify this nesting + onAuthErrorAsync(AuthException(AuthExceptionType.UNAUTHORIZED)) + } + Result.failure() + } + SyncServiceStatus.SERVICE_ERROR -> { + logger.error("Service error") + Result.failure() + } + SyncServiceStatus.OTHER_ERROR -> { + logger.error("'Other' error :(") + Result.failure() + } + } } } private const val SYNC_STATE_PREFS_KEY = "syncPrefs" private const val SYNC_LAST_SYNCED_KEY = "lastSynced" +private const val SYNC_STATE_KEY = "persistedState" private const val SYNC_STAGGER_BUFFER_MS = 10 * 60 * 1000L // 10 minutes. private const val SYNC_STARTUP_DELAY_MS = 5 * 1000L // 5 seconds. @@ -321,3 +472,17 @@ fun setLastSynced(context: Context, ts: Long) { .putLong(SYNC_LAST_SYNCED_KEY, ts) .apply() } + +fun getSyncState(context: Context): String? { + return context + .getSharedPreferences(SYNC_STATE_PREFS_KEY, Context.MODE_PRIVATE) + .getString(SYNC_STATE_KEY, null) +} + +fun setSyncState(context: Context, state: String) { + context + .getSharedPreferences(SYNC_STATE_PREFS_KEY, Context.MODE_PRIVATE) + .edit() + .putString(SYNC_STATE_KEY, state) + .apply() +} diff --git a/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt b/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt index 292ce5fd1b0..faa8bb3d829 100644 --- a/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt +++ b/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaAccountManagerTest.kt @@ -33,6 +33,7 @@ import mozilla.components.service.fxa.sharing.ShareableAccount import mozilla.components.service.fxa.sharing.ShareableAuthInfo import mozilla.components.service.fxa.sync.SyncManager import mozilla.components.service.fxa.sync.SyncDispatcher +import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.service.fxa.sync.SyncStatusObserver import mozilla.components.support.base.observer.Observable import mozilla.components.support.base.observer.ObserverRegistry @@ -58,6 +59,7 @@ import org.mockito.Mockito.never import org.mockito.Mockito.reset import org.mockito.Mockito.times import org.mockito.Mockito.verify +import org.mockito.Mockito.verifyNoMoreInteractions import java.lang.Exception import java.lang.IllegalArgumentException import java.util.concurrent.TimeUnit @@ -190,8 +192,8 @@ class FxaAccountManagerTest { return inner.isSyncActive() } - override fun syncNow(startup: Boolean, debounce: Boolean) { - inner.syncNow(startup, debounce) + override fun syncNow(reason: SyncReason, debounce: Boolean) { + inner.syncNow(reason, debounce) } override fun startPeriodicSync(unit: TimeUnit, period: Long) { @@ -307,23 +309,23 @@ class FxaAccountManagerTest { assertEquals(0, syncStatusObserver.onErrorCount) // No periodic sync. - manager.setSyncConfigAsync(SyncConfig(setOf(SyncEngine.HISTORY))).await() + manager.setSyncConfigAsync(SyncConfig(setOf(SyncEngine.History))).await() - assertEquals(setOf(SyncEngine.HISTORY), manager.supportedSyncEngines()) + assertEquals(setOf(SyncEngine.History), manager.supportedSyncEngines()) assertNotNull(latestSyncManager) assertNotNull(latestSyncManager?.dispatcher) assertNotNull(latestSyncManager?.dispatcher?.inner) verify(latestSyncManager!!.dispatcher.inner, never()).startPeriodicSync(any(), anyLong()) verify(latestSyncManager!!.dispatcher.inner, never()).stopPeriodicSync() - verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(anyBoolean(), anyBoolean()) + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(eq(SyncReason.FirstSync), anyBoolean()) // With periodic sync. - manager.setSyncConfigAsync(SyncConfig(setOf(SyncEngine.HISTORY, SyncEngine.PASSWORDS), 60 * 1000L)).await() + manager.setSyncConfigAsync(SyncConfig(setOf(SyncEngine.History, SyncEngine.Passwords), 60 * 1000L)).await() - assertEquals(setOf(SyncEngine.HISTORY, SyncEngine.PASSWORDS), manager.supportedSyncEngines()) + assertEquals(setOf(SyncEngine.History, SyncEngine.Passwords), manager.supportedSyncEngines()) verify(latestSyncManager!!.dispatcher.inner, times(1)).startPeriodicSync(any(), anyLong()) verify(latestSyncManager!!.dispatcher.inner, never()).stopPeriodicSync() - verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(anyBoolean(), anyBoolean()) + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(eq(SyncReason.FirstSync), anyBoolean()) // Make sure sync status listeners are working. // TODO fix these tests. @@ -364,7 +366,7 @@ class FxaAccountManagerTest { // With a sync config this time. var latestSyncManager: TestSyncManager? = null - val syncConfig = SyncConfig(setOf(SyncEngine.HISTORY), syncPeriodInMinutes = 120L) + val syncConfig = SyncConfig(setOf(SyncEngine.History), syncPeriodInMinutes = 120L) val manager = object : TestableFxaAccountManager( context = testContext, config = ServerConfig.release("dummyId", "http://auth-url/redirect"), @@ -408,15 +410,15 @@ class FxaAccountManagerTest { assertNotNull(latestSyncManager!!.dispatcher.inner) verify(latestSyncManager!!.dispatcher.inner, times(1)).startPeriodicSync(any(), anyLong()) verify(latestSyncManager!!.dispatcher.inner, never()).stopPeriodicSync() - verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(anyBoolean(), anyBoolean()) + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(eq(SyncReason.Startup), anyBoolean()) // Can trigger syncs. - manager.syncNowAsync().await() - verify(latestSyncManager!!.dispatcher.inner, times(2)).syncNow(startup = false, debounce = false) - manager.syncNowAsync(startup = true).await() - verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(startup = true, debounce = false) - manager.syncNowAsync(startup = true, debounce = true).await() - verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(startup = true, debounce = true) + manager.syncNowAsync(SyncReason.User).await() + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(SyncReason.User, debounce = false) + manager.syncNowAsync(SyncReason.Startup).await() + verify(latestSyncManager!!.dispatcher.inner, times(2)).syncNow(SyncReason.Startup, debounce = false) + manager.syncNowAsync(SyncReason.EngineChange, debounce = true).await() + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(SyncReason.EngineChange, debounce = true) // TODO fix these tests // assertEquals(0, syncStatusObserver.onStartedCount) @@ -433,17 +435,17 @@ class FxaAccountManagerTest { // assertEquals(1, syncStatusObserver.onErrorCount) // Turn off periodic syncing. - manager.setSyncConfigAsync(SyncConfig(setOf(SyncEngine.HISTORY))).await() + manager.setSyncConfigAsync(SyncConfig(setOf(SyncEngine.History))).await() verify(latestSyncManager!!.dispatcher.inner, never()).startPeriodicSync(any(), anyLong()) verify(latestSyncManager!!.dispatcher.inner, never()).stopPeriodicSync() - verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(anyBoolean(), anyBoolean()) + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(SyncReason.FirstSync, debounce = false) // Can trigger syncs. - manager.syncNowAsync().await() - verify(latestSyncManager!!.dispatcher.inner, times(2)).syncNow(startup = false, debounce = false) - manager.syncNowAsync(startup = true).await() - verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(startup = true, debounce = false) + manager.syncNowAsync(SyncReason.User).await() + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(SyncReason.User, debounce = false) + manager.syncNowAsync(SyncReason.Startup).await() + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(SyncReason.Startup, debounce = false) // Pretend sync is running. `when`(latestSyncManager!!.dispatcher.inner.isSyncActive()).thenReturn(true) @@ -779,7 +781,7 @@ class FxaAccountManagerTest { } override fun getCurrentDeviceId(): String? { - return null + return "testFxaDeviceId" } override fun getSessionToken(): String? { @@ -914,6 +916,7 @@ class FxaAccountManagerTest { `when`(mockAccount.getProfileAsync(anyBoolean())).thenReturn(CompletableDeferred(profile)) // We have an account at the start. `when`(accountStorage.read()).thenReturn(mockAccount) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(mockAccount.deviceConstellation()).thenReturn(constellation) `when`(constellation.ensureCapabilitiesAsync(any())).thenReturn(CompletableDeferred(true)) @@ -984,6 +987,7 @@ class FxaAccountManagerTest { assertNull(manager.authenticatedAccount()) assertNull(manager.accountProfile()) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(mockAccount.deviceConstellation()).thenReturn(constellation) `when`(constellation.initDeviceAsync(any(), any(), any())).thenReturn(CompletableDeferred(true)) @@ -1072,6 +1076,7 @@ class FxaAccountManagerTest { assertNull(manager.authenticatedAccount()) assertNull(manager.accountProfile()) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(mockAccount.deviceConstellation()).thenReturn(constellation) `when`(constellation.initDeviceAsync(any(), any(), any())).thenReturn(CompletableDeferred(true)) @@ -1098,6 +1103,7 @@ class FxaAccountManagerTest { val manager = prepareUnhappyAuthenticationFlow(mockAccount, profile, accountStorage, accountObserver, this.coroutineContext) `when`(mockAccount.deviceConstellation()).thenReturn(constellation) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") // We start off as logged-out, but the event won't be called (initial default state is assumed). verify(accountObserver, never()).onLoggedOut() @@ -1142,6 +1148,7 @@ class FxaAccountManagerTest { val accountObserver: AccountObserver = mock() val manager = prepareUnhappyAuthenticationFlow(mockAccount, profile, accountStorage, accountObserver, this.coroutineContext) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(mockAccount.deviceConstellation()).thenReturn(constellation) // We start off as logged-out, but the event won't be called (initial default state is assumed). @@ -1197,6 +1204,7 @@ class FxaAccountManagerTest { assertNull(manager.accountProfile()) `when`(mockAccount.deviceConstellation()).thenReturn(constellation) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(constellation.initDeviceAsync(any(), any(), any())).thenReturn(CompletableDeferred(true)) assertTrue(manager.finishAuthenticationAsync(FxaAuthData(AuthType.Signin, "dummyCode", EXPECTED_AUTH_STATE)).await()) @@ -1250,6 +1258,7 @@ class FxaAccountManagerTest { assertNull(manager.authenticatedAccount()) assertNull(manager.accountProfile()) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(mockAccount.deviceConstellation()).thenReturn(constellation) `when`(constellation.initDeviceAsync(any(), any(), any())).thenReturn(CompletableDeferred(true)) @@ -1281,6 +1290,7 @@ class FxaAccountManagerTest { val constellation: DeviceConstellation = mock() `when`(mockAccount.deviceConstellation()).thenReturn(constellation) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(constellation.initDeviceAsync(any(), any(), any())).thenReturn(CompletableDeferred(true)) `when`(mockAccount.getProfileAsync(anyBoolean())).thenReturn(CompletableDeferred(value = null)) `when`(mockAccount.beginOAuthFlowAsync(any())).thenReturn(CompletableDeferred(AuthFlowUrl(EXPECTED_AUTH_STATE, "auth://url"))) @@ -1343,6 +1353,7 @@ class FxaAccountManagerTest { val mockAccount: OAuthAccount = mock() val constellation: DeviceConstellation = mock() + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(mockAccount.deviceConstellation()).thenReturn(constellation) `when`(constellation.initDeviceAsync(any(), any(), any())).thenReturn(CompletableDeferred(true)) @@ -1400,6 +1411,7 @@ class FxaAccountManagerTest { val constellation: DeviceConstellation = mock() `when`(mockAccount.deviceConstellation()).thenReturn(constellation) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(constellation.initDeviceAsync(any(), any(), any())).thenReturn(CompletableDeferred(true)) `when`(mockAccount.getProfileAsync(anyBoolean())).then { @@ -1456,6 +1468,7 @@ class FxaAccountManagerTest { val constellation: DeviceConstellation = mock() val captor = argumentCaptor() + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(mockAccount.deviceConstellation()).thenReturn(constellation) `when`(constellation.initDeviceAsync(any(), any(), any())).thenReturn(CompletableDeferred(true)) @@ -1532,6 +1545,7 @@ class FxaAccountManagerTest { val fxaException = FxaPanicException("500") exceptionalProfile.completeExceptionally(fxaException) + `when`(mockAccount.getCurrentDeviceId()).thenReturn("testDeviceId") `when`(mockAccount.deviceConstellation()).thenReturn(constellation) `when`(constellation.initDeviceAsync(any(), any(), any())).thenReturn(CompletableDeferred(true)) `when`(mockAccount.getProfileAsync(anyBoolean())).thenReturn(exceptionalProfile) @@ -1567,6 +1581,43 @@ class FxaAccountManagerTest { assertTrue(manager.finishAuthenticationAsync(FxaAuthData(AuthType.Signin, "dummyCode", EXPECTED_AUTH_STATE)).await()) } + @Test + fun `accounts to sync integration`() { + val syncManager: SyncManager = mock() + val integration = FxaAccountManager.AccountsToSyncIntegration(syncManager) + + // onAuthenticated - mapping of AuthType to SyncReason + integration.onAuthenticated(mock(), AuthType.Signin) + verify(syncManager, times(1)).start(SyncReason.FirstSync) + integration.onAuthenticated(mock(), AuthType.Signup) + verify(syncManager, times(2)).start(SyncReason.FirstSync) + integration.onAuthenticated(mock(), AuthType.Pairing) + verify(syncManager, times(3)).start(SyncReason.FirstSync) + integration.onAuthenticated(mock(), AuthType.Shared) + verify(syncManager, times(4)).start(SyncReason.FirstSync) + integration.onAuthenticated(mock(), AuthType.OtherExternal("test")) + verify(syncManager, times(5)).start(SyncReason.FirstSync) + integration.onAuthenticated(mock(), AuthType.Existing) + verify(syncManager, times(1)).start(SyncReason.Startup) + integration.onAuthenticated(mock(), AuthType.Recovered) + verify(syncManager, times(2)).start(SyncReason.Startup) + verifyNoMoreInteractions(syncManager) + + // onProfileUpdated - no-op + integration.onProfileUpdated(mock()) + verifyNoMoreInteractions(syncManager) + + // onAuthenticationProblems + integration.onAuthenticationProblems() + verify(syncManager).stop() + verifyNoMoreInteractions(syncManager) + + // onLoggedOut + integration.onLoggedOut() + verify(syncManager, times(2)).stop() + verifyNoMoreInteractions(syncManager) + } + private fun prepareHappyAuthenticationFlow( mockAccount: OAuthAccount, profile: Profile, diff --git a/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaDeviceConstellationTest.kt b/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaDeviceConstellationTest.kt index ab11775f992..9b376beaa7b 100644 --- a/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaDeviceConstellationTest.kt +++ b/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/FxaDeviceConstellationTest.kt @@ -25,6 +25,7 @@ import mozilla.components.concept.sync.DevicePushSubscription import mozilla.components.concept.sync.DeviceType import mozilla.components.concept.sync.TabData import mozilla.components.support.test.mock +import mozilla.components.support.test.robolectric.testContext import org.junit.Assert import mozilla.appservices.fxaclient.FirefoxAccount as NativeFirefoxAccount import mozilla.appservices.fxaclient.Device as NativeDevice @@ -76,7 +77,7 @@ class FxaDeviceConstellationTest { `when`(account.getDevices()).thenReturn(arrayOf(currentDevice)) // No device state observer. - assertTrue(constellation.setDeviceNameAsync("new name").await()) + assertTrue(constellation.setDeviceNameAsync("new name", testContext).await()) verify(account).setDeviceDisplayName("new name") // Set up the observer... @@ -89,7 +90,7 @@ class FxaDeviceConstellationTest { } constellation.registerDeviceObserver(observer, startedLifecycleOwner(), false) - assertTrue(constellation.setDeviceNameAsync("another name").await()) + assertTrue(constellation.setDeviceNameAsync("another name", testContext).await()) verify(account).setDeviceDisplayName("another name") // Since we're faking the data, here we're just testing that observer is notified with the diff --git a/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt b/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt index 2a61e66b4e7..bc9683d683b 100644 --- a/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt +++ b/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/AsyncLoginsStorage.kt @@ -386,6 +386,11 @@ data class SyncableLoginsStore( } } + override fun getHandle(): Long { + // TODO + return 123L + } + /** * Run some [block] which operates over an unlocked instance of [AsyncLoginsStorage]. * Database is locked once [block] is done. diff --git a/components/support/sync-telemetry/src/main/java/mozilla/components/support/sync/telemetry/BaseGleanSyncPing.kt b/components/support/sync-telemetry/src/main/java/mozilla/components/support/sync/telemetry/BaseGleanSyncPing.kt index 1e4a60d7611..d426c988dac 100644 --- a/components/support/sync-telemetry/src/main/java/mozilla/components/support/sync/telemetry/BaseGleanSyncPing.kt +++ b/components/support/sync-telemetry/src/main/java/mozilla/components/support/sync/telemetry/BaseGleanSyncPing.kt @@ -36,14 +36,14 @@ internal data class BaseGleanSyncPing( } return BaseGleanSyncPing( uid = uid, - startedAt = Date(info.at.toLong() * MILLIS_PER_SEC), + startedAt = Date(info.at * MILLIS_PER_SEC), // Glean intentionally doesn't support recording arbitrary // durations in timespans, and we can't use the timespan // measurement API because everything is measured in Rust // code. Instead, we record absolute start and end times. // The Sync ping records both `at` _and_ `took`, so this doesn't // leak additional info. - finishedAt = Date(info.at.toLong() * MILLIS_PER_SEC + info.took), + finishedAt = Date(info.at * MILLIS_PER_SEC + info.took), applied = info.incoming?.applied ?: 0, failedToApply = failedToApply, reconciled = info.incoming?.reconciled ?: 0, @@ -54,4 +54,4 @@ internal data class BaseGleanSyncPing( ) } } -} \ No newline at end of file +} diff --git a/samples/sync-logins/src/main/java/org/mozilla/samples/sync/logins/MainActivity.kt b/samples/sync-logins/src/main/java/org/mozilla/samples/sync/logins/MainActivity.kt index 72444271a20..62b28665480 100644 --- a/samples/sync-logins/src/main/java/org/mozilla/samples/sync/logins/MainActivity.kt +++ b/samples/sync-logins/src/main/java/org/mozilla/samples/sync/logins/MainActivity.kt @@ -30,6 +30,7 @@ import mozilla.components.service.fxa.ServerConfig import mozilla.components.service.fxa.SyncConfig import mozilla.components.service.fxa.SyncEngine import mozilla.components.service.fxa.sync.GlobalSyncableStoreProvider +import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.service.fxa.sync.SyncStatusObserver import mozilla.components.service.fxa.toAuthType import mozilla.components.service.sync.logins.AsyncLoginsStorageAdapter @@ -55,7 +56,7 @@ class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener, applicationContext, ServerConfig.release(CLIENT_ID, REDIRECT_URL), DeviceConfig("A-C Logins Sync Sample", DeviceType.MOBILE, setOf()), - SyncConfig(setOf(SyncEngine.PASSWORDS)) + SyncConfig(setOf(SyncEngine.Passwords)) ) } @@ -87,7 +88,7 @@ class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener, ) { CompletableDeferred("my-not-so-secret-password") } - GlobalSyncableStoreProvider.configureStore(SyncEngine.PASSWORDS to loginsStorage) + GlobalSyncableStoreProvider.configureStore(SyncEngine.Passwords to loginsStorage) accountManager.register(accountObserver, owner = this, autoPause = true) @@ -111,7 +112,7 @@ class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener, override fun onLoggedOut() {} override fun onAuthenticated(account: OAuthAccount, authType: AuthType) { - accountManager.syncNowAsync() + accountManager.syncNowAsync(SyncReason.User) } @Suppress("EmptyFunctionBlock") diff --git a/samples/sync/src/main/java/org/mozilla/samples/sync/MainActivity.kt b/samples/sync/src/main/java/org/mozilla/samples/sync/MainActivity.kt index 1ffe4ae420e..f33cca5404b 100644 --- a/samples/sync/src/main/java/org/mozilla/samples/sync/MainActivity.kt +++ b/samples/sync/src/main/java/org/mozilla/samples/sync/MainActivity.kt @@ -40,6 +40,7 @@ import mozilla.components.support.base.log.Log import mozilla.components.lib.fetch.httpurlconnection.HttpURLConnectionClient import mozilla.components.service.fxa.FxaAuthData import mozilla.components.service.fxa.SyncEngine +import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.service.fxa.toAuthType import mozilla.components.support.base.log.sink.AndroidLogSink import mozilla.components.support.rusthttp.RustHttpConfig @@ -61,8 +62,8 @@ class MainActivity : } init { - GlobalSyncableStoreProvider.configureStore(SyncEngine.HISTORY to historyStorage) - GlobalSyncableStoreProvider.configureStore(SyncEngine.BOOKMARKS to bookmarksStorage) + GlobalSyncableStoreProvider.configureStore(SyncEngine.History to historyStorage) + GlobalSyncableStoreProvider.configureStore(SyncEngine.Bookmarks to bookmarksStorage) } private val accountManager by lazy { @@ -74,7 +75,7 @@ class MainActivity : type = DeviceType.MOBILE, capabilities = setOf(DeviceCapability.SEND_TAB) ), - SyncConfig(setOf(SyncEngine.HISTORY, SyncEngine.BOOKMARKS), syncPeriodInMinutes = 15L) + SyncConfig(setOf(SyncEngine.History, SyncEngine.Bookmarks), syncPeriodInMinutes = 15L) ) } @@ -154,7 +155,7 @@ class MainActivity : launch { accountManager.initAsync().await() } findViewById(R.id.buttonSync).setOnClickListener { - accountManager.syncNowAsync() + accountManager.syncNowAsync(SyncReason.User) } }