Skip to content

Commit

Permalink
For mozilla-mobile#12469 - Allow for multiple PlacesApi readers.
Browse files Browse the repository at this point in the history
Multiple readers allow to start and cancel history suggestions requests without
any impact on other in-progress PlacesApi queries.
  • Loading branch information
Mugurell committed Jul 14, 2022
1 parent a8cdec1 commit 2a7132b
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,26 @@ const val DB_NAME = "places.sqlite"
internal interface Connection : Closeable {
fun registerWithSyncManager()

/**
* Allows to read history, bookmarks, and other data from persistent storage.
* All calls on the same reader are queued. Multiple readers are allowed.
*/
fun reader(): PlacesReaderConnection

/**
* Create a new reader for history, bookmarks and other data from persistent storage.
* Allows for disbanded calls from the default reader from [reader] and so being able to
* easily start and cancel any data requests without impacting others using another reader.
*
* All [PlacesApi] requests are synchronized at lower levels so even with using multiple readers
* all requests are ordered with concurrency not allowed.
*/
fun newReader(): PlacesReaderConnection

/**
* Allowed to add history, bookmarks and other data to persistent storage.
* All calls are queued and synchronized at lower levels. Only one writer is recommended.
*/
fun writer(): PlacesWriterConnection

// Until we get a real SyncManager in application-services libraries, we'll have to live with this
Expand Down Expand Up @@ -87,6 +106,12 @@ internal object RustPlacesConnection : Connection {
return cachedReader!!
}

override fun newReader(): PlacesReaderConnection = synchronized(this) {
val api = safeGetApi()
check(api != null) { "must call init first" }
return api.openReader()
}

override fun writer(): PlacesWriterConnection {
val api = safeGetApi()
check(api != null) { "must call init first" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,24 @@ abstract class PlacesStorage(

abstract val logger: Logger

@VisibleForTesting
@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
internal open val places: Connection by lazy {
RustPlacesConnection.init(storageDir)
RustPlacesConnection
}

/**
* Default writer for adding history, bookmarks and other data to persistent storage.
*/
@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
internal val writer: PlacesWriterConnection by lazy { places.writer() }
internal val reader: PlacesReaderConnection by lazy { places.reader() }

/**
* Default reader for querying history, bookmarks and other data from persistent storage.
* Override to use a different instance if special calls synchronization / cancellation is needed.
*/
@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
internal open val reader: PlacesReaderConnection by lazy { places.reader() }

override suspend fun warmUp() {
logElapsedTime(logger, "Warming up places storage") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,11 @@ class PlacesHistoryStorageTest {
return mock()
}

override fun newReader(): PlacesReaderConnection {
fail()
return mock()
}

override fun writer(): PlacesWriterConnection {
fail()
return mock()
Expand Down Expand Up @@ -654,6 +659,11 @@ class PlacesHistoryStorageTest {
return mock()
}

override fun newReader(): PlacesReaderConnection {
fail()
return mock()
}

override fun writer(): PlacesWriterConnection {
fail()
return mock()
Expand Down Expand Up @@ -702,6 +712,11 @@ class PlacesHistoryStorageTest {
return mock()
}

override fun newReader(): PlacesReaderConnection {
fail()
return mock()
}

override fun writer(): PlacesWriterConnection {
fail()
return mock()
Expand Down Expand Up @@ -757,6 +772,11 @@ class PlacesHistoryStorageTest {
return mock()
}

override fun newReader(): PlacesReaderConnection {
fail()
return mock()
}

override fun writer(): PlacesWriterConnection {
fail()
return mock()
Expand Down

0 comments on commit 2a7132b

Please sign in to comment.