From 0bcad0d3646d9957dd63e8c71f3e293cd5892186 Mon Sep 17 00:00:00 2001
From: Grisha Kruglov <gkruglov@mozilla.com>
Date: Thu, 4 Apr 2019 15:28:27 -0700
Subject: [PATCH] History UI limits: visit type & time

These are temporary limitations to make History UI somewhat functional,
until we get relevant UI and API changes in place.
---
 .../fenix/library/history/HistoryComponent.kt | 15 +-----------
 .../fenix/library/history/HistoryFragment.kt  | 24 ++++++++++++++++---
 .../library/history/HistoryComponentTest.kt   |  6 ++---
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryComponent.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryComponent.kt
index a87bfe313..6c4967c94 100644
--- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryComponent.kt
+++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryComponent.kt
@@ -10,21 +10,8 @@ import org.mozilla.fenix.mvi.ActionBusFactory
 import org.mozilla.fenix.mvi.Change
 import org.mozilla.fenix.mvi.UIComponent
 import org.mozilla.fenix.mvi.ViewState
-import java.net.URL
 
-data class HistoryItem(val id: Int, val url: String, val visitedAt: Long) {
-    val title: String
-        get() = siteTitle()
-
-    @SuppressWarnings("TooGenericExceptionCaught")
-    private fun siteTitle(): String {
-        return try {
-            URL(url).host
-        } catch (e: Exception) {
-            url
-        }
-    }
-}
+data class HistoryItem(val id: Int, val title: String, val url: String, val visitedAt: Long)
 
 @Mockable
 class HistoryComponent(
diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt
index 03d002618..2b97f2e3f 100644
--- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt
@@ -21,6 +21,7 @@ import kotlinx.coroutines.Job
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.coroutineScope
+import mozilla.components.concept.storage.VisitType
 import mozilla.components.support.base.feature.BackHandler
 import org.mozilla.fenix.utils.ItsNotBrokenSnack
 import org.mozilla.fenix.R
@@ -28,6 +29,7 @@ import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.mvi.ActionBusFactory
 import org.mozilla.fenix.mvi.getAutoDisposeObservable
 import org.mozilla.fenix.mvi.getManagedEmitter
+import java.net.URL
 import kotlin.coroutines.CoroutineContext
 
 @SuppressWarnings("TooManyFunctions")
@@ -141,9 +143,25 @@ class HistoryFragment : Fragment(), CoroutineScope, BackHandler {
     override fun onBackPressed(): Boolean = (historyComponent.uiView as HistoryUIView).onBackPressed()
 
     private suspend fun reloadData() {
-        val items = requireComponents.core.historyStorage.getDetailedVisits(0)
-            .asReversed()
-            .mapIndexed { id, item -> HistoryItem(id, item.url, item.visitTime) }
+        val allowedVisitTypes = listOf(VisitType.LINK, VisitType.TYPED, VisitType.BOOKMARK)
+        // Until we have proper pagination, only display a limited set of history to avoid blowing up the UI.
+        // See https://github.com/mozilla-mobile/fenix/issues/1393
+        @SuppressWarnings("MagicNumber")
+        val historyCutoffMs = 1000L * 60 * 60 * 24 * 3 // past few days
+        val items = requireComponents.core.historyStorage.getDetailedVisits(
+            System.currentTimeMillis() - historyCutoffMs
+        )
+            // We potentially have a large amount of visits, and multiple processing steps.
+            // Wrapping iterator in a sequence should make this a little more efficient.
+            .asSequence()
+            .sortedByDescending { it.visitTime }
+
+            // Temporary filtering until we can do it at the API level.
+            // See https://github.com/mozilla-mobile/android-components/issues/2643
+            .filter { allowedVisitTypes.contains(it.visitType) }
+
+            .mapIndexed { id, item -> HistoryItem(id, item.title ?: URL(item.url).host, item.url, item.visitTime) }
+            .toList()
 
         coroutineScope {
             launch(Dispatchers.Main) {
diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryComponentTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryComponentTest.kt
index 95a419f22..edb6dc064 100644
--- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryComponentTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryComponentTest.kt
@@ -40,7 +40,7 @@ class HistoryComponentTest {
 
     @Test
     fun `add and remove one history item normally`() {
-        val historyItem = HistoryItem(123, "http://mozilla.org", 0)
+        val historyItem = HistoryItem(123, "Mozilla", "http://mozilla.org", 0)
 
         emitter.onNext(HistoryChange.Change(listOf(historyItem)))
         emitter.onNext(HistoryChange.EnterEditMode(historyItem))
@@ -62,8 +62,8 @@ class HistoryComponentTest {
     @Test
     fun `try making changes when not in edit mode`() {
         val historyItems = listOf(
-            HistoryItem(1337, "http://reddit.com", 0),
-            HistoryItem(31337, "http://leethaxor.com", 0)
+            HistoryItem(1337, "Reddit", "http://reddit.com", 0),
+            HistoryItem(31337, "Haxor", "http://leethaxor.com", 0)
         )
 
         emitter.onNext(HistoryChange.Change(historyItems))