Unify download notifications

Signed-off-by: alperozturk <alper_ozturk@proton.me>

app/src/main/java/com/nextcloud/client/files/downloader/DownloadNotificationManager.kt

@@ -31,30 +31,27 @@ import android.os.Build
 import android.os.Handler
 import android.os.Looper
 import androidx.core.app.NotificationCompat
-import com.nextcloud.client.account.User
 import com.owncloud.android.R
-import com.owncloud.android.authentication.AuthenticatorActivity
-import com.owncloud.android.datamodel.OCFile
-import com.owncloud.android.lib.common.operations.RemoteOperationResult
 import com.owncloud.android.lib.resources.files.FileUtils
 import com.owncloud.android.operations.DownloadFileOperation
 import com.owncloud.android.ui.notifications.NotificationUtils
-import com.owncloud.android.utils.ErrorMessageAdapter
 import com.owncloud.android.utils.theme.ViewThemeUtils
 import java.io.File
-import java.security.SecureRandom
 
 @Suppress("TooManyFunctions")
-class DownloadNotificationManager(private val context: Context, private val viewThemeUtils: ViewThemeUtils) {
+class DownloadNotificationManager(
+    private val id: Int,
+    private val context: Context,
+    private val viewThemeUtils: ViewThemeUtils
+) {
 
-    private var notification: Notification? = null
-    private lateinit var notificationBuilder: NotificationCompat.Builder
+    private var notification: Notification
+    private var notificationBuilder: NotificationCompat.Builder
     private val notificationManager = context.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
 
-    fun init() {
+    init {
         notificationBuilder = NotificationUtils.newNotificationBuilder(context, viewThemeUtils).apply {
             setContentTitle(context.resources.getString(R.string.app_name))
-            setContentText(context.resources.getString(R.string.worker_download))
             setSmallIcon(R.drawable.notification_icon)
             setLargeIcon(BitmapFactory.decodeResource(context.resources, R.drawable.notification_icon))
 
@@ -67,11 +64,9 @@ class DownloadNotificationManager(private val context: Context, private val view
     }
 
     @Suppress("MagicNumber")
-    fun notifyForStart(operation: DownloadFileOperation) {
+    fun prepareForStart(operation: DownloadFileOperation) {
         notificationBuilder = NotificationUtils.newNotificationBuilder(context, viewThemeUtils).apply {
             setSmallIcon(R.drawable.notification_icon)
-            setTicker(context.getString(R.string.downloader_download_in_progress_ticker))
-            setContentTitle(context.getString(R.string.downloader_download_in_progress_ticker))
             setOngoing(true)
             setProgress(100, 0, operation.size < 0)
             setContentText(
@@ -84,141 +79,52 @@ class DownloadNotificationManager(private val context: Context, private val view
             if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
                 setChannelId(NotificationUtils.NOTIFICATION_CHANNEL_DOWNLOAD)
             }
+
+            notificationManager.notify(
+                id,
+                this.build()
+            )
         }
     }
 
-    fun prepareForResult(
-        downloadResult: RemoteOperationResult<*>,
-        needsToUpdateCredentials: Boolean
-    ) {
-        val tickerId = getTickerId(downloadResult.isSuccess, needsToUpdateCredentials, null, null)
-
+    fun prepareForResult() {
         notificationBuilder
-            .setTicker(context.getString(tickerId))
-            .setContentTitle(context.getString(tickerId))
             .setAutoCancel(true)
             .setOngoing(false)
             .setProgress(0, 0, false)
     }
 
     @Suppress("MagicNumber")
-    fun notifyForResult(
-        result: RemoteOperationResult<*>?,
-        download: DownloadFileOperation?,
-        folder: OCFile?,
-        isAnyOperationFailed: Boolean?
-    ) {
-        dismissDownloadInProgressNotification()
-
-        val tickerId = getTickerId(result?.isSuccess, null, folder, isAnyOperationFailed)
-        val notifyId = SecureRandom().nextInt()
-        val resultText = getResultText(result, download, folder, isAnyOperationFailed)
-
+    fun updateDownloadProgress(filePath: String, percent: Int, totalToTransfer: Long) {
         notificationBuilder.run {
-            setTicker(context.getString(tickerId))
-            setContentText(resultText)
-            notificationManager.notify(notifyId, this.build())
-        }
-
-        NotificationUtils.cancelWithDelay(
-            notificationManager,
-            notifyId,
-            2000
-        )
-    }
-
-    private fun getResultText(
-        result: RemoteOperationResult<*>?,
-        download: DownloadFileOperation?,
-        folder: OCFile?,
-        isAnyOperationFailed: Boolean?
-    ): String {
-        return folder?.let {
-            getFolderResultText(isAnyOperationFailed, it)
-        } ?: if (result?.isSuccess == true) {
-            download?.file?.fileName ?: ""
-        } else {
-            ErrorMessageAdapter.getErrorCauseMessage(result, download, context.resources)
-        }
-    }
-
-    private fun getFolderResultText(isAnyOperationFailed: Boolean?, folder: OCFile): String {
-        return if (isAnyOperationFailed == false) {
-            context.getString(R.string.downloader_folder_downloaded, folder.fileName)
-        } else {
-            context.getString(R.string.downloader_folder_download_failed, folder.fileName)
-        }
-    }
-
-    private fun getTickerId(
-        isSuccess: Boolean?,
-        needsToUpdateCredentials: Boolean?,
-        folder: OCFile?,
-        isAnyOperationFailed: Boolean?
-    ): Int {
-        return if (needsToUpdateCredentials == true) {
-            R.string.downloader_download_failed_credentials_error
-        } else {
-            folder?.let { getFolderTickerId(isAnyOperationFailed) } ?: getFileTickerId(isSuccess)
-        }
-    }
-
-    private fun getFileTickerId(isSuccess: Boolean?): Int {
-        return if (isSuccess == true) {
-            R.string.downloader_download_succeeded_ticker
-        } else {
-            R.string.downloader_download_failed_ticker
-        }
-    }
-
-    private fun getFolderTickerId(isAnyOperationFailed: Boolean?): Int {
-        return if (isAnyOperationFailed == false) {
-            R.string.downloader_folder_downloaded
-        } else {
-            R.string.downloader_folder_download_failed
+            setProgress(100, percent, totalToTransfer < 0)
+            val fileName: String = filePath.substring(filePath.lastIndexOf(FileUtils.PATH_SEPARATOR) + 1)
+            val text =
+                String.format(context.getString(R.string.downloader_download_in_progress_content), percent, fileName)
+            updateNotificationText(text)
         }
     }
 
     @Suppress("MagicNumber")
-    fun updateDownloadProgressNotification(filePath: String, percent: Int, totalToTransfer: Long) {
-        notificationBuilder.setProgress(100, percent, totalToTransfer < 0)
-        val fileName: String = filePath.substring(filePath.lastIndexOf(FileUtils.PATH_SEPARATOR) + 1)
-        val text =
-            String.format(context.getString(R.string.downloader_download_in_progress_content), percent, fileName)
-        notificationBuilder.setContentText(text)
-    }
-
-    fun showDownloadInProgressNotification() {
-        notificationManager.notify(
-            R.string.downloader_download_in_progress_ticker,
-            notificationBuilder.build()
-        )
-    }
-
-    fun dismissDownloadInProgressNotification() {
-        notificationManager.cancel(R.string.downloader_download_in_progress_ticker)
-    }
-
-    @Suppress("MagicNumber")
-    fun dismissAll() {
+    fun showCompleteNotification(text: String) {
         Handler(Looper.getMainLooper()).postDelayed({
-            notificationManager.cancelAll()
+            updateNotificationText(text)
+            dismissNotification()
+        }, 3000)
+    }
+
+    @Suppress("MagicNumber")
+    fun dismissNotification() {
+        Handler(Looper.getMainLooper()).postDelayed({
+            notificationManager.cancel(id)
         }, 2000)
     }
 
-    fun setCredentialContentIntent(user: User) {
-        val intent = Intent(context, AuthenticatorActivity::class.java).apply {
-            putExtra(AuthenticatorActivity.EXTRA_ACCOUNT, user.toPlatformAccount())
-            putExtra(
-                AuthenticatorActivity.EXTRA_ACTION,
-                AuthenticatorActivity.ACTION_UPDATE_EXPIRED_TOKEN
-            )
-            addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
-            addFlags(Intent.FLAG_ACTIVITY_EXCLUDE_FROM_RECENTS)
-            addFlags(Intent.FLAG_FROM_BACKGROUND)
+    private fun updateNotificationText(text: String) {
+        notificationBuilder.run {
+            setContentText(text)
+            notificationManager.notify(id, this.build())
         }
-
-        setContentIntent(intent, PendingIntent.FLAG_ONE_SHOT or PendingIntent.FLAG_IMMUTABLE)
     }
 
     fun setContentIntent(intent: Intent, flag: Int) {

app/src/main/java/com/nextcloud/client/files/downloader/FileDownloadIntents.kt

@@ -23,6 +23,8 @@ package com.nextcloud.client.files.downloader
 
 import android.content.Context
 import android.content.Intent
+import com.nextcloud.client.account.User
+import com.owncloud.android.authentication.AuthenticatorActivity
 import com.owncloud.android.lib.common.operations.RemoteOperationResult
 import com.owncloud.android.operations.DownloadFileOperation
 import com.owncloud.android.ui.activity.FileActivity
@@ -65,6 +67,19 @@ class FileDownloadIntents(private val context: Context) {
         }
     }
 
+    fun credentialContentIntent(user: User): Intent {
+        return Intent(context, AuthenticatorActivity::class.java).apply {
+            putExtra(AuthenticatorActivity.EXTRA_ACCOUNT, user.toPlatformAccount())
+            putExtra(
+                AuthenticatorActivity.EXTRA_ACTION,
+                AuthenticatorActivity.ACTION_UPDATE_EXPIRED_TOKEN
+            )
+            addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+            addFlags(Intent.FLAG_ACTIVITY_EXCLUDE_FROM_RECENTS)
+            addFlags(Intent.FLAG_FROM_BACKGROUND)
+        }
+    }
+
     fun detailsIntent(operation: DownloadFileOperation?): Intent {
         return if (operation != null) {
             if (PreviewImageFragment.canBePreviewed(operation.file)) {

app/src/main/java/com/nextcloud/client/files/downloader/FileDownloadWorker.kt

@@ -36,9 +36,9 @@ import com.nextcloud.client.account.UserAccountManager
 import com.nextcloud.java.util.Optional
 import com.nextcloud.model.WorkerState
 import com.nextcloud.model.WorkerStateLiveData
+import com.owncloud.android.R
 import com.owncloud.android.datamodel.FileDataStorageManager
 import com.owncloud.android.datamodel.OCFile
-import com.owncloud.android.datamodel.UploadsStorageManager
 import com.owncloud.android.files.services.IndexedForest
 import com.owncloud.android.lib.common.OwnCloudAccount
 import com.owncloud.android.lib.common.OwnCloudClientManagerFactory
@@ -49,14 +49,14 @@ import com.owncloud.android.lib.common.utils.Log_OC
 import com.owncloud.android.operations.DownloadFileOperation
 import com.owncloud.android.operations.DownloadType
 import com.owncloud.android.utils.theme.ViewThemeUtils
+import java.security.SecureRandom
 import java.util.AbstractList
 import java.util.Vector
 
 @Suppress("LongParameterList", "TooManyFunctions")
 class FileDownloadWorker(
-    viewThemeUtils: ViewThemeUtils,
+    private val viewThemeUtils: ViewThemeUtils,
     private val accountManager: UserAccountManager,
-    private val uploadsStorageManager: UploadsStorageManager,
     private var localBroadcastManager: LocalBroadcastManager,
     private val context: Context,
     params: WorkerParameters
@@ -113,7 +113,7 @@ class FileDownloadWorker(
     private var lastPercent = 0
 
     private val intents = FileDownloadIntents(context)
-    private val notificationManager = DownloadNotificationManager(context, viewThemeUtils)
+    private val notificationManager = DownloadNotificationManager(SecureRandom().nextInt(), context, viewThemeUtils)
     private var downloadProgressListener = FileDownloadProgressListener()
 
     private var user: User? = null
@@ -123,28 +123,26 @@ class FileDownloadWorker(
     private var fileDataStorageManager: FileDataStorageManager? = null
 
     private var folder: OCFile? = null
-    private var isAnyOperationFailed = true
+    private var failedFileNames: ArrayList<String> = arrayListOf()
 
     @Suppress("TooGenericExceptionCaught")
     override fun doWork(): Result {
         return try {
             val requestDownloads = getRequestDownloads()
 
-            notificationManager.init()
             addAccountUpdateListener()
 
             requestDownloads.forEach {
                 downloadFile(it)
             }
 
-            folder?.let {
-                notifyForFolderResult(it)
-            }
+            showCompleteNotification()
 
             setIdleWorkerState()
             Log_OC.e(TAG, "FilesDownloadWorker successfully completed")
             Result.success()
         } catch (t: Throwable) {
+            notificationManager.showCompleteNotification(context.getString(R.string.downloader_unexpected_error))
             Log_OC.e(TAG, "Error caught at FilesDownloadWorker(): " + t.localizedMessage)
             Result.failure()
         }
@@ -155,7 +153,7 @@ class FileDownloadWorker(
 
         removePendingDownload(currentDownload?.user?.accountName)
         cancelAllDownloads()
-        notificationManager.dismissAll()
+        notificationManager.dismissNotification()
         setIdleWorkerState()
 
         super.onStopped()
@@ -166,6 +164,7 @@ class FileDownloadWorker(
     }
 
     private fun setIdleWorkerState() {
+        failedFileNames.clear()
         pendingDownloads.all.clear()
         currentDownload = null
         WorkerStateLiveData.instance().setWorkState(WorkerState.Idle)
@@ -181,8 +180,25 @@ class FileDownloadWorker(
         pendingDownloads.remove(accountName)
     }
 
-    private fun notifyForFolderResult(folder: OCFile) {
-        notificationManager.notifyForResult(null, null, folder, isAnyOperationFailed)
+    private fun showCompleteNotification() {
+        val result = if (failedFileNames.isEmpty()) {
+            getSuccessNotificationText()
+        } else {
+            val fileNames = failedFileNames.joinToString()
+            context.getString(R.string.downloader_files_download_failed, fileNames)
+        }
+
+        notificationManager.showCompleteNotification(result)
+    }
+
+    private fun getSuccessNotificationText(): String {
+        return if (folder != null) {
+            context.getString(R.string.downloader_folder_downloaded, folder?.fileName)
+        } else if (currentDownload?.file != null) {
+            context.getString(R.string.downloader_file_downloaded, currentDownload?.file?.fileName)
+        } else {
+            context.getString(R.string.downloader_download_completed)
+        }
     }
 
     private fun getRequestDownloads(): AbstractList<String> {
@@ -241,7 +257,7 @@ class FileDownloadWorker(
     private fun setFolder() {
         val folderPath = inputData.keyValueMap[FOLDER_REMOTE_PATH] as? String?
         if (folderPath != null) {
-            folder = currentUserFileStorageManager?.getFileByEncryptedRemotePath(folderPath)
+            folder = fileDataStorageManager?.getFileByEncryptedRemotePath(folderPath)
         }
     }
 
@@ -327,9 +343,8 @@ class FileDownloadWorker(
         lastPercent = 0
 
         notificationManager.run {
-            notifyForStart(download)
+            prepareForStart(download)
             setContentIntent(intents.detailsIntent(download), PendingIntent.FLAG_IMMUTABLE)
-            showDownloadInProgressNotification()
         }
     }
 
@@ -360,7 +375,7 @@ class FileDownloadWorker(
 
     private fun cleanupDownloadProcess(result: RemoteOperationResult<*>?) {
         result?.let {
-            isAnyOperationFailed = !it.isSuccess
+            checkOperationFailures(it)
         }
 
         val removeResult = pendingDownloads.removePayload(
@@ -383,6 +398,14 @@ class FileDownloadWorker(
         }
     }
 
+    private fun checkOperationFailures(result: RemoteOperationResult<*>) {
+        if (!result.isSuccess) {
+            currentDownload?.file?.fileName?.let { fileName ->
+                failedFileNames.add(fileName)
+            }
+        }
+    }
+
     private fun notifyDownloadResult(
         download: DownloadFileOperation,
         downloadResult: RemoteOperationResult<*>
@@ -391,38 +414,21 @@ class FileDownloadWorker(
             return
         }
 
-        // TODO Check why we calling only for success?
-        if (downloadResult.isSuccess) {
-            dismissDownloadInProgressNotification()
-        }
-
         val needsToUpdateCredentials = (ResultCode.UNAUTHORIZED == downloadResult.code)
         notificationManager.run {
-            prepareForResult(downloadResult, needsToUpdateCredentials)
+            prepareForResult()
 
             if (needsToUpdateCredentials) {
-                setCredentialContentIntent(download.user)
+                setContentIntent(
+                    intents.credentialContentIntent(download.user),
+                    PendingIntent.FLAG_ONE_SHOT or PendingIntent.FLAG_IMMUTABLE
+                )
             } else {
                 setContentIntent(intents.detailsIntent(null), PendingIntent.FLAG_IMMUTABLE)
             }
-
-            if (folder == null) {
-                notifyForResult(downloadResult, download, null, null)
-            }
         }
     }
 
-    private fun dismissDownloadInProgressNotification() {
-        // TODO Check necessity of this function call
-        conflictUploadId?.let {
-            if (it > 0) {
-                uploadsStorageManager.removeUpload(it)
-            }
-        }
-
-        notificationManager.dismissDownloadInProgressNotification()
-    }
-
     override fun onAccountsUpdated(accounts: Array<out Account>?) {
         if (!accountManager.exists(currentDownload?.user?.toPlatformAccount())) {
             currentDownload?.cancel()
@@ -440,8 +446,7 @@ class FileDownloadWorker(
 
         if (percent != lastPercent) {
             notificationManager.run {
-                updateDownloadProgressNotification(filePath, percent, totalToTransfer)
-                showDownloadInProgressNotification()
+                updateDownloadProgress(filePath, percent, totalToTransfer)
             }
         }
 

app/src/main/java/com/nextcloud/client/jobs/BackgroundJobFactory.kt

@@ -259,7 +259,6 @@ class BackgroundJobFactory @Inject constructor(
         return FileDownloadWorker(
             viewThemeUtils.get(),
             accountManager,
-            uploadsStorageManager,
             localBroadcastManager.get(),
             context,
             params

app/src/main/res/values/strings.xml

@@ -168,14 +168,18 @@
     <string name="uploads_view_later_waiting_to_upload">Waiting to upload</string>
     <string name="uploads_view_group_header" translatable="false">%1$s (%2$d)</string>
     <string name="downloader_download_in_progress_ticker">Downloading…</string>
+    <string name="downloader_download_completed">Downloads are completed</string>
     <string name="downloader_download_in_progress_content">%1$d%% Downloading %2$s</string>
     <string name="downloader_download_succeeded_ticker">Downloaded</string>
     <string name="downloader_download_succeeded_content">%1$s downloaded</string>
     <string name="downloader_download_failed_ticker">Download failed</string>
     <string name="downloader_download_failed_content">Could not download %1$s</string>
     <string name="downloader_not_downloaded_yet">Not downloaded yet</string>
+    <string name="downloader_files_download_failed">Error occurred while downloading %s files</string>
     <string name="downloader_folder_download_failed">Error occurred while downloading %s folder</string>
     <string name="downloader_folder_downloaded">%s folder successfully downloaded</string>
+    <string name="downloader_file_downloaded">%s file successfully downloaded</string>
+    <string name="downloader_unexpected_error">Unexpected error occurred while downloading files</string>
     <string name="downloader_download_failed_credentials_error">Download failed, log in again</string>
     <string name="common_choose_account">Choose account</string>
     <string name="common_switch_account">Switch account</string>