Updated service architecture and extracted useful tools to separate class

app/src/main/java/a/k.kt

@@ -1,5 +1,5 @@
 package a
 
-import com.topjohnwu.magisk.model.download.CompoundDownloadService
+import com.topjohnwu.magisk.model.download.DownloadService
 
-class k : CompoundDownloadService()
+class k : DownloadService()

app/src/main/java/com/topjohnwu/magisk/ClassMap.kt

@@ -1,7 +1,7 @@
 package com.topjohnwu.magisk
 
-import com.topjohnwu.magisk.model.download.CompoundDownloadService
 import com.topjohnwu.magisk.model.download.DownloadModuleService
+import com.topjohnwu.magisk.model.download.DownloadService
 import com.topjohnwu.magisk.model.receiver.GeneralReceiver
 import com.topjohnwu.magisk.model.update.UpdateCheckService
 import com.topjohnwu.magisk.ui.MainActivity
@@ -18,7 +18,7 @@ object ClassMap {
         UpdateCheckService::class.java to a.g::class.java,
         GeneralReceiver::class.java to a.h::class.java,
         DownloadModuleService::class.java to a.j::class.java,
-        CompoundDownloadService::class.java to a.k::class.java,
+        DownloadService::class.java to a.k::class.java,
         SuRequestActivity::class.java to a.m::class.java
     )
 

app/src/main/java/com/topjohnwu/magisk/model/download/DownloadService.kt

@@ -6,22 +6,39 @@ import android.app.PendingIntent
 import android.content.Context
 import android.content.Intent
 import android.os.Build
+import android.webkit.MimeTypeMap
+import android.widget.Toast
 import androidx.annotation.RequiresPermission
 import androidx.core.app.NotificationCompat
 import com.topjohnwu.magisk.ClassMap
+import com.topjohnwu.magisk.Const
+import com.topjohnwu.magisk.R
 import com.topjohnwu.magisk.model.entity.internal.Configuration.*
 import com.topjohnwu.magisk.model.entity.internal.Configuration.Flash.Secondary
 import com.topjohnwu.magisk.model.entity.internal.DownloadSubject
 import com.topjohnwu.magisk.model.entity.internal.DownloadSubject.Magisk
 import com.topjohnwu.magisk.model.entity.internal.DownloadSubject.Module
 import com.topjohnwu.magisk.ui.flash.FlashActivity
+import com.topjohnwu.magisk.utils.Utils
+import com.topjohnwu.magisk.utils.provide
 import java.io.File
 import kotlin.random.Random.Default.nextInt
 
+/* More of a facade for [RemoteFileService], but whatever... */
 @SuppressLint("Registered")
-open class CompoundDownloadService : SubstrateDownloadService() {
+open class DownloadService : RemoteFileService() {
 
     private val context get() = this
+    private val String.downloadsFile get() = File(Const.EXTERNAL_PATH, this)
+    private val File.type
+        get() = MimeTypeMap.getSingleton()
+            .getMimeTypeFromExtension(extension)
+            .orEmpty()
+
+    override fun map(subject: DownloadSubject, file: File): File = when (subject) {
+        is Module -> file // todo tbd
+        else -> file
+    }
 
     override fun onFinished(file: File, subject: DownloadSubject) = when (subject) {
         is Magisk -> onFinishedInternal(file, subject)
@@ -83,19 +100,44 @@ open class CompoundDownloadService : SubstrateDownloadService() {
         PendingIntent.getActivity(context, nextInt(), intent, PendingIntent.FLAG_ONE_SHOT)
             .let { setContentIntent(it) }
 
+    // ---
+
+    private fun moveToDownloads(file: File) {
+        val destination = file.name.downloadsFile
+        if (file != destination) {
+            destination.deleteRecursively()
+            file.copyTo(destination)
+        }
+        Utils.toast(
+            getString(R.string.internal_storage, "/Download/${file.name}"),
+            Toast.LENGTH_LONG
+        )
+    }
+
+    private fun fileIntent(fileName: String): Intent {
+        val file = fileName.downloadsFile
+        return Intent(Intent.ACTION_VIEW)
+            .setDataAndType(file.provide(this), file.type)
+            .addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
+    }
+
+    class Builder {
+        lateinit var subject: DownloadSubject
+    }
+
     companion object {
 
         @RequiresPermission(allOf = [Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.WRITE_EXTERNAL_STORAGE])
-        fun download(context: Context, subject: DownloadSubject) {
+        inline operator fun invoke(context: Context, argBuilder: Builder.() -> Unit) {
-            Intent(context, ClassMap[CompoundDownloadService::class.java])
+            val builder = Builder().apply(argBuilder)
-                .putExtra(ARG_URL, subject)
+            val intent = Intent(context, ClassMap[DownloadService::class.java])
-                .let {
+                .putExtra(ARG_URL, builder.subject)
-                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+
-                        context.startForegroundService(it)
+            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-                    } else {
+                context.startForegroundService(intent)
-                        context.startService(it)
+            } else {
-                    }
+                context.startService(intent)
-                }
+            }
         }
 
     }

app/src/main/java/com/topjohnwu/magisk/model/download/NotificationService.kt

@@ -0,0 +1,86 @@
+package com.topjohnwu.magisk.model.download
+
+import android.app.Notification
+import android.app.NotificationManager
+import android.app.Service
+import android.content.Intent
+import android.os.IBinder
+import androidx.core.app.NotificationCompat
+import androidx.core.content.getSystemService
+import java.util.*
+import kotlin.random.Random.Default.nextInt
+
+abstract class NotificationService : Service() {
+
+    abstract val defaultNotification: NotificationCompat.Builder
+
+    private val manager get() = getSystemService<NotificationManager>()
+    private val hasNotifications get() = notifications.isEmpty()
+
+    private val notifications =
+        Collections.synchronizedMap(mutableMapOf<Int, NotificationCompat.Builder>())
+
+    override fun onTaskRemoved(rootIntent: Intent?) {
+        super.onTaskRemoved(rootIntent)
+        notifications.values.forEach { cancel(it.hashCode()) }
+        notifications.clear()
+    }
+
+    // --
+
+    protected fun update(
+        id: Int,
+        body: (NotificationCompat.Builder) -> Unit = {}
+    ) {
+        val notification = notifications.getOrPut(id) { defaultNotification }
+
+        notify(id, notification.also(body).build())
+
+        if (notifications.size == 1) {
+            updateForeground()
+        }
+    }
+
+    protected fun finishWork(
+        id: Int,
+        editBody: (NotificationCompat.Builder) -> NotificationCompat.Builder? = { null }
+    ) {
+        val currentNotification = remove(id)?.run(editBody) ?: let {
+            cancel(id)
+            return
+        }
+
+        updateForeground()
+
+        cancel(id)
+        notify(nextInt(), currentNotification.build())
+
+        if (!hasNotifications) {
+            stopForeground(true)
+            stopSelf()
+        }
+    }
+
+    // ---
+
+    private fun notify(id: Int, notification: Notification) {
+        manager?.notify(id, notification)
+    }
+
+    private fun cancel(id: Int) {
+        manager?.cancel(id)
+    }
+
+    private fun remove(id: Int) = notifications.remove(id)
+        .also { updateForeground() }
+
+    private fun updateForeground() {
+        runCatching { notifications.keys.first() to notifications.values.first() }
+            .getOrNull()
+            ?.let { startForeground(it.first, it.second.build()) }
+    }
+
+    // --
+
+    override fun onBind(p0: Intent?): IBinder? = null
+}

app/src/main/java/com/topjohnwu/magisk/model/download/RemoteFileService.kt

@@ -1,21 +1,13 @@
 package com.topjohnwu.magisk.model.download
 
-import android.app.NotificationManager
-import android.app.Service
 import android.content.Intent
-import android.os.IBinder
-import android.webkit.MimeTypeMap
-import android.widget.Toast
 import androidx.core.app.NotificationCompat
-import androidx.core.content.getSystemService
 import com.skoumal.teanity.extensions.subscribeK
 import com.topjohnwu.magisk.Config
 import com.topjohnwu.magisk.Const
 import com.topjohnwu.magisk.R
 import com.topjohnwu.magisk.data.repository.FileRepository
 import com.topjohnwu.magisk.model.entity.internal.DownloadSubject
-import com.topjohnwu.magisk.utils.Utils
-import com.topjohnwu.magisk.utils.provide
 import com.topjohnwu.magisk.utils.writeToCachedFile
 import com.topjohnwu.magisk.view.Notifications
 import com.topjohnwu.superuser.ShellUtils
@@ -24,35 +16,26 @@ import okhttp3.ResponseBody
 import org.koin.android.ext.android.inject
 import timber.log.Timber
 import java.io.File
-import java.util.*
-import kotlin.random.Random.Default.nextInt
 
-abstract class SubstrateDownloadService : Service() {
+abstract class RemoteFileService : NotificationService() {
 
     private val repo by inject<FileRepository>()
-    private val manager get() = getSystemService<NotificationManager>()
 
-    private val notifications =
+    override val defaultNotification: NotificationCompat.Builder
-        Collections.synchronizedMap(mutableMapOf<Int, NotificationCompat.Builder>())
+        get() = Notifications
-
+            .progress(this, "")
-    override fun onBind(p0: Intent?): IBinder? = null
+            .setContentText(getString(R.string.download_local))
 
     override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
         intent?.getParcelableExtra<DownloadSubject>(ARG_URL)?.let { start(it) }
         return START_REDELIVER_INTENT
     }
 
-    override fun onTaskRemoved(rootIntent: Intent?) {
-        super.onTaskRemoved(rootIntent)
-        notifications.values.forEach { manager?.cancel(it.hashCode()) }
-        notifications.clear()
-    }
-
     // ---
 
     private fun start(subject: DownloadSubject) = search(subject)
         .onErrorResumeNext(download(subject))
-        .doOnSubscribe { updateNotification(subject.hashCode()) { it.setContentTitle(subject.fileName) } }
+        .doOnSubscribe { update(subject.hashCode()) { it.setContentTitle(subject.fileName) } }
         .subscribeK {
             runCatching { onFinished(it, subject) }.onFailure { Timber.e(it) }
             finish(it, subject)
@@ -84,45 +67,18 @@ abstract class SubstrateDownloadService : Service() {
 
     private fun download(subject: DownloadSubject) = repo.downloadFile(subject.url)
         .map { it.toFile(subject.hashCode(), subject.fileName) }
+        .map { map(subject, it) }
 
     // ---
 
-    protected fun fileIntent(fileName: String): Intent {
-        val file = downloadsFile(fileName)
-        return Intent(Intent.ACTION_VIEW)
-            .setDataAndType(file.provide(this), file.type)
-            .addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
-    }
-
-    protected fun moveToDownloads(file: File) {
-        val destination = downloadsFile(file.name)
-        if (file != destination) {
-            destination.deleteRecursively()
-            file.copyTo(destination)
-        }
-        Utils.toast(
-            getString(R.string.internal_storage, "/Download/${file.name}"),
-            Toast.LENGTH_LONG
-        )
-    }
-
-    // ---
-
-    private val File.type
-        get() = MimeTypeMap.getSingleton()
-            .getMimeTypeFromExtension(extension)
-            .orEmpty()
-
-    private fun downloadsFile(name: String) = File(Const.EXTERNAL_PATH, name)
-
     private fun ResponseBody.toFile(id: Int, name: String): File {
         val maxRaw = contentLength()
         val max = maxRaw / 1_000_000f
 
-        return writeToCachedFile(this@SubstrateDownloadService, name) {
+        return writeToCachedFile(this@RemoteFileService, name) {
             val progress = it / 1_000_000f
 
-            updateNotification(id) { notification ->
+            update(id) { notification ->
                 notification
                     .setProgress(maxRaw.toInt(), it.toInt(), false)
                     .setContentText(getString(R.string.download_progress, progress, max))
@@ -130,51 +86,13 @@ abstract class SubstrateDownloadService : Service() {
         }
     }
 
-    private fun finish(file: File, subject: DownloadSubject) {
+    private fun finish(file: File, subject: DownloadSubject) = finishWork(subject.hashCode()) {
-        val currentNotification = notifications.remove(subject.hashCode()) ?: let {
+        it.addActions(file, subject)
-            manager?.cancel(subject.hashCode())
-            return
-        }
-        val notification = currentNotification.addActions(file, subject)
             .setContentText(getString(R.string.download_complete))
             .setSmallIcon(android.R.drawable.stat_sys_download_done)
             .setProgress(0, 0, false)
             .setOngoing(false)
             .setAutoCancel(true)
-            .build()
-
-        updateForeground()
-
-        manager?.cancel(subject.hashCode())
-        manager?.notify(nextInt(), notification)
-
-        if (notifications.isEmpty()) {
-            stopForeground(true)
-            stopSelf()
-        }
-    }
-
-    private inline fun updateNotification(
-        id: Int,
-        body: (NotificationCompat.Builder) -> Unit = {}
-    ) {
-        val notification = notifications.getOrPut(id) {
-            Notifications
-                .progress(this, "")
-                .setContentText(getString(R.string.download_local))
-        }
-
-        manager?.notify(id, notification.also(body).build())
-
-        if (notifications.size == 1) {
-            updateForeground()
-        }
-    }
-
-    private fun updateForeground() {
-        runCatching { notifications.keys.first() to notifications.values.first() }
-            .getOrNull()
-            ?.let { startForeground(it.first, it.second.build()) }
     }
 
     // ---
@@ -188,6 +106,7 @@ abstract class SubstrateDownloadService : Service() {
         subject: DownloadSubject
     ): NotificationCompat.Builder
 
+    protected abstract fun map(subject: DownloadSubject, file: File): File
 
     companion object {
         const val ARG_URL = "arg_url"

app/src/main/java/com/topjohnwu/magisk/ui/module/ReposFragment.kt

@@ -10,7 +10,7 @@ import com.skoumal.teanity.viewevents.ViewEvent
 import com.topjohnwu.magisk.Config
 import com.topjohnwu.magisk.R
 import com.topjohnwu.magisk.databinding.FragmentReposBinding
-import com.topjohnwu.magisk.model.download.CompoundDownloadService
+import com.topjohnwu.magisk.model.download.DownloadService
 import com.topjohnwu.magisk.model.entity.Repo
 import com.topjohnwu.magisk.model.entity.internal.Configuration
 import com.topjohnwu.magisk.model.entity.internal.DownloadSubject
@@ -96,12 +96,11 @@ class ReposFragment : MagiskFragment<ModuleViewModel, FragmentReposBinding>(),
     private fun installModule(item: Repo) {
         val context = magiskActivity
 
-        fun download(install: Boolean) {
+        fun download(install: Boolean) = context.withExternalRW {
-            context.withExternalRW {
+            onSuccess {
-                onSuccess {
+                DownloadService(context) {
                     val config = if (install) Configuration.Flash() else Configuration.Download
-                    val subject = DownloadSubject.Module(item, config)
+                    subject = DownloadSubject.Module(item, config)
-                    CompoundDownloadService.download(context, subject)
                 }
             }
         }

app/src/main/java/com/topjohnwu/magisk/view/dialogs/InstallMethodDialog.kt

@@ -8,7 +8,7 @@ import android.widget.Toast
 import androidx.appcompat.app.AlertDialog
 import com.topjohnwu.magisk.Const
 import com.topjohnwu.magisk.R
-import com.topjohnwu.magisk.model.download.CompoundDownloadService
+import com.topjohnwu.magisk.model.download.DownloadService
 import com.topjohnwu.magisk.model.entity.internal.Configuration
 import com.topjohnwu.magisk.model.entity.internal.DownloadSubject
 import com.topjohnwu.magisk.ui.base.MagiskActivity
@@ -32,8 +32,9 @@ internal class InstallMethodDialog(activity: MagiskActivity<*, *>, options: List
     @SuppressLint("MissingPermission")
     private fun flash(activity: MagiskActivity<*, *>) = activity.withExternalRW {
         onSuccess {
-            val subject = DownloadSubject.Magisk(Configuration.Flash.Primary)
+            DownloadService(context) {
-            CompoundDownloadService.download(context, subject)
+                subject = DownloadSubject.Magisk(Configuration.Flash.Primary)
+            }
         }
     }
 
@@ -46,9 +47,10 @@ internal class InstallMethodDialog(activity: MagiskActivity<*, *>, options: List
                 .addCategory(Intent.CATEGORY_OPENABLE)
             activity.startActivityForResult(intent, Const.ID.SELECT_BOOT) { resultCode, data ->
                 if (resultCode == Activity.RESULT_OK && data != null) {
-                    val safeData = data.data ?: Uri.EMPTY
+                    DownloadService(activity) {
-                    val subject = DownloadSubject.Magisk(Configuration.Patch(safeData))
+                        val safeData = data.data ?: Uri.EMPTY
-                    CompoundDownloadService.download(activity, subject)
+                        subject = DownloadSubject.Magisk(Configuration.Patch(safeData))
+                    }
                 }
             }
         }
@@ -57,8 +59,9 @@ internal class InstallMethodDialog(activity: MagiskActivity<*, *>, options: List
     @SuppressLint("MissingPermission")
     private fun downloadOnly(activity: MagiskActivity<*, *>) = activity.withExternalRW {
         onSuccess {
-            val subject = DownloadSubject.Magisk(Configuration.Download)
+            DownloadService(activity) {
-            CompoundDownloadService.download(activity, subject)
+                subject = DownloadSubject.Magisk(Configuration.Download)
+            }
         }
     }
 
@@ -69,8 +72,9 @@ internal class InstallMethodDialog(activity: MagiskActivity<*, *>, options: List
             .setMessage(R.string.install_inactive_slot_msg)
             .setCancelable(true)
             .setPositiveButton(R.string.yes) { _, _ ->
-                val subject = DownloadSubject.Magisk(Configuration.Flash.Secondary)
+                DownloadService(activity) {
-                CompoundDownloadService.download(activity, subject)
+                    subject = DownloadSubject.Magisk(Configuration.Flash.Secondary)
+                }
             }
             .setNegativeButton(R.string.no_thanks, null)
             .show()