From c0e447515c17a6c4fd35b71fb30f36b1e7be789e Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Wed, 5 Aug 2020 17:57:31 +0200 Subject: [PATCH 01/25] removing method from interface as it's not used by external callers --- .../java/com/duckduckgo/app/tabs/model/TabDataRepository.kt | 2 +- .../main/java/com/duckduckgo/app/tabs/model/TabRepository.kt | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) 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 92ba97f48674..60242a22dcd3 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 @@ -83,7 +83,7 @@ class TabDataRepository @Inject constructor( return data } - override suspend fun add(tabId: String, data: MutableLiveData, skipHome: Boolean, isDefaultTab: Boolean, sourceTabId: String?) { + private fun add(tabId: String, data: MutableLiveData, skipHome: Boolean, isDefaultTab: Boolean, sourceTabId: String? = null) { siteData[tabId] = data databaseExecutor().scheduleDirect { diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt index 84b87a3941c3..61c22cab7bd0 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt @@ -31,8 +31,6 @@ interface TabRepository { */ suspend fun add(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false): String - suspend fun add(tabId: String, data: MutableLiveData, skipHome: Boolean = false, isDefaultTab: Boolean = false, sourceTabId: String? = null) - suspend fun addWithSource(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false): String suspend fun addNewTabAfterExistingTab(url: String? = null, tabId: String) From f65fee45743a365c9af39527367b0ecdf29c59cc Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Wed, 5 Aug 2020 17:58:11 +0200 Subject: [PATCH 02/25] only store sourceTabId when tab is opened after clicking on a link --- .../main/java/com/duckduckgo/app/browser/BrowserActivity.kt | 2 +- .../java/com/duckduckgo/app/browser/BrowserViewModel.kt | 6 +++++- 2 files changed, 6 insertions(+), 2 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 f7ae7d72d72d..4823232918db 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -295,7 +295,7 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { fun openMessageInNewTab(message: Message) { openMessageInNewTabJob = launch { - val tabId = viewModel.onNewTabRequested() + val tabId = viewModel.onNewTabRequestedWithSource() val fragment = openNewTab(tabId, null, false) fragment.messageFromPreviousTab = message } 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 b37d4a3d4f72..30c6da72b032 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -127,11 +127,15 @@ class BrowserViewModel( } suspend fun onNewTabRequested(isDefaultTab: Boolean = false): String { + return tabRepository.add(isDefaultTab = isDefaultTab) + } + + suspend fun onNewTabRequestedWithSource(isDefaultTab: Boolean = false): String { return tabRepository.addWithSource(isDefaultTab = isDefaultTab) } suspend fun onOpenInNewTabRequested(query: String, skipHome: Boolean = false): String { - return tabRepository.addWithSource( + return tabRepository.add( queryUrlConverter.convertQueryToUrl(query), skipHome, isDefaultTab = false From 74e59f1c967a648622fa951c93c30b886815028a Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 12:09:41 +0200 Subject: [PATCH 03/25] Store sourceTabId only when tab opened from current tab --- .../duckduckgo/app/browser/BrowserActivity.kt | 10 ++++--- .../app/browser/BrowserTabFragment.kt | 2 +- .../app/browser/BrowserTabViewModel.kt | 19 ++++-------- .../app/browser/BrowserViewModel.kt | 30 ++++++++++++------- 4 files changed, 32 insertions(+), 29 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 4823232918db..ab32bb0612d6 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -211,7 +211,7 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { launch { viewModel.onOpenShortcut(sharedText) } } else { Timber.w("opening in new tab requested for $sharedText") - launch { viewModel.onOpenInNewTabRequested(sharedText, true) } + launch { viewModel.onOpenInNewTabRequested(query = sharedText, skipHome = true, fromCurrentTab = false) } return } } @@ -289,13 +289,15 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { launch { viewModel.onNewTabRequested() } } - fun openInNewTab(query: String) { - launch { viewModel.onOpenInNewTabRequested(query) } + fun openInNewTab(query: String, fromCurrentTab: Boolean) { + launch { + viewModel.onOpenInNewTabRequested(query = query, fromCurrentTab = fromCurrentTab) + } } fun openMessageInNewTab(message: Message) { openMessageInNewTabJob = launch { - val tabId = viewModel.onNewTabRequestedWithSource() + val tabId = viewModel.onNewTabRequested(fromCurrentTab = true) val fragment = openNewTab(tabId, null, false) fragment.messageFromPreviousTab = message } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index c8b2cf7e92f0..22818fd5144b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -470,7 +470,7 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi when (it) { is Command.Refresh -> refresh() is Command.OpenInNewTab -> { - browserActivity?.openInNewTab(it.query) + browserActivity?.openInNewTab(it.query, it.fromCurrentTab) } is Command.OpenMessageInNewTab -> { browserActivity?.openMessageInNewTab(it.message) 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 db73d0c8d4a0..027bc01fd5eb 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -51,32 +51,25 @@ import com.duckduckgo.app.browser.SpecialUrlDetector.UrlType.IntentType import com.duckduckgo.app.browser.WebNavigationStateChange.* import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector import com.duckduckgo.app.browser.favicon.FaviconDownloader -import com.duckduckgo.app.browser.logindetection.NavigationEvent import com.duckduckgo.app.browser.logindetection.LoginDetected import com.duckduckgo.app.browser.logindetection.NavigationAwareLoginDetector +import com.duckduckgo.app.browser.logindetection.NavigationEvent import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials import com.duckduckgo.app.browser.model.BasicAuthenticationRequest import com.duckduckgo.app.browser.model.LongPressTarget import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.browser.ui.HttpAuthenticationDialogFragment.HttpAuthenticationListener -import com.duckduckgo.app.cta.ui.Cta -import com.duckduckgo.app.cta.ui.CtaViewModel -import com.duckduckgo.app.cta.ui.DaxDialogCta -import com.duckduckgo.app.cta.ui.DialogCta -import com.duckduckgo.app.cta.ui.HomePanelCta -import com.duckduckgo.app.cta.ui.HomeTopPanelCta -import com.duckduckgo.app.cta.ui.UseOurAppCta +import com.duckduckgo.app.cta.ui.* import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteEntity import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteRepository import com.duckduckgo.app.global.* +import com.duckduckgo.app.global.events.db.UserEventKey +import com.duckduckgo.app.global.events.db.UserEventsStore import com.duckduckgo.app.global.model.Site import com.duckduckgo.app.global.model.SiteFactory import com.duckduckgo.app.global.model.domain import com.duckduckgo.app.global.model.domainMatchesUrl -import com.duckduckgo.app.global.events.db.UserEventsStore -import com.duckduckgo.app.global.events.db.UserEventKey -import com.duckduckgo.app.global.toDesktopUri import com.duckduckgo.app.global.useourapp.UseOurAppDetector import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_SHORTCUT_TITLE import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_SHORTCUT_URL @@ -212,7 +205,7 @@ class BrowserTabViewModel( class Navigate(val url: String) : Command() class NavigateBack(val steps: Int) : Command() object NavigateForward : Command() - class OpenInNewTab(val query: String) : Command() + class OpenInNewTab(val query: String, val fromCurrentTab: Boolean = false) : Command() class OpenMessageInNewTab(val message: Message) : Command() class OpenInNewBackgroundTab(val query: String) : Command() object LaunchNewTab : Command() @@ -1034,7 +1027,7 @@ class BrowserTabViewModel( return when (requiredAction) { is RequiredAction.OpenInNewTab -> { command.value = GenerateWebViewPreviewImage - command.value = OpenInNewTab(requiredAction.url) + command.value = OpenInNewTab(query = requiredAction.url, fromCurrentTab = true) true } is RequiredAction.OpenInNewBackgroundTab -> { 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 30c6da72b032..804917f74df7 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -126,20 +126,28 @@ class BrowserViewModel( appEnjoymentPromptEmitter.promptType.observeForever(appEnjoymentObserver) } - suspend fun onNewTabRequested(isDefaultTab: Boolean = false): String { - return tabRepository.add(isDefaultTab = isDefaultTab) - } - - suspend fun onNewTabRequestedWithSource(isDefaultTab: Boolean = false): String { - return tabRepository.addWithSource(isDefaultTab = isDefaultTab) + suspend fun onNewTabRequested(isDefaultTab: Boolean = false, fromCurrentTab: Boolean = false): String { + return if (fromCurrentTab) { + tabRepository.addWithSource(isDefaultTab = isDefaultTab) + } else { + tabRepository.add(isDefaultTab = isDefaultTab) + } } - suspend fun onOpenInNewTabRequested(query: String, skipHome: Boolean = false): String { - return tabRepository.add( - queryUrlConverter.convertQueryToUrl(query), - skipHome, + suspend fun onOpenInNewTabRequested(query: String, skipHome: Boolean = false, fromCurrentTab: Boolean = false): String { + return if (fromCurrentTab) { + tabRepository.addWithSource( + url = queryUrlConverter.convertQueryToUrl(query), + isDefaultTab = false, + skipHome = skipHome + ) + } else { + tabRepository.add( + url = queryUrlConverter.convertQueryToUrl(query), + skipHome = skipHome, isDefaultTab = false - ) + ) + } } suspend fun onTabsUpdated(tabs: List?) { From 87eeaaded8d0df981d71150af277902fa8dd842a Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 12:10:06 +0200 Subject: [PATCH 04/25] store sourceTabId when tab opened in background from currentTab --- .../java/com/duckduckgo/app/tabs/model/TabDataRepository.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 60242a22dcd3..ead6fcae6cbd 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 @@ -140,7 +140,8 @@ class TabDataRepository @Inject constructor( title = title, skipHome = false, viewed = false, - position = position + 1 + position = position + 1, + sourceTabId = tabId ) tabsDao.insertTabAtPosition(tab) } From 87e097d73cddc28c50f89143f87827d61c47ce83 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 12:10:27 +0200 Subject: [PATCH 05/25] When tab deleted, set to null references to that tab --- .../main/java/com/duckduckgo/app/tabs/db/TabsDao.kt | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt index 8ce0b347393c..85e832677af6 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt @@ -65,6 +65,9 @@ abstract class TabsDao { @Query("update tabs set position = position + 1 where position >= :position") abstract fun incrementPositionStartingAt(position: Int) + @Query("update tabs set sourceTabId = null where sourceTabId = :removedTabId") + abstract fun updateChildTabSourceTabId(removedTabId: String) + @Transaction open fun addAndSelectTab(tab: TabEntity) { deleteBlankTabs() @@ -74,7 +77,7 @@ abstract class TabsDao { @Transaction open fun deleteTabAndUpdateSelection(tab: TabEntity) { - deleteTab(tab) + deleteTabAndChildRef(tab) if (selectedTab() != null) { return @@ -87,7 +90,7 @@ abstract class TabsDao { @Transaction open fun deleteTabAndUpdateSelection(tab: TabEntity, newSelectedTab: TabEntity? = null) { - deleteTab(tab) + deleteTabAndChildRef(tab) if (newSelectedTab != null) { insertTabSelection(TabSelectionEntity(tabId = newSelectedTab.tabId)) @@ -112,6 +115,11 @@ abstract class TabsDao { insertTab(tab) } + private fun deleteTabAndChildRef(tab: TabEntity) { + deleteTab(tab) + updateChildTabSourceTabId(tab.tabId) + } + fun lastTab(): TabEntity? { return tabs().lastOrNull() } From bae6921b7faf0b071ded899dc211a2132524fa96 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 12:10:50 +0200 Subject: [PATCH 06/25] When back pressed, if sourceTab of current tab is different to null, navigate to that tab --- .../java/com/duckduckgo/app/browser/BrowserTabViewModel.kt | 6 ++++++ 1 file changed, 6 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 027bc01fd5eb..8d7b8a6f346e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -535,6 +535,7 @@ class BrowserTabViewModel( fun onUserPressedBack(): Boolean { navigationAwareLoginDetector.onEvent(NavigationEvent.UserAction.NavigateBack) val navigation = webNavigationState ?: return false + val hasParentTab = tabRepository.liveSelectedTab.value?.sourceTabId != null if (currentFindInPageViewState().visible) { dismissFindInView() @@ -548,6 +549,11 @@ class BrowserTabViewModel( if (navigation.canGoBack) { command.value = NavigateBack(navigation.stepsToPreviousPage) return true + } else if (hasParentTab) { + viewModelScope.launch { + tabRepository.deleteCurrentTabAndSelectSource() + } + return true } else if (!skipHome) { navigateHome() command.value = ShowKeyboard From ba882e0e469339e59159e12f2e7c2bbc4946ff0f Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 16:28:03 +0200 Subject: [PATCH 07/25] skipHome by default if value is not passed as argument and if tab has been opened from another tab --- .../main/java/com/duckduckgo/app/browser/BrowserViewModel.kt | 2 +- 1 file changed, 1 insertion(+), 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 804917f74df7..f1ade3c58d5b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -134,7 +134,7 @@ class BrowserViewModel( } } - suspend fun onOpenInNewTabRequested(query: String, skipHome: Boolean = false, fromCurrentTab: Boolean = false): String { + suspend fun onOpenInNewTabRequested(query: String, fromCurrentTab: Boolean = false, skipHome: Boolean = fromCurrentTab): String { return if (fromCurrentTab) { tabRepository.addWithSource( url = queryUrlConverter.convertQueryToUrl(query), From 5629708b8361ca268a4259eba82400c577cdd973 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 16:28:26 +0200 Subject: [PATCH 08/25] use sourceTab naming instead of child --- .../main/java/com/duckduckgo/app/tabs/db/TabsDao.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt index 85e832677af6..1cb1c6d4c5f9 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt @@ -66,7 +66,7 @@ abstract class TabsDao { abstract fun incrementPositionStartingAt(position: Int) @Query("update tabs set sourceTabId = null where sourceTabId = :removedTabId") - abstract fun updateChildTabSourceTabId(removedTabId: String) + abstract fun updateSourceTabIdRefs(removedTabId: String) @Transaction open fun addAndSelectTab(tab: TabEntity) { @@ -77,7 +77,7 @@ abstract class TabsDao { @Transaction open fun deleteTabAndUpdateSelection(tab: TabEntity) { - deleteTabAndChildRef(tab) + deleteTabAndClearTabRefs(tab) if (selectedTab() != null) { return @@ -90,7 +90,7 @@ abstract class TabsDao { @Transaction open fun deleteTabAndUpdateSelection(tab: TabEntity, newSelectedTab: TabEntity? = null) { - deleteTabAndChildRef(tab) + deleteTabAndClearTabRefs(tab) if (newSelectedTab != null) { insertTabSelection(TabSelectionEntity(tabId = newSelectedTab.tabId)) @@ -115,9 +115,9 @@ abstract class TabsDao { insertTab(tab) } - private fun deleteTabAndChildRef(tab: TabEntity) { + private fun deleteTabAndClearTabRefs(tab: TabEntity) { deleteTab(tab) - updateChildTabSourceTabId(tab.tabId) + updateSourceTabIdRefs(tab.tabId) } fun lastTab(): TabEntity? { From ed0633fe10a9e0384aff08c7fc0973f1d829e255 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 16:49:08 +0200 Subject: [PATCH 09/25] hasSourceTab instead of Parent Tab --- .../java/com/duckduckgo/app/browser/BrowserTabViewModel.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 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 8d7b8a6f346e..6b88f5ab7e06 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -535,7 +535,7 @@ class BrowserTabViewModel( fun onUserPressedBack(): Boolean { navigationAwareLoginDetector.onEvent(NavigationEvent.UserAction.NavigateBack) val navigation = webNavigationState ?: return false - val hasParentTab = tabRepository.liveSelectedTab.value?.sourceTabId != null + val hasSourceTab = tabRepository.liveSelectedTab.value?.sourceTabId != null if (currentFindInPageViewState().visible) { dismissFindInView() @@ -549,7 +549,7 @@ class BrowserTabViewModel( if (navigation.canGoBack) { command.value = NavigateBack(navigation.stepsToPreviousPage) return true - } else if (hasParentTab) { + } else if (hasSourceTab) { viewModelScope.launch { tabRepository.deleteCurrentTabAndSelectSource() } From 44bb3c26750266ae404dd923d8b5ae70479dc163 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 17:55:00 +0200 Subject: [PATCH 10/25] use room to nullify sourceTabId when tab deleted --- .../24.json | 20 +++++++++++++++---- .../com/duckduckgo/app/tabs/db/TabsDao.kt | 9 ++------- .../duckduckgo/app/tabs/model/TabEntitiy.kt | 9 +++++++++ 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/app/schemas/com.duckduckgo.app.global.db.AppDatabase/24.json b/app/schemas/com.duckduckgo.app.global.db.AppDatabase/24.json index bd4a3afcb987..3fef6ddc1ee2 100644 --- a/app/schemas/com.duckduckgo.app.global.db.AppDatabase/24.json +++ b/app/schemas/com.duckduckgo.app.global.db.AppDatabase/24.json @@ -2,7 +2,7 @@ "formatVersion": 1, "database": { "version": 24, - "identityHash": "03673f6d5937091013955e2520b94c0e", + "identityHash": "d6df0e21b463f404e4ea0a430f7d905c", "entities": [ { "tableName": "tds_tracker", @@ -258,7 +258,7 @@ }, { "tableName": "tabs", - "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`tabId` TEXT NOT NULL, `url` TEXT, `title` TEXT, `skipHome` INTEGER NOT NULL, `viewed` INTEGER NOT NULL, `position` INTEGER NOT NULL, `tabPreviewFile` TEXT, `sourceTabId` TEXT, PRIMARY KEY(`tabId`))", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`tabId` TEXT NOT NULL, `url` TEXT, `title` TEXT, `skipHome` INTEGER NOT NULL, `viewed` INTEGER NOT NULL, `position` INTEGER NOT NULL, `tabPreviewFile` TEXT, `sourceTabId` TEXT, PRIMARY KEY(`tabId`), FOREIGN KEY(`sourceTabId`) REFERENCES `tabs`(`tabId`) ON UPDATE SET NULL ON DELETE SET NULL )", "fields": [ { "fieldPath": "tabId", @@ -325,7 +325,19 @@ "createSql": "CREATE INDEX IF NOT EXISTS `index_tabs_tabId` ON `${TABLE_NAME}` (`tabId`)" } ], - "foreignKeys": [] + "foreignKeys": [ + { + "table": "tabs", + "onDelete": "SET NULL", + "onUpdate": "SET NULL", + "columns": [ + "sourceTabId" + ], + "referencedColumns": [ + "tabId" + ] + } + ] }, { "tableName": "tab_selection", @@ -746,7 +758,7 @@ "views": [], "setupQueries": [ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", - "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '03673f6d5937091013955e2520b94c0e')" + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'd6df0e21b463f404e4ea0a430f7d905c')" ] } } \ No newline at end of file diff --git a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt index 1cb1c6d4c5f9..1e7a3c5b30a0 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt @@ -77,7 +77,7 @@ abstract class TabsDao { @Transaction open fun deleteTabAndUpdateSelection(tab: TabEntity) { - deleteTabAndClearTabRefs(tab) + deleteTab(tab) if (selectedTab() != null) { return @@ -90,7 +90,7 @@ abstract class TabsDao { @Transaction open fun deleteTabAndUpdateSelection(tab: TabEntity, newSelectedTab: TabEntity? = null) { - deleteTabAndClearTabRefs(tab) + deleteTab(tab) if (newSelectedTab != null) { insertTabSelection(TabSelectionEntity(tabId = newSelectedTab.tabId)) @@ -115,11 +115,6 @@ abstract class TabsDao { insertTab(tab) } - private fun deleteTabAndClearTabRefs(tab: TabEntity) { - deleteTab(tab) - updateSourceTabIdRefs(tab.tabId) - } - fun lastTab(): TabEntity? { return tabs().lastOrNull() } diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabEntitiy.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabEntitiy.kt index 4a4cd53958be..5acd7918bcfd 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabEntitiy.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabEntitiy.kt @@ -17,11 +17,20 @@ package com.duckduckgo.app.tabs.model import androidx.room.Entity +import androidx.room.ForeignKey import androidx.room.Index import androidx.room.PrimaryKey @Entity( tableName = "tabs", + foreignKeys = [ + ForeignKey( + entity = TabEntity::class, + parentColumns = ["tabId"], + childColumns = ["sourceTabId"], + onDelete = ForeignKey.SET_NULL, + onUpdate = ForeignKey.SET_NULL + )], indices = [ Index("tabId") ] From 4fd29054d97f42137413440c8d540fccd92a78cd Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 18:16:01 +0200 Subject: [PATCH 11/25] Rename method to be explicit about what sourceTab will be added. Cleanup a isDefaultTab as it was never used in some parts of the code. --- .../duckduckgo/app/browser/BrowserViewModelTest.kt | 4 ++-- .../app/tabs/model/TabDataRepositoryTest.kt | 2 +- .../com/duckduckgo/app/browser/BrowserViewModel.kt | 12 +++++------- .../duckduckgo/app/tabs/model/TabDataRepository.kt | 2 +- .../com/duckduckgo/app/tabs/model/TabRepository.kt | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt index 5eae0cced112..08e04b90195b 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt @@ -123,7 +123,7 @@ class BrowserViewModelTest { fun whenNewTabRequestedThenTabAddedToRepository() = runBlocking { whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData()) testee.onNewTabRequested() - verify(mockTabRepository).addWithSource() + verify(mockTabRepository).addFromCurrentTab() } @Test @@ -132,7 +132,7 @@ class BrowserViewModelTest { whenever(mockOmnibarEntryConverter.convertQueryToUrl(url)).thenReturn(url) whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData()) testee.onOpenInNewTabRequested(url) - verify(mockTabRepository).addWithSource(url) + verify(mockTabRepository).addFromCurrentTab(url) } @Test diff --git a/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt b/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt index 64123aab20b9..8a3827ffc75e 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt @@ -306,7 +306,7 @@ class TabDataRepositoryTest { testee = TabDataRepository(dao, SiteFactory(mockPrivacyPractices, mockEntityLookup), mockWebViewPreviewPersister, useOurAppDetector) - val addedTabId = testee.addWithSource("http://www.example.com", skipHome = false, isDefaultTab = false) + val addedTabId = testee.addFromCurrentTab("http://www.example.com", skipHome = false, isDefaultTab = false) val addedTab = testee.liveSelectedTab.blockingObserve() assertEquals(addedTabId, addedTab?.tabId) assertEquals(addedTab?.sourceTabId, sourceTab.tabId) 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 f1ade3c58d5b..5ee8288b72c5 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -126,26 +126,24 @@ class BrowserViewModel( appEnjoymentPromptEmitter.promptType.observeForever(appEnjoymentObserver) } - suspend fun onNewTabRequested(isDefaultTab: Boolean = false, fromCurrentTab: Boolean = false): String { + suspend fun onNewTabRequested(fromCurrentTab: Boolean = false): String { return if (fromCurrentTab) { - tabRepository.addWithSource(isDefaultTab = isDefaultTab) + tabRepository.addFromCurrentTab() } else { - tabRepository.add(isDefaultTab = isDefaultTab) + tabRepository.add() } } suspend fun onOpenInNewTabRequested(query: String, fromCurrentTab: Boolean = false, skipHome: Boolean = fromCurrentTab): String { return if (fromCurrentTab) { - tabRepository.addWithSource( + tabRepository.addFromCurrentTab( url = queryUrlConverter.convertQueryToUrl(query), - isDefaultTab = false, skipHome = skipHome ) } else { tabRepository.add( url = queryUrlConverter.convertQueryToUrl(query), - skipHome = skipHome, - isDefaultTab = false + skipHome = skipHome ) } } 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 ead6fcae6cbd..22b48bd3e227 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 @@ -55,7 +55,7 @@ class TabDataRepository @Inject constructor( return tabId } - override suspend fun addWithSource(url: String?, skipHome: Boolean, isDefaultTab: Boolean): String { + override suspend fun addFromCurrentTab(url: String?, skipHome: Boolean, isDefaultTab: Boolean): String { val tabId = generateTabId() val sourceTabId = withContext(Dispatchers.IO) { tabsDao.selectedTab()?.tabId diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt index 61c22cab7bd0..f08b9eaa5257 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt @@ -31,7 +31,7 @@ interface TabRepository { */ suspend fun add(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false): String - suspend fun addWithSource(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false): String + suspend fun addFromCurrentTab(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false): String suspend fun addNewTabAfterExistingTab(url: String? = null, tabId: String) From ef0ae0a5cae4199c58126d0a8a986ff7612b5e7f Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 18:36:17 +0200 Subject: [PATCH 12/25] Passing around sourceTabId from tab fragment. Adds flexibility and it's more explicit about what's happening. --- .../com/duckduckgo/app/browser/BrowserActivity.kt | 10 +++++----- .../duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++-- .../duckduckgo/app/browser/BrowserTabViewModel.kt | 8 ++++---- .../duckduckgo/app/browser/BrowserViewModel.kt | 15 ++++++++------- .../app/tabs/model/TabDataRepository.kt | 15 ++++++--------- .../duckduckgo/app/tabs/model/TabRepository.kt | 2 +- 6 files changed, 26 insertions(+), 28 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 ab32bb0612d6..e2199ad25ebd 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt @@ -211,7 +211,7 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { launch { viewModel.onOpenShortcut(sharedText) } } else { Timber.w("opening in new tab requested for $sharedText") - launch { viewModel.onOpenInNewTabRequested(query = sharedText, skipHome = true, fromCurrentTab = false) } + launch { viewModel.onOpenInNewTabRequested(query = sharedText, skipHome = true) } return } } @@ -289,15 +289,15 @@ class BrowserActivity : DuckDuckGoActivity(), CoroutineScope by MainScope() { launch { viewModel.onNewTabRequested() } } - fun openInNewTab(query: String, fromCurrentTab: Boolean) { + fun openInNewTab(query: String, sourceTabId: String?) { launch { - viewModel.onOpenInNewTabRequested(query = query, fromCurrentTab = fromCurrentTab) + viewModel.onOpenInNewTabRequested(query = query, sourceTabId = sourceTabId) } } - fun openMessageInNewTab(message: Message) { + fun openMessageInNewTab(message: Message, sourceTabId: String?) { openMessageInNewTabJob = launch { - val tabId = viewModel.onNewTabRequested(fromCurrentTab = true) + val tabId = viewModel.onNewTabRequested(sourceTabId = sourceTabId) val fragment = openNewTab(tabId, null, false) fragment.messageFromPreviousTab = message } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 22818fd5144b..4ec2966e4c9e 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -470,10 +470,10 @@ class BrowserTabFragment : Fragment(), FindListener, CoroutineScope, DaxDialogLi when (it) { is Command.Refresh -> refresh() is Command.OpenInNewTab -> { - browserActivity?.openInNewTab(it.query, it.fromCurrentTab) + browserActivity?.openInNewTab(it.query, it.sourceTabId) } is Command.OpenMessageInNewTab -> { - browserActivity?.openMessageInNewTab(it.message) + browserActivity?.openMessageInNewTab(it.message, it.sourceTabId) } is Command.OpenInNewBackgroundTab -> { openInNewBackgroundTab() 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 6b88f5ab7e06..c60671e51eae 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -205,8 +205,8 @@ class BrowserTabViewModel( class Navigate(val url: String) : Command() class NavigateBack(val steps: Int) : Command() object NavigateForward : Command() - class OpenInNewTab(val query: String, val fromCurrentTab: Boolean = false) : Command() - class OpenMessageInNewTab(val message: Message) : Command() + class OpenInNewTab(val query: String, val sourceTabId: String? = null) : Command() + class OpenMessageInNewTab(val message: Message, val sourceTabId: String? = null) : Command() class OpenInNewBackgroundTab(val query: String) : Command() object LaunchNewTab : Command() object ResetHistory : Command() @@ -1033,7 +1033,7 @@ class BrowserTabViewModel( return when (requiredAction) { is RequiredAction.OpenInNewTab -> { command.value = GenerateWebViewPreviewImage - command.value = OpenInNewTab(query = requiredAction.url, fromCurrentTab = true) + command.value = OpenInNewTab(query = requiredAction.url, sourceTabId = tabId) true } is RequiredAction.OpenInNewBackgroundTab -> { @@ -1316,7 +1316,7 @@ class BrowserTabViewModel( } override fun openMessageInNewTab(message: Message) { - command.value = OpenMessageInNewTab(message) + command.value = OpenMessageInNewTab(message, tabId) } override fun recoverFromRenderProcessGone() { 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 5ee8288b72c5..d368a7c8b45b 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -126,19 +126,20 @@ class BrowserViewModel( appEnjoymentPromptEmitter.promptType.observeForever(appEnjoymentObserver) } - suspend fun onNewTabRequested(fromCurrentTab: Boolean = false): String { - return if (fromCurrentTab) { - tabRepository.addFromCurrentTab() + suspend fun onNewTabRequested(sourceTabId: String? = null): String { + return if (sourceTabId != null) { + tabRepository.addFromSourceTab(sourceTabId = sourceTabId) } else { tabRepository.add() } } - suspend fun onOpenInNewTabRequested(query: String, fromCurrentTab: Boolean = false, skipHome: Boolean = fromCurrentTab): String { - return if (fromCurrentTab) { - tabRepository.addFromCurrentTab( + suspend fun onOpenInNewTabRequested(query: String, sourceTabId: String? = null, skipHome: Boolean = false): String { + return if (sourceTabId != null) { + tabRepository.addFromSourceTab( url = queryUrlConverter.convertQueryToUrl(query), - skipHome = skipHome + skipHome = skipHome, + sourceTabId = sourceTabId ) } else { tabRepository.add( 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 22b48bd3e227..1bfba1f13804 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 @@ -55,18 +55,15 @@ class TabDataRepository @Inject constructor( return tabId } - override suspend fun addFromCurrentTab(url: String?, skipHome: Boolean, isDefaultTab: Boolean): String { + override suspend fun addFromSourceTab(url: String?, skipHome: Boolean, isDefaultTab: Boolean, sourceTabId: String?): String { val tabId = generateTabId() - val sourceTabId = withContext(Dispatchers.IO) { - tabsDao.selectedTab()?.tabId - } add( - tabId, - buildSiteData(url), - skipHome = skipHome, - isDefaultTab = isDefaultTab, - sourceTabId = sourceTabId + tabId, + buildSiteData(url), + skipHome = skipHome, + isDefaultTab = isDefaultTab, + sourceTabId = sourceTabId ) return tabId diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt index f08b9eaa5257..894c5d43fcf0 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt @@ -31,7 +31,7 @@ interface TabRepository { */ suspend fun add(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false): String - suspend fun addFromCurrentTab(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false): String + suspend fun addFromSourceTab(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean= false, sourceTabId: String?): String suspend fun addNewTabAfterExistingTab(url: String? = null, tabId: String) From fd9238d4f4fc57aeebadd1d3adad5560fac5be1b Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 18:52:51 +0200 Subject: [PATCH 13/25] apply code style --- .../app/tabs/model/TabDataRepository.kt | 26 +++++++++---------- .../app/tabs/model/TabRepository.kt | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) 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 1bfba1f13804..3aa13f2bb4fb 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 @@ -26,10 +26,8 @@ import com.duckduckgo.app.global.useourapp.UseOurAppDetector import com.duckduckgo.app.tabs.db.TabsDao import io.reactivex.Scheduler import io.reactivex.schedulers.Schedulers -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext import timber.log.Timber import java.util.* import javax.inject.Inject @@ -59,11 +57,11 @@ class TabDataRepository @Inject constructor( val tabId = generateTabId() add( - tabId, - buildSiteData(url), - skipHome = skipHome, - isDefaultTab = isDefaultTab, - sourceTabId = sourceTabId + tabId, + buildSiteData(url), + skipHome = skipHome, + isDefaultTab = isDefaultTab, + sourceTabId = sourceTabId ) return tabId @@ -99,7 +97,8 @@ class TabDataRepository @Inject constructor( } Timber.i("About to add a new tab, isDefaultTab: $isDefaultTab. $tabId, position: $position") - tabsDao.addAndSelectTab(TabEntity( + tabsDao.addAndSelectTab( + TabEntity( tabId = tabId, url = data.value?.url, title = data.value?.title, @@ -107,7 +106,8 @@ class TabDataRepository @Inject constructor( viewed = true, position = position, sourceTabId = sourceTabId - )) + ) + ) } } @@ -176,10 +176,10 @@ class TabDataRepository @Inject constructor( deleteOldPreviewImages(tabToDelete.tabId) val tabToSelect = tabToDelete.sourceTabId - .takeUnless { it.isNullOrBlank() } - ?.let { - tabsDao.tab(it) - } + .takeUnless { it.isNullOrBlank() } + ?.let { + tabsDao.tab(it) + } tabsDao.deleteTabAndUpdateSelection(tabToDelete, tabToSelect) siteData.remove(tabToDelete.tabId) } diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt index 894c5d43fcf0..4f1900686a72 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt @@ -31,7 +31,7 @@ interface TabRepository { */ suspend fun add(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false): String - suspend fun addFromSourceTab(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean= false, sourceTabId: String?): String + suspend fun addFromSourceTab(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false, sourceTabId: String?): String suspend fun addNewTabAfterExistingTab(url: String? = null, tabId: String) From d05d08ea011cd2a6b923184e440c264f8c56a2df Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 23:11:13 +0200 Subject: [PATCH 14/25] Removing isDefaultTab param from multiple methods where it was always false --- .../app/browser/BrowserViewModel.kt | 4 +-- .../app/tabs/model/TabDataRepository.kt | 27 ++++++++++++++----- .../app/tabs/model/TabRepository.kt | 6 +++-- 3 files changed, 26 insertions(+), 11 deletions(-) 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 d368a7c8b45b..84bae806e0fc 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt @@ -150,9 +150,9 @@ class BrowserViewModel( } suspend fun onTabsUpdated(tabs: List?) { - if (tabs == null || tabs.isEmpty()) { + if (tabs.isNullOrEmpty()) { Timber.i("Tabs list is null or empty; adding default tab") - tabRepository.add(isDefaultTab = true) + tabRepository.addDefaultTab() return } } 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 3aa13f2bb4fb..6bed8db48f45 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 @@ -47,26 +47,39 @@ class TabDataRepository @Inject constructor( private val siteData: LinkedHashMap> = LinkedHashMap() - override suspend fun add(url: String?, skipHome: Boolean, isDefaultTab: Boolean): String { + override suspend fun add(url: String?, skipHome: Boolean): String { val tabId = generateTabId() - add(tabId, buildSiteData(url), skipHome = skipHome, isDefaultTab = isDefaultTab) + add(tabId, buildSiteData(url), skipHome = skipHome, isDefaultTab = false) return tabId } - override suspend fun addFromSourceTab(url: String?, skipHome: Boolean, isDefaultTab: Boolean, sourceTabId: String?): String { + override suspend fun addFromSourceTab(url: String?, skipHome: Boolean, sourceTabId: String?): String { val tabId = generateTabId() add( - tabId, - buildSiteData(url), + tabId = tabId, + data = buildSiteData(url), skipHome = skipHome, - isDefaultTab = isDefaultTab, + isDefaultTab = false, sourceTabId = sourceTabId ) return tabId } + override suspend fun addDefaultTab(): String { + val tabId = generateTabId() + + add( + tabId = tabId, + data = buildSiteData(null), + skipHome = false, + isDefaultTab = true + ) + + return tabId + } + private fun generateTabId() = UUID.randomUUID().toString() private fun buildSiteData(url: String?): MutableLiveData { @@ -122,7 +135,7 @@ class TabDataRepository @Inject constructor( if (tabId != null) { select(tabId) } else { - add(url, skipHome = true, isDefaultTab = false) + add(url, skipHome = true) } } diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt index 4f1900686a72..0e91cd9106bf 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt @@ -29,9 +29,11 @@ interface TabRepository { /** * @return tabId of new record */ - suspend fun add(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false): String + suspend fun add(url: String? = null, skipHome: Boolean = false): String - suspend fun addFromSourceTab(url: String? = null, skipHome: Boolean = false, isDefaultTab: Boolean = false, sourceTabId: String?): String + suspend fun addDefaultTab(): String + + suspend fun addFromSourceTab(url: String? = null, skipHome: Boolean = false, sourceTabId: String?): String suspend fun addNewTabAfterExistingTab(url: String? = null, tabId: String) From 7f4cde2cc1a1555704b879275089cf894e2183a9 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 23:44:12 +0200 Subject: [PATCH 15/25] remove unused method --- app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt index 1e7a3c5b30a0..8ce0b347393c 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt @@ -65,9 +65,6 @@ abstract class TabsDao { @Query("update tabs set position = position + 1 where position >= :position") abstract fun incrementPositionStartingAt(position: Int) - @Query("update tabs set sourceTabId = null where sourceTabId = :removedTabId") - abstract fun updateSourceTabIdRefs(removedTabId: String) - @Transaction open fun addAndSelectTab(tab: TabEntity) { deleteBlankTabs() From 5af2af72cd34440eb71591a74065083114a55571 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Thu, 6 Aug 2020 23:44:56 +0200 Subject: [PATCH 16/25] Unit test new foreign key added logic --- .../java/com/duckduckgo/app/tabs/db/TabsDaoTest.kt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 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 26b056ea0e08..751da71f4bae 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 @@ -206,7 +206,6 @@ class TabsDaoTest { @Test fun whenTabInsertedAtPositionThenOtherTabsReordered() { - testee.insertTab(TabEntity("TAB_ID1", position = 0)) testee.insertTab(TabEntity("TAB_ID2", position = 1)) testee.insertTab(TabEntity("TAB_ID3", position = 2)) @@ -226,7 +225,18 @@ class TabsDaoTest { assertEquals(3, tabs[3].position) assertEquals("TAB_ID3", tabs[3].tabId) - } + @Test + fun whenSourceTabDeletedThenRelatedTabsUpdated() { + val firstTab = TabEntity("TAB_ID", "http//updatedexample.com", position = 0) + val secondTab = TabEntity("TAB_ID_1", "http//updatedexample.com", position = 1, sourceTabId = "TAB_ID") + testee.insertTab(firstTab) + testee.insertTab(secondTab) + + testee.deleteTab(firstTab) + + assertNotNull(testee.tab("TAB_ID_1")) + assertNull(testee.tab("TAB_ID_1")?.sourceTabId) + } } From 88c4928a30b9d1cde4f1421ac4d22c4900796c12 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 7 Aug 2020 00:42:30 +0200 Subject: [PATCH 17/25] Add necessary migration to create foreign key in table tabs --- .../java/com/duckduckgo/app/global/db/AppDatabase.kt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt b/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt index ea4e529ffab0..e2dbb0373dcf 100644 --- a/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt +++ b/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt @@ -318,7 +318,16 @@ class MigrationsProvider( val MIGRATION_23_TO_24: Migration = object : Migration(23, 24) { override fun migrate(database: SupportSQLiteDatabase) { - database.execSQL("ALTER TABLE `tabs` ADD COLUMN `sourceTabId` TEXT") + database.execSQL("CREATE TABLE IF NOT EXISTS tabs_new " + + "(tabId TEXT NOT NULL, url TEXT, title TEXT, skipHome INTEGER NOT NULL, viewed INTEGER NOT NULL, position INTEGER NOT NULL, tabPreviewFile TEXT, sourceTabId TEXT," + + " PRIMARY KEY(tabId)," + + " FOREIGN KEY(sourceTabId) REFERENCES tabs(tabId) ON UPDATE SET NULL ON DELETE SET NULL )") + database.execSQL("INSERT INTO tabs_new (tabId, url, title, skipHome, viewed, position, tabPreviewFile) " + + "SELECT tabId, url, title, skipHome, viewed, position, tabPreviewFile " + + "FROM tabs") + database.execSQL("DROP TABLE tabs") + database.execSQL("ALTER TABLE tabs_new RENAME TO tabs") + database.execSQL("CREATE INDEX IF NOT EXISTS index_tabs_tabId ON tabs (tabId)") } } From b1bbbbd98b484590e87fa38d1dec76b949bc243c Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 7 Aug 2020 08:44:31 +0200 Subject: [PATCH 18/25] Tidy up and add new tests to TabDataRepositoryTest --- .../app/tabs/model/TabDataRepositoryTest.kt | 77 ++++++++++--------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt b/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt index 8a3827ffc75e..83458af67edb 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt @@ -45,6 +45,7 @@ import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.runBlocking +import org.junit.After import org.junit.Assert.* import org.junit.Before import org.junit.Rule @@ -94,7 +95,6 @@ class TabDataRepositoryTest { @Test fun whenAddNewTabAfterExistingTabWithUrlWithNoHostThenUsesUrlAsTitle() = runBlocking { - val badUrl = "//bad/url" testee.addNewTabAfterExistingTab(badUrl, "tabid") val captor = argumentCaptor() @@ -163,21 +163,6 @@ class TabDataRepositoryTest { assertEquals(url, testee.retrieveSiteData(createdId).value!!.url) } - @Test - fun whenAddRecordCalledThenTabAddedAndSiteDataAdded() = runBlocking { - val record = MutableLiveData() - testee.add(TAB_ID, record) - verify(mockDao).addAndSelectTab(any()) - assertSame(record, testee.retrieveSiteData(TAB_ID)) - } - - @Test - fun whenDataExistsForTabThenRetrieveReturnsIt() = runBlocking { - val record = MutableLiveData() - testee.add(TAB_ID, record) - assertSame(record, testee.retrieveSiteData(TAB_ID)) - } - @Test fun whenDataDoesNotExistForTabThenRetrieveCreatesIt() { assertNotNull(testee.retrieveSiteData(TAB_ID)) @@ -185,21 +170,24 @@ class TabDataRepositoryTest { @Test fun whenTabDeletedThenTabAndDataCleared() = runBlocking { - val siteData = MutableLiveData() - testee.add(TAB_ID, siteData) + val addedTabId = testee.add() + val siteData = testee.retrieveSiteData(addedTabId) + + testee.delete(TabEntity(addedTabId, position = 0)) - testee.delete(TabEntity(TAB_ID, position = 0)) verify(mockDao).deleteTabAndUpdateSelection(any()) - assertNotSame(siteData, testee.retrieveSiteData(TAB_ID)) + assertNotSame(siteData, testee.retrieveSiteData(addedTabId)) } @Test fun whenAllDeletedThenTabAndDataCleared() = runBlocking { - val siteData = MutableLiveData() - testee.add(TAB_ID, siteData) + val addedTabId = testee.add() + val siteData = testee.retrieveSiteData(addedTabId) + testee.deleteAll() + verify(mockDao).deleteAllTabs() - assertNotSame(siteData, testee.retrieveSiteData(TAB_ID)) + assertNotSame(siteData, testee.retrieveSiteData(addedTabId)) } @Test @@ -216,7 +204,6 @@ class TabDataRepositoryTest { val captor = argumentCaptor() verify(mockDao).addAndSelectTab(captor.capture()) - assertTrue(captor.firstValue.position == 0) } @@ -231,23 +218,43 @@ class TabDataRepositoryTest { val captor = argumentCaptor() verify(mockDao).addAndSelectTab(captor.capture()) - assertTrue(captor.firstValue.position == 1) } + @Test + fun whenAddDefaultTabToExistingListOfTabsThenTabIsNotCreated() = runBlocking { + val db = createDatabase() + val dao = db.tabsDao() + testee = TabDataRepository(dao, SiteFactory(mockPrivacyPractices, mockEntityLookup), mockWebViewPreviewPersister, useOurAppDetector) + testee.add("example.com") + + testee.addDefaultTab() + + assertTrue(testee.liveTabs.blockingObserve()?.size == 1) + } + + @Test + fun whenAddDefaultTabToEmptyTabsThenTabIsCreated() = runBlocking { + val db = createDatabase() + val dao = db.tabsDao() + testee = TabDataRepository(dao, SiteFactory(mockPrivacyPractices, mockEntityLookup), mockWebViewPreviewPersister, useOurAppDetector) + + testee.addDefaultTab() + + assertTrue(testee.liveTabs.blockingObserve()?.size == 1) + } + @Test fun whenSelectByUrlOrNewTabIfUrlAlreadyExistedInATabThenSelectTheTab() = runBlocking { val db = createDatabase() val dao = db.tabsDao() dao.insertTab(TabEntity(tabId = "id", url = "http://www.example.com", skipHome = false, viewed = true, position = 0)) - testee = TabDataRepository(dao, SiteFactory(mockPrivacyPractices, mockEntityLookup), mockWebViewPreviewPersister, useOurAppDetector) testee.selectByUrlOrNewTab("http://www.example.com") val value = testee.liveSelectedTab.blockingObserve()?.tabId assertEquals("id", value) - db.close() } @@ -255,14 +262,12 @@ class TabDataRepositoryTest { fun whenSelectByUrlOrNewTabIfUrlNotExistedInATabThenAddNewTab() = runBlocking { val db = createDatabase() val dao = db.tabsDao() - testee = TabDataRepository(dao, SiteFactory(mockPrivacyPractices, mockEntityLookup), mockWebViewPreviewPersister, useOurAppDetector) testee.selectByUrlOrNewTab("http://www.example.com") val value = testee.liveSelectedTab.blockingObserve()?.url assertEquals("http://www.example.com", value) - db.close() } @@ -271,14 +276,12 @@ class TabDataRepositoryTest { val db = createDatabase() val dao = db.tabsDao() dao.insertTab(TabEntity(tabId = "id", url = "http://www.$USE_OUR_APP_DOMAIN/test", skipHome = false, viewed = true, position = 0)) - testee = TabDataRepository(dao, SiteFactory(mockPrivacyPractices, mockEntityLookup), mockWebViewPreviewPersister, useOurAppDetector) testee.selectByUrlOrNewTab("http://m.$USE_OUR_APP_DOMAIN") val value = testee.liveSelectedTab.blockingObserve()?.tabId assertEquals("id", value) - db.close() } @@ -286,27 +289,25 @@ class TabDataRepositoryTest { fun whenSelectByUrlOrNewTabIfUrlNotExistedInATabAndUrlMatchesUseOurAppDomainThenAddNewTabWithCorrectUrl() = runBlocking { val db = createDatabase() val dao = db.tabsDao() - testee = TabDataRepository(dao, SiteFactory(mockPrivacyPractices, mockEntityLookup), mockWebViewPreviewPersister, useOurAppDetector) testee.selectByUrlOrNewTab("http://m.$USE_OUR_APP_DOMAIN") val value = testee.liveSelectedTab.blockingObserve()?.url assertEquals("http://m.$USE_OUR_APP_DOMAIN", value) - db.close() } @Test - fun whenAddWithSourceEnsureTabEntryContainsExpectedSourceId() = runBlocking { + fun whenAddFromSourceTabEnsureTabEntryContainsExpectedSourceId() = runBlocking { val db = createDatabase() val dao = db.tabsDao() val sourceTab = TabEntity(tabId = "sourceId", url = "http://www.example.com", position = 0) dao.addAndSelectTab(sourceTab) - testee = TabDataRepository(dao, SiteFactory(mockPrivacyPractices, mockEntityLookup), mockWebViewPreviewPersister, useOurAppDetector) - val addedTabId = testee.addFromCurrentTab("http://www.example.com", skipHome = false, isDefaultTab = false) + val addedTabId = testee.addFromSourceTab("http://www.example.com", skipHome = false, sourceTabId = "sourceId") + val addedTab = testee.liveSelectedTab.blockingObserve() assertEquals(addedTabId, addedTab?.tabId) assertEquals(addedTab?.sourceTabId, sourceTab.tabId) @@ -320,12 +321,12 @@ class TabDataRepositoryTest { val tabToDelete = TabEntity(tabId = "tabToDeleteId", url = "http://www.example.com", position = 1, sourceTabId = "sourceId") dao.addAndSelectTab(sourceTab) dao.addAndSelectTab(tabToDelete) - testee = TabDataRepository(dao, SiteFactory(mockPrivacyPractices, mockEntityLookup), mockWebViewPreviewPersister, useOurAppDetector) - var currentSelectedTabId = testee.liveSelectedTab.blockingObserve()?.tabId assertEquals(currentSelectedTabId, tabToDelete.tabId) + testee.deleteCurrentTabAndSelectSource() + currentSelectedTabId = testee.liveSelectedTab.blockingObserve()?.tabId assertEquals(currentSelectedTabId, sourceTab.tabId) } From fd52f6a5615add7f6a10e4cbe769efe6b3507d0b Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 7 Aug 2020 09:24:08 +0200 Subject: [PATCH 19/25] remove unused method --- .../app/browser/BrowserTabViewModelTest.kt | 12 ------------ .../duckduckgo/app/browser/BrowserTabViewModel.kt | 4 ---- .../duckduckgo/app/browser/WebViewClientListener.kt | 1 - 3 files changed, 17 deletions(-) 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 ba957ba2ac48..d2da8c096b57 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -1319,18 +1319,6 @@ class BrowserTabViewModelTest { assertEquals(true, testee.browserViewState.value?.browserShowing) } - @Test - fun whenOpenInNewTabThenOpenInNewTabCommandWithCorrectUrlSent() { - val url = "https://example.com" - testee.openInNewTab(url) - verify(mockCommandObserver).onChanged(commandCaptor.capture()) - - val command = commandCaptor.lastValue - assertTrue(command is Command.OpenInNewTab) - command as Command.OpenInNewTab - assertEquals(url, command.query) - } - @Test fun whenRecoveringFromProcessGoneThenShowErrorWithAction() { testee.recoverFromRenderProcessGone() 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 c60671e51eae..46e79419ab04 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -1311,10 +1311,6 @@ class BrowserTabViewModel( command.value = HandleExternalAppLink(appLink) } - override fun openInNewTab(url: String?) { - command.value = OpenInNewTab(url.orEmpty()) - } - override fun openMessageInNewTab(message: Message) { command.value = OpenMessageInNewTab(message, tabId) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt index 15859a2161ab..dba8ae8ea816 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/WebViewClientListener.kt @@ -42,7 +42,6 @@ interface WebViewClientListener { fun exitFullScreen() fun showFileChooser(filePathCallback: ValueCallback>, fileChooserParams: WebChromeClient.FileChooserParams) fun externalAppLinkClicked(appLink: SpecialUrlDetector.UrlType.IntentType) - fun openInNewTab(url: String?) fun openMessageInNewTab(message: Message) fun recoverFromRenderProcessGone() fun requiresAuthentication(request: BasicAuthenticationRequest) From 84ed2b7ecc719a051ad1c341991b1230a5beb492 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 7 Aug 2020 09:27:03 +0200 Subject: [PATCH 20/25] update BrowserTabViewModel tests --- .../app/browser/BrowserTabViewModelTest.kt | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) 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 d2da8c096b57..bd2fa8b65752 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -476,7 +476,9 @@ class BrowserTabViewModelTest { givenOneActiveTabSelected() givenInvalidatedGlobalLayout() testee.onUserSubmittedQuery("foo") - assertCommandIssued() + assertCommandIssued { + assertNull(sourceTabId) + } } @Test @@ -1093,7 +1095,9 @@ class BrowserTabViewModelTest { testee.onRefreshRequested() - assertCommandIssued() + assertCommandIssued() { + assertNull(sourceTabId) + } } @Test @@ -1188,6 +1192,10 @@ class BrowserTabViewModelTest { testee.userSelectedItemFromLongPressMenu(longPressTarget, mockMenItem) val command = captureCommands().value as Command.OpenInNewTab assertEquals("http://example.com", command.query) + + assertCommandIssued { + assertNotNull(sourceTabId) + } } @Test @@ -1336,6 +1344,7 @@ class BrowserTabViewModelTest { assertCommandIssued { assertEquals("https://example.com", query) + assertNull(sourceTabId) } } @@ -1546,6 +1555,16 @@ class BrowserTabViewModelTest { assertTrue(commandCaptor.allValues.contains(Command.ShowKeyboard)) } + @Test + fun whenUserPressesBackOnATabWithASourceTabThenDeleteCurrentAndSelectSource() = coroutineRule.runBlocking { + selectedTabLiveData.value = TabEntity("TAB_ID", "https://example.com", position = 0, sourceTabId = "TAB_ID_SOURCE") + setupNavigation(isBrowsing = true) + + testee.onUserPressedBack() + + verify(mockTabsRepository).deleteCurrentTabAndSelectSource() + } + @Test fun whenScheduledSurveyChangesAndInstalledDaysMatchThenCtaIsSurvey() { testee.onSurveyChanged(Survey("abc", "http://example.com", daysInstalled = 1, status = Survey.Status.SCHEDULED)) From 1f1170a07bed66b342d47cdb070045509c2f3d1a Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 7 Aug 2020 09:36:00 +0200 Subject: [PATCH 21/25] udpate unit tests for BrowserViewModel --- .../app/browser/BrowserViewModelTest.kt | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt index 08e04b90195b..a1dcda909021 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt @@ -123,7 +123,14 @@ class BrowserViewModelTest { fun whenNewTabRequestedThenTabAddedToRepository() = runBlocking { whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData()) testee.onNewTabRequested() - verify(mockTabRepository).addFromCurrentTab() + verify(mockTabRepository).add() + } + + @Test + fun whenNewTabRequestedFromSourceTabThenTabAddedToRepositoryWithSourceTabId() = runBlocking { + whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData()) + testee.onNewTabRequested("sourceTabId") + verify(mockTabRepository).addFromSourceTab(sourceTabId = "sourceTabId") } @Test @@ -132,13 +139,22 @@ class BrowserViewModelTest { whenever(mockOmnibarEntryConverter.convertQueryToUrl(url)).thenReturn(url) whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData()) testee.onOpenInNewTabRequested(url) - verify(mockTabRepository).addFromCurrentTab(url) + verify(mockTabRepository).add(url = url, skipHome = false) + } + + @Test + fun whenOpenInNewTabRequestedWithSourceTabIdThenTabAddedToRepositoryWithSourceTabId() = runBlocking { + val url = "http://example.com" + whenever(mockOmnibarEntryConverter.convertQueryToUrl(url)).thenReturn(url) + whenever(mockTabRepository.liveSelectedTab).doReturn(MutableLiveData()) + testee.onOpenInNewTabRequested(url, sourceTabId = "tabId") + verify(mockTabRepository).addFromSourceTab(url = url, skipHome = false, sourceTabId = "tabId") } @Test - fun whenTabsUpdatedAndNoTabsThenNewTabAddedToRepository() = runBlocking { + fun whenTabsUpdatedAndNoTabsThenDefaultTabAddedToRepository() = runBlocking { testee.onTabsUpdated(ArrayList()) - verify(mockTabRepository).add(null, false, true) + verify(mockTabRepository).addDefaultTab() } @Test From 343635f7f7a2791d67d4ab9dfc367a6d86b2615c Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 7 Aug 2020 09:37:29 +0200 Subject: [PATCH 22/25] apply code style --- .../app/tabs/model/TabDataRepositoryTest.kt | 11 +-------- .../duckduckgo/app/global/db/AppDatabase.kt | 24 +++++++++++-------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt b/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt index 83458af67edb..56f0caa1c055 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/tabs/model/TabDataRepositoryTest.kt @@ -19,7 +19,6 @@ package com.duckduckgo.app.tabs.model import androidx.arch.core.executor.testing.InstantTaskExecutorRule -import androidx.lifecycle.MutableLiveData import androidx.room.Room import androidx.test.annotation.UiThreadTest import androidx.test.platform.app.InstrumentationRegistry @@ -29,23 +28,15 @@ import com.duckduckgo.app.blockingObserve import com.duckduckgo.app.browser.tabpreview.WebViewPreviewPersister import com.duckduckgo.app.global.db.AppDatabase import com.duckduckgo.app.global.events.db.UserEventsStore -import com.duckduckgo.app.global.model.Site import com.duckduckgo.app.global.model.SiteFactory import com.duckduckgo.app.global.useourapp.UseOurAppDetector import com.duckduckgo.app.global.useourapp.UseOurAppDetector.Companion.USE_OUR_APP_DOMAIN import com.duckduckgo.app.privacy.model.PrivacyPractices import com.duckduckgo.app.tabs.db.TabsDao import com.duckduckgo.app.trackerdetection.EntityLookup -import com.nhaarman.mockitokotlin2.any -import com.nhaarman.mockitokotlin2.anyOrNull -import com.nhaarman.mockitokotlin2.argumentCaptor -import com.nhaarman.mockitokotlin2.eq -import com.nhaarman.mockitokotlin2.mock -import com.nhaarman.mockitokotlin2.verify -import com.nhaarman.mockitokotlin2.whenever +import com.nhaarman.mockitokotlin2.* import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.runBlocking -import org.junit.After import org.junit.Assert.* import org.junit.Before import org.junit.Rule diff --git a/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt b/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt index e2dbb0373dcf..61cddf9532a0 100644 --- a/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt +++ b/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt @@ -33,12 +33,12 @@ import com.duckduckgo.app.cta.db.DismissedCtaDao import com.duckduckgo.app.cta.model.DismissedCta import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteDao import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteEntity +import com.duckduckgo.app.global.events.db.UserEventEntity +import com.duckduckgo.app.global.events.db.UserEventTypeConverter +import com.duckduckgo.app.global.events.db.UserEventsDao import com.duckduckgo.app.global.exception.UncaughtExceptionDao import com.duckduckgo.app.global.exception.UncaughtExceptionEntity import com.duckduckgo.app.global.exception.UncaughtExceptionSourceConverter -import com.duckduckgo.app.global.events.db.UserEventsDao -import com.duckduckgo.app.global.events.db.UserEventEntity -import com.duckduckgo.app.global.events.db.UserEventTypeConverter import com.duckduckgo.app.httpsupgrade.db.HttpsBloomFilterSpecDao import com.duckduckgo.app.httpsupgrade.db.HttpsWhitelistDao import com.duckduckgo.app.httpsupgrade.model.HttpsBloomFilterSpec @@ -318,13 +318,17 @@ class MigrationsProvider( val MIGRATION_23_TO_24: Migration = object : Migration(23, 24) { override fun migrate(database: SupportSQLiteDatabase) { - database.execSQL("CREATE TABLE IF NOT EXISTS tabs_new " + - "(tabId TEXT NOT NULL, url TEXT, title TEXT, skipHome INTEGER NOT NULL, viewed INTEGER NOT NULL, position INTEGER NOT NULL, tabPreviewFile TEXT, sourceTabId TEXT," + - " PRIMARY KEY(tabId)," + - " FOREIGN KEY(sourceTabId) REFERENCES tabs(tabId) ON UPDATE SET NULL ON DELETE SET NULL )") - database.execSQL("INSERT INTO tabs_new (tabId, url, title, skipHome, viewed, position, tabPreviewFile) " - + "SELECT tabId, url, title, skipHome, viewed, position, tabPreviewFile " - + "FROM tabs") + database.execSQL( + "CREATE TABLE IF NOT EXISTS tabs_new " + + "(tabId TEXT NOT NULL, url TEXT, title TEXT, skipHome INTEGER NOT NULL, viewed INTEGER NOT NULL, position INTEGER NOT NULL, tabPreviewFile TEXT, sourceTabId TEXT," + + " PRIMARY KEY(tabId)," + + " FOREIGN KEY(sourceTabId) REFERENCES tabs(tabId) ON UPDATE SET NULL ON DELETE SET NULL )" + ) + database.execSQL( + "INSERT INTO tabs_new (tabId, url, title, skipHome, viewed, position, tabPreviewFile) " + + "SELECT tabId, url, title, skipHome, viewed, position, tabPreviewFile " + + "FROM tabs" + ) database.execSQL("DROP TABLE tabs") database.execSQL("ALTER TABLE tabs_new RENAME TO tabs") database.execSQL("CREATE INDEX IF NOT EXISTS index_tabs_tabId ON tabs (tabId)") From e3e6b57a59b1cca94607091e3fa85cb42dc542f3 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 7 Aug 2020 09:45:08 +0200 Subject: [PATCH 23/25] add comment to document issue with sqlite --- app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt b/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt index 61cddf9532a0..ab43973b8d7b 100644 --- a/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt +++ b/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt @@ -318,6 +318,8 @@ class MigrationsProvider( val MIGRATION_23_TO_24: Migration = object : Migration(23, 24) { override fun migrate(database: SupportSQLiteDatabase) { + //https://stackoverflow.com/a/57797179/980345 + //SQLite does not support Alter table operations like Foreign keys database.execSQL( "CREATE TABLE IF NOT EXISTS tabs_new " + "(tabId TEXT NOT NULL, url TEXT, title TEXT, skipHome INTEGER NOT NULL, viewed INTEGER NOT NULL, position INTEGER NOT NULL, tabPreviewFile TEXT, sourceTabId TEXT," + From 7ab35691edcb08da94c49e97065c33cd0b6bd7eb Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 7 Aug 2020 10:04:07 +0200 Subject: [PATCH 24/25] apply codestyle --- app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt b/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt index ab43973b8d7b..736081db1ae4 100644 --- a/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt +++ b/app/src/main/java/com/duckduckgo/app/global/db/AppDatabase.kt @@ -318,8 +318,8 @@ class MigrationsProvider( val MIGRATION_23_TO_24: Migration = object : Migration(23, 24) { override fun migrate(database: SupportSQLiteDatabase) { - //https://stackoverflow.com/a/57797179/980345 - //SQLite does not support Alter table operations like Foreign keys + // https://stackoverflow.com/a/57797179/980345 + // SQLite does not support Alter table operations like Foreign keys database.execSQL( "CREATE TABLE IF NOT EXISTS tabs_new " + "(tabId TEXT NOT NULL, url TEXT, title TEXT, skipHome INTEGER NOT NULL, viewed INTEGER NOT NULL, position INTEGER NOT NULL, tabPreviewFile TEXT, sourceTabId TEXT," + From 3795162dbbdbbfdf4320d1129e4335756671bd75 Mon Sep 17 00:00:00 2001 From: Cristian Monforte Date: Fri, 7 Aug 2020 12:25:39 +0200 Subject: [PATCH 25/25] make method param sourceTabId non-nullable --- .../java/com/duckduckgo/app/tabs/model/TabDataRepository.kt | 2 +- .../main/java/com/duckduckgo/app/tabs/model/TabRepository.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 6bed8db48f45..7e1f085f1473 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 @@ -53,7 +53,7 @@ class TabDataRepository @Inject constructor( return tabId } - override suspend fun addFromSourceTab(url: String?, skipHome: Boolean, sourceTabId: String?): String { + override suspend fun addFromSourceTab(url: String?, skipHome: Boolean, sourceTabId: String): String { val tabId = generateTabId() add( diff --git a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt index 0e91cd9106bf..26a60acd9ad7 100644 --- a/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt +++ b/app/src/main/java/com/duckduckgo/app/tabs/model/TabRepository.kt @@ -33,7 +33,7 @@ interface TabRepository { suspend fun addDefaultTab(): String - suspend fun addFromSourceTab(url: String? = null, skipHome: Boolean = false, sourceTabId: String?): String + suspend fun addFromSourceTab(url: String? = null, skipHome: Boolean = false, sourceTabId: String): String suspend fun addNewTabAfterExistingTab(url: String? = null, tabId: String)