Closes mozilla-mobile#7230: Add downloads to the system downloads dat…

…abase for devices older than Q
Amejia481 · Jun 3, 2020 · a27c2fd · a27c2fd
1 parent ba7a238
commit a27c2fd
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 23 deletions.
diff --git a/...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt b/...nloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt
@@ -299,12 +299,16 @@ abstract class AbstractFetchDownloadService : Service() {
      * Be aware that you need to pass [latestUIStatus] as [DownloadJobState.status] can be modified
      * from another thread, causing inconsistencies in the ui.
      */
-    private fun updateDownloadNotification(latestUIStatus: DownloadJobStatus, download: DownloadJobState) {
+    @VisibleForTesting
+    internal fun updateDownloadNotification(latestUIStatus: DownloadJobStatus, download: DownloadJobState) {
         val notification = when (latestUIStatus) {
             ACTIVE -> DownloadNotification.createOngoingDownloadNotification(context, download)
             PAUSED -> DownloadNotification.createPausedDownloadNotification(context, download)
-            COMPLETED -> DownloadNotification.createDownloadCompletedNotification(context, download)
             FAILED -> DownloadNotification.createDownloadFailedNotification(context, download)
+            COMPLETED -> {
+                addToDownloadSystemDatabaseCompact(download.state)
+                DownloadNotification.createDownloadCompletedNotification(context, download)
+            }
             CANCELLED -> {
                 removeNotification(context, download)
                 download.lastNotificationUpdate = System.currentTimeMillis()
@@ -360,6 +364,33 @@ abstract class AbstractFetchDownloadService : Service() {
         downloadedFile.delete()
     }
 
+    /**
+     * Adds a file to the downloads database system, so it could appear in Downloads App
+     * (and thus become eligible for management by the Downloads App) only for compatible devices
+     * otherwise nothing will happen.
+     */
+    @VisibleForTesting
+    internal fun addToDownloadSystemDatabaseCompact(download: DownloadState) {
+        if (SDK_INT <= Build.VERSION_CODES.P) {
+            val fileName = download.fileName
+                    ?: throw IllegalStateException("A fileName for a download is required")
+            val file = File(download.filePath)
+
+            context.addCompletedDownload(
+                    title = fileName,
+                    description = fileName,
+                    isMediaScannerScannable = true,
+                    mimeType = download.contentType ?: "*/*",
+                    path = file.absolutePath,
+                    length = download.contentLength ?: file.length(),
+                    // Only show notifications if our channel is blocked
+                    showNotification = !DownloadNotification.isChannelEnabled(context),
+                    uri = download.url.toUri(),
+                    referer = download.referrerUrl?.toUri()
+            )
+        }
+    }
+
     @VisibleForTesting
     internal fun registerNotificationActionsReceiver() {
         val filter = IntentFilter().apply {
@@ -670,27 +701,7 @@ abstract class AbstractFetchDownloadService : Service() {
     @TargetApi(Build.VERSION_CODES.P)
     @Suppress("Deprecation")
     private fun useFileStreamLegacy(download: DownloadState, append: Boolean, block: (OutputStream) -> Unit) {
-        val fileName = download.fileName ?: throw IllegalStateException("A fileName for a download is required")
-        val dir = Environment.getExternalStoragePublicDirectory(download.destinationDirectory)
-        val file = File(dir, fileName)
-
-        FileOutputStream(file, append).use(block)
-
-        val downloadJobState = downloadJobs[download.id] ?: return
-        if (getDownloadJobStatus(downloadJobState) != COMPLETED) { return }
-
-        addCompletedDownload(
-            title = fileName,
-            description = fileName,
-            isMediaScannerScannable = true,
-            mimeType = download.contentType ?: "*/*",
-            path = file.absolutePath,
-            length = download.contentLength ?: file.length(),
-            // Only show notifications if our channel is blocked
-            showNotification = !DownloadNotification.isChannelEnabled(context),
-            uri = download.url.toUri(),
-            referer = download.referrerUrl?.toUri()
-        )
+        FileOutputStream(File(download.filePath), append).use(block)
     }
 
     companion object {

diff --git a/...ds/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt b/...ds/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt
@@ -1058,6 +1058,27 @@ class AbstractFetchDownloadServiceTest {
         verify(service).unregisterNotificationActionsReceiver()
     }
 
+    @Test
+    @Config(sdk = [Build.VERSION_CODES.P])
+    fun `WHEN a download is completed the file MUST be added to the download system database`() {
+        val download = DownloadState(
+                url = "http://www.mozilla.org",
+                fileName = "example.apk",
+                destinationDirectory = folder.root.path
+        )
+        val service = spy(object : AbstractFetchDownloadService() {
+            override val httpClient = client
+            override val store = browserStore
+        })
+
+        val downloadJobState = DownloadJobState(state = download, status = COMPLETED)
+
+        doReturn(testContext).`when`(service).context
+        service.updateDownloadNotification(COMPLETED, downloadJobState)
+
+        verify(service).addToDownloadSystemDatabaseCompact(any())
+    }
+
     @Test
     fun `cancelled download does not prevent other notifications`() = runBlocking {
         val cancelledDownload = DownloadState("https://example.com/file.txt", "file.txt")

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -15,6 +15,9 @@ permalink: /changelog/
 * **browser-icons**
   * Fixed issue [#7142](https://github.com/mozilla-mobile/android-components/issues/7142)
 
+* **feature-downloads**
+  * On devices older than Q we were not adding downloads to the system download database for more information see [#7230](https://github.com/mozilla-mobile/android-components/issues/7230)
+
 # 44.0.0
 
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v43.0.0...v44.0.0)