[fenix] Closes https://github.com/mozilla-mobile/fenix/issues/22472: Filter out redirects from History search groups

app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt

@@ -50,6 +50,14 @@ class DefaultPagedHistoryProvider(
         VisitType.FRAMED_LINK,
     )
 
+    /**
+     * All types of visits that aren't redirects. This is used for fetching only redirecting visits
+     * from the store so that we can filter them out.
+     */
+    private val notRedirectTypes = VisitType.values().filterNot {
+        it == VisitType.REDIRECT_PERMANENT || it == VisitType.REDIRECT_TEMPORARY
+    }
+
     @Volatile private var historyGroups: List<History.Group>? = null
 
     @Suppress("LongMethod")
@@ -64,7 +72,7 @@ class DefaultPagedHistoryProvider(
             val history: List<History>
 
             if (showHistorySearchGroups) {
-                // We need to refetch all the history metadata if the offset resets back at 0
+                // We need to re-fetch all the history metadata if the offset resets back at 0
                 // in the case of a pull to refresh.
                 if (historyGroups == null || offset == 0L) {
                     historyGroups = historyStorage.getHistoryMetadataSince(Long.MIN_VALUE)
@@ -134,6 +142,22 @@ class DefaultPagedHistoryProvider(
             )
             .mapIndexed(transformVisitInfoToHistoryItem(offset.toInt()))
 
+        // We'll use this list to filter out redirects from metadata groups below.
+        val redirectsInThePage = if (history.isNotEmpty()) {
+            historyStorage.getDetailedVisits(
+                start = history.last().visitedAt,
+                end = history.first().visitedAt,
+                excludeTypes = notRedirectTypes
+            ).map { it.url }
+        } else {
+            // Edge-case this doesn't cover: if we only had redirects in the current page,
+            // we'd end up with an empty 'history' list since the redirects would have been
+            // filtered out above. One possible solution would be to look at redirects in all of
+            // history, but that's potentially quite expensive on large profiles, and introduces
+            // other problems (e.g. pages that were redirects a month ago may not be redirects today).
+            emptyList()
+        }
+
         // History metadata items are recorded after their associated visited info, we add an
         // additional buffer time to the most recent visit to account for a history group
         // appearing as the most recent item.
@@ -164,7 +188,7 @@ class DefaultPagedHistoryProvider(
         // url, but we don't have a use case for this currently in the history view.
         result.addAll(
             historyGroupsInOffset.map { group ->
-                group.copy(items = group.items.distinctBy { it.url })
+                group.copy(items = group.items.distinctBy { it.url }.filterNot { redirectsInThePage.contains(it.url) })
             }
         )
 

app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt

@@ -89,6 +89,7 @@ class PagedHistoryProviderTest {
         )
 
         coEvery { storage.getVisitsPaginated(any(), any(), any()) } returns listOf(visitInfo1, visitInfo2, visitInfo3)
+        coEvery { storage.getDetailedVisits(any(), any(), any()) } returns emptyList()
         coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1, historyEntry2, historyEntry3)
 
         var actualResults: List<History>? = null
@@ -175,6 +176,7 @@ class PagedHistoryProviderTest {
         )
 
         coEvery { storage.getVisitsPaginated(any(), any(), any()) } returns listOf(visitInfo1)
+        coEvery { storage.getDetailedVisits(any(), any(), any()) } returns emptyList()
         coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1)
 
         var actualResults: List<History>? = null
@@ -248,6 +250,7 @@ class PagedHistoryProviderTest {
         )
 
         coEvery { storage.getVisitsPaginated(any(), any(), any()) } returns listOf(visitInfo1)
+        coEvery { storage.getDetailedVisits(any(), any(), any()) } returns emptyList()
         coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1)
 
         var actualResults: List<History>? = null
@@ -292,4 +295,161 @@ class PagedHistoryProviderTest {
 
         assertEquals(results, actualResults)
     }
+
+    @Test
+    fun `redirects are filtered out from history metadata groups`() {
+        val provider = DefaultPagedHistoryProvider(
+            historyStorage = storage,
+            showHistorySearchGroups = true
+        )
+
+        val visitInfo1 = VisitInfo(
+            url = "http://www.mozilla.com",
+            title = "mozilla",
+            visitTime = 5,
+            visitType = VisitType.LINK,
+            previewImageUrl = null
+        )
+        val visitInfo2 = VisitInfo(
+            url = "http://www.firefox.com",
+            title = "firefox",
+            visitTime = 2,
+            visitType = VisitType.LINK,
+            previewImageUrl = null
+        )
+        val visitInfo3 = VisitInfo(
+            url = "http://www.google.com/link?url=http://www.firefox.com",
+            title = "",
+            visitTime = 1,
+            visitType = VisitType.REDIRECT_TEMPORARY,
+            previewImageUrl = null
+        )
+        val visitInfo4 = VisitInfo(
+            url = "http://mozilla.com",
+            title = "",
+            visitTime = 1,
+            visitType = VisitType.REDIRECT_PERMANENT,
+            previewImageUrl = null
+        )
+
+        val historyMetadataKey1 = HistoryMetadataKey("http://www.mozilla.com", "mozilla", null)
+        val historyEntry1 = HistoryMetadata(
+            key = historyMetadataKey1,
+            title = "mozilla",
+            createdAt = 1,
+            updatedAt = 10,
+            totalViewTime = 10,
+            documentType = DocumentType.Regular,
+            previewImageUrl = null
+        )
+        val historyMetadataKey2 = HistoryMetadataKey("http://www.firefox.com", "mozilla", null)
+        val historyEntry2 = HistoryMetadata(
+            key = historyMetadataKey2,
+            title = "firefox",
+            createdAt = 2,
+            updatedAt = 11,
+            totalViewTime = 20,
+            documentType = DocumentType.Regular,
+            previewImageUrl = null
+        )
+        val historyMetadataKey3 = HistoryMetadataKey("http://www.google.com/link?url=http://www.firefox.com", "mozilla", null)
+        val historyEntry3 = HistoryMetadata(
+            key = historyMetadataKey3,
+            title = "",
+            createdAt = 2,
+            updatedAt = 11,
+            totalViewTime = 0,
+            documentType = DocumentType.Regular,
+            previewImageUrl = null
+        )
+        val historyMetadataKey4 = HistoryMetadataKey("http://mozilla.com", "mozilla", null)
+        val historyEntry4 = HistoryMetadata(
+            key = historyMetadataKey4,
+            title = "",
+            createdAt = 2,
+            updatedAt = 11,
+            totalViewTime = 0,
+            documentType = DocumentType.Regular,
+            previewImageUrl = null
+        )
+
+        // Normal visits.
+        coEvery {
+            storage.getVisitsPaginated(
+                any(), any(),
+                eq(
+                    listOf(
+                        VisitType.NOT_A_VISIT,
+                        VisitType.DOWNLOAD,
+                        VisitType.REDIRECT_PERMANENT,
+                        VisitType.REDIRECT_TEMPORARY,
+                        VisitType.RELOAD,
+                        VisitType.EMBED,
+                        VisitType.FRAMED_LINK,
+                    )
+                )
+            )
+        } returns listOf(visitInfo1, visitInfo2)
+        // Redirects.
+        coEvery {
+            storage.getDetailedVisits(
+                any(), any(),
+                eq(
+                    VisitType.values().filterNot {
+                        it == VisitType.REDIRECT_PERMANENT || it == VisitType.REDIRECT_TEMPORARY
+                    }
+                )
+            )
+        } returns listOf(visitInfo3, visitInfo4)
+
+        coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1, historyEntry2, historyEntry3, historyEntry4)
+
+        var actualResults: List<History>? = null
+        provider.getHistory(10L, 5) {
+            actualResults = it
+        }
+
+        coVerify {
+            storage.getVisitsPaginated(
+                offset = 10L,
+                count = 5,
+                excludeTypes = listOf(
+                    VisitType.NOT_A_VISIT,
+                    VisitType.DOWNLOAD,
+                    VisitType.REDIRECT_PERMANENT,
+                    VisitType.REDIRECT_TEMPORARY,
+                    VisitType.RELOAD,
+                    VisitType.EMBED,
+                    VisitType.FRAMED_LINK,
+                )
+            )
+        }
+
+        val results = listOf(
+            History.Group(
+                id = historyEntry2.createdAt.toInt(),
+                title = historyEntry2.key.searchTerm!!,
+                visitedAt = historyEntry2.createdAt,
+                items = listOf(
+                    History.Metadata(
+                        id = historyEntry2.createdAt.toInt(),
+                        title = historyEntry2.title!!,
+                        url = historyEntry2.key.url,
+                        visitedAt = historyEntry2.createdAt,
+                        totalViewTime = historyEntry2.totalViewTime,
+                        historyMetadataKey = historyMetadataKey2
+                    ),
+                    History.Metadata(
+                        id = historyEntry1.createdAt.toInt(),
+                        title = historyEntry1.title!!,
+                        url = historyEntry1.key.url,
+                        visitedAt = historyEntry1.createdAt,
+                        totalViewTime = historyEntry1.totalViewTime,
+                        historyMetadataKey = historyMetadataKey1
+                    ),
+                )
+            )
+        )
+        assertEquals(results, actualResults)
+    }
 }