Skip to content

Commit

Permalink
Support multiple listeners in a Glide request.
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=191378721
  • Loading branch information
@sjudd
adrianv authored and sjudd committed Apr 25, 2018
1 parent a683e04 commit 9328999
Show file tree
Hide file tree
Showing 4 changed files with 176 additions and 73 deletions.
30 changes: 27 additions & 3 deletions library/src/main/java/com/bumptech/glide/RequestBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import com.bumptech.glide.util.Util;
import java.io.File;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;

/**
* A generic class that can handle setting options and staring loads for generic resource types.
Expand Down Expand Up @@ -66,7 +68,7 @@ public class RequestBuilder<TranscodeType> implements Cloneable,
@Nullable private Object model;
// model may occasionally be null, so to enforce that load() was called, put a boolean rather
// than relying on model not to be null.
@Nullable private RequestListener<TranscodeType> requestListener;
@Nullable private List<RequestListener<TranscodeType>> requestListeners;
@Nullable private RequestBuilder<TranscodeType> thumbnailBuilder;
@Nullable private RequestBuilder<TranscodeType> errorBuilder;
@Nullable private Float thumbSizeMultiplier;
Expand Down Expand Up @@ -144,6 +146,9 @@ public RequestBuilder<TranscodeType> transition(
* 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.
*
* <p>Subsequent calls to this method will replace previously set listeners. To set multiple
* listeners, use {@link #addListener} instead.
*
* @param requestListener The request listener to use.
* @return This request builder.
*/
Expand All @@ -152,8 +157,27 @@ public RequestBuilder<TranscodeType> transition(
@SuppressWarnings("unchecked")
public RequestBuilder<TranscodeType> listener(
@Nullable RequestListener<TranscodeType> requestListener) {
this.requestListener = requestListener;
this.requestListeners = null;
return addListener(requestListener);
}

/**
* Adds a {@link RequestListener}. If called multiple times, all passed
* {@link RequestListener listeners} will be called in order.
*
* @param requestListener The request listener to use. If {@code null}, this method is a noop.
* @return This request builder.
*/
@NonNull
@CheckResult
public RequestBuilder<TranscodeType> addListener(
@Nullable RequestListener<TranscodeType> requestListener) {
if (requestListener != null) {
if (this.requestListeners == null) {
this.requestListeners = new ArrayList<>();
}
this.requestListeners.add(requestListener);
}
return this;
}

Expand Down Expand Up @@ -1041,7 +1065,7 @@ private Request obtainRequest(
priority,
target,
targetListener,
requestListener,
requestListeners,
requestCoordinator,
glideContext.getEngine(),
transitionOptions.getTransitionFactory());
Expand Down
58 changes: 40 additions & 18 deletions library/src/main/java/com/bumptech/glide/request/SingleRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.bumptech.glide.util.Util;
import com.bumptech.glide.util.pool.FactoryPools;
import com.bumptech.glide.util.pool.StateVerifier;
import java.util.List;

/**
* A {@link Request} that loads a {@link com.bumptech.glide.load.engine.Resource} into a given
Expand Down Expand Up @@ -103,7 +104,7 @@ private enum Status {
private int overrideHeight;
private Priority priority;
private Target<R> target;
private RequestListener<R> requestListener;
@Nullable private List<RequestListener<R>> requestListeners;
private Engine engine;
private TransitionFactory<? super R> animationFactory;
private Resource<R> resource;
Expand All @@ -127,7 +128,7 @@ public static <R> SingleRequest<R> obtain(
Priority priority,
Target<R> target,
RequestListener<R> targetListener,
RequestListener<R> requestListener,
@Nullable List<RequestListener<R>> requestListeners,
RequestCoordinator requestCoordinator,
Engine engine,
TransitionFactory<? super R> animationFactory) {
Expand All @@ -147,7 +148,7 @@ public static <R> SingleRequest<R> obtain(
priority,
target,
targetListener,
requestListener,
requestListeners,
requestCoordinator,
engine,
animationFactory);
Expand All @@ -171,7 +172,7 @@ private void init(
Priority priority,
Target<R> target,
RequestListener<R> targetListener,
RequestListener<R> requestListener,
@Nullable List<RequestListener<R>> requestListeners,
RequestCoordinator requestCoordinator,
Engine engine,
TransitionFactory<? super R> animationFactory) {
Expand All @@ -185,7 +186,7 @@ private void init(
this.priority = priority;
this.target = target;
this.targetListener = targetListener;
this.requestListener = requestListener;
this.requestListeners = requestListeners;
this.requestCoordinator = requestCoordinator;
this.engine = engine;
this.animationFactory = animationFactory;
Expand All @@ -209,7 +210,7 @@ public void recycle() {
overrideWidth = -1;
overrideHeight = -1;
target = null;
requestListener = null;
requestListeners = null;
targetListener = null;
requestCoordinator = null;
animationFactory = null;
Expand Down Expand Up @@ -570,10 +571,18 @@ private void onResourceReady(Resource<R> resource, R result, DataSource dataSour

isCallingCallbacks = true;
try {
if ((requestListener == null
|| !requestListener.onResourceReady(result, model, target, dataSource, isFirstResource))
&& (targetListener == null
|| !targetListener.onResourceReady(result, model, target, dataSource, isFirstResource))) {
boolean anyListenerHandledUpdatingTarget = false;
if (requestListeners != null) {
for (RequestListener<R> listener : requestListeners) {
anyListenerHandledUpdatingTarget |=
listener.onResourceReady(result, model, target, dataSource, isFirstResource);
}
}
anyListenerHandledUpdatingTarget |=
targetListener != null
&& targetListener.onResourceReady(result, model, target, dataSource, isFirstResource);

if (!anyListenerHandledUpdatingTarget) {
Transition<? super R> animation =
animationFactory.build(dataSource, isFirstResource);
target.onResourceReady(result, animation);
Expand Down Expand Up @@ -609,10 +618,18 @@ private void onLoadFailed(GlideException e, int maxLogLevel) {
isCallingCallbacks = true;
try {
//TODO: what if this is a thumbnail request?
if ((requestListener == null
|| !requestListener.onLoadFailed(e, model, target, isFirstReadyResource()))
&& (targetListener == null
|| !targetListener.onLoadFailed(e, model, target, isFirstReadyResource()))) {
boolean anyListenerHandledUpdatingTarget = false;
if (requestListeners != null) {
for (RequestListener<R> listener : requestListeners) {
anyListenerHandledUpdatingTarget |=
listener.onLoadFailed(e, model, target, isFirstReadyResource());
}
}
anyListenerHandledUpdatingTarget |=
targetListener != null
&& targetListener.onLoadFailed(e, model, target, isFirstReadyResource());

if (!anyListenerHandledUpdatingTarget) {
setErrorPlaceholder();
}
} finally {
Expand All @@ -633,14 +650,19 @@ public boolean isEquivalentTo(Request o) {
&& requestOptions.equals(that.requestOptions)
&& priority == that.priority
// We do not want to require that RequestListeners implement equals/hashcode, so we don't
// compare them using equals(). We can however, at least assert that the request listener
// is either present or not present in both requests.
&& (requestListener != null
? that.requestListener != null : that.requestListener == null);
// compare them using equals(). We can however, at least assert that the same amount of
// request listeners are present in both requests
&& listenerCountEquals(this, that);
}
return false;
}

private static boolean listenerCountEquals(SingleRequest<?> first, SingleRequest<?> second) {
int firstListenerCount = first.requestListeners == null ? 0 : first.requestListeners.size();
int secondListenerCount = second.requestListeners == null ? 0 : second.requestListeners.size();
return firstListenerCount == secondListenerCount;
}

private void logV(String message) {
Log.v(TAG, message + " this: " + tag);
}
Expand Down
45 changes: 42 additions & 3 deletions library/test/src/test/java/com/bumptech/glide/RequestBuilderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,22 @@

import static com.bumptech.glide.tests.BackgroundUtil.testInBackground;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.eq;
import static org.mockito.Matchers.isA;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import android.app.Application;
import android.widget.ImageView;
import com.bumptech.glide.load.DataSource;
import com.bumptech.glide.load.resource.SimpleResource;
import com.bumptech.glide.request.Request;
import com.bumptech.glide.request.RequestListener;
import com.bumptech.glide.request.RequestOptions;
import com.bumptech.glide.request.SingleRequest;
import com.bumptech.glide.request.target.Target;
import com.bumptech.glide.request.target.ViewTarget;
import com.bumptech.glide.tests.BackgroundUtil.BackgroundTester;
Expand All @@ -20,6 +26,8 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.robolectric.RobolectricTestRunner;
Expand All @@ -32,8 +40,12 @@
public class RequestBuilderTest {
@Rule public TearDownGlide tearDownGlide = new TearDownGlide();

@Mock private RequestListener<Object> listener1;
@Mock private RequestListener<Object> listener2;
@Mock private Target<Object> target;
@Mock private GlideContext glideContext;
@Mock private RequestManager requestManager;
@Captor private ArgumentCaptor<SingleRequest<Object>> requestCaptor;
private Glide glide;
private Application context;

Expand All @@ -57,12 +69,11 @@ public void testThrowsWhenTransitionsOptionsIsNull() {

@Test
public void testDoesNotThrowWithNullModelWhenRequestIsBuilt() {
getNullModelRequest().into(mock(Target.class));
getNullModelRequest().into(target);
}

@Test
public void testAddsNewRequestToRequestTracker() {
Target<Object> target = mock(Target.class);
getNullModelRequest().into(target);

verify(requestManager).track(eq(target), isA(Request.class));
Expand All @@ -71,7 +82,6 @@ public void testAddsNewRequestToRequestTracker() {
@Test
public void testRemovesPreviousRequestFromRequestTracker() {
Request previous = mock(Request.class);
Target<Object> target = mock(Target.class);
when(target.getRequest()).thenReturn(previous);

getNullModelRequest().into(target);
Expand Down Expand Up @@ -112,6 +122,35 @@ public void runTest() {
});
}

@Test
public void testMultipleRequestListeners() {
getNullModelRequest().addListener(listener1).addListener(listener2).into(target);
verify(requestManager).track(any(Target.class), requestCaptor.capture());
requestCaptor.getValue().onResourceReady(new SimpleResource<>(new Object()), DataSource.LOCAL);

verify(listener1)
.onResourceReady(
any(), any(), isA(Target.class), isA(DataSource.class), anyBoolean());
verify(listener2)
.onResourceReady(
any(), any(), isA(Target.class), isA(DataSource.class), anyBoolean());
}

@Test
public void testListenerApiOverridesListeners() {
getNullModelRequest().addListener(listener1).listener(listener2).into(target);
verify(requestManager).track(any(Target.class), requestCaptor.capture());
requestCaptor.getValue().onResourceReady(new SimpleResource<>(new Object()), DataSource.LOCAL);

// The #listener API removes any previous listeners, so the first listener should not be called.
verify(listener1, never())
.onResourceReady(
any(), any(), isA(Target.class), isA(DataSource.class), anyBoolean());
verify(listener2)
.onResourceReady(
any(), any(), isA(Target.class), isA(DataSource.class), anyBoolean());
}

private RequestBuilder<Object> getNullModelRequest() {
when(glideContext.buildImageViewTarget(isA(ImageView.class), isA(Class.class)))
.thenReturn(mock(ViewTarget.class));
Expand Down
Loading

0 comments on commit 9328999

Please sign in to comment.