From b06b0cc2bd79e10a77155466da8a83459edf371b Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Fri, 6 Oct 2017 17:35:54 -0700 Subject: [PATCH] Add API to start a new Request when a load fails. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We’re going with .error() to match the behavior of the .error Drawable and resource id methods. fallback() is a somewhat better name, but we unfortunately already use it to handle cases where models are null and expected to be null. Fixes #451. --- .../com/bumptech/glide/RequestBuilder.java | 80 +++++++++- .../request/ErrorRequestCoordinator.java | 145 ++++++++++++++++++ .../glide/request/RequestCoordinator.java | 5 +- .../bumptech/glide/request/SingleRequest.java | 13 +- .../request/ThumbnailRequestCoordinator.java | 17 +- 5 files changed, 249 insertions(+), 11 deletions(-) create mode 100644 library/src/main/java/com/bumptech/glide/request/ErrorRequestCoordinator.java diff --git a/library/src/main/java/com/bumptech/glide/RequestBuilder.java b/library/src/main/java/com/bumptech/glide/RequestBuilder.java index 6ac3518b6c..37650c3f1c 100644 --- a/library/src/main/java/com/bumptech/glide/RequestBuilder.java +++ b/library/src/main/java/com/bumptech/glide/RequestBuilder.java @@ -8,6 +8,7 @@ import android.support.annotation.Nullable; import android.widget.ImageView; import com.bumptech.glide.load.engine.DiskCacheStrategy; +import com.bumptech.glide.request.ErrorRequestCoordinator; import com.bumptech.glide.request.FutureTarget; import com.bumptech.glide.request.Request; import com.bumptech.glide.request.RequestCoordinator; @@ -55,6 +56,7 @@ public class RequestBuilder implements Cloneable { // than relying on model not to be null. @Nullable private RequestListener requestListener; @Nullable private RequestBuilder thumbnailBuilder; + @Nullable private RequestBuilder errorBuilder; @Nullable private Float thumbSizeMultiplier; private boolean isDefaultTransitionOptionsSet = true; private boolean isModelSet; @@ -115,7 +117,7 @@ public RequestBuilder transition( } /** - * Sets a RequestBuilder listener to monitor the resource load. It's best to create a single + * Sets a {@link RequestListener} to monitor the resource load. It's best to create a single * instance of an exception handler per type of request (usually activity/fragment) rather than * pass one in per request to avoid some redundant object allocation. * @@ -131,6 +133,34 @@ public RequestBuilder listener( return this; } + /** + * Sets a {@link RequestBuilder} that is built and run iff the load started by this + * {@link RequestBuilder} fails. + * + *

If this {@link RequestBuilder} uses a thumbnail that succeeds the given error + * {@link RequestBuilder} will be started anyway if the non-thumbnail request fails. + * + *

Recursive calls to {@link #error(RequestBuilder)} as well as calls to + * {@link #thumbnail(float)} and {@link #thumbnail(RequestBuilder)} are supported for the given + * error {@link RequestBuilder}. + * + *

Unlike {@link #thumbnail(RequestBuilder)} and {@link #thumbnail(float)}, no options from + * this primary {@link RequestBuilder} are propagated to the given error {@link RequestBuilder}. + * Options like priority, override widths and heights and transitions must be applied + * independently to the error builder. + * + *

The given {@link RequestBuilder} will start and potentially override a fallback drawable + * if it's set on this {@link RequestBuilder} via + * {@link RequestOptions#fallback(android.graphics.drawable.Drawable)} or + * {@link RequestOptions#fallback(int)}. + * + * @return This {@link RequestBuilder}. + */ + public RequestBuilder error(@Nullable RequestBuilder errorBuilder) { + this.errorBuilder = errorBuilder; + return this; + } + /** * Loads and displays the resource retrieved by the given thumbnail request if it finishes before * this request. Best used for loading thumbnail resources that are smaller and will be loaded @@ -607,9 +637,55 @@ private Request buildRequest(Target target, RequestOptions reques } private Request buildRequestRecursive(Target target, - @Nullable ThumbnailRequestCoordinator parentCoordinator, + @Nullable RequestCoordinator parentCoordinator, TransitionOptions transitionOptions, Priority priority, int overrideWidth, int overrideHeight, RequestOptions requestOptions) { + + // Build the ErrorRequestCoordinator first if necessary so we can update parentCoordinator. + ErrorRequestCoordinator errorRequestCoordinator = null; + if (errorBuilder != null) { + errorRequestCoordinator = new ErrorRequestCoordinator(parentCoordinator); + parentCoordinator = errorRequestCoordinator; + } + + Request mainRequest = + buildThumbnailRequestRecursive( + target, + parentCoordinator, + transitionOptions, + priority, + overrideWidth, + overrideHeight, + requestOptions); + + if (errorRequestCoordinator == null) { + return mainRequest; + } + + int errorOverrideWidth = errorBuilder.requestOptions.getOverrideWidth(); + int errorOverrideHeight = errorBuilder.requestOptions.getOverrideHeight(); + if (Util.isValidDimensions(overrideWidth, overrideHeight) + && !errorBuilder.requestOptions.isValidOverride()) { + errorOverrideWidth = requestOptions.getOverrideWidth(); + errorOverrideHeight = requestOptions.getOverrideHeight(); + } + + Request errorRequest = errorBuilder.buildRequestRecursive( + target, + errorRequestCoordinator, + errorBuilder.transitionOptions, + errorBuilder.requestOptions.getPriority(), + errorOverrideWidth, + errorOverrideHeight, + errorBuilder.requestOptions); + errorRequestCoordinator.setRequests(mainRequest, errorRequest); + return errorRequestCoordinator; + } + + private Request buildThumbnailRequestRecursive(Target target, + @Nullable RequestCoordinator parentCoordinator, + TransitionOptions transitionOptions, + Priority priority, int overrideWidth, int overrideHeight, RequestOptions requestOptions) { if (thumbnailBuilder != null) { // Recursive case: contains a potentially recursive thumbnail request builder. if (isThumbnailBuilt) { diff --git a/library/src/main/java/com/bumptech/glide/request/ErrorRequestCoordinator.java b/library/src/main/java/com/bumptech/glide/request/ErrorRequestCoordinator.java new file mode 100644 index 0000000000..e1037656b6 --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/request/ErrorRequestCoordinator.java @@ -0,0 +1,145 @@ +package com.bumptech.glide.request; + +/** + * Runs a single primary {@link Request} until it completes and then a fallback error request only + * if the single primary request fails. + */ +public final class ErrorRequestCoordinator implements RequestCoordinator, + Request { + + private final RequestCoordinator coordinator; + private Request primary; + private Request error; + + public ErrorRequestCoordinator(RequestCoordinator coordinator) { + this.coordinator = coordinator; + } + + public void setRequests(Request primary, Request error) { + this.primary = primary; + this.error = error; + } + + @Override + public void begin() { + if (!primary.isRunning()) { + primary.begin(); + } + } + + @Override + public void pause() { + if (!primary.isFailed()) { + primary.pause(); + } + if (error.isRunning()) { + error.pause(); + } + } + + @Override + public void clear() { + if (primary.isFailed()) { + error.clear(); + } else { + primary.clear(); + } + } + + @Override + public boolean isPaused() { + return primary.isFailed() ? error.isPaused() : primary.isPaused(); + } + + @Override + public boolean isRunning() { + return primary.isFailed() ? error.isRunning() : primary.isRunning(); + } + + @Override + public boolean isComplete() { + return primary.isFailed() ? error.isComplete() : primary.isComplete(); + } + + @Override + public boolean isResourceSet() { + return primary.isFailed() ? error.isResourceSet() : primary.isResourceSet(); + } + + @Override + public boolean isCancelled() { + return primary.isFailed() ? error.isCancelled() : primary.isCancelled(); + } + + @Override + public boolean isFailed() { + return primary.isFailed() && error.isFailed(); + } + + @Override + public void recycle() { + primary.recycle(); + error.recycle(); + } + + @Override + public boolean isEquivalentTo(Request o) { + if (o instanceof ErrorRequestCoordinator) { + ErrorRequestCoordinator other = (ErrorRequestCoordinator) o; + return primary.isEquivalentTo(other.primary) && error.isEquivalentTo(other.error); + } + return false; + } + + @Override + public boolean canSetImage(Request request) { + return parentCanSetImage() && isValidRequest(request); + } + + private boolean parentCanSetImage() { + return coordinator == null || coordinator.canSetImage(this); + } + + @Override + public boolean canNotifyStatusChanged(Request request) { + return parentCanNotifyStatusChanged() && isValidRequest(request); + } + + private boolean parentCanNotifyStatusChanged() { + return coordinator == null || coordinator.canNotifyStatusChanged(this); + } + + private boolean isValidRequest(Request request) { + return request.equals(primary) || (primary.isFailed() && request.equals(error)); + } + + @Override + public boolean isAnyResourceSet() { + return parentIsAnyResourceSet() || isResourceSet(); + } + + private boolean parentIsAnyResourceSet() { + return coordinator != null && coordinator.isAnyResourceSet(); + } + + @Override + public void onRequestSuccess(Request request) { + if (coordinator != null) { + coordinator.onRequestSuccess(this); + } + } + + @Override + public void onRequestFailed(Request request) { + if (!request.equals(error)) { + if (!error.isRunning()) { + error.begin(); + } + return; + } + + if (coordinator != null) { + coordinator.onRequestFailed(error); + } + } +} diff --git a/library/src/main/java/com/bumptech/glide/request/RequestCoordinator.java b/library/src/main/java/com/bumptech/glide/request/RequestCoordinator.java index 2143366eb1..36f3915e9f 100644 --- a/library/src/main/java/com/bumptech/glide/request/RequestCoordinator.java +++ b/library/src/main/java/com/bumptech/glide/request/RequestCoordinator.java @@ -28,7 +28,10 @@ public interface RequestCoordinator { boolean isAnyResourceSet(); /** - * Must be called when a request coordinated by this object completes successfully. + * Must be called when a {@link Request} coordinated by this object completes successfully. */ void onRequestSuccess(Request request); + + /** Must be called when a {@link Request} coordinated by this object fails. */ + void onRequestFailed(Request request); } diff --git a/library/src/main/java/com/bumptech/glide/request/SingleRequest.java b/library/src/main/java/com/bumptech/glide/request/SingleRequest.java index 6951e10d65..bd9ee37f79 100644 --- a/library/src/main/java/com/bumptech/glide/request/SingleRequest.java +++ b/library/src/main/java/com/bumptech/glide/request/SingleRequest.java @@ -277,8 +277,9 @@ void cancel() { private void assertNotCallingCallbacks() { if (isCallingCallbacks) { throw new IllegalStateException("You can't start or clear loads in RequestListener or" - + " Target callbacks. If you must do so, consider posting your into() or clear() calls" - + " to the main thread using a Handler instead."); + + " Target callbacks. If you're trying to start a fallback request when a load fails, use" + + " RequestBuilder#error(RequestBuilder). Otherwise consider posting your into() or" + + " clear() calls to the main thread using a Handler instead."); } } @@ -492,6 +493,12 @@ private void notifyLoadSuccess() { } } + private void notifyLoadFailed() { + if (requestCoordinator != null) { + requestCoordinator.onRequestFailed(this); + } + } + /** * A callback method that should never be invoked directly. */ @@ -595,6 +602,8 @@ private void onLoadFailed(GlideException e, int maxLogLevel) { } finally { isCallingCallbacks = false; } + + notifyLoadFailed(); } @Override diff --git a/library/src/main/java/com/bumptech/glide/request/ThumbnailRequestCoordinator.java b/library/src/main/java/com/bumptech/glide/request/ThumbnailRequestCoordinator.java index 6b62270e2f..8bee19db70 100644 --- a/library/src/main/java/com/bumptech/glide/request/ThumbnailRequestCoordinator.java +++ b/library/src/main/java/com/bumptech/glide/request/ThumbnailRequestCoordinator.java @@ -77,6 +77,17 @@ public void onRequestSuccess(Request request) { } } + @Override + public void onRequestFailed(Request request) { + if (!request.equals(full)) { + return; + } + + if (coordinator != null) { + coordinator.onRequestFailed(this); + } + } + private boolean parentIsAnyResourceSet() { return coordinator != null && coordinator.isAnyResourceSet(); } @@ -102,9 +113,6 @@ public void pause() { thumb.pause(); } - /** - * {@inheritDoc} - */ @Override public void clear() { isRunning = false; @@ -151,9 +159,6 @@ public boolean isFailed() { return full.isFailed(); } - /** - * {@inheritDoc}. - */ @Override public void recycle() { full.recycle();