diff --git a/library/src/main/java/com/bumptech/glide/RequestBuilder.java b/library/src/main/java/com/bumptech/glide/RequestBuilder.java index 5e523276fa..6b6a385a99 100644 --- a/library/src/main/java/com/bumptech/glide/RequestBuilder.java +++ b/library/src/main/java/com/bumptech/glide/RequestBuilder.java @@ -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. @@ -66,7 +68,7 @@ public class RequestBuilder 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 requestListener; + @Nullable private List> requestListeners; @Nullable private RequestBuilder thumbnailBuilder; @Nullable private RequestBuilder errorBuilder; @Nullable private Float thumbSizeMultiplier; @@ -144,6 +146,9 @@ public RequestBuilder 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. * + *

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. */ @@ -152,8 +157,27 @@ public RequestBuilder transition( @SuppressWarnings("unchecked") public RequestBuilder listener( @Nullable RequestListener 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 addListener( + @Nullable RequestListener requestListener) { + if (requestListener != null) { + if (this.requestListeners == null) { + this.requestListeners = new ArrayList<>(); + } + this.requestListeners.add(requestListener); + } return this; } @@ -1041,7 +1065,7 @@ private Request obtainRequest( priority, target, targetListener, - requestListener, + requestListeners, requestCoordinator, glideContext.getEngine(), transitionOptions.getTransitionFactory()); 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 1cc122df3e..eee923aa3f 100644 --- a/library/src/main/java/com/bumptech/glide/request/SingleRequest.java +++ b/library/src/main/java/com/bumptech/glide/request/SingleRequest.java @@ -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 @@ -103,7 +104,7 @@ private enum Status { private int overrideHeight; private Priority priority; private Target target; - private RequestListener requestListener; + @Nullable private List> requestListeners; private Engine engine; private TransitionFactory animationFactory; private Resource resource; @@ -127,7 +128,7 @@ public static SingleRequest obtain( Priority priority, Target target, RequestListener targetListener, - RequestListener requestListener, + @Nullable List> requestListeners, RequestCoordinator requestCoordinator, Engine engine, TransitionFactory animationFactory) { @@ -147,7 +148,7 @@ public static SingleRequest obtain( priority, target, targetListener, - requestListener, + requestListeners, requestCoordinator, engine, animationFactory); @@ -171,7 +172,7 @@ private void init( Priority priority, Target target, RequestListener targetListener, - RequestListener requestListener, + @Nullable List> requestListeners, RequestCoordinator requestCoordinator, Engine engine, TransitionFactory animationFactory) { @@ -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; @@ -209,7 +210,7 @@ public void recycle() { overrideWidth = -1; overrideHeight = -1; target = null; - requestListener = null; + requestListeners = null; targetListener = null; requestCoordinator = null; animationFactory = null; @@ -570,10 +571,18 @@ private void onResourceReady(Resource 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 listener : requestListeners) { + anyListenerHandledUpdatingTarget |= + listener.onResourceReady(result, model, target, dataSource, isFirstResource); + } + } + anyListenerHandledUpdatingTarget |= + targetListener != null + && targetListener.onResourceReady(result, model, target, dataSource, isFirstResource); + + if (!anyListenerHandledUpdatingTarget) { Transition animation = animationFactory.build(dataSource, isFirstResource); target.onResourceReady(result, animation); @@ -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 listener : requestListeners) { + anyListenerHandledUpdatingTarget |= + listener.onLoadFailed(e, model, target, isFirstReadyResource()); + } + } + anyListenerHandledUpdatingTarget |= + targetListener != null + && targetListener.onLoadFailed(e, model, target, isFirstReadyResource()); + + if (!anyListenerHandledUpdatingTarget) { setErrorPlaceholder(); } } finally { @@ -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); } diff --git a/library/test/src/test/java/com/bumptech/glide/RequestBuilderTest.java b/library/test/src/test/java/com/bumptech/glide/RequestBuilderTest.java index 10a6bc568e..140bf5b921 100644 --- a/library/test/src/test/java/com/bumptech/glide/RequestBuilderTest.java +++ b/library/test/src/test/java/com/bumptech/glide/RequestBuilderTest.java @@ -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; @@ -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; @@ -32,8 +40,12 @@ public class RequestBuilderTest { @Rule public TearDownGlide tearDownGlide = new TearDownGlide(); + @Mock private RequestListener listener1; + @Mock private RequestListener listener2; + @Mock private Target target; @Mock private GlideContext glideContext; @Mock private RequestManager requestManager; + @Captor private ArgumentCaptor> requestCaptor; private Glide glide; private Application context; @@ -57,12 +69,11 @@ public void testThrowsWhenTransitionsOptionsIsNull() { @Test public void testDoesNotThrowWithNullModelWhenRequestIsBuilt() { - getNullModelRequest().into(mock(Target.class)); + getNullModelRequest().into(target); } @Test public void testAddsNewRequestToRequestTracker() { - Target target = mock(Target.class); getNullModelRequest().into(target); verify(requestManager).track(eq(target), isA(Request.class)); @@ -71,7 +82,6 @@ public void testAddsNewRequestToRequestTracker() { @Test public void testRemovesPreviousRequestFromRequestTracker() { Request previous = mock(Request.class); - Target target = mock(Target.class); when(target.getRequest()).thenReturn(previous); getNullModelRequest().into(target); @@ -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 getNullModelRequest() { when(glideContext.buildImageViewTarget(isA(ImageView.class), isA(Class.class))) .thenReturn(mock(ViewTarget.class)); diff --git a/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java b/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java index b415962507..b5aca57582 100644 --- a/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java +++ b/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java @@ -46,6 +46,8 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.robolectric.RobolectricTestRunner; @@ -57,9 +59,12 @@ public class SingleRequestTest { private SingleRequestBuilder builder; + @Mock private RequestListener listener1; + @Mock private RequestListener listener2; @Before public void setUp() { + MockitoAnnotations.initMocks(this); builder = new SingleRequestBuilder(); } @@ -72,31 +77,31 @@ public void testIsNotCompleteBeforeReceivingResource() { @Test public void testCanHandleNullResources() { - SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); request.onResourceReady(null, DataSource.LOCAL); assertTrue(request.isFailed()); - verify(builder.requestListener).onLoadFailed(isAGlideException(), isA(Number.class), + verify(listener1).onLoadFailed(isAGlideException(), isA(Number.class), eq(builder.target), anyBoolean()); } @Test public void testCanHandleEmptyResources() { - SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); when(builder.resource.get()).thenReturn(null); request.onResourceReady(builder.resource, DataSource.REMOTE); assertTrue(request.isFailed()); verify(builder.engine).release(eq(builder.resource)); - verify(builder.requestListener).onLoadFailed(isAGlideException(), any(Number.class), + verify(listener1).onLoadFailed(isAGlideException(), any(Number.class), eq(builder.target), anyBoolean()); } @Test public void testCanHandleNonConformingResources() { - SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); when(((Resource) (builder.resource)).get()) .thenReturn("Invalid mocked String, this should be a List"); @@ -104,7 +109,7 @@ public void testCanHandleNonConformingResources() { assertTrue(request.isFailed()); verify(builder.engine).release(eq(builder.resource)); - verify(builder.requestListener).onLoadFailed(isAGlideException(), any(Number.class), + verify(listener1).onLoadFailed(isAGlideException(), any(Number.class), eq(builder.target), anyBoolean()); } @@ -453,18 +458,21 @@ public void testIsNotRunningAfterClear() { @Test public void testCallsTargetOnResourceReadyIfNoRequestListener() { - SingleRequest request = builder - .setRequestListener(null) - .build(); + SingleRequest request = builder.build(); request.onResourceReady(builder.resource, DataSource.LOCAL); verify(builder.target).onResourceReady(eq(builder.result), anyTransition()); } @Test - public void testCallsTargetOnResourceReadyIfRequestListenerReturnsFalse() { - SingleRequest request = builder.build(); - when(builder.requestListener + public void testCallsTargetOnResourceReadyIfAllRequestListenersReturnFalse() { + SingleRequest request = + builder.addRequestListener(listener1).addRequestListener(listener2).build(); + + when(listener1 + .onResourceReady(any(List.class), any(Number.class), eq(builder.target), isADataSource(), + anyBoolean())).thenReturn(false); + when(listener2 .onResourceReady(any(List.class), any(Number.class), eq(builder.target), isADataSource(), anyBoolean())).thenReturn(false); request.onResourceReady(builder.resource, DataSource.LOCAL); @@ -473,9 +481,14 @@ public void testCallsTargetOnResourceReadyIfRequestListenerReturnsFalse() { } @Test - public void testDoesNotCallTargetOnResourceReadyIfRequestListenerReturnsTrue() { - SingleRequest request = builder.build(); - when(builder.requestListener + public void testDoesNotCallTargetOnResourceReadyIfAnyRequestListenerReturnsTrue() { + SingleRequest request = + builder.addRequestListener(listener1).addRequestListener(listener2).build(); + + when(listener1 + .onResourceReady(any(List.class), any(Number.class), eq(builder.target), isADataSource(), + anyBoolean())).thenReturn(false); + when(listener1 .onResourceReady(any(List.class), any(Number.class), eq(builder.target), isADataSource(), anyBoolean())).thenReturn(true); request.onResourceReady(builder.resource, DataSource.REMOTE); @@ -485,18 +498,21 @@ public void testDoesNotCallTargetOnResourceReadyIfRequestListenerReturnsTrue() { @Test public void testCallsTargetOnExceptionIfNoRequestListener() { - SingleRequest request = builder - .setRequestListener(null) - .build(); + SingleRequest request = builder.build(); request.onLoadFailed(new GlideException("test")); verify(builder.target).onLoadFailed(eq(builder.errorDrawable)); } @Test - public void testCallsTargetOnExceptionIfRequestListenerReturnsFalse() { - SingleRequest request = builder.build(); - when(builder.requestListener.onLoadFailed(isAGlideException(), any(Number.class), + public void testCallsTargetOnExceptionIfAllRequestListenersReturnFalse() { + SingleRequest request = + builder.addRequestListener(listener1).addRequestListener(listener2).build(); + + when(listener1.onLoadFailed(isAGlideException(), any(Number.class), + eq(builder.target), anyBoolean())) + .thenReturn(false); + when(listener2.onLoadFailed(isAGlideException(), any(Number.class), eq(builder.target), anyBoolean())) .thenReturn(false); request.onLoadFailed(new GlideException("test")); @@ -505,9 +521,14 @@ public void testCallsTargetOnExceptionIfRequestListenerReturnsFalse() { } @Test - public void testDoesNotCallTargetOnExceptionIfRequestListenerReturnsTrue() { - SingleRequest request = builder.build(); - when(builder.requestListener.onLoadFailed(isAGlideException(), any(Number.class), + public void testDoesNotCallTargetOnExceptionIfAnyRequestListenerReturnsTrue() { + SingleRequest request = + builder.addRequestListener(listener1).addRequestListener(listener2).build(); + + when(listener1.onLoadFailed(isAGlideException(), any(Number.class), + eq(builder.target), anyBoolean())) + .thenReturn(false); + when(listener2.onLoadFailed(isAGlideException(), any(Number.class), eq(builder.target), anyBoolean())) .thenReturn(true); @@ -518,37 +539,37 @@ public void testDoesNotCallTargetOnExceptionIfRequestListenerReturnsTrue() { @Test public void testRequestListenerIsCalledWithResourceResult() { - SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); request.onResourceReady(builder.resource, DataSource.DATA_DISK_CACHE); - verify(builder.requestListener) + verify(listener1) .onResourceReady(eq(builder.result), any(Number.class), isAListTarget(), isADataSource(), anyBoolean()); } @Test public void testRequestListenerIsCalledWithModel() { - SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); request.onResourceReady(builder.resource, DataSource.DATA_DISK_CACHE); - verify(builder.requestListener) + verify(listener1) .onResourceReady(any(List.class), eq(builder.model), isAListTarget(), isADataSource(), anyBoolean()); } @Test public void testRequestListenerIsCalledWithTarget() { - SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); request.onResourceReady(builder.resource, DataSource.DATA_DISK_CACHE); - verify(builder.requestListener) + verify(listener1) .onResourceReady(any(List.class), any(Number.class), eq(builder.target), isADataSource(), anyBoolean()); } @Test public void testRequestListenerIsCalledWithLoadedFromMemoryIfLoadCompletesSynchronously() { - final SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); when(builder.engine .load( @@ -580,7 +601,7 @@ public Object answer(InvocationOnMock invocation) { request.begin(); request.onSizeReady(100, 100); - verify(builder.requestListener) + verify(listener1) .onResourceReady(eq(builder.result), any(Number.class), isAListTarget(), eq(DataSource.MEMORY_CACHE), anyBoolean()); } @@ -588,11 +609,11 @@ public Object answer(InvocationOnMock invocation) { @Test public void testRequestListenerIsCalledWithNotLoadedFromMemoryCacheIfLoadCompletesAsynchronously() { - SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); request.onSizeReady(100, 100); request.onResourceReady(builder.resource, DataSource.LOCAL); - verify(builder.requestListener) + verify(listener1) .onResourceReady(eq(builder.result), any(Number.class), isAListTarget(), eq(DataSource.LOCAL), anyBoolean()); } @@ -601,21 +622,22 @@ public Object answer(InvocationOnMock invocation) { public void testRequestListenerIsCalledWithIsFirstResourceIfNoRequestCoordinator() { SingleRequest request = builder .setRequestCoordinator(null) + .addRequestListener(listener1) .build(); request.onResourceReady(builder.resource, DataSource.DATA_DISK_CACHE); - verify(builder.requestListener) + verify(listener1) .onResourceReady(eq(builder.result), any(Number.class), isAListTarget(), isADataSource(), eq(true)); } @Test public void testRequestListenerIsCalledWithFirstImageIfRequestCoordinatorReturnsNoResourceSet() { - SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); when(builder.requestCoordinator.isAnyResourceSet()).thenReturn(false); request.onResourceReady(builder.resource, DataSource.DATA_DISK_CACHE); - verify(builder.requestListener) + verify(listener1) .onResourceReady(eq(builder.result), any(Number.class), isAListTarget(), isADataSource(), eq(true)); } @@ -623,11 +645,11 @@ public void testRequestListenerIsCalledWithFirstImageIfRequestCoordinatorReturns @Test public void testRequestListenerIsCalledWithNotIsFirstRequestIfRequestCoordinatorReturnsResourceSet() { - SingleRequest request = builder.build(); + SingleRequest request = builder.addRequestListener(listener1).build(); when(builder.requestCoordinator.isAnyResourceSet()).thenReturn(true); request.onResourceReady(builder.resource, DataSource.DATA_DISK_CACHE); - verify(builder.requestListener) + verify(listener1) .onResourceReady(eq(builder.result), any(Number.class), isAListTarget(), isADataSource(), eq(false)); } @@ -872,13 +894,9 @@ protected int doHash(@NonNull SingleRequestBuilder listSingleRequest) { }); tester .addEquivalenceGroup( - new SingleRequestBuilder(), - new SingleRequestBuilder(), // Non-null request listeners are treated as equivalent, even if they're not equal. - new SingleRequestBuilder().setRequestListener(mock(RequestListener.class))) - .addEquivalenceGroup( - new SingleRequestBuilder().setRequestListener(null), - new SingleRequestBuilder().setRequestListener(null)) + new SingleRequestBuilder().addRequestListener(listener1), + new SingleRequestBuilder().addRequestListener(listener2)) .addEquivalenceGroup( new SingleRequestBuilder().setOverrideHeight(500), new SingleRequestBuilder().setOverrideHeight(500)) @@ -911,7 +929,7 @@ static final class SingleRequestBuilder { private Drawable errorDrawable = null; private Drawable fallbackDrawable = null; @SuppressWarnings("unchecked") - private RequestListener requestListener = mock(RequestListener.class); + private List> requestListeners = new ArrayList<>(); @SuppressWarnings("unchecked") private final TransitionFactory transitionFactory = mock(TransitionFactory.class); private int overrideWidth = -1; @@ -971,8 +989,8 @@ SingleRequestBuilder setFallbackDrawable(Drawable fallbackDrawable) { return this; } - SingleRequestBuilder setRequestListener(RequestListener requestListener) { - this.requestListener = requestListener; + SingleRequestBuilder addRequestListener(RequestListener requestListener) { + this.requestListeners.add(requestListener); return this; } @@ -1022,7 +1040,7 @@ SingleRequest build() { priority, target, /*targetListener=*/ null, - requestListener, + requestListeners, requestCoordinator, engine, transitionFactory);