From 9f41116241c7cbf568458d9aab026fcbf553d957 Mon Sep 17 00:00:00 2001 From: tom5079 Date: Mon, 22 Jun 2020 09:48:04 +0900 Subject: [PATCH 1/2] Auto-Retry Bug fix --- app/build.gradle | 4 +- .../quaver/pupil/ExampleInstrumentedTest.kt | 2 +- .../quaver/pupil/adapters/ReaderAdapter.kt | 39 ++------ .../java/xyz/quaver/pupil/ui/MainActivity.kt | 4 +- .../xyz/quaver/pupil/ui/ReaderActivity.kt | 9 +- .../xyz/quaver/pupil/util/download/Cache.kt | 13 ++- .../pupil/util/download/DownloadWorker.kt | 88 ++++--------------- app/src/main/res/values-ko/strings.xml | 2 +- app/src/main/res/values/strings.xml | 2 +- 9 files changed, 46 insertions(+), 117 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 5d545827..a5e7ece0 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -19,8 +19,8 @@ android { applicationId "xyz.quaver.pupil" minSdkVersion 16 targetSdkVersion 29 - versionCode 54 - versionName "4.18.1" + versionCode 55 + versionName "4.18.2" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" multiDexEnabled true vectorDrawables.useSupportLibrary = true diff --git a/app/src/androidTest/java/xyz/quaver/pupil/ExampleInstrumentedTest.kt b/app/src/androidTest/java/xyz/quaver/pupil/ExampleInstrumentedTest.kt index d747e481..b3661f2f 100644 --- a/app/src/androidTest/java/xyz/quaver/pupil/ExampleInstrumentedTest.kt +++ b/app/src/androidTest/java/xyz/quaver/pupil/ExampleInstrumentedTest.kt @@ -99,7 +99,7 @@ class ExampleInstrumentedTest { while(worker.progress.indexOfKey(galleryID) < 0 || worker.progress[galleryID] != null) { Log.i("PUPILD", worker.progress[galleryID]?.joinToString(" ") ?: "null") - if (worker.progress[galleryID]?.all { !it.isFinite() } == true) + if (worker.progress[galleryID]?.all { it.isInfinite() } == true) break } diff --git a/app/src/main/java/xyz/quaver/pupil/adapters/ReaderAdapter.kt b/app/src/main/java/xyz/quaver/pupil/adapters/ReaderAdapter.kt index a6505838..97157dda 100644 --- a/app/src/main/java/xyz/quaver/pupil/adapters/ReaderAdapter.kt +++ b/app/src/main/java/xyz/quaver/pupil/adapters/ReaderAdapter.kt @@ -18,6 +18,7 @@ package xyz.quaver.pupil.adapters +import android.app.Activity import android.view.LayoutInflater import android.view.View import android.view.ViewGroup @@ -28,9 +29,6 @@ import com.bumptech.glide.RequestManager import com.bumptech.glide.load.engine.DiskCacheStrategy import com.bumptech.glide.load.model.GlideUrl import com.bumptech.glide.load.model.LazyHeaders -import com.google.android.material.snackbar.Snackbar -import com.google.firebase.crashlytics.FirebaseCrashlytics -import kotlinx.android.synthetic.main.activity_reader.view.* import kotlinx.android.synthetic.main.item_reader.view.* import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -50,7 +48,8 @@ import kotlin.concurrent.schedule import kotlin.math.roundToInt class ReaderAdapter(private val glide: RequestManager, - private val galleryID: Int) : RecyclerView.Adapter() { + private val galleryID: Int, + private val activity: Activity) : RecyclerView.Adapter() { var reader: Reader? = null val timer = Timer() @@ -149,33 +148,13 @@ class ReaderAdapter(private val glide: RequestManager, glide.clear(holder.view.image) - if (progress?.isNaN() == true) { - FirebaseCrashlytics.getInstance().recordException( - DownloadWorker.getInstance(holder.view.context).exception[galleryID]?.get(position)!! - ) + holder.view.reader_item_progressbar.progress = + if (progress?.isInfinite() == true) + 100 + else + progress?.roundToInt() ?: 0 - glide - .load(R.drawable.image_broken_variant) - .into(holder.view.image) - - Snackbar.make(holder.view.reader_layout, R.string.reader_error_retry, Snackbar.LENGTH_SHORT).apply { - setAction(android.R.string.no) { } - setAction(android.R.string.yes) { - downloadWorker!!.cancel(galleryID) - downloadWorker!!.queue.add(galleryID) - } - }.show() - - return - } else { - holder.view.reader_item_progressbar.progress = - if (progress?.isInfinite() == true) - 100 - else - progress?.roundToInt() ?: 0 - - holder.view.image.setImageDrawable(null) - } + holder.view.image.setImageDrawable(null) timer.schedule(1000) { CoroutineScope(Dispatchers.Main).launch { diff --git a/app/src/main/java/xyz/quaver/pupil/ui/MainActivity.kt b/app/src/main/java/xyz/quaver/pupil/ui/MainActivity.kt index 2a1aa3b0..4af3f620 100644 --- a/app/src/main/java/xyz/quaver/pupil/ui/MainActivity.kt +++ b/app/src/main/java/xyz/quaver/pupil/ui/MainActivity.kt @@ -440,8 +440,10 @@ class MainActivity : AppCompatActivity() { if (PreferenceManager.getDefaultSharedPreferences(context).getBoolean("cache_disable", false)) Toast.makeText(context, R.string.settings_download_when_cache_disable_warning, Toast.LENGTH_SHORT).show() else { - if (Cache(context).isDownloading(galleryID)) //download in progress + if (worker.progress.indexOfKey(galleryID) >= 0 && Cache(context).isDownloading(galleryID)) { //download in progress + Cache(context).setDownloading(galleryID, false) worker.cancel(galleryID) + } else { Cache(context).setDownloading(galleryID, true) diff --git a/app/src/main/java/xyz/quaver/pupil/ui/ReaderActivity.kt b/app/src/main/java/xyz/quaver/pupil/ui/ReaderActivity.kt index c4ce1917..f8c46611 100644 --- a/app/src/main/java/xyz/quaver/pupil/ui/ReaderActivity.kt +++ b/app/src/main/java/xyz/quaver/pupil/ui/ReaderActivity.kt @@ -264,6 +264,7 @@ class ReaderActivity : AppCompatActivity() { private fun initDownloader() { val worker = DownloadWorker.getInstance(this).apply { + cancel(galleryID) queue.add(galleryID) } @@ -280,7 +281,7 @@ class ReaderActivity : AppCompatActivity() { runOnUiThread { reader_download_progressbar.max = reader_recyclerview.adapter?.itemCount ?: 0 - reader_download_progressbar.progress = worker.progress[galleryID]?.count { !it.isFinite() } ?: 0 + reader_download_progressbar.progress = worker.progress[galleryID]?.count { it.isInfinite() } ?: 0 reader_progressbar.max = reader_recyclerview.adapter?.itemCount ?: 0 if (title == getString(R.string.reader_loading)) { @@ -305,7 +306,7 @@ class ReaderActivity : AppCompatActivity() { } } - if (worker.progress[galleryID]?.all { !it.isFinite() } == true) { //Download finished + if (worker.progress[galleryID]?.all { it.isInfinite() } == true) { //Download finished reader_download_progressbar.visibility = View.GONE animateDownloadFAB(false) @@ -316,7 +317,7 @@ class ReaderActivity : AppCompatActivity() { private fun initView() { with(reader_recyclerview) { - adapter = ReaderAdapter(Glide.with(this@ReaderActivity), galleryID).apply { + adapter = ReaderAdapter(Glide.with(this@ReaderActivity), galleryID, this@ReaderActivity).apply { onItemClickListener = { if (isScroll) { isScroll = false @@ -428,7 +429,7 @@ class ReaderActivity : AppCompatActivity() { icon?.registerAnimationCallback(object : Animatable2Compat.AnimationCallback() { override fun onAnimationEnd(drawable: Drawable?) { val worker = DownloadWorker.getInstance(context) - if (worker.progress[galleryID]?.all { !it.isFinite() } == true) // If download is finished, stop animating + if (worker.progress[galleryID]?.all { it.isInfinite() } == true) // If download is finished, stop animating post { setImageResource(R.drawable.ic_download) labelText = getString(R.string.reader_fab_download_cancel) diff --git a/app/src/main/java/xyz/quaver/pupil/util/download/Cache.kt b/app/src/main/java/xyz/quaver/pupil/util/download/Cache.kt index 51389012..3b208f97 100644 --- a/app/src/main/java/xyz/quaver/pupil/util/download/Cache.kt +++ b/app/src/main/java/xyz/quaver/pupil/util/download/Cache.kt @@ -21,7 +21,6 @@ package xyz.quaver.pupil.util.download import android.content.Context import android.content.ContextWrapper import android.util.Base64 -import android.util.Log import android.util.SparseArray import androidx.preference.PreferenceManager import com.google.firebase.crashlytics.FirebaseCrashlytics @@ -69,7 +68,7 @@ class Cache(context: Context) : ContextWrapper(context) { // Search in this order // Download -> Cache fun getCachedGallery(galleryID: Int) = getCachedGallery(this, galleryID).also { - if (!it.exists() && !preference.getBoolean("cache_disable", false)) + if (!it.exists()) it.mkdirs() } @@ -289,17 +288,17 @@ class Cache(context: Context) : ContextWrapper(context) { if (download.isParentOf(cache)) return@launch - Log.i("PUPILD", "MOVING ${cache.canonicalPath} --> ${download.canonicalPath}") + FirebaseCrashlytics.getInstance().log("MOVING ${cache.canonicalPath} --> ${download.canonicalPath}") cache.copyRecursively(download, true) { file, err -> - Log.i("PUPILD", "MOVING ERROR ${file.canonicalPath} ${err.message}") + FirebaseCrashlytics.getInstance().log("MOVING ERROR ${file.canonicalPath} ${err.message}") OnErrorAction.SKIP } - Log.i("PUPILD", "MOVED ${cache.canonicalPath}") + FirebaseCrashlytics.getInstance().log("MOVED ${cache.canonicalPath}") - Log.i("PUPILD", "DELETING ${cache.canonicalPath}") + FirebaseCrashlytics.getInstance().log("DELETING ${cache.canonicalPath}") cache.deleteRecursively() - Log.i("PUPILD", "DELETED ${cache.canonicalPath}") + FirebaseCrashlytics.getInstance().log("DELETED ${cache.canonicalPath}") } } diff --git a/app/src/main/java/xyz/quaver/pupil/util/download/DownloadWorker.kt b/app/src/main/java/xyz/quaver/pupil/util/download/DownloadWorker.kt index 4ff09b62..63a9ca9c 100644 --- a/app/src/main/java/xyz/quaver/pupil/util/download/DownloadWorker.kt +++ b/app/src/main/java/xyz/quaver/pupil/util/download/DownloadWorker.kt @@ -128,21 +128,8 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont * SECONDARY VALUE * 0 <= value < 100 -> Download in progress * Float.POSITIVE_INFINITY -> Download completed - * Float.NaN -> Exception */ val progress = SparseArray?>() - /* - * KEY - * primary galleryID - * secondary index - * PRIMARY VALUE - * MutableList -> Download in progress / Loading - * null -> Gallery doesn't exist - * SECONDARY VALUE - * Throwable -> Exception - * null -> Download in progress / Loading - */ - val exception = SparseArray?>() val notification = SparseArray() private val loop = loop() @@ -194,7 +181,6 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont } progress.clear() - exception.clear() notification.clear() notificationManager.cancelAll() } @@ -210,15 +196,11 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont } progress.remove(galleryID) - exception.remove(galleryID) notification.remove(galleryID) notificationManager.cancel(galleryID) - - if (progress.indexOfKey(galleryID) >= 0) - Cache(this@DownloadWorker).setDownloading(galleryID, false) } - fun isCompleted(galleryID: Int) = progress[galleryID]?.all { !it.isFinite() } == true + fun isCompleted(galleryID: Int) = progress[galleryID]?.all { it.isInfinite() } == true private fun queueDownload(galleryID: Int, reader: Reader, index: Int, callback: Callback) { val lowQuality = preferences.getBoolean("low_quality", false) @@ -248,8 +230,6 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont }.build() client.newCall(request).enqueue(callback) - - Log.i("PUPILD", "DOWNLOADING ($galleryID, $index) from ${request.url()}") } private fun download(galleryID: Int) = CoroutineScope(Dispatchers.IO).launch { @@ -258,7 +238,6 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont //gallery doesn't exist if (reader == null) { progress.put(galleryID, null) - exception.put(galleryID, null) Cache(this@DownloadWorker).setDownloading(galleryID, false) return@launch @@ -272,7 +251,6 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont else 0F }.toMutableList()) - exception.put(galleryID, reader.galleryInfo.files.map { null }.toMutableList()) if (notification[galleryID] == null) initNotification(galleryID) @@ -294,30 +272,21 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont for (i in reader.galleryInfo.files.indices) { val callback = object : Callback { override fun onFailure(call: Call, e: IOException) { + if (e.message?.contains("cancel", true) != false) + return + Log.i("PUPILD", "FAIL ${call.request().tag()} (${e.message})") - if (e.message?.contains("cancel", true) != true) - FirebaseCrashlytics.getInstance().recordException(e) - - progress[galleryID]?.set(i, Float.NaN) - exception[galleryID]?.set(i, e) - - notify(galleryID) - - CoroutineScope(Dispatchers.IO).launch { - if (isCompleted(galleryID)) { - with(Cache(this@DownloadWorker)) { - if (isDownloading(galleryID)) { - moveToDownload(galleryID) - setDownloading(galleryID, false) - } - } - } + FirebaseCrashlytics.getInstance().apply { + log("FAIL ${call.request().tag()} (${e.message})") + setCustomKey("POS", "FAIL") + recordException(e) } + + cancel(galleryID) + queue.add(galleryID) } override fun onResponse(call: Call, response: Response) { - Log.i("PUPILD", "OK ${call.request().tag()}") - val ext = call.request().url().encodedPath().split('.').last() try { @@ -338,47 +307,28 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont } } } - - Log.i("PUPILD", "SUCCESS ${call.request().tag()}") } catch (e: Exception) { - - progress[galleryID]?.set(i, Float.NaN) - exception[galleryID]?.set(i, e) - - notify(galleryID) - - CoroutineScope(Dispatchers.IO).launch { - if (isCompleted(galleryID)) { - with(Cache(this@DownloadWorker)) { - if (isDownloading(galleryID)) { - moveToDownload(galleryID) - setDownloading(galleryID, false) - } - } - } + FirebaseCrashlytics.getInstance().apply { + setCustomKey("POS", "FAIL ON OK") + recordException(e) } File(Cache(this@DownloadWorker).getCachedGallery(galleryID), "%05d.$ext".format(i)).delete() - Log.i("PUPILD", "FAIL ON OK ${call.request().tag()} (${e.message})") + cancel(galleryID) + queue.add(galleryID) } } } - if (progress[galleryID]?.get(i)?.isFinite() == true) { + if (progress[galleryID]?.get(i)?.isFinite() == true) queueDownload(galleryID, reader, i, callback) - Log.i("PUPILD", "$galleryID QUEUED $i") - } else { - Log.i("PUPILD", "$galleryID SKIPPED $i (${progress[galleryID]?.get(i)})") - } } } private fun notify(galleryID: Int) { val max = progress[galleryID]?.size ?: 0 - val progress = progress[galleryID]?.count { !it.isFinite() } ?: 0 - - Log.i("PUPILD", "NOTIFY $galleryID ${isCompleted(galleryID)} $progress/$max") + val progress = progress[galleryID]?.count { it.isInfinite() } ?: 0 if (isCompleted(galleryID)) { notification[galleryID] @@ -434,8 +384,6 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont if (Cache(this@DownloadWorker).isDownloading(galleryID)) notificationManager.notify(galleryID, notification[galleryID].build()) - Log.i("PUPILD", "QUEUED $galleryID") - worker.put(galleryID, download(galleryID)) queue.poll() } diff --git a/app/src/main/res/values-ko/strings.xml b/app/src/main/res/values-ko/strings.xml index fb18e2a7..4c065d0b 100644 --- a/app/src/main/res/values-ko/strings.xml +++ b/app/src/main/res/values-ko/strings.xml @@ -148,6 +148,6 @@ 캐시를 활성화 해야 다운로드를 진행할 수 있습니다 유저 ID 유저 ID를 클립보드에 복사했습니다 - 다운로드 에러가 발생했습니다. 재시도 하시겠습니까? + 다운로드 에러가 발생했습니. 재시도 하시겠습니까? 재시도 \ No newline at end of file diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 531e9997..f1765704 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -121,7 +121,7 @@ Download complete Download error - Download Error occurred. Retry? + Download Error. Retry? Help From d763f5dca08868c1d3b30b9504255f9b73b1cb53 Mon Sep 17 00:00:00 2001 From: tom5079 Date: Mon, 22 Jun 2020 09:52:25 +0900 Subject: [PATCH 2/2] App built --- app/release/output.json | 4 ++-- .../java/xyz/quaver/pupil/util/download/DownloadWorker.kt | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/release/output.json b/app/release/output.json index 967b8923..b6d220aa 100644 --- a/app/release/output.json +++ b/app/release/output.json @@ -11,8 +11,8 @@ "type": "SINGLE", "filters": [], "properties": [], - "versionCode": 54, - "versionName": "54", + "versionCode": 55, + "versionName": "55", "enabled": true, "outputFile": "app-release.apk" } diff --git a/app/src/main/java/xyz/quaver/pupil/util/download/DownloadWorker.kt b/app/src/main/java/xyz/quaver/pupil/util/download/DownloadWorker.kt index 63a9ca9c..d91cc987 100644 --- a/app/src/main/java/xyz/quaver/pupil/util/download/DownloadWorker.kt +++ b/app/src/main/java/xyz/quaver/pupil/util/download/DownloadWorker.kt @@ -309,6 +309,7 @@ class DownloadWorker private constructor(context: Context) : ContextWrapper(cont } } catch (e: Exception) { FirebaseCrashlytics.getInstance().apply { + log("FAIL ON OK ${call.request().tag()} (${e.message})") setCustomKey("POS", "FAIL ON OK") recordException(e) }