From 7d9e1a3659a23c58457338ce449a37cdcddc62fb Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Mon, 15 Aug 2022 20:49:21 -0700 Subject: [PATCH] Add an experimental integration with Flows Currently the only Flow will emit the status of the load along with the current placeholder or resource, clearing the request when the Flow ends. In future versions we might consider making clearing the target optional so that resources could be used after the flow is over. We could also expose some additional helper functions to make it easier to work with the first run of a flow, or to exclude placeholders. For now this seems like a reasonable starting point. --- gradle.properties | 7 +- integration/ktx/build.gradle | 54 ++ integration/ktx/gradle.properties | 9 + integration/ktx/src/main/AndroidManifest.xml | 2 + .../com/bumptech/glide/GlideIntegration.kt | 19 + .../bumptech/glide/integration/ktx/Flows.kt | 313 ++++++++++++ .../glide/integration/ktx/FlowsTest.kt | 472 ++++++++++++++++++ .../com/bumptech/glide/RequestBuilder.java | 6 +- .../glide/manager/RequestTracker.java | 2 + library/test/build.gradle | 3 +- settings.gradle | 1 + 11 files changed, 883 insertions(+), 5 deletions(-) create mode 100644 integration/ktx/build.gradle create mode 100644 integration/ktx/gradle.properties create mode 100644 integration/ktx/src/main/AndroidManifest.xml create mode 100644 integration/ktx/src/main/java/com/bumptech/glide/GlideIntegration.kt create mode 100644 integration/ktx/src/main/java/com/bumptech/glide/integration/ktx/Flows.kt create mode 100644 integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt diff --git a/gradle.properties b/gradle.properties index 9d859ee904..755dcdb633 100644 --- a/gradle.properties +++ b/gradle.properties @@ -57,13 +57,16 @@ ANDROID_X_TEST_CORE_VERSION=1.4.0 ANDROID_X_TEST_JUNIT_VERSION=1.1.3 ANDROID_X_TEST_RULES_VERSION=1.4.0 ANDROID_X_TEST_RUNNER_VERSION=1.4.0 +ANDROID_X_TEST_CORE_KTX_VERSION=1.4.0 +ANDROID_X_TEST_JUNIT_KTX_VERSION=1.1.3 ANDROID_X_TRACING_VERSION=1.0.0 ANDROID_X_VECTOR_DRAWABLE_ANIMATED_VERSION=1.1.0 ANDROID_X_CORE_KTX_VERSION=1.8.0 ANDROID_X_LIFECYCLE_KTX_VERSION=2.4.1 # org.jetbrains versions -JETBRAINS_KOTLINX_COROUTINES_VERSION=1.6.3 +JETBRAINS_KOTLINX_COROUTINES_VERSION=1.6.4 +JETBRAINS_KOTLINX_COROUTINES_TEST_VERSION=1.6.4 JETBRAINS_KOTLIN_VERSION=1.7.0 JETBRAINS_KOTLIN_TEST_VERSION=1.7.0 @@ -86,6 +89,6 @@ MOCKWEBSERVER_VERSION=3.0.0-RC1 OK_HTTP_VERSION=3.10.0 PMD_VERSION=6.0.0 ROBOLECTRIC_VERSION=4.8.1 -TRUTH_VERSION=0.45 +TRUTH_VERSION=1.1.3 VIOLATIONS_PLUGIN_VERSION=1.8 VOLLEY_VERSION=1.2.0 diff --git a/integration/ktx/build.gradle b/integration/ktx/build.gradle new file mode 100644 index 0000000000..1c236eaa14 --- /dev/null +++ b/integration/ktx/build.gradle @@ -0,0 +1,54 @@ +plugins { + id 'com.android.library' + id 'org.jetbrains.kotlin.android' +} + +android { + compileSdk COMPILE_SDK_VERSION as int + + defaultConfig { + minSdk MIN_SDK_VERSION as int + targetSdk TARGET_SDK_VERSION as int + + testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" + consumerProguardFiles "consumer-rules.pro" + } + + buildTypes { + release { + minifyEnabled false + proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro' + } + } + compileOptions { + sourceCompatibility JavaVersion.VERSION_1_7 + targetCompatibility JavaVersion.VERSION_1_7 + } + kotlinOptions { + jvmTarget = '1.8' + } +} + +// Enable strict mode, but exclude tests. +tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile) { + if (!it.name.contains("Test")) { + kotlinOptions.freeCompilerArgs += "-Xexplicit-api=strict" + } +} + +dependencies { + api project(":library") + implementation "androidx.core:core-ktx:$ANDROID_X_CORE_KTX_VERSION" + implementation "androidx.test:core-ktx:$ANDROID_X_TEST_CORE_KTX_VERSION" + implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:$JETBRAINS_KOTLINX_COROUTINES_VERSION" + testImplementation "androidx.test.ext:junit-ktx:$ANDROID_X_TEST_JUNIT_KTX_VERSION" + testImplementation "androidx.test.ext:junit:$ANDROID_X_TEST_JUNIT_VERSION" + testImplementation "org.robolectric:robolectric:$ROBOLECTRIC_VERSION" + testImplementation "androidx.test:runner:$ANDROID_X_TEST_RUNNER_VERSION" + testImplementation "junit:junit:$JUNIT_VERSION" + testImplementation "org.jetbrains.kotlinx:kotlinx-coroutines-test:$JETBRAINS_KOTLINX_COROUTINES_TEST_VERSION" + testImplementation "com.google.truth:truth:$TRUTH_VERSION" + androidTestImplementation "androidx.test.ext:junit:$ANDROID_X_TEST_JUNIT_VERSION" +} + +apply from: "${rootProject.projectDir}/scripts/upload.gradle" diff --git a/integration/ktx/gradle.properties b/integration/ktx/gradle.properties new file mode 100644 index 0000000000..6316c805d8 --- /dev/null +++ b/integration/ktx/gradle.properties @@ -0,0 +1,9 @@ +POM_NAME=Glide Kotlin Extensions +POM_ARTIFACT_ID=ktx +POM_PACKAGING=aar +POM_DESCRIPTION=An integration library to improve Kotlin interop with Glide + +VERSION_MAJOR=1 +VERSION_MINOR=0 +VERSION_PATCH=0 +VERSION_NAME=1.0.0-alpha.0-SNAPSHOT diff --git a/integration/ktx/src/main/AndroidManifest.xml b/integration/ktx/src/main/AndroidManifest.xml new file mode 100644 index 0000000000..1a4808764d --- /dev/null +++ b/integration/ktx/src/main/AndroidManifest.xml @@ -0,0 +1,2 @@ + + \ No newline at end of file diff --git a/integration/ktx/src/main/java/com/bumptech/glide/GlideIntegration.kt b/integration/ktx/src/main/java/com/bumptech/glide/GlideIntegration.kt new file mode 100644 index 0000000000..acd13c348a --- /dev/null +++ b/integration/ktx/src/main/java/com/bumptech/glide/GlideIntegration.kt @@ -0,0 +1,19 @@ +/** + * Functions that give us access to some of Glide's non-public internals to make the flows API a bit + * better. + */ +package com.bumptech.glide + +import com.bumptech.glide.request.RequestListener +import com.bumptech.glide.request.target.Target + +internal fun RequestBuilder<*>.requestManager() = this.requestManager + +internal fun RequestBuilder.intoDirect( + targetAndRequestListener: TargetAndRequestListenerT, +) where TargetAndRequestListenerT : Target, + TargetAndRequestListenerT : RequestListener { + this.into(targetAndRequestListener, targetAndRequestListener) { + it.run() + } +} \ No newline at end of file diff --git a/integration/ktx/src/main/java/com/bumptech/glide/integration/ktx/Flows.kt b/integration/ktx/src/main/java/com/bumptech/glide/integration/ktx/Flows.kt new file mode 100644 index 0000000000..803894f88d --- /dev/null +++ b/integration/ktx/src/main/java/com/bumptech/glide/integration/ktx/Flows.kt @@ -0,0 +1,313 @@ +package com.bumptech.glide.integration.ktx + +import android.graphics.drawable.Drawable +import androidx.annotation.GuardedBy +import com.bumptech.glide.RequestBuilder +import com.bumptech.glide.intoDirect +import com.bumptech.glide.load.DataSource +import com.bumptech.glide.load.engine.GlideException +import com.bumptech.glide.request.Request +import com.bumptech.glide.request.RequestListener +import com.bumptech.glide.request.target.SizeReadyCallback +import com.bumptech.glide.request.target.Target +import com.bumptech.glide.request.transition.Transition +import com.bumptech.glide.requestManager +import kotlinx.coroutines.channels.ProducerScope +import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.callbackFlow +import kotlinx.coroutines.launch + +@RequiresOptIn( + level = RequiresOptIn.Level.ERROR, + message = "Glide's flow integration is very experimental and subject to breaking API or behavior changes" +) +@Retention(AnnotationRetention.BINARY) +@kotlin.annotation.Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION) +public annotation class ExperimentGlideFlows + +/** + * The current status of a flow + * + * There is no well established graph that defines the valid Status transitions. Depending on + * various factors like the parameters of the request, whether or not the resource is in the memory + * cache, or even various calls to Glide's APIs, these may be emitted in different orders. As an + * example, [RUNNING] is skipped if a request can be immediately completed from the memory cache. + * + * See [flow] for more details. + */ +@ExperimentGlideFlows +public enum class Status { + /** The load is not started or has been cleared. */ + CLEARED, + /** At least the primary load is still in progress. */ + RUNNING, + /** + * The primary load or the error load ([RequestBuilder.error]) associated with the primary have + * finished successfully. + */ + SUCCEEDED, + /** The primary load has failed. One or more thumbnails may have succeeded. */ + FAILED, +} + +/** + * Identical to `flow(dimension, dimension)` + */ +@ExperimentGlideFlows +public fun RequestBuilder.flow( + dimension: Int +): Flow> = flow(dimension, dimension) + +/** + * Convert a load in Glide into a flow that emits placeholders and resources in the order they'd + * be seen by a [Target]. + * + * Just like a [Target] there is no well defined end to a Glide request. Barring cancellation, the + * flow should eventually reach [Status.SUCCEEDED] or [Status.FAILED] at least once. However + * connectivity changes, calls to [com.bumptech.glide.RequestManager.pauseAllRequests] or + * [com.bumptech.glide.RequestManager.resumeRequests], or the lifecycle associated with this request + * may cause the request to be started multiple times. As long as the flow is active, callers will + * receive emissions from every run. + * + * This flow will clear the associated Glide request when it's cancelled. This means that callers + * must keep the flow active while any resource emitted by the flow is still in use. For UI + * contexts, collecting the flow in the appropriate fragment or view model coroutine context is + * sufficient as long as you avoid truncating methods like [kotlinx.coroutines.flow.take], + * [kotlinx.coroutines.flow.takeWhile], etc. If you do use these methods, you must be sure that + * you're no longer using or displaying the associated resource once the flow is no longer active + * (ie [kotlinx.coroutines.flow.collect] finishes). One way to do this would be to mimic the UI + * by creating and keeping active a coroutine context that collects from the flow while the resource + * is in use. If this restriction is limiting for you, please file an issue on Github so we can + * think of alternative options. + */ +@ExperimentGlideFlows +public fun RequestBuilder.flow( + width: Int, height: Int +): Flow> = + flow(ImmediateGlideSize(Size(width = width, height = height))) + +/** + * A [Status] and value pair, where the value is either a [Placeholder] or a [Resource] depending + * on how far the Glide load has progressed and/or how successful it's been. + */ +@ExperimentGlideFlows +public sealed class GlideFlowInstant { + public abstract val status: Status +} + +/** + * Wraps a [Status] and a placeholder [Drawable] (from [RequestBuilder.placeholder], + * [RequestBuilder.fallback], [RequestBuilder.error] etc). + */ +@ExperimentGlideFlows +public data class Placeholder( + public override val status: Status, public val placeholder: Drawable?, +) : GlideFlowInstant() { + init { + require( + when(status) { + Status.SUCCEEDED -> false + Status.CLEARED -> true + // Placeholder will be present prior to the first thumbnail succeeding + Status.RUNNING -> true + Status.FAILED -> true + } + ) + } +} + +/** + * Wraps a [Status] and a resource loaded from the primary request, a [RequestBuilder.thumbnail] + * request, or a [RequestBuilder.error] request. + * + * **Status.FAILED** is a perfectly valid status with this class. If the primary request fails, + * but at least one thumbnail succeeds, the flow will emit `Resource(FAILED, resource)` to indicate + * both that we have some value but also that the primary request has failed. + */ +@ExperimentGlideFlows +public data class Resource( + public override val status: Status, public val resource: ResourceT, +) : GlideFlowInstant() { + init { + require( + when (status) { + Status.SUCCEEDED -> true + // A load with thumbnail(s) where the thumbnail(s) have finished but not the main request + Status.RUNNING -> true + // The primary request of the load failed, but at least one thumbnail was successful. + Status.FAILED -> true + // Once the load is cleared, it can only show a placeholder + Status.CLEARED -> false + } + ) + } +} + +@ExperimentGlideFlows +private fun RequestBuilder.flow( + size: ResolvableGlideSize, +): Flow> { + val requestBuilder = this + val requestManager = requestBuilder.requestManager() + return callbackFlow { + val target = FlowTarget(this, size) + requestBuilder.intoDirect(target) + awaitClose { + launch { + requestManager.clear(target) + } + } + } +} + +/** + * Observes a glide request using [Target] and [RequestListener] and tries to emit something + * resembling a coherent set of placeholders and resources for it. + * + * Threading in this class is a bit complicated. As a general rule, the callback methods are + * ordered by callers. So we have to handle being called from multiple threads, but we don't need + * to try to handle callbacks being called in parallel. + * + * The primary area of concern around thread is that [resolvedSize] and [sizeReadyCallbacks] + * must be updated atomically, but can be modified on different threads. + * + * [currentRequest] would normally be a concern because [Target]s can be cancelled on threads + * other than where they were started. However in our case, [currentRequest] is set once when our + * request is started (by us) and is only cancelled when the request finishes. So we just have + * to avoid NPEs and make sure the state is reasonably up to date. + * + * [lastResource] is an unfortunate hack that tries to make sure that we emit [Status.FAILED] if + * a thumbnail request succeeds, but then the primary request fails. In that case, we'd normally + * already have emitted [Resource] with [Status.RUNNING] and the thumbnail value and then we'd + * emit nothing else. That's not very satisfying for callers who expect some resolution. So instead + * we track the last resource produced by thumbnails and emit that along with [Status.FAILED] when + * we see that the primary request has failed. As a result we're not concerned with ordering with + * regards to [lastResource], but it is possible the callbacks will be called on different threads, + * so the value may be updated from different threads even if it's not concurrent. + */ +@ExperimentGlideFlows +private class FlowTarget( + private val scope: ProducerScope>, + private val size: ResolvableGlideSize, +) : Target, RequestListener { + @Volatile private var resolvedSize: Size? = null + @Volatile private var currentRequest: Request? = null + @Volatile private var lastResource: ResourceT? = null + + @GuardedBy("this") + private val sizeReadyCallbacks = mutableListOf() + + init { + when (size) { + // If we have a size, skip the coroutine, we can continue immediately. + is ImmediateGlideSize -> resolvedSize = size.size + // Otherwise, we do not want to block the flow while waiting on a size because one or more + // requests in the chain may have a fixed size, even if the primary request does not. + // Starting the Glide request right away allows any subrequest that has a fixed size to + // begin immediately, shaving off some small amount of time. + is AsyncGlideSize -> scope.launch { + val localResolvedSize = size.asyncSize() + val callbacksToNotify: List + synchronized(this) { + resolvedSize = localResolvedSize + callbacksToNotify = ArrayList(sizeReadyCallbacks) + sizeReadyCallbacks.clear() + } + callbacksToNotify.forEach { + it.onSizeReady(localResolvedSize.width, localResolvedSize.height) + } + } + } + } + + override fun onStart() {} + override fun onStop() {} + override fun onDestroy() {} + + override fun onLoadStarted(placeholder: Drawable?) { + lastResource = null + scope.trySend(Placeholder(Status.RUNNING, placeholder)) + } + + override fun onLoadFailed(errorDrawable: Drawable?) { + scope.trySend(Placeholder(Status.FAILED, errorDrawable)) + } + + override fun onResourceReady(resource: ResourceT, transition: Transition?) { + lastResource = resource + scope.trySend( + Resource( + // currentRequest is the entire request state, so we can use it to figure out if this + // resource is from a thumbnail request (isComplete is false) or the primary request. + if (currentRequest?.isComplete == true) Status.SUCCEEDED else Status.RUNNING, resource + ) + ) + } + + override fun onLoadCleared(placeholder: Drawable?) { + lastResource = null + scope.trySend(Placeholder(Status.CLEARED, placeholder)) + } + + override fun getSize(cb: SizeReadyCallback) { + val localResolvedSize = resolvedSize + if (localResolvedSize != null) { + cb.onSizeReady(localResolvedSize.width, localResolvedSize.height) + return + } + + synchronized(this@FlowTarget) { + val lockedResolvedSize = resolvedSize + if (lockedResolvedSize != null) { + cb.onSizeReady(lockedResolvedSize.width, lockedResolvedSize.height) + } else { + sizeReadyCallbacks.add(cb) + } + } + } + + override fun removeCallback(cb: SizeReadyCallback) { + synchronized(this) { + sizeReadyCallbacks.remove(cb) + } + } + + override fun setRequest(request: Request?) { + currentRequest = request + } + + override fun getRequest(): Request? { + return currentRequest + } + + override fun onLoadFailed( + e: GlideException?, + model: Any?, + target: Target?, + isFirstResource: Boolean, + ): Boolean { + val localLastResource = lastResource + val localRequest = currentRequest + if (localLastResource != null && localRequest?.isComplete == false && !localRequest.isRunning) { + scope.channel.trySend(Resource(Status.FAILED, localLastResource)) + } + return false + } + + override fun onResourceReady( + resource: ResourceT, + model: Any?, + target: Target?, + dataSource: DataSource?, + isFirstResource: Boolean, + ): Boolean { + return false + } +} + +private data class Size(val width: Int, val height: Int) + +private sealed class ResolvableGlideSize +private data class ImmediateGlideSize(val size: Size) : ResolvableGlideSize() +private data class AsyncGlideSize(val asyncSize: suspend () -> Size) : ResolvableGlideSize() diff --git a/integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt b/integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt new file mode 100644 index 0000000000..a557de1ac6 --- /dev/null +++ b/integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt @@ -0,0 +1,472 @@ +package com.bumptech.glide.integration.ktx + +import android.content.Context +import android.graphics.Bitmap +import android.graphics.Canvas +import android.graphics.Color +import android.graphics.drawable.ColorDrawable +import android.graphics.drawable.Drawable +import android.net.Uri +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.bumptech.glide.Glide +import com.bumptech.glide.GlideBuilder +import com.bumptech.glide.RequestManager +import com.bumptech.glide.load.DataSource +import com.bumptech.glide.load.engine.GlideException +import com.bumptech.glide.load.engine.executor.GlideExecutor +import com.bumptech.glide.request.RequestListener +import com.bumptech.glide.request.target.Target +import com.google.common.truth.Correspondence +import com.google.common.truth.IterableSubject +import com.google.common.truth.Truth.assertThat +import java.io.File +import kotlin.reflect.KClass +import kotlinx.coroutines.DelicateCoroutinesApi +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.take +import kotlinx.coroutines.flow.takeWhile +import kotlinx.coroutines.flow.toList +import kotlinx.coroutines.newSingleThreadContext +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith + +// newFile throws IOException, which triggers this warning even though there's no reasonable +// alternative :/. +@Suppress("BlockingMethodInNonBlockingContext") +@OptIn(ExperimentalCoroutinesApi::class, ExperimentGlideFlows::class) +@RunWith(AndroidJUnit4::class) +class FlowsTest { + private val context = ApplicationProvider.getApplicationContext() + @get:Rule val temporaryFolder = TemporaryFolder() + + @After + fun tearDown() { + Glide.tearDown() + } + + @Test + fun flow_withPlaceholderDrawable_emitsPlaceholderDrawableFirst() = runTest { + val placeholderDrawable = ColorDrawable(Color.RED) + val first = + Glide.with(context) + .load(temporaryFolder.newFile()) + .placeholder(placeholderDrawable) + .flow(100) + .first() + + assertThat(first).isEqualTo(Placeholder(Status.RUNNING, placeholderDrawable)) + } + + @Test + fun flow_withNoPlaceholderDrawable_emitsNullPlaceholderFirst() = runTest { + val first = + Glide.with(context) + .load(temporaryFolder.newFile()) + .flow(100) + .first() + + assertThat(first).isEqualTo(Placeholder(Status.RUNNING, placeholder = null)) + } + + @Test + fun flow_failingNonNullModel_emitsRunningThenFailed() = runTest { + val missingResourceId = 123 + val results = + Glide.with(context) + .load(missingResourceId) + .flow(100) + .firstLoad() + .toList() + + assertThat(results) + .containsExactly( + Placeholder(Status.RUNNING, placeholder = null), + Placeholder(Status.FAILED, placeholder = null) + ) + .inOrder() + } + + @Test + fun flow_failingNonNullModel_whenRestartedAfterFailure_emitsSecondLoad() = runTest { + val requestManager = Glide.with(context) + val missingResourceId = 123 + + val flow = + requestManager + .load(missingResourceId) + .listener( + onFailure( + atMostOnce { + restartAllRequestsOnNewThread(requestManager) + } + ) + ) + .flow(100) + + assertThat(flow.take(4).toList()) + .comparingStatus() + .containsExactly( + Status.RUNNING, + Status.FAILED, + Status.RUNNING, + Status.FAILED + ) + } + + @Test + fun flow_successfulNonNullModel_emitsRunningThenSuccess() = runTest { + val results = + Glide.with(context) + .load(newImageFile()) + .flow(100) + .firstLoad() + .toList() + + assertThat(results) + .compareStatusAndType() + .containsExactly(placeholder(Status.RUNNING), resource(Status.SUCCEEDED)) + .inOrder() + } + + @Test + fun flow_withNullModel_andFallbackDrawable_emitsFailureWithFallbackDrawable() = runTest { + val fallbackDrawable = ColorDrawable(Color.BLUE) + val first = + Glide.with(context) + .load(null as Uri?) + .fallback(fallbackDrawable) + .flow(100) + .first() + assertThat(first).isEqualTo(Placeholder(Status.FAILED, fallbackDrawable)) + } + + @Test + fun flow_successfulNonNullModel_whenRestartedAfterSuccess_emitsSecondLoad() = runTest { + val requestManager = Glide.with(context) + + val flow = + requestManager + .load(newImageFile()) + .listener( + onSuccess( + atMostOnce { + restartAllRequestsOnNewThread(requestManager) + } + ) + ) + .flow(100) + + assertThat(flow.take(4).toList()) + .comparingStatus() + .containsExactly( + Status.RUNNING, + Status.SUCCEEDED, + Status.CLEARED, // See the TODO in RequestTracker#pauseAllRequests + Status.SUCCEEDED, // The request completes from in memory, so it never goes to RUNNING + ) + } + + @Test + fun flow_successfulNonNullModel_oneSuccessfulThumbnail_emitsThumbnailAndMainResources() = runTest { + makeGlideSingleThreadedToOrderThumbnailRequests() + + val output = + Glide.with(context) + .load(newImageFile()) + .thumbnail( + Glide.with(context).load(newImageFile()) + ) + .flow(100) + .firstLoad() + .toList() + assertThat(output) + .compareStatusAndType() + .containsExactly( + placeholder(Status.RUNNING), + resource(Status.RUNNING), + resource(Status.SUCCEEDED), + ) + } + + @Test + fun flow_successfulNonNullModel_oneFailingThumbnail_emitMainResourceOnly() = runTest { + makeGlideSingleThreadedToOrderThumbnailRequests() + + val missingResourceId = 123 + val output = + Glide.with(context) + .load(newImageFile()) + .thumbnail( + Glide.with(context).load(missingResourceId) + ) + .flow(100) + .firstLoad() + .toList() + assertThat(output) + .compareStatusAndType() + .containsExactly( + placeholder(Status.RUNNING), + resource(Status.SUCCEEDED), + ) + } + + @Test + fun flow_failingNonNullModel_successfulThumbnail_emitsThumbnailWithFailedStatus() = runTest { + makeGlideSingleThreadedToOrderThumbnailRequests() + + val missingResourceId = 123 + val output = + Glide.with(context) + .load(missingResourceId) + .thumbnail( + Glide.with(context).load(newImageFile()) + ) + .flow(100) + .firstLoad() + .toList() + assertThat(output) + .compareStatusAndType() + .containsExactly( + placeholder(Status.RUNNING), + resource(Status.RUNNING), + resource(Status.FAILED), + ) + } + + @Test + fun flow_failingNonNullModel_failingNonNullThumbnail_emitsRunningThenFailed() = runTest { + makeGlideSingleThreadedToOrderThumbnailRequests() + + val missingResourceId = 123 + val output = + Glide.with(context) + .load(missingResourceId) + .thumbnail( + Glide.with(context).load(missingResourceId) + ) + .flow(100) + .firstLoad() + .toList() + + assertThat(output) + .compareStatusAndType() + .containsExactly( + placeholder(Status.RUNNING), + placeholder(Status.FAILED) + ) + } + + @Test + fun flow_failingNonNullModel_succeedingNonNullError_emitsRunningThenSuccess() = runTest { + makeGlideSingleThreadedToOrderThumbnailRequests() + + val missingResourceId = 123 + val output = + Glide.with(context) + .load(missingResourceId) + .error(Glide.with(context).load(newImageFile())) + .flow(100) + .firstLoad() + .toList() + + assertThat(output) + .compareStatusAndType() + .containsExactly( + placeholder(Status.RUNNING), + // TODO(judds): This is probably another case where resource(Status.FAILURE) is more + // appropriate. TO do so, we'd need to avoid passing TargetListener in RequestBuilder into + // thumbnails (and probably error request builders). That's a larger change + resource(Status.SUCCEEDED) + ) + } + + + @Test + fun flow_failingNonNullModel_failingNonNullError_emitsRunningThenFailure() = runTest { + makeGlideSingleThreadedToOrderThumbnailRequests() + + val missingResourceId = 123 + val output = + Glide.with(context) + .load(missingResourceId) + .error(Glide.with(context).load(missingResourceId)) + .flow(100) + .firstLoad() + .toList() + + assertThat(output) + .compareStatusAndType() + .containsExactly( + placeholder(Status.RUNNING), + placeholder(Status.FAILED) + ) + } + + @Test + fun flow_failingNonNullModel_failingNonNullError_succeedingErrorThumbnail_emitsRunningThenRunningWithResourceThenFailureWithResource() = + runTest { + makeGlideSingleThreadedToOrderThumbnailRequests() + + val missingResourceId = 123 + val output = + Glide.with(context) + .load(missingResourceId) + .error( + Glide.with(context).load(missingResourceId) + .thumbnail(Glide.with(context).load(newImageFile())) + ) + .flow(100) + .firstLoad() + .toList() + + assertThat(output) + .compareStatusAndType() + .containsExactly( + placeholder(Status.RUNNING), + resource(Status.RUNNING), + resource(Status.FAILED), + ) + } + + // Avoid race conditions where the main request finishes first by making sure they execute + // sequentially using a single threaded executor. + private fun makeGlideSingleThreadedToOrderThumbnailRequests() { + Glide.init( + context, + GlideBuilder().setSourceExecutor(GlideExecutor.newSourceBuilder().setThreadCount(1).build()), + ) + } + + // Robolectric will produce a Bitmap from any File, but this is relatively easy and will work on + // emulators as well as robolectric. + private fun newImageFile(): File { + val file = temporaryFolder.newFile() + val bitmap = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888) + val canvas = Canvas(bitmap) + canvas.drawColor(Color.GREEN) + file.outputStream().use { + bitmap.compress(Bitmap.CompressFormat.JPEG, 75, it) + } + return file + } +} + +private fun atMostOnce(function: () -> Unit): () -> Unit { + var isCalled = false + return { + if (!isCalled) { + isCalled = true + function() + } + } +} + +private fun onSuccess(onSuccess: () -> Unit) = + simpleRequestListener(onSuccess) {} + +private fun onFailure(onFailure: () -> Unit) = + simpleRequestListener({}, onFailure) + +private fun simpleRequestListener(onSuccess: () -> Unit, onFailure: () -> Unit) : + RequestListener = + object : RequestListener { + override fun onResourceReady( + resource: ResourceT?, + model: Any?, + target: Target?, + dataSource: DataSource?, + isFirstResource: Boolean, + ): Boolean { + onSuccess() + return false + } + + override fun onLoadFailed( + e: GlideException?, + model: Any?, + target: Target?, + isFirstResource: Boolean, + ): Boolean { + onFailure() + return false + } + } + +// TODO(judds): This function may be useful in production code as well, consider exposing it. +@OptIn(ExperimentGlideFlows::class) +private fun Flow>.firstLoad(): + Flow> +{ + val originalFlow = this + return flow { + var completion: GlideFlowInstant? = null + originalFlow.takeWhile { + if (it.status != Status.SUCCEEDED && it.status != Status.FAILED) { + true + } else { + completion = it + false + } + } + .collect { emit(it) } + + emit(completion!!) + } +} + +@OptIn(DelicateCoroutinesApi::class) +private fun restartAllRequestsOnNewThread(requestManager: RequestManager) = + newSingleThreadContext("restart").use { + it.executor.execute { + requestManager.pauseAllRequests() + requestManager.resumeRequests() + } + } + +@OptIn(ExperimentGlideFlows::class) +private fun placeholder(status: Status) = StatusAndType(status, Placeholder::class) +@OptIn(ExperimentGlideFlows::class) +private fun resource(status: Status) = StatusAndType(status, Resource::class) + +@OptIn(ExperimentGlideFlows::class) +private data class StatusAndType( + val status: Status, val type: KClass>, +) + +@OptIn(ExperimentGlideFlows::class) +private fun IterableSubject.compareStatusAndType() + = comparingElementsUsing(statusAndType()) + +@OptIn(ExperimentGlideFlows::class) +private fun statusAndType() : Correspondence, StatusAndType> = + ktCorrespondenceFrom("statusAndType") { actual, expected -> actual?.statusAndType() == expected } + +@OptIn(ExperimentGlideFlows::class) +private fun GlideFlowInstant<*>.statusAndType() = + StatusAndType( + status, + when(this) { + is Placeholder<*> -> Placeholder::class + is Resource<*> -> Resource::class + } + ) + +@OptIn(ExperimentGlideFlows::class) +private fun IterableSubject.comparingStatus() = + comparingElementsUsing(status()) + +@OptIn(ExperimentGlideFlows::class) +private fun status() : Correspondence, Status> = + ktCorrespondenceFrom("status") { actual, expected -> actual?.status == expected } + +private fun ktCorrespondenceFrom( + description: String, predicate: Correspondence.BinaryPredicate +) = Correspondence.from(predicate, description) diff --git a/library/src/main/java/com/bumptech/glide/RequestBuilder.java b/library/src/main/java/com/bumptech/glide/RequestBuilder.java index a592e119ac..9341ad790e 100644 --- a/library/src/main/java/com/bumptech/glide/RequestBuilder.java +++ b/library/src/main/java/com/bumptech/glide/RequestBuilder.java @@ -36,7 +36,6 @@ import com.bumptech.glide.signature.AndroidResourceSignature; import com.bumptech.glide.util.Executors; import com.bumptech.glide.util.Preconditions; -import com.bumptech.glide.util.Synthetic; import com.bumptech.glide.util.Util; import java.io.File; import java.net.URL; @@ -102,6 +101,10 @@ protected RequestBuilder( apply(requestManager.getDefaultRequestOptions()); } + RequestManager getRequestManager() { + return requestManager; + } + @SuppressLint("CheckResult") @SuppressWarnings("PMD.ConstructorCallsOverridableMethod") protected RequestBuilder(Class transcodeClass, RequestBuilder other) { @@ -772,7 +775,6 @@ public > Y into(@NonNull Y target) { } @NonNull - @Synthetic > Y into( @NonNull Y target, @Nullable RequestListener targetListener, diff --git a/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java b/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java index 96d536da74..7f51edfb49 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java @@ -95,6 +95,8 @@ public void pauseRequests() { public void pauseAllRequests() { isPaused = true; for (Request request : Util.getSnapshot(requests)) { + // TODO(judds): Failed requests return false from isComplete(). They're still restarted in + // resumeRequests, but they're not cleared here. We should probably clear all requests here? if (request.isRunning() || request.isComplete()) { request.clear(); pendingRequests.add(request); diff --git a/library/test/build.gradle b/library/test/build.gradle index a298c55c52..247205cd55 100644 --- a/library/test/build.gradle +++ b/library/test/build.gradle @@ -13,7 +13,8 @@ dependencies { testImplementation "com.squareup.okhttp3:mockwebserver:${MOCKWEBSERVER_VERSION}" testImplementation "androidx.test:core:${ANDROID_X_TEST_CORE_VERSION}" testImplementation "androidx.test.ext:junit:${ANDROID_X_TEST_JUNIT_VERSION}" - testImplementation "androidx.test:runner:${ANDROID_X_TEST_RUNNER_VERSION}"} + testImplementation "androidx.test:runner:${ANDROID_X_TEST_RUNNER_VERSION}" +} tasks.withType(JavaCompile) { options.fork = true diff --git a/settings.gradle b/settings.gradle index f66291a08e..00fa14b380 100644 --- a/settings.gradle +++ b/settings.gradle @@ -26,6 +26,7 @@ include ':integration:avif' include ':integration:concurrent' include ':integration:cronet' include ':integration:gifencoder' +include ':integration:ktx' include ':integration:okhttp' include ':integration:okhttp3' include ':integration:recyclerview'