Issue #11483: Implement a ContileTopSitesProvider

.buildconfig.yml

@@ -303,6 +303,10 @@ projects:
     path: components/service/pocket
     description: 'A library to communicate with the Pocket API'
     publish: true
+  service-contile:
+    path: components/service/contile
+    description: 'A library to communicate with the Contile services API'
+    publish: true
   support-base:
     path: components/support/base
     description: 'Base component containing building blocks for components.'

components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/OnDevicePinnedSitesStorageTest.kt → components/feature/top-sites/src/androidTest/java/mozilla/components/feature/top/sites/PinnedSitesStorageTest.kt

@@ -12,13 +12,12 @@ import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
 import androidx.test.core.app.ApplicationProvider
 import androidx.test.platform.app.InstrumentationRegistry
 import kotlinx.coroutines.runBlocking
-import mozilla.components.feature.top.sites.TopSite.Type.DEFAULT
-import mozilla.components.feature.top.sites.TopSite.Type.PINNED
 import mozilla.components.feature.top.sites.db.Migrations
 import mozilla.components.feature.top.sites.db.TopSiteDatabase
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -28,7 +27,7 @@ import java.util.concurrent.Executors
 private const val MIGRATION_TEST_DB = "migration-test"
 
 @Suppress("LargeClass")
-class OnDevicePinnedSitesStorageTest {
+class PinnedSitesStorageTest {
     private lateinit var context: Context
     private lateinit var storage: PinnedSiteStorage
     private lateinit var executor: ExecutorService
@@ -77,16 +76,16 @@ class OnDevicePinnedSitesStorageTest {
 
         assertEquals("Mozilla", topSites[0].title)
         assertEquals("https://www.mozilla.org", topSites[0].url)
-        assertEquals(DEFAULT, topSites[0].type)
+        assertTrue(topSites[0] is TopSite.Default)
         assertEquals("Firefox", topSites[1].title)
         assertEquals("https://www.firefox.com", topSites[1].url)
-        assertEquals(DEFAULT, topSites[2].type)
+        assertTrue(topSites[1] is TopSite.Default)
         assertEquals("Wikipedia", topSites[2].title)
         assertEquals("https://www.wikipedia.com", topSites[2].url)
-        assertEquals(DEFAULT, topSites[2].type)
+        assertTrue(topSites[2] is TopSite.Default)
         assertEquals("Pocket", topSites[3].title)
         assertEquals("https://www.getpocket.com", topSites[3].url)
-        assertEquals(DEFAULT, topSites[3].type)
+        assertTrue(topSites[3] is TopSite.Default)
     }
 
     @Test
@@ -101,10 +100,10 @@ class OnDevicePinnedSitesStorageTest {
 
         assertEquals("Mozilla", topSites[0].title)
         assertEquals("https://www.mozilla.org", topSites[0].url)
-        assertEquals(PINNED, topSites[0].type)
+        assertTrue(topSites[0] is TopSite.Pinned)
         assertEquals("Firefox", topSites[1].title)
         assertEquals("https://www.firefox.com", topSites[1].url)
-        assertEquals(DEFAULT, topSites[1].type)
+        assertTrue(topSites[1] is TopSite.Pinned)
     }
 
     @Test
@@ -142,13 +141,13 @@ class OnDevicePinnedSitesStorageTest {
         with(topSites[0]) {
             assertEquals("Mozilla", title)
             assertEquals("https://www.mozilla.org", url)
-            assertEquals(PINNED, type)
+            assertTrue(this is TopSite.Pinned)
         }
 
         with(topSites[1]) {
             assertEquals("Firefox", title)
             assertEquals("https://www.firefox.com", url)
-            assertEquals(DEFAULT, type)
+            assertTrue(this is TopSite.Default)
         }
     }
 

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

@@ -9,10 +9,10 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
 import mozilla.components.browser.storage.sync.PlacesHistoryStorage
 import mozilla.components.concept.storage.FrecencyThresholdOption
-import mozilla.components.feature.top.sites.TopSite.Type.FRECENT
 import mozilla.components.feature.top.sites.ext.hasUrl
 import mozilla.components.feature.top.sites.ext.toTopSite
 import mozilla.components.feature.top.sites.facts.emitTopSitesCountFact
+import mozilla.components.support.base.log.logger.Logger
 import mozilla.components.support.base.observer.Observable
 import mozilla.components.support.base.observer.ObserverRegistry
 import kotlin.coroutines.CoroutineContext
@@ -23,17 +23,21 @@ import kotlin.coroutines.CoroutineContext
  * @param pinnedSitesStorage An instance of [PinnedSiteStorage], used for storing pinned sites.
  * @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.
  * @param defaultTopSites A list containing a title to url pair of default top sites to be added
  * to the [PinnedSiteStorage].
  */
 class DefaultTopSitesStorage(
     private val pinnedSitesStorage: PinnedSiteStorage,
     private val historyStorage: PlacesHistoryStorage,
+    private val topSitesProvider: TopSitesProvider? = null,
     private val defaultTopSites: List<Pair<String, String>> = listOf(),
     coroutineContext: CoroutineContext = Dispatchers.IO
 ) : TopSitesStorage, Observable<TopSitesStorage.Observer> by ObserverRegistry() {
 
     private var scope = CoroutineScope(coroutineContext)
+    private val logger = Logger("DefaultTopSitesStorage")
 
     // Cache of the last retrieved top sites
     var cachedTopSites = listOf<TopSite>()
@@ -55,37 +59,51 @@ class DefaultTopSitesStorage(
 
     override fun removeTopSite(topSite: TopSite) {
         scope.launch {
-            if (topSite.type != FRECENT) {
+            if (topSite is TopSite.Default || topSite is TopSite.Pinned) {
                 pinnedSitesStorage.removePinnedSite(topSite)
             }
 
             // Remove the top site from both history and pinned sites storage to avoid having it
             // show up as a frecent site if it is a pinned site.
-            historyStorage.deleteVisitsFor(topSite.url)
+            if (topSite !is TopSite.Provided) {
+                historyStorage.deleteVisitsFor(topSite.url)
+            }
 
             notifyObservers { onStorageUpdated() }
         }
     }
 
     override fun updateTopSite(topSite: TopSite, title: String, url: String) {
         scope.launch {
-            if (topSite.type != FRECENT) {
+            if (topSite is TopSite.Default || topSite is TopSite.Pinned) {
                 pinnedSitesStorage.updatePinnedSite(topSite, title, url)
             }
 
             notifyObservers { onStorageUpdated() }
         }
     }
 
+    @Suppress("TooGenericExceptionCaught")
     override suspend fun getTopSites(
         totalSites: Int,
         frecencyConfig: FrecencyThresholdOption?
     ): List<TopSite> {
         val topSites = ArrayList<TopSite>()
         val pinnedSites = pinnedSitesStorage.getPinnedSites().take(totalSites)
-        val numSitesRequired = totalSites - pinnedSites.size
+        var numSitesRequired = totalSites - pinnedSites.size
+
         topSites.addAll(pinnedSites)
 
+        topSitesProvider?.let { provider ->
+            try {
+                val providerTopSites = provider.getTopSites()
+                topSites.addAll(providerTopSites.take(numSitesRequired))
+                numSitesRequired -= providerTopSites.size
+            } catch (e: Exception) {
+                logger.error("Failed to fetch top sites from provider", e)
+            }
+        }
+
         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/TopSite.kt

@@ -6,37 +6,78 @@ package mozilla.components.feature.top.sites
 
 /**
  * A top site.
- *
- * @property id Unique ID of this top site.
- * @property title The title of the top site.
- * @property url The URL of the top site.
- * @property createdAt The optional date the top site was added.
- * @property type The type of a top site.
  */
-data class TopSite(
-    val id: Long?,
-    val title: String?,
-    val url: String,
-    val createdAt: Long?,
-    val type: Type
-) {
+sealed class TopSite {
+    abstract val id: Long?
+    abstract val title: String?
+    abstract val url: String
+    abstract val createdAt: Long?
+
+    /**
+     * This top site was added as a default by the application.
+     *
+     * @property id Unique ID of this top site.
+     * @property title The title of the top site.
+     * @property url The URL of the top site.
+     * @property createdAt The optional date the top site was added.
+     */
+    data class Default(
+        override val id: Long?,
+        override val title: String?,
+        override val url: String,
+        override val createdAt: Long?,
+    ) : TopSite()
+
     /**
-     * The type of a [TopSite].
+     * This top site was pinned by an user.
+     *
+     * @property id Unique ID of this top site.
+     * @property title The title of the top site.
+     * @property url The URL of the top site.
+     * @property createdAt The optional date the top site was added.
      */
-    enum class Type {
-        /**
-         * This top site was added as a default by the application.
-         */
-        DEFAULT,
+    data class Pinned(
+        override val id: Long?,
+        override val title: String?,
+        override val url: String,
+        override val createdAt: Long?,
+    ) : TopSite()
 
-        /**
-         * This top site was pinned by an user.
-         */
-        PINNED,
+    /**
+     * This top site is auto-generated from the history storage based on the most frecent site.
+     *
+     * @property id Unique ID of this top site.
+     * @property title The title of the top site.
+     * @property url The URL of the top site.
+     * @property createdAt The optional date the top site was added.
+     */
+    data class Frecent(
+        override val id: Long?,
+        override val title: String?,
+        override val url: String,
+        override val createdAt: Long?,
+    ) : TopSite()
 
-        /**
-         * This top site is auto-generated from the history storage based on the most frecent site.
-         */
-        FRECENT
-    }
+    /**
+     * This top site is provided by the [TopSitesProvider].
+     *
+     * @property id Unique ID of this top site.
+     * @property title The title of the top site.
+     * @property url The URL of the top site.
+     * @property clickUrl The click URL of the top site.
+     * @property imageUrl The image URL of the top site.
+     * @property impressionUrl The URL that needs to be fired when the top site is displayed.
+     * @property position The position of the top site.
+     * @property createdAt The optional date the top site was added.
+     */
+    data class Provided(
+        override val id: Long?,
+        override val title: String?,
+        override val url: String,
+        val clickUrl: String,
+        val imageUrl: String,
+        val impressionUrl: String,
+        val position: Int,
+        override val createdAt: Long?,
+    ) : TopSite()
 }

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

@@ -0,0 +1,16 @@
+/* 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.top.sites
+
+/**
+ * A contract that indicates how a top sites provider must behave.
+ */
+interface TopSitesProvider {
+
+    /**
+     * Provides a list of top sites.
+     */
+    suspend fun getTopSites(): List<TopSite>
+}

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

@@ -8,8 +8,6 @@ import androidx.room.ColumnInfo
 import androidx.room.Entity
 import androidx.room.PrimaryKey
 import mozilla.components.feature.top.sites.TopSite
-import mozilla.components.feature.top.sites.TopSite.Type.DEFAULT
-import mozilla.components.feature.top.sites.TopSite.Type.PINNED
 
 /**
  * Internal entity representing a pinned site.
@@ -32,24 +30,30 @@ internal data class PinnedSiteEntity(
     @ColumnInfo(name = "created_at")
     var createdAt: Long = System.currentTimeMillis()
 ) {
-    internal fun toTopSite(): TopSite {
-        val type = if (isDefault) DEFAULT else PINNED
-        return TopSite(
-            id,
-            title,
-            url,
-            createdAt,
-            type
-        )
-    }
+    internal fun toTopSite(): TopSite =
+        if (isDefault) {
+            TopSite.Default(
+                id = id,
+                title = title,
+                url = url,
+                createdAt = createdAt
+            )
+        } else {
+            TopSite.Pinned(
+                id = id,
+                title = title,
+                url = url,
+                createdAt = createdAt
+            )
+        }
 }
 
 internal fun TopSite.toPinnedSite(): PinnedSiteEntity {
     return PinnedSiteEntity(
         id = id,
         title = title ?: "",
         url = url,
-        isDefault = type === DEFAULT,
+        isDefault = this is TopSite.Default,
         createdAt = createdAt ?: 0
     )
 }

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

@@ -6,18 +6,16 @@ package mozilla.components.feature.top.sites.ext
 
 import mozilla.components.concept.storage.TopFrecentSiteInfo
 import mozilla.components.feature.top.sites.TopSite
-import mozilla.components.feature.top.sites.TopSite.Type.FRECENT
 import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl
 
 /**
  * Returns a [TopSite] for the given [TopFrecentSiteInfo].
  */
 fun TopFrecentSiteInfo.toTopSite(): TopSite {
-    return TopSite(
+    return TopSite.Frecent(
         id = null,
         title = this.title?.takeIf(String::isNotBlank) ?: this.url.tryGetHostFromUrl(),
         url = this.url,
-        createdAt = null,
-        type = FRECENT
+        createdAt = null
     )
 }