Closes issue mozilla-mobile#8456: Don't process add request

for downloads already added
Amejia481 · Sep 21, 2020 · c05d3db · c05d3db
1 parent 6d759bf
commit c05d3db
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 11 deletions.
diff --git a/.buildconfig.yml b/.buildconfig.yml
@@ -1,4 +1,4 @@
-componentsVersion: 60.0.0
+componentsVersion: 0.0.0
 projects:
   concept-awesomebar:
     path: components/concept/awesomebar

diff --git a/...eature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadMiddleware.kt b/...eature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadMiddleware.kt
@@ -55,21 +55,19 @@ class DownloadMiddleware(
             is DownloadAction.RemoveAllDownloadsAction -> removeDownloads()
             is DownloadAction.UpdateDownloadAction -> updateDownload(action.download, context)
             is DownloadAction.RestoreDownloadsStateAction -> restoreDownloads(context.store)
+            is DownloadAction.AddDownloadAction -> {
+                if (!saveDownload(context.store, action.download)) {
+                    // The download was already added before, so we are ignoring this request.
+                    return
+                }
+            }
         }
 
         next(action)
 
         when (action) {
-            is DownloadAction.AddDownloadAction -> {
-                scope.launch {
-                    downloadStorage.add(action.download)
-                    logger.debug("Added download ${action.download.fileName} to the storage")
-                }
-                sendDownloadIntent(action.download)
-            }
-            is DownloadAction.RestoreDownloadStateAction -> {
-                sendDownloadIntent(action.download)
-            }
+            is DownloadAction.AddDownloadAction -> sendDownloadIntent(action.download)
+            is DownloadAction.RestoreDownloadStateAction -> sendDownloadIntent(action.download)
         }
     }
 
@@ -111,6 +109,18 @@ class DownloadMiddleware(
         }
     }
 
+    private fun saveDownload(store: Store<BrowserState, BrowserAction>, download: DownloadState): Boolean {
+        return if (!store.state.downloads.containsKey(download.id)) {
+            scope.launch {
+                downloadStorage.add(download)
+                logger.debug("Added download ${download.fileName} to the storage")
+            }
+            true
+        } else {
+            false
+        }
+    }
+
     @VisibleForTesting
     internal fun sendDownloadIntent(download: DownloadState) {
         if (download.status !in arrayOf(COMPLETED, CANCELLED, FAILED)) {

diff --git a/...re/downloads/src/test/java/mozilla/components/feature/downloads/DownloadMiddlewareTest.kt b/...re/downloads/src/test/java/mozilla/components/feature/downloads/DownloadMiddlewareTest.kt
@@ -112,6 +112,29 @@ class DownloadMiddlewareTest {
         verify(downloadStorage).add(download)
     }
 
+    @Test
+    fun `previously added downloads MUST be ignored`() = runBlockingTest {
+        val applicationContext: Context = mock()
+        val downloadStorage: DownloadStorage = mock()
+        val download = DownloadState("https://mozilla.org/download")
+        val downloadMiddleware = DownloadMiddleware(
+            applicationContext,
+            AbstractFetchDownloadService::class.java,
+            downloadStorage = downloadStorage,
+            coroutineContext = dispatcher
+        )
+        val store = BrowserStore(
+            initialState = BrowserState(
+                    downloads = mapOf(download.id to download)
+            ),
+            middleware = listOf(downloadMiddleware)
+        )
+
+        store.dispatch(DownloadAction.AddDownloadAction(download)).joinBlocking()
+
+        verify(downloadStorage, never()).add(download)
+    }
+
     @Test
     fun `RemoveDownloadAction MUST remove from the storage`() = runBlockingTest {
         val applicationContext: Context = mock()

diff --git a/docs/changelog.md b/docs/changelog.md
@@ -19,6 +19,8 @@ permalink: /changelog/
   * ⚠️ **This is a breaking change**: Removed `TabsUseCases.removeAllTabsOfType()`.
 * **browser-session**
   * Added `SessionManager.removeNormalSessions()` and `SessionManager.removePrivateSessions()`.
+* **feature-downloads**
+  * 🚒 Bug fixed [issue #8456](https://github.com/mozilla-mobile/android-components/issues/8456) Crash SQLiteConstraintException UNIQUE constraint failed: downloads.id (code 1555).
 
 # 59.0.0