From ee804c9fa1e851e796133c796023658447077225 Mon Sep 17 00:00:00 2001 From: Sebastian Kaspari Date: Fri, 8 Jan 2021 17:00:57 +0100 Subject: [PATCH] Closes #4279: Migrate TabIntentProcessor to use TabsUseCases instead of SessionManager. --- components/feature/intent/build.gradle | 5 +- .../intent/processing/TabIntentProcessor.kt | 45 +++----- .../processing/TabIntentProcessorTest.kt | 105 ++---------------- .../components/feature/tabs/TabsUseCases.kt | 32 ++++++ .../feature/tabs/TabsUseCasesTest.kt | 42 +++++++ .../samples/browser/DefaultComponents.kt | 2 +- 6 files changed, 106 insertions(+), 125 deletions(-) diff --git a/components/feature/intent/build.gradle b/components/feature/intent/build.gradle index 4b19ce98304..34d87f89b54 100644 --- a/components/feature/intent/build.gradle +++ b/components/feature/intent/build.gradle @@ -25,14 +25,15 @@ android { dependencies { implementation project(':concept-engine') - implementation project(':browser-session') + implementation project(':browser-state') implementation project(':feature-search') implementation project(':feature-session') + implementation project(':feature-tabs') implementation project(':support-utils') implementation project(':support-ktx') implementation Dependencies.kotlin_stdlib - testImplementation project(':feature-tabs') + testImplementation project(":browser-session") testImplementation project(':browser-search') testImplementation project(':support-test') testImplementation Dependencies.androidx_browser diff --git a/components/feature/intent/src/main/java/mozilla/components/feature/intent/processing/TabIntentProcessor.kt b/components/feature/intent/src/main/java/mozilla/components/feature/intent/processing/TabIntentProcessor.kt index 22c0698a597..4a35cad6c26 100644 --- a/components/feature/intent/src/main/java/mozilla/components/feature/intent/processing/TabIntentProcessor.kt +++ b/components/feature/intent/src/main/java/mozilla/components/feature/intent/processing/TabIntentProcessor.kt @@ -12,12 +12,11 @@ import android.content.Intent.ACTION_SEARCH import android.content.Intent.ACTION_WEB_SEARCH import android.content.Intent.EXTRA_TEXT import android.nfc.NfcAdapter.ACTION_NDEF_DISCOVERED -import mozilla.components.browser.session.Session import mozilla.components.browser.state.state.SessionState.Source -import mozilla.components.browser.session.SessionManager import mozilla.components.concept.engine.EngineSession.LoadUrlFlags import mozilla.components.feature.search.SearchUseCases import mozilla.components.feature.session.SessionUseCases +import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.support.ktx.kotlin.isUrl import mozilla.components.support.utils.SafeIntent import mozilla.components.support.utils.WebURLFinder @@ -25,19 +24,16 @@ import mozilla.components.support.utils.WebURLFinder /** * Processor for intents which should trigger session-related actions. * - * @property sessionManager The application's [SessionManager]. + * @property tabsUseCases An instance of [TabsUseCases] used to open new tabs. * @property loadUrlUseCase A reference to [SessionUseCases.DefaultLoadUrlUseCase] used to load URLs. * @property newTabSearchUseCase A reference to [SearchUseCases.NewTabSearchUseCase] to be used for * ACTION_SEND intents if the provided text is not a URL. - * @property openNewTab Whether a processed intent should open a new tab or - * open URLs in the currently selected tab. * @property isPrivate Whether a processed intent should open a new tab as private */ class TabIntentProcessor( - private val sessionManager: SessionManager, + private val tabsUseCases: TabsUseCases, private val loadUrlUseCase: SessionUseCases.DefaultLoadUrlUseCase, private val newTabSearchUseCase: SearchUseCases.NewTabSearchUseCase, - private val openNewTab: Boolean = true, private val isPrivate: Boolean = false ) : IntentProcessor { @@ -50,17 +46,12 @@ class TabIntentProcessor( return if (url.isNullOrEmpty()) { false } else { - val existingSession = sessionManager.sessions.find { it.url == url } - if (existingSession != null) { - sessionManager.select(existingSession) - } else { - val session = createSession(url, private = isPrivate, source = Source.ACTION_VIEW) - loadUrlUseCase( - url, - session.id, - LoadUrlFlags.external() - ) - } + tabsUseCases.selectOrAddTab( + url, + private = isPrivate, + source = Source.ACTION_VIEW, + flags = LoadUrlFlags.external() + ) true } } @@ -77,10 +68,9 @@ class TabIntentProcessor( } else { val url = WebURLFinder(extraText).bestWebURL() if (url != null) { - val session = createSession(url, private = isPrivate, source = Source.ACTION_SEND) - loadUrlUseCase(url, session.id, LoadUrlFlags.external()) + addNewTab(url, Source.ACTION_SEND) } else { - newTabSearchUseCase(extraText, Source.ACTION_SEND, openNewTab) + newTabSearchUseCase(extraText, Source.ACTION_SEND) } true } @@ -93,20 +83,19 @@ class TabIntentProcessor( false } else { if (searchQuery.isUrl()) { - val session = createSession(searchQuery, private = isPrivate, source = Source.ACTION_SEARCH) - loadUrlUseCase(searchQuery, session.id, LoadUrlFlags.external()) + addNewTab(searchQuery, Source.ACTION_SEARCH) } else { - newTabSearchUseCase(searchQuery, Source.ACTION_SEARCH, openNewTab) + newTabSearchUseCase(searchQuery, Source.ACTION_SEARCH) } true } } - private fun createSession(url: String, private: Boolean = false, source: Source): Session { - return if (openNewTab) { - Session(url, private, source).also { sessionManager.add(it, selected = true) } + private fun addNewTab(url: String, source: Source) { + if (isPrivate) { + tabsUseCases.addPrivateTab(url, source = source, flags = LoadUrlFlags.external()) } else { - sessionManager.selectedSession ?: Session(url, private, source) + tabsUseCases.addTab(url, source = source, flags = LoadUrlFlags.external()) } } diff --git a/components/feature/intent/src/test/java/mozilla/components/feature/intent/processing/TabIntentProcessorTest.kt b/components/feature/intent/src/test/java/mozilla/components/feature/intent/processing/TabIntentProcessorTest.kt index c2119ad9377..c29b17c3c19 100644 --- a/components/feature/intent/src/test/java/mozilla/components/feature/intent/processing/TabIntentProcessorTest.kt +++ b/components/feature/intent/src/test/java/mozilla/components/feature/intent/processing/TabIntentProcessorTest.kt @@ -70,7 +70,7 @@ class TabIntentProcessorTest { val sessionManager = spy(SessionManager(engine)) val useCases = SessionUseCases(store, sessionManager) val handler = - TabIntentProcessor(sessionManager, useCases.loadUrl, searchUseCases.newTabSearch) + TabIntentProcessor(TabsUseCases(store, sessionManager), useCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_VIEW) @@ -102,55 +102,13 @@ class TabIntentProcessorTest { assertEquals(Source.ACTION_VIEW, session.source) } - @Test - fun processViewIntentUsingSelectedSession() { - val engine = mock() - val sessionManager = spy(SessionManager(engine)) - val session = Session("http://mozilla.org") - val handler = TabIntentProcessor( - sessionManager, - sessionUseCases.loadUrl, - searchUseCases.newTabSearch, - openNewTab = false - ) - - val intent = mock() - whenever(intent.action).thenReturn(Intent.ACTION_VIEW) - whenever(intent.dataString).thenReturn("http://mozilla.org") - sessionManager.add(session) - - handler.process(intent) - verify(sessionManager).select(session) - verify(store, never()).dispatch(any()) - } - - @Test - fun processViewIntentHavingNoSelectedSession() { - whenever(sessionManager.selectedSession).thenReturn(null) - - val handler = TabIntentProcessor( - sessionManager, - sessionUseCases.loadUrl, - searchUseCases.newTabSearch, - openNewTab = false - ) - val intent = mock() - whenever(intent.action).thenReturn(Intent.ACTION_VIEW) - whenever(intent.dataString).thenReturn("http://mozilla.org") - - handler.process(intent) - val actionCaptor = argumentCaptor() - verify(store).dispatch(actionCaptor.capture()) - assertEquals("http://mozilla.org", actionCaptor.value.url) - } - @Test fun processNfcIntent() { val engine = mock() val sessionManager = spy(SessionManager(engine)) val useCases = SessionUseCases(store, sessionManager) val handler = - TabIntentProcessor(sessionManager, useCases.loadUrl, searchUseCases.newTabSearch) + TabIntentProcessor(TabsUseCases(store, sessionManager), useCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(ACTION_NDEF_DISCOVERED) @@ -174,50 +132,9 @@ class TabIntentProcessorTest { assertEquals(Source.ACTION_VIEW, session.source) } - @Test - fun processNfcIntentUsingSelectedSession() { - val engine = mock() - val sessionManager = spy(SessionManager(engine)) - val session = Session("http://mozilla.org") - val handler = TabIntentProcessor( - sessionManager, - sessionUseCases.loadUrl, - searchUseCases.newTabSearch, - openNewTab = false - ) - val intent = mock() - whenever(intent.action).thenReturn(ACTION_NDEF_DISCOVERED) - whenever(intent.dataString).thenReturn("http://mozilla.org") - sessionManager.add(session) - - handler.process(intent) - verify(sessionManager).select(session) - verify(store, never()).dispatch(any()) - } - - @Test - fun processNfcIntentHavingNoSelectedSession() { - whenever(sessionManager.selectedSession).thenReturn(null) - - val handler = TabIntentProcessor( - sessionManager, - sessionUseCases.loadUrl, - searchUseCases.newTabSearch, - openNewTab = false - ) - val intent = mock() - whenever(intent.action).thenReturn(ACTION_NDEF_DISCOVERED) - whenever(intent.dataString).thenReturn("http://mozilla.org") - - handler.process(intent) - val actionCaptor = argumentCaptor() - verify(store).dispatch(actionCaptor.capture()) - assertEquals("http://mozilla.org", actionCaptor.value.url) - } - @Test fun `load URL on ACTION_SEND if text contains URL`() { - val handler = TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store, sessionManager), sessionUseCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEND) @@ -274,7 +191,7 @@ class TabIntentProcessorTest { val searchTerms = "mozilla android" val searchUrl = "http://search-url.com?$searchTerms" - val handler = TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store, sessionManager), sessionUseCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEND) @@ -299,7 +216,7 @@ class TabIntentProcessorTest { @Test fun `processor handles ACTION_SEND with empty text`() { - val handler = TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store, sessionManager), sessionUseCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEND) @@ -311,7 +228,7 @@ class TabIntentProcessorTest { @Test fun `processor handles ACTION_SEARCH with empty text`() { - val handler = TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store, sessionManager), sessionUseCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEARCH) @@ -323,7 +240,7 @@ class TabIntentProcessorTest { @Test fun `load URL on ACTION_SEARCH if text is an URL`() { - val handler = TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store, sessionManager), sessionUseCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEARCH) @@ -352,7 +269,7 @@ class TabIntentProcessorTest { val searchTerms = "mozilla android" val searchUrl = "http://search-url.com?$searchTerms" - val handler = TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store, sessionManager), sessionUseCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_SEARCH) @@ -377,7 +294,7 @@ class TabIntentProcessorTest { @Test fun `processor handles ACTION_WEB_SEARCH with empty text`() { - val handler = TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store, sessionManager), sessionUseCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_WEB_SEARCH) @@ -389,7 +306,7 @@ class TabIntentProcessorTest { @Test fun `load URL on ACTION_WEB_SEARCH if text is an URL`() { - val handler = TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store, sessionManager), sessionUseCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_WEB_SEARCH) @@ -418,7 +335,7 @@ class TabIntentProcessorTest { val searchTerms = "mozilla android" val searchUrl = "http://search-url.com?$searchTerms" - val handler = TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + val handler = TabIntentProcessor(TabsUseCases(store, sessionManager), sessionUseCases.loadUrl, searchUseCases.newTabSearch) val intent = mock() whenever(intent.action).thenReturn(Intent.ACTION_WEB_SEARCH) diff --git a/components/feature/tabs/src/main/java/mozilla/components/feature/tabs/TabsUseCases.kt b/components/feature/tabs/src/main/java/mozilla/components/feature/tabs/TabsUseCases.kt index 2e1dec3b117..766d37ccda7 100644 --- a/components/feature/tabs/src/main/java/mozilla/components/feature/tabs/TabsUseCases.kt +++ b/components/feature/tabs/src/main/java/mozilla/components/feature/tabs/TabsUseCases.kt @@ -360,6 +360,37 @@ class TabsUseCases( } } + /** + * Use case for selecting an existing tab or creating a new tab with a specific URL. + */ + class SelectOrAddUseCase( + private val store: BrowserStore, + private val sessionManager: SessionManager + ) { + /** + * Selects an already existing tab displaying [url] or otherwise creates a new tab. + */ + operator fun invoke( + url: String, + private: Boolean = false, + source: Source = Source.NEW_TAB, + flags: LoadUrlFlags = LoadUrlFlags.none() + ) { + val existingSession = sessionManager.sessions.find { it.url == url } + if (existingSession != null) { + sessionManager.select(existingSession) + } else { + val session = Session(url, private, source) + sessionManager.add(session, selected = true) + store.dispatch(EngineAction.LoadUrlAction( + session.id, + url, + flags + )) + } + } + } + val selectTab: SelectTabUseCase by lazy { DefaultSelectTabUseCase(sessionManager) } val removeTab: RemoveTabUseCase by lazy { DefaultRemoveTabUseCase(sessionManager) } val addTab: AddNewTabUseCase by lazy { AddNewTabUseCase(store, sessionManager) } @@ -370,4 +401,5 @@ class TabsUseCases( val removePrivateTabs: RemovePrivateTabsUseCase by lazy { RemovePrivateTabsUseCase(sessionManager) } val undo by lazy { UndoTabRemovalUseCase(store) } val restore: RestoreUseCase by lazy { RestoreUseCase(store, sessionManager, selectTab) } + val selectOrAddTab: SelectOrAddUseCase by lazy { SelectOrAddUseCase(store, sessionManager) } } diff --git a/components/feature/tabs/src/test/java/mozilla/components/feature/tabs/TabsUseCasesTest.kt b/components/feature/tabs/src/test/java/mozilla/components/feature/tabs/TabsUseCasesTest.kt index 9abc469f265..2e4069857b9 100644 --- a/components/feature/tabs/src/test/java/mozilla/components/feature/tabs/TabsUseCasesTest.kt +++ b/components/feature/tabs/src/test/java/mozilla/components/feature/tabs/TabsUseCasesTest.kt @@ -10,6 +10,7 @@ import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager import mozilla.components.browser.session.storage.SessionStorage import mozilla.components.browser.state.action.EngineAction +import mozilla.components.browser.state.selector.selectedTab import mozilla.components.browser.state.state.SessionState.Source import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.state.recover.RecoverableTab @@ -22,6 +23,7 @@ import mozilla.components.support.test.mock import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test @@ -337,4 +339,44 @@ class TabsUseCasesTest { assertEquals("wikipedia", sessionManager.selectedSessionOrThrow.id) assertEquals("wikipedia", store.state.selectedTabId) } + + @Test + fun `selectOrAddTab selects already existing tab`() { + val store = BrowserStore() + val sessionManager = SessionManager(engine = mock(), store = store) + val useCases = TabsUseCases(store, sessionManager) + + sessionManager.add(Session("https://www.mozilla.org", id = "mozilla")) + sessionManager.add(Session("https://firefox.com", id = "firefox")) + sessionManager.add(Session("https://getpocket.com", id = "pocket")) + + assertEquals("mozilla", store.state.selectedTabId) + assertEquals(3, store.state.tabs.size) + + useCases.selectOrAddTab("https://getpocket.com") + + assertEquals("pocket", store.state.selectedTabId) + assertEquals(3, store.state.tabs.size) + } + + @Test + fun `selectOrAddTab adds new tab if no matching existing tab could be found`() { + val store = BrowserStore() + val sessionManager = SessionManager(engine = mock(), store = store) + val useCases = TabsUseCases(store, sessionManager) + + sessionManager.add(Session("https://www.mozilla.org", id = "mozilla")) + sessionManager.add(Session("https://firefox.com", id = "firefox")) + sessionManager.add(Session("https://getpocket.com", id = "pocket")) + + assertEquals("mozilla", store.state.selectedTabId) + assertEquals(3, store.state.tabs.size) + + useCases.selectOrAddTab("https://youtube.com") + + assertNotEquals("mozilla", store.state.selectedTabId) + assertEquals(4, store.state.tabs.size) + assertEquals("https://youtube.com", store.state.tabs.last().content.url) + assertEquals("https://youtube.com", store.state.selectedTab!!.content.url) + } } diff --git a/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt b/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt index 684644b0c8a..fa6fe4f0e7a 100644 --- a/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt +++ b/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt @@ -235,7 +235,7 @@ open class DefaultComponents(private val applicationContext: Context) { // Intent val tabIntentProcessor by lazy { - TabIntentProcessor(sessionManager, sessionUseCases.loadUrl, searchUseCases.newTabSearch) + TabIntentProcessor(tabsUseCases, sessionUseCases.loadUrl, searchUseCases.newTabSearch) } val externalAppIntentProcessors by lazy { listOf(