From 8e32d4469bb161f2dc5cffdd5391676580485a9d Mon Sep 17 00:00:00 2001 From: shabinder Date: Sun, 26 Sep 2021 00:44:33 +0530 Subject: [PATCH] Performance, Background Crashes and Notification Cancellation Fixes --- android/build.gradle.kts | 1 + .../com/shabinder/spotiflyer/MainActivity.kt | 10 +- .../spotiflyer/service/ForegroundService.kt | 48 ++++-- .../spotiflyer/service/TrackStatusFlowMap.kt | 14 +- .../common/uikit/screens/SpotiFlyerRootUi.kt | 2 +- common/core-components/build.gradle.kts | 2 +- .../parallel_executor/ParallelExecutor.kt | 4 +- .../kotlin/com/shabinder/common/utils/Ext.kt | 15 +- .../list/integration/SpotiFlyerListImpl.kt | 2 +- .../list/store/SpotiFlyerListStoreProvider.kt | 153 +++++++++++------- .../main/integration/SpotiFlyerMainImpl.kt | 2 +- .../main/store/SpotiFlyerMainStoreProvider.kt | 1 + 12 files changed, 165 insertions(+), 89 deletions(-) diff --git a/android/build.gradle.kts b/android/build.gradle.kts index 7c1889b2..f37792e1 100644 --- a/android/build.gradle.kts +++ b/android/build.gradle.kts @@ -126,6 +126,7 @@ dependencies { with(Versions.androidxLifecycle) { implementation("androidx.lifecycle:lifecycle-service:$this") implementation("androidx.lifecycle:lifecycle-common-java8:$this") + implementation("androidx.lifecycle:lifecycle-runtime-ktx:$this") } implementation(Extras.kermit) diff --git a/android/src/main/java/com/shabinder/spotiflyer/MainActivity.kt b/android/src/main/java/com/shabinder/spotiflyer/MainActivity.kt index 0e97a424..51452b8f 100644 --- a/android/src/main/java/com/shabinder/spotiflyer/MainActivity.kt +++ b/android/src/main/java/com/shabinder/spotiflyer/MainActivity.kt @@ -40,7 +40,9 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalView import androidx.core.content.ContextCompat import androidx.core.view.WindowCompat +import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.repeatOnLifecycle import com.arkivanov.decompose.ComponentContext import com.arkivanov.decompose.defaultComponentContext import com.arkivanov.mvikotlin.logging.store.LoggingStoreFactory @@ -71,10 +73,12 @@ import com.shabinder.spotiflyer.ui.AnalyticsDialog import com.shabinder.spotiflyer.ui.NetworkDialog import com.shabinder.spotiflyer.ui.PermissionDialog import com.shabinder.spotiflyer.utils.* +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.conflate import kotlinx.coroutines.flow.emitAll +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.launch import org.koin.android.ext.android.inject import org.koin.core.parameter.parametersOf @@ -204,8 +208,10 @@ class MainActivity : ComponentActivity() { foregroundService = binder.service isServiceBound = true lifecycleScope.launch { - foregroundService?.trackStatusFlowMap?.statusFlow?.let { - trackStatusFlow.emitAll(it.conflate()) + repeatOnLifecycle(Lifecycle.State.STARTED) { + foregroundService?.trackStatusFlowMap?.statusFlow?.let { + trackStatusFlow.emitAll(it.conflate().flowOn(Dispatchers.Default)) + } } } } diff --git a/android/src/main/java/com/shabinder/spotiflyer/service/ForegroundService.kt b/android/src/main/java/com/shabinder/spotiflyer/service/ForegroundService.kt index 87de6ada..81386757 100644 --- a/android/src/main/java/com/shabinder/spotiflyer/service/ForegroundService.kt +++ b/android/src/main/java/com/shabinder/spotiflyer/service/ForegroundService.kt @@ -21,7 +21,6 @@ import android.app.Notification import android.app.NotificationChannel import android.app.NotificationManager import android.app.PendingIntent -import android.app.PendingIntent.FLAG_CANCEL_CURRENT import android.content.Context import android.content.Intent import android.os.Binder @@ -44,8 +43,6 @@ import com.shabinder.common.models.event.coroutines.failure import com.shabinder.common.providers.FetchPlatformQueryResult import com.shabinder.common.translations.Strings import com.shabinder.spotiflyer.R -import com.shabinder.spotiflyer.utils.autoclear.autoClear -import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.MutableSharedFlow @@ -57,12 +54,11 @@ import java.io.File class ForegroundService : LifecycleService() { private lateinit var downloadService: ParallelExecutor - val trackStatusFlowMap by autoClear { - TrackStatusFlowMap( - MutableSharedFlow(replay = 1), - lifecycleScope - ) - } + val trackStatusFlowMap = TrackStatusFlowMap( + MutableSharedFlow(replay = 1), + lifecycleScope + ) + private val fetcher: FetchPlatformQueryResult by inject() private val logger: Kermit by inject() private val dir: FileManager by inject() @@ -73,7 +69,12 @@ class ForegroundService : LifecycleService() { private var isServiceStarted = false private val cancelIntent: PendingIntent by lazy { val intent = Intent(this, ForegroundService::class.java).apply { action = "kill" } - PendingIntent.getService(this, 0, intent, FLAG_CANCEL_CURRENT) + val flags = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE + } else { + PendingIntent.FLAG_UPDATE_CURRENT + } + PendingIntent.getService(this, 0, intent, flags) } /* Variables Holding Download State */ @@ -98,6 +99,7 @@ class ForegroundService : LifecycleService() { override fun onCreate() { super.onCreate() downloadService = ParallelExecutor(Dispatchers.IO) + trackStatusFlowMap.scope = lifecycleScope createNotificationChannel(CHANNEL_ID, "Downloader Service") } @@ -271,12 +273,16 @@ class ForegroundService : LifecycleService() { private fun killService() { lifecycleScope.launch { logger.d(TAG) { "Killing Self" } + resetVar() messageList = messageList.getEmpty().apply { set(index = 0, Message(Strings.cleaningAndExiting(), DownloadStatus.NotDownloaded)) } downloadService.close() updateNotification() - trackStatusFlowMap.clear() + trackStatusFlowMap.apply { + clear() + scope = null + } cleanFiles(File(dir.defaultDir())) // cleanFiles(File(dir.imageCacheDir())) messageList = messageList.getEmpty() @@ -290,6 +296,13 @@ class ForegroundService : LifecycleService() { } } + private fun resetVar() { + total = 0 + downloaded = 0 + failed = 0 + converted = 0 + } + private fun createNotification(): Notification = NotificationCompat.Builder(this, CHANNEL_ID).run { setSmallIcon(R.drawable.ic_download_arrow) @@ -323,6 +336,7 @@ class ForegroundService : LifecycleService() { updateNotification() } + @Suppress("unused") private fun updateProgressInNotification(message: Message) { synchronized(messageList) { val index = messageList.indexOfFirst { it.title == message.title } @@ -331,10 +345,16 @@ class ForegroundService : LifecycleService() { updateNotification() } + // Update Notification only if Service is Still Active private fun updateNotification() { - val mNotificationManager: NotificationManager = - getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager - mNotificationManager.notify(NOTIFICATION_ID, createNotification()) + if (!downloadService.isClosed.value) { + val mNotificationManager: NotificationManager = + getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager + mNotificationManager.notify(NOTIFICATION_ID, createNotification()) + } else { + // Service is Inactive so clear status + resetVar() + } } override fun onDestroy() { diff --git a/android/src/main/java/com/shabinder/spotiflyer/service/TrackStatusFlowMap.kt b/android/src/main/java/com/shabinder/spotiflyer/service/TrackStatusFlowMap.kt index c93b38d6..4cb31814 100644 --- a/android/src/main/java/com/shabinder/spotiflyer/service/TrackStatusFlowMap.kt +++ b/android/src/main/java/com/shabinder/spotiflyer/service/TrackStatusFlowMap.kt @@ -7,12 +7,12 @@ import kotlinx.coroutines.launch class TrackStatusFlowMap( val statusFlow: MutableSharedFlow>, - private val scope: CoroutineScope + var scope: CoroutineScope? ) : HashMap() { override fun put(key: String, value: DownloadStatus): DownloadStatus? { synchronized(this) { val res = super.put(key, value) - emitValue() + emitValue(this) return res } } @@ -25,13 +25,13 @@ class TrackStatusFlowMap( super.put(title,DownloadStatus.NotDownloaded) } } - emitValue() - //super.clear() - //emitValue() + emitValue(this) + super.clear() + emitValue(this) } } - private fun emitValue() { - scope.launch { statusFlow.emit(this@TrackStatusFlowMap) } + private fun emitValue(map: HashMap) { + scope?.launch { statusFlow.emit(map) } } } diff --git a/common/compose/src/commonMain/kotlin/com/shabinder/common/uikit/screens/SpotiFlyerRootUi.kt b/common/compose/src/commonMain/kotlin/com/shabinder/common/uikit/screens/SpotiFlyerRootUi.kt index 4b14a6ee..32811760 100644 --- a/common/compose/src/commonMain/kotlin/com/shabinder/common/uikit/screens/SpotiFlyerRootUi.kt +++ b/common/compose/src/commonMain/kotlin/com/shabinder/common/uikit/screens/SpotiFlyerRootUi.kt @@ -63,7 +63,7 @@ import com.shabinder.common.uikit.screens.splash.Splash import com.shabinder.common.uikit.screens.splash.SplashState import com.shabinder.common.uikit.utils.verticalGradientScrim -// To Not Show Splash Again After Configuration Change in Android +// Splash Status private var isSplashShown = SplashState.Show @Composable diff --git a/common/core-components/build.gradle.kts b/common/core-components/build.gradle.kts index f67ee2c0..9a618a93 100644 --- a/common/core-components/build.gradle.kts +++ b/common/core-components/build.gradle.kts @@ -10,7 +10,7 @@ kotlin { dependencies { implementation(project(":common:data-models")) implementation(project(":common:database")) - implementation("org.jetbrains.kotlinx:atomicfu:0.16.2") + api("org.jetbrains.kotlinx:atomicfu:0.16.2") api(MultiPlatformSettings.dep) implementation(MVIKotlin.rx) } diff --git a/common/core-components/src/commonMain/kotlin/com.shabinder.common.core_components/parallel_executor/ParallelExecutor.kt b/common/core-components/src/commonMain/kotlin/com.shabinder.common.core_components/parallel_executor/ParallelExecutor.kt index f8cd76a2..923859c9 100644 --- a/common/core-components/src/commonMain/kotlin/com.shabinder.common.core_components/parallel_executor/ParallelExecutor.kt +++ b/common/core-components/src/commonMain/kotlin/com.shabinder.common.core_components/parallel_executor/ParallelExecutor.kt @@ -62,7 +62,8 @@ class ParallelExecutor( private var service: Job = SupervisorJob() override val coroutineContext get() = context + service - private var isClosed = atomic(false) + var isClosed = atomic(false) + private set private var killQueue = Channel(Channel.UNLIMITED) private var operationQueue = Channel>(Channel.RENDEZVOUS) private var concurrentOperationLimit = atomic(concurrentOperationLimit) @@ -132,6 +133,7 @@ class ParallelExecutor( } // TODO This launches all coroutines in advance even if they're never needed. Find a lazy way to do this. + @Suppress("unused") fun setConcurrentOperationLimit(limit: Int) { require(limit >= 1) { "'limit' must be greater than zero: $limit" } require(limit < 1_000_000) { "Don't use a very high limit because it will cause a lot of coroutines to be started eagerly: $limit" } diff --git a/common/data-models/src/commonMain/kotlin/com/shabinder/common/utils/Ext.kt b/common/data-models/src/commonMain/kotlin/com/shabinder/common/utils/Ext.kt index 6164033f..ba992418 100644 --- a/common/data-models/src/commonMain/kotlin/com/shabinder/common/utils/Ext.kt +++ b/common/data-models/src/commonMain/kotlin/com/shabinder/common/utils/Ext.kt @@ -1,6 +1,10 @@ package com.shabinder.common.utils import com.shabinder.common.models.TrackDetails +import com.shabinder.common.models.dispatcherIO +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import kotlin.contracts.ExperimentalContracts import kotlin.contracts.InvocationKind import kotlin.contracts.contract @@ -22,4 +26,13 @@ fun StringBuilder.appendPadded(data: Any?) { fun StringBuilder.appendPadded(header: Any?, data: Any?) { appendLine().append(header).appendLine(data).appendLine() -} \ No newline at end of file +} + +suspend fun runOnMain(block: suspend CoroutineScope.() -> T): T = + withContext(Dispatchers.Main, block) + +suspend fun runOnIO(block: suspend CoroutineScope.() -> T): T = + withContext(dispatcherIO, block) + +suspend fun runOnDefault(block: suspend CoroutineScope.() -> T): T = + withContext(Dispatchers.Default, block) diff --git a/common/list/src/commonMain/kotlin/com/shabinder/common/list/integration/SpotiFlyerListImpl.kt b/common/list/src/commonMain/kotlin/com/shabinder/common/list/integration/SpotiFlyerListImpl.kt index f73e2340..768daaad 100644 --- a/common/list/src/commonMain/kotlin/com/shabinder/common/list/integration/SpotiFlyerListImpl.kt +++ b/common/list/src/commonMain/kotlin/com/shabinder/common/list/integration/SpotiFlyerListImpl.kt @@ -50,7 +50,7 @@ internal class SpotiFlyerListImpl( private val cache = Cache.Builder .newBuilder() - .maximumCacheSize(75) + .maximumCacheSize(30) .build() override val model: Value = store.asValue() diff --git a/common/list/src/commonMain/kotlin/com/shabinder/common/list/store/SpotiFlyerListStoreProvider.kt b/common/list/src/commonMain/kotlin/com/shabinder/common/list/store/SpotiFlyerListStoreProvider.kt index c2faa540..a6162851 100644 --- a/common/list/src/commonMain/kotlin/com/shabinder/common/list/store/SpotiFlyerListStoreProvider.kt +++ b/common/list/src/commonMain/kotlin/com/shabinder/common/list/store/SpotiFlyerListStoreProvider.kt @@ -23,9 +23,16 @@ import com.arkivanov.mvikotlin.extensions.coroutines.SuspendExecutor import com.shabinder.common.list.SpotiFlyerList import com.shabinder.common.list.SpotiFlyerList.State import com.shabinder.common.list.store.SpotiFlyerListStore.Intent -import com.shabinder.common.models.* +import com.shabinder.common.models.Actions +import com.shabinder.common.models.DownloadStatus +import com.shabinder.common.models.PlatformQueryResult +import com.shabinder.common.models.TrackDetails import com.shabinder.common.providers.downloadTracks +import com.shabinder.common.utils.runOnDefault +import com.shabinder.common.utils.runOnMain +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.withContext internal class SpotiFlyerListStoreProvider(dependencies: SpotiFlyerList.Dependencies) : SpotiFlyerList.Dependencies by dependencies { @@ -41,7 +48,11 @@ internal class SpotiFlyerListStoreProvider(dependencies: SpotiFlyerList.Dependen ) {} private sealed class Result { - data class ResultFetched(val result: PlatformQueryResult, val trackList: List) : Result() + data class ResultFetched( + val result: PlatformQueryResult, + val trackList: List + ) : Result() + data class UpdateTrackList(val list: List) : Result() data class UpdateTrackItem(val item: TrackDetails) : Result() data class ErrorOccurred(val error: Throwable) : Result() @@ -52,79 +63,102 @@ internal class SpotiFlyerListStoreProvider(dependencies: SpotiFlyerList.Dependen override suspend fun executeAction(action: Unit, getState: () -> State) { executeIntent(Intent.SearchLink(link), getState) + runOnDefault { + fileManager.db?.downloadRecordDatabaseQueries?.getLastInsertId() + ?.executeAsOneOrNull()?.also { + // See if It's Time we can request for support for maintaining this project or not + fetchQuery.logger.d( + message = { "Database List Last ID: $it" }, + tag = "Database Last ID" + ) + val offset = preferenceManager.getDonationOffset + dispatchOnMain( + Result.AskForSupport( + // Every 3rd Interval or After some offset + isAllowed = offset < 4 && (it % offset == 0L) + ) + ) + } - fileManager.db?.downloadRecordDatabaseQueries?.getLastInsertId()?.executeAsOneOrNull()?.also { - // See if It's Time we can request for support for maintaining this project or not - fetchQuery.logger.d(message = { "Database List Last ID: $it" }, tag = "Database Last ID") - val offset = preferenceManager.getDonationOffset - dispatch( - Result.AskForSupport( - // Every 3rd Interval or After some offset - isAllowed = offset < 4 && (it % offset == 0L) - ) - ) - } - - downloadProgressFlow.collect { map -> - // logger.d(map.size.toString(), "ListStore: flow Updated") - val updatedTrackList = getState().trackList.updateTracksStatuses(map) - if (updatedTrackList.isNotEmpty()) dispatch(Result.UpdateTrackList(updatedTrackList)) + downloadProgressFlow.collect { map -> + // logger.d(map.size.toString(), "ListStore: flow Updated") + getState().trackList.updateTracksStatuses(map).also { + if (it.isNotEmpty()) + dispatchOnMain(Result.UpdateTrackList(it)) + } + } } } override suspend fun executeIntent(intent: Intent, getState: () -> State) { - when (intent) { - is Intent.SearchLink -> { - val resp = fetchQuery.query(link) - resp.fold( - success = { result -> - result.trackList = result.trackList.toMutableList() - dispatch( - (Result.ResultFetched( - result, - result.trackList.updateTracksStatuses(downloadProgressFlow.replayCache.getOrElse(0) { hashMapOf() }) - )) - ) - executeIntent(Intent.RefreshTracksStatuses, getState) - }, - failure = { - dispatch(Result.ErrorOccurred(it)) - } - ) - } + withContext(Dispatchers.Default) { + when (intent) { + is Intent.SearchLink -> { + val resp = fetchQuery.query(link) + resp.fold( + success = { result -> + result.trackList = + result.trackList.toMutableList() + .updateTracksStatuses( + downloadProgressFlow.replayCache.getOrElse(0) { hashMapOf() } + ) - is Intent.StartDownloadAll -> { - val list = intent.trackList.map { - if (it.downloaded is DownloadStatus.NotDownloaded || it.downloaded is DownloadStatus.Failed) - return@map it.copy(downloaded = DownloadStatus.Queued) - it - } - dispatch( - Result.UpdateTrackList( - list.updateTracksStatuses( - downloadProgressFlow.replayCache.getOrElse( - 0 - ) { hashMapOf() }) + dispatchOnMain( + (Result.ResultFetched( + result, + result.trackList + )) + ) + executeIntent(Intent.RefreshTracksStatuses, getState) + }, + failure = { + dispatchOnMain(Result.ErrorOccurred(it)) + } ) - ) + } - val finalList = intent.trackList.filter { it.downloaded == DownloadStatus.NotDownloaded } - if (finalList.isEmpty()) Actions.instance.showPopUpMessage("All Songs are Processed") - else downloadTracks(finalList, fetchQuery, fileManager) + is Intent.StartDownloadAll -> { + val list = intent.trackList.map { + if (it.downloaded is DownloadStatus.NotDownloaded || it.downloaded is DownloadStatus.Failed) + return@map it.copy(downloaded = DownloadStatus.Queued) + it + } + dispatchOnMain( + Result.UpdateTrackList( + list.updateTracksStatuses( + downloadProgressFlow.replayCache.getOrElse( + 0 + ) { hashMapOf() }) + ) + ) + + val finalList = + intent.trackList.filter { it.downloaded == DownloadStatus.NotDownloaded } + if (finalList.isEmpty()) Actions.instance.showPopUpMessage("All Songs are Processed") + else downloadTracks(finalList, fetchQuery, fileManager) + } + + is Intent.StartDownload -> { + dispatchOnMain(Result.UpdateTrackItem(intent.track.copy(downloaded = DownloadStatus.Queued))) + downloadTracks(listOf(intent.track), fetchQuery, fileManager) + } + + is Intent.RefreshTracksStatuses -> Actions.instance.queryActiveTracks() } - is Intent.StartDownload -> { - dispatch(Result.UpdateTrackItem(intent.track.copy(downloaded = DownloadStatus.Queued))) - downloadTracks(listOf(intent.track), fetchQuery, fileManager) - } - is Intent.RefreshTracksStatuses -> Actions.instance.queryActiveTracks() } } + + private suspend fun dispatchOnMain(result: Result) = runOnMain { dispatch(result) } } private object ReducerImpl : Reducer { override fun State.reduce(result: Result): State = when (result) { - is Result.ResultFetched -> copy(queryResult = result.result, trackList = result.trackList, link = link) + is Result.ResultFetched -> copy( + queryResult = result.result, + trackList = result.trackList, + link = link + ) is Result.UpdateTrackList -> copy(trackList = result.list) is Result.UpdateTrackItem -> updateTrackItem(result.item) is Result.ErrorOccurred -> copy(errorOccurred = result.error) @@ -158,7 +192,6 @@ internal class SpotiFlyerListStoreProvider(dependencies: SpotiFlyerList.Dependen } } } - return updatedList } } diff --git a/common/main/src/commonMain/kotlin/com/shabinder/common/main/integration/SpotiFlyerMainImpl.kt b/common/main/src/commonMain/kotlin/com/shabinder/common/main/integration/SpotiFlyerMainImpl.kt index f94eea91..df6bc142 100644 --- a/common/main/src/commonMain/kotlin/com/shabinder/common/main/integration/SpotiFlyerMainImpl.kt +++ b/common/main/src/commonMain/kotlin/com/shabinder/common/main/integration/SpotiFlyerMainImpl.kt @@ -49,7 +49,7 @@ internal class SpotiFlyerMainImpl( private val cache = Cache.Builder .newBuilder() - .maximumCacheSize(25) + .maximumCacheSize(20) .build() override val model: Value = store.asValue() diff --git a/common/main/src/commonMain/kotlin/com/shabinder/common/main/store/SpotiFlyerMainStoreProvider.kt b/common/main/src/commonMain/kotlin/com/shabinder/common/main/store/SpotiFlyerMainStoreProvider.kt index 21efcc88..87e7908a 100644 --- a/common/main/src/commonMain/kotlin/com/shabinder/common/main/store/SpotiFlyerMainStoreProvider.kt +++ b/common/main/src/commonMain/kotlin/com/shabinder/common/main/store/SpotiFlyerMainStoreProvider.kt @@ -25,6 +25,7 @@ import com.shabinder.common.main.SpotiFlyerMain.State import com.shabinder.common.main.store.SpotiFlyerMainStore.Intent import com.shabinder.common.models.DownloadRecord import com.shabinder.common.models.Actions +import com.shabinder.common.utils.runOnMain import com.squareup.sqldelight.runtime.coroutines.asFlow import com.squareup.sqldelight.runtime.coroutines.mapToList import kotlinx.coroutines.Dispatchers