diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index 0a1519a1d..a623c9c5c 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -42,5 +42,5 @@ object FeatureFlags { /** * Enables recording of history metadata. */ - const val historyMetadataFeature = false + val historyMetadataFeature = Config.channel.isDebug } diff --git a/app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt b/app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt index 09376083d..fe936688d 100644 --- a/app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddleware.kt @@ -18,6 +18,7 @@ import mozilla.components.feature.search.ext.parseSearchTerms import mozilla.components.lib.state.Middleware import mozilla.components.lib.state.MiddlewareContext import mozilla.components.lib.state.Store +import mozilla.components.support.base.log.logger.Logger /** * This [Middleware] reacts to various browsing events and records history metadata as needed. @@ -26,6 +27,8 @@ class HistoryMetadataMiddleware( private val historyMetadataService: HistoryMetadataService ) : Middleware { + private val logger = Logger("HistoryMetadataMiddleware") + @Suppress("ComplexMethod") override fun invoke( context: MiddlewareContext, @@ -117,11 +120,22 @@ class HistoryMetadataMiddleware( } previousUrlIndex >= 0 -> { val previousUrl = tab.content.history.items[previousUrlIndex].uri - context.state.search.parseSearchTerms(previousUrl) to previousUrl + val searchTerms = context.state.search.parseSearchTerms(previousUrl) + if (searchTerms != null) { + searchTerms to previousUrl + } else { + null to null + } } else -> null to null } + // Sanity check to make sure we don't record a metadata record referring to itself. + if (tab.content.url == referrerUrl) { + logger.debug("Current url same as referrer. Skipping metadata recording.") + return + } + val key = historyMetadataService.createMetadata(tab, searchTerm, referrerUrl) context.dispatch(HistoryMetadataAction.SetHistoryMetadataKeyAction(tab.id, key)) } diff --git a/app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt index bb844a026..735db8438 100644 --- a/app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt +++ b/app/src/test/java/org/mozilla/fenix/historymetadata/HistoryMetadataMiddlewareTest.kt @@ -92,6 +92,17 @@ class HistoryMetadataMiddlewareTest { } } + @Test + fun `GIVEN normal tab has parent WHEN url is the same THEN nothing happens`() { + val parentTab = createTab("https://mozilla.org", searchTerms = "mozilla website") + val tab = createTab("https://mozilla.org", parent = parentTab) + store.dispatch(TabListAction.AddTabAction(parentTab)).joinBlocking() + store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() + + store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, emptyList(), currentIndex = 0)).joinBlocking() + verify { service wasNot Called } + } + @Test fun `GIVEN normal tab has no parent WHEN history metadata is recorded THEN search terms and referrer url are provided`() { val tab = createTab("https://mozilla.org") @@ -127,6 +138,41 @@ class HistoryMetadataMiddlewareTest { } } + @Test + fun `GIVEN normal tab has no parent WHEN history metadata is recorded without search terms THEN no referrer is provided`() { + val tab = createTab("https://mozilla.org") + store.dispatch(TabListAction.AddTabAction(tab)).joinBlocking() + store.dispatch(SearchAction.SetSearchEnginesAction( + regionSearchEngines = listOf( + SearchEngine( + id = "google", + name = "Google", + icon = mock(), + type = SearchEngine.Type.BUNDLED, + resultUrls = listOf("https://google.com?q={searchTerms}") + ) + ), + userSelectedSearchEngineId = null, + userSelectedSearchEngineName = null, + regionDefaultSearchEngineId = "google", + customSearchEngines = emptyList(), + hiddenSearchEngines = emptyList(), + additionalAvailableSearchEngines = emptyList(), + additionalSearchEngines = emptyList(), + regionSearchEnginesOrder = listOf("google") + )).joinBlocking() + + val historyState = listOf( + HistoryItem("firefox", "https://mozilla.org"), + HistoryItem("mozilla", "https://firefox.com") + ) + store.dispatch(ContentAction.UpdateHistoryStateAction(tab.id, historyState, currentIndex = 1)).joinBlocking() + + verify { + service.createMetadata(any(), null, null) + } + } + @Test fun `GIVEN private tab WHEN loading completed THEN no meta data is recorded`() { val tab = createTab("https://mozilla.org", private = true)