Dedupe history metadata in groups based on url

app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataFeature.kt

@@ -15,6 +15,7 @@ import mozilla.components.support.base.feature.LifecycleAwareFeature
 import org.mozilla.fenix.home.HomeFragment
 import org.mozilla.fenix.home.HomeFragmentAction
 import org.mozilla.fenix.home.HomeFragmentStore
+import kotlin.math.max
 
 /**
  * View-bound feature that retrieves a list of history metadata and dispatches updates to the
@@ -41,6 +42,21 @@ class HistoryMetadataFeature(
             val historyMetadata = historyMetadataStorage.getHistoryMetadataSince(Long.MIN_VALUE)
                 .filter { it.totalViewTime > 0 && it.key.searchTerm != null }
                 .groupBy { it.key.searchTerm!! }
+                .mapValues { group ->
+                    // Within a group, we dedupe entries based on their url so we don't display
+                    // a page multiple times in the same group, and we sum up the total view time
+                    // of deduped entries while making sure to keep the latest updatedAt value.
+                    val metadataInGroup = group.value
+                    val metadataUrlGroups = metadataInGroup.groupBy { metadata -> metadata.key.url }
+                    metadataUrlGroups.map { metadata ->
+                        metadata.value.reduce { acc, elem ->
+                            acc.copy(
+                                totalViewTime = acc.totalViewTime + elem.totalViewTime,
+                                updatedAt = max(acc.updatedAt, elem.updatedAt)
+                            )
+                        }
+                    }
+                }
                 .map { (title, data) ->
                     HistoryMetadataGroup(
                         title = title,

app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataFeatureTest.kt

@@ -18,7 +18,6 @@ import mozilla.components.concept.storage.HistoryMetadataStorage
 import mozilla.components.support.test.libstate.ext.waitUntilIdle
 import mozilla.components.support.test.middleware.CaptureActionsMiddleware
 import mozilla.components.support.test.rule.MainCoroutineRule
-import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Rule
@@ -34,20 +33,6 @@ class HistoryMetadataFeatureTest {
 
     private val middleware = CaptureActionsMiddleware<HomeFragmentState, HomeFragmentAction>()
     private val homeStore = HomeFragmentStore(middlewares = listOf(middleware))
-
-    private val historyEntry = HistoryMetadata(
-        key = HistoryMetadataKey("http://www.mozilla.com", "mozilla", null),
-        title = "mozilla",
-        createdAt = System.currentTimeMillis(),
-        updatedAt = System.currentTimeMillis(),
-        totalViewTime = 10,
-        documentType = DocumentType.Regular
-    )
-    private val historyGroup = HistoryMetadataGroup(
-        title = "mozilla",
-        historyMetadata = listOf(historyEntry)
-    )
-
     private val testDispatcher = TestCoroutineDispatcher()
 
     @get:Rule
@@ -56,42 +41,162 @@ class HistoryMetadataFeatureTest {
     @Before
     fun setup() {
         historyMetadataStorage = mockk(relaxed = true)
-
-        coEvery { historyMetadataStorage.getHistoryMetadataSince(any()) }.coAnswers {
-            listOf(
-                historyEntry
-            )
-        }
-    }
-
-    @After
-    fun cleanUp() {
-        testDispatcher.cleanupTestCoroutines()
     }
 
     @Test
     fun `GIVEN no history metadata WHEN feature starts THEN fetch history metadata and notify store`() =
         testDispatcher.runBlockingTest {
-            val feature = HistoryMetadataFeature(
-                homeStore,
-                historyMetadataStorage,
-                CoroutineScope(testDispatcher),
-                testDispatcher
+            val historyEntry = HistoryMetadata(
+                key = HistoryMetadataKey("http://www.mozilla.com", "mozilla", null),
+                title = "mozilla",
+                createdAt = System.currentTimeMillis(),
+                updatedAt = System.currentTimeMillis(),
+                totalViewTime = 10,
+                documentType = DocumentType.Regular
+            )
+            val expectedHistoryGroup = HistoryMetadataGroup(
+                title = "mozilla",
+                historyMetadata = listOf(historyEntry)
             )
 
-            assertEquals(emptyList<HistoryMetadataGroup>(), homeStore.state.historyMetadata)
-
-            feature.start()
-
-            testDispatcher.advanceUntilIdle()
-            homeStore.waitUntilIdle()
-
-            coVerify {
-                historyMetadataStorage.getHistoryMetadataSince(any())
+            coEvery { historyMetadataStorage.getHistoryMetadataSince(any()) }.coAnswers {
+                listOf(
+                    historyEntry
+                )
             }
 
+            startHistoryMetadataFeature()
+
             middleware.assertLastAction(HomeFragmentAction.HistoryMetadataChange::class) {
-                assertEquals(listOf(historyGroup), it.historyMetadata)
+                assertEquals(listOf(expectedHistoryGroup), it.historyMetadata)
             }
         }
+
+    @Test
+    fun `GIVEN history metadata WHEN group contains multiple entries with same url THEN entries are deduped`() =
+        testDispatcher.runBlockingTest {
+            val historyEntry1 = HistoryMetadata(
+                key = HistoryMetadataKey("http://www.mozilla.com", "mozilla", null),
+                title = "mozilla",
+                createdAt = System.currentTimeMillis(),
+                updatedAt = 1,
+                totalViewTime = 10,
+                documentType = DocumentType.Regular
+            )
+
+            val historyEntry2 = HistoryMetadata(
+                key = HistoryMetadataKey("http://firefox.com", "mozilla", null),
+                title = "firefox",
+                createdAt = System.currentTimeMillis(),
+                updatedAt = 2,
+                totalViewTime = 20,
+                documentType = DocumentType.Regular
+            )
+
+            val historyEntry3 = HistoryMetadata(
+                key = HistoryMetadataKey("http://www.mozilla.com", "mozilla", null),
+                title = "mozilla",
+                createdAt = System.currentTimeMillis(),
+                updatedAt = 3,
+                totalViewTime = 30,
+                documentType = DocumentType.Regular
+            )
+
+            val expectedHistoryGroup = HistoryMetadataGroup(
+                title = "mozilla",
+                historyMetadata = listOf(
+                    // Expected total view time to be summed up for deduped entries
+                    historyEntry1.copy(
+                        totalViewTime = historyEntry1.totalViewTime + historyEntry3.totalViewTime,
+                        updatedAt = historyEntry3.updatedAt
+                    ),
+                    historyEntry2
+                )
+            )
+
+            coEvery { historyMetadataStorage.getHistoryMetadataSince(any()) }.coAnswers {
+                listOf(
+                    historyEntry1, historyEntry2, historyEntry3
+                )
+            }
+
+            startHistoryMetadataFeature()
+
+            middleware.assertLastAction(HomeFragmentAction.HistoryMetadataChange::class) {
+                assertEquals(listOf(expectedHistoryGroup), it.historyMetadata)
+            }
+        }
+
+    @Test
+    fun `GIVEN history metadata WHEN different groups contain entries with same url THEN entries are not deduped`() =
+        testDispatcher.runBlockingTest {
+            val historyEntry1 = HistoryMetadata(
+                key = HistoryMetadataKey("http://www.mozilla.com", "mozilla", null),
+                title = "mozilla",
+                createdAt = System.currentTimeMillis(),
+                updatedAt = System.currentTimeMillis(),
+                totalViewTime = 10,
+                documentType = DocumentType.Regular
+            )
+
+            val historyEntry2 = HistoryMetadata(
+                key = HistoryMetadataKey("http://firefox.com", "mozilla", null),
+                title = "firefox",
+                createdAt = System.currentTimeMillis(),
+                updatedAt = System.currentTimeMillis(),
+                totalViewTime = 20,
+                documentType = DocumentType.Regular
+            )
+
+            val historyEntry3 = HistoryMetadata(
+                key = HistoryMetadataKey("http://www.mozilla.com", "firefox", null),
+                title = "mozilla",
+                createdAt = System.currentTimeMillis(),
+                updatedAt = System.currentTimeMillis(),
+                totalViewTime = 30,
+                documentType = DocumentType.Regular
+            )
+
+            val expectedHistoryGroup1 = HistoryMetadataGroup(
+                title = "mozilla",
+                historyMetadata = listOf(historyEntry1, historyEntry2)
+            )
+
+            val expectedHistoryGroup2 = HistoryMetadataGroup(
+                title = "firefox",
+                historyMetadata = listOf(historyEntry3)
+            )
+
+            coEvery { historyMetadataStorage.getHistoryMetadataSince(any()) }.coAnswers {
+                listOf(
+                    historyEntry1, historyEntry2, historyEntry3
+                )
+            }
+
+            startHistoryMetadataFeature()
+
+            middleware.assertLastAction(HomeFragmentAction.HistoryMetadataChange::class) {
+                assertEquals(listOf(expectedHistoryGroup1, expectedHistoryGroup2), it.historyMetadata)
+            }
+        }
+
+    private fun startHistoryMetadataFeature() {
+        val feature = HistoryMetadataFeature(
+            homeStore,
+            historyMetadataStorage,
+            CoroutineScope(testDispatcher),
+            testDispatcher
+        )
+
+        assertEquals(emptyList<HistoryMetadataGroup>(), homeStore.state.historyMetadata)
+
+        feature.start()
+
+        testDispatcher.advanceUntilIdle()
+        homeStore.waitUntilIdle()
+
+        coVerify {
+            historyMetadataStorage.getHistoryMetadataSince(any())
+        }
+    }
 }