Issue #11654: Change fetchProvidedTopSites into TopSitesProviderConfig

README.md

@@ -201,6 +201,8 @@ _Components and libraries to interact with backend services._
 
 * 🔴 [**Pocket**](components/service/pocket/README.md) - A library for communicating with the Pocket API.
 
+* 🔴 [**Contile**](components/service/contile/README.md) - A library for communicating with the Contile services API.
+
 ## Support
 
 _Supporting components with generic helper code._

components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt

@@ -24,7 +24,7 @@ import kotlin.coroutines.CoroutineContext
  * @param historyStorage An instance of [PlacesHistoryStorage], used for retrieving top frecent
  * sites from history.
  * @param topSitesProvider An optional instance of [TopSitesProvider], used for retrieving
- * additional top sites from a provider.
+ * additional top sites from a provider. The returned top sites are added before pinned sites.
  * @param defaultTopSites A list containing a title to url pair of default top sites to be added
  * to the [PinnedSiteStorage].
  */
@@ -83,28 +83,34 @@ class DefaultTopSitesStorage(
         }
     }
 
-    @Suppress("TooGenericExceptionCaught")
+    @Suppress("ComplexCondition", "TooGenericExceptionCaught")
     override suspend fun getTopSites(
         totalSites: Int,
-        fetchProvidedTopSites: Boolean,
-        frecencyConfig: FrecencyThresholdOption?
+        frecencyConfig: FrecencyThresholdOption?,
+        providerConfig: TopSitesProviderConfig?
     ): List<TopSite> {
         val topSites = ArrayList<TopSite>()
         val pinnedSites = pinnedSitesStorage.getPinnedSites().take(totalSites)
         var numSitesRequired = totalSites - pinnedSites.size
 
-        topSites.addAll(pinnedSites)
-
-        if (fetchProvidedTopSites && topSitesProvider != null) {
+        if (topSitesProvider != null &&
+            providerConfig != null &&
+            providerConfig.showProviderTopSites &&
+            pinnedSites.size < providerConfig.maxThreshold
+        ) {
             try {
-                val providerTopSites = topSitesProvider.getTopSites()
-                topSites.addAll(providerTopSites.take(numSitesRequired))
+                val providerTopSites = topSitesProvider
+                    .getTopSites()
+                    .take(numSitesRequired)
+                topSites.addAll(providerTopSites)
                 numSitesRequired -= providerTopSites.size
             } catch (e: Exception) {
                 logger.error("Failed to fetch top sites from provider", e)
             }
         }
 
+        topSites.addAll(pinnedSites)
+
         if (frecencyConfig != null && numSitesRequired > 0) {
             // Get 'totalSites' sites for duplicate entries with
             // existing pinned sites

components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesConfig.kt

@@ -11,13 +11,26 @@ import mozilla.components.concept.storage.FrecencyThresholdOption
  * whether or not to include top frecent sites in the top sites feature.
  *
  * @property totalSites A total number of sites that will be displayed.
- * @property fetchProvidedTopSites Whether or not to fetch top sites from the [TopSitesProvider].
  * @property frecencyConfig If [frecencyConfig] is specified, only visited sites with a frecency
  * score above the given threshold will be returned. Otherwise, frecent top site results are
  * not included.
+ * @property providerConfig An instance of [TopSitesProviderConfig] that specifies whether or
+ * not to fetch top sites from the [TopSitesProvider].
  */
 data class TopSitesConfig(
     val totalSites: Int,
-    val fetchProvidedTopSites: Boolean,
-    val frecencyConfig: FrecencyThresholdOption?
+    val frecencyConfig: FrecencyThresholdOption?,
+    val providerConfig: TopSitesProviderConfig?
+)
+
+/**
+ * Top sites provider configuration to specify whether or not to fetch top sites from the provider.
+ *
+ * @property showProviderTopSites Whether or not to display the top sites from the provider.
+ * @property maxThreshold Only fetch the top sites from the provider if the number of top sites are
+ * below the maximum threshold.
+ */
+data class TopSitesProviderConfig(
+    val showProviderTopSites: Boolean,
+    val maxThreshold: Int = Int.MAX_VALUE
 )

components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSitesStorage.kt

@@ -43,15 +43,16 @@ interface TopSitesStorage : Observable<TopSitesStorage.Observer> {
      * If `frecencyConfig` is specified, fill in any missing top sites with frecent top site results.
      *
      * @param totalSites A total number of sites that will be retrieve if possible.
-     * @param fetchProvidedTopSites Whether or not to fetch top sites from the [TopSitesProvider].
      * @param frecencyConfig If [frecencyConfig] is specified, only visited sites with a frecency
      * score above the given threshold will be returned. Otherwise, frecent top site results are
      * not included.
+     * @param providerConfig An instance of [TopSitesProviderConfig] that specifies whether or
+     * not to fetch top sites from the [TopSitesProvider].
      */
     suspend fun getTopSites(
         totalSites: Int,
-        fetchProvidedTopSites: Boolean,
-        frecencyConfig: FrecencyThresholdOption?
+        frecencyConfig: FrecencyThresholdOption? = null,
+        providerConfig: TopSitesProviderConfig? = null
     ): List<TopSite>
 
     /**

components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/presenter/DefaultTopSitesPresenter.kt

@@ -45,9 +45,9 @@ internal class DefaultTopSitesPresenter(
 
         scope.launch {
             val topSites = storage.getTopSites(
-                innerConfig.totalSites,
-                innerConfig.fetchProvidedTopSites,
-                innerConfig.frecencyConfig
+                totalSites = innerConfig.totalSites,
+                frecencyConfig = innerConfig.frecencyConfig,
+                providerConfig = innerConfig.providerConfig
             )
 
             scope.launch(Dispatchers.Main) {