From 8308693d600dcb02011bd04cc2b01916d6ba368b Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Tue, 25 Jun 2019 14:34:55 -0700 Subject: [PATCH] Move sync scope ownership into account manager; API simplification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I've started pulling on one little thread, and ended up with a few more changes than initially anticipated. Raison d'être for this PR - introducing access token caching for Sync. - Some background on the issue: Rust FirefoxAccount object maintains an in-memory cache of access tokens, keyed by 'scope'. During every sync, we "rehydrate" an instance of FirefoxAccount, starting with a fresh cache. We then obtain an access token from it to sync; this performs a network request (since the internal cache is empty), which is quite costly at scale for our services. This creates a situation when we may overwhelm our own servers with a large enough, actively syncing user base. - This PR adds a caching layer for sync authInfo objects. Sync workers no longer interact with the account directly, and instead look into the cache to obtain authentication info necessary for syncing. No more "talk to the FxA server before every sync". Account manager is responsible for keeping the cache up-to-date, and resetting it when necessary. Cache is currently updated: on startup (but only if access token has expired), on authentication, and when we recover from auth problems. And this is where the "thread pulling" begins! In order to "own" the access token for sync, account manager needs to be aware of the "sync scope". Before, we just relied on the application to specify that scope. Instead, I've changed account manager's constructor to take a SyncConfig object which allows consuming application to configure how sync should behave (enabled at all?, periodic syncing enabled? how often to sync? which stores should be synced?). Ownership of the "sync manager" moved down the stack, from the application layer into the account manager. Application is now expected to interact with sync only via AccountManager's `sync` method, which exposes an internal SyncManager instance (if sync is enabled). Above changes were a good reason to move support classes from feature-sync and into services-firefox-account. Note that since "sync" is part of our "storage" modules, this change doesn't mean that you need to take an extra native dependency on your classpath simply if you need to use FxA. Thanks to concept-sync, actual "Firefox Sync" machinery (within libplaces) is still fully decoupled from FxA. `feature-sync` has been removed entirely. Since we're churning the public API anyway, I took the chance to introduce a few more simplifications at the API layer: - 'SyncManager' interface was removed, since we're not expecting to have multiple implementations of it - 'Config' was renamed to 'ServerConfig' - 'DeviceTuple' was renamed to 'DeviceConfig' - account manager grew a new public API, 'setSyncConfig', which allows application to re-configure how it wants sync to behave - 'AuthInfo' was renamed to 'SyncAuthInfo', and a bunch of cleanup happened in that area - 'AccountObservable'@'onError' method was removed. The only error that could have been passed into it (unable to restore account) wasn't actionable by the application anyway, and none of the integrations did anything with that call Documentation of public APIs and classes was improved. --- .buildconfig.yml | 4 - .../browser/storage/sync/Connection.kt | 5 +- .../storage/sync/PlacesBookmarksStorage.kt | 6 +- .../storage/sync/PlacesHistoryStorage.kt | 8 +- .../components/browser/storage/sync/Types.kt | 6 +- .../sync/PlacesBookmarksStorageTest.kt | 8 +- .../storage/sync/PlacesHistoryStorageTest.kt | 19 +- .../components/concept/sync/Devices.kt | 4 +- .../components/concept/sync/OAuthAccount.kt | 37 -- .../mozilla/components/concept/sync/Sync.kt | 88 ++-- .../FirefoxAccountsAuthFeatureTest.kt | 20 +- components/feature/sync/README.md | 94 ----- components/feature/sync/build.gradle | 42 -- components/feature/sync/proguard-rules.pro | 21 - .../feature/sync/src/main/AndroidManifest.xml | 5 - .../feature/sync/GeneralSyncManagerTest.kt | 277 ------------- .../feature/sync/StorageSyncTest.kt | 134 ------ .../service/firefox-accounts/build.gradle | 1 + .../mozilla/components/service/fxa/Config.kt | 38 +- .../components/service/fxa/FirefoxAccount.kt | 4 +- .../service/fxa/FxaDeviceConstellation.kt | 4 +- .../service/fxa/SyncAuthInfoCache.kt | 83 ++++ .../mozilla/components/service/fxa/Types.kt | 22 + .../service/fxa/manager/FxaAccountManager.kt | 247 ++++++++--- .../service/fxa}/sync/StorageSync.kt | 58 +-- .../service/fxa/sync/SyncManager.kt} | 205 +++++----- .../fxa/sync/WorkManagerSyncManager.kt} | 85 ++-- .../service/fxa/FxaAccountManagerTest.kt | 383 ++++++++++++++---- .../service/fxa/SyncAuthInfoCacheTest.kt | 51 +++ .../service/sync/logins/AsyncLoginsStorage.kt | 4 +- .../components/service/sync/logins/Types.kt | 4 +- .../org/mozilla/samples/fxa/MainActivity.kt | 4 +- samples/sync-logins/build.gradle | 1 - .../samples/sync/logins/MainActivity.kt | 33 +- samples/sync/build.gradle | 1 - .../org/mozilla/samples/sync/MainActivity.kt | 38 +- 36 files changed, 972 insertions(+), 1072 deletions(-) delete mode 100644 components/feature/sync/README.md delete mode 100644 components/feature/sync/build.gradle delete mode 100644 components/feature/sync/proguard-rules.pro delete mode 100644 components/feature/sync/src/main/AndroidManifest.xml delete mode 100644 components/feature/sync/src/test/java/mozilla/components/feature/sync/GeneralSyncManagerTest.kt delete mode 100644 components/feature/sync/src/test/java/mozilla/components/feature/sync/StorageSyncTest.kt create mode 100644 components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SyncAuthInfoCache.kt rename components/{feature/sync/src/main/java/mozilla/components/feature => service/firefox-accounts/src/main/java/mozilla/components/service/fxa}/sync/StorageSync.kt (55%) rename components/{feature/sync/src/main/java/mozilla/components/feature/sync/BackgroundSyncManager.kt => service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/SyncManager.kt} (50%) rename components/{feature/sync/src/main/java/mozilla/components/feature/sync/WorkManagerSyncDispatcher.kt => service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt} (79%) create mode 100644 components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/SyncAuthInfoCacheTest.kt diff --git a/.buildconfig.yml b/.buildconfig.yml index 0bef85dd0dd..5aa56e2c31d 100644 --- a/.buildconfig.yml +++ b/.buildconfig.yml @@ -72,10 +72,6 @@ projects: path: components/feature/session description: 'Feature implementation connecting an engine implementation with the session module.' publish: true - feature-sync: - path: components/feature/sync - description: 'Feature implementation that enables syncing of syncable components.' - publish: true feature-tabs: path: components/feature/tabs description: 'Feature implementation connecting a tabs tray implementation with the session and toolbar modules.' 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 a12caeef9ee..6d95c111d1e 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 @@ -14,6 +14,7 @@ import mozilla.appservices.sync15.SyncTelemetryPing import mozilla.components.browser.storage.sync.GleanMetrics.BookmarksSync import mozilla.components.browser.storage.sync.GleanMetrics.HistorySync import mozilla.components.browser.storage.sync.GleanMetrics.Pings +import mozilla.components.concept.sync.SyncAuthInfo import java.io.Closeable import java.io.File @@ -184,7 +185,7 @@ internal object RustPlacesConnection : Connection { override fun syncHistory(syncInfo: SyncAuthInfo) { check(api != null) { "must call init first" } - val ping = api!!.syncHistory(syncInfo) + val ping = api!!.syncHistory(syncInfo.into()) assembleHistoryPing(ping) } @@ -194,7 +195,7 @@ internal object RustPlacesConnection : Connection { override fun syncBookmarks(syncInfo: SyncAuthInfo) { check(api != null) { "must call init first" } - val ping = api!!.syncBookmarks(syncInfo) + val ping = api!!.syncBookmarks(syncInfo.into()) assembleBookmarksPing(ping) } 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 c52f81b0a50..ab7d82dfa70 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 @@ -16,7 +16,7 @@ import mozilla.components.concept.storage.BookmarkInfo import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.storage.BookmarksStorage -import mozilla.components.concept.sync.AuthInfo +import mozilla.components.concept.sync.SyncAuthInfo import mozilla.components.concept.sync.SyncStatus import mozilla.components.concept.sync.SyncableStore import mozilla.components.support.base.log.logger.Logger @@ -177,10 +177,10 @@ open class PlacesBookmarksStorage(context: Context) : PlacesStorage(context), Bo * @param authInfo The authentication information to sync with. * @return Sync status of OK or Error */ - override suspend fun sync(authInfo: AuthInfo): SyncStatus { + override suspend fun sync(authInfo: SyncAuthInfo): SyncStatus { return withContext(scope.coroutineContext) { syncAndHandleExceptions { - places.syncBookmarks(authInfo.into()) + places.syncBookmarks(authInfo) } } } 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 5e579aebd42..5427fa166ff 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 @@ -13,7 +13,7 @@ import mozilla.components.concept.storage.PageObservation import mozilla.components.concept.storage.SearchResult import mozilla.components.concept.storage.VisitInfo import mozilla.components.concept.storage.VisitType -import mozilla.components.concept.sync.AuthInfo +import mozilla.components.concept.sync.SyncAuthInfo import mozilla.components.concept.sync.SyncStatus import mozilla.components.concept.sync.SyncableStore import mozilla.components.support.base.log.logger.Logger @@ -21,8 +21,6 @@ import mozilla.components.support.utils.segmentAwareDomainMatch const val AUTOCOMPLETE_SOURCE_NAME = "placesHistory" -typealias SyncAuthInfo = mozilla.appservices.places.SyncAuthInfo - /** * Implementation of the [HistoryStorage] which is backed by a Rust Places lib via [PlacesApi]. */ @@ -172,10 +170,10 @@ open class PlacesHistoryStorage(context: Context) : PlacesStorage(context), Hist * @param authInfo The authentication information to sync with. * @return Sync status of OK or Error */ - override suspend fun sync(authInfo: AuthInfo): SyncStatus { + override suspend fun sync(authInfo: SyncAuthInfo): SyncStatus { return withContext(scope.coroutineContext) { syncAndHandleExceptions { - places.syncHistory(authInfo.into()) + places.syncHistory(authInfo) } } } diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Types.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Types.kt index fdd0e43357e..b08d17c93b3 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Types.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/Types.kt @@ -5,20 +5,20 @@ @file:Suppress("MatchingDeclarationName") package mozilla.components.browser.storage.sync +import mozilla.appservices.places.SyncAuthInfo import java.util.Date import mozilla.appservices.sync15.EngineInfo import mozilla.appservices.sync15.FailureReason import mozilla.components.concept.storage.VisitInfo import mozilla.components.concept.storage.VisitType -import mozilla.components.concept.sync.AuthInfo // We have type definitions at the concept level, and "external" types defined within Places. // In practice these two types are largely the same, and this file is the conversion point. /** - * Conversion from a generic AuthInfo type into a type 'places' lib uses at the interface boundary. + * Conversion from our SyncAuthInfo into its "native" version used at the interface boundary. */ -internal fun AuthInfo.into(): SyncAuthInfo { +internal fun mozilla.components.concept.sync.SyncAuthInfo.into(): SyncAuthInfo { return SyncAuthInfo( kid = this.kid, fxaAccessToken = this.fxaAccessToken, diff --git a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt index c8482b66fed..e14f9c4cbad 100644 --- a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt +++ b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt @@ -19,7 +19,7 @@ import mozilla.components.browser.storage.sync.GleanMetrics.Pings import mozilla.components.concept.storage.BookmarkInfo import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType -import mozilla.components.concept.sync.AuthInfo +import mozilla.components.concept.sync.SyncAuthInfo import mozilla.components.concept.sync.SyncStatus import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext @@ -374,7 +374,7 @@ class PlacesBookmarksStorageTest { } val storage = TestablePlacesBookmarksStorage(conn) - val result = storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + val result = storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) Assert.assertTrue(result is SyncStatus.Ok) Assert.assertEquals(conn.pingCount, 1) @@ -542,7 +542,7 @@ class PlacesBookmarksStorageTest { } val storage = TestablePlacesBookmarksStorage(conn) - val result = storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + val result = storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) Assert.assertTrue(result is SyncStatus.Ok) Assert.assertEquals(6, conn.pingCount) @@ -611,7 +611,7 @@ class PlacesBookmarksStorageTest { } val storage = TestablePlacesBookmarksStorage(conn) - val result = storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + val result = storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) Assert.assertTrue(result is SyncStatus.Ok) Assert.assertEquals(1, conn.pingCount) 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 606bdeadd5d..e7afea5d19e 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 @@ -21,7 +21,7 @@ import mozilla.components.browser.storage.sync.GleanMetrics.HistorySync import mozilla.components.browser.storage.sync.GleanMetrics.Pings import mozilla.components.concept.storage.PageObservation import mozilla.components.concept.storage.VisitType -import mozilla.components.concept.sync.AuthInfo +import mozilla.components.concept.sync.SyncAuthInfo import mozilla.components.concept.sync.SyncStatus import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext @@ -518,11 +518,12 @@ class PlacesHistoryStorageTest { } val storage = MockingPlacesHistoryStorage(conn) - storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) assertEquals("kid", passedAuthInfo!!.kid) - assertEquals("serverUrl", passedAuthInfo!!.tokenserverURL) + assertEquals("serverUrl", passedAuthInfo!!.tokenServerUrl) assertEquals("token", passedAuthInfo!!.fxaAccessToken) + assertEquals(123L, passedAuthInfo!!.fxaAccessTokenExpiresAt) assertEquals("key", passedAuthInfo!!.syncKey) } @@ -549,7 +550,7 @@ class PlacesHistoryStorageTest { } val storage = MockingPlacesHistoryStorage(conn) - val result = storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + val result = storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) assertEquals(SyncStatus.Ok, result) } @@ -581,7 +582,7 @@ class PlacesHistoryStorageTest { } val storage = MockingPlacesHistoryStorage(conn) - val result = storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + val result = storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) assertTrue(result is SyncStatus.Error) @@ -724,7 +725,7 @@ class PlacesHistoryStorageTest { } val storage = MockingPlacesHistoryStorage(conn) - val result = storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + val result = storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) assertTrue(result is SyncStatus.Ok) assertEquals(2, conn.pingCount) @@ -901,7 +902,7 @@ class PlacesHistoryStorageTest { } val storage = MockingPlacesHistoryStorage(conn) - val result = storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + val result = storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) assertTrue(result is SyncStatus.Ok) assertEquals(6, conn.pingCount) @@ -970,7 +971,7 @@ class PlacesHistoryStorageTest { } val storage = MockingPlacesHistoryStorage(conn) - val result = storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + val result = storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) assertTrue(result is SyncStatus.Ok) assertEquals(1, conn.pingCount) @@ -1003,7 +1004,7 @@ class PlacesHistoryStorageTest { } } val storage = MockingPlacesHistoryStorage(conn) - storage.sync(AuthInfo("kid", "token", "key", "serverUrl")) + storage.sync(SyncAuthInfo("kid", "token", 123L, "key", "serverUrl")) 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 90d22d4f6bc..73ceaeb295a 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 @@ -23,7 +23,7 @@ interface DeviceConstellation : Observable { fun initDeviceAsync( name: String, type: DeviceType = DeviceType.MOBILE, - capabilities: List + capabilities: Set ): Deferred /** @@ -43,7 +43,7 @@ interface DeviceConstellation : Observable { * not supported. * @return A [Deferred] that will be resolved with a success flag once operation is complete. */ - fun ensureCapabilitiesAsync(capabilities: List): Deferred + fun ensureCapabilitiesAsync(capabilities: Set): Deferred /** * Current state of the constellation. May be missing if state was never queried. diff --git a/components/concept/sync/src/main/java/mozilla/components/concept/sync/OAuthAccount.kt b/components/concept/sync/src/main/java/mozilla/components/concept/sync/OAuthAccount.kt index 39e9b20a0d2..be006338f70 100644 --- a/components/concept/sync/src/main/java/mozilla/components/concept/sync/OAuthAccount.kt +++ b/components/concept/sync/src/main/java/mozilla/components/concept/sync/OAuthAccount.kt @@ -36,27 +36,6 @@ interface OAuthAccount : AutoCloseable { fun registerPersistenceCallback(callback: StatePersistenceCallback) fun deviceConstellation(): DeviceConstellation fun toJSONString(): String - - /** - * Returns an [AuthInfo] instance which may be used for data synchronization. - * - * @return An [AuthInfo] which is guaranteed to have a sync key. - * @throws AuthException if account needs to restart the OAuth flow. - */ - suspend fun authInfo(singleScope: String): AuthInfo { - val tokenServerURL = this.getTokenServerEndpointURL() - val tokenInfo = this.getAccessTokenAsync(singleScope).await() - ?: throw AuthException(AuthExceptionType.NO_TOKEN) - val keyInfo = tokenInfo.key - ?: throw AuthException(AuthExceptionType.KEY_INFO) - - return AuthInfo( - kid = keyInfo.kid, - fxaAccessToken = tokenInfo.token, - syncKey = keyInfo.k, - tokenServerUrl = tokenServerURL - ) - } } /** @@ -69,16 +48,6 @@ interface StatePersistenceCallback { fun persist(data: String) } -/** - * A Firefox Sync friendly auth object which can be obtained from [OAuthAccount]. - */ -data class AuthInfo( - val kid: String, - val fxaAccessToken: String, - val syncKey: String, - val tokenServerUrl: String -) - /** * Observer interface which lets its users monitor account state changes and major events. */ @@ -104,12 +73,6 @@ interface AccountObserver { * Account needs to be re-authenticated (e.g. due to a password change). */ fun onAuthenticationProblems() - - /** - * Account manager encountered an error. Inspect [error] for details. - * @param error A specific error encountered. - */ - fun onError(error: Exception) } data class Avatar( 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 d1855a8e408..397e646be12 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 @@ -4,9 +4,6 @@ package mozilla.components.concept.sync -import mozilla.components.support.base.observer.Observable -import java.lang.Exception - /** * Results of running a sync via [SyncableStore.sync]. */ @@ -22,6 +19,29 @@ sealed class SyncStatus { data class Error(val exception: Throwable) : SyncStatus() } +/** + * A Firefox Sync friendly auth object which can be obtained from [OAuthAccount]. + * + * Why is there a Firefox Sync-shaped authentication object at the concept level, you ask? + * Mainly because this is what the [SyncableStore] consumes in order to actually perform + * synchronization, which is in turn implemented by `places`-backed storage layer. + * If this class lived in `services-firefox-accounts`, we'd end up with an ugly dependency situation + * between services and storage components. + * + * Turns out that building a generic description of an authentication/synchronization layer is not + * quite the way to go when you only have a single, legacy implementation. + * + * However, this may actually improve once we retire the tokenserver from the architecture. + * We could also consider a heavier use of generics, as well. + */ +data class SyncAuthInfo( + val kid: String, + val fxaAccessToken: String, + val fxaAccessTokenExpiresAt: Long, + val syncKey: String, + val tokenServerUrl: String +) + /** * Describes a "sync" entry point for a storage layer. */ @@ -32,67 +52,7 @@ interface SyncableStore { * @param authInfo Auth information necessary for syncing this store. * @return [SyncStatus] A status object describing how sync went. */ - suspend fun sync(authInfo: AuthInfo): SyncStatus -} - -/** - * Describes a "sync" entry point for an application. - */ -interface SyncManager : Observable { - /** - * An authenticated account is now available. - */ - fun authenticated(account: OAuthAccount) - - /** - * Previously authenticated account has been logged-out. - */ - fun loggedOut() - - /** - * Add a store, by [name], to a set of synchronization stores. - * Implementation is expected to be able to either access or instantiate concrete - * [SyncableStore] instances given this name. - */ - fun addStore(name: String) - - /** - * Remove a store previously specified via [addStore] from a set of synchronization stores. - */ - fun removeStore(name: String) - - /** - * Request an immediate synchronization of all configured stores. - * - * @param startup Boolean flag indicating if sync is being requested in a startup situation. - */ - fun syncNow(startup: Boolean = false) - - /** - * Indicates if synchronization is currently active. - */ - fun isSyncRunning(): Boolean -} - -/** - * An interface for consumers that wish to observer "sync lifecycle" events. - */ -interface SyncStatusObserver { - /** - * Gets called at the start of a sync, before any configured syncable is synchronized. - */ - fun onStarted() - - /** - * Gets called at the end of a sync, after every configured syncable has been synchronized. - */ - fun onIdle() - - /** - * Gets called if sync encounters an error that's worthy of processing by status observers. - * @param error Optional relevant exception. - */ - fun onError(error: Exception?) + suspend fun sync(authInfo: SyncAuthInfo): SyncStatus } /** diff --git a/components/feature/accounts/src/test/java/mozilla/components/feature/accounts/FirefoxAccountsAuthFeatureTest.kt b/components/feature/accounts/src/test/java/mozilla/components/feature/accounts/FirefoxAccountsAuthFeatureTest.kt index 2700090feea..0f30e574749 100644 --- a/components/feature/accounts/src/test/java/mozilla/components/feature/accounts/FirefoxAccountsAuthFeatureTest.kt +++ b/components/feature/accounts/src/test/java/mozilla/components/feature/accounts/FirefoxAccountsAuthFeatureTest.kt @@ -11,8 +11,7 @@ import mozilla.components.concept.sync.DeviceType import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.Profile import mozilla.components.feature.tabs.TabsUseCases -import mozilla.components.service.fxa.Config -import mozilla.components.service.fxa.manager.DeviceTuple +import mozilla.components.service.fxa.ServerConfig import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.support.test.any import mozilla.components.support.test.mock @@ -25,6 +24,7 @@ import org.mockito.Mockito.`when` import org.mockito.Mockito.times import org.mockito.Mockito.verify import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.service.fxa.DeviceConfig // Same as the actual account manager, except we get to control how FirefoxAccountShaped instances // are created. This is necessary because due to some build issues (native dependencies not available @@ -32,12 +32,12 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 // Instead, we express all of our account-related operations over an interface. class TestableFxaAccountManager( context: Context, - config: Config, - scopes: Array, + config: ServerConfig, + scopes: Set, val block: () -> OAuthAccount = { mock() } -) : FxaAccountManager(context, config, scopes, DeviceTuple("test", DeviceType.MOBILE, listOf()), null) { +) : FxaAccountManager(context, config, DeviceConfig("test", DeviceType.MOBILE, setOf()), null, scopes) { - override fun createAccount(config: Config): OAuthAccount { + override fun createAccount(config: ServerConfig): OAuthAccount { return block() } } @@ -118,8 +118,8 @@ class FirefoxAccountsAuthFeatureTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope") + ServerConfig.release("dummyId", "bad://url"), + setOf("test-scope") ) { mockAccount } @@ -143,8 +143,8 @@ class FirefoxAccountsAuthFeatureTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope") + ServerConfig.release("dummyId", "bad://url"), + setOf("test-scope") ) { mockAccount } diff --git a/components/feature/sync/README.md b/components/feature/sync/README.md deleted file mode 100644 index d468f12ea94..00000000000 --- a/components/feature/sync/README.md +++ /dev/null @@ -1,94 +0,0 @@ -# [Android Components](../../../README.md) > Feature > Sync - -A component which provides facilities for managing data synchronization. - -Provided is an implementation of concept-sync's SyncManager, allowing -background execution of synchronization and easily observing synchronization -state. - -Currently, synchronization runs periodically in the background. - -## Usage - -First, set everything up: -```kotlin -// A SyncableStore instance that we want to be able to synchronize. -private val historyStorage = PlacesHistoryStorage(this) - -// Add your SyncableStore instances to the global registry. -GlobalSyncableStoreProvider.configureStore("history" to historyStorage) - -// Configure SyncManager with sync scope and stores to synchronize. -private val syncManager = BackgroundSyncManager("https://identity.mozilla.com/apps/oldsync").also { - // Enable synchronization of store called "history". - // Internally, it will be accessed via GlobalSyncableStoreProvider. - it.addStore("history") -} - -// Configure the account manager with an instance of a SyncManager. -// This will ensure that sync manager is made aware of auth changes. -private val accountManager by lazy { - FxaAccountManager( - this, - Config.release(CLIENT_ID, REDIRECT_URL), - arrayOf("profile", "https://identity.mozilla.com/apps/oldsync"), - syncManager - ) -} -``` - -Once account manager is in an authenticated state, sync manager will immediately -synchronize, as well as schedule periodic syncing. - -It's possible to re-configure which stores are to be synchronized via `addStore` and -`removeStore` calls. - -It's also possible to observe sync states and request immediate sync of configured stores: -```kotlin -// Define what we want to do in response to sync status changes. -private val syncObserver = object : SyncStatusObserver { - override fun onStarted() { - CoroutineScope(Dispatchers.Main).launch { - syncStatus?.text = getString(R.string.syncing) - } - } - - override fun onIdle() { - CoroutineScope(Dispatchers.Main).launch { - syncStatus?.text = getString(R.string.sync_idle) - - val resultTextView: TextView = findViewById(R.id.syncResult) - // Can access synchronized data now. - } - } - - override fun onError(error: Exception?) { - CoroutineScope(Dispatchers.Main).launch { - syncStatus?.text = getString(R.string.sync_error, error) - } - } -} - -// Register a sync status observer with this sync manager. -syncManager.register(syncObserver, owner = this, autoPause = true) - -// Kick off a sync in response to some user action. Sync will run in the -// background until it completes, surviving application shutdown if needed. -findViewById(R.id.buttonSyncNow).setOnClickListener { - syncManager.syncNow() -} -``` - -### Setting up the dependency - -Use Gradle to download the library from [maven.mozilla.org](https://maven.mozilla.org/) ([Setup repository](../../../README.md#maven-repository)): - -```Groovy -implementation "org.mozilla.components:feature-sync:{latest-version}" -``` - -## License - - This Source Code Form is subject to the terms of the Mozilla Public - License, v. 2.0. If a copy of the MPL was not distributed with this - file, You can obtain one at http://mozilla.org/MPL/2.0/ diff --git a/components/feature/sync/build.gradle b/components/feature/sync/build.gradle deleted file mode 100644 index 465cd1ec6dd..00000000000 --- a/components/feature/sync/build.gradle +++ /dev/null @@ -1,42 +0,0 @@ -/* 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/. */ - -apply plugin: 'com.android.library' -apply plugin: 'kotlin-android' - -android { - compileSdkVersion config.compileSdkVersion - - defaultConfig { - minSdkVersion config.minSdkVersion - targetSdkVersion config.targetSdkVersion - } - - buildTypes { - release { - minifyEnabled false - proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro' - } - } -} - -dependencies { - implementation project(':concept-sync') - implementation project(':service-firefox-accounts') - - // ObserverRegistry is part of this module's public API, so 'support-base' becomes necessary for - // its consumers. Use 'api' here instead of 'implementation' to make that easier. - api project(':support-base') - - implementation Dependencies.androidx_work_runtime - - testImplementation Dependencies.testing_junit - testImplementation Dependencies.testing_robolectric - testImplementation Dependencies.testing_mockito - - testImplementation project(':support-test') -} - -apply from: '../../../publish.gradle' -ext.configurePublish(config.componentsGroupId, archivesBaseName, project.ext.description) diff --git a/components/feature/sync/proguard-rules.pro b/components/feature/sync/proguard-rules.pro deleted file mode 100644 index f1b424510da..00000000000 --- a/components/feature/sync/proguard-rules.pro +++ /dev/null @@ -1,21 +0,0 @@ -# Add project specific ProGuard rules here. -# You can control the set of applied configuration files using the -# proguardFiles setting in build.gradle. -# -# For more details, see -# http://developer.android.com/guide/developing/tools/proguard.html - -# If your project uses WebView with JS, uncomment the following -# and specify the fully qualified class name to the JavaScript interface -# class: -#-keepclassmembers class fqcn.of.javascript.interface.for.webview { -# public *; -#} - -# Uncomment this to preserve the line number information for -# debugging stack traces. -#-keepattributes SourceFile,LineNumberTable - -# If you keep the line number information, uncomment this to -# hide the original source file name. -#-renamesourcefileattribute SourceFile diff --git a/components/feature/sync/src/main/AndroidManifest.xml b/components/feature/sync/src/main/AndroidManifest.xml deleted file mode 100644 index 87955243fa7..00000000000 --- a/components/feature/sync/src/main/AndroidManifest.xml +++ /dev/null @@ -1,5 +0,0 @@ - - diff --git a/components/feature/sync/src/test/java/mozilla/components/feature/sync/GeneralSyncManagerTest.kt b/components/feature/sync/src/test/java/mozilla/components/feature/sync/GeneralSyncManagerTest.kt deleted file mode 100644 index 616b58754f7..00000000000 --- a/components/feature/sync/src/test/java/mozilla/components/feature/sync/GeneralSyncManagerTest.kt +++ /dev/null @@ -1,277 +0,0 @@ -/* 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.feature.sync - -import mozilla.components.concept.sync.OAuthAccount -import mozilla.components.concept.sync.SyncStatusObserver -import mozilla.components.support.base.observer.Observable -import mozilla.components.support.base.observer.ObserverRegistry -import mozilla.components.support.test.any -import mozilla.components.support.test.mock -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Assert.fail -import org.junit.Test -import org.mockito.Mockito.`when` -import org.mockito.Mockito.never -import org.mockito.Mockito.times -import org.mockito.Mockito.verify -import java.util.concurrent.TimeUnit - -class GeneralSyncManagerTest { - class TestSyncManager( - private val dispatcher: SyncDispatcher, - val dispatcherValidator: (stores: Set, account: OAuthAccount) -> Unit - ) : GeneralSyncManager() { - override fun createDispatcher(stores: Set, account: OAuthAccount): SyncDispatcher { - dispatcherValidator(stores, account) - return dispatcher - } - - override fun dispatcherUpdated(dispatcher: SyncDispatcher) { - } - } - - @Test - fun `modifying stores should not reset dispatcher if there's no account`() { - val manager = TestSyncManager(mock()) { _, _ -> - fail() - } - - manager.addStore("history") - manager.addStore("logins") - manager.removeStore("logins") - } - - @Test - fun `dispatcher resets should happen once account is set`() { - val dispatcher = mock() - var storez: Set? = null - var akaunt: OAuthAccount? = null - - val manager = TestSyncManager(dispatcher) { stores, account -> - storez = stores - akaunt = account - } - - val mockAccount: OAuthAccount = mock() - - verify(dispatcher, never()).syncNow() -// verify(dispatcher, never()).startPeriodicSync(any(), any()) - - manager.addStore("history") - manager.authenticated(mockAccount) - - verify(dispatcher).syncNow() -// verify(dispatcher).startPeriodicSync(any(), any()) - - assertEquals(1, storez!!.size) - assertTrue(storez!!.contains("history")) - assertEquals(mockAccount, akaunt) - - // Adding the same store multiple times is a no-op. - manager.addStore("history") - assertEquals(1, storez!!.size) - assertTrue(storez!!.contains("history")) - assertEquals(mockAccount, akaunt) - - verify(dispatcher, times(2)).syncNow() -// verify(dispatcher, times(2)).startPeriodicSync(any(), any()) - - manager.addStore("logins") - assertEquals(2, storez!!.size) - assertTrue(storez!!.contains("history")) - assertTrue(storez!!.contains("logins")) - assertEquals(mockAccount, akaunt) - - verify(dispatcher, times(3)).syncNow() -// verify(dispatcher, times(3)).startPeriodicSync(any(), any()) - - manager.removeStore("history") - assertEquals(1, storez!!.size) - assertTrue(storez!!.contains("logins")) - assertEquals(mockAccount, akaunt) - - verify(dispatcher, times(4)).syncNow() -// verify(dispatcher, times(4)).startPeriodicSync(any(), any()) - } - - @Test - fun `logging out should reset an account and clean up the dispatcher`() { - val dispatcher: SyncDispatcher = mock() - val account: OAuthAccount = mock() - val manager = TestSyncManager(dispatcher) { _, _ -> } - - manager.authenticated(account) - verify(dispatcher, never()).stopPeriodicSync() - verify(dispatcher, never()).close() - - manager.loggedOut() - - verify(dispatcher).stopPeriodicSync() - verify(dispatcher).close() - } - - @Test - fun `logging in after logging out should be possible with the same account`() { - val mockAccount: OAuthAccount = mock() - val dispatcher: SyncDispatcher = mock() - - var akaunt: OAuthAccount? = null - var storez: Set? = null - - val manager = TestSyncManager(dispatcher) { stores, account -> - storez = stores - akaunt = account - } - - manager.authenticated(mockAccount) - verify(dispatcher).syncNow() - assertEquals(mockAccount, akaunt) - assertTrue(storez!!.isEmpty()) - - manager.loggedOut() - - manager.authenticated(mockAccount) - - verify(dispatcher, times(2)).syncNow() - assertEquals(mockAccount, akaunt) - assertTrue(storez!!.isEmpty()) - } - - @Test - fun `logging in after logging out should be possible with different accounts`() { - val mockAccount1: OAuthAccount = mock() - val dispatcher: SyncDispatcher = mock() - - var akaunt: OAuthAccount? = null - var storez: Set? = null - - val manager = TestSyncManager(dispatcher) { stores, account -> - storez = stores - akaunt = account - } - - manager.authenticated(mockAccount1) - verify(dispatcher).syncNow() - assertEquals(mockAccount1, akaunt) - assertTrue(storez!!.isEmpty()) - - manager.loggedOut() - - val mockAccount2: OAuthAccount = mock() - - manager.authenticated(mockAccount2) - - verify(dispatcher, times(2)).syncNow() - assertEquals(mockAccount2, akaunt) - assertTrue(storez!!.isEmpty()) - } - - @Test - fun `manager should correctly relay current sync status`() { - val dispatcher: SyncDispatcher = mock() - val manager = TestSyncManager(dispatcher) { _, _ -> } - - `when`(dispatcher.isSyncActive()).thenReturn(false) - - assertFalse(manager.isSyncRunning()) - - manager.authenticated(mock()) - assertFalse(manager.isSyncRunning()) - - `when`(dispatcher.isSyncActive()).thenReturn(true) - assertTrue(manager.isSyncRunning()) - } - - @Test - fun `manager should notify its observers of sync state`() { - val dispatcher: SyncDispatcher = object : SyncDispatcher, Observable by ObserverRegistry() { - override fun isSyncActive(): Boolean { return false } - override fun syncNow(startup: Boolean) {} - override fun startPeriodicSync(unit: TimeUnit, period: Long) {} - override fun stopPeriodicSync() {} - override fun workersStateChanged(isRunning: Boolean) {} - override fun close() {} - } - - val manager = TestSyncManager(dispatcher) { _, _ -> } - - manager.authenticated(mock()) - - val statusObserver: SyncStatusObserver = mock() - manager.register(statusObserver) - - verify(statusObserver, never()).onError(any()) - verify(statusObserver, never()).onIdle() - verify(statusObserver, never()).onStarted() - - dispatcher.notifyObservers { onStarted() } - - verify(statusObserver).onStarted() - - dispatcher.notifyObservers { onIdle() } - - verify(statusObserver).onIdle() - - dispatcher.notifyObservers { onError(null) } - - verify(statusObserver).onError(null) - - val exception = IllegalStateException() - - dispatcher.notifyObservers { onError(exception) } - - verify(statusObserver).onError(exception) - } - - @Test - fun `calling syncNow on startup before authenticating is a mistake`() { - val dispatcher: SyncDispatcher = mock() - val manager = TestSyncManager(dispatcher) { _, _ -> } - manager.syncNow(true) - verify(dispatcher, never()).syncNow(true) - } - - @Test - fun `calling syncNow not on startup before authenticating is a mistake`() { - val dispatcher: SyncDispatcher = mock() - val manager = TestSyncManager(dispatcher) { _, _ -> } - manager.syncNow(true) - verify(dispatcher, never()).syncNow(true) - } - - @Test - fun `manager should relay syncNow calls`() { - val dispatcher: SyncDispatcher = mock() - val manager = TestSyncManager(dispatcher) { _, _ -> } - - manager.authenticated(mock()) - - verify(dispatcher, never()).syncNow(true) - // Authentication should cause a syncNow call. - verify(dispatcher).syncNow(false) - - manager.syncNow(true) - verify(dispatcher).syncNow(true) - - manager.syncNow(false) - verify(dispatcher, times(2)).syncNow(false) - } - - @Test - fun `manager should relay syncNow calls by default as not on startup`() { - val dispatcher: SyncDispatcher = mock() - val manager = TestSyncManager(dispatcher) { _, _ -> } - - manager.authenticated(mock()) - - verify(dispatcher).syncNow(false) - - manager.syncNow() - verify(dispatcher, times(2)).syncNow(false) - } -} \ No newline at end of file diff --git a/components/feature/sync/src/test/java/mozilla/components/feature/sync/StorageSyncTest.kt b/components/feature/sync/src/test/java/mozilla/components/feature/sync/StorageSyncTest.kt deleted file mode 100644 index 69ab54036e5..00000000000 --- a/components/feature/sync/src/test/java/mozilla/components/feature/sync/StorageSyncTest.kt +++ /dev/null @@ -1,134 +0,0 @@ -/* 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.feature.sync - -import kotlinx.coroutines.CompletableDeferred -import kotlinx.coroutines.runBlocking -import mozilla.components.concept.sync.AccessTokenInfo -import mozilla.components.concept.sync.AuthInfo -import mozilla.components.concept.sync.OAuthAccount -import mozilla.components.concept.sync.OAuthScopedKey -import mozilla.components.concept.sync.SyncStatus -import mozilla.components.concept.sync.SyncStatusObserver -import mozilla.components.concept.sync.SyncableStore -import mozilla.components.support.test.any -import mozilla.components.support.test.mock -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Test -import org.junit.Assert.assertTrue -import org.mockito.Mockito.`when` -import org.mockito.Mockito.never -import org.mockito.Mockito.times -import org.mockito.Mockito.verify -import java.lang.Exception - -class StorageSyncTest { - private val testAuth = AuthInfo("kid", "token", "key", "server") - - @Test - fun `sync with no stores`() = runBlocking { - val feature = StorageSync(mapOf(), "sync-scope") - val results = feature.sync(mock()) - assertTrue(results.isEmpty()) - } - - @Test - fun `sync with configured stores`() = runBlocking { - val mockAccount: OAuthAccount = mock() - `when`(mockAccount.getTokenServerEndpointURL()).thenReturn("dummyUrl") - `when`(mockAccount.authInfo("sync-scope")).thenReturn(testAuth) - - val mockAccessTokenInfo = AccessTokenInfo( - scope = "scope", key = OAuthScopedKey("kty", "scope", "kid", "k"), token = "token", expiresAt = 0 - ) - `when`(mockAccount.getAccessTokenAsync(any())).thenReturn(CompletableDeferred((mockAccessTokenInfo))) - - // Single store, different result types. - val testStore: SyncableStore = mock() - val feature = StorageSync(mapOf("testStore" to testStore), "sync-scope") - - `when`(testStore.sync(any())).thenReturn(SyncStatus.Ok) - var results = feature.sync(mockAccount) - assertTrue(results.getValue("testStore").status is SyncStatus.Ok) - assertEquals(1, results.size) - - `when`(testStore.sync(any())).thenReturn(SyncStatus.Error(Exception("test"))) - results = feature.sync(mockAccount) - var error = results.getValue("testStore").status - assertTrue(error is SyncStatus.Error) - assertEquals("test", (error as SyncStatus.Error).exception.message) - assertEquals(1, results.size) - - // Multiple stores, different result types. - val anotherStore: SyncableStore = mock() - val anotherFeature = StorageSync(mapOf( - Pair("testStore", testStore), - Pair("goodStore", anotherStore)), - "sync-scope" - ) - - `when`(anotherStore.sync(any())).thenReturn(SyncStatus.Ok) - - results = anotherFeature.sync(mockAccount) - assertEquals(2, results.size) - error = results.getValue("testStore").status - assertTrue(error is SyncStatus.Error) - assertEquals("test", (error as SyncStatus.Error).exception.message) - assertTrue(results.getValue("goodStore").status is SyncStatus.Ok) - } - - @Test - fun `sync status can be observed`() = runBlocking { - val mockAccount: OAuthAccount = mock() - `when`(mockAccount.getTokenServerEndpointURL()).thenReturn("dummyUrl") - - val mockAccessTokenInfo = AccessTokenInfo( - scope = "scope", key = OAuthScopedKey("kty", "scope", "kid", "k"), token = "token", expiresAt = 0 - ) - `when`(mockAccount.getAccessTokenAsync(any())).thenReturn(CompletableDeferred((mockAccessTokenInfo))) - - val verifier = object { - private val blocks = mutableListOf<() -> Unit>() - - fun addVerifyBlock(block: () -> Unit) { - blocks.add(block) - } - - fun verify() { - blocks.forEach { it() } - } - } - - // A store that runs verifications during a sync. - val testStore = object : SyncableStore { - override suspend fun sync(authInfo: AuthInfo): SyncStatus { - verifier.verify() - return SyncStatus.Ok - } - } - val syncStatusObserver: SyncStatusObserver = mock() - - val feature = StorageSync(mapOf("testStore" to testStore), "sync-scope") - - // These assertions will run while sync is in progress. - verifier.addVerifyBlock { - assertTrue(feature.syncMutex.isLocked) - verify(syncStatusObserver, times(1)).onStarted() - verify(syncStatusObserver, never()).onIdle() - } - - feature.register(syncStatusObserver) - verify(syncStatusObserver, never()).onStarted() - verify(syncStatusObserver, never()).onIdle() - assertFalse(feature.syncMutex.isLocked) - - feature.sync(mockAccount) - - verify(syncStatusObserver, times(1)).onStarted() - verify(syncStatusObserver, times(1)).onIdle() - assertFalse(feature.syncMutex.isLocked) - } -} diff --git a/components/service/firefox-accounts/build.gradle b/components/service/firefox-accounts/build.gradle index cc258d04053..34c15d30051 100644 --- a/components/service/firefox-accounts/build.gradle +++ b/components/service/firefox-accounts/build.gradle @@ -45,6 +45,7 @@ dependencies { testImplementation project(':support-test') testImplementation Dependencies.androidx_test_core testImplementation Dependencies.androidx_test_junit + testImplementation Dependencies.androidx_work_testing testImplementation Dependencies.testing_robolectric testImplementation Dependencies.testing_mockito testImplementation Dependencies.kotlin_coroutines_test 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 4bc72c1f773..da60efff4f5 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 @@ -4,4 +4,40 @@ package mozilla.components.service.fxa -typealias Config = mozilla.appservices.fxaclient.Config +import mozilla.components.concept.sync.DeviceCapability +import mozilla.components.concept.sync.DeviceType +import mozilla.components.service.fxa.manager.FxaAccountManager +import mozilla.components.service.fxa.sync.GlobalSyncableStoreProvider + +typealias ServerConfig = mozilla.appservices.fxaclient.Config + +/** + * Configuration for the current device. + * + * @property name An initial name to use for the device record which will be created during authentication. + * This can be changed later via [FxaDeviceConstellation]. + * + * @property type Type of a device - mobile, desktop - used for displaying identifying icons on other devices. + * This cannot be changed once device record is created. + * + * @property capabilities A set of device capabilities, such as SEND_TAB. This set can be expanded by + * re-initializing [FxaAccountManager] with a new set (e.g. on app restart). + * Shrinking a set of capabilities is currently not supported. + */ +data class DeviceConfig( + val name: String, + val type: DeviceType, + val capabilities: Set +) + +/** + * Configuration for sync. + * + * @property syncableStores A set of store names to sync, exposed via [GlobalSyncableStoreProvider]. + * @property syncPeriodInMinutes Optional, how frequently periodic sync should happen. If this is `null`, + * periodic syncing will be disabled. + */ +data class SyncConfig( + val syncableStores: Set, + val syncPeriodInMinutes: Long? = null +) diff --git a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt index 666fb01baf5..be966c668e9 100644 --- a/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt @@ -17,7 +17,6 @@ import mozilla.components.concept.sync.DeviceConstellation import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.Profile import mozilla.components.concept.sync.StatePersistenceCallback -import mozilla.components.service.fxa.manager.authErrorRegistry import mozilla.components.support.base.log.logger.Logger typealias PersistCallback = mozilla.appservices.fxaclient.FirefoxAccount.PersistCallback @@ -86,7 +85,7 @@ class FirefoxAccount internal constructor( * doing so will not cause an error). */ constructor( - config: Config, + config: ServerConfig, persistCallback: PersistCallback? = null ) : this(InternalFxAcct(config, persistCallback)) @@ -178,7 +177,6 @@ class FirefoxAccount internal constructor( * Tries to fetch an access token for the given scope. * * @param singleScope Single OAuth scope (no spaces) for which the client wants access - * @param notifyAuthErrorRegistry Whether or not to notify [authErrorRegistry] of failures * @return [AccessTokenInfo] that stores the token, along with its scope, key and * expiration timestamp (in seconds) since epoch when complete */ 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 4beb558cf56..a0643136c39 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 @@ -58,7 +58,7 @@ class FxaDeviceConstellation( override fun initDeviceAsync( name: String, type: DeviceType, - capabilities: List + capabilities: Set ): Deferred { return scope.async { handleFxaExceptions(logger, "initializing device") { @@ -86,7 +86,7 @@ class FxaDeviceConstellation( return CompletableDeferred(false) } - override fun ensureCapabilitiesAsync(capabilities: List): Deferred { + override fun ensureCapabilitiesAsync(capabilities: Set): Deferred { return scope.async { handleFxaExceptions(logger, "ensuring capabilities") { account.ensureCapabilities(capabilities.map { it.into() }.toSet()) 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 new file mode 100644 index 00000000000..1ac01804ab6 --- /dev/null +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/SyncAuthInfoCache.kt @@ -0,0 +1,83 @@ +/* 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.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" +private const val CACHE_KEY = CACHE_NAME +private const val KEY_FXA_ACCESS_TOKEN = "fxaAccessToken" +private const val KEY_FXA_ACCESS_TOKEN_EXPIRES_AT = "fxaAccessTokenExpiresAt" +private const val KEY_KID = "kid" +private const val KEY_SYNC_KEY = "syncKey" +private const val KEY_TOKEN_SERVER_URL = "tokenServerUrl" + +/** + * A thin wrapper around [SharedPreferences] which knows how to serialize/deserialize [SyncAuthInfo]. + */ +class SyncAuthInfoCache(val context: Context) { + private val logger = Logger("SyncAuthInfoCache") + + fun setToCache(authInfo: SyncAuthInfo) { + cache().edit().putString(CACHE_KEY, authInfo.toCacheString()).apply() + } + + fun getCached(): SyncAuthInfo? { + val s = cache().getString(CACHE_KEY, null) ?: return null + return fromCacheString(s) + } + + 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 + } + } +} 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 443c7eaee6e..b438428598c 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 @@ -10,10 +10,13 @@ import mozilla.appservices.fxaclient.Device import mozilla.appservices.fxaclient.Profile import mozilla.appservices.fxaclient.ScopedKey import mozilla.appservices.fxaclient.TabHistoryEntry +import mozilla.components.concept.sync.AuthException +import mozilla.components.concept.sync.AuthExceptionType import mozilla.components.concept.sync.Avatar import mozilla.components.concept.sync.DeviceCapability import mozilla.components.concept.sync.DeviceType import mozilla.components.concept.sync.OAuthScopedKey +import mozilla.components.concept.sync.SyncAuthInfo // 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 @@ -29,6 +32,25 @@ fun AccessTokenInfo.into(): mozilla.components.concept.sync.AccessTokenInfo { ) } +/** + * Converts a generic [AccessTokenInfo] into a Firefox Sync-friendly [SyncAuthInfo] instance which + * may be used for data synchronization. + * + * @return An [SyncAuthInfo] which is guaranteed to have a sync key. + * @throws AuthException if [AccessTokenInfo] didn't have key information. + */ +fun mozilla.components.concept.sync.AccessTokenInfo.asSyncAuthInfo(tokenServerUrl: String): SyncAuthInfo { + val keyInfo = this.key ?: throw AuthException(AuthExceptionType.KEY_INFO) + + return SyncAuthInfo( + kid = keyInfo.kid, + fxaAccessToken = this.token, + fxaAccessTokenExpiresAt = this.expiresAt, + syncKey = keyInfo.k, + tokenServerUrl = tokenServerUrl + ) +} + fun ScopedKey.into(): OAuthScopedKey { return OAuthScopedKey(kid = this.kid, k = this.k, kty = this.kty, scope = this.scope) } 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 23dd0929ffd..8d6436ee253 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 @@ -5,6 +5,7 @@ package mozilla.components.service.fxa.manager import android.content.Context +import androidx.annotation.GuardedBy import androidx.annotation.VisibleForTesting import androidx.lifecycle.LifecycleOwner import kotlinx.coroutines.CompletableDeferred @@ -13,41 +14,39 @@ import kotlinx.coroutines.Deferred import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.async +import kotlinx.coroutines.launch import kotlinx.coroutines.cancel import mozilla.components.concept.sync.AccountObserver import mozilla.components.concept.sync.AuthException -import mozilla.components.concept.sync.DeviceCapability import mozilla.components.concept.sync.DeviceEvent import mozilla.components.concept.sync.DeviceEventsObserver -import mozilla.components.concept.sync.DeviceType import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.Profile import mozilla.components.concept.sync.StatePersistenceCallback -import mozilla.components.concept.sync.SyncManager import mozilla.components.service.fxa.AccountStorage -import mozilla.components.service.fxa.Config +import mozilla.components.service.fxa.DeviceConfig import mozilla.components.service.fxa.FirefoxAccount import mozilla.components.service.fxa.FxaException import mozilla.components.service.fxa.FxaPanicException +import mozilla.components.service.fxa.ServerConfig import mozilla.components.service.fxa.SharedPrefAccountStorage +import mozilla.components.service.fxa.SyncAuthInfoCache +import mozilla.components.service.fxa.SyncConfig +import mozilla.components.service.fxa.asSyncAuthInfo +import mozilla.components.service.fxa.sync.SyncManager +import mozilla.components.service.fxa.sync.SyncStatusObserver +import mozilla.components.service.fxa.sync.WorkManagerSyncManager import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.base.observer.Observable import mozilla.components.support.base.observer.ObserverRegistry import java.io.Closeable +import java.lang.Exception +import java.lang.IllegalArgumentException import java.lang.ref.WeakReference import java.util.concurrent.ConcurrentLinkedQueue import java.util.concurrent.Executors import kotlin.coroutines.CoroutineContext -/** - * Propagated via [AccountObserver.onError] if we fail to load a locally stored account during - * initialization. No action is necessary from consumers. - * Account state has been re-initialized. - * - * @param cause Optional original cause of failure. - */ -class FailedToLoadAccountException(cause: Exception?) : Exception(cause) - /** * A global registry for propagating [AuthException] errors. Components such as [SyncManager] and * [FxaDeviceRefreshManager] may encounter authentication problems during their normal operation, and @@ -78,12 +77,8 @@ private interface OAuthObserver { fun onError() } -const val PROFILE_SCOPE = "profile" - -/** - * Helper data class that wraps common device initialization parameters. - */ -data class DeviceTuple(val name: String, val type: DeviceType, val capabilities: List) +const val SCOPE_PROFILE = "profile" +const val SCOPE_SYNC = "https://identity.mozilla.com/apps/oldsync" /** * An account manager which encapsulates various internal details of an account lifecycle and provides @@ -94,27 +89,25 @@ data class DeviceTuple(val name: String, val type: DeviceType, val capabilities: * Class is 'open' to facilitate testing. * * @param context A [Context] instance that's used for internal messaging and interacting with local storage. - * @param config A [Config] used for account initialization. - * @param applicationScopes A list of scopes which will be requested during account authentication. - * @param deviceTuple A description of the current device (name, type, capabilities). + * @param serverConfig A [ServerConfig] used for account initialization. + * @param deviceConfig A description of the current device (name, type, capabilities). + * @param syncConfig Optional, initial sync behaviour configuration. Sync will be disabled if this is `null`. + * @param applicationScopes A set of scopes which will be requested during account authentication. */ -@Suppress("TooManyFunctions") +@Suppress("TooManyFunctions", "LargeClass") open class FxaAccountManager( private val context: Context, - private val config: Config, - private val applicationScopes: Array, - private val deviceTuple: DeviceTuple, - syncManager: SyncManager? = null, + private val serverConfig: ServerConfig, + private val deviceConfig: DeviceConfig, + @Volatile private var syncConfig: SyncConfig?, + private val applicationScopes: Set = emptySet(), // We want a single-threaded execution model for our account-related "actions" (state machine side-effects). // That is, we want to ensure a sequential execution flow, but on a background thread. private val coroutineContext: CoroutineContext = Executors - .newSingleThreadExecutor().asCoroutineDispatcher() + SupervisorJob() + .newSingleThreadExecutor().asCoroutineDispatcher() + SupervisorJob() ) : Closeable, Observable by ObserverRegistry() { private val logger = Logger("FirefoxAccountStateMachine") - // We always obtain a "profile" scope, in addition to whatever scopes application requested. - private val scopes: Set = setOf(PROFILE_SCOPE).plus(applicationScopes) - // This is used during 'beginAuthenticationAsync' call, which returns a Deferred. // 'deferredAuthUrl' is set on this observer and returned, and resolved once state machine goes // through its motions. This allows us to keep around only one observer in the registry. @@ -144,7 +137,6 @@ open class FxaAccountManager( private val fxaAuthErrorObserver = FxaAuthErrorObserver(this) init { - syncManager?.let { this.register(SyncManagerIntegration(it)) } authErrorRegistry.register(fxaAuthErrorObserver) } @@ -220,6 +212,101 @@ open class FxaAccountManager( private val deviceEventObserverRegistry = ObserverRegistry() private val deviceEventsIntegration = DeviceEventsIntegration(deviceEventObserverRegistry) + private val syncStatusObserverRegistry = ObserverRegistry() + + // We always obtain a "profile" scope, as that's assumed to be needed for any application integration. + // We obtain a sync scope only if this was requested by the application via SyncConfig. + // Additionally, we obtain any scopes that application requested explicitly. + private val scopes: Set + get() = if (syncConfig != null) { + setOf(SCOPE_PROFILE, SCOPE_SYNC) + } else { + setOf(SCOPE_PROFILE) + }.plus(applicationScopes) + + // Internal backing field for the syncManager. This will be `null` if passed in SyncConfig (either + // via the constructor, or via [setSyncConfig]) is also `null` - that is, sync will be disabled. + // Note that trying to perform a sync while account isn't authenticated will not succeed. + @GuardedBy("this") + private var syncManager: SyncManager? = null + private var syncManagerIntegration: AccountsToSyncIntegration? = null + private val internalSyncStatusObserver = SyncToAccountsIntegration(this) + + init { + syncConfig?.let { setSyncConfigAsync(it) } + + if (syncManager == null) { + logger.info("Sync is disabled") + } else { + logger.info("Sync is enabled") + } + } + + /** + * Allows setting a new [SyncConfig], changing sync behaviour. + * + * @param config Sync behaviour configuration, see [SyncConfig]. + */ + @SuppressWarnings("LongMethod") + fun setSyncConfigAsync(config: SyncConfig): Deferred = synchronized(this) { + logger.info("Enabling/updating sync with a new SyncConfig: $config") + + syncConfig = config + // Let the existing manager, if it's present, clean up (cancel periodic syncing, etc). + syncManager?.stop() + syncManagerIntegration = null + + val result = CompletableDeferred() + + // Initialize a new sync manager with the passed-in config. + if (config.syncableStores.isEmpty()) { + throw IllegalArgumentException("Set of stores can't be empty") + } + + syncManager = createSyncManager(config).also { manager -> + // Observe account state changes. + syncManagerIntegration = AccountsToSyncIntegration(manager).also { accountToSync -> + this@FxaAccountManager.register(accountToSync) + } + // Observe sync status changes. + manager.registerSyncStatusObserver(internalSyncStatusObserver) + // If account is currently authenticated, 'enable' the sync manager. + if (authenticatedAccount() != null) { + CoroutineScope(coroutineContext).launch { + // Make sure auth-info cache is populated before starting sync manager. + maybeUpdateSyncAuthInfoCache() + manager.start() + result.complete(Unit) + } + } else { + result.complete(Unit) + } + } + + return result + } + + /** + * Request an immediate synchronization, as configured according to [syncConfig]. + * + * @param startup Boolean flag indicating if sync is being requested in a startup situation. + */ + fun syncNowAsync(startup: Boolean = false): Deferred = CoroutineScope(coroutineContext).async { + // Make sure auth cache is populated before we try to sync. + maybeUpdateSyncAuthInfoCache() + + // Access to syncManager is guarded by `this`. + synchronized(this) { + if (syncManager == null) { + throw IllegalStateException( + "Sync is not configured. Construct this class with a 'syncConfig' or use 'setSyncConfig'" + ) + } + syncManager?.now(startup) + } + Unit + } + /** * Call this after registering your observers, and before interacting with this class. */ @@ -293,6 +380,10 @@ open class FxaAccountManager( deviceEventObserverRegistry.register(observer, owner, autoPause) } + fun registerForSyncEvents(observer: SyncStatusObserver, owner: LifecycleOwner, autoPause: Boolean) { + syncStatusObserverRegistry.register(observer, owner, autoPause) + } + override fun close() { coroutineContext.cancel() account.close() @@ -346,10 +437,7 @@ open class FxaAccountManager( // Don't swallow panics from the underlying library. throw e } catch (e: FxaException) { - logger.error("Failed to load saved account.", e) - - notifyObservers { onError(FailedToLoadAccountException(e)) } - + logger.error("Failed to load saved account. Re-initializing...", e) null } @@ -373,15 +461,16 @@ open class FxaAccountManager( account.close() // Delete persisted state. getAccountStorage().clear() + SyncAuthInfoCache(context).clear() // Re-initialize account. - account = createAccount(config) + account = createAccount(serverConfig) notifyObservers { onLoggedOut() } null } Event.AccountNotFound -> { - account = createAccount(config) + account = createAccount(serverConfig) null } @@ -415,9 +504,11 @@ open class FxaAccountManager( logger.info("Initializing device") // NB: underlying API is expected to 'ensureCapabilities' as part of device initialization. account.deviceConstellation().initDeviceAsync( - deviceTuple.name, deviceTuple.type, deviceTuple.capabilities + deviceConfig.name, deviceConfig.type, deviceConfig.capabilities ).await() + maybeUpdateSyncAuthInfoCache() + notifyObservers { onAuthenticated(account) } Event.FetchProfile @@ -431,7 +522,7 @@ open class FxaAccountManager( // If this is the first time ensuring our capabilities, logger.info("Ensuring device capabilities...") - if (account.deviceConstellation().ensureCapabilitiesAsync(deviceTuple.capabilities).await()) { + if (account.deviceConstellation().ensureCapabilitiesAsync(deviceConfig.capabilities).await()) { logger.info("Successfully ensured device capabilities.") } else { logger.warn("Failed to ensure device capabilities.") @@ -443,6 +534,8 @@ open class FxaAccountManager( logger.info("Stopping periodic refresh of the device constellation") account.deviceConstellation().stopPeriodicRefresh() + maybeUpdateSyncAuthInfoCache() + notifyObservers { onAuthenticated(account) } Event.FetchProfile @@ -459,9 +552,11 @@ open class FxaAccountManager( logger.info("Initializing device") // NB: underlying API is expected to 'ensureCapabilities' as part of device initialization. account.deviceConstellation().initDeviceAsync( - deviceTuple.name, deviceTuple.type, deviceTuple.capabilities + deviceConfig.name, deviceConfig.type, deviceConfig.capabilities ).await() + maybeUpdateSyncAuthInfoCache() + notifyObservers { onAuthenticated(account) } Event.FetchProfile @@ -509,10 +604,14 @@ open class FxaAccountManager( // If we fail with a 401, then we know we have a legitimate authentication problem. logger.info("Hit auth problem. Trying to recover.") + // Clear our access token cache; it'll be re-populated as part of the + // regular state machine flow if we manage to recover. + SyncAuthInfoCache(context).clear() + // We request an access token for a "profile" scope since that's the only // scope we're guaranteed to have at this point. That is, we don't rely on // passed-in application-specific scopes. - when (account.checkAuthorizationStatusAsync(PROFILE_SCOPE).await()) { + when (account.checkAuthorizationStatusAsync(SCOPE_PROFILE).await()) { true -> { logger.info("Able to recover from an auth problem.") @@ -537,7 +636,7 @@ open class FxaAccountManager( // Delete persisted state. getAccountStorage().clear() // Re-initialize account. - account = createAccount(config) + account = createAccount(serverConfig) // Finally, tell our listeners we're in a bad auth state. notifyObservers { onAuthenticationProblems() } @@ -552,6 +651,26 @@ open class FxaAccountManager( } } + private suspend fun maybeUpdateSyncAuthInfoCache() { + // Update cached sync auth info only if we have a syncConfig (e.g. sync is enabled)... + if (syncConfig == null) { + return + } + + // .. and our cache is stale. + val cache = SyncAuthInfoCache(context) + if (!cache.expired()) { + return + } + + // NB: this call will inform authErrorRegistry in case an auth error is encountered. + account.getAccessTokenAsync(SCOPE_SYNC).await()?.let { + SyncAuthInfoCache(context).setToCache( + it.asSyncAuthInfo(account.getTokenServerEndpointURL()) + ) + } + } + private suspend fun doAuthenticate(): Event? { val url = account.beginOAuthFlowAsync(scopes, true).await() if (url == null) { @@ -563,10 +682,15 @@ open class FxaAccountManager( } @VisibleForTesting - open fun createAccount(config: Config): OAuthAccount { + open fun createAccount(config: ServerConfig): OAuthAccount { return FirefoxAccount(config) } + @VisibleForTesting + open fun createSyncManager(config: SyncConfig): SyncManager { + return WorkManagerSyncManager(config) + } + @VisibleForTesting open fun getAccountStorage(): AccountStorage { return SharedPrefAccountStorage(context) @@ -588,27 +712,46 @@ open class FxaAccountManager( } } - private class SyncManagerIntegration(private val syncManager: SyncManager) : AccountObserver { + /** + * Account status events flowing into the sync manager. + */ + private class AccountsToSyncIntegration( + private val syncManager: SyncManager + ) : AccountObserver { override fun onLoggedOut() { - syncManager.loggedOut() + syncManager.stop() } override fun onAuthenticated(account: OAuthAccount) { - syncManager.authenticated(account) + syncManager.start() } override fun onProfileUpdated(profile: Profile) { - // SyncManager doesn't care about the FxA profile. + // Sync currently doesn't care about the FxA profile. // In the future, we might kick-off an immediate sync here. } override fun onAuthenticationProblems() { - syncManager.loggedOut() + syncManager.stop() + } + } + + /** + * Sync status changes flowing into account manager. + */ + private class SyncToAccountsIntegration( + private val accountManager: FxaAccountManager + ) : SyncStatusObserver { + override fun onStarted() { + accountManager.syncStatusObserverRegistry.notifyObservers { onStarted() } + } + + override fun onIdle() { + accountManager.syncStatusObserverRegistry.notifyObservers { onIdle() } } - override fun onError(error: Exception) { - // TODO deal with FxaUnauthorizedException this at the state machine level. - // This exception should cause a "logged out" transition. + override fun onError(error: Exception?) { + accountManager.syncStatusObserverRegistry.notifyObservers { onError(error) } } } } diff --git a/components/feature/sync/src/main/java/mozilla/components/feature/sync/StorageSync.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/StorageSync.kt similarity index 55% rename from components/feature/sync/src/main/java/mozilla/components/feature/sync/StorageSync.kt rename to components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/StorageSync.kt index 657608d142a..8ee5b7e7b80 100644 --- a/components/feature/sync/src/main/java/mozilla/components/feature/sync/StorageSync.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/StorageSync.kt @@ -2,31 +2,28 @@ * 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.feature.sync +package mozilla.components.service.fxa.sync import androidx.annotation.VisibleForTesting import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import mozilla.components.concept.sync.AuthException import mozilla.components.concept.sync.AuthExceptionType -import mozilla.components.concept.sync.AuthInfo -import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.StoreSyncStatus +import mozilla.components.concept.sync.SyncAuthInfo import mozilla.components.concept.sync.SyncableStore import mozilla.components.concept.sync.SyncResult import mozilla.components.concept.sync.SyncStatus -import mozilla.components.concept.sync.SyncStatusObserver 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 /** - * A feature implementation which orchestrates data synchronization of a set of [SyncableStore]-s. + * Orchestrates data synchronization of a set of [SyncableStore]-s. */ class StorageSync( - private val syncableStores: Map, - private val syncScope: String + private val syncableStores: Map ) : Observable by ObserverRegistry() { private val logger = Logger("StorageSync") @@ -41,25 +38,16 @@ class StorageSync( * Performs a sync of configured [SyncableStore] history instance. This method guarantees that * only one sync may be running at any given time. * - * @param account [OAuthAccount] for which to perform a sync. + * @param authInfo [SyncAuthInfo] that will be used to authenticate synchronization. * @return a [SyncResult] indicating result of synchronization of configured stores. */ - suspend fun sync(account: OAuthAccount): SyncResult = syncMutex.withLock { withListeners { + suspend fun sync(authInfo: SyncAuthInfo): SyncResult = syncMutex.withLock { withListeners { if (syncableStores.isEmpty()) { return@withListeners mapOf() } val results = mutableMapOf() - val authInfo = try { - account.authInfo(syncScope) - } catch (e: AuthException) { - syncableStores.keys.forEach { storeName -> - results[storeName] = StoreSyncStatus(SyncStatus.Error(e)) - } - return@withListeners results - } - syncableStores.keys.forEach { storeName -> results[storeName] = syncStore(syncableStores.getValue(storeName), storeName, authInfo) } @@ -70,27 +58,25 @@ class StorageSync( private suspend fun syncStore( store: SyncableStore, storeName: String, - auth: AuthInfo - ): StoreSyncStatus { - return StoreSyncStatus(store.sync(auth).also { - if (it is SyncStatus.Error) { - val message = it.exception.message - // TODO hackiness of this check is temporary. Eventually underlying libraries will - // surface reliable authentication exceptions which we can check by type. - // See https://github.com/mozilla-mobile/android-components/issues/3322 - if (message != null && message.contains("401")) { - logger.error("Hit an auth error during syncing") - authErrorRegistry.notifyObservers { - onAuthErrorAsync(AuthException(AuthExceptionType.UNAUTHORIZED)) - } - } else { - logger.error("Error synchronizing a $storeName store", it.exception) + auth: SyncAuthInfo + ): StoreSyncStatus = StoreSyncStatus(store.sync(auth).also { + if (it is SyncStatus.Error) { + val message = it.exception.message + // TODO hackiness of this check is temporary. Eventually underlying libraries will + // surface reliable authentication exceptions which we can check by type. + // See https://github.com/mozilla-mobile/android-components/issues/3322 + if (message != null && message.contains("401")) { + logger.error("Hit an auth error during syncing") + authErrorRegistry.notifyObservers { + onAuthErrorAsync(AuthException(AuthExceptionType.UNAUTHORIZED)) } } else { - logger.info("Synchronized $storeName store.") + logger.error("Error synchronizing a $storeName store", it.exception) } - }) - } + } else { + logger.info("Synchronized $storeName store.") + } + }) private suspend fun withListeners(block: suspend () -> SyncResult): SyncResult { notifyObservers { onStarted() } diff --git a/components/feature/sync/src/main/java/mozilla/components/feature/sync/BackgroundSyncManager.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/SyncManager.kt similarity index 50% rename from components/feature/sync/src/main/java/mozilla/components/feature/sync/BackgroundSyncManager.kt rename to components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/SyncManager.kt index 8a76148cc4b..ca17087747d 100644 --- a/components/feature/sync/src/main/java/mozilla/components/feature/sync/BackgroundSyncManager.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/SyncManager.kt @@ -2,12 +2,10 @@ * 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.feature.sync +package mozilla.components.service.fxa.sync -import mozilla.components.concept.sync.OAuthAccount -import mozilla.components.concept.sync.SyncManager -import mozilla.components.concept.sync.SyncStatusObserver import mozilla.components.concept.sync.SyncableStore +import mozilla.components.service.fxa.SyncConfig import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.base.observer.Observable import mozilla.components.support.base.observer.ObserverRegistry @@ -15,6 +13,27 @@ import java.io.Closeable import java.lang.Exception import java.util.concurrent.TimeUnit +/** + * An interface for consumers that wish to observer "sync lifecycle" events. + */ +interface SyncStatusObserver { + /** + * Gets called at the start of a sync, before any configured syncable is synchronized. + */ + fun onStarted() + + /** + * Gets called at the end of a sync, after every configured syncable has been synchronized. + */ + fun onIdle() + + /** + * Gets called if sync encounters an error that's worthy of processing by status observers. + * @param error Optional relevant exception. + */ + fun onError(error: Exception?) +} + /** * A singleton registry of [SyncableStore] objects. [WorkManagerSyncDispatcher] will use this to * access configured [SyncableStore] instances. @@ -46,39 +65,13 @@ interface SyncDispatcher : Closeable, Observable { } /** - * A SyncManager implementation which uses WorkManager APIs to schedule sync tasks. - */ -class BackgroundSyncManager( - private val syncScope: String -) : GeneralSyncManager() { - override val logger = Logger("BgSyncManager") - - init { - // Initialize the singleton observer. This must be called on the main thread. - WorkersLiveDataObserver.init() - } - - override fun createDispatcher(stores: Set, account: OAuthAccount): SyncDispatcher { - return WorkManagerSyncDispatcher(stores, syncScope, account) - } - - override fun dispatcherUpdated(dispatcher: SyncDispatcher) { - WorkersLiveDataObserver.setDispatcher(dispatcher) - } -} - -private val registry = ObserverRegistry() - -/** - * A base SyncManager implementation which manages a dispatcher, handles authentication and requests - * synchronization in the following manner: - * On authentication AND on store set changes (add or remove)... - * - immediate sync and periodic syncing are requested - * On logout... - * - periodic sync is requested to stop + * A base sync manager implementation. + * @param syncConfig A [SyncConfig] object describing how sync should behave. */ @SuppressWarnings("TooManyFunctions") -abstract class GeneralSyncManager : SyncManager, Observable by registry, SyncStatusObserver { +abstract class SyncManager( + private val syncConfig: SyncConfig +) { companion object { // Periodically sync in the background, to make our syncs a little more incremental. // This isn't strictly necessary, and could be considered an optimization. @@ -98,116 +91,102 @@ abstract class GeneralSyncManager : SyncManager, Observable // // If we wanted to be very fancy, this period could be driven by how much new activity an // account is actually expected to generate. For now, it's just a hard-coded constant. - const val SYNC_PERIOD = 4L - val SYNC_PERIOD_UNIT = TimeUnit.HOURS + val SYNC_PERIOD_UNIT = TimeUnit.MINUTES } - open val logger = Logger("GeneralSyncManager") - - private var account: OAuthAccount? = null - private val stores = mutableSetOf() + open val logger = Logger("SyncManager") // A SyncDispatcher instance bound to an account and a set of syncable stores. private var syncDispatcher: SyncDispatcher? = null - override fun addStore(name: String) = synchronized(this) { - logger.debug("Adding store: $name") - stores.add(name) - account?.let { - syncDispatcher = newDispatcher(syncDispatcher, stores, it) - initDispatcher(syncDispatcher!!) - } - Unit - } - - override fun removeStore(name: String) = synchronized(this) { - logger.debug("Removing store: $name") - stores.remove(name) - account?.let { - syncDispatcher = newDispatcher(syncDispatcher, stores, it) - initDispatcher(syncDispatcher!!) - } - Unit - } - - override fun authenticated(account: OAuthAccount) = synchronized(this) { - logger.debug("Authenticated") - this.account = account - syncDispatcher = newDispatcher(syncDispatcher, stores, account) - initDispatcher(syncDispatcher!!) - logger.debug("set and initialized new dispatcher: $syncDispatcher") - } + private val syncStatusObserverRegistry = ObserverRegistry() - override fun loggedOut() = synchronized(this) { - logger.debug("Logging out") - // We might have never had a chance to initialize the account/dispatcher (e.g. during auth - // problems while restoring an account on startup). Bail out. - if (account == null) { - return@synchronized + // Manager encapsulates events emitted by the wrapped observers, and passes them on to the outside world. + // This split allows us to define a nice public observer API (manager -> consumers), along with + // a more robust internal observer API (dispatcher -> manager). + // Currently the interfaces are the same, hence the name "pass-through". + private val dispatcherStatusObserver = PassThroughSyncStatusObserver(syncStatusObserverRegistry) + + internal fun registerSyncStatusObserver(observer: SyncStatusObserver) { + syncStatusObserverRegistry.register(observer) + } + + /** + * Request an immediate synchronization of all configured stores. + * + * @param startup Boolean flag indicating if sync is being requested in a startup situation. + */ + internal fun now(startup: Boolean = false) = synchronized(this) { + if (syncDispatcher == null) { + logger.info("Sync is not enabled. Ignoring 'sync now' request.") } - account = null - syncDispatcher!!.stopPeriodicSync() - syncDispatcher!!.close() - syncDispatcher = null - } - - override fun syncNow(startup: Boolean) = synchronized(this) { - // TODO This is a workaround until we figure out why we get into this state. - // https://github.com/mozilla-mobile/android-components/issues/3226 - - // We may not have an 'account' here (and by extension, syncDispatcher) if we hit auth - // problems. syncDispatcher?.let { logger.debug("Requesting immediate sync") it.syncNow(startup) } - Unit } - override fun isSyncRunning(): Boolean { - syncDispatcher?.let { return it.isSyncActive() } - return false + /** + * Enables synchronization, with behaviour described in [syncConfig]. + */ + internal fun start() = synchronized(this) { + logger.debug("Enabling...") + syncDispatcher = initDispatcher(newDispatcher(syncDispatcher, syncConfig.syncableStores)) + logger.debug("set and initialized new dispatcher: $syncDispatcher") } - abstract fun createDispatcher(stores: Set, account: OAuthAccount): SyncDispatcher - - abstract fun dispatcherUpdated(dispatcher: SyncDispatcher) - - // Manager encapsulates events emitted by the wrapped observers, and passes them on to the outside world. - // This split allows us to define a nice public observer API (manager -> consumers), along with - // a more robust internal observer API (dispatcher -> manager). Currently the interfaces are the same. - - override fun onStarted() { - notifyObservers { onStarted() } + /** + * Disables synchronization. + */ + internal fun stop() = synchronized(this) { + logger.debug("Disabling...") + syncDispatcher?.unregister(dispatcherStatusObserver) + syncDispatcher?.stopPeriodicSync() + syncDispatcher?.close() + syncDispatcher = null } - override fun onIdle() { - notifyObservers { onIdle() } - } + internal abstract fun createDispatcher(stores: Set): SyncDispatcher - override fun onError(error: Exception?) { - notifyObservers { onError(error) } - } + internal abstract fun dispatcherUpdated(dispatcher: SyncDispatcher) private fun newDispatcher( currentDispatcher: SyncDispatcher?, - stores: Set, - account: OAuthAccount + stores: Set ): SyncDispatcher { // Let the existing dispatcher, if present, cleanup. currentDispatcher?.close() // TODO will events from old and new dispatchers overlap..? How do we ensure correct sequence // for outside observers? - currentDispatcher?.unregister(this) + currentDispatcher?.unregister(dispatcherStatusObserver) // Create a new dispatcher bound to current stores and account. - return createDispatcher(stores, account) + return createDispatcher(stores) } - private fun initDispatcher(dispatcher: SyncDispatcher) { - dispatcher.register(this@GeneralSyncManager) + private fun initDispatcher(dispatcher: SyncDispatcher): SyncDispatcher { + dispatcher.register(dispatcherStatusObserver) dispatcher.syncNow() - dispatcher.startPeriodicSync(SYNC_PERIOD_UNIT, SYNC_PERIOD) + if (syncConfig.syncPeriodInMinutes != null) { + dispatcher.startPeriodicSync(SYNC_PERIOD_UNIT, syncConfig.syncPeriodInMinutes) + } dispatcherUpdated(dispatcher) + return dispatcher + } + + private class PassThroughSyncStatusObserver( + private val passThroughRegistry: ObserverRegistry + ) : SyncStatusObserver { + override fun onStarted() { + passThroughRegistry.notifyObservers { onStarted() } + } + + override fun onIdle() { + passThroughRegistry.notifyObservers { onIdle() } + } + + override fun onError(error: Exception?) { + passThroughRegistry.notifyObservers { onError(error) } + } } } diff --git a/components/feature/sync/src/main/java/mozilla/components/feature/sync/WorkManagerSyncDispatcher.kt b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt similarity index 79% rename from components/feature/sync/src/main/java/mozilla/components/feature/sync/WorkManagerSyncDispatcher.kt rename to components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt index c91fe0d06d7..767adef5d72 100644 --- a/components/feature/sync/src/main/java/mozilla/components/feature/sync/WorkManagerSyncDispatcher.kt +++ b/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/sync/WorkManagerSyncManager.kt @@ -2,7 +2,7 @@ * 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.feature.sync +package mozilla.components.service.fxa.sync import android.content.Context import androidx.annotation.UiThread @@ -20,10 +20,9 @@ import androidx.work.WorkInfo import androidx.work.WorkManager import androidx.work.WorkerParameters import mozilla.components.concept.sync.AuthException -import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.SyncStatus -import mozilla.components.concept.sync.SyncStatusObserver -import mozilla.components.service.fxa.SharedPrefAccountStorage +import mozilla.components.service.fxa.SyncAuthInfoCache +import mozilla.components.service.fxa.SyncConfig import mozilla.components.service.fxa.manager.authErrorRegistry import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.base.observer.Observable @@ -42,6 +41,35 @@ private enum class SyncWorkerName { Immediate } +private const val KEY_DATA_STORES = "stores" + +/** + * A [SyncManager] implementation which uses WorkManager APIs to schedule sync tasks. + * + * Must be initialized on the main thread. + */ +internal class WorkManagerSyncManager(syncConfig: SyncConfig) : SyncManager(syncConfig) { + override val logger = Logger("BgSyncManager") + + init { + WorkersLiveDataObserver.init() + + if (syncConfig.syncPeriodInMinutes == null) { + logger.info("Periodic syncing is disabled.") + } else { + logger.info("Periodic syncing enabled at a ${syncConfig.syncPeriodInMinutes} interval") + } + } + + override fun createDispatcher(stores: Set): SyncDispatcher { + return WorkManagerSyncDispatcher(stores) + } + + override fun dispatcherUpdated(dispatcher: SyncDispatcher) { + WorkersLiveDataObserver.setDispatcher(dispatcher) + } +} + /** * A singleton wrapper around the the LiveData "forever" observer - i.e. an observer not bound * to a lifecycle owner. This observer is always active. @@ -57,9 +85,12 @@ object WorkersLiveDataObserver { @UiThread fun init() { + // Only set our observer once. + if (workersLiveData.hasObservers()) return + + // This must be called on the UI thread. workersLiveData.observeForever { - val isRunningOrNothing = it?.any { worker -> worker.state == WorkInfo.State.RUNNING } - val isRunning = when (isRunningOrNothing) { + val isRunning = when (it?.any { worker -> worker.state == WorkInfo.State.RUNNING }) { null -> false false -> false true -> true @@ -77,15 +108,19 @@ object WorkersLiveDataObserver { } class WorkManagerSyncDispatcher( - private val stores: Set, - private val syncScope: String, - private val account: OAuthAccount + private val stores: Set ) : SyncDispatcher, Observable by ObserverRegistry(), Closeable { private val logger = Logger("WMSyncDispatcher") // TODO does this need to be volatile? private var isSyncActive = false + init { + // Stop any currently active periodic syncing. Consumers of this class are responsible for + // starting periodic syncing via [startPeriodicSync] if they need it. + stopPeriodicSync() + } + override fun workersStateChanged(isRunning: Boolean) { if (isSyncActive && !isRunning) { notifyObservers { onIdle() } @@ -138,6 +173,10 @@ class WorkManagerSyncDispatcher( ) } + /** + * Disables periodic syncing in the background. Currently running syncs may continue until completion. + * Safe to call this even if periodic syncing isn't currently enabled. + */ override fun stopPeriodicSync() { logger.debug("Cancelling periodic syncing") WorkManager.getInstance().cancelUniqueWork(SyncWorkerName.Periodic.name) @@ -175,10 +214,7 @@ class WorkManagerSyncDispatcher( } private fun getWorkerData(): Data { - val dataBuilder = Data.Builder() - .putString("account", account.toJSONString()) - .putString("syncScope", syncScope) - .putStringArray("stores", stores.toTypedArray()) + val dataBuilder = Data.Builder().putStringArray(KEY_DATA_STORES, stores.toTypedArray()) return dataBuilder.build() } @@ -199,7 +235,7 @@ class WorkManagerSyncWorker( return lastSyncedTs != 0L && (System.currentTimeMillis() - lastSyncedTs) < SYNC_STAGGER_BUFFER_MS } - @Suppress("ReturnCount", "ComplexMethod") + @Suppress("ReturnCount", "LongMethod") override suspend fun doWork(): Result { logger.debug("Starting sync... Tagged as: ${params.tags}") @@ -212,18 +248,21 @@ class WorkManagerSyncWorker( } // Otherwise, proceed as normal. - // Read all of the data we'll need to sync: account, syncScope and a map of SyncableStore objects. - val accountStorage = SharedPrefAccountStorage(context) - val account = accountStorage.read() - // Account must have disappeared in the time between this task being scheduled and executed. - // A "logout" is expected to cancel pending sync tasks, but that isn't guaranteed. - ?: return Result.failure() - val syncScope = params.inputData.getString("syncScope")!! - val syncableStores = params.inputData.getStringArray("stores")!!.associate { + // In order to sync, we'll need: + // - a list of SyncableStores... + val syncableStores = params.inputData.getStringArray(KEY_DATA_STORES)!!.associate { it to GlobalSyncableStoreProvider.getStore(it)!! + }.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() } - val syncResult = StorageSync(syncableStores, syncScope).sync(account) + // - and a cached "sync auth info" object. + val syncAuthInfo = SyncAuthInfoCache(context).getCached() ?: return Result.failure() + + // Sync! + val syncResult = StorageSync(syncableStores).sync(syncAuthInfo) val resultBuilder = Data.Builder() syncResult.forEach { 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 dbe56c48924..edb8352abc2 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 @@ -5,6 +5,8 @@ package mozilla.components.service.fxa import android.content.Context +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner import androidx.test.ext.junit.runners.AndroidJUnit4 import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Deferred @@ -17,14 +19,19 @@ import mozilla.components.concept.sync.DeviceCapability import mozilla.components.concept.sync.DeviceConstellation import mozilla.components.concept.sync.DeviceType import mozilla.components.concept.sync.OAuthAccount +import mozilla.components.concept.sync.OAuthScopedKey import mozilla.components.concept.sync.Profile import mozilla.components.concept.sync.StatePersistenceCallback import mozilla.components.service.fxa.manager.AccountState -import mozilla.components.service.fxa.manager.DeviceTuple import mozilla.components.service.fxa.manager.Event -import mozilla.components.service.fxa.manager.FailedToLoadAccountException import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.service.fxa.manager.authErrorRegistry +import mozilla.components.service.fxa.manager.SCOPE_SYNC +import mozilla.components.service.fxa.sync.SyncManager +import mozilla.components.service.fxa.sync.SyncDispatcher +import mozilla.components.service.fxa.sync.SyncStatusObserver +import mozilla.components.support.base.observer.Observable +import mozilla.components.support.base.observer.ObserverRegistry import mozilla.components.support.test.any import mozilla.components.support.test.eq import mozilla.components.support.test.mock @@ -40,27 +47,31 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.anyBoolean import org.mockito.ArgumentMatchers.anyString +import org.mockito.ArgumentMatchers.anyLong import org.mockito.Mockito.`when` import org.mockito.Mockito.never import org.mockito.Mockito.reset import org.mockito.Mockito.times import org.mockito.Mockito.verify +import java.lang.Exception +import java.lang.IllegalArgumentException +import java.util.concurrent.TimeUnit import kotlin.coroutines.CoroutineContext // Same as the actual account manager, except we get to control how FirefoxAccountShaped instances // are created. This is necessary because due to some build issues (native dependencies not available // within the test environment) we can't use fxaclient supplied implementation of FirefoxAccountShaped. // Instead, we express all of our account-related operations over an interface. -class TestableFxaAccountManager( +open class TestableFxaAccountManager( context: Context, - config: Config, - scopes: Array, + config: ServerConfig, private val storage: AccountStorage, - capabilities: List = listOf(), + capabilities: Set = emptySet(), + syncConfig: SyncConfig? = null, coroutineContext: CoroutineContext, - val block: () -> OAuthAccount = { mock() } -) : FxaAccountManager(context, config, scopes, DeviceTuple("test", DeviceType.UNKNOWN, capabilities), null, coroutineContext) { - override fun createAccount(config: Config): OAuthAccount { + private val block: () -> OAuthAccount = { mock() } +) : FxaAccountManager(context, config, DeviceConfig("test", DeviceType.UNKNOWN, capabilities), syncConfig, emptySet(), coroutineContext) { + override fun createAccount(config: ServerConfig): OAuthAccount { return block() } @@ -77,6 +88,7 @@ class FxaAccountManagerTest { // This registry is global, so we need to clear it between test runs, otherwise different // manager instances will be kept around. authErrorRegistry.unregisterObservers() + SyncAuthInfoCache(testContext).clear() } @Test @@ -154,6 +166,256 @@ class FxaAccountManagerTest { assertNull(FxaAccountManager.nextState(state, Event.AuthenticationError(AuthException(AuthExceptionType.UNAUTHORIZED)))) } + class TestSyncDispatcher(registry: ObserverRegistry) : SyncDispatcher, Observable by registry { + val inner: SyncDispatcher = mock() + override fun isSyncActive(): Boolean { + return inner.isSyncActive() + } + + override fun syncNow(startup: Boolean) { + inner.syncNow(startup) + } + + override fun startPeriodicSync(unit: TimeUnit, period: Long) { + inner.startPeriodicSync(unit, period) + } + + override fun stopPeriodicSync() { + inner.stopPeriodicSync() + } + + override fun workersStateChanged(isRunning: Boolean) { + inner.workersStateChanged(isRunning) + } + + override fun close() { + inner.close() + } + } + + class TestSyncManager(config: SyncConfig) : SyncManager(config) { + val dispatcherRegistry = ObserverRegistry() + val dispatcher: TestSyncDispatcher = TestSyncDispatcher(dispatcherRegistry) + + private var dispatcherUpdatedCount = 0 + override fun createDispatcher(stores: Set): SyncDispatcher { + return dispatcher + } + + override fun dispatcherUpdated(dispatcher: SyncDispatcher) { + dispatcherUpdatedCount++ + } + } + + class TestSyncStatusObserver : SyncStatusObserver { + var onStartedCount = 0 + var onIdleCount = 0 + var onErrorCount = 0 + + override fun onStarted() { + onStartedCount++ + } + + override fun onIdle() { + onIdleCount++ + } + + override fun onError(error: Exception?) { + onErrorCount++ + } + } + + @Test + fun `updating sync config, without it at first`() = runBlocking { + val accountStorage: AccountStorage = mock() + val profile = Profile("testUid", "test@example.com", null, "Test Profile") + val constellation: DeviceConstellation = mock() + + val syncAccessTokenExpiresAt = System.currentTimeMillis() + 10 * 60 * 1000L + val account = StatePersistenceTestableAccount(profile, constellation, tokenServerEndpointUrl = "https://some.server.com/test") { + AccessTokenInfo( + SCOPE_SYNC, + "tolkien", + OAuthScopedKey("kty-test", SCOPE_SYNC, "kid-test", "k-test"), + syncAccessTokenExpiresAt + ) + } + + var latestSyncManager: TestSyncManager? = null + // Without sync config to begin with. NB: we're pretending that we "have" a sync scope. + val manager = object : TestableFxaAccountManager( + context = testContext, + config = ServerConfig.release("dummyId", "http://auth-url/redirect"), + storage = accountStorage, + capabilities = setOf(DeviceCapability.SEND_TAB), + syncConfig = null, + coroutineContext = this@runBlocking.coroutineContext, + block = { account } + ) { + override fun createSyncManager(config: SyncConfig): SyncManager { + return TestSyncManager(config).also { latestSyncManager = it } + } + } + + `when`(constellation.ensureCapabilitiesAsync(any())).thenReturn(CompletableDeferred(true)) + // We have an account at the start. + `when`(accountStorage.read()).thenReturn(account) + + manager.initAsync().await() + + val syncStatusObserver = TestSyncStatusObserver() + val lifecycleOwner: LifecycleOwner = mock() + val lifecycle: Lifecycle = mock() + `when`(lifecycle.currentState).thenReturn(Lifecycle.State.STARTED) + `when`(lifecycleOwner.lifecycle).thenReturn(lifecycle) + manager.registerForSyncEvents(syncStatusObserver, lifecycleOwner, true) + + // Bad configuration: no stores. + try { + manager.setSyncConfigAsync(SyncConfig(setOf())).await() + fail() + } catch (e: IllegalArgumentException) {} + + assertNull(latestSyncManager) + + assertEquals(0, syncStatusObserver.onStartedCount) + assertEquals(0, syncStatusObserver.onIdleCount) + assertEquals(0, syncStatusObserver.onErrorCount) + + // No periodic sync. + manager.setSyncConfigAsync(SyncConfig(setOf("history"))).await() + + 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()) + + // With periodic sync. + manager.setSyncConfigAsync(SyncConfig(setOf("history"), 60 * 1000L)).await() + + verify(latestSyncManager!!.dispatcher.inner, times(1)).startPeriodicSync(any(), anyLong()) + verify(latestSyncManager!!.dispatcher.inner, never()).stopPeriodicSync() + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(anyBoolean()) + + // Make sure sync status listeners are working. +// // Test dispatcher -> sync manager -> account manager -> our test observer. +// latestSyncManager!!.dispatcherRegistry.notifyObservers { onStarted() } +// assertEquals(1, syncStatusObserver.onStartedCount) +// latestSyncManager!!.dispatcherRegistry.notifyObservers { onIdle() } +// assertEquals(1, syncStatusObserver.onIdleCount) +// latestSyncManager!!.dispatcherRegistry.notifyObservers { onError(null) } +// assertEquals(1, syncStatusObserver.onErrorCount) + + // Make sure that sync access token was cached correctly. + assertFalse(SyncAuthInfoCache(testContext).expired()) + val cachedAuthInfo = SyncAuthInfoCache(testContext).getCached() + assertNotNull(cachedAuthInfo) + assertEquals("tolkien", cachedAuthInfo!!.fxaAccessToken) + assertEquals(syncAccessTokenExpiresAt, cachedAuthInfo.fxaAccessTokenExpiresAt) + assertEquals("https://some.server.com/test", cachedAuthInfo.tokenServerUrl) + assertEquals("kid-test", cachedAuthInfo.kid) + assertEquals("k-test", cachedAuthInfo.syncKey) + } + + @Test + fun `updating sync config, with one to begin with`() = runBlocking { + val accountStorage: AccountStorage = mock() + val profile = Profile("testUid", "test@example.com", null, "Test Profile") + val constellation: DeviceConstellation = mock() + + val syncAccessTokenExpiresAt = System.currentTimeMillis() + 10 * 60 * 1000L + val account = StatePersistenceTestableAccount(profile, constellation, tokenServerEndpointUrl = "https://some.server.com/test") { + AccessTokenInfo( + SCOPE_SYNC, + "arda", + OAuthScopedKey("kty-test", SCOPE_SYNC, "kid-test", "k-test"), + syncAccessTokenExpiresAt + ) + } + + // With a sync config this time. + var latestSyncManager: TestSyncManager? = null + val syncConfig = SyncConfig(setOf("history"), syncPeriodInMinutes = 120L) + val manager = object : TestableFxaAccountManager( + context = testContext, + config = ServerConfig.release("dummyId", "http://auth-url/redirect"), + storage = accountStorage, + capabilities = setOf(DeviceCapability.SEND_TAB), + syncConfig = syncConfig, + coroutineContext = this@runBlocking.coroutineContext, + block = { account } + ) { + override fun createSyncManager(config: SyncConfig): SyncManager { + return TestSyncManager(config).also { latestSyncManager = it } + } + } + + `when`(constellation.ensureCapabilitiesAsync(any())).thenReturn(CompletableDeferred(true)) + // We have an account at the start. + `when`(accountStorage.read()).thenReturn(account) + + val syncStatusObserver = TestSyncStatusObserver() + val lifecycleOwner: LifecycleOwner = mock() + val lifecycle: Lifecycle = mock() + `when`(lifecycle.currentState).thenReturn(Lifecycle.State.STARTED) + `when`(lifecycleOwner.lifecycle).thenReturn(lifecycle) + manager.registerForSyncEvents(syncStatusObserver, lifecycleOwner, true) + + manager.initAsync().await() + + // Make sure that sync access token was cached correctly. + assertFalse(SyncAuthInfoCache(testContext).expired()) + val cachedAuthInfo = SyncAuthInfoCache(testContext).getCached() + assertNotNull(cachedAuthInfo) + assertEquals("arda", cachedAuthInfo!!.fxaAccessToken) + assertEquals(syncAccessTokenExpiresAt, cachedAuthInfo.fxaAccessTokenExpiresAt) + assertEquals("https://some.server.com/test", cachedAuthInfo.tokenServerUrl) + assertEquals("kid-test", cachedAuthInfo.kid) + assertEquals("k-test", cachedAuthInfo.syncKey) + + // Make sure periodic syncing started, and an immediate sync was requested. + assertNotNull(latestSyncManager) + assertNotNull(latestSyncManager!!.dispatcher) + 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()) + + // Can trigger syncs. + manager.syncNowAsync().await() + verify(latestSyncManager!!.dispatcher.inner, times(2)).syncNow(anyBoolean()) + manager.syncNowAsync(startup = true).await() + verify(latestSyncManager!!.dispatcher.inner, times(3)).syncNow(anyBoolean()) + +// assertEquals(0, syncStatusObserver.onStartedCount) +// assertEquals(0, syncStatusObserver.onIdleCount) +// assertEquals(0, syncStatusObserver.onErrorCount) + + // Make sure sync status listeners are working. +// // Test dispatcher -> sync manager -> account manager -> our test observer. +// latestSyncManager!!.dispatcherRegistry.notifyObservers { onStarted() } +// assertEquals(1, syncStatusObserver.onStartedCount) +// latestSyncManager!!.dispatcherRegistry.notifyObservers { onIdle() } +// assertEquals(1, syncStatusObserver.onIdleCount) +// latestSyncManager!!.dispatcherRegistry.notifyObservers { onError(null) } +// assertEquals(1, syncStatusObserver.onErrorCount) + + // Turn off periodic syncing. + manager.setSyncConfigAsync(SyncConfig(setOf("history"))).await() + + verify(latestSyncManager!!.dispatcher.inner, never()).startPeriodicSync(any(), anyLong()) + verify(latestSyncManager!!.dispatcher.inner, never()).stopPeriodicSync() + verify(latestSyncManager!!.dispatcher.inner, times(1)).syncNow(anyBoolean()) + + // Can trigger syncs. + manager.syncNowAsync().await() + verify(latestSyncManager!!.dispatcher.inner, times(2)).syncNow(anyBoolean()) + manager.syncNowAsync(startup = true).await() + verify(latestSyncManager!!.dispatcher.inner, times(3)).syncNow(anyBoolean()) + } + @Test fun `restored account state persistence`() = runBlocking { val accountStorage: AccountStorage = mock() @@ -162,8 +424,8 @@ class FxaAccountManagerTest { val account = StatePersistenceTestableAccount(profile, constellation) val manager = TestableFxaAccountManager( - testContext, Config.release("dummyId", "http://auth-url/redirect"), arrayOf("profile"), accountStorage, - listOf(DeviceCapability.SEND_TAB), this.coroutineContext + testContext, ServerConfig.release("dummyId", "http://auth-url/redirect"), accountStorage, + setOf(DeviceCapability.SEND_TAB), null, this.coroutineContext ) { account } @@ -179,7 +441,7 @@ class FxaAccountManagerTest { assertNotNull(account.persistenceCallback) // Assert that ensureCapabilities fired, but not the device initialization (since we're restoring). - verify(constellation).ensureCapabilitiesAsync(listOf(DeviceCapability.SEND_TAB)) + verify(constellation).ensureCapabilitiesAsync(setOf(DeviceCapability.SEND_TAB)) verify(constellation, never()).initDeviceAsync(any(), any(), any()) // Assert that periodic account refresh never started. @@ -201,8 +463,8 @@ class FxaAccountManagerTest { val account = StatePersistenceTestableAccount(profile, constellation) val manager = TestableFxaAccountManager( - testContext, Config.release("dummyId", "http://auth-url/redirect"), arrayOf("profile"), accountStorage, - listOf(DeviceCapability.SEND_TAB), this.coroutineContext + testContext, ServerConfig.release("dummyId", "http://auth-url/redirect"), accountStorage, + setOf(DeviceCapability.SEND_TAB), null, this.coroutineContext ) { account } @@ -218,7 +480,7 @@ class FxaAccountManagerTest { assertNotNull(account.persistenceCallback) // Assert that ensureCapabilities fired, but not the device initialization (since we're restoring). - verify(constellation).ensureCapabilitiesAsync(listOf(DeviceCapability.SEND_TAB)) + verify(constellation).ensureCapabilitiesAsync(setOf(DeviceCapability.SEND_TAB)) verify(constellation, never()).initDeviceAsync(any(), any(), any()) // Assert that periodic account refresh never started. @@ -239,8 +501,8 @@ class FxaAccountManagerTest { val accountObserver: AccountObserver = mock() val manager = TestableFxaAccountManager( - testContext, Config.release("dummyId", "http://auth-url/redirect"), arrayOf("profile"), accountStorage, - listOf(DeviceCapability.SEND_TAB), this.coroutineContext + testContext, ServerConfig.release("dummyId", "http://auth-url/redirect"), accountStorage, + setOf(DeviceCapability.SEND_TAB), null, this.coroutineContext ) { account } @@ -277,8 +539,8 @@ class FxaAccountManagerTest { val accountObserver: AccountObserver = mock() val manager = TestableFxaAccountManager( - testContext, Config.release("dummyId", "http://auth-url/redirect"), arrayOf("profile"), accountStorage, - listOf(DeviceCapability.SEND_TAB), this.coroutineContext + testContext, ServerConfig.release("dummyId", "http://auth-url/redirect"), accountStorage, + setOf(DeviceCapability.SEND_TAB), null, this.coroutineContext ) { account } @@ -311,10 +573,9 @@ class FxaAccountManagerTest { // but an actual implementation of the interface. val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, - listOf(DeviceCapability.SEND_TAB), this.coroutineContext + setOf(DeviceCapability.SEND_TAB), null, this.coroutineContext ) { account } @@ -348,7 +609,7 @@ class FxaAccountManagerTest { verify(constellation, never()).startPeriodicRefresh() // Assert that initDevice fired, but not ensureCapabilities (since we're initing a new account). - verify(constellation).initDeviceAsync(any(), any(), eq(listOf(DeviceCapability.SEND_TAB))) + verify(constellation).initDeviceAsync(any(), any(), eq(setOf(DeviceCapability.SEND_TAB))) verify(constellation, never()).ensureCapabilitiesAsync(any()) // Assert that persistence callback is interacting with the storage layer. @@ -356,7 +617,13 @@ class FxaAccountManagerTest { verify(accountStorage).write("test") } - class StatePersistenceTestableAccount(private val profile: Profile, private val constellation: DeviceConstellation, val ableToRecoverFromAuthError: Boolean = false) : OAuthAccount { + class StatePersistenceTestableAccount( + private val profile: Profile, + private val constellation: DeviceConstellation, + val ableToRecoverFromAuthError: Boolean = false, + val tokenServerEndpointUrl: String? = null, + val accessToken: (() -> AccessTokenInfo)? = null + ) : OAuthAccount { var persistenceCallback: StatePersistenceCallback? = null var accessTokenErrorCalled = false @@ -380,9 +647,10 @@ class FxaAccountManagerTest { return CompletableDeferred(true) } - override fun getAccessTokenAsync( - singleScope: String - ): Deferred { + override fun getAccessTokenAsync(singleScope: String): Deferred { + val token = accessToken?.invoke() + if (token != null) return CompletableDeferred(token) + fail() return CompletableDeferred(null) } @@ -393,6 +661,8 @@ class FxaAccountManagerTest { } override fun getTokenServerEndpointURL(): String { + if (tokenServerEndpointUrl != null) return tokenServerEndpointUrl + fail() return "" } @@ -426,13 +696,10 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = this.coroutineContext ) - var onErrorCalled = false - val accountObserver = object : AccountObserver { override fun onLoggedOut() { fail() @@ -449,20 +716,10 @@ class FxaAccountManagerTest { override fun onProfileUpdated(profile: Profile) { fail() } - - override fun onError(error: Exception) { - assertFalse(onErrorCalled) - onErrorCalled = true - assertTrue(error is FailedToLoadAccountException) - assertEquals(error.cause, readException) - } } manager.register(accountObserver) - manager.initAsync().await() - - assertTrue(onErrorCalled) } @Test @@ -473,8 +730,7 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = this.coroutineContext ) @@ -483,7 +739,6 @@ class FxaAccountManagerTest { manager.register(accountObserver) manager.initAsync().await() - verify(accountObserver, never()).onError(any()) verify(accountObserver, never()).onAuthenticated(any()) verify(accountObserver, never()).onProfileUpdated(any()) verify(accountObserver, never()).onLoggedOut() @@ -511,10 +766,9 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, - emptyList(), this.coroutineContext + emptySet(), null, this.coroutineContext ) val accountObserver: AccountObserver = mock() @@ -524,7 +778,6 @@ class FxaAccountManagerTest { manager.initAsync().await() // Make sure that account and profile observers are fired exactly once. - verify(accountObserver, never()).onError(any()) verify(accountObserver, times(1)).onAuthenticated(mockAccount) verify(accountObserver, times(1)).onProfileUpdated(profile) verify(accountObserver, never()).onLoggedOut() @@ -543,7 +796,6 @@ class FxaAccountManagerTest { verify(constellation, never()).destroyCurrentDeviceAsync() manager.logoutAsync().await() - verify(accountObserver, never()).onError(any()) verify(accountObserver, never()).onAuthenticated(any()) verify(accountObserver, never()).onProfileUpdated(any()) verify(accountObserver, times(1)).onLoggedOut() @@ -583,7 +835,6 @@ class FxaAccountManagerTest { verify(accountStorage, times(1)).read() verify(accountStorage, never()).clear() - verify(accountObserver, never()).onError(any()) verify(accountObserver, times(1)).onAuthenticated(mockAccount) verify(accountObserver, times(1)).onProfileUpdated(profile) verify(accountObserver, never()).onLoggedOut() @@ -635,8 +886,7 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = coroutineContext ) { mockAccount @@ -673,7 +923,6 @@ class FxaAccountManagerTest { verify(accountStorage, times(1)).read() verify(accountStorage, never()).clear() - verify(accountObserver, never()).onError(any()) verify(accountObserver, times(1)).onAuthenticated(mockAccount) verify(accountObserver, times(1)).onProfileUpdated(profile) verify(accountObserver, never()).onLoggedOut() @@ -702,7 +951,6 @@ class FxaAccountManagerTest { assertNull(manager.beginAuthenticationAsync().await()) // Confirm that account state observable doesn't receive authentication errors. - verify(accountObserver, never()).onError(any()) assertNull(manager.authenticatedAccount()) assertNull(manager.accountProfile()) @@ -720,7 +968,6 @@ class FxaAccountManagerTest { verify(accountStorage, times(1)).read() verify(accountStorage, never()).clear() - verify(accountObserver, never()).onError(any()) verify(accountObserver, times(1)).onAuthenticated(mockAccount) verify(accountObserver, times(1)).onProfileUpdated(profile) verify(accountObserver, never()).onLoggedOut() @@ -749,7 +996,6 @@ class FxaAccountManagerTest { assertNull(manager.beginAuthenticationAsync(pairingUrl = "auth://pairing").await()) // Confirm that account state observable doesn't receive authentication errors. - verify(accountObserver, never()).onError(any()) assertNull(manager.authenticatedAccount()) assertNull(manager.accountProfile()) @@ -767,7 +1013,6 @@ class FxaAccountManagerTest { verify(accountStorage, times(1)).read() verify(accountStorage, never()).clear() - verify(accountObserver, never()).onError(any()) verify(accountObserver, times(1)).onAuthenticated(mockAccount) verify(accountObserver, times(1)).onProfileUpdated(profile) verify(accountObserver, never()).onLoggedOut() @@ -888,8 +1133,7 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = this.coroutineContext ) { mockAccount @@ -931,7 +1175,6 @@ class FxaAccountManagerTest { manager.updateProfileAsync().await() verify(accountObserver, times(1)).onProfileUpdated(profile) - verify(accountObserver, never()).onError(any()) verify(accountObserver, never()).onAuthenticated(any()) verify(accountObserver, never()).onLoggedOut() assertEquals(profile, manager.accountProfile()) @@ -962,8 +1205,7 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = this.coroutineContext ) { mockAccount @@ -1019,8 +1261,7 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = this.coroutineContext ) { mockAccount @@ -1069,7 +1310,7 @@ class FxaAccountManagerTest { if (!didFailProfileFetch) { didFailProfileFetch = true authErrorRegistry.notifyObservers { onAuthErrorAsync(AuthException(AuthExceptionType.UNAUTHORIZED)) } - CompletableDeferred(value = null) + CompletableDeferred(value = null) } else { CompletableDeferred(profile) } @@ -1085,8 +1326,7 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = this.coroutineContext ) { mockAccount @@ -1142,8 +1382,7 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = this.coroutineContext ) { mockAccount @@ -1185,8 +1424,7 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = coroutineContext ) { mockAccount @@ -1218,8 +1456,7 @@ class FxaAccountManagerTest { val manager = TestableFxaAccountManager( testContext, - Config.release("dummyId", "bad://url"), - arrayOf("profile", "test-scope"), + ServerConfig.release("dummyId", "bad://url"), accountStorage, coroutineContext = coroutineContext ) { mockAccount diff --git a/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/SyncAuthInfoCacheTest.kt b/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/SyncAuthInfoCacheTest.kt new file mode 100644 index 00000000000..06e3a9f7367 --- /dev/null +++ b/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/SyncAuthInfoCacheTest.kt @@ -0,0 +1,51 @@ +/* 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 androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.concept.sync.SyncAuthInfo +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.assertFalse +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class SyncAuthInfoCacheTest { + @Test + fun `public api`() { + val cache = SyncAuthInfoCache(testContext) + assertNull(cache.getCached()) + + val authInfo = SyncAuthInfo( + kid = "testKid", + fxaAccessToken = "fxaAccess", + // expires in the future + fxaAccessTokenExpiresAt = System.currentTimeMillis() + 60 * 1000L, + syncKey = "long secret key", + tokenServerUrl = "http://www.token.server/url" + ) + + cache.setToCache(authInfo) + + assertEquals(authInfo, cache.getCached()) + assertFalse(cache.expired()) + + val authInfo2 = authInfo.copy( + // expires in the past + fxaAccessTokenExpiresAt = System.currentTimeMillis() - 60 * 1000L + ) + + cache.setToCache(authInfo2) + assertEquals(authInfo2, cache.getCached()) + assertTrue(cache.expired()) + + cache.clear() + + assertNull(cache.getCached()) + } +} \ No newline at end of file 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 74c2b9c0dea..e2415326a16 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 @@ -13,7 +13,7 @@ import kotlinx.coroutines.plus import mozilla.appservices.logins.DatabaseLoginsStorage import mozilla.appservices.logins.LoginsStorage import mozilla.appservices.logins.MemoryLoginsStorage -import mozilla.components.concept.sync.AuthInfo +import mozilla.components.concept.sync.SyncAuthInfo import mozilla.components.concept.sync.SyncStatus import mozilla.components.concept.sync.SyncableStore @@ -369,7 +369,7 @@ data class SyncableLoginsStore( val store: AsyncLoginsStorage, val key: () -> Deferred ) : SyncableStore { - override suspend fun sync(authInfo: AuthInfo): SyncStatus { + override suspend fun sync(authInfo: SyncAuthInfo): SyncStatus { return try { withUnlocked { it.sync(authInfo.into()).await() diff --git a/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/Types.kt b/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/Types.kt index 0850de56468..3744447a00a 100644 --- a/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/Types.kt +++ b/components/service/sync-logins/src/main/java/mozilla/components/service/sync/logins/Types.kt @@ -4,12 +4,12 @@ package mozilla.components.service.sync.logins -import mozilla.components.concept.sync.AuthInfo +import mozilla.components.concept.sync.SyncAuthInfo /** * Conversion from a generic AuthInfo type into a type 'logins' lib uses at the interface boundary. */ -fun AuthInfo.into(): SyncUnlockInfo { +fun SyncAuthInfo.into(): SyncUnlockInfo { return SyncUnlockInfo( kid = this.kid, fxaAccessToken = this.fxaAccessToken, diff --git a/samples/firefox-accounts/src/main/java/org/mozilla/samples/fxa/MainActivity.kt b/samples/firefox-accounts/src/main/java/org/mozilla/samples/fxa/MainActivity.kt index df9e70baea7..b04ff37379c 100644 --- a/samples/firefox-accounts/src/main/java/org/mozilla/samples/fxa/MainActivity.kt +++ b/samples/firefox-accounts/src/main/java/org/mozilla/samples/fxa/MainActivity.kt @@ -22,7 +22,7 @@ import mozilla.components.concept.sync.Profile import mozilla.components.feature.qr.QrFeature import mozilla.components.service.fxa.FirefoxAccount import mozilla.components.service.fxa.FxaException -import mozilla.components.service.fxa.Config +import mozilla.components.service.fxa.ServerConfig import mozilla.components.support.base.log.Log import mozilla.components.support.base.log.sink.AndroidLogSink import kotlin.coroutines.CoroutineContext @@ -125,7 +125,7 @@ open class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteList return it } - val config = Config(CONFIG_URL, CLIENT_ID, REDIRECT_URL) + val config = ServerConfig(CONFIG_URL, CLIENT_ID, REDIRECT_URL) return FirefoxAccount(config) } diff --git a/samples/sync-logins/build.gradle b/samples/sync-logins/build.gradle index 493b4e72898..23238f0d007 100644 --- a/samples/sync-logins/build.gradle +++ b/samples/sync-logins/build.gradle @@ -39,7 +39,6 @@ dependencies { implementation project(':concept-storage') implementation project(':service-firefox-accounts') implementation project(':service-sync-logins') - implementation project(':feature-sync') implementation Dependencies.kotlin_stdlib 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 3278f8200e3..3e5037f49da 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 @@ -20,13 +20,13 @@ import mozilla.components.concept.sync.AccountObserver import mozilla.components.concept.sync.DeviceType import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.Profile -import mozilla.components.concept.sync.SyncStatusObserver -import mozilla.components.feature.sync.BackgroundSyncManager -import mozilla.components.feature.sync.GlobalSyncableStoreProvider -import mozilla.components.service.fxa.Config import mozilla.components.service.fxa.FirefoxAccount -import mozilla.components.service.fxa.manager.DeviceTuple import mozilla.components.service.fxa.manager.FxaAccountManager +import mozilla.components.service.fxa.DeviceConfig +import mozilla.components.service.fxa.ServerConfig +import mozilla.components.service.fxa.SyncConfig +import mozilla.components.service.fxa.sync.GlobalSyncableStoreProvider +import mozilla.components.service.fxa.sync.SyncStatusObserver import mozilla.components.service.sync.logins.AsyncLoginsStorageAdapter import mozilla.components.service.sync.logins.SyncableLoginsStore import mozilla.components.support.base.log.Log @@ -46,11 +46,8 @@ class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener, } } - private val syncManager by lazy { + init { GlobalSyncableStoreProvider.configureStore("logins" to loginsStorage) - BackgroundSyncManager("https://identity.mozilla.com/apps/oldsync").also { - it.addStore("logins") - } } private lateinit var listView: ListView @@ -60,10 +57,9 @@ class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener, private val accountManager by lazy { FxaAccountManager( applicationContext, - Config.release(CLIENT_ID, REDIRECT_URL), - arrayOf("profile", "https://identity.mozilla.com/apps/oldsync"), - DeviceTuple("A-C Logins Sync Sample", DeviceType.MOBILE, listOf()), - syncManager + ServerConfig.release(CLIENT_ID, REDIRECT_URL), + DeviceConfig("A-C Logins Sync Sample", DeviceType.MOBILE, setOf()), + SyncConfig(setOf("logins")) ) } @@ -79,7 +75,8 @@ class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener, setContentView(R.layout.activity_main) job = Job() - syncManager.register(this, this) + // Observe sync state changes. + accountManager.registerForSyncEvents(observer = this, owner = this, autoPause = true) listView = findViewById(R.id.logins_list_view) adapter = ArrayAdapter(this, android.R.layout.simple_list_item_1) @@ -108,7 +105,7 @@ class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener, override fun onLoggedOut() {} override fun onAuthenticated(account: OAuthAccount) { - syncManager.syncNow() + accountManager.syncNowAsync() } @Suppress("EmptyFunctionBlock") @@ -119,12 +116,6 @@ class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener, Toast.makeText(this@MainActivity, "Account auth problem", Toast.LENGTH_LONG).show() } } - - override fun onError(error: Exception) { - launch { - Toast.makeText(this@MainActivity, "Account error: $error", Toast.LENGTH_LONG).show() - } - } } override fun onDestroy() { diff --git a/samples/sync/build.gradle b/samples/sync/build.gradle index 6ac0ab15783..2c9fac47772 100644 --- a/samples/sync/build.gradle +++ b/samples/sync/build.gradle @@ -39,7 +39,6 @@ dependencies { implementation project(':concept-storage') implementation project(':browser-storage-sync') implementation project(':service-firefox-accounts') - implementation project(':feature-sync') implementation Dependencies.androidx_recyclerview } 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 97ba6c35ac9..3439d5db6bf 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 @@ -29,12 +29,12 @@ import mozilla.components.concept.sync.DeviceType import mozilla.components.concept.sync.DeviceEventsObserver import mozilla.components.concept.sync.OAuthAccount import mozilla.components.concept.sync.Profile -import mozilla.components.concept.sync.SyncStatusObserver import mozilla.components.service.fxa.manager.FxaAccountManager -import mozilla.components.service.fxa.Config -import mozilla.components.feature.sync.BackgroundSyncManager -import mozilla.components.feature.sync.GlobalSyncableStoreProvider -import mozilla.components.service.fxa.manager.DeviceTuple +import mozilla.components.service.fxa.DeviceConfig +import mozilla.components.service.fxa.ServerConfig +import mozilla.components.service.fxa.SyncConfig +import mozilla.components.service.fxa.sync.GlobalSyncableStoreProvider +import mozilla.components.service.fxa.sync.SyncStatusObserver import mozilla.components.support.base.log.Log import mozilla.components.support.base.log.sink.AndroidLogSink import java.lang.Exception @@ -53,26 +53,21 @@ class MainActivity : PlacesBookmarksStorage(this) } - private val syncManager by lazy { + init { GlobalSyncableStoreProvider.configureStore("history" to historyStorage) GlobalSyncableStoreProvider.configureStore("bookmarks" to bookmarksStorage) - BackgroundSyncManager("https://identity.mozilla.com/apps/oldsync").also { - it.addStore("history") - it.addStore("bookmarks") - } } private val accountManager by lazy { FxaAccountManager( this, - Config.release(CLIENT_ID, REDIRECT_URL), - arrayOf("profile", "https://identity.mozilla.com/apps/oldsync"), - DeviceTuple( + ServerConfig.release(CLIENT_ID, REDIRECT_URL), + DeviceConfig( name = "A-C Sync Sample - ${System.currentTimeMillis()}", type = DeviceType.MOBILE, - capabilities = listOf(DeviceCapability.SEND_TAB) + capabilities = setOf(DeviceCapability.SEND_TAB) ), - syncManager + SyncConfig(setOf("history", "bookmarks"), syncPeriodInMinutes = 15L) ) } @@ -139,16 +134,18 @@ class MainActivity : // NB: ObserverRegistry takes care of unregistering this observer when appropriate, and // cleaning up any internal references to 'observer' and 'owner'. - syncManager.register(syncObserver, owner = this, autoPause = true) // Observe changes to the account and profile. accountManager.register(accountObserver, owner = this, autoPause = true) + // Observe sync state changes. + accountManager.registerForSyncEvents(syncObserver, owner = this, autoPause = true) + // Observe incoming device events. accountManager.registerForDeviceEvents(deviceEventsObserver, owner = this, autoPause = true) // Now that our account state observer is registered, we can kick off the account manager. launch { accountManager.initAsync().await() } findViewById(R.id.buttonSync).setOnClickListener { - syncManager.syncNow() + accountManager.syncNowAsync() } } @@ -286,13 +283,6 @@ class MainActivity : ) } } - - override fun onError(error: Exception) { - launch { - val txtView: TextView = findViewById(R.id.fxaStatusView) - txtView.text = getString(R.string.account_error, error.toString()) - } - } } private val syncObserver = object : SyncStatusObserver {