For mozilla-mobile#11483 - Part 2: Convert TopSites into a sealed class

gabrielluong · Jan 17, 2022 · 39e8286 · 39e8286
1 parent 247150a
commit 39e8286
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 109 deletions.
diff --git a/...p/sites/OnDevicePinnedSitesStorageTest.kt → ...ature/top/sites/PinnedSitesStorageTest.kt b/...p/sites/OnDevicePinnedSitesStorageTest.kt → ...ature/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)
         }
     }
 

diff --git a/...re/top-sites/src/main/java/mozilla/components/feature/top/sites/DefaultTopSitesStorage.kt b/...re/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)
             }
 

diff --git a/components/feature/top-sites/src/main/java/mozilla/components/feature/top/sites/TopSite.kt b/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()
 }
diff --git a/...ature/top-sites/src/main/java/mozilla/components/feature/top/sites/db/PinnedSiteEntity.kt b/...ature/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
     )
 }
diff --git a/...re/top-sites/src/main/java/mozilla/components/feature/top/sites/ext/TopFrecentSiteInfo.kt b/...re/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
     )
 }