From fc06a09674feaf652d7a2932bdfcdd512aa1097a Mon Sep 17 00:00:00 2001 From: "codrut.topliceanu" Date: Fri, 12 Jun 2020 17:29:31 +0300 Subject: [PATCH] For #7364 - Adds sessionId to DownloadState and populates it in ContentStateReducer --- .../state/reducer/ContentStateReducer.kt | 2 +- .../state/state/content/DownloadState.kt | 5 +++- .../browser/state/action/ContentActionTest.kt | 27 ++++++++++++------- .../feature/downloads/DownloadsFeature.kt | 5 ++-- .../feature/downloads/DownloadsFeatureTest.kt | 25 +++++++++++------ 5 files changed, 42 insertions(+), 22 deletions(-) diff --git a/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/ContentStateReducer.kt b/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/ContentStateReducer.kt index e3eff2d73bd..5f9a4b8779f 100644 --- a/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/ContentStateReducer.kt +++ b/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/ContentStateReducer.kt @@ -61,7 +61,7 @@ internal object ContentStateReducer { it.copy(thumbnail = action.thumbnail) } is ContentAction.UpdateDownloadAction -> updateContentState(state, action.sessionId) { - it.copy(download = action.download) + it.copy(download = action.download.copy(sessionId = action.sessionId)) } is ContentAction.ConsumeDownloadAction -> updateContentState(state, action.sessionId) { if (it.download != null && it.download.id == action.downloadId) { diff --git a/components/browser/state/src/main/java/mozilla/components/browser/state/state/content/DownloadState.kt b/components/browser/state/src/main/java/mozilla/components/browser/state/state/content/DownloadState.kt index 4f2331b43bf..b324963673d 100644 --- a/components/browser/state/src/main/java/mozilla/components/browser/state/state/content/DownloadState.kt +++ b/components/browser/state/src/main/java/mozilla/components/browser/state/state/content/DownloadState.kt @@ -22,6 +22,8 @@ import kotlin.random.Random * @property referrerUrl The site that linked to this download. * @property skipConfirmation Whether or not the confirmation dialog should be shown before the download begins. * @property id The unique identifier of this download. + * @property sessionId Identifier of the session that spawned the download. + * @ */ @Suppress("Deprecation") @Parcelize @@ -34,7 +36,8 @@ data class DownloadState( val destinationDirectory: String = Environment.DIRECTORY_DOWNLOADS, val referrerUrl: String? = null, val skipConfirmation: Boolean = false, - val id: Long = Random.nextLong() + val id: Long = Random.nextLong(), + val sessionId: String? = null ) : Parcelable { val filePath: String get() = Environment.getExternalStoragePublicDirectory(destinationDirectory).path + "/" + fileName diff --git a/components/browser/state/src/test/java/mozilla/components/browser/state/action/ContentActionTest.kt b/components/browser/state/src/test/java/mozilla/components/browser/state/action/ContentActionTest.kt index 44fde5238be..5923c46d5a9 100644 --- a/components/browser/state/src/test/java/mozilla/components/browser/state/action/ContentActionTest.kt +++ b/components/browser/state/src/test/java/mozilla/components/browser/state/action/ContentActionTest.kt @@ -32,7 +32,6 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith -import org.mockito.Mockito.doReturn import org.mockito.Mockito.spy @RunWith(AndroidJUnit4::class) @@ -324,27 +323,35 @@ class ContentActionTest { fun `UpdateDownloadAction updates download`() { assertNull(tab.content.download) - val download1: DownloadState = mock() + val download1 = DownloadState( + url = "https://www.mozilla.org", sessionId = tab.id + ) store.dispatch( ContentAction.UpdateDownloadAction(tab.id, download1) ).joinBlocking() - assertEquals(download1, tab.content.download) + assertEquals(download1.url, tab.content.download?.url) + assertEquals(download1.sessionId, tab.content.download?.sessionId) - val download2: DownloadState = mock() + val download2 = DownloadState( + url = "https://www.wikipedia.org", sessionId = tab.id + ) store.dispatch( ContentAction.UpdateDownloadAction(tab.id, download2) ).joinBlocking() - assertEquals(download2, tab.content.download) + assertEquals(download2.url, tab.content.download?.url) + assertEquals(download2.sessionId, tab.content.download?.sessionId) } @Test fun `ConsumeDownloadAction removes download`() { - val download: DownloadState = mock() - doReturn(1337L).`when`(download).id + val download = DownloadState( + id = 1337L, + url = "https://www.mozilla.org", sessionId = tab.id + ) store.dispatch( ContentAction.UpdateDownloadAction(tab.id, download) @@ -361,8 +368,10 @@ class ContentActionTest { @Test fun `ConsumeDownloadAction does not remove download with different id`() { - val download: DownloadState = mock() - doReturn(1337L).`when`(download).id + val download = DownloadState( + id = 1337L, + url = "https://www.mozilla.org", sessionId = tab.id + ) store.dispatch( ContentAction.UpdateDownloadAction(tab.id, download) diff --git a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt index e934bf534b7..9109837bbb2 100644 --- a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt +++ b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt @@ -93,9 +93,8 @@ class DownloadsFeature( flow.mapNotNull { state -> state.findTabOrCustomTabOrSelectedTab(tabId) } .ifChanged { it.content.download } .collect { state -> - val download = state.content.download - if (download != null) { - processDownload(state, download) + state.content.download?.let { downloadState -> + processDownload(state, downloadState) } } } diff --git a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt index 9871c0c6999..e2f25a0eecc 100644 --- a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt +++ b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt @@ -76,6 +76,8 @@ class DownloadsFeatureTest { fun `Adding a download object will request permissions if needed`() { val fragmentManager: FragmentManager = mock() + val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") + var requestedPermissions = false val feature = DownloadsFeature( @@ -90,7 +92,7 @@ class DownloadsFeatureTest { assertFalse(requestedPermissions) - store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download = mock())) + store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download)) .joinBlocking() testDispatcher.advanceUntilIdle() @@ -115,8 +117,9 @@ class DownloadsFeatureTest { feature.start() verify(fragmentManager, never()).beginTransaction() + val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") - store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download = mock())) + store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download)) .joinBlocking() testDispatcher.advanceUntilIdle() @@ -166,7 +169,7 @@ class DownloadsFeatureTest { verify(downloadManager, never()).download(any(), anyString()) - val download = DownloadState(url = "https://www.mozilla.org") + val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download)) .joinBlocking() @@ -200,7 +203,8 @@ class DownloadsFeatureTest { val download = DownloadState( url = "https://www.mozilla.org", - skipConfirmation = true + skipConfirmation = true, + sessionId = "test-tab" ) store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download)) @@ -218,7 +222,8 @@ class DownloadsFeatureTest { fun `When starting the feature will reattach to already existing dialog`() { grantPermissions() - store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download = mock())) + val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") + store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download)) .joinBlocking() val dialogFragment: DownloadDialogFragment = mock() @@ -249,7 +254,7 @@ class DownloadsFeatureTest { @Test fun `onPermissionsResult will start download if permissions were granted`() { - val download = DownloadState(url = "https://www.mozilla.org") + val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download = download)) .joinBlocking() @@ -273,7 +278,11 @@ class DownloadsFeatureTest { feature.onPermissionsResult( arrayOf(INTERNET, WRITE_EXTERNAL_STORAGE), - arrayOf(PackageManager.PERMISSION_GRANTED, PackageManager.PERMISSION_GRANTED).toIntArray()) + arrayOf( + PackageManager.PERMISSION_GRANTED, + PackageManager.PERMISSION_GRANTED + ).toIntArray() + ) verify(downloadManager).download(download) } @@ -364,7 +373,7 @@ class DownloadsFeatureTest { verify(downloadManager, never()).download(any(), anyString()) verify(feature, never()).showDownloadNotSupportedError() - val download = DownloadState(url = "https://www.mozilla.org") + val download = DownloadState(url = "https://www.mozilla.org", sessionId = "test-tab") store.dispatch(ContentAction.UpdateDownloadAction("test-tab", download)) .joinBlocking()