diff options
8 files changed, 312 insertions, 59 deletions
diff --git a/src/main/java/com/android/volley/CacheDispatcher.java b/src/main/java/com/android/volley/CacheDispatcher.java index f616285..13f250b 100644 --- a/src/main/java/com/android/volley/CacheDispatcher.java +++ b/src/main/java/com/android/volley/CacheDispatcher.java @@ -122,75 +122,80 @@ public class CacheDispatcher extends Thread { @VisibleForTesting void processRequest(final Request<?> request) throws InterruptedException { request.addMarker("cache-queue-take"); + request.sendEvent(RequestQueue.RequestEvent.REQUEST_CACHE_LOOKUP_STARTED); - // If the request has been canceled, don't bother dispatching it. - if (request.isCanceled()) { - request.finish("cache-discard-canceled"); - return; - } + try { + // If the request has been canceled, don't bother dispatching it. + if (request.isCanceled()) { + request.finish("cache-discard-canceled"); + return; + } - // Attempt to retrieve this item from cache. - Cache.Entry entry = mCache.get(request.getCacheKey()); - if (entry == null) { - request.addMarker("cache-miss"); - // Cache miss; send off to the network dispatcher. - if (!mWaitingRequestManager.maybeAddToWaitingRequests(request)) { - mNetworkQueue.put(request); + // Attempt to retrieve this item from cache. + Cache.Entry entry = mCache.get(request.getCacheKey()); + if (entry == null) { + request.addMarker("cache-miss"); + // Cache miss; send off to the network dispatcher. + if (!mWaitingRequestManager.maybeAddToWaitingRequests(request)) { + mNetworkQueue.put(request); + } + return; } - return; - } - // If it is completely expired, just send it to the network. - if (entry.isExpired()) { - request.addMarker("cache-hit-expired"); - request.setCacheEntry(entry); - if (!mWaitingRequestManager.maybeAddToWaitingRequests(request)) { - mNetworkQueue.put(request); + // If it is completely expired, just send it to the network. + if (entry.isExpired()) { + request.addMarker("cache-hit-expired"); + request.setCacheEntry(entry); + if (!mWaitingRequestManager.maybeAddToWaitingRequests(request)) { + mNetworkQueue.put(request); + } + return; } - return; - } - // We have a cache hit; parse its data for delivery back to the request. - request.addMarker("cache-hit"); - Response<?> response = - request.parseNetworkResponse( - new NetworkResponse(entry.data, entry.responseHeaders)); - request.addMarker("cache-hit-parsed"); + // We have a cache hit; parse its data for delivery back to the request. + request.addMarker("cache-hit"); + Response<?> response = + request.parseNetworkResponse( + new NetworkResponse(entry.data, entry.responseHeaders)); + request.addMarker("cache-hit-parsed"); - if (!entry.refreshNeeded()) { - // Completely unexpired cache hit. Just deliver the response. - mDelivery.postResponse(request, response); - } else { - // Soft-expired cache hit. We can deliver the cached response, - // but we need to also send the request to the network for - // refreshing. - request.addMarker("cache-hit-refresh-needed"); - request.setCacheEntry(entry); - // Mark the response as intermediate. - response.intermediate = true; + if (!entry.refreshNeeded()) { + // Completely unexpired cache hit. Just deliver the response. + mDelivery.postResponse(request, response); + } else { + // Soft-expired cache hit. We can deliver the cached response, + // but we need to also send the request to the network for + // refreshing. + request.addMarker("cache-hit-refresh-needed"); + request.setCacheEntry(entry); + // Mark the response as intermediate. + response.intermediate = true; - if (!mWaitingRequestManager.maybeAddToWaitingRequests(request)) { - // Post the intermediate response back to the user and have - // the delivery then forward the request along to the network. - mDelivery.postResponse( - request, - response, - new Runnable() { - @Override - public void run() { - try { - mNetworkQueue.put(request); - } catch (InterruptedException e) { - // Restore the interrupted status - Thread.currentThread().interrupt(); + if (!mWaitingRequestManager.maybeAddToWaitingRequests(request)) { + // Post the intermediate response back to the user and have + // the delivery then forward the request along to the network. + mDelivery.postResponse( + request, + response, + new Runnable() { + @Override + public void run() { + try { + mNetworkQueue.put(request); + } catch (InterruptedException e) { + // Restore the interrupted status + Thread.currentThread().interrupt(); + } } - } - }); - } else { - // request has been added to list of waiting requests - // to receive the network response from the first request once it returns. - mDelivery.postResponse(request, response); + }); + } else { + // request has been added to list of waiting requests + // to receive the network response from the first request once it returns. + mDelivery.postResponse(request, response); + } } + } finally { + request.sendEvent(RequestQueue.RequestEvent.REQUEST_CACHE_LOOKUP_FINISHED); } } diff --git a/src/main/java/com/android/volley/NetworkDispatcher.java b/src/main/java/com/android/volley/NetworkDispatcher.java index 327afed..762e030 100644 --- a/src/main/java/com/android/volley/NetworkDispatcher.java +++ b/src/main/java/com/android/volley/NetworkDispatcher.java @@ -114,6 +114,7 @@ public class NetworkDispatcher extends Thread { @VisibleForTesting void processRequest(Request<?> request) { long startTimeMs = SystemClock.elapsedRealtime(); + request.sendEvent(RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_STARTED); try { request.addMarker("network-queue-take"); @@ -164,6 +165,8 @@ public class NetworkDispatcher extends Thread { volleyError.setNetworkTimeMs(SystemClock.elapsedRealtime() - startTimeMs); mDelivery.postError(request, volleyError); request.notifyListenerResponseNotUsable(); + } finally { + request.sendEvent(RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_FINISHED); } } diff --git a/src/main/java/com/android/volley/Request.java b/src/main/java/com/android/volley/Request.java index cd7290a..0b18abb 100644 --- a/src/main/java/com/android/volley/Request.java +++ b/src/main/java/com/android/volley/Request.java @@ -251,6 +251,12 @@ public abstract class Request<T> implements Comparable<Request<T>> { } } + void sendEvent(@RequestQueue.RequestEvent int event) { + if (mRequestQueue != null) { + mRequestQueue.sendRequestEvent(this, event); + } + } + /** * Associates this request with the given queue. The request queue will be notified when this * request has finished. diff --git a/src/main/java/com/android/volley/RequestQueue.java b/src/main/java/com/android/volley/RequestQueue.java index a9312be..42b3fa2 100644 --- a/src/main/java/com/android/volley/RequestQueue.java +++ b/src/main/java/com/android/volley/RequestQueue.java @@ -18,6 +18,9 @@ package com.android.volley; import android.os.Handler; import android.os.Looper; +import android.support.annotation.IntDef; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -38,11 +41,53 @@ public class RequestQueue { // TODO: This should not be a generic class, because the request type can't be determined at // compile time, so all calls to onRequestFinished are unsafe. However, changing this would be // an API-breaking change. See also: https://github.com/google/volley/pull/109 + @Deprecated // Use RequestEventListener instead. public interface RequestFinishedListener<T> { /** Called when a request has finished processing. */ void onRequestFinished(Request<T> request); } + /** Request event types the listeners {@link RequestEventListener} will be notified about. */ + @Retention(RetentionPolicy.SOURCE) + @IntDef({ + RequestEvent.REQUEST_QUEUED, + RequestEvent.REQUEST_CACHE_LOOKUP_STARTED, + RequestEvent.REQUEST_CACHE_LOOKUP_FINISHED, + RequestEvent.REQUEST_NETWORK_DISPATCH_STARTED, + RequestEvent.REQUEST_NETWORK_DISPATCH_FINISHED, + RequestEvent.REQUEST_FINISHED + }) + public @interface RequestEvent { + /** The request was added to the queue. */ + public static final int REQUEST_QUEUED = 0; + /** Cache lookup started for the request. */ + public static final int REQUEST_CACHE_LOOKUP_STARTED = 1; + /** + * Cache lookup finished for the request and cached response is delivered or request is + * queued for network dispatching. + */ + public static final int REQUEST_CACHE_LOOKUP_FINISHED = 2; + /** Network dispatch started for the request. */ + public static final int REQUEST_NETWORK_DISPATCH_STARTED = 3; + /** The network dispatch finished for the request and response (if any) is delivered. */ + public static final int REQUEST_NETWORK_DISPATCH_FINISHED = 4; + /** + * All the work associated with the request is finished and request is removed from all the + * queues. + */ + public static final int REQUEST_FINISHED = 5; + } + + /** Callback interface for request life cycle events. */ + public interface RequestEventListener { + /** + * Called on every request lifecycle event. Can be called from different threads. The call + * is blocking request processing, so any processing should be kept at minimum or moved to + * another thread. + */ + void onRequestEvent(Request<?> request, @RequestEvent int event); + } + /** Used for generating monotonically-increasing sequence numbers for requests. */ private final AtomicInteger mSequenceGenerator = new AtomicInteger(); @@ -78,6 +123,9 @@ public class RequestQueue { private final List<RequestFinishedListener> mFinishedListeners = new ArrayList<>(); + /** Collection of listeners for request life cycle events. */ + private final List<RequestEventListener> mEventListeners = new ArrayList<>(); + /** * Creates the worker pool. Processing will not begin until {@link #start()} is called. * @@ -213,6 +261,7 @@ public class RequestQueue { // Process requests in the order they are added. request.setSequence(getSequenceNumber()); request.addMarker("add-to-queue"); + sendRequestEvent(request, RequestEvent.REQUEST_QUEUED); // If the request is uncacheable, skip the cache queue and go straight to the network. if (!request.shouldCache()) { @@ -238,8 +287,33 @@ public class RequestQueue { listener.onRequestFinished(request); } } + sendRequestEvent(request, RequestEvent.REQUEST_FINISHED); + } + + /** Sends a request life cycle event to the listeners. */ + void sendRequestEvent(Request<?> request, @RequestEvent int event) { + synchronized (mEventListeners) { + for (RequestEventListener listener : mEventListeners) { + listener.onRequestEvent(request, event); + } + } + } + + /** Add a listener for request life cycle events. */ + public void addRequestEventListener(RequestEventListener listener) { + synchronized (mEventListeners) { + mEventListeners.add(listener); + } + } + + /** Remove a listener for request life cycle events. */ + public void removeRequestEventListener(RequestEventListener listener) { + synchronized (mEventListeners) { + mEventListeners.remove(listener); + } } + @Deprecated // Use RequestEventListener instead. public <T> void addRequestFinishedListener(RequestFinishedListener<T> listener) { synchronized (mFinishedListeners) { mFinishedListeners.add(listener); @@ -247,6 +321,7 @@ public class RequestQueue { } /** Remove a RequestFinishedListener. Has no effect if listener was not previously added. */ + @Deprecated // Use RequestEventListener instead. public <T> void removeRequestFinishedListener(RequestFinishedListener<T> listener) { synchronized (mFinishedListeners) { mFinishedListeners.remove(listener); diff --git a/src/test/java/com/android/volley/CacheDispatcherTest.java b/src/test/java/com/android/volley/CacheDispatcherTest.java index 9c5d3c3..2592a0b 100644 --- a/src/test/java/com/android/volley/CacheDispatcherTest.java +++ b/src/test/java/com/android/volley/CacheDispatcherTest.java @@ -20,6 +20,8 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -32,6 +34,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -45,6 +48,7 @@ public class CacheDispatcherTest { private @Mock BlockingQueue<Request<?>> mNetworkQueue; private @Mock Cache mCache; private @Mock ResponseDelivery mDelivery; + private @Mock Network mNetwork; private StringRequest mRequest; @Before @@ -231,4 +235,23 @@ public class CacheDispatcherTest { verify(mDelivery) .postResponse(any(Request.class), any(Response.class), any(Runnable.class)); } + + @Test + public void processRequestNotifiesListener() throws Exception { + RequestQueue.RequestEventListener listener = mock(RequestQueue.RequestEventListener.class); + RequestQueue queue = new RequestQueue(mCache, mNetwork, 0, mDelivery); + queue.addRequestEventListener(listener); + mRequest.setRequestQueue(queue); + + Cache.Entry entry = CacheTestUtils.makeRandomCacheEntry(null, false, false); + when(mCache.get(anyString())).thenReturn(entry); + mDispatcher.processRequest(mRequest); + + InOrder inOrder = inOrder(listener); + inOrder.verify(listener) + .onRequestEvent(mRequest, RequestQueue.RequestEvent.REQUEST_CACHE_LOOKUP_STARTED); + inOrder.verify(listener) + .onRequestEvent(mRequest, RequestQueue.RequestEvent.REQUEST_CACHE_LOOKUP_FINISHED); + inOrder.verifyNoMoreInteractions(); + } } diff --git a/src/test/java/com/android/volley/NetworkDispatcherTest.java b/src/test/java/com/android/volley/NetworkDispatcherTest.java index 51c6971..74dfe8a 100644 --- a/src/test/java/com/android/volley/NetworkDispatcherTest.java +++ b/src/test/java/com/android/volley/NetworkDispatcherTest.java @@ -21,11 +21,14 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.inOrder; +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 static org.mockito.MockitoAnnotations.initMocks; +import com.android.volley.toolbox.NoCache; import com.android.volley.toolbox.StringRequest; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -34,6 +37,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; import org.mockito.Mock; import org.robolectric.RobolectricTestRunner; @@ -71,6 +75,28 @@ public class NetworkDispatcherTest { } @Test + public void successNotifiesListener() throws Exception { + RequestQueue.RequestEventListener listener = mock(RequestQueue.RequestEventListener.class); + RequestQueue queue = new RequestQueue(new NoCache(), mNetwork, 0, mDelivery); + queue.addRequestEventListener(listener); + mRequest.setRequestQueue(queue); + + when(mNetwork.performRequest(any(Request.class))) + .thenReturn(new NetworkResponse(CANNED_DATA)); + + mDispatcher.processRequest(mRequest); + + InOrder inOrder = inOrder(listener); + inOrder.verify(listener) + .onRequestEvent( + mRequest, RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_STARTED); + inOrder.verify(listener) + .onRequestEvent( + mRequest, RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_FINISHED); + inOrder.verifyNoMoreInteractions(); + } + + @Test public void exceptionPostsError() throws Exception { when(mNetwork.performRequest(any(Request.class))).thenThrow(new ServerError()); mDispatcher.processRequest(mRequest); @@ -80,6 +106,27 @@ public class NetworkDispatcherTest { } @Test + public void exceptionNotifiesListener() throws Exception { + RequestQueue.RequestEventListener listener = mock(RequestQueue.RequestEventListener.class); + RequestQueue queue = new RequestQueue(new NoCache(), mNetwork, 0, mDelivery); + queue.addRequestEventListener(listener); + mRequest.setRequestQueue(queue); + + when(mNetwork.performRequest(any(Request.class))).thenThrow(new ServerError()); + + mDispatcher.processRequest(mRequest); + + InOrder inOrder = inOrder(listener); + inOrder.verify(listener) + .onRequestEvent( + mRequest, RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_STARTED); + inOrder.verify(listener) + .onRequestEvent( + mRequest, RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_FINISHED); + inOrder.verifyNoMoreInteractions(); + } + + @Test public void shouldCacheFalse() throws Exception { mRequest.setShouldCache(false); mDispatcher.processRequest(mRequest); diff --git a/src/test/java/com/android/volley/RequestQueueTest.java b/src/test/java/com/android/volley/RequestQueueTest.java index 11c6fe2..ba9b0f8 100644 --- a/src/test/java/com/android/volley/RequestQueueTest.java +++ b/src/test/java/com/android/volley/RequestQueueTest.java @@ -19,6 +19,7 @@ package com.android.volley; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; @@ -72,4 +73,57 @@ public class RequestQueueTest { verify(req2, never()).cancel(); // B not cancelled verify(req4, never()).cancel(); // A added after cancel not cancelled } + + @Test + public void add_notifiesListener() throws Exception { + RequestQueue.RequestEventListener listener = mock(RequestQueue.RequestEventListener.class); + RequestQueue queue = new RequestQueue(new NoCache(), mMockNetwork, 0, mDelivery); + queue.addRequestEventListener(listener); + StringRequest req = mock(StringRequest.class); + + queue.add(req); + + verify(listener).onRequestEvent(req, RequestQueue.RequestEvent.REQUEST_QUEUED); + verifyNoMoreInteractions(listener); + } + + @Test + public void finish_notifiesListener() throws Exception { + RequestQueue.RequestEventListener listener = mock(RequestQueue.RequestEventListener.class); + RequestQueue queue = new RequestQueue(new NoCache(), mMockNetwork, 0, mDelivery); + queue.addRequestEventListener(listener); + StringRequest req = mock(StringRequest.class); + + queue.finish(req); + + verify(listener).onRequestEvent(req, RequestQueue.RequestEvent.REQUEST_FINISHED); + verifyNoMoreInteractions(listener); + } + + @Test + public void sendRequestEvent_notifiesListener() throws Exception { + StringRequest req = mock(StringRequest.class); + RequestQueue.RequestEventListener listener = mock(RequestQueue.RequestEventListener.class); + RequestQueue queue = new RequestQueue(new NoCache(), mMockNetwork, 0, mDelivery); + queue.addRequestEventListener(listener); + + queue.sendRequestEvent(req, RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_STARTED); + + verify(listener) + .onRequestEvent(req, RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_STARTED); + verifyNoMoreInteractions(listener); + } + + @Test + public void removeRequestEventListener_removesListener() throws Exception { + StringRequest req = mock(StringRequest.class); + RequestQueue.RequestEventListener listener = mock(RequestQueue.RequestEventListener.class); + RequestQueue queue = new RequestQueue(new NoCache(), mMockNetwork, 0, mDelivery); + queue.addRequestEventListener(listener); + queue.removeRequestEventListener(listener); + + queue.sendRequestEvent(req, RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_STARTED); + + verifyNoMoreInteractions(listener); + } } diff --git a/src/test/java/com/android/volley/RequestTest.java b/src/test/java/com/android/volley/RequestTest.java index 382d9da..cced39f 100644 --- a/src/test/java/com/android/volley/RequestTest.java +++ b/src/test/java/com/android/volley/RequestTest.java @@ -19,17 +19,31 @@ package com.android.volley; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.MockitoAnnotations.initMocks; import com.android.volley.Request.Method; import com.android.volley.Request.Priority; +import com.android.volley.toolbox.NoCache; import java.util.Collections; import java.util.Map; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; import org.robolectric.RobolectricTestRunner; @RunWith(RobolectricTestRunner.class) public class RequestTest { + private @Mock ResponseDelivery mDelivery; + private @Mock Network mNetwork; + + @Before + public void setUp() throws Exception { + initMocks(this); + } @Test public void compareTo() { @@ -189,4 +203,30 @@ public class RequestTest { // expected } } + + @Test + public void sendEvent_notifiesListeners() throws Exception { + RequestQueue.RequestEventListener listener = mock(RequestQueue.RequestEventListener.class); + RequestQueue queue = new RequestQueue(new NoCache(), mNetwork, 0, mDelivery); + queue.addRequestEventListener(listener); + + Request<Object> request = + new Request<Object>(Method.POST, "url", null) { + @Override + protected void deliverResponse(Object response) {} + + @Override + protected Response<Object> parseNetworkResponse(NetworkResponse response) { + return null; + } + }; + request.setRequestQueue(queue); + + request.sendEvent(RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_STARTED); + + verify(listener) + .onRequestEvent( + request, RequestQueue.RequestEvent.REQUEST_NETWORK_DISPATCH_STARTED); + verifyNoMoreInteractions(listener); + } } |