Issue mozilla-mobile#11483: Convert TopSites into a sealed class

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

@@ -9,7 +9,6 @@ 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
@@ -58,7 +57,7 @@ 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)
             }
 
@@ -72,7 +71,7 @@ class DefaultTopSitesStorage(
 
     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)
             }
 

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

@@ -6,37 +6,55 @@ 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?
+
     /**
-     * The type of a [TopSite].
+     * 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.
      */
-    enum class Type {
-        /**
-         * This top site was added as a default by the application.
-         */
-        DEFAULT,
+    data class Default(
+        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 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.
+     */
+    data class Pinned(
+        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 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()
 }

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
     )
 }

components/feature/top-sites/src/test/java/mozilla/components/feature/top/sites/DefaultTopSitesStorageTest.kt

@@ -72,35 +72,32 @@ class DefaultTopSitesStorageTest {
             coroutineContext = coroutineContext
         )
 
-        val frecentSite = TopSite(
+        val frecentSite = TopSite.Frecent(
             id = 1,
             title = "Mozilla",
             url = "https://mozilla.com",
-            createdAt = 1,
-            type = TopSite.Type.FRECENT
+            createdAt = 1
         )
         defaultTopSitesStorage.removeTopSite(frecentSite)
 
         verify(historyStorage).deleteVisitsFor(frecentSite.url)
 
-        val pinnedSite = TopSite(
+        val pinnedSite = TopSite.Pinned(
             id = 2,
             title = "Firefox",
             url = "https://firefox.com",
-            createdAt = 2,
-            type = TopSite.Type.PINNED
+            createdAt = 2
         )
         defaultTopSitesStorage.removeTopSite(pinnedSite)
 
         verify(pinnedSitesStorage).removePinnedSite(pinnedSite)
         verify(historyStorage).deleteVisitsFor(pinnedSite.url)
 
-        val defaultSite = TopSite(
+        val defaultSite = TopSite.Default(
             id = 3,
             title = "Wikipedia",
             url = "https://wikipedia.com",
-            createdAt = 3,
-            type = TopSite.Type.DEFAULT
+            createdAt = 3
         )
         defaultTopSitesStorage.removeTopSite(defaultSite)
 
@@ -117,34 +114,31 @@ class DefaultTopSitesStorageTest {
             coroutineContext = coroutineContext
         )
 
-        val defaultSite = TopSite(
+        val defaultSite = TopSite.Default(
             id = 1,
             title = "Firefox",
             url = "https://firefox.com",
-            createdAt = 1,
-            type = TopSite.Type.DEFAULT
+            createdAt = 1
         )
         defaultTopSitesStorage.updateTopSite(defaultSite, "Mozilla Firefox", "https://mozilla.com")
 
         verify(pinnedSitesStorage).updatePinnedSite(defaultSite, "Mozilla Firefox", "https://mozilla.com")
 
-        val pinnedSite = TopSite(
+        val pinnedSite = TopSite.Pinned(
             id = 2,
             title = "Wikipedia",
             url = "https://wikipedia.com",
-            createdAt = 2,
-            type = TopSite.Type.PINNED
+            createdAt = 2
         )
         defaultTopSitesStorage.updateTopSite(pinnedSite, "Wiki", "https://en.wikipedia.org/wiki/Wiki")
 
         verify(pinnedSitesStorage).updatePinnedSite(pinnedSite, "Wiki", "https://en.wikipedia.org/wiki/Wiki")
 
-        val frecentSite = TopSite(
+        val frecentSite = TopSite.Frecent(
             id = 1,
             title = "Mozilla",
             url = "https://mozilla.com",
-            createdAt = 1,
-            type = TopSite.Type.FRECENT
+            createdAt = 1
         )
         defaultTopSitesStorage.updateTopSite(frecentSite, "Moz", "")
 
@@ -160,19 +154,17 @@ class DefaultTopSitesStorageTest {
             coroutineContext = coroutineContext
         )
 
-        val defaultSite = TopSite(
+        val defaultSite = TopSite.Default(
             id = 1,
             title = "Firefox",
             url = "https://firefox.com",
-            createdAt = 1,
-            type = TopSite.Type.DEFAULT
+            createdAt = 1
         )
-        val pinnedSite = TopSite(
+        val pinnedSite = TopSite.Pinned(
             id = 2,
             title = "Wikipedia",
             url = "https://wikipedia.com",
-            createdAt = 2,
-            type = TopSite.Type.PINNED
+            createdAt = 2
         )
         whenever(pinnedSitesStorage.getPinnedSites()).thenReturn(
             listOf(
@@ -213,19 +205,17 @@ class DefaultTopSitesStorageTest {
             coroutineContext = coroutineContext
         )
 
-        val defaultSite = TopSite(
+        val defaultSite = TopSite.Default(
             id = 1,
             title = "Firefox",
             url = "https://firefox.com",
-            createdAt = 1,
-            type = TopSite.Type.DEFAULT
+            createdAt = 1
         )
-        val pinnedSite = TopSite(
+        val pinnedSite = TopSite.Pinned(
             id = 2,
             title = "Wikipedia",
             url = "https://wikipedia.com",
-            createdAt = 2,
-            type = TopSite.Type.PINNED
+            createdAt = 2
         )
         whenever(pinnedSitesStorage.getPinnedSites()).thenReturn(
             listOf(
@@ -307,26 +297,23 @@ class DefaultTopSitesStorageTest {
             coroutineContext = coroutineContext
         )
 
-        val defaultSiteFirefox = TopSite(
+        val defaultSiteFirefox = TopSite.Default(
             id = 1,
             title = "Firefox",
             url = "https://firefox.com",
-            createdAt = 1,
-            type = TopSite.Type.DEFAULT
+            createdAt = 1
         )
-        val pinnedSite1 = TopSite(
+        val pinnedSite1 = TopSite.Pinned(
             id = 2,
             title = "Wikipedia",
             url = "https://wikipedia.com",
-            createdAt = 2,
-            type = TopSite.Type.PINNED
+            createdAt = 2
         )
-        val pinnedSite2 = TopSite(
+        val pinnedSite2 = TopSite.Pinned(
             id = 3,
             title = "Example",
             url = "https://example.com",
-            createdAt = 3,
-            type = TopSite.Type.PINNED
+            createdAt = 3
         )
         whenever(pinnedSitesStorage.getPinnedSites()).thenReturn(
             listOf(

components/feature/top-sites/src/test/java/mozilla/components/feature/top/sites/PinnedSitesStorageTest.kt

@@ -5,8 +5,6 @@
 package mozilla.components.feature.top.sites
 
 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.PinnedSiteDao
 import mozilla.components.feature.top.sites.db.PinnedSiteEntity
 import mozilla.components.feature.top.sites.db.TopSiteDatabase
@@ -72,8 +70,8 @@ class PinnedSitesStorageTest {
         val storage = PinnedSiteStorage(mock())
         val dao = mockDao(storage)
 
-        storage.removePinnedSite(TopSite(1, "Mozilla", "https://www.mozilla.org", 1, PINNED))
-        storage.removePinnedSite(TopSite(2, "Firefox", "https://www.firefox.com", 1, DEFAULT))
+        storage.removePinnedSite(TopSite.Pinned(1, "Mozilla", "https://www.mozilla.org", 1))
+        storage.removePinnedSite(TopSite.Pinned(2, "Firefox", "https://www.firefox.com", 1))
 
         verify(dao).deletePinnedSite(PinnedSiteEntity(1, "Mozilla", "https://www.mozilla.org", false, 1))
         verify(dao).deletePinnedSite(PinnedSiteEntity(2, "Firefox", "https://www.firefox.com", true, 1))
@@ -103,15 +101,13 @@ class PinnedSitesStorageTest {
             assertEquals(1L, id)
             assertEquals("Mozilla", title)
             assertEquals("https://www.mozilla.org", url)
-            assertEquals(PINNED, type)
             assertEquals(10L, createdAt)
         }
 
         with(topSites[1]) {
             assertEquals(2L, id)
             assertEquals("Firefox", title)
             assertEquals("https://www.firefox.com", url)
-            assertEquals(DEFAULT, type)
             assertEquals(10L, createdAt)
         }
     }
@@ -121,7 +117,7 @@ class PinnedSitesStorageTest {
         val storage = PinnedSiteStorage(mock())
         val dao = mockDao(storage)
 
-        val site = TopSite(1, "Mozilla", "https://www.mozilla.org", 1, PINNED)
+        val site = TopSite.Pinned(1, "Mozilla", "https://www.mozilla.org", 1)
         storage.updatePinnedSite(site, "Mozilla (IT)", "https://www.mozilla.org/it")
 
         verify(dao).updatePinnedSite(PinnedSiteEntity(1, "Mozilla (IT)", "https://www.mozilla.org/it", false, 1))

docs/changelog.md

@@ -11,6 +11,8 @@ permalink: /changelog/
 * [Gecko](https://github.com/mozilla-mobile/android-components/blob/main/buildSrc/src/main/java/Gecko.kt)
 * [Configuration](https://github.com/mozilla-mobile/android-components/blob/main/.config.yml)
 
+* **feature-top-sites**
+  * ⚠️ **This is a breaking change**: The existing data class `TopSite` has been converted into a sealed class. [#11483](https://github.com/mozilla-mobile/android-components/issues/11483)
 
 # 97.0.0
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v96.0.0...v97.0.0)