Merge #7365

7365: For #7364 - Adds sessionId to DownloadState and populates it in DowloadFeature r=csadilek a=codrut-topliceanu Co-authored-by: codrut.topliceanu <[email protected]>
mozilla-mobile · Jun 16, 2020 · a261a24 · a261a24
2 parents d642612 + fc06a09
commit a261a24
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 22 deletions.
diff --git a/...owser/state/src/main/java/mozilla/components/browser/state/reducer/ContentStateReducer.kt b/...owser/state/src/main/java/mozilla/components/browser/state/reducer/ContentStateReducer.kt
@@ -62,7 +62,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/...owser/state/src/main/java/mozilla/components/browser/state/state/content/DownloadState.kt b/...owser/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/.../browser/state/src/test/java/mozilla/components/browser/state/action/ContentActionTest.kt b/.../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/.../feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt b/.../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/...ture/downloads/src/test/java/mozilla/components/feature/downloads/DownloadsFeatureTest.kt b/...ture/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()