Skip to content
GitLab
Commit 8302e88f authored by Sawyer Blatz's avatar Sawyer Blatz
Browse files

Closes #5077: Renames onDownloadCompleted

parent 8acc806a
Hide whitespace changes
Inline Side-by-side
components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadsFeature.kt
View file @ 8302e88f
......@@ -22,8 +22,8 @@ import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.feature.downloads.DownloadDialogFragment.Companion.FRAGMENT_TAG
import mozilla.components.feature.downloads.manager.AndroidDownloadManager
import mozilla.components.feature.downloads.manager.DownloadManager
import mozilla.components.feature.downloads.manager.OnDownloadCompleted
import mozilla.components.feature.downloads.manager.noop
import mozilla.components.feature.downloads.manager.onDownloadStopped
import mozilla.components.lib.state.ext.flowScoped
import mozilla.components.support.base.feature.LifecycleAwareFeature
import mozilla.components.support.base.feature.OnNeedToRequestPermissions
......@@ -42,7 +42,7 @@ import java.io.File
* @property onNeedToRequestPermissions a callback invoked when permissions
* need to be requested before a download can be performed. Once the request
* is completed, [onPermissionsResult] needs to be invoked.
* @property onDownloadCompleted a callback invoked when a download is completed.
* @property onDownloadStopped a callback invoked when a download is paused or completed.
* @property downloadManager a reference to the [DownloadManager] which is
* responsible for performing the downloads.
* @property store a reference to the application's [BrowserStore].
......@@ -59,7 +59,7 @@ class DownloadsFeature(
private val store: BrowserStore,
private val useCases: DownloadsUseCases,
override var onNeedToRequestPermissions: OnNeedToRequestPermissions = { },
onDownloadCompleted: OnDownloadCompleted = noop,
onDownloadStopped: onDownloadStopped = noop,
private val downloadManager: DownloadManager = AndroidDownloadManager(applicationContext),
private val customTabId: String? = null,
private val fragmentManager: FragmentManager? = null,
......@@ -70,12 +70,12 @@ class DownloadsFeature(
)
) : LifecycleAwareFeature, PermissionsFeature {
var onDownloadCompleted: OnDownloadCompleted
get() = downloadManager.onDownloadCompleted
set(value) { downloadManager.onDownloadCompleted = value }
var onDownloadStopped: onDownloadStopped
get() = downloadManager.onDownloadStopped
set(value) { downloadManager.onDownloadStopped = value }
init {
this.onDownloadCompleted = onDownloadCompleted
this.onDownloadStopped = onDownloadStopped
}
private var scope: CoroutineScope? = null
......
components/feature/downloads/src/main/java/mozilla/components/feature/downloads/manager/AndroidDownloadManager.kt
View file @ 8302e88f
......@@ -36,7 +36,7 @@ typealias SystemRequest = android.app.DownloadManager.Request
*/
class AndroidDownloadManager(
private val applicationContext: Context,
override var onDownloadCompleted: OnDownloadCompleted = noop
override var onDownloadStopped: onDownloadStopped = noop
) : BroadcastReceiver(), DownloadManager {
private val queuedDownloads = LongSparseArray<DownloadStateWithRequest>()
......@@ -111,7 +111,7 @@ class AndroidDownloadManager(
}
/**
* Invoked when a download is complete. Calls [onDownloadCompleted] and unregisters the
* Invoked when a download is complete. Calls [onDownloadStopped] and unregisters the
* broadcast receiver if there are no more queued downloads.
*/
override fun onReceive(context: Context, intent: Intent) {
......@@ -121,7 +121,7 @@ class AndroidDownloadManager(
as AbstractFetchDownloadService.DownloadJobStatus
if (download != null) {
onDownloadCompleted(download.state, downloadID, downloadStatus)
onDownloadStopped(download.state, downloadID, downloadStatus)
}
}
}
......
components/feature/downloads/src/main/java/mozilla/components/feature/downloads/manager/DownloadManager.kt
View file @ 8302e88f
......@@ -9,13 +9,13 @@ import mozilla.components.browser.state.state.content.DownloadState
import mozilla.components.feature.downloads.AbstractFetchDownloadService
import mozilla.components.support.ktx.android.content.isPermissionGranted
typealias OnDownloadCompleted = (DownloadState, Long, AbstractFetchDownloadService.DownloadJobStatus) -> Unit
typealias onDownloadStopped = (DownloadState, Long, AbstractFetchDownloadService.DownloadJobStatus) -> Unit
interface DownloadManager {
val permissions: Array<String>
var onDownloadCompleted: OnDownloadCompleted
var onDownloadStopped: onDownloadStopped
/**
* Schedules a download through the [DownloadManager].
......@@ -45,4 +45,4 @@ fun DownloadManager.validatePermissionGranted(context: Context) {
}
}
internal val noop: OnDownloadCompleted = { _, _, _ -> }
internal val noop: onDownloadStopped = { _, _, _ -> }
components/feature/downloads/src/main/java/mozilla/components/feature/downloads/manager/FetchDownloadManager.kt
View file @ 8302e88f
......@@ -35,7 +35,7 @@ class FetchDownloadManager<T : AbstractFetchDownloadService>(
private val applicationContext: Context,
private val service: KClass<T>,
private val broadcastManager: LocalBroadcastManager = LocalBroadcastManager.getInstance(applicationContext),
override var onDownloadCompleted: OnDownloadCompleted = noop
override var onDownloadStopped: onDownloadStopped = noop
) : BroadcastReceiver(), DownloadManager {
private val queuedDownloads = LongSparseArray<DownloadState>()
......@@ -104,7 +104,7 @@ class FetchDownloadManager<T : AbstractFetchDownloadService>(
}
/**
* Invoked when a download is complete. Calls [onDownloadCompleted] and unregisters the
* Invoked when a download is complete. Calls [onDownloadStopped] and unregisters the
* broadcast receiver if there are no more queued downloads.
*/
override fun onReceive(context: Context, intent: Intent) {
......@@ -114,7 +114,7 @@ class FetchDownloadManager<T : AbstractFetchDownloadService>(
val download = queuedDownloads[downloadID]
if (download != null) {
onDownloadCompleted(download, downloadID, downloadStatus)
onDownloadStopped(download, downloadID, downloadStatus)
}
}
}
components/feature/downloads/src/test/java/mozilla/components/feature/downloads/manager/AndroidDownloadManagerTest.kt
View file @ 8302e88f
......@@ -51,7 +51,7 @@ class AndroidDownloadManagerTest {
fun `calling download must download the file`() {
var downloadCompleted = false
downloadManager.onDownloadCompleted = { _, _, _ -> downloadCompleted = true }
downloadManager.onDownloadStopped = { _, _, _ -> downloadCompleted = true }
grantPermissions()
......@@ -66,7 +66,7 @@ class AndroidDownloadManagerTest {
fun `calling tryAgain starts the download again`() {
var downloadCompleted = false
downloadManager.onDownloadCompleted = { _, _, _ -> downloadCompleted = true }
downloadManager.onDownloadStopped = { _, _, _ -> downloadCompleted = true }
grantPermissions()
......@@ -97,7 +97,7 @@ class AndroidDownloadManagerTest {
}
@Test
fun `sendBroadcast with valid downloadID must call onDownloadCompleted after download`() {
fun `sendBroadcast with valid downloadID must call onDownloadStopped after download`() {
var downloadCompleted = false
var downloadStatus: DownloadJobStatus? = null
val downloadWithFileName = download.copy(fileName = "5MB.zip")
......@@ -109,7 +109,7 @@ class AndroidDownloadManagerTest {
cookie = "yummy_cookie=choco"
)!!
downloadManager.onDownloadCompleted = { _, _, status ->
downloadManager.onDownloadStopped = { _, _, status ->
downloadStatus = status
downloadCompleted = true
}
......
components/feature/downloads/src/test/java/mozilla/components/feature/downloads/manager/FetchDownloadManagerTest.kt
View file @ 8302e88f
......@@ -62,7 +62,7 @@ class FetchDownloadManagerTest {
downloadManager = FetchDownloadManager(context, MockDownloadService::class, broadcastManager)
var downloadCompleted = false
downloadManager.onDownloadCompleted = { _, _, _ -> downloadCompleted = true }
downloadManager.onDownloadStopped = { _, _, _ -> downloadCompleted = true }
grantPermissions()
......@@ -81,7 +81,7 @@ class FetchDownloadManagerTest {
downloadManager = FetchDownloadManager(context, MockDownloadService::class, broadcastManager)
var downloadCompleted = false
downloadManager.onDownloadCompleted = { _, _, _ -> downloadCompleted = true }
downloadManager.onDownloadStopped = { _, _, _ -> downloadCompleted = true }
grantPermissions()
......@@ -113,14 +113,14 @@ class FetchDownloadManagerTest {
}
@Test
fun `sendBroadcast with valid downloadID must call onDownloadCompleted after download`() {
fun `sendBroadcast with valid downloadID must call onDownloadStopped after download`() {
var downloadCompleted = false
var downloadStatus: DownloadJobStatus? = null
val downloadWithFileName = download.copy(fileName = "5MB.zip")
grantPermissions()
downloadManager.onDownloadCompleted = { _, _, status ->
downloadManager.onDownloadStopped = { _, _, status ->
downloadStatus = status
downloadCompleted = true
}
......
docs/changelog.md
View file @ 8302e88f
......@@ -12,6 +12,10 @@ permalink: /changelog/
* [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt)
* [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Config.kt)
* **feature-downloads**
* ⚠️ **This is a breaking change**:
* Renamed to `OnDownloadCompleted` to `OnDownloadStopped` for increased clarity on when it's triggered
* **browser-state**
* ⚠️ **This is a breaking change**: `DownloadState` doesn't include the property `filePath` in its constructor anymore, now it is a computed property. As the previous behavior caused some situations where `fileName` was initially null and after assigned a value to produce `filePath` values like "/storage/emulated/0/Download/null" [for more info](https://sentry.prod.mozaws.net/operations/reference-browser/issues/6609701/).
......
samples/browser/src/main/java/org/mozilla/samples/browser/BaseBrowserFragment.kt
View file @ 8302e88f
......@@ -109,7 +109,7 @@ abstract class BaseBrowserFragment : Fragment(), BackHandler {
store = components.store,
useCases = components.downloadsUseCases,
fragmentManager = childFragmentManager,
onDownloadCompleted = { download, id, status ->
onDownloadStopped = { download, id, status ->
Logger.debug("Download done. ID#$id $download with status $status")
},
downloadManager = FetchDownloadManager(
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment