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'