From cc82bfc23b5690a27f74784d8ea16f84c06d0369 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Mon, 23 Sep 2024 11:38:53 +0200 Subject: [PATCH 01/15] add forward slash if the url has no path This is going to make it easier for us to search for the url in the DB as we store the fully resolved url for each tab e.g. bbc.com entered in Omnibar ends up as "https://www.bbc.com/" in the DB I also added missing tests for non http/https schemes --- .../ShowOnAppLaunchUrlConverterImpl.kt | 18 ++++++-- .../ShowOnAppLaunchUrlConverterImplTest.kt | 46 +++++++++++++------ 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchUrlConverterImpl.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchUrlConverterImpl.kt index 15f4c86f42c2..bb218d89505a 100644 --- a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchUrlConverterImpl.kt +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchUrlConverterImpl.kt @@ -26,8 +26,10 @@ class ShowOnAppLaunchUrlConverterImpl : UrlConverter { val uri = Uri.parse(url.trim()) - val convertedUri = if (uri.scheme == null) { - Uri.Builder().scheme("http").authority(uri.path?.lowercase()) + val uriWithScheme = if (uri.scheme == null) { + Uri.Builder() + .scheme("http") + .authority(uri.path?.lowercase()) } else { uri.buildUpon() .scheme(uri.scheme?.lowercase()) @@ -37,9 +39,15 @@ class ShowOnAppLaunchUrlConverterImpl : UrlConverter { query(uri.query) fragment(uri.fragment) } - .build() - .toString() - return Uri.decode(convertedUri) + val uriWithPath = if (uri.path.isNullOrBlank()) { + uriWithScheme.path("/") + } else { + uriWithScheme + } + + val processedUrl = uriWithPath.build().toString() + + return Uri.decode(processedUrl) } } diff --git a/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchUrlConverterImplTest.kt b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchUrlConverterImplTest.kt index ed3109a79593..45a87f1c1ec9 100644 --- a/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchUrlConverterImplTest.kt +++ b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchUrlConverterImplTest.kt @@ -58,15 +58,21 @@ class ShowOnAppLaunchUrlConverterImplTest { } @Test - fun whenUrlHasASchemeThenShouldReturnTheSameUrl() { + fun whenUrlDoesNotHaveAPathThenForwardSlashIsAdded() { val result = urlConverter.convertUrl("https://www.example.com") - assertEquals("https://www.example.com", result) + assertEquals("https://www.example.com/", result) + } + + @Test + fun whenUrlHasASchemeThenShouldReturnTheSameUrl() { + val result = urlConverter.convertUrl("https://www.example.com/") + assertEquals("https://www.example.com/", result) } @Test fun whenUrlHasDifferentSchemeThenShouldReturnTheSameUrl() { - val result = urlConverter.convertUrl("ftp://www.example.com") - assertEquals("ftp://www.example.com", result) + val result = urlConverter.convertUrl("ftp://www.example.com/") + assertEquals("ftp://www.example.com/", result) } @Test @@ -77,8 +83,8 @@ class ShowOnAppLaunchUrlConverterImplTest { @Test fun whenUrlHasPortThenShouldReturnTheSameUrl() { - val result = urlConverter.convertUrl("https://www.example.com:8080") - assertEquals("https://www.example.com:8080", result) + val result = urlConverter.convertUrl("https://www.example.com:8080/") + assertEquals("https://www.example.com:8080/", result) } @Test @@ -89,26 +95,26 @@ class ShowOnAppLaunchUrlConverterImplTest { @Test fun whenUrlHasUppercaseProtocolThenShouldLowercaseProtocol() { - val result = urlConverter.convertUrl("HTTPS://www.example.com") - assertEquals("https://www.example.com", result) + val result = urlConverter.convertUrl("HTTPS://www.example.com/") + assertEquals("https://www.example.com/", result) } @Test fun whenUrlHasUppercaseSubdomainThenShouldLowercaseSubdomain() { - val result = urlConverter.convertUrl("https://WWW.example.com") - assertEquals("https://www.example.com", result) + val result = urlConverter.convertUrl("https://WWW.example.com/") + assertEquals("https://www.example.com/", result) } @Test fun whenUrlHasUppercaseDomainThenShouldLowercaseDomain() { - val result = urlConverter.convertUrl("https://www.EXAMPLE.com") - assertEquals("https://www.example.com", result) + val result = urlConverter.convertUrl("https://www.EXAMPLE.com/") + assertEquals("https://www.example.com/", result) } @Test fun whenUrlHasUppercaseTopLevelDomainThenShouldLowercaseTopLevelDomain() { - val result = urlConverter.convertUrl("https://www.example.COM") - assertEquals("https://www.example.com", result) + val result = urlConverter.convertUrl("https://www.example.COM/") + assertEquals("https://www.example.com/", result) } @Test @@ -122,4 +128,16 @@ class ShowOnAppLaunchUrlConverterImplTest { val result = urlConverter.convertUrl("example") assertEquals("http://example", result) } + + @Test + fun whenUrlHasADifferentSchemeThenSameUrlReturned() { + val result = urlConverter.convertUrl("ftp://example.com/") + assertEquals("ftp://example.com/", result) + } + + @Test + fun whenUrlHasADifferentSchemeAndNoTrailingSlashThenTrailingSlashAdded() { + val result = urlConverter.convertUrl("ftp://example.com") + assertEquals("ftp://example.com/", result) + } } From 17e56b74dde311b14f3bc7ac2b970276a4d1c913 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Wed, 9 Oct 2024 09:54:49 +0100 Subject: [PATCH 02/15] extract ShowOnAppLaunch logic to own class This is extracting the existing behaviour, in a future commit we'll fetch the resolved url Added a test to BrowserViewModelTest which isn't massively helpful but better than nothing --- .../app/browser/BrowserViewModel.kt | 22 ++----- .../ShowOnAppLaunchOptionHandler.kt | 66 +++++++++++++++++++ .../app/tabs/model/TabDataRepository.kt | 6 +- .../app/browser/BrowserViewModelTest.kt | 51 ++++---------- .../app/tabs/model/TabRepository.kt | 2 + 5 files changed, 88 insertions(+), 59 deletions(-) create mode 100644 app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index ef21a246db76..b536ba294a05 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -27,10 +27,7 @@ import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter import com.duckduckgo.app.fire.DataClearer import com.duckduckgo.app.generalsettings.showonapplaunch.ShowOnAppLaunchFeature -import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.LastOpenedTab -import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.NewTabPage -import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.SpecificPage -import com.duckduckgo.app.generalsettings.showonapplaunch.store.ShowOnAppLaunchOptionDataStore +import com.duckduckgo.app.generalsettings.showonapplaunch.ShowOnAppLaunchOptionHandler import com.duckduckgo.app.global.ApplicationClearDataState import com.duckduckgo.app.global.rating.AppEnjoymentPromptEmitter import com.duckduckgo.app.global.rating.AppEnjoymentPromptOptions @@ -58,12 +55,12 @@ import com.duckduckgo.common.utils.SingleLiveEvent import com.duckduckgo.di.scopes.ActivityScope import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.feature.toggles.api.Toggle -import javax.inject.Inject -import kotlin.coroutines.CoroutineContext import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import timber.log.Timber +import javax.inject.Inject +import kotlin.coroutines.CoroutineContext @ContributesViewModel(ActivityScope::class) class BrowserViewModel @Inject constructor( @@ -77,7 +74,7 @@ class BrowserViewModel @Inject constructor( private val pixel: Pixel, private val skipUrlConversionOnNewTabFeature: SkipUrlConversionOnNewTabFeature, private val showOnAppLaunchFeature: ShowOnAppLaunchFeature, - private val showOnAppLaunchOptionDataStore: ShowOnAppLaunchOptionDataStore, + private val showOnAppLaunchOptionHandler: ShowOnAppLaunchOptionHandler, ) : ViewModel(), CoroutineScope { @@ -297,16 +294,7 @@ class BrowserViewModel @Inject constructor( fun handleShowOnAppLaunchOption() { if (showOnAppLaunchFeature.self().isEnabled()) { viewModelScope.launch { - when (val option = showOnAppLaunchOptionDataStore.optionFlow.first()) { - LastOpenedTab -> Unit - NewTabPage -> onNewTabRequested() - is SpecificPage -> { - val liveSelectedTabUrl = tabRepository.getSelectedTab()?.url - if (liveSelectedTabUrl != option.url) { - onOpenInNewTabRequested(option.url) - } - } - } + showOnAppLaunchOptionHandler.handleAppLaunchOption() } } } diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt new file mode 100644 index 000000000000..80a0d26c0faa --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.generalsettings.showonapplaunch + +import android.net.Uri +import androidx.core.net.toUri +import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.LastOpenedTab +import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.NewTabPage +import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.SpecificPage +import com.duckduckgo.app.generalsettings.showonapplaunch.store.ShowOnAppLaunchOptionDataStore +import com.duckduckgo.app.tabs.model.TabRepository +import com.duckduckgo.common.utils.toHttps +import com.duckduckgo.di.scopes.AppScope +import com.squareup.anvil.annotations.ContributesBinding +import kotlinx.coroutines.flow.first +import javax.inject.Inject + +interface ShowOnAppLaunchOptionHandler { + suspend fun handleAppLaunchOption() +} + +@ContributesBinding(AppScope::class) +class ShowOnAppLaunchOptionHandlerImpl @Inject constructor( + private val showOnAppLaunchOptionDataStore: ShowOnAppLaunchOptionDataStore, + private val tabRepository: TabRepository, +): ShowOnAppLaunchOptionHandler { + + override suspend fun handleAppLaunchOption() { + when (val option = showOnAppLaunchOptionDataStore.optionFlow.first()) { + LastOpenedTab -> Unit + NewTabPage -> tabRepository.add() + is SpecificPage -> handleSpecificPageOption(option) + } + } + + private suspend fun handleSpecificPageOption(option: SpecificPage) { + val uri = option.url.toUri() + + if (isTabAlreadyAdded(uri)) { + tabRepository.select(option.url) + } else { + tabRepository.add(option.url) + } + } + + private suspend fun isTabAlreadyAdded(uri: Uri): Boolean { + val tabId = tabRepository.getTabId(uri.toString()) + val httpsTabId = tabRepository.getTabId(uri.toHttps.toString()) + + return tabId != null || httpsTabId != null + } +} diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt index f34550357e0f..aca6fb71eab8 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt @@ -35,8 +35,6 @@ import com.duckduckgo.di.scopes.AppScope import dagger.SingleInstanceIn import io.reactivex.Scheduler import io.reactivex.schedulers.Schedulers -import java.util.UUID -import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow @@ -47,6 +45,8 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import timber.log.Timber +import java.util.UUID +import javax.inject.Inject @SingleInstanceIn(AppScope::class) class TabDataRepository @Inject constructor( @@ -179,6 +179,8 @@ class TabDataRepository @Inject constructor( } } + override suspend fun getTabId(url: String): String? = tabsDao.selectTabByUrl(url) + override suspend fun setIsUserNew(isUserNew: Boolean) { if (tabSwitcherDataStore.data.first().userState == UserState.UNKNOWN) { val userState = if (isUserNew) UserState.NEW else UserState.EXISTING diff --git a/app/src/test/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt b/app/src/test/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt index a4a166090084..90f688fb24fe 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt @@ -24,8 +24,7 @@ import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter import com.duckduckgo.app.fire.DataClearer import com.duckduckgo.app.generalsettings.showonapplaunch.ShowOnAppLaunchFeature -import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption -import com.duckduckgo.app.generalsettings.showonapplaunch.store.ShowOnAppLaunchOptionDataStore +import com.duckduckgo.app.generalsettings.showonapplaunch.ShowOnAppLaunchOptionHandler import com.duckduckgo.app.global.rating.AppEnjoymentPromptEmitter import com.duckduckgo.app.global.rating.AppEnjoymentPromptOptions import com.duckduckgo.app.global.rating.AppEnjoymentUserEventRecorder @@ -38,7 +37,6 @@ import com.duckduckgo.app.tabs.model.TabRepository import com.duckduckgo.common.test.CoroutineTestRule import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory import com.duckduckgo.feature.toggles.api.Toggle.State -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Assert.assertEquals @@ -48,7 +46,12 @@ import org.junit.Rule import org.junit.Test import org.mockito.Mock import org.mockito.MockitoAnnotations -import org.mockito.kotlin.* +import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.doReturn +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever class BrowserViewModelTest { @@ -76,7 +79,7 @@ class BrowserViewModelTest { @Mock private lateinit var mockDefaultBrowserDetector: DefaultBrowserDetector - @Mock private lateinit var showOnAppLaunchOptionDataStore: ShowOnAppLaunchOptionDataStore + @Mock private lateinit var showOnAppLaunchOptionHandler: ShowOnAppLaunchOptionHandler private val fakeShowOnAppLaunchFeatureToggle = FakeFeatureToggleFactory.create(ShowOnAppLaunchFeature::class.java) @@ -257,48 +260,16 @@ class BrowserViewModelTest { } @Test - fun whenAppOpenAndLastOpenedTabSetThenNoTabsAdded() = runTest { - whenever(showOnAppLaunchOptionDataStore.optionFlow) - .thenReturn(flowOf(ShowOnAppLaunchOption.LastOpenedTab)) - - testee.handleShowOnAppLaunchOption() - - verify(mockTabRepository, never()).add(url = any(), skipHome = any()) - verify(mockTabRepository, never()).addFromSourceTab(url = any(), skipHome = any(), sourceTabId = any()) - verify(mockTabRepository, never()).addDefaultTab() - } - - @Test - fun whenAppOpenAndNewTabPageSetThenNewTabAdded() = runTest { - whenever(showOnAppLaunchOptionDataStore.optionFlow) - .thenReturn(flowOf(ShowOnAppLaunchOption.NewTabPage)) - - testee.handleShowOnAppLaunchOption() - - verify(mockTabRepository, atMost(1)).add() - verify(mockTabRepository, never()).addFromSourceTab(url = any(), skipHome = any(), sourceTabId = any()) - verify(mockTabRepository, never()).addDefaultTab() - } - - @Test - fun whenAppOpenAndSpecificPageSetThenNewTabAddedWithUrl() = runTest { - whenever(showOnAppLaunchOptionDataStore.optionFlow) - .thenReturn(flowOf(ShowOnAppLaunchOption.SpecificPage("example.com"))) - + fun whenHandleShowOnAppLaunchCalledThenShowOnAppLaunchHandled() = runTest { testee.handleShowOnAppLaunchOption() - verify(mockTabRepository, atMost(1)).add(url = "example.com", skipHome = false) - verify(mockTabRepository, never()).addFromSourceTab(url = any(), skipHome = any(), sourceTabId = any()) - verify(mockTabRepository, never()).addDefaultTab() + verify(showOnAppLaunchOptionHandler).handleAppLaunchOption() } @Test fun whenShowOnAppLaunchFeatureToggleIsOffAndNewTabPageIsSetThenNoTabIsAdded() = runTest { fakeShowOnAppLaunchFeatureToggle.self().setRawStoredState(State(enable = false)) - whenever(showOnAppLaunchOptionDataStore.optionFlow) - .thenReturn(flowOf(ShowOnAppLaunchOption.NewTabPage)) - testee.handleShowOnAppLaunchOption() verify(mockTabRepository, never()).add() @@ -318,7 +289,7 @@ class BrowserViewModelTest { pixel = mockPixel, skipUrlConversionOnNewTabFeature = skipUrlConversionOnNewTabFeature, showOnAppLaunchFeature = fakeShowOnAppLaunchFeatureToggle, - showOnAppLaunchOptionDataStore = showOnAppLaunchOptionDataStore, + showOnAppLaunchOptionHandler = showOnAppLaunchOptionHandler, ) } diff --git a/browser-api/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt b/browser-api/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt index d9adb1780081..0e7e877c8020 100644 --- a/browser-api/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt +++ b/browser-api/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt @@ -109,6 +109,8 @@ interface TabRepository { suspend fun selectByUrlOrNewTab(url: String) + suspend fun getTabId(url: String): String? + suspend fun setIsUserNew(isUserNew: Boolean) suspend fun setTabLayoutType(layoutType: LayoutType) From 347f09ff7d1f87a985afeb7ac76fb4288d96feb5 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Wed, 9 Oct 2024 17:42:22 +0100 Subject: [PATCH 03/15] update logic to remove scheme and subdomain from url --- .../ShowOnAppLaunchOptionHandler.kt | 22 ++++++++++++------- .../duckduckgo/common/utils/UriExtension.kt | 3 +++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt index 80a0d26c0faa..69fc3382afdb 100644 --- a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt @@ -23,7 +23,7 @@ import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchO import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.SpecificPage import com.duckduckgo.app.generalsettings.showonapplaunch.store.ShowOnAppLaunchOptionDataStore import com.duckduckgo.app.tabs.model.TabRepository -import com.duckduckgo.common.utils.toHttps +import com.duckduckgo.common.utils.isHttpOrHttps import com.duckduckgo.di.scopes.AppScope import com.squareup.anvil.annotations.ContributesBinding import kotlinx.coroutines.flow.first @@ -50,17 +50,23 @@ class ShowOnAppLaunchOptionHandlerImpl @Inject constructor( private suspend fun handleSpecificPageOption(option: SpecificPage) { val uri = option.url.toUri() - if (isTabAlreadyAdded(uri)) { - tabRepository.select(option.url) + val url = if (uri.isHttpOrHttps) { + stripUri(uri) + } else { + option.url + } + + val existingTabId = tabRepository.getTabId(url) + + if (existingTabId != null) { + tabRepository.select(existingTabId) } else { tabRepository.add(option.url) } } - private suspend fun isTabAlreadyAdded(uri: Uri): Boolean { - val tabId = tabRepository.getTabId(uri.toString()) - val httpsTabId = tabRepository.getTabId(uri.toHttps.toString()) - - return tabId != null || httpsTabId != null + private fun stripUri(uri: Uri): String { + val host = uri.host?.removePrefix("www.") ?: "" + return "$host${uri.path.orEmpty()}" } } diff --git a/common/common-utils/src/main/java/com/duckduckgo/common/utils/UriExtension.kt b/common/common-utils/src/main/java/com/duckduckgo/common/utils/UriExtension.kt index 5dbb21893018..0156af20d780 100644 --- a/common/common-utils/src/main/java/com/duckduckgo/common/utils/UriExtension.kt +++ b/common/common-utils/src/main/java/com/duckduckgo/common/utils/UriExtension.kt @@ -56,6 +56,9 @@ val Uri.isHttps: Boolean val Uri.toHttps: Uri get() = buildUpon().scheme(UrlScheme.https).build() +val Uri.isHttpOrHttps: Boolean + get() = isHttp || isHttps + val Uri.hasIpHost: Boolean get() { return baseHost?.matches(IP_REGEX) ?: false From dcb3e8090c67e463d0e730e6fdd65ab4aa116b6e Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Thu, 17 Oct 2024 16:48:21 +0100 Subject: [PATCH 04/15] add ability to save a resolved url in the store We'll also use the store to keep hold of the tabId that is in used when setting the SpecificPage so that we can more easily identify when we should be attempting to store this url. We'll see this in a future commit. --- .../model/ShowOnAppLaunchOption.kt | 2 +- .../store/ShowOnAppLaunchOptionDataStore.kt | 30 +++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/model/ShowOnAppLaunchOption.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/model/ShowOnAppLaunchOption.kt index 806794a2df61..4552c471cfbf 100644 --- a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/model/ShowOnAppLaunchOption.kt +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/model/ShowOnAppLaunchOption.kt @@ -24,7 +24,7 @@ sealed class ShowOnAppLaunchOption(val id: Int) { data object LastOpenedTab : ShowOnAppLaunchOption(1) data object NewTabPage : ShowOnAppLaunchOption(2) - data class SpecificPage(val url: String) : ShowOnAppLaunchOption(3) + data class SpecificPage(val url: String, val resolvedUrl: String? = null) : ShowOnAppLaunchOption(3) companion object { diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/ShowOnAppLaunchOptionDataStore.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/ShowOnAppLaunchOptionDataStore.kt index 298800165e4e..c045db82ea41 100644 --- a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/ShowOnAppLaunchOptionDataStore.kt +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/ShowOnAppLaunchOptionDataStore.kt @@ -29,16 +29,20 @@ import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchO import com.duckduckgo.app.generalsettings.showonapplaunch.store.ShowOnAppLaunchOptionDataStore.Companion.DEFAULT_SPECIFIC_PAGE_URL import com.duckduckgo.di.scopes.AppScope import com.squareup.anvil.annotations.ContributesBinding -import javax.inject.Inject +import dagger.SingleInstanceIn import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map +import javax.inject.Inject interface ShowOnAppLaunchOptionDataStore { val optionFlow: Flow val specificPageUrlFlow: Flow + val showOnAppLaunchTabId: String? + fun setShowOnAppLaunchTabId(tabId: String) suspend fun setShowOnAppLaunchOption(showOnAppLaunchOption: ShowOnAppLaunchOption) suspend fun setSpecificPageUrl(url: String) + suspend fun setResolvedPageUrl(url: String) companion object { const val DEFAULT_SPECIFIC_PAGE_URL = "https://duckduckgo.com/" @@ -46,10 +50,14 @@ interface ShowOnAppLaunchOptionDataStore { } @ContributesBinding(AppScope::class) +@SingleInstanceIn(AppScope::class) class ShowOnAppLaunchOptionPrefsDataStore @Inject constructor( @ShowOnAppLaunch private val store: DataStore, ) : ShowOnAppLaunchOptionDataStore { + override var showOnAppLaunchTabId: String? = null + private set + override val optionFlow: Flow = store.data.map { preferences -> preferences[intPreferencesKey(KEY_SHOW_ON_APP_LAUNCH_OPTION)]?.let { optionId -> when (val option = ShowOnAppLaunchOption.mapToOption(optionId)) { @@ -58,7 +66,8 @@ class ShowOnAppLaunchOptionPrefsDataStore @Inject constructor( -> option is SpecificPage -> { val url = preferences[stringPreferencesKey(KEY_SHOW_ON_APP_LAUNCH_SPECIFIC_PAGE_URL)]!! - SpecificPage(url) + val resolvedUrl = preferences[stringPreferencesKey(KEY_SHOW_ON_APP_LAUNCH_SPECIFIC_PAGE_RESOLVED_URL)] + SpecificPage(url, resolvedUrl) } } } ?: LastOpenedTab @@ -74,22 +83,39 @@ class ShowOnAppLaunchOptionPrefsDataStore @Inject constructor( if (showOnAppLaunchOption is SpecificPage) { preferences.setShowOnAppLaunch(showOnAppLaunchOption.url) + preferences.remove(stringPreferencesKey(KEY_SHOW_ON_APP_LAUNCH_SPECIFIC_PAGE_RESOLVED_URL)) + showOnAppLaunchTabId = null } } } + override fun setShowOnAppLaunchTabId(tabId: String) { + showOnAppLaunchTabId = tabId + } + override suspend fun setSpecificPageUrl(url: String) { store.edit { preferences -> preferences.setShowOnAppLaunch(url) } } + override suspend fun setResolvedPageUrl(url: String) { + store.edit { preferences -> + preferences.setShowOnAppLaunchResolvedUrl(url) + } + } + private fun MutablePreferences.setShowOnAppLaunch(url: String) { set(stringPreferencesKey(KEY_SHOW_ON_APP_LAUNCH_SPECIFIC_PAGE_URL), url) } + private fun MutablePreferences.setShowOnAppLaunchResolvedUrl(url: String) { + set(stringPreferencesKey(KEY_SHOW_ON_APP_LAUNCH_SPECIFIC_PAGE_RESOLVED_URL), url) + } + companion object { private const val KEY_SHOW_ON_APP_LAUNCH_OPTION = "SHOW_ON_APP_LAUNCH_OPTION" private const val KEY_SHOW_ON_APP_LAUNCH_SPECIFIC_PAGE_URL = "SHOW_ON_APP_LAUNCH_SPECIFIC_PAGE_URL" + private const val KEY_SHOW_ON_APP_LAUNCH_SPECIFIC_PAGE_RESOLVED_URL = "SHOW_ON_APP_LAUNCH_SPECIFIC_PAGE_RESOLVED_URL" } } From aafb5cc629c5680c15982918db86dd5fdd4f4a12 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Thu, 17 Oct 2024 16:52:31 +0100 Subject: [PATCH 05/15] update ShowOnAppLaunchOptionHandler to check a user and resolved url Now we're storing both the url and resolved urls, we need to check for duplicate tabs. Using a DB query seemed too complex and I think we can assume that we will not have 1000s of tabs to check (maybe it not in my case if XD) --- .../ShowOnAppLaunchOptionHandler.kt | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt index 69fc3382afdb..d8004863f1fb 100644 --- a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt @@ -22,6 +22,7 @@ import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchO import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.NewTabPage import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.SpecificPage import com.duckduckgo.app.generalsettings.showonapplaunch.store.ShowOnAppLaunchOptionDataStore +import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.app.tabs.model.TabRepository import com.duckduckgo.common.utils.isHttpOrHttps import com.duckduckgo.di.scopes.AppScope @@ -48,20 +49,31 @@ class ShowOnAppLaunchOptionHandlerImpl @Inject constructor( } private suspend fun handleSpecificPageOption(option: SpecificPage) { - val uri = option.url.toUri() + val userUri = option.url.toUri() + val resolvedUri = option.resolvedUrl?.toUri() - val url = if (uri.isHttpOrHttps) { - stripUri(uri) - } else { - option.url + val urls = listOfNotNull(userUri, resolvedUri).map { uri -> + stripIfHttpOrHttps(uri) } - val existingTabId = tabRepository.getTabId(url) + val tabIdUrlMap = getTabIdUrlMap(tabRepository.flowTabs.first()) + + val existingTabId = tabIdUrlMap.entries.findLast { it.value in urls }?.key if (existingTabId != null) { + showOnAppLaunchOptionDataStore.setShowOnAppLaunchTabId(existingTabId) tabRepository.select(existingTabId) } else { - tabRepository.add(option.url) + val tabId = tabRepository.add(url = option.url) + showOnAppLaunchOptionDataStore.setShowOnAppLaunchTabId(tabId) + } + } + + private fun stripIfHttpOrHttps(uri: Uri): String { + return if (uri.isHttpOrHttps) { + stripUri(uri) + } else { + uri.toString() } } @@ -69,4 +81,14 @@ class ShowOnAppLaunchOptionHandlerImpl @Inject constructor( val host = uri.host?.removePrefix("www.") ?: "" return "$host${uri.path.orEmpty()}" } + + private fun getTabIdUrlMap(tabs: List): Map { + return tabs + .filterNot { tab -> tab.url.isNullOrBlank() } + .associate { tab -> + val tabUri = tab.url!!.toUri() + val strippedUrl = stripIfHttpOrHttps(tabUri) + tab.tabId to strippedUrl + } + } } From 82ba755b00f425fc10d5d08adc80e4432834a3b6 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Thu, 17 Oct 2024 17:07:11 +0100 Subject: [PATCH 06/15] handle storing of resolved urls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We add a function to our Handler (should it be manager? 🤷) and if we know that we're on the root of the tab, we have a matching tab id with what was stored when we launched the tab, and it's not null, then we can set the resolved url in the store and re-use it for future launches to avoid duplicate tabs Next up, tabs! --- .../app/browser/BrowserTabViewModel.kt | 11 +++++++++ .../ShowOnAppLaunchOptionHandler.kt | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 359d9fe9613d..41471bbc177f 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -208,6 +208,7 @@ import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteEntity import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepository import com.duckduckgo.app.fire.fireproofwebsite.ui.AutomaticFireproofSetting.ALWAYS import com.duckduckgo.app.fire.fireproofwebsite.ui.AutomaticFireproofSetting.ASK_EVERY_TIME +import com.duckduckgo.app.generalsettings.showonapplaunch.ShowOnAppLaunchOptionHandler import com.duckduckgo.app.global.events.db.UserEventKey import com.duckduckgo.app.global.events.db.UserEventsStore import com.duckduckgo.app.global.model.PrivacyShield @@ -420,6 +421,7 @@ class BrowserTabViewModel @Inject constructor( private val loadingBarExperimentManager: LoadingBarExperimentManager, private val refreshPixelSender: RefreshPixelSender, private val changeOmnibarPositionFeature: ChangeOmnibarPositionFeature, + private val showOnAppLaunchOptionHandler: ShowOnAppLaunchOptionHandler, ) : WebViewClientListener, EditSavedSiteListener, DeleteBookmarkListener, @@ -1306,6 +1308,15 @@ class BrowserTabViewModel @Inject constructor( override fun navigationStateChanged(newWebNavigationState: WebNavigationState) { val stateChange = newWebNavigationState.compare(webNavigationState) + + viewModelScope.launch { + showOnAppLaunchOptionHandler.handleResolvedUrlStorage( + currentUrl = newWebNavigationState.currentUrl, + isRootOfTab = !newWebNavigationState.canGoBack, + tabId = tabId + ) + } + webNavigationState = newWebNavigationState if (!currentBrowserViewState().browserShowing) return diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt index d8004863f1fb..f0fae4456bd9 100644 --- a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt @@ -24,18 +24,26 @@ import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchO import com.duckduckgo.app.generalsettings.showonapplaunch.store.ShowOnAppLaunchOptionDataStore import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.app.tabs.model.TabRepository +import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.isHttpOrHttps import com.duckduckgo.di.scopes.AppScope import com.squareup.anvil.annotations.ContributesBinding import kotlinx.coroutines.flow.first +import kotlinx.coroutines.withContext import javax.inject.Inject interface ShowOnAppLaunchOptionHandler { suspend fun handleAppLaunchOption() + suspend fun handleResolvedUrlStorage( + currentUrl: String?, + isRootOfTab: Boolean, + tabId: String + ) } @ContributesBinding(AppScope::class) class ShowOnAppLaunchOptionHandlerImpl @Inject constructor( + private val dispatchers: DispatcherProvider, private val showOnAppLaunchOptionDataStore: ShowOnAppLaunchOptionDataStore, private val tabRepository: TabRepository, ): ShowOnAppLaunchOptionHandler { @@ -48,6 +56,22 @@ class ShowOnAppLaunchOptionHandlerImpl @Inject constructor( } } + override suspend fun handleResolvedUrlStorage( + currentUrl: String?, + isRootOfTab: Boolean, + tabId: String + ) { + withContext(dispatchers.io()) { + val shouldSaveCurrentUrlForShowOnAppLaunch = currentUrl != null && + isRootOfTab && + tabId == showOnAppLaunchOptionDataStore.showOnAppLaunchTabId + + if (shouldSaveCurrentUrlForShowOnAppLaunch) { + showOnAppLaunchOptionDataStore.setResolvedPageUrl(currentUrl!!) + } + } + } + private suspend fun handleSpecificPageOption(option: SpecificPage) { val userUri = option.url.toUri() val resolvedUri = option.resolvedUrl?.toUri() From e5d002309fa6acf712347ab629ebe500cc2bbad2 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Thu, 17 Oct 2024 17:44:32 +0100 Subject: [PATCH 07/15] add tests to TabsDaoTest I noticed we had nothing for selectTabByUrl so added some for that too --- .../com/duckduckgo/app/tabs/db/TabsDaoTest.kt | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/app/src/androidTest/java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt b/app/src/androidTest/java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt index 422c12dcbb60..6191f26cb33d 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt @@ -22,6 +22,7 @@ import androidx.test.platform.app.InstrumentationRegistry import com.duckduckgo.app.global.db.AppDatabase import com.duckduckgo.app.tabs.model.TabEntity import com.duckduckgo.app.tabs.model.TabSelectionEntity +import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Assert.* import org.junit.Before @@ -337,4 +338,95 @@ class TabsDaoTest { assertEquals(tab.copy(deletable = false), testee.tab(tab.tabId)) } + + @Test + fun whenSelectTabByUrlAndTabExistsThenTabIdReturned() = runTest { + val tab = TabEntity( + tabId = "TAB_ID", + url = "https://www.duckduckgo.com/", + position = 0, + deletable = true, + ) + + testee.insertTab(tab) + val tabId = testee.selectTabByUrl("https://www.duckduckgo.com/") + + assertEquals(tabId, tab.tabId) + } + + @Test + fun whenSelectTabByUrlAndTabDoesNotExistThenNullReturned() = runTest { + val tab = TabEntity( + tabId = "TAB_ID", + url = "https://www.duckduckgo.com/", + position = 0, + ) + + testee.insertTab(tab) + val tabId = testee.selectTabByUrl("https://www.quackquackno.com/") + + assertNull(tabId) + } + + @Test + fun whenFindTabIdByUrlPatternQueriedAndTabExistsThenTabIdIdReturned() = runTest { + val tab = TabEntity( + tabId = "TAB_ID", + url = "https://www.duckduckgo.com/", + position = 0, + deletable = true, + ) + + testee.insertTab(tab) + val tabId = testee.findTabIdByUrlPattern("duckduckgo.com/") + + assertEquals(tab.tabId, tabId) + } + + @Test + fun whenFindTabIdByUrlPatternQueriedWithFullUrlAndTabExistsThenTabIdReturned() = runTest { + val tab = TabEntity( + tabId = "TAB_ID", + url = "https://www.duckduckgo.com/", + position = 0, + deletable = true, + ) + + testee.insertTab(tab) + val tabId = testee.findTabIdByUrlPattern("https://www.duckduckgo.com/") + + assertEquals(tab.tabId, tabId) + } + + @Test + fun whenFindTabIdByUrlPatternQueriedAndMultipleTabsExistThenLastAddedTabIdIdReturned() = runTest { + val tab1 = TabEntity( + tabId = "TAB_ID_1", + url = "https://www.duckduckgo.com/", + position = 0, + ) + + val tab2 = tab1.copy(tabId = "TAB_ID_2", position = 1) + + testee.insertTab(tab1) + testee.insertTab(tab2) + val tabId = testee.findTabIdByUrlPattern("duckduckgo.com/") + + assertEquals(tab2.tabId, tabId) + } + + @Test + fun whenFindTabIdByUrlPatternQueriedWithNoTrailingSlashAndTabExistsThenNullReturned() = runTest { + val tab = TabEntity( + tabId = "TAB_ID", + url = "https://www.duckduckgo.com/", + position = 0, + deletable = true, + ) + + testee.insertTab(tab) + val tabId = testee.findTabIdByUrlPattern("duckduckgo.com") + + assertNull(tabId) + } } From fa6dde7f1bdfbc6ffa440a63fe52055e40d99912 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Thu, 17 Oct 2024 18:19:12 +0100 Subject: [PATCH 08/15] add BrowserTabViewModelTest --- .../duckduckgo/app/browser/BrowserTabViewModelTest.kt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 40a1e201ee21..64801279ddff 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -131,6 +131,7 @@ import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteDao import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteEntity import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepositoryImpl import com.duckduckgo.app.fire.fireproofwebsite.ui.AutomaticFireproofSetting +import com.duckduckgo.app.generalsettings.showonapplaunch.ShowOnAppLaunchOptionHandler import com.duckduckgo.app.global.db.AppDatabase import com.duckduckgo.app.global.events.db.UserEventsStore import com.duckduckgo.app.global.install.AppInstallStore @@ -403,6 +404,8 @@ class BrowserTabViewModelTest { private var loadingBarExperimentManager: LoadingBarExperimentManager = mock() + private val mockShowOnAppLaunchHandler: ShowOnAppLaunchOptionHandler = mock() + private lateinit var remoteMessagingModel: RemoteMessagingModel private val lazyFaviconManager = Lazy { mockFaviconManager } @@ -649,6 +652,7 @@ class BrowserTabViewModelTest { loadingBarExperimentManager = loadingBarExperimentManager, refreshPixelSender = refreshPixelSender, changeOmnibarPositionFeature = changeOmnibarPositionFeature, + showOnAppLaunchOptionHandler = mockShowOnAppLaunchHandler, ) testee.loadData("abc", null, false, false) @@ -5982,6 +5986,13 @@ class BrowserTabViewModelTest { verify(refreshPixelSender).sendCustomTabRefreshPixel() } + @Test + fun whenNavigationStateChangedCalledThenHandleResolvedUrlIsChecked() = runTest { + testee.navigationStateChanged(buildWebNavigation("https://example.com")) + + verify(mockShowOnAppLaunchHandler).handleResolvedUrlStorage(eq("https://example.com"), any(), any()) + } + private fun aCredential(): LoginCredentials { return LoginCredentials(domain = null, username = null, password = null) } From 7859581834b3db2f382f8db9173689c9af31f365 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Fri, 18 Oct 2024 12:06:13 +0100 Subject: [PATCH 09/15] add ShowOnAppLaunchOptionHandlerImplTest --- .../ShowOnAppLaunchOptionHandlerImplTest.kt | 796 ++++++++++++++++++ 1 file changed, 796 insertions(+) create mode 100644 app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt diff --git a/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt new file mode 100644 index 000000000000..ddb999649749 --- /dev/null +++ b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt @@ -0,0 +1,796 @@ +/* + * Copyright (c) 2024 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.generalsettings.showonapplaunch + +import androidx.lifecycle.LiveData +import androidx.lifecycle.MutableLiveData +import androidx.test.ext.junit.runners.AndroidJUnit4 +import app.cash.turbine.test +import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.LastOpenedTab +import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.NewTabPage +import com.duckduckgo.app.generalsettings.showonapplaunch.model.ShowOnAppLaunchOption.SpecificPage +import com.duckduckgo.app.generalsettings.showonapplaunch.store.FakeShowOnAppLaunchOptionDataStore +import com.duckduckgo.app.generalsettings.showonapplaunch.store.ShowOnAppLaunchOptionDataStore +import com.duckduckgo.app.global.model.Site +import com.duckduckgo.app.tabs.model.TabEntity +import com.duckduckgo.app.tabs.model.TabRepository +import com.duckduckgo.app.tabs.model.TabSwitcherData +import com.duckduckgo.app.tabs.model.TabSwitcherData.LayoutType +import com.duckduckgo.common.test.CoroutineTestRule +import com.duckduckgo.common.utils.DispatcherProvider +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.SharedFlow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ShowOnAppLaunchOptionHandlerImplTest { + + @get:Rule + val coroutineTestRule = CoroutineTestRule() + private val dispatcherProvider: DispatcherProvider = coroutineTestRule.testDispatcherProvider + + private lateinit var fakeDataStore: ShowOnAppLaunchOptionDataStore + private lateinit var fakeTabRepository: TabRepository + private lateinit var testee: ShowOnAppLaunchOptionHandler + + @Before + fun setup() { + fakeDataStore = FakeShowOnAppLaunchOptionDataStore() + fakeTabRepository = FakeTabRepository() + testee = + ShowOnAppLaunchOptionHandlerImpl(dispatcherProvider, fakeDataStore, fakeTabRepository) + } + + @Test + fun whenOptionIsLastTabOpenedThenNoTabIsAdded() = runTest { + fakeDataStore.setShowOnAppLaunchOption(LastOpenedTab) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.isEmpty()) + } + } + + @Test + fun whenOptionIsNewTabPageOpenedThenNewTabPageIsAdded() = runTest { + fakeDataStore.setShowOnAppLaunchOption(NewTabPage) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == "") + } + } + + @Test + fun whenOptionIsSpecificUrlThenTabIsAdded() = runTest { + val url = "https://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlAndTabDoesNotExistThenTabIdIsStored() = runTest { + val url = "https://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tab = awaitItem() + awaitComplete() + + assertTrue(fakeDataStore.showOnAppLaunchTabId == tab.first().tabId) + } + } + + @Test + fun whenOptionIsSpecificUrlAndTabExistsThenExistingTabIdIsStored() = runTest { + val url = "https://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + val existingTabId = fakeTabRepository.add(url) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + awaitItem() + awaitComplete() + + assertTrue(fakeDataStore.showOnAppLaunchTabId == existingTabId) + } + } + + @Test + fun whenOptionIsSpecificUrlWithSubdomainThenTabIsAdded() = runTest { + val url = "https://www.example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlAndUrlIsHttpThenTabIsAdded() = runTest { + val url = "http://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlAndTabAlreadyAddedThenTabIsSelected() = runTest { + val url = "https://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + fakeTabRepository.add(url) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlWithSubdomainAndTabAlreadyAddedThenTabIsSelected() = runTest { + val url = "https://www.example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + fakeTabRepository.add(url) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlAndIsHttpAndTabAlreadyAddedThenTabIsSelected() = runTest { + val url = "http://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + fakeTabRepository.add(url) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlAndIsHttpAndHttpsTabAlreadyAddedThenTabIsNotAdded() = runTest { + val url = "http://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + fakeTabRepository.add("https://example.com/") + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlAndIsHttpsAndHttpTabAlreadyAddedThenTabIsNotAdded() = runTest { + val url = "https://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + fakeTabRepository.add("http://example.com/") + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlWithDomainOnlyAndTabAlreadyAddedWithSchemeAndSubdomainThenTabIsNotAdded() = + runTest { + val url = "https://www.example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + fakeTabRepository.add(url) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlWithPathThenTabIsAdded() = runTest { + val queryUrl = "https://example.com/article/1234" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(queryUrl)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == queryUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithPathAndTabAlreadyAddedThenTabIsNotAdded() = runTest { + val queryUrl = "https://example.com/article/1234" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(queryUrl)) + fakeTabRepository.add(queryUrl) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == queryUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithNoPathAndTabExistsWithPathThenTabIsAdded() = runTest { + val url = "http://example.com/" + val pathUrl = "https://example.com/article/1234/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + fakeTabRepository.add(pathUrl) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 2) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlWithPathAndTabExistsWithoutPathThenTabIsAdded() = runTest { + val url = "https://example.com/article/1234/" + val pathUrl = "https://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + fakeTabRepository.add(pathUrl) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 2) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlWithDifferentPathThenTabIsAdded() = runTest { + val url1 = "https://example.com/path1" + val url2 = "https://example.com/path2" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url1)) + fakeTabRepository.add(url2) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 2) + assertTrue(tabs.last().url == url1) + } + } + + @Test + fun whenOptionIsSpecificUrlWithQueryStringThenTabIsAdded() = runTest { + val queryUrl = "https://example.com/?query=1" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(queryUrl)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == queryUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithDifferentQueryParameterThenTabIsAdded() = runTest { + val url1 = "https://example.com/path?query1=value1" + val url2 = "https://example.com/path?query2=value2" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url1)) + fakeTabRepository.add(url2) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 2) + assertTrue(tabs.last().url == url1) + } + } + + @Test + fun whenOptionIsSpecificUrlWithFragmentThenTabIsAdded() = runTest { + val fragmentUrl = "https://example.com/#fragment" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(fragmentUrl)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == fragmentUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithDifferentFragmentThenTabIsAdded() = runTest { + val url1 = "https://example.com/path?query=value#fragment1" + val url2 = "https://example.com/path?query=value#fragment2" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url1)) + fakeTabRepository.add(url2) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 2) + assertTrue(tabs.last().url == url1) + } + } + + @Test + fun whenOptionIsSpecificUrlWithFragmentAndIsAddedThenTabIsNotAdded() = runTest { + val fragmentUrl = "https://example.com/#fragment" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(fragmentUrl)) + fakeTabRepository.add(fragmentUrl) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == fragmentUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithQueryStringAndFragmentThenTabIsAdded() = runTest { + val queryFragmentUrl = "https://example.com/?query=1#fragment" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(queryFragmentUrl)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == queryFragmentUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithQueryStringAndFragmentAndIsAddedThenTabIsNotAdded() = runTest { + val queryFragmentUrl = "https://example.com/?query=1#fragment" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(queryFragmentUrl)) + fakeTabRepository.add(queryFragmentUrl) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == queryFragmentUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithNonHttpOrHttpsProtocolAndNotAddedThenTabIsAdded() = runTest { + val ftpUrl = "ftp://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(ftpUrl)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == ftpUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithNonHttpOrHttpsProtocolAndAddedThenTabIsNotAdded() = runTest { + val ftpUrl = "ftp://example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(ftpUrl)) + fakeTabRepository.add(ftpUrl) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == ftpUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithResolvedUrlThenTabIsAdded() = runTest { + val url = "https://www.example.com/" + val resolvedUrl = "https://www.example.co.uk/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url, resolvedUrl)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlWithResolvedUrlAndTabMatchesResolvedUrlThenTabIsNotAdded() = + runTest { + val url = "https://example.com/" + val resolvedUrl = "https://www.example.co.uk/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url, resolvedUrl)) + fakeTabRepository.add(resolvedUrl) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == resolvedUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithResolvedUrlAndTabMatchesBothUrlsThenTabIsNotAdded() = runTest { + val url = "https://www.example.co.uk/" + val resolvedUrl = "https://www.example.co.uk/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url, resolvedUrl)) + fakeTabRepository.add(resolvedUrl) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == resolvedUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithNonWwwSubdomainThenTabIsAdded() = runTest { + val url = "https://blog.example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlWithNonWwwSubdomainAndTabExistsThenTabIsNotAdded() = runTest { + val url = "https://blog.example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + fakeTabRepository.add(url) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + @Test + fun whenOptionIsSpecificUrlWithNoSubdomainAndTabWithDifferentSubdomainExistsThenTabIsAdded() = + runTest { + val noSubdomainUrl = "https://example.com/" + val subdomainUrl = "https://blog.example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(noSubdomainUrl)) + fakeTabRepository.add(subdomainUrl) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 2) + assertTrue(tabs.last().url == noSubdomainUrl) + } + } + + @Test + fun whenOptionIsSpecificUrlWithDifferentPortThenTabIsAdded() = runTest { + val url = "https://example.com:8080/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url) + } + } + + private class FakeTabRepository : TabRepository { + + private val tabs = mutableMapOf() + + override suspend fun select(tabId: String) = Unit + + override suspend fun add( + url: String?, + skipHome: Boolean, + ): String { + tabs[tabs.size + 1] = url ?: "" + return tabs.size.toString() + } + + override suspend fun getTabId(url: String): String? { + return tabs.values.firstOrNull { it.contains(url) } + } + + override val flowTabs: Flow> = flowOf(tabs).map { + it.map { (id, url) -> TabEntity(tabId = id.toString(), url = url, position = id) } + } + + override val liveTabs: LiveData> + get() = TODO("Not yet implemented") + override val childClosedTabs: SharedFlow + get() = TODO("Not yet implemented") + override val flowDeletableTabs: Flow> + get() = TODO("Not yet implemented") + override val liveSelectedTab: LiveData + get() = TODO("Not yet implemented") + override val tabSwitcherData: Flow + get() = TODO("Not yet implemented") + + override suspend fun addDefaultTab(): String { + TODO("Not yet implemented") + } + + override suspend fun addFromSourceTab( + url: String?, + skipHome: Boolean, + sourceTabId: String, + ): String { + TODO("Not yet implemented") + } + + override suspend fun addNewTabAfterExistingTab( + url: String?, + tabId: String, + ) { + TODO("Not yet implemented") + } + + override suspend fun update( + tabId: String, + site: Site?, + ) { + TODO("Not yet implemented") + } + + override suspend fun updateTabPosition( + from: Int, + to: Int, + ) { + TODO("Not yet implemented") + } + + override fun retrieveSiteData(tabId: String): MutableLiveData { + TODO("Not yet implemented") + } + + override suspend fun delete(tab: TabEntity) { + TODO("Not yet implemented") + } + + override suspend fun markDeletable(tab: TabEntity) { + TODO("Not yet implemented") + } + + override suspend fun undoDeletable(tab: TabEntity) { + TODO("Not yet implemented") + } + + override suspend fun purgeDeletableTabs() { + TODO("Not yet implemented") + } + + override suspend fun getDeletableTabIds(): List { + TODO("Not yet implemented") + } + + override suspend fun deleteTabAndSelectSource(tabId: String) { + TODO("Not yet implemented") + } + + override suspend fun deleteAll() { + TODO("Not yet implemented") + } + + override suspend fun getSelectedTab(): TabEntity? { + TODO("Not yet implemented") + } + + override fun updateTabPreviewImage( + tabId: String, + fileName: String?, + ) { + TODO("Not yet implemented") + } + + override fun updateTabFavicon( + tabId: String, + fileName: String?, + ) { + TODO("Not yet implemented") + } + + override suspend fun selectByUrlOrNewTab(url: String) { + TODO("Not yet implemented") + } + + override suspend fun setIsUserNew(isUserNew: Boolean) { + TODO("Not yet implemented") + } + + override suspend fun setTabLayoutType(layoutType: LayoutType) { + TODO("Not yet implemented") + } + } +} From 6441bf4bedc390c36a90db8f5506bea5b91355cc Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Fri, 18 Oct 2024 12:06:23 +0100 Subject: [PATCH 10/15] fix FakeShowOnAppLaunchOptionDataStore --- .../store/FakeShowOnAppLaunchOptionDataStore.kt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/FakeShowOnAppLaunchOptionDataStore.kt b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/FakeShowOnAppLaunchOptionDataStore.kt index d96d1c396cba..e24ae2050472 100644 --- a/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/FakeShowOnAppLaunchOptionDataStore.kt +++ b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/FakeShowOnAppLaunchOptionDataStore.kt @@ -24,6 +24,9 @@ import kotlinx.coroutines.flow.filterNotNull class FakeShowOnAppLaunchOptionDataStore(defaultOption: ShowOnAppLaunchOption? = null) : ShowOnAppLaunchOptionDataStore { + override var showOnAppLaunchTabId: String? = null + private set + private var currentOptionStateFlow = MutableStateFlow(defaultOption) private var currentSpecificPageUrl = MutableStateFlow("https://duckduckgo.com") @@ -39,4 +42,12 @@ class FakeShowOnAppLaunchOptionDataStore(defaultOption: ShowOnAppLaunchOption? = override suspend fun setSpecificPageUrl(url: String) { currentSpecificPageUrl.value = url } + + override suspend fun setResolvedPageUrl(url: String) { + TODO("Not yet implemented") + } + + override fun setShowOnAppLaunchTabId(tabId: String) { + showOnAppLaunchTabId = tabId + } } From 54253466953ffb19e30be294568a0d7946b2007e Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Fri, 18 Oct 2024 13:41:33 +0100 Subject: [PATCH 11/15] formatting --- .../com/duckduckgo/app/browser/BrowserTabViewModel.kt | 10 +++++----- .../com/duckduckgo/app/browser/BrowserViewModel.kt | 4 ++-- .../showonapplaunch/ShowOnAppLaunchOptionHandler.kt | 8 ++++---- .../store/ShowOnAppLaunchOptionDataStore.kt | 2 +- .../com/duckduckgo/app/tabs/model/TabDataRepository.kt | 4 ++-- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 41471bbc177f..c78466e0c945 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -1310,11 +1310,11 @@ class BrowserTabViewModel @Inject constructor( val stateChange = newWebNavigationState.compare(webNavigationState) viewModelScope.launch { - showOnAppLaunchOptionHandler.handleResolvedUrlStorage( - currentUrl = newWebNavigationState.currentUrl, - isRootOfTab = !newWebNavigationState.canGoBack, - tabId = tabId - ) + showOnAppLaunchOptionHandler.handleResolvedUrlStorage( + currentUrl = newWebNavigationState.currentUrl, + isRootOfTab = !newWebNavigationState.canGoBack, + tabId = tabId, + ) } webNavigationState = newWebNavigationState diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index b536ba294a05..97f8890f5c37 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -55,12 +55,12 @@ import com.duckduckgo.common.utils.SingleLiveEvent import com.duckduckgo.di.scopes.ActivityScope import com.duckduckgo.di.scopes.AppScope import com.duckduckgo.feature.toggles.api.Toggle +import javax.inject.Inject +import kotlin.coroutines.CoroutineContext import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import timber.log.Timber -import javax.inject.Inject -import kotlin.coroutines.CoroutineContext @ContributesViewModel(ActivityScope::class) class BrowserViewModel @Inject constructor( diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt index f0fae4456bd9..09f765436196 100644 --- a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt @@ -28,16 +28,16 @@ import com.duckduckgo.common.utils.DispatcherProvider import com.duckduckgo.common.utils.isHttpOrHttps import com.duckduckgo.di.scopes.AppScope import com.squareup.anvil.annotations.ContributesBinding +import javax.inject.Inject import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext -import javax.inject.Inject interface ShowOnAppLaunchOptionHandler { suspend fun handleAppLaunchOption() suspend fun handleResolvedUrlStorage( currentUrl: String?, isRootOfTab: Boolean, - tabId: String + tabId: String, ) } @@ -46,7 +46,7 @@ class ShowOnAppLaunchOptionHandlerImpl @Inject constructor( private val dispatchers: DispatcherProvider, private val showOnAppLaunchOptionDataStore: ShowOnAppLaunchOptionDataStore, private val tabRepository: TabRepository, -): ShowOnAppLaunchOptionHandler { +) : ShowOnAppLaunchOptionHandler { override suspend fun handleAppLaunchOption() { when (val option = showOnAppLaunchOptionDataStore.optionFlow.first()) { @@ -59,7 +59,7 @@ class ShowOnAppLaunchOptionHandlerImpl @Inject constructor( override suspend fun handleResolvedUrlStorage( currentUrl: String?, isRootOfTab: Boolean, - tabId: String + tabId: String, ) { withContext(dispatchers.io()) { val shouldSaveCurrentUrlForShowOnAppLaunch = currentUrl != null && diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/ShowOnAppLaunchOptionDataStore.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/ShowOnAppLaunchOptionDataStore.kt index c045db82ea41..3afb3f017cd7 100644 --- a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/ShowOnAppLaunchOptionDataStore.kt +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/store/ShowOnAppLaunchOptionDataStore.kt @@ -30,9 +30,9 @@ import com.duckduckgo.app.generalsettings.showonapplaunch.store.ShowOnAppLaunchO import com.duckduckgo.di.scopes.AppScope import com.squareup.anvil.annotations.ContributesBinding import dagger.SingleInstanceIn +import javax.inject.Inject import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map -import javax.inject.Inject interface ShowOnAppLaunchOptionDataStore { val optionFlow: Flow diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt index aca6fb71eab8..0338a282c400 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabDataRepository.kt @@ -35,6 +35,8 @@ import com.duckduckgo.di.scopes.AppScope import dagger.SingleInstanceIn import io.reactivex.Scheduler import io.reactivex.schedulers.Schedulers +import java.util.UUID +import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow @@ -45,8 +47,6 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import timber.log.Timber -import java.util.UUID -import javax.inject.Inject @SingleInstanceIn(AppScope::class) class TabDataRepository @Inject constructor( From 08c0a045f163358c41b6a53b14e9a1a624b7a2fc Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Fri, 18 Oct 2024 16:25:28 +0100 Subject: [PATCH 12/15] fix stripping urls It wasn't working correctly when using queries or fragments previously so move back to using URI builder like we do with the ShowOnAppLaunchConverter I added some more tests as well --- .../ShowOnAppLaunchOptionHandler.kt | 10 ++- .../ShowOnAppLaunchOptionHandlerImplTest.kt | 73 +++++++++++++++++-- 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt index 09f765436196..a0a2a0332018 100644 --- a/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt +++ b/app/src/main/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandler.kt @@ -101,9 +101,13 @@ class ShowOnAppLaunchOptionHandlerImpl @Inject constructor( } } - private fun stripUri(uri: Uri): String { - val host = uri.host?.removePrefix("www.") ?: "" - return "$host${uri.path.orEmpty()}" + private fun stripUri(uri: Uri): String = uri.run { + val authority = uri.authority?.removePrefix("www.") + uri.buildUpon() + .scheme(null) + .authority(authority) + .toString() + .replaceFirst("//", "") } private fun getTabIdUrlMap(tabs: List): Map { diff --git a/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt index ddb999649749..d663c75048eb 100644 --- a/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt +++ b/app/src/test/java/com/duckduckgo/app/generalsettings/showonapplaunch/ShowOnAppLaunchOptionHandlerImplTest.kt @@ -232,9 +232,10 @@ class ShowOnAppLaunchOptionHandlerImplTest { @Test fun whenOptionIsSpecificUrlAndIsHttpAndHttpsTabAlreadyAddedThenTabIsNotAdded() = runTest { val url = "http://example.com/" + val httpsUrl = "https://example.com/" fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) - fakeTabRepository.add("https://example.com/") + fakeTabRepository.add(httpsUrl) testee.handleAppLaunchOption() @@ -243,16 +244,17 @@ class ShowOnAppLaunchOptionHandlerImplTest { awaitComplete() assertTrue(tabs.size == 1) - assertTrue(tabs.last().url == url) + assertTrue(tabs.last().url == httpsUrl) } } @Test fun whenOptionIsSpecificUrlAndIsHttpsAndHttpTabAlreadyAddedThenTabIsNotAdded() = runTest { val url = "https://example.com/" + val httpUrl = "http://example.com/" fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url)) - fakeTabRepository.add("http://example.com/") + fakeTabRepository.add(httpUrl) testee.handleAppLaunchOption() @@ -261,7 +263,7 @@ class ShowOnAppLaunchOptionHandlerImplTest { awaitComplete() assertTrue(tabs.size == 1) - assertTrue(tabs.last().url == url) + assertTrue(tabs.last().url == httpUrl) } } @@ -376,6 +378,63 @@ class ShowOnAppLaunchOptionHandlerImplTest { } } + @Test + fun whenOptionIsSpecificUrlWithWWWSubdomainAndDifferentPathThenTabIsAdded() = runTest { + val url1 = "https://www.example.com/path1" + val url2 = "https://www.example.com/path2" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url1)) + fakeTabRepository.add(url2) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 2) + assertTrue(tabs.last().url == url1) + } + } + + @Test + fun whenOptionIsSpecificUrlWithNonWWWSubdomainAndTabExistsWithWWWSubdomainThenTabIsAdded() = runTest { + val url1 = "https://blog.example.com/" + val url2 = "https://www.example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url1)) + fakeTabRepository.add(url2) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 2) + assertTrue(tabs.last().url == url1) + } + } + + @Test + fun whenOptionIsSpecificUrlWithNoSubdomainAndTabExistsWithWWWSubdomainThenTabIsNotAdded() = runTest { + val url1 = "https://example.com/" + val url2 = "https://www.example.com/" + + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url1)) + fakeTabRepository.add(url2) + + testee.handleAppLaunchOption() + + fakeTabRepository.flowTabs.test { + val tabs = awaitItem() + awaitComplete() + + assertTrue(tabs.size == 1) + assertTrue(tabs.last().url == url2) + } + } + @Test fun whenOptionIsSpecificUrlWithQueryStringThenTabIsAdded() = runTest { val queryUrl = "https://example.com/?query=1" @@ -434,8 +493,8 @@ class ShowOnAppLaunchOptionHandlerImplTest { val url1 = "https://example.com/path?query=value#fragment1" val url2 = "https://example.com/path?query=value#fragment2" - fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url1)) - fakeTabRepository.add(url2) + fakeTabRepository.add(url1) + fakeDataStore.setShowOnAppLaunchOption(SpecificPage(url2)) testee.handleAppLaunchOption() @@ -444,7 +503,7 @@ class ShowOnAppLaunchOptionHandlerImplTest { awaitComplete() assertTrue(tabs.size == 2) - assertTrue(tabs.last().url == url1) + assertTrue(tabs.last().url == url2) } } From fa96facb154fa6916ad483da8c67ee21bf23c9e5 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Mon, 21 Oct 2024 08:23:50 +0100 Subject: [PATCH 13/15] remove unused tests These were part of an old approach and are no longer needed --- .../com/duckduckgo/app/tabs/db/TabsDaoTest.kt | 62 ------------------- 1 file changed, 62 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt b/app/src/androidTest/java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt index 6191f26cb33d..e9c6d1d1e72e 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt @@ -367,66 +367,4 @@ class TabsDaoTest { assertNull(tabId) } - - @Test - fun whenFindTabIdByUrlPatternQueriedAndTabExistsThenTabIdIdReturned() = runTest { - val tab = TabEntity( - tabId = "TAB_ID", - url = "https://www.duckduckgo.com/", - position = 0, - deletable = true, - ) - - testee.insertTab(tab) - val tabId = testee.findTabIdByUrlPattern("duckduckgo.com/") - - assertEquals(tab.tabId, tabId) - } - - @Test - fun whenFindTabIdByUrlPatternQueriedWithFullUrlAndTabExistsThenTabIdReturned() = runTest { - val tab = TabEntity( - tabId = "TAB_ID", - url = "https://www.duckduckgo.com/", - position = 0, - deletable = true, - ) - - testee.insertTab(tab) - val tabId = testee.findTabIdByUrlPattern("https://www.duckduckgo.com/") - - assertEquals(tab.tabId, tabId) - } - - @Test - fun whenFindTabIdByUrlPatternQueriedAndMultipleTabsExistThenLastAddedTabIdIdReturned() = runTest { - val tab1 = TabEntity( - tabId = "TAB_ID_1", - url = "https://www.duckduckgo.com/", - position = 0, - ) - - val tab2 = tab1.copy(tabId = "TAB_ID_2", position = 1) - - testee.insertTab(tab1) - testee.insertTab(tab2) - val tabId = testee.findTabIdByUrlPattern("duckduckgo.com/") - - assertEquals(tab2.tabId, tabId) - } - - @Test - fun whenFindTabIdByUrlPatternQueriedWithNoTrailingSlashAndTabExistsThenNullReturned() = runTest { - val tab = TabEntity( - tabId = "TAB_ID", - url = "https://www.duckduckgo.com/", - position = 0, - deletable = true, - ) - - testee.insertTab(tab) - val tabId = testee.findTabIdByUrlPattern("duckduckgo.com") - - assertNull(tabId) - } } From d4796047bb5ddcad4080ae90e53d1df4190a2354 Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Mon, 21 Oct 2024 13:15:28 +0100 Subject: [PATCH 14/15] formatting --- app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt index 97f8890f5c37..377d66f84685 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -58,7 +58,6 @@ import com.duckduckgo.feature.toggles.api.Toggle import javax.inject.Inject import kotlin.coroutines.CoroutineContext import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import timber.log.Timber From 36f9161453421262b698b486ddf241b84f8d9ffc Mon Sep 17 00:00:00 2001 From: Mike Scamell Date: Tue, 22 Oct 2024 18:12:04 +0200 Subject: [PATCH 15/15] Show on App Launch: Fix website showing after clear data action (#5166) Task/Issue URL: https://app.asana.com/0/1207908166761516/1208588631698784/f ### Description When clearing data with the fire button the website set for Specific Page would load if that option was enabled. This stops that happening and instead shows the new tab page. ### Steps to test this PR _Fire button_ - [x] Open Settings - [x] Open General Settings - [x] Open Show on App Launch feature - [x] Enable Specific Page option - [x] Leave default web address - [x] Go back to Browser screen - [x] Press the fire button - [x] Wait until the app relaunches - [x] The default webpage (duckduckgo.com) should not load - [x] The new tab page should be visible ### UI changes N/A ### Demo https://github.com/user-attachments/assets/03298b73-9682-485d-8be7-50b5ad5d13a4 --- .../java/com/duckduckgo/app/browser/BrowserActivity.kt | 9 ++++++--- .../main/java/com/duckduckgo/app/fire/FireActivity.kt | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt index b290d481a16a..69a9522c7ff7 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -379,7 +379,9 @@ open class BrowserActivity : DuckDuckGoActivity() { } } - viewModel.handleShowOnAppLaunchOption() + if (!intent.getBooleanExtra(LAUNCH_FROM_CLEAR_DATA_ACTION, false)) { + viewModel.handleShowOnAppLaunchOption() + } } private fun configureObservers() { @@ -564,6 +566,7 @@ open class BrowserActivity : DuckDuckGoActivity() { openInCurrentTab: Boolean = false, selectedText: Boolean = false, isExternal: Boolean = false, + isLaunchFromClearDataAction: Boolean = false, ): Intent { val intent = Intent(context, BrowserActivity::class.java) intent.putExtra(EXTRA_TEXT, queryExtra) @@ -572,6 +575,7 @@ open class BrowserActivity : DuckDuckGoActivity() { intent.putExtra(OPEN_IN_CURRENT_TAB_EXTRA, openInCurrentTab) intent.putExtra(SELECTED_TEXT_EXTRA, selectedText) intent.putExtra(LAUNCH_FROM_EXTERNAL_EXTRA, isExternal) + intent.putExtra(LAUNCH_FROM_CLEAR_DATA_ACTION, isLaunchFromClearDataAction) return intent } @@ -584,9 +588,8 @@ open class BrowserActivity : DuckDuckGoActivity() { const val OPEN_IN_CURRENT_TAB_EXTRA = "OPEN_IN_CURRENT_TAB_EXTRA" const val SELECTED_TEXT_EXTRA = "SELECTED_TEXT_EXTRA" - private const val APP_ENJOYMENT_DIALOG_TAG = "AppEnjoyment" - private const val LAUNCH_FROM_EXTERNAL_EXTRA = "LAUNCH_FROM_EXTERNAL_EXTRA" + private const val LAUNCH_FROM_CLEAR_DATA_ACTION = "LAUNCH_FROM_CLEAR_DATA_ACTION" private const val MAX_ACTIVE_TABS = 40 } diff --git a/app/src/main/java/com/duckduckgo/app/fire/FireActivity.kt b/app/src/main/java/com/duckduckgo/app/fire/FireActivity.kt index 5143c0d42a15..97dc42580371 100644 --- a/app/src/main/java/com/duckduckgo/app/fire/FireActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/fire/FireActivity.kt @@ -80,7 +80,7 @@ class FireActivity : AppCompatActivity() { context: Context, notifyDataCleared: Boolean = false, ): Intent { - val intent = BrowserActivity.intent(context, notifyDataCleared = notifyDataCleared) + val intent = BrowserActivity.intent(context, notifyDataCleared = notifyDataCleared, isLaunchFromClearDataAction = true) intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TASK) return intent }