Commit 86baa3a7 authored by Grisha Kruglov's avatar Grisha Kruglov Committed by Grisha Kruglov
Browse files

Part 3: Remove generics from StorageSync

Now that we own a unified type definitions at the concept level,
we can provide type conversions when necessary and remove the
generics abstraction.
parent a0aa405e
......@@ -20,10 +20,10 @@ import mozilla.components.concept.sync.SyncOk
import mozilla.components.concept.sync.SyncStatus
import mozilla.components.concept.sync.SyncableStore
import mozilla.components.concept.storage.VisitType
import mozilla.components.concept.sync.AuthInfo
import mozilla.components.support.utils.segmentAwareDomainMatch
import org.mozilla.places.PlacesConnection
import org.mozilla.places.PlacesException
import org.mozilla.places.SyncAuthInfo
import org.mozilla.places.VisitObservation
const val AUTOCOMPLETE_SOURCE_NAME = "placesHistory"
......@@ -33,7 +33,7 @@ typealias SyncAuthInfo = org.mozilla.places.SyncAuthInfo
/**
* Implementation of the [HistoryStorage] which is backed by a Rust Places lib via [PlacesConnection].
*/
open class PlacesHistoryStorage(context: Context) : HistoryStorage, SyncableStore<SyncAuthInfo> {
open class PlacesHistoryStorage(context: Context) : HistoryStorage, SyncableStore {
private val scope by lazy { CoroutineScope(Dispatchers.IO) }
private val storageDir by lazy { context.filesDir }
......@@ -45,7 +45,7 @@ open class PlacesHistoryStorage(context: Context) : HistoryStorage, SyncableStor
override suspend fun recordVisit(uri: String, visitType: VisitType) {
scope.launch {
places.api().noteObservation(VisitObservation(uri, visitType = visitType.toPlacesType()))
places.api().noteObservation(VisitObservation(uri, visitType = visitType.into()))
}.join()
}
......@@ -103,27 +103,14 @@ open class PlacesHistoryStorage(context: Context) : HistoryStorage, SyncableStor
}
}
override suspend fun sync(authInfo: SyncAuthInfo): SyncStatus {
override suspend fun sync(authInfo: AuthInfo): SyncStatus {
return RustPlacesConnection.newConnection(storageDir).use {
try {
it.sync(authInfo)
it.sync(authInfo.into())
SyncOk
} catch (e: PlacesException) {
SyncError(e)
}
}
}
/**
* We have an internal visit type definition defined at the concept level, and an external type
* defined within Places. In practice these two types are the same, with the Places one being a
* little richer.
*/
private fun VisitType.toPlacesType(): org.mozilla.places.VisitType {
return when (this) {
VisitType.LINK -> org.mozilla.places.VisitType.LINK
VisitType.RELOAD -> org.mozilla.places.VisitType.RELOAD
VisitType.TYPED -> org.mozilla.places.VisitType.TYPED
}
}
}
/* 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.browser.storage.sync
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.
*/
internal fun AuthInfo.into(): SyncAuthInfo {
return SyncAuthInfo(
kid = this.kid,
fxaAccessToken = this.fxaAccessToken,
syncKey = this.syncKey,
tokenserverURL = this.tokenServerUrl
)
}
/**
* Conversion from a generic [VisitType] into its richer comrade within the 'places' lib.
*/
internal fun VisitType.into(): org.mozilla.places.VisitType {
return when (this) {
VisitType.LINK -> org.mozilla.places.VisitType.LINK
VisitType.RELOAD -> org.mozilla.places.VisitType.RELOAD
VisitType.TYPED -> org.mozilla.places.VisitType.TYPED
}
}
......@@ -9,7 +9,7 @@ import java.lang.Exception
/**
* Describes a "sync" entry point for store which operates over [AuthInfo].
*/
interface SyncableStore<AuthInfo> {
interface SyncableStore {
/**
* Performs a sync.
* @param authInfo Auth information of type [AuthInfo] necessary for syncing this store.
......
......@@ -13,6 +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.SyncError
import mozilla.components.concept.sync.SyncOk
import mozilla.components.concept.sync.SyncStatus
......@@ -369,11 +370,11 @@ open class AsyncLoginsStorageAdapter<T : LoginsStorage>(private val wrapped: T)
data class SyncableLoginsStore(
val store: AsyncLoginsStorage,
val key: () -> Deferred<String>
) : SyncableStore<SyncUnlockInfo> {
override suspend fun sync(authInfo: SyncUnlockInfo): SyncStatus {
) : SyncableStore {
override suspend fun sync(authInfo: AuthInfo): SyncStatus {
return try {
withUnlocked {
it.sync(authInfo).await()
it.sync(authInfo.into()).await()
SyncOk
}
} catch (e: LoginsStorageException) {
......
/* 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.sync.logins
import mozilla.components.concept.sync.AuthInfo
/**
* Conversion from a generic AuthInfo type into a type 'logins' lib uses at the interface boundary.
*/
fun AuthInfo.into(): SyncUnlockInfo {
return SyncUnlockInfo(
kid = this.kid,
fxaAccessToken = this.fxaAccessToken,
syncKey = this.syncKey,
tokenserverURL = this.tokenServerUrl
)
}
......@@ -21,21 +21,11 @@ import mozilla.components.support.base.observer.ObserverRegistry
/**
* A feature implementation which orchestrates data synchronization of a set of [SyncableStore] which
* all share a common [AuthType].
*
* [AuthType] provides us with a layer of indirection that allows consumers of [StorageSync]
* to use entirely different types of [SyncableStore], without this feature needing to depend on
* their specific implementations. Those implementations might have heavy native dependencies
* (e.g. places and logins depend on native libraries), and we do not want to force a consumer which
* only cares about syncing logins to have to import a places native library.
*
* @param reifyAuth A conversion method which reifies a generic [AuthInfo] into an object of
* type [AuthType].
* all share a common, generic [AuthInfo].
*/
class StorageSync<AuthType>(
private val syncableStores: Map<String, SyncableStore<AuthType>>,
private val syncScope: String,
private val reifyAuth: suspend (authInfo: AuthInfo) -> AuthType
class StorageSync(
private val syncableStores: Map<String, SyncableStore>,
private val syncScope: String
) : Observable<SyncStatusObserver> by ObserverRegistry() {
private val logger = Logger("StorageSync")
......@@ -60,8 +50,8 @@ class StorageSync<AuthType>(
val results = mutableMapOf<String, StoreSyncStatus>()
val reifiedAuthInfo = try {
reifyAuth(account.authInfo(syncScope))
val authInfo = try {
account.authInfo(syncScope)
} catch (e: AuthException) {
syncableStores.keys.forEach { storeName ->
results[storeName] = StoreSyncStatus(SyncError(e))
......@@ -70,18 +60,18 @@ class StorageSync<AuthType>(
}
syncableStores.keys.forEach { storeName ->
results[storeName] = syncStore(syncableStores.getValue(storeName), storeName, reifiedAuthInfo)
results[storeName] = syncStore(syncableStores.getValue(storeName), storeName, authInfo)
}
return@withListeners results
} }
private suspend fun syncStore(
store: SyncableStore<AuthType>,
storeName: String,
account: AuthType
store: SyncableStore,
storeName: String,
auth: AuthInfo
): StoreSyncStatus {
return StoreSyncStatus(store.sync(account).also {
return StoreSyncStatus(store.sync(auth).also {
if (it is SyncError) {
logger.error("Error synchronizing a $storeName store", it.exception)
} else {
......
......@@ -32,7 +32,7 @@ class StorageSyncTest {
@Test
fun `sync with no stores`() = runBlocking {
val feature = StorageSync(mapOf(), "sync-scope") { it }
val feature = StorageSync(mapOf(), "sync-scope")
val results = feature.sync(mock())
assertTrue(results.isEmpty())
}
......@@ -49,8 +49,8 @@ class StorageSyncTest {
`when`(mockAccount.getAccessToken(any())).thenReturn(CompletableDeferred((mockAccessTokenInfo)))
// Single store, different result types.
val testStore: SyncableStore<AuthInfo> = mock()
val feature = StorageSync(mapOf("testStore" to testStore), "sync-scope") { it }
val testStore: SyncableStore = mock()
val feature = StorageSync(mapOf("testStore" to testStore), "sync-scope")
`when`(testStore.sync(any())).thenReturn(SyncOk)
var results = feature.sync(mockAccount)
......@@ -65,12 +65,12 @@ class StorageSyncTest {
assertEquals(1, results.size)
// Multiple stores, different result types.
val anotherStore: SyncableStore<AuthInfo> = mock()
val anotherStore: SyncableStore = mock()
val anotherFeature = StorageSync(mapOf(
Pair("testStore", testStore),
Pair("goodStore", anotherStore)),
"sync-scope"
) { it }
)
`when`(anotherStore.sync(any())).thenReturn(SyncOk)
......@@ -105,7 +105,7 @@ class StorageSyncTest {
}
// A store that runs verifications during a sync.
val testStore = object : SyncableStore<AuthInfo> {
val testStore = object : SyncableStore {
override suspend fun sync(authInfo: AuthInfo): SyncStatus {
verifier.verify()
return SyncOk
......@@ -113,7 +113,7 @@ class StorageSyncTest {
}
val syncStatusObserver: SyncStatusObserver = mock()
val feature = StorageSync(mapOf("testStore" to testStore), "sync-scope") { it }
val feature = StorageSync(mapOf("testStore" to testStore), "sync-scope")
// These assertions will run while sync is in progress.
verifier.addVerifyBlock {
......
......@@ -15,7 +15,6 @@ import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.launch
import mozilla.components.browser.storage.sync.PlacesHistoryStorage
import mozilla.components.browser.storage.sync.SyncAuthInfo
import mozilla.components.concept.sync.AccountObserver
import mozilla.components.concept.sync.OAuthAccount
import mozilla.components.concept.sync.Profile
......@@ -46,14 +45,7 @@ class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteListener,
StorageSync(
syncableStores = mapOf(historyStoreName to historyStorage),
syncScope = "https://identity.mozilla.com/apps/oldsync"
) { authInfo ->
SyncAuthInfo(
fxaAccessToken = authInfo.fxaAccessToken,
kid = authInfo.kid,
syncKey = authInfo.syncKey,
tokenserverURL = authInfo.tokenServerUrl
)
}
)
}
private var job = Job()
......
......@@ -27,7 +27,6 @@ import mozilla.components.service.fxa.FxaException
import mozilla.components.service.sync.StorageSync
import mozilla.components.service.sync.logins.AsyncLoginsStorageAdapter
import mozilla.components.service.sync.logins.SyncableLoginsStore
import mozilla.components.service.sync.logins.SyncUnlockInfo
import mozilla.components.support.base.log.Log
import mozilla.components.support.base.log.sink.AndroidLogSink
import java.io.File
......@@ -50,14 +49,7 @@ open class MainActivity : AppCompatActivity(), LoginFragment.OnLoginCompleteList
StorageSync(
syncableStores = mapOf(Pair(loginsStoreName, loginsStore)),
syncScope = "https://identity.mozilla.com/apps/oldsync"
) { authInfo ->
SyncUnlockInfo(
fxaAccessToken = authInfo.fxaAccessToken,
kid = authInfo.kid,
syncKey = authInfo.syncKey,
tokenserverURL = authInfo.tokenServerUrl
)
}
)
}
private lateinit var listView: ListView
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment