Issue mozilla-mobile#1968 - Convert DownloadManager to interface

NotWoods · Jun 7, 2019 · d067516 · d067516
1 parent 6970229
commit d067516
Show file tree

Hide file tree

Showing 10 changed files with 72 additions and 35 deletions.
diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt
@@ -76,6 +76,7 @@ object Dependencies {
     const val androidx_cardview = "androidx.cardview:cardview:${Versions.AndroidX.cardview}"
     const val androidx_constraintlayout = "androidx.constraintlayout:constraintlayout:${Versions.AndroidX.constraintlayout}"
     const val androidx_core = "androidx.core:core:${Versions.AndroidX.core}"
+    const val androidx_core_ktx = "androidx.core:core-ktx:${Versions.AndroidX.core}"
     const val androidx_fragment = "androidx.fragment:fragment:${Versions.AndroidX.fragment}"
     const val androidx_lifecycle_extensions = "androidx.lifecycle:lifecycle-extensions:${Versions.AndroidX.lifecycle}"
     const val androidx_lifecycle_compiler = "androidx.lifecycle:lifecycle-compiler:${Versions.AndroidX.lifecycle}"

diff --git a/components/browser/session/build.gradle b/components/browser/session/build.gradle
@@ -23,6 +23,7 @@ android {
 
 dependencies {
     implementation project(':concept-engine')
+    implementation project(':concept-fetch')
     implementation project(':support-utils')
     implementation project(':support-ktx')
 

diff --git a/components/browser/session/src/main/java/mozilla/components/browser/session/Download.kt b/components/browser/session/src/main/java/mozilla/components/browser/session/Download.kt
@@ -5,6 +5,7 @@
 package mozilla.components.browser.session
 
 import android.os.Environment
+import mozilla.components.concept.fetch.Response
 
 /**
  * Value type that represents a Download.
@@ -22,5 +23,6 @@ data class Download(
     val contentType: String? = null,
     val contentLength: Long? = null,
     val userAgent: String? = null,
-    val destinationDirectory: String = Environment.DIRECTORY_DOWNLOADS
+    val destinationDirectory: String = Environment.DIRECTORY_DOWNLOADS,
+    val response: Response? = null
 )
diff --git a/components/concept/fetch/src/main/java/mozilla/components/concept/fetch/Response.kt b/components/concept/fetch/src/main/java/mozilla/components/concept/fetch/Response.kt
@@ -4,14 +4,15 @@
 
 package mozilla.components.concept.fetch
 
+import mozilla.components.concept.fetch.Response.Body
 import java.io.BufferedReader
 import java.io.Closeable
 import java.io.IOException
 import java.io.InputStream
 import java.nio.charset.Charset
 
 /**
- * The [Response] data class represents a reponse to a [Request] send by a [Client].
+ * The [Response] data class represents a response to a [Request] send by a [Client].
  *
  * You can create a [Response] object using the constructor, but you are more likely to encounter a [Response] object
  * being returned as the result of calling [Client.fetch].

diff --git a/components/feature/downloads/build.gradle b/components/feature/downloads/build.gradle
@@ -25,8 +25,11 @@ android {
 dependencies {
 
     implementation project(':browser-session')
+    implementation project(':concept-fetch')
     implementation project(':support-ktx')
     implementation project(':support-utils')
+
+    implementation Dependencies.androidx_core_ktx
     implementation Dependencies.kotlin_stdlib
 
     testImplementation Dependencies.testing_junit

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
@@ -16,6 +16,9 @@ import mozilla.components.browser.session.SelectionAwareSessionObserver
 import mozilla.components.browser.session.Session
 import mozilla.components.browser.session.SessionManager
 import mozilla.components.feature.downloads.DownloadDialogFragment.Companion.FRAGMENT_TAG
+import mozilla.components.feature.downloads.manager.AndroidDownloadManager
+import mozilla.components.feature.downloads.manager.DownloadManager
+import mozilla.components.feature.downloads.manager.OnDownloadCompleted
 import mozilla.components.support.base.feature.LifecycleAwareFeature
 import mozilla.components.support.base.observer.Consumable
 import mozilla.components.support.ktx.android.content.isPermissionGranted
@@ -44,7 +47,7 @@ class DownloadsFeature(
     private val applicationContext: Context,
     var onNeedToRequestPermissions: OnNeedToRequestPermissions = { },
     var onDownloadCompleted: OnDownloadCompleted = { _, _ -> },
-    private val downloadManager: DownloadManager = DownloadManager(applicationContext, onDownloadCompleted),
+    private val downloadManager: DownloadManager = AndroidDownloadManager(applicationContext, onDownloadCompleted),
     sessionManager: SessionManager,
     private val sessionId: String? = null,
     private val fragmentManager: FragmentManager? = null,

diff --git a/...ents/feature/downloads/DownloadManager.kt → ...wnloads/manager/AndroidDownloadManager.kt b/...ents/feature/downloads/DownloadManager.kt → ...wnloads/manager/AndroidDownloadManager.kt
@@ -2,58 +2,61 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-package mozilla.components.feature.downloads
+package mozilla.components.feature.downloads.manager
 
 import android.Manifest.permission.INTERNET
 import android.Manifest.permission.WRITE_EXTERNAL_STORAGE
 import android.app.DownloadManager.ACTION_DOWNLOAD_COMPLETE
 import android.app.DownloadManager.EXTRA_DOWNLOAD_ID
-import android.app.DownloadManager.Request
 import android.app.DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED
 import android.content.BroadcastReceiver
 import android.content.Context
-import android.content.Context.DOWNLOAD_SERVICE
 import android.content.Intent
 import android.content.IntentFilter
 import android.net.Uri
+import android.util.LongSparseArray
 import android.widget.Toast
 import androidx.annotation.RequiresPermission
+import androidx.core.content.getSystemService
+import androidx.core.net.toUri
+import androidx.core.util.contains
+import androidx.core.util.isEmpty
+import androidx.core.util.set
 import mozilla.components.browser.session.Download
+import mozilla.components.feature.downloads.R
 import mozilla.components.support.ktx.android.content.appName
 import mozilla.components.support.ktx.android.content.isPermissionGranted
 import mozilla.components.support.utils.DownloadUtils
 
-typealias OnDownloadCompleted = (Download, Long) -> Unit
-typealias AndroidDownloadManager = android.app.DownloadManager
-
-internal const val FILE_NOT_SUPPORTED = -1L
+typealias SystemDownloadManager = android.app.DownloadManager
+typealias SystemRequest = android.app.DownloadManager.Request
 
 /**
  * Handles the interactions with the [AndroidDownloadManager].
  * @property onDownloadCompleted a callback to be notified when a download is completed.
  * @property applicationContext a reference to [Context] applicationContext.
  */
-class DownloadManager(
+class AndroidDownloadManager(
     private val applicationContext: Context,
-    var onDownloadCompleted: OnDownloadCompleted = { _, _ -> }
-) {
+    override var onDownloadCompleted: OnDownloadCompleted = { _, _ -> }
+) : DownloadManager {
 
-    private val queuedDownloads = HashMap<Long, Download>()
+    private val queuedDownloads = LongSparseArray<Download>()
     private var isSubscribedReceiver = false
-    private lateinit var androidDownloadManager: AndroidDownloadManager
+    private lateinit var androidDownloadManager: SystemDownloadManager
 
     /**
      * Schedule a download through the [AndroidDownloadManager].
      * @param download metadata related to the download.
-     * @param refererURL the url from where this download was referred.
+     * @param refererUrl the url from where this download was referred.
      * @param cookie any additional cookie to add as part of the download request.
      * @return the id reference of the scheduled download.
      */
     @RequiresPermission(allOf = [INTERNET, WRITE_EXTERNAL_STORAGE])
-    fun download(
+    override fun download(
         download: Download,
-        refererURL: String = "",
-        cookie: String = ""
+        refererUrl: String,
+        cookie: String
     ): Long {
 
         if (download.isSupportedProtocol()) {
@@ -68,11 +71,11 @@ class DownloadManager(
             throw SecurityException("You must be granted INTERNET and WRITE_EXTERNAL_STORAGE permissions")
         }
 
-        androidDownloadManager = applicationContext.getSystemService(DOWNLOAD_SERVICE) as AndroidDownloadManager
+        androidDownloadManager = applicationContext.getSystemService()!!
 
         val fileName = getFileName(download)
 
-        val request = Request(Uri.parse(download.url))
+        val request = SystemRequest(download.url.toUri())
             .setNotificationVisibility(VISIBILITY_VISIBLE_NOTIFY_COMPLETED)
 
         if (!download.contentType.isNullOrEmpty()) {
@@ -82,7 +85,7 @@ class DownloadManager(
         with(request) {
             addRequestHeaderSafely("User-Agent", download.userAgent)
             addRequestHeaderSafely("Cookie", cookie)
-            addRequestHeaderSafely("Referer", refererURL)
+            addRequestHeaderSafely("Referer", refererUrl)
         }
 
         request.setDestinationInExternalPublicDir(download.destinationDirectory, fileName)
@@ -100,7 +103,7 @@ class DownloadManager(
     /**
      * Remove all the listeners.
      */
-    fun unregisterListener() {
+    override fun unregisterListener() {
         if (isSubscribedReceiver) {
             applicationContext.unregisterReceiver(onDownloadComplete)
             isSubscribedReceiver = false
@@ -115,7 +118,7 @@ class DownloadManager(
     }
 
     private fun getFileName(download: Download): String? {
-        return if (!download.fileName.isEmpty()) {
+        return if (download.fileName.isNotEmpty()) {
             download.fileName
         } else {
             DownloadUtils.guessFileName(
@@ -141,7 +144,7 @@ class DownloadManager(
                 download?.let {
                     onDownloadCompleted.invoke(download, downloadID)
                 }
-                queuedDownloads -= (downloadID)
+                queuedDownloads.remove(downloadID)
 
                 if (queuedDownloads.isEmpty()) {
                     unregisterListener()
@@ -165,9 +168,7 @@ class DownloadManager(
     }
 }
 
-internal fun Request.addRequestHeaderSafely(name: String, value: String?) {
-    if (value.isNullOrEmpty()) {
-        return
-    }
+internal fun SystemRequest.addRequestHeaderSafely(name: String, value: String?) {
+    if (value.isNullOrEmpty()) return
     addRequestHeader(name, value)
 }
diff --git a/...e/downloads/src/main/java/mozilla/components/feature/downloads/manager/DownloadManager.kt b/...e/downloads/src/main/java/mozilla/components/feature/downloads/manager/DownloadManager.kt
@@ -0,0 +1,24 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.feature.downloads.manager
+
+import mozilla.components.browser.session.Download
+
+typealias OnDownloadCompleted = (Download, Long) -> Unit
+
+internal const val FILE_NOT_SUPPORTED = -1L
+
+interface DownloadManager {
+
+    var onDownloadCompleted: OnDownloadCompleted
+
+    fun download(
+        download: Download,
+        refererUrl: String = "",
+        cookie: String = ""
+    ): Long
+
+    fun unregisterListener()
+}
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
@@ -13,6 +13,7 @@ import mozilla.components.browser.session.Session
 import mozilla.components.browser.session.SessionManager
 import mozilla.components.concept.engine.Engine
 import mozilla.components.feature.downloads.DownloadDialogFragment.Companion.FRAGMENT_TAG
+import mozilla.components.feature.downloads.manager.DownloadManager
 import mozilla.components.support.base.observer.Consumable
 import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
@@ -319,4 +320,4 @@ class DownloadsFeatureTest {
     private fun grantPermissions() {
         grantPermission(INTERNET, WRITE_EXTERNAL_STORAGE)
     }
-}
+}
diff --git a/.../feature/downloads/DownloadManagerTest.kt → ...ads/manager/AndroidDownloadManagerTest.kt b/.../feature/downloads/DownloadManagerTest.kt → ...ads/manager/AndroidDownloadManagerTest.kt
@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-package mozilla.components.feature.downloads
+package mozilla.components.feature.downloads.manager
 
 import android.Manifest.permission.INTERNET
 import android.Manifest.permission.WRITE_EXTERNAL_STORAGE
@@ -24,7 +24,7 @@ import org.mockito.Mockito.verifyZeroInteractions
 import org.robolectric.RobolectricTestRunner
 
 @RunWith(RobolectricTestRunner::class)
-class DownloadManagerTest {
+class AndroidDownloadManagerTest {
 
     private lateinit var download: Download
     private lateinit var downloadManager: DownloadManager
@@ -36,7 +36,7 @@ class DownloadManagerTest {
             "", "application/zip", 5242880,
             "Mozilla/5.0 (Linux; Android 7.1.1) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Focus/8.0 Chrome/69.0.3497.100 Mobile Safari/537.36"
         )
-        downloadManager = DownloadManager(testContext)
+        downloadManager = AndroidDownloadManager(testContext)
     }
 
     @Test(expected = SecurityException::class)
@@ -86,7 +86,7 @@ class DownloadManagerTest {
         val id = downloadManager.download(
             downloadWithFileName,
             cookie = "yummy_cookie=choco",
-            refererURL = "https://www.mozilla.org"
+            refererUrl = "https://www.mozilla.org"
         )
 
         downloadManager.onDownloadCompleted = { _, _ -> downloadCompleted = true }
@@ -129,4 +129,4 @@ class DownloadManagerTest {
     private fun grantPermissions() {
         grantPermission(INTERNET, WRITE_EXTERNAL_STORAGE)
     }
-}
+}