Clean up image activity and prepare for tests

2024-09-20 07:31:40 +02:00 · 2018-12-25 16:32:51 -05:00 · 2018-12-25 16:32:51 -05:00 · 697e457da4
commit 697e457da4
parent 25760fa2d0
7 changed files with 131 additions and 98 deletions
--- a/app/build.gradle
+++ b/app/build.gradle
@ -156,6 +156,7 @@ dependencies {
    androidTestImplementation kauDependency.espresso
    androidTestImplementation kauDependency.testRules
    androidTestImplementation kauDependency.testRunner
+    androidTestImplementation "com.squareup.okhttp3:mockwebserver:${OKHTTP}"

    testImplementation kauDependency.kotlinTest
    testImplementation "org.jetbrains.kotlin:kotlin-reflect:${KOTLIN}"
@ -180,7 +181,10 @@ dependencies {
    //noinspection GradleDependency
    implementation "ca.allanwang.kau:core-ui:$KAU"

-    implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:$COROUTINES"
+    // TODO temp
+    implementation "org.jetbrains.anko:anko-commons:0.10.8"
+
+    implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:${COROUTINES}"

    implementation "org.apache.commons:commons-text:${COMMONS_TEXT}"

--- a/app/src/androidTest/kotlin/com/pitchedapps/frost/activities/ImageActivityTest.kt
+++ b/app/src/androidTest/kotlin/com/pitchedapps/frost/activities/ImageActivityTest.kt
@ -0,0 +1,32 @@
+package com.pitchedapps.frost.activities
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.rule.ActivityTestRule
+import org.junit.Rule
+import org.junit.runner.RunWith
+import android.content.Intent
+import com.pitchedapps.frost.utils.ARG_COOKIE
+import com.pitchedapps.frost.utils.ARG_IMAGE_URL
+import com.pitchedapps.frost.utils.ARG_TEXT
+import org.junit.Test
+
+@RunWith(AndroidJUnit4::class)
+class ImageActivityTest {
+
+    @get:Rule
+    val activity: ActivityTestRule<ImageActivity> = ActivityTestRule(ImageActivity::class.java, true, false)
+
+    private fun launchActivity(imageUrl: String, text: String? = null, cookie: String? = null) {
+        val intent = Intent().apply {
+            putExtra(ARG_IMAGE_URL, imageUrl)
+            putExtra(ARG_TEXT, text)
+            putExtra(ARG_COOKIE, cookie)
+        }
+        activity.launchActivity(intent)
+    }
+
+    @Test
+    fun intent() {
+
+    }
+}
--- a/app/src/main/kotlin/com/pitchedapps/frost/activities/ImageActivity.kt
+++ b/app/src/main/kotlin/com/pitchedapps/frost/activities/ImageActivity.kt
@ -28,13 +28,14 @@ import ca.allanwang.kau.mediapicker.scanMedia
 import ca.allanwang.kau.permissions.PERMISSION_WRITE_EXTERNAL_STORAGE
 import ca.allanwang.kau.permissions.kauRequestPermissions
 import ca.allanwang.kau.utils.colorToForeground
+import ca.allanwang.kau.utils.copyFromInputStream
 import ca.allanwang.kau.utils.fadeOut
 import ca.allanwang.kau.utils.fadeScaleTransition
 import ca.allanwang.kau.utils.isHidden
+import ca.allanwang.kau.utils.isVisible
 import ca.allanwang.kau.utils.scaleXY
 import ca.allanwang.kau.utils.setIcon
 import ca.allanwang.kau.utils.tint
-import ca.allanwang.kau.utils.use
 import ca.allanwang.kau.utils.withAlpha
 import ca.allanwang.kau.utils.withMinAlpha
 import com.davemorrissey.labs.subscaleview.ImageSource
@ -53,7 +54,6 @@ import com.pitchedapps.frost.utils.ARG_IMAGE_URL
 import com.pitchedapps.frost.utils.ARG_TEXT
 import com.pitchedapps.frost.utils.L
 import com.pitchedapps.frost.utils.Prefs
-import com.pitchedapps.frost.utils.createFreshFile
 import com.pitchedapps.frost.utils.frostSnackbar
 import com.pitchedapps.frost.utils.frostUriFromFile
 import com.pitchedapps.frost.utils.isIndirectImageUrl
@ -63,10 +63,11 @@ import com.pitchedapps.frost.utils.sendFrostEmail
 import com.pitchedapps.frost.utils.setFrostColors
 import com.sothree.slidinguppanel.SlidingUpPanelLayout
 import kotlinx.android.synthetic.main.activity_image.*
+import kotlinx.coroutines.CoroutineExceptionHandler
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import okhttp3.Response
-import org.jetbrains.anko.activityUiThreadWithContext
-import org.jetbrains.anko.doAsync
-import org.jetbrains.anko.uiThread
 import java.io.File
 import java.io.FileFilter
 import java.io.IOException
@ -79,6 +80,7 @@ import java.util.Locale
 */
 class ImageActivity : KauBaseActivity() {

+    @Volatile
    internal var errorRef: Throwable? = null

    private lateinit var tempDir: File
@ -138,6 +140,16 @@ class ImageActivity : KauBaseActivity() {
        )}_${Math.abs(imageUrl.hashCode())}"
    }

+    private fun loadError(e: Throwable) {
+        errorRef = e
+        e.logFrostEvent("Image load error")
+        L.e { "Failed to load image $imageHash" }
+        if (image_progress.isVisible)
+            image_progress.fadeOut()
+        tempFile.delete()
+        fabAction = FabStates.ERROR
+    }
+
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        intent?.extras ?: return finish()
@ -165,12 +177,8 @@ class ImageActivity : KauBaseActivity() {
        })
        image_fab.setOnClickListener { fabAction.onClick(this) }
        image_photo.setOnImageEventListener(object : SubsamplingScaleImageView.DefaultOnImageEventListener() {
-            override fun onImageLoadError(e: Exception?) {
-                errorRef = e
-                e.logFrostEvent("Image load error")
-                L.e { "Failed to load image $imageUrl" }
-                tempFile?.delete()
-                fabAction = FabStates.ERROR
+            override fun onImageLoadError(e: Exception) {
+                loadError(e)
            }
        })
        setFrostColors {
@ -178,68 +186,14 @@ class ImageActivity : KauBaseActivity() {
        }
        tempDir = File(cacheDir, IMAGE_FOLDER)
        tempFile = File(tempDir, imageHash)
-        doAsync({
-            L.e(it) { "Failed to load image $imageHash" }
-            errorRef = it
-            runOnUiThread { image_progress.fadeOut() }
-            tempFile.delete()
-            fabAction = FabStates.ERROR
-        }) {
-            val loaded = loadImage(tempFile)
-            uiThread {
+        launch(CoroutineExceptionHandler { _, err -> loadError(err) }) {
+            downloadImageTo(tempFile)
            image_progress.fadeOut()
-                if (!loaded) {
-                    fabAction = FabStates.ERROR
-                } else {
            image_photo.setImage(ImageSource.uri(frostUriFromFile(tempFile)))
            fabAction = FabStates.DOWNLOAD
            image_photo.animate().alpha(1f).scaleXY(1f).start()
        }
    }
-        }
-    }
-
-    /**
-     * Attempts to load the image to [file]
-     * Returns true if successful
-     * Note that this is a long execution and should not be done on the UI thread
-     */
-    private fun loadImage(file: File): Boolean {
-        if (file.exists() && file.length() > 1) {
-            file.setLastModified(System.currentTimeMillis())
-            L.d { "Loading from local cache ${file.absolutePath}" }
-            return true
-        }
-        val response = getImageResponse()
-
-        if (!response.isSuccessful) {
-            L.e { "Unsuccessful response for image" }
-            errorRef = Throwable("Unsuccessful response for image")
-            return false
-        }
-
-        if (!file.createFreshFile()) {
-            L.e { "Could not create temp file" }
-            return false
-        }
-
-        var valid = false
-
-        response.body()?.byteStream()?.use { input ->
-            file.outputStream().use { output ->
-                input.copyTo(output)
-                valid = true
-            }
-        }
-
-        if (!valid) {
-            L.e { "Failed to copy file" }
-            file.delete()
-            return false
-        }
-
-        return true
-    }

    @Throws(IOException::class)
    private fun createPublicMediaFile(): File {
@ -257,29 +211,58 @@ class ImageActivity : KauBaseActivity() {
        .call()
        .execute()

+    /**
+     * Saves the image to the specified file, creating it if it doesn't exist.
+     * Returns true if a change is made, false otherwise.
+     * Throws an error if something goes wrong.
+     */
    @Throws(IOException::class)
-    private fun downloadImageTo(file: File) {
-        val body = getImageResponse().body()
-            ?: throw IOException("Failed to retrieve image body")
-        body.byteStream().use { input ->
-            file.outputStream().use { output ->
-                input.copyTo(output)
+    private suspend fun downloadImageTo(file: File): Boolean {
+        val exceptionHandler = CoroutineExceptionHandler { _, _ ->
+            if (file.isFile && file.length() == 0L) {
+                file.delete()
            }
        }
+        return withContext(Dispatchers.IO + exceptionHandler) {
+            if (!file.isFile) {
+                file.mkdirs()
+                file.createNewFile()
+            }
+
+            file.setLastModified(System.currentTimeMillis())
+
+            // Forbid overwrites
+            if (file.length() > 1)
+                return@withContext false
+            if (tempFile.isFile && tempFile.length() > 1) {
+                if (tempFile == file)
+                    return@withContext false
+                tempFile.copyTo(file)
+                return@withContext true
+            }
+            // No temp file, download ourselves
+            val response = getImageResponse()
+
+            if (!response.isSuccessful) {
+                throw IOException("Unsuccessful response for image: ${response.peekBody(128).string()}")
+            }
+
+            val body = response.body() ?: throw IOException("Failed to retrieve image body")
+
+            file.copyFromInputStream(body.byteStream())
+
+            return@withContext true
+        }
    }

    internal fun saveImage() {
        kauRequestPermissions(PERMISSION_WRITE_EXTERNAL_STORAGE) { granted, _ ->
            L.d { "Download image callback granted: $granted" }
            if (granted) {
-                doAsync {
+                launch {
                    val destination = createPublicMediaFile()
                    var success = true
                    try {
-                        val temp = tempFile
-                        if (temp != null)
-                            temp.copyTo(destination, true)
-                        else
                        downloadImageTo(destination)
                    } catch (e: Exception) {
                        errorRef = e
@ -295,7 +278,6 @@ class ImageActivity : KauBaseActivity() {
                            } catch (ignore: Exception) {
                            }
                        }
-                        activityUiThreadWithContext {
                        val text = if (success) R.string.image_download_success else R.string.image_download_fail
                        frostSnackbar(text)
                        if (success) fabAction = FabStates.SHARE
@ -304,7 +286,6 @@ class ImageActivity : KauBaseActivity() {
            }
        }
    }
-    }

    override fun onDestroy() {
        val purge = System.currentTimeMillis() - PURGE_TIME
--- a/app/src/main/kotlin/com/pitchedapps/frost/facebook/FbCookie.kt
+++ b/app/src/main/kotlin/com/pitchedapps/frost/facebook/FbCookie.kt
@ -72,14 +72,14 @@ object FbCookie {

    private suspend fun CookieManager.suspendSetWebCookie(cookie: String?): Boolean {
        cookie ?: return true
-        L.test { "Orig ${webCookie}" }
+        L.test { "Orig $webCookie" }
        removeAllCookies()
        L.test { "Save $cookie" }
        // Save all cookies regardless of result, then check if all succeeded
        val result = cookie.split(";").map { setSingleWebCookie(it) }.all { it }
-        L.test { "AAAA ${webCookie}" }
+        L.test { "AAAA $webCookie" }
        flush()
-        L.test { "SSSS ${webCookie}" }
+        L.test { "SSSS $webCookie" }
        return result
    }

--- a/app/src/main/kotlin/com/pitchedapps/frost/fragments/FragmentBase.kt
+++ b/app/src/main/kotlin/com/pitchedapps/frost/fragments/FragmentBase.kt
@ -41,6 +41,11 @@ import com.pitchedapps.frost.utils.REQUEST_TEXT_ZOOM
 import com.pitchedapps.frost.utils.frostEvent
 import io.reactivex.android.schedulers.AndroidSchedulers
 import io.reactivex.disposables.Disposable
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.SupervisorJob
+import kotlin.coroutines.CoroutineContext

 /**
 * Created by Allan Wang on 2017-11-07.
@ -48,7 +53,7 @@ import io.reactivex.disposables.Disposable
 * All fragments pertaining to the main view
 * Must be attached to activities implementing [MainActivityContract]
 */
-abstract class BaseFragment : Fragment(), FragmentContract, DynamicUiContract {
+abstract class BaseFragment : Fragment(), CoroutineScope, FragmentContract, DynamicUiContract {

    companion object {
        private const val ARG_POSITION = "arg_position"
@ -71,6 +76,10 @@ abstract class BaseFragment : Fragment(), FragmentContract, DynamicUiContract {
        }
    }

+    open lateinit var job: Job
+    override val coroutineContext: CoroutineContext
+        get() = Dispatchers.Main + job
+
    override val baseUrl: String by lazy { arguments!!.getString(ARG_URL) }
    override val baseEnum: FbItem by lazy { FbItem[arguments]!! }
    override val position: Int by lazy { arguments!!.getInt(ARG_POSITION) }
@ -98,6 +107,7 @@ abstract class BaseFragment : Fragment(), FragmentContract, DynamicUiContract {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
+        job = SupervisorJob()
        firstLoad = true
        if (context !is MainActivityContract)
            throw IllegalArgumentException("${this::class.java.simpleName} is not attached to a context implementing MainActivityContract")
@ -207,6 +217,11 @@ abstract class BaseFragment : Fragment(), FragmentContract, DynamicUiContract {
        super.onDestroyView()
    }

+    override fun onDestroy() {
+        job.cancel()
+        super.onDestroy()
+    }
+
    override fun reloadTheme() {
        reloadThemeSelf()
        content?.reloadTextSize()
--- a/app/src/main/kotlin/com/pitchedapps/frost/services/NotificationService.kt
+++ b/app/src/main/kotlin/com/pitchedapps/frost/services/NotificationService.kt
@ -31,6 +31,7 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.async
+import kotlinx.coroutines.isActive
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlin.coroutines.CoroutineContext
@ -77,7 +78,7 @@ class NotificationService : JobService(), CoroutineScope {
            try {
                async { sendNotifications(params) }.await()
            } finally {
-                if (!job.isCancelled)
+                if (!isActive)
                    prepareFinish(false)
                jobFinished(params, false)
            }
@ -85,14 +86,14 @@ class NotificationService : JobService(), CoroutineScope {
        return true
    }

-    private suspend fun sendNotifications(params: JobParameters?): Unit = suspendCancellableCoroutine { cont ->
+    private suspend fun sendNotifications(params: JobParameters?): Unit = suspendCancellableCoroutine {
        val currentId = Prefs.userId
        val cookies = loadFbCookiesSync()
-        if (cont.isCancelled) return@suspendCancellableCoroutine
+        if (it.isCancelled) return@suspendCancellableCoroutine
        val jobId = params?.extras?.getInt(NOTIFICATION_PARAM_ID, -1) ?: -1
        var notifCount = 0
        for (cookie in cookies) {
-            if (cont.isCancelled) break
+            if (it.isCancelled) break
            val current = cookie.id == currentId
            if (Prefs.notificationsGeneral &&
                (current || Prefs.notificationAllAccounts)
--- a/gradle.properties
+++ b/gradle.properties
@ -14,7 +14,7 @@ org.gradle.jvmargs=-Xmx2048m -XX:MaxPermSize=512m -XX:+HeapDumpOnOutOfMemoryErro
 APP_ID=Frost
 APP_GROUP=com.pitchedapps

-KAU=b4a2ded
+KAU=d850474
 KOTLIN=1.3.11

 # https://mvnrepository.com/artifact/com.android.tools.build/gradle?repo=google