Commit b70bef44 authored by MozLando's avatar MozLando
Browse files

Merge #8293 

8293: Closes #8292: DownloadMiddleware uses context.dispatch on wrong thread r=Amejia481 a=csadilek

@Amejia481 when reviewing https://github.com/mozilla-mobile/android-components/issues/8271

 I missed that we're switching to an IO thread in `restoreDownloads`. My mistake.

We're not allowing a synchronous dispatch on the store from another thread. Undoing that and adding a fix for the intermittent test failure.

Co-authored-by: default avatarChristian Sadilek <christian.sadilek@gmail.com>
parents b85d7391 f94bdbf5

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

+1 −1
Original line number Diff line number Diff line
@@ -101,7 +101,7 @@ class DownloadMiddleware( 
        downloadStorage.getDownloads().collect { downloads ->

            downloads.forEach { download ->

                if (!context.state.downloads.containsKey(download.id)) {

                    context.dispatch(DownloadAction.RestoreDownloadStateAction(download))

                    context.store.dispatch(DownloadAction.RestoreDownloadStateAction(download))

                    logger.debug("Download restarted from the storage ${download.fileName}")

                }

            }

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

+35 −6
Original line number Diff line number Diff line
@@ -8,8 +8,14 @@ import android.app.DownloadManager.EXTRA_DOWNLOAD_ID 
import android.content.Context

import android.content.Intent

import androidx.test.ext.junit.runners.AndroidJUnit4

import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.Dispatchers

import kotlinx.coroutines.cancel

import kotlinx.coroutines.flow.flow

import kotlinx.coroutines.test.TestCoroutineDispatcher

import kotlinx.coroutines.test.resetMain

import kotlinx.coroutines.test.runBlockingTest

import kotlinx.coroutines.test.setMain

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

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

import mozilla.components.browser.state.state.content.DownloadState
@@ -21,8 +27,10 @@ import mozilla.components.support.test.argumentCaptor 
import mozilla.components.support.test.mock

import mozilla.components.support.test.whenever

import mozilla.components.support.test.ext.joinBlocking

import org.junit.After

import org.junit.Assert.assertEquals

import org.junit.Assert.assertTrue

import org.junit.Before

import org.junit.Test

import org.junit.runner.RunWith

import org.mockito.Mockito.verify
@@ -32,13 +40,32 @@ import org.mockito.Mockito.never 
@RunWith(AndroidJUnit4::class)

class DownloadMiddlewareTest {



    private lateinit var dispatcher: TestCoroutineDispatcher

    private lateinit var scope: CoroutineScope



    @Before

    fun setUp() {

        dispatcher = TestCoroutineDispatcher()

        scope = CoroutineScope(dispatcher)



        Dispatchers.setMain(dispatcher)

    }



    @After

    fun tearDown() {

        dispatcher.cleanupTestCoroutines()

        scope.cancel()



        Dispatchers.resetMain()

    }



    @Test

    fun `service is started when download is queued`() = runBlockingTest {

        val applicationContext: Context = mock()

        val downloadMiddleware = DownloadMiddleware(

            applicationContext,

            AbstractFetchDownloadService::class.java,

            coroutineContext = coroutineContext,

            coroutineContext = dispatcher,

            downloadStorage = mock()

        )

        val store = BrowserStore(
@@ -62,7 +89,7 @@ class DownloadMiddlewareTest { 
            applicationContext,

            AbstractFetchDownloadService::class.java,

            downloadStorage = downloadStorage,

            coroutineContext = coroutineContext

            coroutineContext = dispatcher

        )

        val store = BrowserStore(

                initialState = BrowserState(),
@@ -88,7 +115,7 @@ class DownloadMiddlewareTest { 
            applicationContext,

            AbstractFetchDownloadService::class.java,

            downloadStorage = downloadStorage,

            coroutineContext = coroutineContext

            coroutineContext = dispatcher

        )

        val store = BrowserStore(

            initialState = BrowserState(),
@@ -111,7 +138,7 @@ class DownloadMiddlewareTest { 
            applicationContext,

            AbstractFetchDownloadService::class.java,

            downloadStorage = downloadStorage,

            coroutineContext = coroutineContext

            coroutineContext = dispatcher

        )

        val store = BrowserStore(

            initialState = BrowserState(),
@@ -134,7 +161,7 @@ class DownloadMiddlewareTest { 
            applicationContext,

            AbstractFetchDownloadService::class.java,

            downloadStorage = downloadStorage,

            coroutineContext = coroutineContext

            coroutineContext = dispatcher

        )

        val store = BrowserStore(

            initialState = BrowserState(),
@@ -165,11 +192,12 @@ class DownloadMiddlewareTest { 
    fun `RestoreDownloadsState MUST populate the store with items in the storage`() = runBlockingTest {

        val applicationContext: Context = mock()

        val downloadStorage: DownloadStorage = mock()

        val dispatcher = TestCoroutineDispatcher()

        val downloadMiddleware = DownloadMiddleware(

            applicationContext,

            AbstractFetchDownloadService::class.java,

            downloadStorage = downloadStorage,

            coroutineContext = coroutineContext

            coroutineContext = dispatcher

        )

        val store = BrowserStore(

            initialState = BrowserState(),
@@ -186,6 +214,7 @@ class DownloadMiddlewareTest { 
        assertTrue(store.state.downloads.isEmpty())



        store.dispatch(DownloadAction.RestoreDownloadsStateAction).joinBlocking()

        dispatcher.advanceUntilIdle()



        assertEquals(download, store.state.downloads.values.first())

    }