Commit c0f92864 authored by Arturo Mejia's avatar Arturo Mejia
Browse files

Closes #7308: Update the store queuedDownloads when mutate 

the download state.
parent ebec5623

components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt

+5 −0
Original line number Diff line number Diff line
@@ -577,4 +577,9 @@ sealed class DownloadAction : BrowserAction() { 
     * Updates the [BrowserState] to remove all queued downloads.

     */

    object RemoveAllQueuedDownloadsAction : DownloadAction()



    /**

     * Updates the provided [download] on the [BrowserState].

     */

    data class UpdateQueuedDownloadAction(val download: DownloadState) : DownloadAction()

}

components/browser/state/src/main/java/mozilla/components/browser/state/reducer/DownloadStateReducer.kt

+7 −2
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@ package mozilla.components.browser.state.reducer 


import mozilla.components.browser.state.action.DownloadAction

import mozilla.components.browser.state.state.BrowserState

import mozilla.components.browser.state.state.content.DownloadState



internal object DownloadStateReducer {
@@ -14,8 +15,9 @@ internal object DownloadStateReducer { 
     */

    fun reduce(state: BrowserState, action: DownloadAction): BrowserState {

        return when (action) {

            is DownloadAction.QueueDownloadAction -> {

                state.copy(queuedDownloads = state.queuedDownloads + (action.download.id to action.download))

            is DownloadAction.QueueDownloadAction -> updateQueuedDownloads(state, action.download)

            is DownloadAction.UpdateQueuedDownloadAction -> {

                updateQueuedDownloads(state, action.download)

            }

            is DownloadAction.RemoveQueuedDownloadAction -> {

                state.copy(queuedDownloads = state.queuedDownloads - action.downloadId)
@@ -25,4 +27,7 @@ internal object DownloadStateReducer { 
            }

        }

    }



    private fun updateQueuedDownloads(state: BrowserState, download: DownloadState) =

            state.copy(queuedDownloads = state.queuedDownloads + (download.id to download))

}

components/browser/state/src/test/java/mozilla/components/browser/state/action/DownloadActionTest.kt

+18 −0
Original line number Diff line number Diff line
@@ -58,4 +58,22 @@ class DownloadActionTest { 
        store.dispatch(DownloadAction.RemoveAllQueuedDownloadsAction).joinBlocking()

        assertTrue(store.state.queuedDownloads.isEmpty())

    }



    @Test

    fun `UpdateQueuedDownloadAction updates the provided download`() {

        val store = BrowserStore(BrowserState())

        val download = DownloadState("https://mozilla.org/download1", destinationDirectory = "")

        val download2 = DownloadState("https://mozilla.org/download2", destinationDirectory = "")



        store.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking()

        store.dispatch(DownloadAction.QueueDownloadAction(download2)).joinBlocking()



        val updatedDownload = download.copy(fileName = "filename.txt")



        store.dispatch(DownloadAction.UpdateQueuedDownloadAction(updatedDownload)).joinBlocking()



        assertFalse(store.state.queuedDownloads.isEmpty())

        assertEquals(2, store.state.queuedDownloads.size)

        assertEquals(updatedDownload, store.state.queuedDownloads[updatedDownload.id])

    }

}

components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt

+10 −1
Original line number Diff line number Diff line
@@ -628,7 +628,7 @@ abstract class AbstractFetchDownloadService : Service() { 
        block: (OutputStream) -> Unit

    ) {

        val downloadWithUniqueFileName = makeUniqueFileNameIfNecessary(download, append)

        downloadJobs[download.id]?.state = downloadWithUniqueFileName

        updateDownloadState(downloadWithUniqueFileName)



        if (SDK_INT >= Build.VERSION_CODES.Q) {

            useFileStreamScopedStorage(downloadWithUniqueFileName, block)
@@ -637,6 +637,15 @@ abstract class AbstractFetchDownloadService : Service() { 
        }

    }



    /**

     * Updates the given [updatedDownload] in the store and in the [downloadJobs].

     */

    @VisibleForTesting

    internal fun updateDownloadState(updatedDownload: DownloadState) {

        downloadJobs[updatedDownload.id]?.state = updatedDownload

        store.dispatch(DownloadAction.UpdateQueuedDownloadAction(updatedDownload))

    }



    /**

     * Returns an updated [DownloadState] with a unique fileName if the file is not being appended

     */

components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt

+18 −0
Original line number Diff line number Diff line
@@ -962,6 +962,24 @@ class AbstractFetchDownloadServiceTest { 
        assertEquals(0, shadowNotificationService.size())

    }



    @Test

    fun `updateDownloadState must update the download state in the store and in the downloadJobs`() {

        val download = DownloadState("https://example.com/file.txt", "file1.txt")

        val downloadJob = DownloadJobState(state = mock(), status = ACTIVE)

        val mockStore = mock<BrowserStore>()

        val service = spy(object : AbstractFetchDownloadService() {

            override val httpClient = client

            override val store = mockStore

        })



        service.downloadJobs[download.id] = downloadJob



        service.updateDownloadState(download)



        assertEquals(download, service.downloadJobs[download.id]!!.state)

        verify(mockStore).dispatch(DownloadAction.UpdateQueuedDownloadAction(download))

    }



    @Test

    fun `onTaskRemoved cancels all notifications on the shadow notification manager`() = runBlocking {

        val download = DownloadState("https://example.com/file.txt", "file.txt")