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 2b0a2f28dfd..04a68b55991 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 @@ -7,6 +7,7 @@ package mozilla.components.feature.downloads import android.annotation.TargetApi import android.app.DownloadManager.ACTION_DOWNLOAD_COMPLETE import android.app.DownloadManager.EXTRA_DOWNLOAD_ID +import android.app.Notification import android.app.Service import android.content.ActivityNotFoundException import android.content.BroadcastReceiver @@ -44,6 +45,12 @@ import mozilla.components.concept.fetch.Headers.Names.CONTENT_RANGE import mozilla.components.concept.fetch.Headers.Names.RANGE import mozilla.components.concept.fetch.MutableHeaders import mozilla.components.concept.fetch.Request +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.CANCELLED +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.COMPLETED +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.ACTIVE +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.addCompletedDownload import mozilla.components.feature.downloads.ext.getDownloadExtra import mozilla.components.feature.downloads.ext.withResponse @@ -77,6 +84,8 @@ abstract class AbstractFetchDownloadService : Service() { internal val broadcastManager by lazy { LocalBroadcastManager.getInstance(this) } @VisibleForTesting internal val context: Context get() = this + @VisibleForTesting + internal var compatForegroundNotificationId: Int = COMPAT_DEFAULT_FOREGROUND_ID internal var downloadJobs = mutableMapOf() @@ -145,27 +154,25 @@ abstract class AbstractFetchDownloadService : Service() { when (intent.action) { ACTION_PAUSE -> { - setDownloadJobStatus(currentDownloadJobState, DownloadJobStatus.PAUSED) + setDownloadJobStatus(currentDownloadJobState, PAUSED) currentDownloadJobState.job?.cancel() emitNotificationPauseFact() } ACTION_RESUME -> { - setDownloadJobStatus(currentDownloadJobState, DownloadJobStatus.ACTIVE) + setDownloadJobStatus(currentDownloadJobState, ACTIVE) currentDownloadJobState.job = CoroutineScope(IO).launch { - startDownloadJob(downloadId) + startDownloadJob(currentDownloadJobState) } emitNotificationResumeFact() } ACTION_CANCEL -> { - NotificationManagerCompat.from(context).cancel( - currentDownloadJobState.foregroundServiceId - ) + removeNotification(context, currentDownloadJobState) currentDownloadJobState.lastNotificationUpdate = System.currentTimeMillis() - setDownloadJobStatus(currentDownloadJobState, DownloadJobStatus.CANCELLED) + setDownloadJobStatus(currentDownloadJobState, CANCELLED) currentDownloadJobState.job?.cancel() currentDownloadJobState.job = CoroutineScope(IO).launch { @@ -173,23 +180,24 @@ abstract class AbstractFetchDownloadService : Service() { currentDownloadJobState.downloadDeleted = true } + removeDownloadJob(currentDownloadJobState) emitNotificationCancelFact() } ACTION_TRY_AGAIN -> { - NotificationManagerCompat.from(context).cancel( - currentDownloadJobState.foregroundServiceId - ) + removeNotification(context, currentDownloadJobState) currentDownloadJobState.lastNotificationUpdate = System.currentTimeMillis() - setDownloadJobStatus(currentDownloadJobState, DownloadJobStatus.ACTIVE) + setDownloadJobStatus(currentDownloadJobState, ACTIVE) currentDownloadJobState.job = CoroutineScope(IO).launch { - startDownloadJob(downloadId) + startDownloadJob(currentDownloadJobState) } emitNotificationTryAgainFact() } + ACTION_DISMISS -> removeDownloadJob(currentDownloadJobState) + ACTION_OPEN -> { if (!openFile( context = context, @@ -223,16 +231,20 @@ abstract class AbstractFetchDownloadService : Service() { val foregroundServiceId = downloadJobs[download.id]?.foregroundServiceId ?: Random.nextInt() // Create a new job and add it, with its downloadState to the map - downloadJobs[download.id] = DownloadJobState( + val downloadJobState = DownloadJobState( state = download, foregroundServiceId = foregroundServiceId, - status = DownloadJobStatus.ACTIVE + status = ACTIVE ) - downloadJobs[download.id]?.job = CoroutineScope(IO).launch { - startDownloadJob(download.id) + downloadJobState.job = CoroutineScope(IO).launch { + startDownloadJob(downloadJobState) } + downloadJobs[download.id] = downloadJobState + + setForegroundNotification(downloadJobState) + notificationUpdateScope.launch { while (isActive) { delay(PROGRESS_UPDATE_INTERVAL) @@ -260,67 +272,50 @@ abstract class AbstractFetchDownloadService : Service() { */ val uiStatus = getDownloadJobStatus(download) - // Dispatch the corresponding notification based on the current status - val notification = when (uiStatus) { - DownloadJobStatus.ACTIVE -> { - DownloadNotification.createOngoingDownloadNotification( - context, - download - ) - } - - DownloadJobStatus.PAUSED -> { - DownloadNotification.createPausedDownloadNotification(context, download) - } - - DownloadJobStatus.COMPLETED -> { - DownloadNotification.createDownloadCompletedNotification(context, download) - } + updateForegroundNotificationIfNeeded(download) - DownloadJobStatus.FAILED -> { - DownloadNotification.createDownloadFailedNotification(context, download) - } + // Dispatch the corresponding notification based on the current status + updateDownloadNotification(uiStatus, download) - DownloadJobStatus.CANCELLED -> { - NotificationManagerCompat.from(context).cancel(download.foregroundServiceId) - download.lastNotificationUpdate = System.currentTimeMillis() - null - } + if (uiStatus != ACTIVE) { + sendDownloadStopped(download) } + } + } - notification?.let { - NotificationManagerCompat.from(context).notify( - download.foregroundServiceId, - it - ) + /*** + * Updates the notification state with the passed [download] data. + * Be aware that you need to pass [latestUIStatus] as [DownloadJobState.status] can be modified + * from another thread, causing inconsistencies in the ui. + */ + private fun updateDownloadNotification(latestUIStatus: DownloadJobStatus, download: DownloadJobState) { + val notification = when (latestUIStatus) { + ACTIVE -> DownloadNotification.createOngoingDownloadNotification(context, download) + PAUSED -> DownloadNotification.createPausedDownloadNotification(context, download) + COMPLETED -> DownloadNotification.createDownloadCompletedNotification(context, download) + FAILED -> DownloadNotification.createDownloadFailedNotification(context, download) + CANCELLED -> { + removeNotification(context, download) download.lastNotificationUpdate = System.currentTimeMillis() + null } + } - if (uiStatus != DownloadJobStatus.ACTIVE) { - sendDownloadStopped(download) - } + notification?.let { + NotificationManagerCompat.from(context).notify(download.foregroundServiceId, it) + download.lastNotificationUpdate = System.currentTimeMillis() } } override fun onTaskRemoved(rootIntent: Intent?) { - // If the task gets removed (app gets swiped away in the task switcher) our process and this - // service gets killed. In this situation we remove the notification since the download will - // stop and cannot be controlled via the notification anymore (until we persist enough data - // to resume downloads from a new process). - - clearAllDownloadsNotificationsAndJobs() + stopSelf() } - // As we are not fulfilling the api contract of a foreground service, we can be easily killed - // by the system when low on memory, without any call to onDestroy(). - // To fulfill the contract, we need a pinned notification and call startForeground(). - // This means can be left with stuck notifications. - // https://developer.android.com/guide/components/services#Foreground - // https://github.com/mozilla-mobile/android-components/issues/6893 override fun onDestroy() { super.onDestroy() clearAllDownloadsNotificationsAndJobs() + unregisterNotificationActionsReceiver() } // Cancels all running jobs and remove all notifications. @@ -328,6 +323,9 @@ abstract class AbstractFetchDownloadService : Service() { internal fun clearAllDownloadsNotificationsAndJobs() { val notificationManager = NotificationManagerCompat.from(context) + stopForeground(true) + compatForegroundNotificationId = COMPAT_DEFAULT_FOREGROUND_ID + // Before doing any cleaning, we have to stop the notification updater scope. // To ensure we are not recreating the notifications. notificationUpdateScope.cancel() @@ -336,16 +334,13 @@ abstract class AbstractFetchDownloadService : Service() { notificationManager.cancel(state.foregroundServiceId) state.job?.cancel() } - unregisterNotificationActionsReceiver() } - internal fun startDownloadJob(downloadId: Long) { - val currentDownloadJobState = downloadJobs[downloadId] ?: return - + internal fun startDownloadJob(currentDownloadJobState: DownloadJobState) { try { - performDownload(currentDownloadJobState.state) + performDownload(currentDownloadJobState) } catch (e: IOException) { - setDownloadJobStatus(currentDownloadJobState, DownloadJobStatus.FAILED) + setDownloadJobStatus(currentDownloadJobState, FAILED) } } @@ -360,6 +355,7 @@ abstract class AbstractFetchDownloadService : Service() { addAction(ACTION_PAUSE) addAction(ACTION_RESUME) addAction(ACTION_CANCEL) + addAction(ACTION_DISMISS) addAction(ACTION_TRY_AGAIN) addAction(ACTION_OPEN) } @@ -372,9 +368,134 @@ abstract class AbstractFetchDownloadService : Service() { context.unregisterReceiver(broadcastReceiver) } + @VisibleForTesting + internal fun removeDownloadJob(downloadJobState: DownloadJobState) { + downloadJobs.remove(downloadJobState.state.id) + if (downloadJobs.isEmpty()) { + stopSelf() + } else { + updateForegroundNotificationIfNeeded(downloadJobState) + removeNotification(context, downloadJobState) + } + } + + @VisibleForTesting + internal fun removeNotification(context: Context, currentDownloadJobState: DownloadJobState) { + NotificationManagerCompat.from(context).cancel(currentDownloadJobState.foregroundServiceId) + } + + /*** + * Refresh the notification group content only for devices that support it, + * otherwise nothing will happen. + */ + @VisibleForTesting + internal fun updateNotificationGroup(): Notification? { + return if (SDK_INT >= Build.VERSION_CODES.N) { + val downloadList = downloadJobs.values.toList() + val notificationGroup = DownloadNotification.createDownloadGroupNotification(context, downloadList) + NotificationManagerCompat.from(context).apply { + notify(NOTIFICATION_DOWNLOAD_GROUP_ID, notificationGroup) + } + notificationGroup + } else { + null + } + } + + internal fun createCompactForegroundNotification(downloadJobState: DownloadJobState): Notification { + val notification = DownloadNotification.createOngoingDownloadNotification(context, downloadJobState) + compatForegroundNotificationId = downloadJobState.foregroundServiceId + NotificationManagerCompat.from(context).apply { + notify(compatForegroundNotificationId, notification) + downloadJobState.lastNotificationUpdate = System.currentTimeMillis() + } + return notification + } + + @VisibleForTesting + internal fun getForegroundId(): Int { + return if (SDK_INT >= Build.VERSION_CODES.N) { + NOTIFICATION_DOWNLOAD_GROUP_ID + } else { + compatForegroundNotificationId + } + } + + /*** + * We have two different behaviours as notification groups are not supported in all devices. + * For devices that support it, we create a separate notification which will be the foreground + * notification, it will be always present until we don't have more active downloads. + * For devices that doesn't support notification groups, we set the latest active notification as + * the foreground notification and we keep changing it to the latest active download. + */ + @VisibleForTesting + internal fun setForegroundNotification(downloadJobState: DownloadJobState) { + var previousDownload: DownloadJobState? = null + + val (notificationId, notification) = if (SDK_INT >= Build.VERSION_CODES.N) { + NOTIFICATION_DOWNLOAD_GROUP_ID to updateNotificationGroup() + } else { + previousDownload = downloadJobs.values.firstOrNull { + it.foregroundServiceId == compatForegroundNotificationId + } + downloadJobState.foregroundServiceId to createCompactForegroundNotification(downloadJobState) + } + + startForeground(notificationId, notification) + /*** + * In devices that doesn't use notification groups, every new download becomes the new foreground one, + * unfortunately, when we call startForeground it removes the previous foreground notification + * when it's not an ongoing one, for this reason, we have to recreate the deleted notification. + * By the way ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_DETACH) + * doesn't work neither calling stopForeground(false) and then calling startForeground, + * it always deletes the previous notification :( + */ + previousDownload?.let { + updateDownloadNotification(previousDownload.status, it) + } + } + + /*** + * Indicates the status of a download has changed and maybe the foreground notification needs, + * to be updated. For devices that support group notifications, we update the overview + * notification. For devices that don't support group notifications, we try to find a new + * active download and selected it as the new foreground notification. + */ + internal fun updateForegroundNotificationIfNeeded(download: DownloadJobState) { + if (SDK_INT < Build.VERSION_CODES.N) { + /*** + * For devices that don't support notification groups, we have to keep updating + * the foreground notification id, when the previous one gets a state that + * is likely to be dismissed. + */ + val status = download.status + val foregroundId = download.foregroundServiceId + val isSelectedForegroundId = compatForegroundNotificationId == foregroundId + val needNewForegroundNotification = when (status) { + COMPLETED, FAILED, CANCELLED -> true + else -> false + } + + if (isSelectedForegroundId && needNewForegroundNotification) { + // We need to deselect the actual foreground notification, because while it is + // selected the user will not be able to dismiss it. + stopForeground(false) + + // Now we need to find a new foreground notification, if needed. + val newSelectedForegroundDownload = downloadJobs.values.firstOrNull { it.status == ACTIVE } + newSelectedForegroundDownload?.let { + setForegroundNotification(it) + } + } + } else { + // This device supports notification groups, we just need to update the summary notification + updateNotificationGroup() + } + } + @Suppress("ComplexCondition") - internal fun performDownload(download: DownloadState) { - val currentDownloadJobState = downloadJobs[download.id] ?: return + internal fun performDownload(currentDownloadJobState: DownloadJobState) { + val download = currentDownloadJobState.state val isResumingDownload = currentDownloadJobState.currentBytesCopied > 0L val headers = MutableHeaders() @@ -391,7 +512,7 @@ abstract class AbstractFetchDownloadService : Service() { (isResumingDownload && !response.headers.contains(CONTENT_RANGE))) { // We experienced a problem trying to fetch the file, send a failure notification currentDownloadJobState.currentBytesCopied = 0 - setDownloadJobStatus(currentDownloadJobState, DownloadJobStatus.FAILED) + setDownloadJobStatus(currentDownloadJobState, FAILED) return } @@ -413,9 +534,9 @@ abstract class AbstractFetchDownloadService : Service() { internal fun verifyDownload(download: DownloadJobState) { if (getDownloadJobStatus(download) == DownloadJobStatus.ACTIVE && download.currentBytesCopied < download.state.contentLength ?: 0) { - setDownloadJobStatus(download, DownloadJobStatus.FAILED) + setDownloadJobStatus(download, FAILED) } else if (getDownloadJobStatus(download) == DownloadJobStatus.ACTIVE) { - setDownloadJobStatus(download, DownloadJobStatus.COMPLETED) + setDownloadJobStatus(download, COMPLETED) } } @@ -545,7 +666,7 @@ abstract class AbstractFetchDownloadService : Service() { FileOutputStream(file, append).use(block) val downloadJobState = downloadJobs[download.id] ?: return - if (getDownloadJobStatus(downloadJobState) != DownloadJobStatus.COMPLETED) { return } + if (getDownloadJobStatus(downloadJobState) != COMPLETED) { return } addCompletedDownload( title = fileName, @@ -605,6 +726,8 @@ abstract class AbstractFetchDownloadService : Service() { const val ACTION_PAUSE = "mozilla.components.feature.downloads.PAUSE" const val ACTION_RESUME = "mozilla.components.feature.downloads.RESUME" const val ACTION_CANCEL = "mozilla.components.feature.downloads.CANCEL" + const val ACTION_DISMISS = "mozilla.components.feature.downloads.DISMISS" const val ACTION_TRY_AGAIN = "mozilla.components.feature.downloads.TRY_AGAIN" + const val COMPAT_DEFAULT_FOREGROUND_ID = -1 } } diff --git a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadNotification.kt b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadNotification.kt index e905fde32ef..c16338c42e5 100644 --- a/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadNotification.kt +++ b/components/feature/downloads/src/main/java/mozilla/components/feature/downloads/DownloadNotification.kt @@ -12,28 +12,64 @@ import android.content.Context import android.content.Intent import android.os.Build import android.os.Build.VERSION.SDK_INT +import androidx.annotation.VisibleForTesting import androidx.core.app.NotificationCompat import androidx.core.app.NotificationManagerCompat import androidx.core.app.NotificationManagerCompat.IMPORTANCE_NONE import androidx.core.content.ContextCompat import androidx.core.content.getSystemService import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.ACTION_CANCEL +import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.ACTION_DISMISS import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.ACTION_OPEN import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.ACTION_PAUSE import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.ACTION_RESUME import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.ACTION_TRY_AGAIN import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobState +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.CANCELLED +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.COMPLETED +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.ACTIVE +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.FAILED +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.PAUSED import kotlin.random.Random -@Suppress("TooManyFunctions") +@Suppress("LargeClass", "TooManyFunctions") internal object DownloadNotification { private const val NOTIFICATION_CHANNEL_ID = "mozac.feature.downloads.generic" + private const val NOTIFICATION_GROUP_KEY = "mozac.feature.downloads.group" + internal const val NOTIFICATION_DOWNLOAD_GROUP_ID = 100 private const val LEGACY_NOTIFICATION_CHANNEL_ID = "Downloads" - private const val PERCENTAGE_MULTIPLIER = 100 + internal const val PERCENTAGE_MULTIPLIER = 100 internal const val EXTRA_DOWNLOAD_ID = "downloadId" + @VisibleForTesting + internal fun createDownloadGroupNotification( + context: Context, + notifications: List + ): Notification { + val allDownloadsHaveFinished = notifications.all { it.status != ACTIVE } + val icon = if (allDownloadsHaveFinished) { + R.drawable.mozac_feature_download_ic_download_complete + } else { + R.drawable.mozac_feature_download_ic_ongoing_download + } + val summaryList = getSummaryList(context, notifications) + val summaryLine1 = summaryList.first() + val summaryLine2 = if (summaryList.size == 2) summaryList[1] else "" + + return NotificationCompat.Builder(context, ensureChannelExists(context)) + .setSmallIcon(icon) + .setColor(ContextCompat.getColor(context, R.color.mozac_feature_downloads_notification)) + .setContentTitle(context.getString(R.string.mozac_feature_downloads_notification_channel)) + .setContentText(summaryList.joinToString("\n")) + .setStyle(NotificationCompat.InboxStyle().addLine(summaryLine1).addLine(summaryLine2)) + .setGroup(NOTIFICATION_GROUP_KEY) + .setGroupSummary(true) + .setPriority(NotificationCompat.PRIORITY_HIGH) + .build() + } + /** * Build the notification to be displayed while the download service is active. */ @@ -44,18 +80,12 @@ internal object DownloadNotification { val downloadState = downloadJobState.state val bytesCopied = downloadJobState.currentBytesCopied val channelId = ensureChannelExists(context) - val fileSizeText = (downloadState.contentLength?.toMegabyteString() ?: "") val isIndeterminate = downloadState.contentLength == null - val contentText = if (isIndeterminate) { - fileSizeText - } else { - "${PERCENTAGE_MULTIPLIER * bytesCopied / downloadState.contentLength!!}%" - } return NotificationCompat.Builder(context, channelId) .setSmallIcon(R.drawable.mozac_feature_download_ic_ongoing_download) .setContentTitle(downloadState.fileName) - .setContentText(contentText) + .setContentText(downloadJobState.getProgress()) .setColor(ContextCompat.getColor(context, R.color.mozac_feature_downloads_notification)) .setCategory(NotificationCompat.CATEGORY_PROGRESS) .setProgress(downloadState.contentLength?.toInt() ?: 0, bytesCopied.toInt(), isIndeterminate) @@ -65,6 +95,7 @@ internal object DownloadNotification { .addAction(getPauseAction(context, downloadState.id)) .addAction(getCancelAction(context, downloadState.id)) .setPriority(NotificationCompat.PRIORITY_LOW) + .setCompatGroup(NOTIFICATION_GROUP_KEY) .build() } @@ -86,6 +117,8 @@ internal object DownloadNotification { .setOnlyAlertOnce(true) .addAction(getResumeAction(context, downloadState.id)) .addAction(getCancelAction(context, downloadState.id)) + .setDeleteIntent(createDismissPendingIntent(context, downloadState.id)) + .setCompatGroup(NOTIFICATION_GROUP_KEY) .build() } @@ -105,6 +138,8 @@ internal object DownloadNotification { .setColor(ContextCompat.getColor(context, R.color.mozac_feature_downloads_notification)) .setContentIntent(createPendingIntent(context, ACTION_OPEN, downloadState.id)) .setPriority(NotificationCompat.PRIORITY_LOW) + .setDeleteIntent(createDismissPendingIntent(context, downloadState.id)) + .setCompatGroup(NOTIFICATION_GROUP_KEY) .build() } @@ -126,9 +161,18 @@ internal object DownloadNotification { .setWhen(downloadJobState.createdTime) .setOnlyAlertOnce(true) .setPriority(NotificationCompat.PRIORITY_LOW) + .setDeleteIntent(createDismissPendingIntent(context, downloadState.id)) + .setCompatGroup(NOTIFICATION_GROUP_KEY) .build() } + @VisibleForTesting + internal fun getSummaryList(context: Context, notifications: List): List { + return notifications.take(2).map { downloadState -> + "${downloadState.state.fileName} ${downloadState.getStatusDescription(context)}" + } + } + /** * Check if notifications from the download channel are enabled. * Verifies that app notifications, channel notifications, and group notifications are enabled. @@ -211,6 +255,10 @@ internal object DownloadNotification { ).build() } + private fun createDismissPendingIntent(context: Context, downloadStateId: Long): PendingIntent { + return createPendingIntent(context, ACTION_DISMISS, downloadStateId) + } + private fun createPendingIntent(context: Context, action: String, downloadStateId: Long): PendingIntent { val intent = Intent(action) intent.setPackage(context.applicationContext.packageName) @@ -221,3 +269,44 @@ internal object DownloadNotification { return PendingIntent.getBroadcast(context.applicationContext, Random.nextInt(), intent, 0) } } + +@VisibleForTesting +internal fun NotificationCompat.Builder.setCompatGroup(groupKey: String): NotificationCompat.Builder { + return if (SDK_INT >= Build.VERSION_CODES.N) { + setGroup(groupKey) + } else this +} + +@VisibleForTesting +internal fun DownloadJobState.getProgress(): String { + val bytesCopied = currentBytesCopied + val isIndeterminate = state.contentLength == null || bytesCopied == 0L + return if (isIndeterminate) { + "" + } else { + "${DownloadNotification.PERCENTAGE_MULTIPLIER * bytesCopied / state.contentLength!!}%" + } +} + +@VisibleForTesting +internal fun DownloadJobState.getStatusDescription(context: Context): String { + return when (this.status) { + ACTIVE -> { + getProgress() + } + + PAUSED -> { + context.getString(R.string.mozac_feature_downloads_paused_notification_text) + } + + COMPLETED -> { + context.getString(R.string.mozac_feature_downloads_completed_notification_text2) + } + + FAILED -> { + context.getString(R.string.mozac_feature_downloads_failed_notification_text2) + } + + CANCELLED -> "" + } +} 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 862a74b74f3..424a0c2ab12 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,10 +4,12 @@ package mozilla.components.feature.downloads +import android.app.Notification import android.app.NotificationManager import android.app.Service import android.content.Context import android.content.Intent +import android.os.Build import androidx.core.app.NotificationManagerCompat import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -30,11 +32,13 @@ import mozilla.components.feature.downloads.AbstractFetchDownloadService.Compani import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.ACTION_RESUME import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.ACTION_TRY_AGAIN import mozilla.components.feature.downloads.AbstractFetchDownloadService.Companion.PROGRESS_UPDATE_INTERVAL +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobState import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.ACTIVE import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.CANCELLED import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.COMPLETED 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 @@ -49,6 +53,7 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue +import org.junit.Assert.assertNull import org.junit.Before import org.junit.Rule import org.junit.Test @@ -65,6 +70,7 @@ import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations.initMocks import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config import org.robolectric.shadows.ShadowNotificationManager import java.io.IOException import java.io.InputStream @@ -122,15 +128,15 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - assertEquals(download.url, providedDownload.value.url) - assertEquals(download.fileName, providedDownload.value.fileName) + assertEquals(download.url, providedDownload.value.state.url) + assertEquals(download.fileName, providedDownload.value.state.fileName) // Ensure the job is properly added to the map assertEquals(1, service.downloadJobs.count()) - assertNotNull(service.downloadJobs[providedDownload.value.id]) + assertNotNull(service.downloadJobs[providedDownload.value.state.id]) } @Test @@ -149,7 +155,7 @@ class AbstractFetchDownloadServiceTest { contentLength = 50L ) - val downloadJobState = AbstractFetchDownloadService.DownloadJobState( + val downloadJobState = DownloadJobState( job = null, state = downloadState, currentBytesCopied = 5, @@ -170,7 +176,7 @@ class AbstractFetchDownloadServiceTest { contentLength = 50L ) - val downloadJobState = AbstractFetchDownloadService.DownloadJobState( + val downloadJobState = DownloadJobState( job = null, state = downloadState, currentBytesCopied = 5, @@ -191,7 +197,7 @@ class AbstractFetchDownloadServiceTest { contentLength = 50L ) - val downloadJobState = AbstractFetchDownloadService.DownloadJobState( + val downloadJobState = DownloadJobState( job = null, state = downloadState, currentBytesCopied = 50, @@ -212,7 +218,7 @@ class AbstractFetchDownloadServiceTest { contentLength = 50L ) - val downloadJobState = AbstractFetchDownloadService.DownloadJobState( + val downloadJobState = DownloadJobState( job = null, state = downloadState, currentBytesCopied = 50, @@ -233,7 +239,7 @@ class AbstractFetchDownloadServiceTest { contentLength = 50L ) - val downloadJobState = AbstractFetchDownloadService.DownloadJobState( + val downloadJobState = DownloadJobState( job = null, state = downloadState, currentBytesCopied = 50, @@ -265,12 +271,12 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) val pauseIntent = Intent(ACTION_PAUSE).apply { setPackage(testContext.applicationContext.packageName) - putExtra(DownloadNotification.EXTRA_DOWNLOAD_ID, providedDownload.value.id) + putExtra(DownloadNotification.EXTRA_DOWNLOAD_ID, providedDownload.value.state.id) } CollectionProcessor.withFactCollection { facts -> @@ -281,8 +287,8 @@ class AbstractFetchDownloadServiceTest { assertEquals(NOTIFICATION, pauseFact.item) } - service.downloadJobs[providedDownload.value.id]?.job?.join() - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! + service.downloadJobs[providedDownload.value.state.id]?.job?.join() + val downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! assertEquals(PAUSED, service.getDownloadJobStatus(downloadJobState)) } @@ -304,15 +310,15 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) val cancelIntent = Intent(ACTION_CANCEL).apply { setPackage(testContext.applicationContext.packageName) - putExtra(DownloadNotification.EXTRA_DOWNLOAD_ID, providedDownload.value.id) + putExtra(DownloadNotification.EXTRA_DOWNLOAD_ID, providedDownload.value.state.id) } - assertFalse(service.downloadJobs[providedDownload.value.id]!!.downloadDeleted) + assertFalse(service.downloadJobs[providedDownload.value.state.id]!!.downloadDeleted) CollectionProcessor.withFactCollection { facts -> service.broadcastReceiver.onReceive(testContext, cancelIntent) @@ -321,12 +327,6 @@ class AbstractFetchDownloadServiceTest { assertEquals(Action.CANCEL, cancelFact.action) assertEquals(NOTIFICATION, cancelFact.item) } - - service.downloadJobs[providedDownload.value.id]?.job?.join() - - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! - assertEquals(CANCELLED, service.getDownloadJobStatus(downloadJobState)) - assertTrue(downloadJobState.downloadDeleted) } @Test @@ -357,18 +357,18 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) // Simulate a pause - var downloadJobState = service.downloadJobs[providedDownload.value.id]!! + var downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! downloadJobState.currentBytesCopied = 1 service.setDownloadJobStatus(downloadJobState, PAUSED) - service.downloadJobs[providedDownload.value.id]?.job?.cancel() + service.downloadJobs[providedDownload.value.state.id]?.job?.cancel() val resumeIntent = Intent(ACTION_RESUME).apply { setPackage(testContext.applicationContext.packageName) - putExtra(DownloadNotification.EXTRA_DOWNLOAD_ID, providedDownload.value.id) + putExtra(DownloadNotification.EXTRA_DOWNLOAD_ID, providedDownload.value.state.id) } CollectionProcessor.withFactCollection { facts -> @@ -379,15 +379,15 @@ class AbstractFetchDownloadServiceTest { assertEquals(NOTIFICATION, resumeFact.item) } - downloadJobState = service.downloadJobs[providedDownload.value.id]!! + downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! assertEquals(ACTIVE, service.getDownloadJobStatus(downloadJobState)) // Make sure the download job is completed (break out of copyInChunks) service.setDownloadJobStatus(downloadJobState, PAUSED) - service.downloadJobs[providedDownload.value.id]?.job?.join() + service.downloadJobs[providedDownload.value.state.id]?.job?.join() - verify(service).startDownloadJob(providedDownload.value.id) + verify(service).startDownloadJob(providedDownload.value) } @Test @@ -408,18 +408,18 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - service.downloadJobs[providedDownload.value.id]?.job?.join() + service.downloadJobs[providedDownload.value.state.id]?.job?.join() // Simulate a failure - var downloadJobState = service.downloadJobs[providedDownload.value.id]!! + var downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! service.setDownloadJobStatus(downloadJobState, FAILED) - service.downloadJobs[providedDownload.value.id]?.job?.cancel() + service.downloadJobs[providedDownload.value.state.id]?.job?.cancel() val tryAgainIntent = Intent(ACTION_TRY_AGAIN).apply { setPackage(testContext.applicationContext.packageName) - putExtra(DownloadNotification.EXTRA_DOWNLOAD_ID, providedDownload.value.id) + putExtra(DownloadNotification.EXTRA_DOWNLOAD_ID, providedDownload.value.state.id) } CollectionProcessor.withFactCollection { facts -> @@ -430,15 +430,15 @@ class AbstractFetchDownloadServiceTest { assertEquals(NOTIFICATION, tryAgainFact.item) } - downloadJobState = service.downloadJobs[providedDownload.value.id]!! + downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! assertEquals(ACTIVE, service.getDownloadJobStatus(downloadJobState)) // Make sure the download job is completed (break out of copyInChunks) service.setDownloadJobStatus(downloadJobState, PAUSED) - service.downloadJobs[providedDownload.value.id]?.job?.join() + service.downloadJobs[providedDownload.value.state.id]?.job?.join() - verify(service).startDownloadJob(providedDownload.value.id) + verify(service).startDownloadJob(providedDownload.value) } @Test @@ -459,11 +459,11 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - service.downloadJobs[providedDownload.value.id]?.job?.join() - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! + service.downloadJobs[providedDownload.value.state.id]?.job?.join() + val downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! assertEquals(FAILED, service.getDownloadJobStatus(downloadJobState)) } @@ -513,17 +513,249 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - service.downloadJobs[providedDownload.value.id]?.job?.join() - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! + service.downloadJobs[providedDownload.value.state.id]?.job?.join() + val downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! service.setDownloadJobStatus(downloadJobState, ACTIVE) assertEquals(ACTIVE, service.getDownloadJobStatus(downloadJobState)) testDispatcher.advanceTimeBy(PROGRESS_UPDATE_INTERVAL) + // The additional notification is the summary one (the notification group). + assertEquals(2, shadowNotificationService.size()) + } + + @Test + 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) + } + + doNothing().`when`(service).performDownload(any()) + + service.onStartCommand(downloadIntent, 0, 0) + + verify(service).setForegroundNotification(any()) + } + + @Test + fun `sets the notification foreground in devices that support notification group`() = runBlocking { + val download = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt") + val downloadState = DownloadJobState( + state = download, + foregroundServiceId = Random.nextInt(), + status = ACTIVE + ) + val notification = mock() + + doReturn(notification).`when`(service).updateNotificationGroup() + + service.downloadJobs[1L] = downloadState + + service.setForegroundNotification(downloadState) + + verify(service).startForeground(NOTIFICATION_DOWNLOAD_GROUP_ID, notification) + } + + @Test + @Config(sdk = [Build.VERSION_CODES.M]) + fun `sets the notification foreground in devices that DO NOT support notification group`() { + val download = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt") + val downloadState = DownloadJobState( + state = download, + foregroundServiceId = Random.nextInt(), + status = ACTIVE + ) + val notification = mock() + + doReturn(notification).`when`(service).createCompactForegroundNotification(downloadState) + + service.downloadJobs[1L] = downloadState + + service.setForegroundNotification(downloadState) + + verify(service).startForeground(downloadState.foregroundServiceId, notification) + } + + @Test + @Config(sdk = [Build.VERSION_CODES.M]) + fun createCompactForegroundNotification() { + val download = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt") + val downloadState = DownloadJobState( + state = download, + foregroundServiceId = Random.nextInt(), + status = ACTIVE + ) + + assertEquals(0, shadowNotificationService.size()) + + val notification = service.createCompactForegroundNotification(downloadState) + + service.downloadJobs[1L] = downloadState + + service.setForegroundNotification(downloadState) + + assertNull(notification.group) assertEquals(1, shadowNotificationService.size()) + assertNotNull(shadowNotificationService.getNotification(downloadState.foregroundServiceId)) + } + + @Test + 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) + } + + doNothing().`when`(service).performDownload(any()) + + service.onStartCommand(downloadIntent, 0, 0) + + assertEquals(NOTIFICATION_DOWNLOAD_GROUP_ID, service.getForegroundId()) + } + + @Test + @Config(sdk = [Build.VERSION_CODES.M]) + 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) + } + + doNothing().`when`(service).performDownload(any()) + + service.onStartCommand(downloadIntent, 0, 0) + + val foregroundId = service.downloadJobs.values.first().foregroundServiceId + assertEquals(foregroundId, service.getForegroundId()) + assertEquals(foregroundId, service.compatForegroundNotificationId) + } + + @Test + @Config(sdk = [Build.VERSION_CODES.M]) + fun `updateNotificationGroup will do nothing on devices that do not support notificaiton groups`() = runBlocking { + val download = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt") + val downloadState = DownloadJobState( + state = download, + foregroundServiceId = Random.nextInt(), + status = ACTIVE + ) + + service.downloadJobs[1L] = downloadState + + val notificationGroup = service.updateNotificationGroup() + + assertNull(notificationGroup) + assertEquals(0, shadowNotificationService.size()) + } + + @Test + fun `removeDownloadJob will update the background notification if there are other pending downloads`() { + val download = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt") + val downloadState = DownloadJobState( + state = download, + foregroundServiceId = Random.nextInt(), + status = ACTIVE + ) + + service.downloadJobs[1L] = downloadState + service.downloadJobs[2L] = mock() + + doNothing().`when`(service).updateForegroundNotificationIfNeeded(downloadState) + + service.removeDownloadJob(downloadJobState = downloadState) + + assertEquals(1, service.downloadJobs.size) + verify(service).updateForegroundNotificationIfNeeded(downloadState) + verify(service).removeNotification(testContext, downloadState) + } + + @Test + fun `removeDownloadJob will stop the service if there are none pending downloads`() { + val download = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt") + val downloadState = DownloadJobState( + state = download, + foregroundServiceId = Random.nextInt(), + status = ACTIVE + ) + + doNothing().`when`(service).stopForeground(false) + doNothing().`when`(service).clearAllDownloadsNotificationsAndJobs() + doNothing().`when`(service).stopSelf() + + service.downloadJobs[1L] = downloadState + + service.removeDownloadJob(downloadJobState = downloadState) + + assertTrue(service.downloadJobs.isEmpty()) + verify(service).stopSelf() + verify(service, times(0)).updateForegroundNotificationIfNeeded(downloadState) + } + + @Test + fun `updateForegroundNotification will update the notification group for devices that support it`() { + doReturn(null).`when`(service).updateNotificationGroup() + + service.updateForegroundNotificationIfNeeded(mock()) + + verify(service).updateNotificationGroup() + } + + @Test + @Config(sdk = [Build.VERSION_CODES.M]) + fun `updateForegroundNotification will select a new foreground notification`() { + val downloadState1 = DownloadJobState( + state = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt"), + foregroundServiceId = Random.nextInt(), + status = COMPLETED + ) + val downloadState2 = DownloadJobState( + state = DownloadState(id = 2, url = "https://example.com/file.txt", fileName = "file.txt"), + foregroundServiceId = Random.nextInt(), + status = ACTIVE + ) + + service.compatForegroundNotificationId = downloadState1.foregroundServiceId + + service.downloadJobs[1L] = downloadState1 + service.downloadJobs[2L] = downloadState2 + + service.updateForegroundNotificationIfNeeded(downloadState1) + + verify(service).setForegroundNotification(downloadState2) + assertEquals(downloadState2.foregroundServiceId, service.compatForegroundNotificationId) + } + + @Test + @Config(sdk = [Build.VERSION_CODES.M]) + fun `updateForegroundNotification will NOT select a new foreground notification`() { + val downloadState1 = DownloadJobState( + state = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt"), + foregroundServiceId = Random.nextInt(), + status = ACTIVE + ) + val downloadState2 = DownloadJobState( + state = DownloadState(id = 2, url = "https://example.com/file.txt", fileName = "file.txt"), + foregroundServiceId = Random.nextInt(), + status = ACTIVE + ) + + service.compatForegroundNotificationId = downloadState1.foregroundServiceId + + service.downloadJobs[1L] = downloadState1 + service.downloadJobs[2L] = downloadState2 + + service.updateForegroundNotificationIfNeeded(downloadState1) + + verify(service, times(0)).setForegroundNotification(downloadState2) + verify(service, times(0)).updateNotificationGroup() + assertEquals(downloadState1.foregroundServiceId, service.compatForegroundNotificationId) } @Test @@ -544,17 +776,18 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - service.downloadJobs[providedDownload.value.id]?.job?.join() - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! + service.downloadJobs[providedDownload.value.state.id]?.job?.join() + val downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! service.setDownloadJobStatus(downloadJobState, PAUSED) assertEquals(PAUSED, service.getDownloadJobStatus(downloadJobState)) testDispatcher.advanceTimeBy(PROGRESS_UPDATE_INTERVAL) - assertEquals(1, shadowNotificationService.size()) + // one of the notifications it is the group notification only for devices the support it + assertEquals(2, shadowNotificationService.size()) } @Test @@ -575,17 +808,17 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - service.downloadJobs[providedDownload.value.id]?.job?.join() - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! + service.downloadJobs[providedDownload.value.state.id]?.job?.join() + val downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! service.setDownloadJobStatus(downloadJobState, COMPLETED) assertEquals(COMPLETED, service.getDownloadJobStatus(downloadJobState)) testDispatcher.advanceTimeBy(PROGRESS_UPDATE_INTERVAL) - assertEquals(1, shadowNotificationService.size()) + assertEquals(2, shadowNotificationService.size()) } @Test @@ -606,17 +839,18 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - service.downloadJobs[providedDownload.value.id]?.job?.join() - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! + service.downloadJobs[providedDownload.value.state.id]?.job?.join() + val downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! service.setDownloadJobStatus(downloadJobState, FAILED) assertEquals(FAILED, service.getDownloadJobStatus(downloadJobState)) testDispatcher.advanceTimeBy(PROGRESS_UPDATE_INTERVAL) - assertEquals(1, shadowNotificationService.size()) + // one of the notifications it is the group notification only for devices the support it + assertEquals(2, shadowNotificationService.size()) } @Test @@ -637,17 +871,18 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - service.downloadJobs[providedDownload.value.id]?.job?.join() - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! + service.downloadJobs[providedDownload.value.state.id]?.job?.join() + val downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! service.setDownloadJobStatus(downloadJobState, CANCELLED) assertEquals(CANCELLED, service.getDownloadJobStatus(downloadJobState)) testDispatcher.advanceTimeBy(PROGRESS_UPDATE_INTERVAL) - assertEquals(0, shadowNotificationService.size()) + // The additional notification is the summary one (the notification group). + assertEquals(1, shadowNotificationService.size()) } @Test @@ -662,10 +897,10 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! + val downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! assertEquals(FAILED, service.getDownloadJobStatus(downloadJobState)) } @@ -696,12 +931,13 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { assertTrue(it.job!!.isActive) } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) // Advance the clock so that the puller posts a notification. testDispatcher.advanceTimeBy(PROGRESS_UPDATE_INTERVAL) - assertEquals(1, shadowNotificationService.size()) + // One of the notifications it is the group notification only for devices the support it + assertEquals(2, shadowNotificationService.size()) // Now destroy service.onDestroy() @@ -735,26 +971,25 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) service.downloadJobs[download.id]?.status = PAUSED // Advance the clock so that the poller posts a notification. testDispatcher.advanceTimeBy(PROGRESS_UPDATE_INTERVAL) - assertEquals(1, shadowNotificationService.size()) + assertEquals(2, shadowNotificationService.size()) // Now simulate onTaskRemoved. service.onTaskRemoved(null) - // Assert that all currently shown notifications are gone. - assertEquals(0, shadowNotificationService.size()) + verify(service).stopSelf() } @Test fun `clearAllDownloadsNotificationsAndJobs cancels all running jobs and remove all notifications`() = runBlocking { val download = DownloadState(id = 1, url = "https://example.com/file.txt", fileName = "file.txt") - val downloadState = AbstractFetchDownloadService.DownloadJobState( + val downloadState = DownloadJobState( state = download, foregroundServiceId = Random.nextInt(), status = ACTIVE, @@ -784,7 +1019,7 @@ class AbstractFetchDownloadServiceTest { } @Test - fun `onTaskRemoved and onDestroy will remove all download notifications and jobs`() = runBlocking { + fun `onDestroy will remove all download notifications, jobs and will call unregisterNotificationActionsReceiver`() = runBlocking { val service = spy(object : AbstractFetchDownloadService() { override val httpClient = client }) @@ -792,18 +1027,15 @@ class AbstractFetchDownloadServiceTest { doReturn(testContext).`when`(service).context service.registerNotificationActionsReceiver() - service.onTaskRemoved(null) - - service.registerNotificationActionsReceiver() - verify(service).clearAllDownloadsNotificationsAndJobs() service.onDestroy() - verify(service, times(2)).clearAllDownloadsNotificationsAndJobs() + verify(service).clearAllDownloadsNotificationsAndJobs() + verify(service).unregisterNotificationActionsReceiver() } @Test - fun `register and unregister notification actions receiver`() = runBlocking { + fun `register and unregister notification actions receiver`() { val service = spy(object : AbstractFetchDownloadService() { override val httpClient = client }) @@ -814,15 +1046,9 @@ class AbstractFetchDownloadServiceTest { verify(service).registerNotificationActionsReceiver() - service.onTaskRemoved(null) - - verify(service).unregisterNotificationActionsReceiver() - - service.registerNotificationActionsReceiver() - service.onDestroy() - verify(service, times(2)).unregisterNotificationActionsReceiver() + verify(service).unregisterNotificationActionsReceiver() } @Test @@ -843,17 +1069,18 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(cancelledDownloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } - val providedDownload = argumentCaptor() + val providedDownload = argumentCaptor() verify(service).performDownload(providedDownload.capture()) - service.downloadJobs[providedDownload.value.id]?.job?.join() + service.downloadJobs[providedDownload.value.state.id]?.job?.join() - val cancelledDownloadJobState = service.downloadJobs[providedDownload.value.id]!! + val cancelledDownloadJobState = service.downloadJobs[providedDownload.value.state.id]!! service.setDownloadJobStatus(cancelledDownloadJobState, CANCELLED) assertEquals(CANCELLED, service.getDownloadJobStatus(cancelledDownloadJobState)) testDispatcher.advanceTimeBy(PROGRESS_UPDATE_INTERVAL) - assertEquals(0, shadowNotificationService.size()) + // The additional notification is the summary one (the notification group). + assertEquals(1, shadowNotificationService.size()) val download = DownloadState("https://example.com/file.txt", "file.txt") val downloadIntent = Intent("ACTION_DOWNLOAD").apply { @@ -864,19 +1091,20 @@ class AbstractFetchDownloadServiceTest { service.onStartCommand(downloadIntent, 0, 0) service.downloadJobs.values.forEach { it.job?.join() } verify(service, times(2)).performDownload(providedDownload.capture()) - service.downloadJobs[providedDownload.value.id]?.job?.join() + service.downloadJobs[providedDownload.value.state.id]?.job?.join() - val downloadJobState = service.downloadJobs[providedDownload.value.id]!! + val downloadJobState = service.downloadJobs[providedDownload.value.state.id]!! service.setDownloadJobStatus(downloadJobState, COMPLETED) assertEquals(COMPLETED, service.getDownloadJobStatus(downloadJobState)) testDispatcher.advanceTimeBy(PROGRESS_UPDATE_INTERVAL) - assertEquals(1, shadowNotificationService.size()) + // one of the notifications it is the group notification only for devices the support it + assertEquals(2, shadowNotificationService.size()) } @Test fun `keeps track of how many seconds have passed since the last update to a notification`() = runBlocking { - val downloadJobState = AbstractFetchDownloadService.DownloadJobState(state = mock(), status = ACTIVE) + val downloadJobState = DownloadJobState(state = mock(), status = ACTIVE) val oneSecond = 1000L downloadJobState.lastNotificationUpdate = System.currentTimeMillis() @@ -896,7 +1124,7 @@ class AbstractFetchDownloadServiceTest { @Test fun `is a notification under the time limit for updates`() = runBlocking { - val downloadJobState = AbstractFetchDownloadService.DownloadJobState(state = mock(), status = ACTIVE) + val downloadJobState = DownloadJobState(state = mock(), status = ACTIVE) val oneSecond = 1000L downloadJobState.lastNotificationUpdate = System.currentTimeMillis() @@ -910,7 +1138,7 @@ class AbstractFetchDownloadServiceTest { @Test fun `try to update a notification`() = runBlocking { - val downloadJobState = AbstractFetchDownloadService.DownloadJobState(state = mock(), status = ACTIVE) + val downloadJobState = DownloadJobState(state = mock(), status = ACTIVE) val oneSecond = 1000L downloadJobState.lastNotificationUpdate = System.currentTimeMillis() diff --git a/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadNotificationTest.kt b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadNotificationTest.kt new file mode 100644 index 00000000000..11a99b77578 --- /dev/null +++ b/components/feature/downloads/src/test/java/mozilla/components/feature/downloads/DownloadNotificationTest.kt @@ -0,0 +1,111 @@ +/* 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.os.Build +import androidx.core.app.NotificationCompat +import androidx.test.ext.junit.runners.AndroidJUnit4 +import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobState +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.PAUSED +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.FAILED +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.ACTIVE +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.CANCELLED +import mozilla.components.feature.downloads.AbstractFetchDownloadService.DownloadJobStatus.COMPLETED +import mozilla.components.support.test.mock +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.annotation.Config + +@RunWith(AndroidJUnit4::class) +class DownloadNotificationTest { + + @Test + fun getProgress() { + val downloadJobState = DownloadJobState( + job = null, + state = DownloadState(url = "mozilla.org/mozilla.txt", contentLength = 100L), + currentBytesCopied = 10, + status = ACTIVE, + foregroundServiceId = 1, + downloadDeleted = false + ) + + assertEquals("10%", downloadJobState.getProgress()) + + val newDownload = downloadJobState.copy(state = downloadJobState.state.copy(contentLength = null)) + + assertEquals("", newDownload.getProgress()) + } + + @Test + fun setCompatGroup() { + val notificationBuilder = NotificationCompat.Builder(testContext, "") + .setCompatGroup("myGroup").build() + + assertEquals("myGroup", notificationBuilder.group) + } + + @Test + @Config(sdk = [Build.VERSION_CODES.M]) + fun `setCompatGroup will not set the group`() { + val notificationBuilder = NotificationCompat.Builder(testContext, "") + .setCompatGroup("myGroup").build() + + assertNotEquals("myGroup", notificationBuilder.group) + } + + @Test + fun getStatusDescription() { + val pausedText = testContext.getString(R.string.mozac_feature_downloads_paused_notification_text) + val completedText = testContext.getString(R.string.mozac_feature_downloads_completed_notification_text2) + val failedText = testContext.getString(R.string.mozac_feature_downloads_failed_notification_text2) + var downloadJobState = DownloadJobState(state = mock(), status = ACTIVE) + + assertEquals(downloadJobState.getProgress(), downloadJobState.getStatusDescription(testContext)) + + downloadJobState = DownloadJobState(state = mock(), status = PAUSED) + + assertEquals(pausedText, downloadJobState.getStatusDescription(testContext)) + + downloadJobState = DownloadJobState(state = mock(), status = COMPLETED) + + assertEquals(completedText, downloadJobState.getStatusDescription(testContext)) + + downloadJobState = DownloadJobState(state = mock(), status = FAILED) + + assertEquals(failedText, downloadJobState.getStatusDescription(testContext)) + + downloadJobState = DownloadJobState(state = mock(), status = CANCELLED) + + assertEquals("", downloadJobState.getStatusDescription(testContext)) + } + + @Test + fun getDownloadSummary() { + val download1 = DownloadJobState( + job = null, + state = DownloadState(fileName = "mozilla.txt", url = "mozilla.org/mozilla.txt", contentLength = 100L), + currentBytesCopied = 10, + status = ACTIVE, + foregroundServiceId = 1, + downloadDeleted = false + ) + val download2 = DownloadJobState( + job = null, + state = DownloadState(fileName = "mozilla2.txt", url = "mozilla.org/mozilla.txt", contentLength = 100L), + currentBytesCopied = 20, + status = ACTIVE, + foregroundServiceId = 1, + downloadDeleted = false + ) + + val summary = DownloadNotification.getSummaryList(testContext, listOf(download1, download2)) + assertEquals(listOf("mozilla.txt 10%", "mozilla2.txt 20%"), summary) + } +} \ No newline at end of file diff --git a/docs/changelog.md b/docs/changelog.md index 14b3865462d..f2c15933d39 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -51,6 +51,8 @@ permalink: /changelog/ * **feature-downloads** * Fixed issue [#6881](https://github.com/mozilla-mobile/android-components/issues/6881). + * 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). * **feature-addons** * Added optional `addonAllowPrivateBrowsingLabelDrawableRes` DrawableRes parameter to `AddonPermissionsAdapter.Style` constructor to allow the clients to add their own drawable. This is used to clearly label the WebExtensions that run in private browsing.