Skip to content

Commit 581acc3

Browse files
Tobi823Tobi823
committed
- simplify network code to reduce the risk of network problems
- for tracking the download progress: track the network InputStream instead of using an OkHttp network interceptor - only use HTTP 1.1 to prevent network problems (thanks TeeTeeHaa for the multiple and helpful debugging sessions #629) - execute the download in DownloadActivity inside the viewModelScope. This keeps the download running when the screen is rotated (#625)
1 parent b2abea6 commit 581acc3

File tree

8 files changed

+132
-252
lines changed

8 files changed

+132
-252
lines changed

‎ffupdater/src/main/java/de/marmaro/krt/ffupdater/activity/download/DownloadActivity.kt

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import de.marmaro.krt.ffupdater.utils.visibleAfterExecution
4444
import de.marmaro.krt.ffupdater.utils.visibleDuringExecution
4545
import kotlinx.coroutines.Deferred
4646
import kotlinx.coroutines.Dispatchers
47+
import kotlinx.coroutines.async
4748
import kotlinx.coroutines.channels.Channel
4849
import kotlinx.coroutines.launch
4950
import kotlinx.coroutines.withContext
@@ -83,7 +84,7 @@ class DownloadActivity : AppCompatActivity() {
8384
}
8485

8586
fun isDownloadForCurrentAppRunning(status: InstalledAppStatus): Boolean {
86-
return this.status == status && deferred?.isActive == true
87+
return this.status?.app == status.app && this.status?.latestVersion == status.latestVersion && deferred?.isActive == true
8788
}
8889

8990
fun clear() {
@@ -282,7 +283,12 @@ class DownloadActivity : AppCompatActivity() {
282283
private suspend fun reuseCurrentDownload(status: InstalledAppStatus): Boolean {
283284
debug("start reusing existing download")
284285
return try {
285-
reuseCurrentDownloadWithoutErrorChecking(status)
286+
findViewById<View>(R.id.downloadingFile).visibleDuringExecution {
287+
findViewById<View>(R.id.downloadedFile).visibleAfterExecution {
288+
reuseCurrentDownloadWithoutErrorChecking(status)
289+
}
290+
}
291+
286292
true
287293
} catch (e: Exception) {
288294
debug("reusing the existing download of $[app.name} failed", e)
@@ -297,19 +303,29 @@ class DownloadActivity : AppCompatActivity() {
297303
debug("reuse existing download")
298304
gui.setText(R.id.downloadingFileUrl, status.latestVersion.downloadUrl)
299305
gui.setText(R.id.downloadingFileText, getString(download_activity__download_app_with_status))
300-
301306
findViewById<View>(R.id.downloadingFile).visibleDuringExecution {
302-
gui.showDownloadProgress(downloadViewModel.progressChannel!!)
303-
// NPE was thrown in https://github.com/Tobi823/ffupdater/issues/359 - it should be safe to ignore null values
304-
downloadViewModel.deferred?.await()
307+
for (update in downloadViewModel.progressChannel!!) {
308+
withContext(Dispatchers.Main) {
309+
gui.updateDownloadProgressIndication(update)
310+
}
311+
}
305312
}
313+
downloadViewModel.deferred!!.await()
306314
}
307315

308316
@MainThread
309317
private suspend fun startDownload(status: InstalledAppStatus): Boolean {
310-
debug("start download (1/3)")
318+
debug("start download (1/2)")
311319
return try {
312-
startDownloadWithoutErrorChecking(status)
320+
gui.setText(R.id.downloadingFileUrl, status.latestVersion.downloadUrl)
321+
gui.setText(R.id.downloadedFileUrl, status.latestVersion.downloadUrl)
322+
gui.setText(R.id.downloadingFileText, getString(download_activity__download_app_with_status))
323+
findViewById<View>(R.id.downloadingFile).visibleDuringExecution {
324+
findViewById<View>(R.id.downloadedFile).visibleAfterExecution {
325+
downloadWithUiProgressIndication(status)
326+
}
327+
}
328+
true
313329
} catch (e: Exception) {
314330
debug("download failed", e)
315331
if (e !is DisplayableException) throw e
@@ -319,30 +335,24 @@ class DownloadActivity : AppCompatActivity() {
319335
}
320336

321337
@MainThread
322-
private suspend fun startDownloadWithoutErrorChecking(status: InstalledAppStatus): Boolean {
323-
debug("start downloading (2/3)")
324-
gui.setText(R.id.downloadingFileUrl, status.latestVersion.downloadUrl)
325-
gui.setText(R.id.downloadedFileUrl, status.latestVersion.downloadUrl)
326-
gui.setText(R.id.downloadingFileText, getString(download_activity__download_app_with_status))
327-
findViewById<View>(R.id.downloadingFile).visibleDuringExecution {
328-
findViewById<View>(R.id.downloadedFile).visibleAfterExecution {
329-
startDownloadInternal(status)
338+
private suspend fun downloadWithUiProgressIndication(status: InstalledAppStatus) {
339+
debug("start downloading (2/2)")
340+
341+
val viewModelDownload = downloadViewModel.viewModelScope.launch {
342+
val channel = Channel<DownloadStatus>()
343+
val download = async {
344+
appImpl.download(applicationContext, status.latestVersion, channel)
330345
}
346+
downloadViewModel.storeNewRunningDownload(status, download, channel)
331347
}
332-
return true
333-
}
334348

335-
@MainThread
336-
private suspend fun startDownloadInternal(status: InstalledAppStatus) {
337-
debug("start downloading (3/3)")
338-
val appImpl = app.findImpl()
339-
val coroutineContext = downloadViewModel.viewModelScope.coroutineContext
340-
withContext(coroutineContext) {
341-
appImpl.download(applicationContext, status.latestVersion) { deferred, progressChannel ->
342-
downloadViewModel.storeNewRunningDownload(status, deferred, progressChannel)
343-
gui.showDownloadProgress(progressChannel)
349+
for (update in downloadViewModel.progressChannel!!) {
350+
withContext(Dispatchers.Main) {
351+
gui.updateDownloadProgressIndication(update)
344352
}
345353
}
354+
355+
viewModelDownload.join()
346356
}
347357

348358
@MainThread

‎ffupdater/src/main/java/de/marmaro/krt/ffupdater/activity/download/GuiHelper.kt

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import de.marmaro.krt.ffupdater.network.exceptions.NetworkNotSuitableException
1818
import de.marmaro.krt.ffupdater.network.file.DownloadStatus
1919
import de.marmaro.krt.ffupdater.settings.ForegroundSettings
2020
import de.marmaro.krt.ffupdater.settings.InstallerSettings
21-
import kotlinx.coroutines.channels.Channel
2221

2322
class GuiHelper(val activity: DownloadActivity) {
2423

@@ -81,20 +80,17 @@ class GuiHelper(val activity: DownloadActivity) {
8180
}
8281
}
8382

84-
suspend fun showDownloadProgress(progressChannel: Channel<DownloadStatus>) {
85-
for (progress in progressChannel) {
86-
if (progress.progressInPercent != null) {
87-
activity.findViewById<ProgressBar>(R.id.downloadingFileProgressBar).progress =
88-
progress.progressInPercent
89-
}
83+
fun updateDownloadProgressIndication(progress: DownloadStatus) {
84+
if (progress.progressInPercent != null) {
85+
activity.findViewById<ProgressBar>(R.id.downloadingFileProgressBar).progress = progress.progressInPercent
86+
}
9087

91-
val text = when {
92-
progress.progressInPercent != null -> "(${progress.progressInPercent}%)"
93-
else -> "(${progress.totalMB}MB)"
94-
}
95-
val statusText = activity.getString(R.string.download_activity__download_app_with_status)
96-
setText(R.id.downloadingFileText, "$statusText $text")
88+
val text = when {
89+
progress.progressInPercent != null -> "(${progress.progressInPercent}%)"
90+
else -> "(${progress.totalMB}MB)"
9791
}
92+
val statusText = activity.getString(R.string.download_activity__download_app_with_status)
93+
setText(R.id.downloadingFileText, "$statusText $text")
9894
}
9995

10096
@MainThread

‎ffupdater/src/main/java/de/marmaro/krt/ffupdater/app/impl/base/ApkDownloader.kt

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import de.marmaro.krt.ffupdater.network.exceptions.NetworkException
1111
import de.marmaro.krt.ffupdater.network.file.DownloadStatus
1212
import de.marmaro.krt.ffupdater.network.file.FileDownloader
1313
import de.marmaro.krt.ffupdater.storage.StorageUtil
14-
import kotlinx.coroutines.Deferred
1514
import kotlinx.coroutines.Dispatchers
1615
import kotlinx.coroutines.channels.Channel
1716
import kotlinx.coroutines.withContext
@@ -22,10 +21,10 @@ import java.util.zip.ZipFile
2221
@Keep
2322
interface ApkDownloader : AppAttributes {
2423

25-
suspend fun <R> download(
24+
suspend fun download(
2625
context: Context,
2726
latestVersion: LatestVersion,
28-
progress: suspend (Deferred<Any>, Channel<DownloadStatus>) -> R,
27+
progress: Channel<DownloadStatus>,
2928
) {
3029
val downloadFile = getDownloadFile(context.applicationContext)
3130
downloadFile.delete()
@@ -53,8 +52,7 @@ interface ApkDownloader : AppAttributes {
5352
suspend fun deleteFileCache(context: Context) {
5453
withContext(Dispatchers.IO) {
5554
getApkCacheFolder(context.applicationContext).listFiles()!!
56-
.filter { it.name.startsWith("${getSanitizedPackageName()}_") }
57-
.filter { it.name.endsWith(".apk") }
55+
.filter { it.name.startsWith("${getSanitizedPackageName()}_") }.filter { it.name.endsWith(".apk") }
5856
.forEach {
5957
Log.d(LOG_TAG, "ApkDownloader: Delete cached APK of ${app.name} (${it.absolutePath}).")
6058
it.delete()
@@ -65,10 +63,8 @@ interface ApkDownloader : AppAttributes {
6563
suspend fun deleteFileCacheExceptLatest(context: Context, latestVersion: LatestVersion) {
6664
withContext(Dispatchers.IO) {
6765
val latestFile = getApkFile(context.applicationContext, latestVersion)
68-
getApkCacheFolder(context.applicationContext).listFiles()!!
69-
.filter { it != latestFile }
70-
.filter { it.name.startsWith("${getSanitizedPackageName()}_") }
71-
.filter { it.name.endsWith(".apk") }
66+
getApkCacheFolder(context.applicationContext).listFiles()!!.filter { it != latestFile }
67+
.filter { it.name.startsWith("${getSanitizedPackageName()}_") }.filter { it.name.endsWith(".apk") }
7268
.also { if (it.isNotEmpty()) Log.i(LOG_TAG, "ApkDownloader: Delete older files from ${app.name}") }
7369
.forEach { it.delete() }
7470
}
@@ -90,20 +86,13 @@ interface ApkDownloader : AppAttributes {
9086
return sanitizedVersionText + dateOrEmptyString
9187
}
9288

93-
private suspend fun <R> download(
89+
private suspend fun download(
9490
context: Context,
9591
latestVersion: LatestVersion,
9692
downloadFile: File,
97-
progress: suspend (Deferred<Any>, Channel<DownloadStatus>) -> R,
93+
progress: Channel<DownloadStatus>,
9894
) {
99-
val (deferred, progressChannel) = FileDownloader.downloadFileWithProgress(
100-
latestVersion.downloadUrl,
101-
downloadFile
102-
)
103-
withContext(Dispatchers.Main) {
104-
progress(deferred, progressChannel)
105-
}
106-
deferred.await()
95+
FileDownloader.downloadFileWithProgress(latestVersion.downloadUrl, downloadFile, progress)
10796
checkDownloadFile(downloadFile, latestVersion)
10897
processDownload(context.applicationContext, downloadFile, latestVersion)
10998
}

‎ffupdater/src/main/java/de/marmaro/krt/ffupdater/background/AppUpdater.kt

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ import de.marmaro.krt.ffupdater.settings.DataStoreHelper
3737
import de.marmaro.krt.ffupdater.settings.InstallerSettings
3838
import de.marmaro.krt.ffupdater.storage.StorageUtil
3939
import kotlinx.coroutines.CompletableDeferred
40+
import kotlinx.coroutines.Dispatchers
41+
import kotlinx.coroutines.async
42+
import kotlinx.coroutines.channels.Channel
43+
import kotlinx.coroutines.coroutineScope
44+
import kotlinx.coroutines.withContext
4045
import java.time.Duration.ofDays
4146
import kotlin.Result.Companion.failure
4247
import kotlin.Result.Companion.success
@@ -106,7 +111,9 @@ class AppUpdater(context: Context, workerParams: WorkerParameters) : CoroutineWo
106111
logInfo("Start downloading ${app.name}")
107112
showDownloadRunningNotification(applicationContext, app, null, null)
108113
val result = downloadApp(installedAppStatus) {
109-
showDownloadRunningNotification(applicationContext, app, it.progressInPercent, it.totalMB)
114+
withContext(Dispatchers.Main) {
115+
showDownloadRunningNotification(applicationContext, app, it.progressInPercent, it.totalMB)
116+
}
110117
}
111118
removeDownloadRunningNotification(applicationContext, app)
112119
return result
@@ -187,20 +194,27 @@ class AppUpdater(context: Context, workerParams: WorkerParameters) : CoroutineWo
187194
}
188195

189196
private suspend fun downloadApp(
190-
installedAppStatus: InstalledAppStatus,
191-
onUpdate: (DownloadStatus) -> Unit
197+
installedAppStatus: InstalledAppStatus, onUpdate: suspend (DownloadStatus) -> Unit
192198
): kotlin.Result<Boolean> {
193199
logInfo("Download update for ${installedAppStatus.app}.")
194200
val appImpl = installedAppStatus.app.findImpl()
195201
try {
196-
appImpl.download(applicationContext, installedAppStatus.latestVersion) { _, progressChannel ->
202+
coroutineScope {
203+
logInfo("Download update for ${installedAppStatus.app}.")
204+
val progress = Channel<DownloadStatus>()
205+
val download = async {
206+
appImpl.download(applicationContext, installedAppStatus.latestVersion, progress)
207+
}
208+
197209
var lastTime = System.currentTimeMillis()
198-
for (progress in progressChannel) {
199-
if ((System.currentTimeMillis() - lastTime) >= 1000) {
210+
for (update in progress) {
211+
if ((System.currentTimeMillis() - lastTime) >= UPDATE_NOTIFICATION_ONLY_AFTER_MS) {
200212
lastTime = System.currentTimeMillis()
201-
onUpdate(progress)
213+
onUpdate(update)
202214
}
203215
}
216+
217+
download.await()
204218
}
205219
} catch (e: Exception) {
206220
return failure(e)
@@ -298,6 +312,7 @@ class AppUpdater(context: Context, workerParams: WorkerParameters) : CoroutineWo
298312
private const val CLASS_LOGTAG = "AppUpdater:"
299313
private const val MAX_RETRIES = 5 // total of 15.5min waiting time (30s + 60s + 120s + 240s + 480s)
300314
private val ERROR_IGNORE_TIMESPAN = ofDays(2)
315+
private const val UPDATE_NOTIFICATION_ONLY_AFTER_MS = 3000
301316

302317
fun createWorkRequest(app: App): OneTimeWorkRequest {
303318
val data = Data.Builder().putString(APP_NAME_KEY, app.name).build()

‎ffupdater/src/main/java/de/marmaro/krt/ffupdater/network/file/DownloadProgressInterceptor.kt

Lines changed: 0 additions & 22 deletions
This file was deleted.

0 commit comments

Comments
 (0)