From e07643519137f285bfa856ff64bb2f7863f41a26 Mon Sep 17 00:00:00 2001 From: mike a Date: Tue, 15 Mar 2022 22:59:38 -0700 Subject: [PATCH] [fenix] Closes https://github.com/mozilla-mobile/fenix/issues/24276: moved getHistory() to suspend --- .../history/PagedHistoryProvider.kt | 55 +++++++++---------- .../history/PagedHistoryProviderTest.kt | 9 +-- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt index 0d72a5f187..744a6b44d6 100644 --- a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt +++ b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt @@ -14,7 +14,6 @@ import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.library.history.History import org.mozilla.fenix.library.history.HistoryItemTimeGroup -import org.mozilla.fenix.perf.runBlockingIncrement import org.mozilla.fenix.utils.Settings.Companion.SEARCH_GROUP_MINIMUM_SITES private const val BUFFER_TIME = 15000 /* 15 seconds in ms */ @@ -78,7 +77,7 @@ interface PagedHistoryProvider { * @param numberOfItems How many items to fetch * @return list of [HistoryDB] */ - fun getHistory(offset: Int, numberOfItems: Int): List + suspend fun getHistory(offset: Int, numberOfItems: Int): List } /** @@ -112,38 +111,36 @@ class DefaultPagedHistoryProvider( @Volatile private var historyGroups: List? = null - override fun getHistory( + override suspend fun getHistory( offset: Int, numberOfItems: Int ): List { - return runBlockingIncrement { - // We need to re-fetch all the history metadata if the offset resets back at 0 - // in the case of a pull to refresh. - if (historyGroups == null || offset == 0) { - historyGroups = historyStorage.getHistoryMetadataSince(Long.MIN_VALUE) - .asSequence() - .sortedByDescending { it.createdAt } - .filter { it.key.searchTerm != null } - .groupBy { it.key.searchTerm!! } - .map { (searchTerm, items) -> - HistoryDB.Group( - title = searchTerm, - visitedAt = items.first().createdAt, - items = items.map { it.toHistoryDBMetadata() } - ) - } - .filter { - if (historyImprovementFeatures) { - it.items.size >= SEARCH_GROUP_MINIMUM_SITES - } else { - true - } + // We need to re-fetch all the history metadata if the offset resets back at 0 + // in the case of a pull to refresh. + if (historyGroups == null || offset == 0) { + historyGroups = historyStorage.getHistoryMetadataSince(Long.MIN_VALUE) + .asSequence() + .sortedByDescending { it.createdAt } + .filter { it.key.searchTerm != null } + .groupBy { it.key.searchTerm!! } + .map { (searchTerm, items) -> + HistoryDB.Group( + title = searchTerm, + visitedAt = items.first().createdAt, + items = items.map { it.toHistoryDBMetadata() } + ) + } + .filter { + if (historyImprovementFeatures) { + it.items.size >= SEARCH_GROUP_MINIMUM_SITES + } else { + true } - .toList() - } - - getHistoryAndSearchGroups(offset, numberOfItems) + } + .toList() } + + return getHistoryAndSearchGroups(offset, numberOfItems) } /** diff --git a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt index c2ce51ea17..8366166f5d 100644 --- a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt @@ -7,6 +7,7 @@ package org.mozilla.fenix.components.history import io.mockk.coEvery import io.mockk.coVerify import io.mockk.mockk +import kotlinx.coroutines.test.runBlockingTest import mozilla.components.browser.storage.sync.PlacesHistoryStorage import mozilla.components.concept.storage.DocumentType import mozilla.components.concept.storage.HistoryMetadata @@ -27,7 +28,7 @@ class PagedHistoryProviderTest { } @Test - fun `getHistory uses getVisitsPaginated`() { + fun `getHistory uses getVisitsPaginated`() = runBlockingTest { val provider = DefaultPagedHistoryProvider( historyStorage = storage, historyImprovementFeatures = false, @@ -144,7 +145,7 @@ class PagedHistoryProviderTest { } @Test - fun `history metadata matching lower bound`() { + fun `history metadata matching lower bound`() = runBlockingTest { val provider = DefaultPagedHistoryProvider( historyStorage = storage, historyImprovementFeatures = false, @@ -214,7 +215,7 @@ class PagedHistoryProviderTest { } @Test - fun `history metadata matching upper bound`() { + fun `history metadata matching upper bound`() = runBlockingTest { val provider = DefaultPagedHistoryProvider( historyStorage = storage, historyImprovementFeatures = false, @@ -284,7 +285,7 @@ class PagedHistoryProviderTest { } @Test - fun `redirects are filtered out from history metadata groups`() { + fun `redirects are filtered out from history metadata groups`() = runBlockingTest { val provider = DefaultPagedHistoryProvider( historyStorage = storage, historyImprovementFeatures = false,