From 5ff071dc60478167d7b6b79ae3217fcd70c57e8e Mon Sep 17 00:00:00 2001 From: Christian Sadilek Date: Tue, 26 May 2020 17:44:16 -0400 Subject: [PATCH] Closes #7103 #5217: DownloadService: Read initial download state from store --- .../browser/state/action/BrowserAction.kt | 20 +++ .../state/reducer/BrowserStateReducer.kt | 2 + .../state/reducer/DownloadStateReducer.kt | 28 ++++ .../browser/state/state/BrowserState.kt | 5 +- .../state/action/DownloadActionTest.kt | 60 +++++++++ .../downloads/AbstractFetchDownloadService.kt | 13 +- .../feature/downloads/DownloadMiddleware.kt | 39 ++++++ .../feature/downloads/ext/Intent.kt | 54 -------- .../downloads/manager/FetchDownloadManager.kt | 30 ++--- .../AbstractFetchDownloadServiceTest.kt | 123 +++++++++--------- .../downloads/DownloadMiddlewareTest.kt | 41 ++++++ .../manager/FetchDownloadManagerTest.kt | 81 +++++++----- docs/changelog.md | 34 +++++ .../samples/browser/BaseBrowserFragment.kt | 1 + .../samples/browser/DefaultComponents.kt | 3 + .../browser/downloads/DownloadService.kt | 2 + 16 files changed, 367 insertions(+), 169 deletions(-) create mode 100644 components/browser/state/src/main/java/mozilla/components/browser/state/reducer/DownloadStateReducer.kt create mode 100644 components/browser/state/src/test/java/mozilla/components/browser/state/action/DownloadActionTest.kt create mode 100644 components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadMiddleware.kt delete mode 100644 components/feature/downloads/src/main/java/mozilla/components/feature/downloads/ext/Intent.kt create mode 100644 components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadMiddlewareTest.kt diff --git a/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt b/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt index 3ec25e49167..b4fb47ef300 100644 --- a/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt +++ b/components/browser/state/src/main/java/mozilla/components/browser/state/action/BrowserAction.kt @@ -558,3 +558,23 @@ sealed class MediaAction : BrowserAction() { val aggregate: MediaState.Aggregate ) : MediaAction() } + +/** + * [BrowserAction] implementations related to updating the global download state. + */ +sealed class DownloadAction : BrowserAction() { + /** + * Updates the [BrowserState] to track the provided [download] as queued. + */ + data class QueueDownloadAction(val download: DownloadState) : DownloadAction() + + /** + * Updates the [BrowserState] to remove queued download with the provided [downloadId]. + */ + data class RemoveQueuedDownloadAction(val downloadId: Long) : DownloadAction() + + /** + * Updates the [BrowserState] to remove all queued downloads. + */ + object RemoveAllQueuedDownloadsAction : DownloadAction() +} diff --git a/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/BrowserStateReducer.kt b/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/BrowserStateReducer.kt index b4bea1405aa..1eaab8165c8 100644 --- a/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/BrowserStateReducer.kt +++ b/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/BrowserStateReducer.kt @@ -7,6 +7,7 @@ package mozilla.components.browser.state.reducer import mozilla.components.browser.state.action.BrowserAction import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.CustomTabListAction +import mozilla.components.browser.state.action.DownloadAction import mozilla.components.browser.state.action.EngineAction import mozilla.components.browser.state.action.MediaAction import mozilla.components.browser.state.action.ReaderAction @@ -39,6 +40,7 @@ internal object BrowserStateReducer { is TrackingProtectionAction -> TrackingProtectionStateReducer.reduce(state, action) is WebExtensionAction -> WebExtensionReducer.reduce(state, action) is MediaAction -> MediaReducer.reduce(state, action) + is DownloadAction -> DownloadStateReducer.reduce(state, action) } } } diff --git a/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/DownloadStateReducer.kt b/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/DownloadStateReducer.kt new file mode 100644 index 00000000000..af625861944 --- /dev/null +++ b/components/browser/state/src/main/java/mozilla/components/browser/state/reducer/DownloadStateReducer.kt @@ -0,0 +1,28 @@ +/* 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.browser.state.reducer + +import mozilla.components.browser.state.action.DownloadAction +import mozilla.components.browser.state.state.BrowserState + +internal object DownloadStateReducer { + + /** + * [DownloadAction] Reducer function for modifying [BrowserState.queuedDownloads]. + */ + 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.RemoveQueuedDownloadAction -> { + state.copy(queuedDownloads = state.queuedDownloads - action.downloadId) + } + is DownloadAction.RemoveAllQueuedDownloadsAction -> { + state.copy(queuedDownloads = emptyMap()) + } + } + } +} diff --git a/components/browser/state/src/main/java/mozilla/components/browser/state/state/BrowserState.kt b/components/browser/state/src/main/java/mozilla/components/browser/state/state/BrowserState.kt index 6f10e738393..11502156181 100644 --- a/components/browser/state/src/main/java/mozilla/components/browser/state/state/BrowserState.kt +++ b/components/browser/state/src/main/java/mozilla/components/browser/state/state/BrowserState.kt @@ -4,6 +4,7 @@ package mozilla.components.browser.state.state +import mozilla.components.browser.state.state.content.DownloadState import mozilla.components.lib.state.State /** @@ -16,11 +17,13 @@ import mozilla.components.lib.state.State * The extensions here represent the default values for all [BrowserState.extensions] and can * be overridden per [SessionState]. * @property media The state of all media elements and playback states for all tabs. + * @property queuedDownloads queued downloads ([DownloadState]s) mapped to their ID. */ data class BrowserState( val tabs: List = emptyList(), val selectedTabId: String? = null, val customTabs: List = emptyList(), val extensions: Map = emptyMap(), - val media: MediaState = MediaState() + val media: MediaState = MediaState(), + val queuedDownloads: Map = emptyMap() ) : State diff --git a/components/browser/state/src/test/java/mozilla/components/browser/state/action/DownloadActionTest.kt b/components/browser/state/src/test/java/mozilla/components/browser/state/action/DownloadActionTest.kt new file mode 100644 index 00000000000..de68a558bc0 --- /dev/null +++ b/components/browser/state/src/test/java/mozilla/components/browser/state/action/DownloadActionTest.kt @@ -0,0 +1,60 @@ +/* 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.browser.state.action + +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.support.test.ext.joinBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class DownloadActionTest { + + @Test + fun `QueueDownloadAction adds download`() { + val store = BrowserStore(BrowserState()) + + val download1 = DownloadState("https://mozilla.org/download1", destinationDirectory = "") + store.dispatch(DownloadAction.QueueDownloadAction(download1)).joinBlocking() + assertEquals(download1, store.state.queuedDownloads[download1.id]) + assertEquals(1, store.state.queuedDownloads.size) + + val download2 = DownloadState("https://mozilla.org/download2", destinationDirectory = "") + store.dispatch(DownloadAction.QueueDownloadAction(download2)).joinBlocking() + assertEquals(download2, store.state.queuedDownloads[download2.id]) + assertEquals(2, store.state.queuedDownloads.size) + } + + @Test + fun `RemoveQueuedDownloadAction removes download`() { + val store = BrowserStore(BrowserState()) + + val download = DownloadState("https://mozilla.org/download1", destinationDirectory = "") + store.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() + assertEquals(download, store.state.queuedDownloads[download.id]) + assertFalse(store.state.queuedDownloads.isEmpty()) + + store.dispatch(DownloadAction.RemoveQueuedDownloadAction(download.id)).joinBlocking() + assertTrue(store.state.queuedDownloads.isEmpty()) + } + + @Test + fun `RemoveAllQueuedDownloadsAction removes download`() { + val store = BrowserStore(BrowserState()) + + val download = DownloadState("https://mozilla.org/download1", destinationDirectory = "") + store.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() + val download2 = DownloadState("https://mozilla.org/download2", destinationDirectory = "") + store.dispatch(DownloadAction.QueueDownloadAction(download2)).joinBlocking() + assertFalse(store.state.queuedDownloads.isEmpty()) + assertEquals(2, store.state.queuedDownloads.size) + + store.dispatch(DownloadAction.RemoveAllQueuedDownloadsAction).joinBlocking() + assertTrue(store.state.queuedDownloads.isEmpty()) + } +} diff --git a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt index 924b7d0622b..610ee61192e 100644 --- a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt +++ b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt @@ -40,7 +40,9 @@ import kotlinx.coroutines.cancel import kotlinx.coroutines.delay import kotlinx.coroutines.isActive import kotlinx.coroutines.launch +import mozilla.components.browser.state.action.DownloadAction import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.fetch.Client import mozilla.components.concept.fetch.Headers.Names.CONTENT_RANGE import mozilla.components.concept.fetch.Headers.Names.RANGE @@ -53,7 +55,6 @@ import mozilla.components.feature.downloads.AbstractFetchDownloadService.Downloa import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.PAUSED import mozilla.components.feature.downloads.DownloadNotification.NOTIFICATION_DOWNLOAD_GROUP_ID import mozilla.components.feature.downloads.ext.addCompletedDownload -import mozilla.components.feature.downloads.ext.getDownloadExtra import mozilla.components.feature.downloads.ext.withResponse import mozilla.components.feature.downloads.facts.emitNotificationResumeFact import mozilla.components.feature.downloads.facts.emitNotificationPauseFact @@ -77,6 +78,7 @@ import kotlin.random.Random */ @Suppress("TooManyFunctions", "LargeClass") abstract class AbstractFetchDownloadService : Service() { + protected abstract val store: BrowserStore private val notificationUpdateScope = MainScope() @@ -90,6 +92,8 @@ abstract class AbstractFetchDownloadService : Service() { internal var downloadJobs = mutableMapOf() + // TODO Move this to browser store and make immutable: + // https://github.com/mozilla-mobile/android-components/issues/7050 internal data class DownloadJobState( var job: Job? = null, @Volatile var state: DownloadState, @@ -230,7 +234,9 @@ abstract class AbstractFetchDownloadService : Service() { } override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { - val download = intent?.getDownloadExtra() ?: return START_REDELIVER_INTENT + val download = intent?.getLongExtra(EXTRA_DOWNLOAD_ID, -1)?.let { + store.state.queuedDownloads[it] + } ?: return START_REDELIVER_INTENT // If the job already exists, then don't create a new ID. This can happen when calling tryAgain val foregroundServiceId = downloadJobs[download.id]?.foregroundServiceId ?: Random.nextInt() @@ -376,6 +382,7 @@ abstract class AbstractFetchDownloadService : Service() { @VisibleForTesting internal fun removeDownloadJob(downloadJobState: DownloadJobState) { downloadJobs.remove(downloadJobState.state.id) + store.dispatch(DownloadAction.RemoveQueuedDownloadAction(downloadJobState.state.id)) if (downloadJobs.isEmpty()) { stopSelf() } else { @@ -570,7 +577,6 @@ abstract class AbstractFetchDownloadService : Service() { val intent = Intent(ACTION_DOWNLOAD_COMPLETE) intent.putExtra(EXTRA_DOWNLOAD_STATUS, getDownloadJobStatus(downloadState)) - intent.putExtra(EXTRA_DOWNLOAD, downloadState.state) intent.putExtra(EXTRA_DOWNLOAD_ID, downloadState.state.id) broadcastManager.sendBroadcast(intent) @@ -725,7 +731,6 @@ abstract class AbstractFetchDownloadService : Service() { */ internal const val PROGRESS_UPDATE_INTERVAL = 750L - const val EXTRA_DOWNLOAD = "mozilla.components.feature.downloads.extras.DOWNLOAD" const val EXTRA_DOWNLOAD_STATUS = "mozilla.components.feature.downloads.extras.DOWNLOAD_STATUS" const val ACTION_OPEN = "mozilla.components.feature.downloads.OPEN" const val ACTION_PAUSE = "mozilla.components.feature.downloads.PAUSE" diff --git a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadMiddleware.kt b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadMiddleware.kt new file mode 100644 index 00000000000..9d503f587c4 --- /dev/null +++ b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadMiddleware.kt @@ -0,0 +1,39 @@ +/* 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 + +import android.app.DownloadManager +import android.content.Context +import android.content.Intent +import mozilla.components.browser.state.action.BrowserAction +import mozilla.components.browser.state.action.DownloadAction +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.lib.state.Middleware +import mozilla.components.lib.state.MiddlewareStore + +/** + * [Middleware] implementation for managing downloads in response to + * [BrowserState.queuedDownloads] changes. + */ +class DownloadMiddleware( + private val applicationContext: Context, + private val downloadServiceClass: Class<*> +) : Middleware { + + override fun invoke( + store: MiddlewareStore, + next: (BrowserAction) -> Unit, + action: BrowserAction + ) { + next(action) + when (action) { + is DownloadAction.QueueDownloadAction -> { + val intent = Intent(applicationContext, downloadServiceClass) + intent.putExtra(DownloadManager.EXTRA_DOWNLOAD_ID, action.download.id) + applicationContext.startService(intent) + } + } + } +} diff --git a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/ext/Intent.kt b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/ext/Intent.kt deleted file mode 100644 index 8133ed339e0..00000000000 --- a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/ext/Intent.kt +++ /dev/null @@ -1,54 +0,0 @@ -/* 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.ext - -import android.app.DownloadManager.EXTRA_DOWNLOAD_ID -import android.content.Intent -import androidx.core.os.bundleOf -import mozilla.components.browser.state.state.content.DownloadState -import mozilla.components.concept.fetch.Headers.Names.CONTENT_LENGTH -import mozilla.components.concept.fetch.Headers.Names.CONTENT_TYPE -import mozilla.components.concept.fetch.Headers.Names.REFERRER -import mozilla.components.concept.fetch.Headers.Names.USER_AGENT - -private const val INTENT_DOWNLOAD = "mozilla.components.feature.downloads.DOWNLOAD" -private const val INTENT_URL = "mozilla.components.feature.downloads.URL" -private const val INTENT_FILE_NAME = "mozilla.components.feature.downloads.FILE_NAME" -private const val INTENT_DESTINATION = "mozilla.components.feature.downloads.DESTINATION" - -fun Intent.putDownloadExtra(download: DownloadState) { - download.apply { - putExtra(INTENT_DOWNLOAD, bundleOf( - INTENT_URL to url, - INTENT_FILE_NAME to fileName, - CONTENT_TYPE to contentType, - CONTENT_LENGTH to contentLength, - USER_AGENT to userAgent, - INTENT_DESTINATION to destinationDirectory, - REFERRER to referrerUrl, - EXTRA_DOWNLOAD_ID to id - )) - } -} - -fun Intent.getDownloadExtra(): DownloadState? = - getBundleExtra(INTENT_DOWNLOAD)?.run { - val url = getString(INTENT_URL) - val fileName = getString(INTENT_FILE_NAME) - val destination = getString(INTENT_DESTINATION) - val id = getLong(EXTRA_DOWNLOAD_ID) - if (url == null || destination == null) return null - - DownloadState( - url = url, - fileName = fileName, - contentType = getString(CONTENT_TYPE), - contentLength = get(CONTENT_LENGTH) as? Long?, - userAgent = getString(USER_AGENT), - destinationDirectory = destination, - referrerUrl = getString(REFERRER), - id = id - ) - } diff --git a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/manager/FetchDownloadManager.kt b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/manager/FetchDownloadManager.kt index 11d464d21e0..e4bcdee0ee3 100644 --- a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/manager/FetchDownloadManager.kt +++ b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/manager/FetchDownloadManager.kt @@ -15,15 +15,13 @@ import android.content.Intent import android.content.IntentFilter import android.os.Build.VERSION.SDK_INT import android.os.Build.VERSION_CODES.P -import android.util.LongSparseArray -import androidx.core.util.set import androidx.localbroadcastmanager.content.LocalBroadcastManager +import mozilla.components.browser.state.action.DownloadAction import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.feature.downloads.AbstractFetchDownloadService -import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.EXTRA_DOWNLOAD import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.EXTRA_DOWNLOAD_STATUS import mozilla.components.feature.downloads.ext.isScheme -import mozilla.components.feature.downloads.ext.putDownloadExtra import kotlin.reflect.KClass /** @@ -34,12 +32,12 @@ import kotlin.reflect.KClass */ class FetchDownloadManager( private val applicationContext: Context, + private val store: BrowserStore, private val service: KClass, private val broadcastManager: LocalBroadcastManager = LocalBroadcastManager.getInstance(applicationContext), override var onDownloadStopped: onDownloadStopped = noop ) : BroadcastReceiver(), DownloadManager { - private val queuedDownloads = LongSparseArray() private var isSubscribedReceiver = false override val permissions = if (SDK_INT >= P) { @@ -58,25 +56,21 @@ class FetchDownloadManager( if (!download.isScheme(listOf("http", "https", "data", "blob"))) { return null } - validatePermissionGranted(applicationContext) - queuedDownloads[download.id] = download - - val intent = Intent(applicationContext, service.java) - intent.putDownloadExtra(download) - applicationContext.startService(intent) + // The middleware will notify the service to start the download + // once this action is processed. + store.dispatch(DownloadAction.QueueDownloadAction(download)) registerBroadcastReceiver() - return download.id } override fun tryAgain(downloadId: Long) { - val download = queuedDownloads[downloadId] ?: return + val download = store.state.queuedDownloads[downloadId] ?: return val intent = Intent(applicationContext, service.java) - intent.putDownloadExtra(download) + intent.putExtra(EXTRA_DOWNLOAD_ID, download.id) applicationContext.startService(intent) registerBroadcastReceiver() @@ -89,7 +83,7 @@ class FetchDownloadManager( if (isSubscribedReceiver) { broadcastManager.unregisterReceiver(this) isSubscribedReceiver = false - queuedDownloads.clear() + store.dispatch(DownloadAction.RemoveAllQueuedDownloadsAction) } } @@ -106,11 +100,15 @@ class FetchDownloadManager( * broadcast receiver if there are no more queued downloads. */ override fun onReceive(context: Context, intent: Intent) { - val download = intent.getParcelableExtra(EXTRA_DOWNLOAD) val downloadID = intent.getLongExtra(EXTRA_DOWNLOAD_ID, -1) val downloadStatus = intent.getSerializableExtra(EXTRA_DOWNLOAD_STATUS) as AbstractFetchDownloadService.DownloadJobStatus + if (downloadStatus == AbstractFetchDownloadService.DownloadJobStatus.COMPLETED) { + store.dispatch(DownloadAction.RemoveQueuedDownloadAction(downloadID)) + } + + val download = store.state.queuedDownloads[downloadID] if (download != null) { onDownloadStopped(download, downloadID, downloadStatus) } diff --git a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt index 424a0c2ab12..a7650d2e6c0 100644 --- a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt +++ b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt @@ -4,6 +4,7 @@ package mozilla.components.feature.downloads +import android.app.DownloadManager.EXTRA_DOWNLOAD_ID import android.app.Notification import android.app.NotificationManager import android.app.Service @@ -22,7 +23,9 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.test.TestCoroutineDispatcher import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.setMain +import mozilla.components.browser.state.action.DownloadAction import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.fetch.Client import mozilla.components.concept.fetch.MutableHeaders import mozilla.components.concept.fetch.Request @@ -39,12 +42,12 @@ import mozilla.components.feature.downloads.AbstractFetchDownloadService.Downloa import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.FAILED import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.PAUSED import mozilla.components.feature.downloads.DownloadNotification.NOTIFICATION_DOWNLOAD_GROUP_ID -import mozilla.components.feature.downloads.ext.putDownloadExtra import mozilla.components.feature.downloads.facts.DownloadsFacts.Items.NOTIFICATION import mozilla.components.support.base.facts.Action import mozilla.components.support.base.facts.processor.CollectionProcessor import mozilla.components.support.test.any import mozilla.components.support.test.argumentCaptor +import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext import org.junit.After @@ -83,6 +86,7 @@ class AbstractFetchDownloadServiceTest { val folder = TemporaryFolder() @Mock private lateinit var client: Client + private lateinit var browserStore: BrowserStore @Mock private lateinit var broadcastManager: LocalBroadcastManager private lateinit var service: AbstractFetchDownloadService @@ -93,8 +97,10 @@ class AbstractFetchDownloadServiceTest { fun setup() { Dispatchers.setMain(testDispatcher) initMocks(this) + browserStore = BrowserStore() service = spy(object : AbstractFetchDownloadService() { override val httpClient = client + override val store = browserStore }) doReturn(broadcastManager).`when`(service).broadcastManager @@ -121,10 +127,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -264,10 +270,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -303,10 +309,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -350,10 +356,10 @@ class AbstractFetchDownloadServiceTest { doReturn(resumeResponse).`when`(client) .fetch(Request("https://example.com/file.txt", headers = MutableHeaders("Range" to "bytes=1-"))) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -401,10 +407,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -452,10 +458,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -506,10 +512,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -531,12 +537,12 @@ class AbstractFetchDownloadServiceTest { fun `onStartCommand sets the notification foreground`() = runBlocking { val download = DownloadState("https://example.com/file.txt", "file.txt") - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) doNothing().`when`(service).performDownload(any()) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) verify(service).setForegroundNotification(any()) @@ -608,9 +614,8 @@ class AbstractFetchDownloadServiceTest { fun `getForegroundId in devices that support notification group will return NOTIFICATION_DOWNLOAD_GROUP_ID`() { val download = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt") - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) doNothing().`when`(service).performDownload(any()) @@ -624,12 +629,12 @@ class AbstractFetchDownloadServiceTest { fun `getForegroundId in devices that support DO NOT notification group will return the latest active download`() { val download = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt") - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) doNothing().`when`(service).performDownload(any()) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) val foregroundId = service.downloadJobs.values.first().foregroundServiceId @@ -769,10 +774,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -801,10 +806,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -832,10 +837,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -864,10 +869,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -890,10 +895,10 @@ class AbstractFetchDownloadServiceTest { doThrow(IOException()).`when`(client).fetch(any()) val download = DownloadState("https://example.com/file.txt", "file.txt") - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -923,10 +928,10 @@ class AbstractFetchDownloadServiceTest { doCallRealMethod().`when`(service).useFileStream(any(), anyBoolean(), any()) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.registerNotificationActionsReceiver() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { assertTrue(it.job!!.isActive) } @@ -963,10 +968,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.registerNotificationActionsReceiver() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -1022,6 +1027,7 @@ class AbstractFetchDownloadServiceTest { fun `onDestroy will remove all download notifications, jobs and will call unregisterNotificationActionsReceiver`() = runBlocking { val service = spy(object : AbstractFetchDownloadService() { override val httpClient = client + override val store = browserStore }) doReturn(testContext).`when`(service).context @@ -1038,6 +1044,7 @@ class AbstractFetchDownloadServiceTest { fun `register and unregister notification actions receiver`() { val service = spy(object : AbstractFetchDownloadService() { override val httpClient = client + override val store = browserStore }) doReturn(testContext).`when`(service).context @@ -1062,10 +1069,10 @@ class AbstractFetchDownloadServiceTest { ) doReturn(response).`when`(client).fetch(Request("https://example.com/file.txt")) - val cancelledDownloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(cancelledDownload) - } + val cancelledDownloadIntent = Intent("ACTION_DOWNLOAD") + cancelledDownloadIntent.putExtra(EXTRA_DOWNLOAD_ID, cancelledDownload.id) + browserStore.dispatch(DownloadAction.QueueDownloadAction(cancelledDownload)).joinBlocking() service.onStartCommand(cancelledDownloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } @@ -1083,11 +1090,11 @@ class AbstractFetchDownloadServiceTest { assertEquals(1, shadowNotificationService.size()) val download = DownloadState("https://example.com/file.txt", "file.txt") - val downloadIntent = Intent("ACTION_DOWNLOAD").apply { - putDownloadExtra(download) - } + val downloadIntent = Intent("ACTION_DOWNLOAD") + downloadIntent.putExtra(EXTRA_DOWNLOAD_ID, download.id) // Start another download to ensure its notifications are presented + browserStore.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } verify(service, times(2)).performDownload(providedDownload.capture()) diff --git a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadMiddlewareTest.kt b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadMiddlewareTest.kt new file mode 100644 index 00000000000..4588d7e0162 --- /dev/null +++ b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadMiddlewareTest.kt @@ -0,0 +1,41 @@ +/* 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 + +import android.app.DownloadManager.EXTRA_DOWNLOAD_ID +import android.content.Context +import android.content.Intent +import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.browser.state.action.DownloadAction +import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.store.BrowserStore +import mozilla.components.support.test.argumentCaptor +import mozilla.components.support.test.ext.joinBlocking +import mozilla.components.support.test.mock +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.verify + +@RunWith(AndroidJUnit4::class) +class DownloadMiddlewareTest { + + @Test + fun `service is started when download is queued`() { + val applicationContext: Context = mock() + val store = BrowserStore( + initialState = BrowserState(), + middleware = listOf(DownloadMiddleware(applicationContext, AbstractFetchDownloadService::class.java)) + ) + + val download = DownloadState("https://mozilla.org/download", destinationDirectory = "") + store.dispatch(DownloadAction.QueueDownloadAction(download)).joinBlocking() + + val intentCaptor = argumentCaptor() + verify(applicationContext).startService(intentCaptor.capture()) + assertEquals(download.id, intentCaptor.value.getLongExtra(EXTRA_DOWNLOAD_ID, -1)) + } +} diff --git a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/manager/FetchDownloadManagerTest.kt b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/manager/FetchDownloadManagerTest.kt index 0423877748c..bfca4f8e6e3 100644 --- a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/manager/FetchDownloadManagerTest.kt +++ b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/manager/FetchDownloadManagerTest.kt @@ -13,10 +13,11 @@ import android.content.Context import android.content.Intent import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.browser.state.action.DownloadAction import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.fetch.Client import mozilla.components.feature.downloads.AbstractFetchDownloadService -import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.EXTRA_DOWNLOAD import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.EXTRA_DOWNLOAD_STATUS import mozilla.components.support.test.any import mozilla.components.support.test.mock @@ -27,12 +28,13 @@ import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.verify import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus -import org.junit.Assert.assertFalse +import mozilla.components.support.test.ext.joinBlocking +import mozilla.components.support.test.libstate.ext.waitUntilIdle import org.junit.Assert.assertTrue import org.junit.Assert.assertNull import org.junit.Assert.assertNotNull import org.junit.Assert.assertEquals -import org.mockito.Mockito.times +import org.mockito.Mockito.never @RunWith(AndroidJUnit4::class) class FetchDownloadManagerTest { @@ -41,17 +43,19 @@ class FetchDownloadManagerTest { private lateinit var service: MockDownloadService private lateinit var download: DownloadState private lateinit var downloadManager: FetchDownloadManager + private lateinit var store: BrowserStore @Before fun setup() { broadcastManager = LocalBroadcastManager.getInstance(testContext) service = MockDownloadService() + store = BrowserStore() download = DownloadState( "http://ipv4.download.thinkbroadband.com/5MB.zip", "", "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 = FetchDownloadManager(testContext, MockDownloadService::class, broadcastManager) + downloadManager = FetchDownloadManager(testContext, store, MockDownloadService::class, broadcastManager) } @Test(expected = SecurityException::class) @@ -60,28 +64,28 @@ class FetchDownloadManagerTest { } @Test - fun `calling download must download the file`() { + fun `calling download must queue the download`() { val context: Context = mock() - downloadManager = FetchDownloadManager(context, MockDownloadService::class, broadcastManager) + downloadManager = FetchDownloadManager(context, store, MockDownloadService::class, broadcastManager) var downloadCompleted = false downloadManager.onDownloadStopped = { _, _, _ -> downloadCompleted = true } grantPermissions() + assertTrue(store.state.queuedDownloads.isEmpty()) val id = downloadManager.download(download)!! - - verify(context).startService(any()) + store.waitUntilIdle() + assertEquals(download, store.state.queuedDownloads[download.id]) notifyDownloadCompleted(id) - assertTrue(downloadCompleted) } @Test fun `calling tryAgain starts the download again`() { val context: Context = mock() - downloadManager = FetchDownloadManager(context, MockDownloadService::class, broadcastManager) + downloadManager = FetchDownloadManager(context, store, MockDownloadService::class, broadcastManager) var downloadCompleted = false downloadManager.onDownloadStopped = { _, _, _ -> downloadCompleted = true } @@ -89,16 +93,10 @@ class FetchDownloadManagerTest { grantPermissions() val id = downloadManager.download(download)!! - - verify(context).startService(any()) - notifyDownloadCompleted(id) - assertTrue(downloadCompleted) - - downloadCompleted = false + store.waitUntilIdle() downloadManager.tryAgain(id) - - verify(context, times(2)).startService(any()) + verify(context).startService(any()) notifyDownloadCompleted(id) assertTrue(downloadCompleted) } @@ -106,34 +104,21 @@ class FetchDownloadManagerTest { @Test fun `try again should not crash when download does not exist`() { val context: Context = mock() - downloadManager = FetchDownloadManager(context, MockDownloadService::class, broadcastManager) - var downloadCompleted = false - - downloadManager.onDownloadStopped = { _, _, _ -> downloadCompleted = true } + downloadManager = FetchDownloadManager(context, store, MockDownloadService::class, broadcastManager) grantPermissions() - val id = downloadManager.download(download)!! - verify(context).startService(any()) - notifyDownloadCompleted(id) - assertTrue(downloadCompleted) - - downloadCompleted = false downloadManager.tryAgain(id + 1) - assertFalse(downloadCompleted) - verify(context, times(1)).startService(any()) + verify(context, never()).startService(any()) } @Test fun `trying to download a file with invalid protocol must NOT triggered a download`() { - val invalidDownload = download.copy(url = "ftp://ipv4.download.thinkbroadband.com/5MB.zip") - grantPermissions() val id = downloadManager.download(invalidDownload) - assertNull(id) } @@ -171,6 +156,30 @@ class FetchDownloadManagerTest { assertEquals(DownloadJobStatus.COMPLETED, downloadStatus) } + @Test + fun `sendBroadcast with completed download removes queued download from store`() { + var downloadStatus: DownloadJobStatus? = null + val downloadWithFileName = download.copy(fileName = "5MB.zip") + store.dispatch(DownloadAction.QueueDownloadAction(downloadWithFileName)).joinBlocking() + assertEquals(downloadWithFileName, store.state.queuedDownloads[downloadWithFileName.id]) + + grantPermissions() + + downloadManager.onDownloadStopped = { _, _, status -> + downloadStatus = status + } + + val id = downloadManager.download( + downloadWithFileName, + cookie = "yummy_cookie=choco" + )!! + + notifyDownloadCompleted(id) + store.waitUntilIdle() + assertEquals(DownloadJobStatus.COMPLETED, downloadStatus) + assertTrue(store.state.queuedDownloads.isEmpty()) + } + @Test fun `onReceive properly gets download object form sendBroadcast`() { var downloadCompleted = false @@ -190,7 +199,7 @@ class FetchDownloadManagerTest { val id = downloadManager.download(downloadWithFileName)!! - notifyDownloadCompleted(id, downloadWithFileName) + notifyDownloadCompleted(id) assertTrue(downloadCompleted) assertEquals("5MB.zip", downloadName) @@ -198,11 +207,10 @@ class FetchDownloadManagerTest { assertEquals(DownloadJobStatus.COMPLETED, downloadStatus) } - private fun notifyDownloadCompleted(id: Long, download: DownloadState = DownloadState(url = "")) { + private fun notifyDownloadCompleted(id: Long) { val intent = Intent(ACTION_DOWNLOAD_COMPLETE) intent.putExtra(EXTRA_DOWNLOAD_ID, id) intent.putExtra(EXTRA_DOWNLOAD_STATUS, DownloadJobStatus.COMPLETED) - intent.putExtra(EXTRA_DOWNLOAD, download) broadcastManager.sendBroadcast(intent) } @@ -213,5 +221,6 @@ class FetchDownloadManagerTest { class MockDownloadService : AbstractFetchDownloadService() { override val httpClient: Client = mock() + override val store: BrowserStore = mock() } } diff --git a/docs/changelog.md b/docs/changelog.md index 7aaf31b1ccd..a1e4b2eb520 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -13,6 +13,40 @@ permalink: /changelog/ * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Config.kt) * **feature-downloads** + * ⚠️ **This is a breaking change**: DownloadManager and DownloadService are now using the browser store to keep track of queued downloads. Therefore, an instance of the store needs to be provided when constructing manager and service. There's also a new DownloadMiddleware which needs to be provided to the store. + ```kotlin + val store by lazy { + BrowserStore(middleware = listOf( + MediaMiddleware(applicationContext, MediaService::class.java), + DownloadMiddleware(applicationContext, DownloadService::class.java), + ... + )) + } + ) + + downloadsFeature.set( + feature = DownloadsFeature( + requireContext().applicationContext, + store = components.store, // Store needs to be provided now + useCases = components.downloadsUseCases, + fragmentManager = childFragmentManager, + onDownloadStopped = { download, id, status -> + Logger.debug("Download done. ID#$id $download with status $status") + }, + downloadManager = FetchDownloadManager( + requireContext().applicationContext, + components.store, // Store needs to be provided now + DownloadService::class + ), + tabId = sessionId, + onNeedToRequestPermissions = { permissions -> + requestPermissions(permissions, REQUEST_CODE_DOWNLOAD_PERMISSIONS) + }), + owner = this, + view = layout + ) + ``` + * Fixed issue [#6893](https://github.com/mozilla-mobile/android-components/issues/6893). * Add notification grouping to downloads Fenix issue [#4910](https://github.com/mozilla-mobile/android-components/issues/4910). diff --git a/samples/browser/src/main/java/org/mozilla/samples/browser/BaseBrowserFragment.kt b/samples/browser/src/main/java/org/mozilla/samples/browser/BaseBrowserFragment.kt index 99338721d92..50249324060 100644 --- a/samples/browser/src/main/java/org/mozilla/samples/browser/BaseBrowserFragment.kt +++ b/samples/browser/src/main/java/org/mozilla/samples/browser/BaseBrowserFragment.kt @@ -117,6 +117,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler { }, downloadManager = FetchDownloadManager( requireContext().applicationContext, + components.store, DownloadService::class ), tabId = sessionId, diff --git a/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt b/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt index 7506cf61a7f..da2e5b1008d 100644 --- a/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt +++ b/samples/browser/src/main/java/org/mozilla/samples/browser/DefaultComponents.kt @@ -44,6 +44,7 @@ import mozilla.components.feature.app.links.AppLinksUseCases import mozilla.components.feature.contextmenu.ContextMenuUseCases import mozilla.components.feature.customtabs.CustomTabIntentProcessor import mozilla.components.feature.customtabs.store.CustomTabsServiceStore +import mozilla.components.feature.downloads.DownloadMiddleware import mozilla.components.feature.downloads.DownloadsUseCases import mozilla.components.feature.intent.processing.TabIntentProcessor import mozilla.components.feature.media.RecordingDevicesNotificationFeature @@ -62,6 +63,7 @@ import mozilla.components.feature.webnotifications.WebNotificationFeature import mozilla.components.lib.fetch.httpurlconnection.HttpURLConnectionClient import mozilla.components.lib.nearby.NearbyConnection import org.mozilla.samples.browser.addons.AddonsActivity +import org.mozilla.samples.browser.downloads.DownloadService import org.mozilla.samples.browser.ext.components import org.mozilla.samples.browser.integration.FindInPageIntegration import org.mozilla.samples.browser.media.MediaService @@ -112,6 +114,7 @@ open class DefaultComponents(private val applicationContext: Context) { val store by lazy { BrowserStore(middleware = listOf( MediaMiddleware(applicationContext, MediaService::class.java), + DownloadMiddleware(applicationContext, DownloadService::class.java), ReaderViewMiddleware() )) } diff --git a/samples/browser/src/main/java/org/mozilla/samples/browser/downloads/DownloadService.kt b/samples/browser/src/main/java/org/mozilla/samples/browser/downloads/DownloadService.kt index be5218e51aa..189d42ec6ef 100644 --- a/samples/browser/src/main/java/org/mozilla/samples/browser/downloads/DownloadService.kt +++ b/samples/browser/src/main/java/org/mozilla/samples/browser/downloads/DownloadService.kt @@ -4,9 +4,11 @@ package org.mozilla.samples.browser.downloads +import mozilla.components.browser.state.store.BrowserStore import mozilla.components.feature.downloads.AbstractFetchDownloadService import org.mozilla.samples.browser.ext.components class DownloadService : AbstractFetchDownloadService() { override val httpClient by lazy { components.client } + override val store: BrowserStore by lazy { components.store } }