Clean up image activity and prepare for tests

4 files changed, 93 insertions, 96 deletions

diff --git a/app/src/main/kotlin/com/pitchedapps/frost/activities/ImageActivity.kt b/app/src/main/kotlin/com/pitchedapps/frost/activities/ImageActivity.kt
index 83f617ba..bbd0463a 100644
--- 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,69 +186,15 @@ 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 {
-                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()
-                }
-            }
+        launch(CoroutineExceptionHandler { _, err -> loadError(err) }) {
+            downloadImageTo(tempFile)
+            image_progress.fadeOut()
+            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 {
         val timeStamp = SimpleDateFormat(TIME_FORMAT, Locale.getDefault()).format(Date())
@@ -257,30 +211,59 @@ 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)
+                        downloadImageTo(destination)
                     } catch (e: Exception) {
                         errorRef = e
                         success = false
@@ -295,11 +278,9 @@ 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
-                        }
+                        val text = if (success) R.string.image_download_success else R.string.image_download_fail
+                        frostSnackbar(text)
+                        if (success) fabAction = FabStates.SHARE
                     }
                 }
             }
diff --git a/app/src/main/kotlin/com/pitchedapps/frost/facebook/FbCookie.kt b/app/src/main/kotlin/com/pitchedapps/frost/facebook/FbCookie.kt
index 627b0186..47c0ecc4 100644
--- 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
     }
 
diff --git a/app/src/main/kotlin/com/pitchedapps/frost/fragments/FragmentBase.kt b/app/src/main/kotlin/com/pitchedapps/frost/fragments/FragmentBase.kt
index 98e28bd3..2c46edbc 100644
--- 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()
diff --git a/app/src/main/kotlin/com/pitchedapps/frost/services/NotificationService.kt b/app/src/main/kotlin/com/pitchedapps/frost/services/NotificationService.kt
index 40a78b04..3470ca07 100644
--- 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)