diff --git a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt index e94b1989f6..dee137c074 100644 --- a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt +++ b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt @@ -28,7 +28,7 @@ interface PagedHistoryProvider { * @param numberOfItems How many items to fetch * @param onComplete A callback that returns the list of [History] */ - fun getHistory(offset: Long, numberOfItems: Long, onComplete: (List) -> Unit) + fun getHistory(offset: Int, numberOfItems: Int, onComplete: (List) -> Unit) } /** @@ -63,8 +63,8 @@ class DefaultPagedHistoryProvider( @Suppress("LongMethod") override fun getHistory( - offset: Long, - numberOfItems: Long, + offset: Int, + numberOfItems: Int, onComplete: (List) -> Unit, ) { // A PagedList DataSource runs on a background thread automatically. @@ -75,14 +75,13 @@ class DefaultPagedHistoryProvider( if (showHistorySearchGroups) { // 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) { + if (historyGroups == null || offset == 0) { historyGroups = historyStorage.getHistoryMetadataSince(Long.MIN_VALUE) .sortedByDescending { it.createdAt } .filter { it.key.searchTerm != null } .groupBy { it.key.searchTerm!! } .map { (searchTerm, items) -> History.Group( - id = items.first().createdAt.toInt(), title = searchTerm, visitedAt = items.first().createdAt, items = items.map { it.toHistoryMetadata() } @@ -94,11 +93,11 @@ class DefaultPagedHistoryProvider( } else { history = historyStorage .getVisitsPaginated( - offset, - numberOfItems, + offset.toLong(), + numberOfItems.toLong(), excludeTypes = excludedVisitTypes ) - .mapIndexed(transformVisitInfoToHistoryItem(offset.toInt())) + .map { transformVisitInfoToHistoryItem(it) } } onComplete(history) @@ -142,17 +141,17 @@ class DefaultPagedHistoryProvider( @Suppress("MagicNumber") private suspend fun getHistoryAndSearchGroups( - offset: Long, - numberOfItems: Long, + offset: Int, + numberOfItems: Int, ): List { val result = mutableListOf() val history: List = historyStorage .getVisitsPaginated( - offset, - numberOfItems, + offset.toLong(), + numberOfItems.toLong(), excludeTypes = excludedVisitTypes ) - .mapIndexed(transformVisitInfoToHistoryItem(offset.toInt())) + .map { transformVisitInfoToHistoryItem(it) } // We'll use this list to filter out redirects from metadata groups below. val redirectsInThePage = if (history.isNotEmpty()) { @@ -173,7 +172,7 @@ class DefaultPagedHistoryProvider( // 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. - val visitedAtBuffer = if (offset == 0L) BUFFER_TIME else 0 + val visitedAtBuffer = if (offset == 0) BUFFER_TIME else 0 // Get the history groups that fit within the range of visited times in the current history // items. @@ -208,19 +207,16 @@ class DefaultPagedHistoryProvider( .sortedByDescending { it.visitedAt } } - private fun transformVisitInfoToHistoryItem(offset: Int): (id: Int, visit: VisitInfo) -> History.Regular { - return { id, visit -> - val title = visit.title - ?.takeIf(String::isNotEmpty) - ?: visit.url.tryGetHostFromUrl() - - History.Regular( - id = offset + id, - title = title, - url = visit.url, - visitedAt = visit.visitTime - ) - } + private fun transformVisitInfoToHistoryItem(visit: VisitInfo): History.Regular { + val title = visit.title + ?.takeIf(String::isNotEmpty) + ?: visit.url.tryGetHostFromUrl() + + return History.Regular( + title = title, + url = visit.url, + visitedAt = visit.visitTime + ) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryDataSource.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryDataSource.kt index a9b7c60089..fd70d88428 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryDataSource.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryDataSource.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.library.history +import androidx.annotation.VisibleForTesting import androidx.paging.ItemKeyedDataSource import org.mozilla.fenix.components.history.PagedHistoryProvider @@ -14,26 +15,61 @@ class HistoryDataSource( // Because the pagination is not based off of the key // we want to start at 1, not 0 to be able to send the correct offset // to the `historyProvider.getHistory` call. - override fun getKey(item: History): Int = item.id + 1 + override fun getKey(item: History): Int = item.position!! + 1 override fun loadInitial( params: LoadInitialParams, callback: LoadInitialCallback ) { - historyProvider.getHistory(INITIAL_OFFSET, params.requestedLoadSize.toLong()) { history -> - callback.onResult(history) + historyProvider.getHistory(INITIAL_OFFSET, params.requestedLoadSize) { history -> + val positionedHistory = assignPositions(offset = INITIAL_OFFSET, history) + callback.onResult(positionedHistory) } } override fun loadAfter(params: LoadParams, callback: LoadCallback) { - historyProvider.getHistory(params.key.toLong(), params.requestedLoadSize.toLong()) { history -> - callback.onResult(history) + historyProvider.getHistory(params.key, params.requestedLoadSize) { history -> + val positionedHistory = assignPositions(offset = params.key, history) + callback.onResult(positionedHistory) } } override fun loadBefore(params: LoadParams, callback: LoadCallback) { /* noop */ } companion object { - private const val INITIAL_OFFSET = 0L + private const val INITIAL_OFFSET = 0 + + @VisibleForTesting + internal fun assignPositions(offset: Int, history: List): List { + return history.foldIndexed(listOf()) { index, prev, item -> + // Only offset once while folding, so that we don't accumulate the offset for each element. + val itemOffset = if (index == 0) { + offset + } else { + 0 + } + val previousPosition = prev.lastOrNull()?.position ?: 0 + when (item) { + is History.Group -> { + // XXX considering an empty group to have a non-zero offset is the obvious + // limitation of the current approach, and indicates that we're conflating + // two concepts here - position of an element for the sake of a RecyclerView, + // and an offset for the sake of our history pagination API. + val groupOffset = if (item.items.isEmpty()) { + 1 + } else { + item.items.size + } + prev + item.copy(position = previousPosition + itemOffset + groupOffset) + } + is History.Metadata -> { + prev + item.copy(position = previousPosition + itemOffset + 1) + } + is History.Regular -> { + prev + item.copy(position = previousPosition + itemOffset + 1) + } + } + } + } } } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt index 587e292f80..2fe0d98ef3 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt @@ -17,7 +17,7 @@ import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl * Class representing a history entry. */ sealed class History : Parcelable { - abstract val id: Int + abstract val position: Int? abstract val title: String abstract val visitedAt: Long abstract val selected: Boolean @@ -32,7 +32,7 @@ sealed class History : Parcelable { * @property selected Whether or not the history item is selected. */ @Parcelize data class Regular( - override val id: Int, + override val position: Int? = null, override val title: String, val url: String, override val visitedAt: Long, @@ -52,7 +52,7 @@ sealed class History : Parcelable { * @property selected Whether or not the history metadata item is selected. */ @Parcelize data class Metadata( - override val id: Int, + override val position: Int? = null, override val title: String, val url: String, override val visitedAt: Long, @@ -71,7 +71,7 @@ sealed class History : Parcelable { * @property selected Whether or not the history group is selected. */ @Parcelize data class Group( - override val id: Int, + override val position: Int? = null, override val title: String, override val visitedAt: Long, val items: List, @@ -82,9 +82,9 @@ sealed class History : Parcelable { /** * Extension function for converting a [HistoryMetadata] into a [History.Metadata]. */ -fun HistoryMetadata.toHistoryMetadata(): History.Metadata { +fun HistoryMetadata.toHistoryMetadata(position: Int? = null): History.Metadata { return History.Metadata( - id = createdAt.toInt(), + position = position, title = title?.takeIf(String::isNotEmpty) ?: key.url.tryGetHostFromUrl(), url = key.url, diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt index 24f818367c..c771b76c04 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt @@ -76,7 +76,7 @@ class HistoryView( val unselectedItems = oldMode.selectedItems - state.mode.selectedItems state.mode.selectedItems.union(unselectedItems).forEach { item -> - historyAdapter.notifyItemChanged(item.id) + historyAdapter.notifyItemChanged(item.position!!) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupAdapter.kt b/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupAdapter.kt index e0a5d6bbbe..524b784055 100644 --- a/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupAdapter.kt @@ -46,7 +46,7 @@ class HistoryMetadataGroupAdapter( internal object DiffCallback : DiffUtil.ItemCallback() { override fun areContentsTheSame(oldItem: History.Metadata, newItem: History.Metadata): Boolean = - oldItem.id == newItem.id + oldItem.position == newItem.position override fun areItemsTheSame(oldItem: History.Metadata, newItem: History.Metadata): Boolean = oldItem == newItem diff --git a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt index 81a5742a75..986b2f8ffe 100644 --- a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt @@ -93,7 +93,7 @@ class PagedHistoryProviderTest { coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1, historyEntry2, historyEntry3) var actualResults: List? = null - provider.getHistory(10L, 5) { + provider.getHistory(10, 5) { actualResults = it } @@ -115,13 +115,11 @@ class PagedHistoryProviderTest { val results = listOf( History.Group( - id = historyEntry1.createdAt.toInt(), title = historyEntry1.key.searchTerm!!, visitedAt = historyEntry1.createdAt, // Results are de-duped by URL and sorted descending by createdAt/visitedAt items = listOf( History.Metadata( - id = historyEntry1.createdAt.toInt(), title = historyEntry1.title!!, url = historyEntry1.key.url, visitedAt = historyEntry1.createdAt, @@ -129,7 +127,6 @@ class PagedHistoryProviderTest { historyMetadataKey = historyMetadataKey1 ), History.Metadata( - id = historyEntry3.createdAt.toInt(), title = historyEntry3.title!!, url = historyEntry3.key.url, visitedAt = historyEntry3.createdAt, @@ -139,7 +136,6 @@ class PagedHistoryProviderTest { ) ), History.Regular( - id = 12, title = visitInfo3.title!!, url = visitInfo3.url, visitedAt = visitInfo3.visitTime @@ -180,7 +176,7 @@ class PagedHistoryProviderTest { coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1) var actualResults: List? = null - provider.getHistory(0L, 5) { + provider.getHistory(0, 5) { actualResults = it } @@ -202,13 +198,11 @@ class PagedHistoryProviderTest { val results = listOf( History.Group( - id = historyEntry1.createdAt.toInt(), title = historyEntry1.key.searchTerm!!, visitedAt = historyEntry1.createdAt, // Results are de-duped by URL and sorted descending by createdAt/visitedAt items = listOf( History.Metadata( - id = historyEntry1.createdAt.toInt(), title = historyEntry1.title!!, url = historyEntry1.key.url, visitedAt = historyEntry1.createdAt, @@ -254,7 +248,7 @@ class PagedHistoryProviderTest { coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1) var actualResults: List? = null - provider.getHistory(0L, 5) { + provider.getHistory(0, 5) { actualResults = it } @@ -276,13 +270,11 @@ class PagedHistoryProviderTest { val results = listOf( History.Group( - id = historyEntry1.createdAt.toInt(), title = historyEntry1.key.searchTerm!!, visitedAt = historyEntry1.createdAt, // Results are de-duped by URL and sorted descending by createdAt/visitedAt items = listOf( History.Metadata( - id = historyEntry1.createdAt.toInt(), title = historyEntry1.title!!, url = historyEntry1.key.url, visitedAt = historyEntry1.createdAt, @@ -405,7 +397,7 @@ class PagedHistoryProviderTest { coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1, historyEntry2, historyEntry3, historyEntry4) var actualResults: List? = null - provider.getHistory(10L, 5) { + provider.getHistory(10, 5) { actualResults = it } @@ -427,12 +419,10 @@ class PagedHistoryProviderTest { 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, @@ -440,7 +430,6 @@ class PagedHistoryProviderTest { historyMetadataKey = historyMetadataKey2 ), History.Metadata( - id = historyEntry1.createdAt.toInt(), title = historyEntry1.title!!, url = historyEntry1.key.url, visitedAt = historyEntry1.createdAt, @@ -457,37 +446,31 @@ class PagedHistoryProviderTest { fun `WHEN removeConsecutiveDuplicates is called THEN all consecutive duplicates must be removed`() { val results = listOf( History.Group( - id = 1, title = "Group 1", visitedAt = 0, items = emptyList() ), History.Regular( - id = 2, title = "No duplicate item", url = "url", visitedAt = 0 ), History.Regular( - id = 3, title = "Duplicate item 1", url = "url", visitedAt = 0 ), History.Regular( - id = 4, title = "Duplicate item 2", url = "url", visitedAt = 0 ), History.Group( - id = 5, title = "Group 5", visitedAt = 0, items = emptyList() ), History.Regular( - id = 6, title = "No duplicate item", url = "url", visitedAt = 0 @@ -496,25 +479,21 @@ class PagedHistoryProviderTest { val expectedList = listOf( History.Group( - id = 1, title = "Group 1", visitedAt = 0, items = emptyList() ), History.Regular( - id = 2, title = "No duplicate item", url = "url", visitedAt = 0 ), History.Group( - id = 5, title = "Group 5", visitedAt = 0, items = emptyList() ), History.Regular( - id = 6, title = "No duplicate item", url = "url", visitedAt = 0 diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryAdapterTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryAdapterTest.kt index 550f8b6119..8bfe6dc93a 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryAdapterTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryAdapterTest.kt @@ -14,7 +14,6 @@ class HistoryAdapterTest { @Test fun `WHEN grouping history item with future date THEN item is grouped to today`() { val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = System.currentTimeMillis() + DateUtils.WEEK_IN_MILLIS @@ -27,7 +26,6 @@ class HistoryAdapterTest { @Test fun `WHEN grouping history item with today's date THEN item is grouped to today`() { val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = System.currentTimeMillis() - DateUtils.MINUTE_IN_MILLIS @@ -46,7 +44,6 @@ class HistoryAdapterTest { } val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = calendar.timeInMillis @@ -65,7 +62,6 @@ class HistoryAdapterTest { } val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = calendar.timeInMillis - DateUtils.HOUR_IN_MILLIS @@ -83,7 +79,6 @@ class HistoryAdapterTest { calendar.set(Calendar.SECOND, 0) val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = calendar.timeInMillis - (DateUtils.HOUR_IN_MILLIS * 23) @@ -102,7 +97,6 @@ class HistoryAdapterTest { } val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = calendar.timeInMillis - (DateUtils.HOUR_IN_MILLIS * 25) @@ -115,7 +109,6 @@ class HistoryAdapterTest { @Test fun `WHEN grouping history item with 3 days ago date THEN item is grouped to this week`() { val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 3) @@ -128,7 +121,6 @@ class HistoryAdapterTest { @Test fun `WHEN grouping history item with 6 days ago date THEN item is grouped to this week`() { val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 6) @@ -141,7 +133,6 @@ class HistoryAdapterTest { @Test fun `WHEN grouping history item with 8 days ago date THEN item is grouped to this month`() { val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 8) @@ -154,7 +145,6 @@ class HistoryAdapterTest { @Test fun `WHEN grouping history item with 29 days ago date THEN item is grouped to this month`() { val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 29) @@ -167,7 +157,6 @@ class HistoryAdapterTest { @Test fun `WHEN grouping history item with 31 days ago date THEN item is grouped to older`() { val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 31) @@ -180,7 +169,6 @@ class HistoryAdapterTest { @Test fun `WHEN grouping history item with zero date THEN item is grouped to older`() { val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = 0 @@ -193,7 +181,6 @@ class HistoryAdapterTest { @Test fun `WHEN grouping history item with negative date THEN item is grouped to older`() { val history = History.Regular( - id = 1, title = "test item", url = "url", visitedAt = -100 diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryDataSourceTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryDataSourceTest.kt new file mode 100644 index 0000000000..87a469c6e2 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryDataSourceTest.kt @@ -0,0 +1,204 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.library.history + +import mozilla.components.concept.storage.HistoryMetadataKey +import org.junit.Assert.assertEquals +import org.junit.Test + +class HistoryDataSourceTest { + private val testCases = listOf( + listOf() to listOf(), + + listOf(1) to listOf( + TestHistory.Regular("http://www.mozilla.com") + ), + listOf(1, 2) to listOf( + TestHistory.Regular("http://www.mozilla.com"), + TestHistory.Regular("http://www.mozilla.com/2") + ), + listOf(1, 2) to listOf( + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Regular("http://www.mozilla.com/2") + ), + listOf(1, 2, 3) to listOf( + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Regular("http://www.mozilla.com/2"), + TestHistory.Metadata("http://www.mozilla.com"), + ), + listOf(1, 2, 3, 4, 5) to listOf( + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Regular("http://www.mozilla.com/2"), + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Regular("http://www.mozilla.com/3"), + TestHistory.Regular("http://www.mozilla.com/2"), + ), + listOf(1, 2, 3) to listOf( + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Regular("http://www.mozilla.com/2"), + TestHistory.Group("firefox", items = listOf()) + ), + listOf(1, 2, 3) to listOf( + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Regular("http://www.mozilla.com/2"), + TestHistory.Group( + "firefox", + items = listOf( + "http://www.firefox.com" + ) + ) + ), + listOf(1, 2, 7) to listOf( + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Regular("http://www.mozilla.com/2"), + TestHistory.Group( + "firefox", + items = listOf( + "http://www.firefox.com", + "http://www.firefox.com/2", + "http://www.firefox.com/3", + "http://www.firefox.com/4", + "http://www.firefox.com/5" + ) + ) + ), + listOf(5, 6, 7) to listOf( + TestHistory.Group( + "firefox", + items = listOf( + "http://www.firefox.com", + "http://www.firefox.com/2", + "http://www.firefox.com/3", + "http://www.firefox.com/4", + "http://www.firefox.com/5" + ) + ), + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Regular("http://www.mozilla.com/2") + ), + listOf(1, 6, 7) to listOf( + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Group( + "firefox", + items = listOf( + "http://www.firefox.com", + "http://www.firefox.com/2", + "http://www.firefox.com/3", + "http://www.firefox.com/4", + "http://www.firefox.com/5" + ) + ), + TestHistory.Regular("http://www.mozilla.com/2") + ), + listOf(1, 6, 8, 9) to listOf( + TestHistory.Metadata("http://www.mozilla.com"), + TestHistory.Group( + "firefox", + items = listOf( + "http://www.firefox.com", + "http://www.firefox.com/2", + "http://www.firefox.com/3", + "http://www.firefox.com/4", + "http://www.firefox.com/5" + ) + ), + TestHistory.Group( + "mdn", + items = listOf( + "https://developer.mozilla.org/en-US/1", + "https://developer.mozilla.org/en-US/2" + ) + ), + TestHistory.Regular("http://www.mozilla.com/2") + ), + listOf(1) to listOf( + TestHistory.Group( + "mozilla", + items = listOf( + "http://www.mozilla.com" + ) + ) + ) + ) + + @Test + fun `assign positions basics - initial offset`() { + testCases.forEach { + verifyPositions(it.first, offset = 0, it.second) + } + } + + @Test + fun `assign position basics - positive offset`() { + val offset = 25 + testCases.forEach { + verifyPositions(it.first.map { pos -> pos + offset }, offset = offset, it.second) + } + } + + @Test + fun `assign position basics - negative offset`() { + // Even though conceptually it doesn't make sense for us to handle negative offsets, + // as far as simple positioning logic is concerned there's no harm in doing the naive thing. + // Assertions around offset being a positive value should happen elsewhere, before we're + // even dealing with positions. + val offset = -25 + testCases.forEach { + verifyPositions(it.first.map { pos -> pos + offset }, offset = offset, it.second) + } + } + + private fun verifyPositions(expectedPositions: List, offset: Int, history: List) { + assertEquals( + "For case $history with offset $offset", + expectedPositions, + HistoryDataSource.assignPositions( + offset = offset, + history.toHistoryWithoutPositions() + ).map { it.position } + ) + } + + private sealed class TestHistory { + data class Regular(val url: String) : TestHistory() + data class Metadata(val url: String) : TestHistory() + data class Group(val title: String, val items: List) : TestHistory() + } + + // For position tests, we just care about the basic tree structure here, + // the details (view times, timestamps, etc) don't matter. + private fun List.toHistoryWithoutPositions(): List { + return this.map { + when (it) { + is TestHistory.Regular -> { + History.Regular(title = it.url, url = it.url, visitedAt = 0) + } + is TestHistory.Metadata -> { + History.Metadata( + title = it.url, + url = it.url, + visitedAt = 0, + totalViewTime = 0, + historyMetadataKey = HistoryMetadataKey(url = it.url) + ) + } + is TestHistory.Group -> { + History.Group( + title = it.title, visitedAt = 0, + items = it.items.map { item -> + History.Metadata( + title = item, + url = item, + visitedAt = 0, + totalViewTime = 0, + historyMetadataKey = HistoryMetadataKey(url = item) + ) + } + ) + } + } + } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt index 8552851bb9..87ff396175 100644 --- a/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt @@ -19,7 +19,6 @@ class HistoryMetadataGroupFragmentStoreTest { private lateinit var store: HistoryMetadataGroupFragmentStore private val mozillaHistoryMetadataItem = History.Metadata( - id = 0, title = "Mozilla", url = "mozilla.org", visitedAt = 0, @@ -27,7 +26,6 @@ class HistoryMetadataGroupFragmentStoreTest { historyMetadataKey = HistoryMetadataKey("http://www.mozilla.com", "mozilla", null) ) private val firefoxHistoryMetadataItem = History.Metadata( - id = 0, title = "Firefox", url = "firefox.com", visitedAt = 0, diff --git a/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt index 23db70cef6..196c0ab854 100644 --- a/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt @@ -53,7 +53,6 @@ class HistoryMetadataGroupControllerTest { private val searchTerm = "mozilla" private val historyMetadataKey = HistoryMetadataKey("http://www.mozilla.com", searchTerm, null) private val mozillaHistoryMetadataItem = History.Metadata( - id = 0, title = "Mozilla", url = "mozilla.org", visitedAt = 0, @@ -61,7 +60,6 @@ class HistoryMetadataGroupControllerTest { historyMetadataKey = historyMetadataKey ) private val firefoxHistoryMetadataItem = History.Metadata( - id = 0, title = "Firefox", url = "firefox.com", visitedAt = 0, diff --git a/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt index de0458f099..574b5a8bdf 100644 --- a/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt @@ -29,7 +29,6 @@ class HistoryMetadataGroupItemViewHolderTest { private lateinit var selectionHolder: SelectionHolder private val item = History.Metadata( - id = 0, title = "Mozilla", url = "mozilla.org", visitedAt = 0,