From 697e457da453568ca703c2b655a2dd490157b443 Mon Sep 17 00:00:00 2001 From: Allan Wang Date: Tue, 25 Dec 2018 16:32:51 -0500 Subject: Clean up image activity and prepare for tests --- app/build.gradle | 6 +- .../frost/activities/ImageActivityTest.kt | 32 +++++ .../pitchedapps/frost/activities/ImageActivity.kt | 157 +++++++++------------ .../com/pitchedapps/frost/facebook/FbCookie.kt | 6 +- .../pitchedapps/frost/fragments/FragmentBase.kt | 17 ++- .../frost/services/NotificationService.kt | 9 +- gradle.properties | 2 +- 7 files changed, 131 insertions(+), 98 deletions(-) create mode 100644 app/src/androidTest/kotlin/com/pitchedapps/frost/activities/ImageActivityTest.kt diff --git a/app/build.gradle b/app/build.gradle index ebf3ecf0..25361854 100644 --- 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}" diff --git a/app/src/androidTest/kotlin/com/pitchedapps/frost/activities/ImageActivityTest.kt b/app/src/androidTest/kotlin/com/pitchedapps/frost/activities/ImageActivityTest.kt new file mode 100644 index 00000000..abffb106 --- /dev/null +++ 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 = 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() { + + } +} \ No newline at end of file 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) diff --git a/gradle.properties b/gradle.properties index 792f6ac0..6778bba4 100644 --- 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 -- cgit v1.2.3