diff options
author | Jeff Davidson <jpd@google.com> | 2015-10-15 15:26:10 -0700 |
---|---|---|
committer | Jeff Davidson <jpd@google.com> | 2016-03-10 13:49:44 -0800 |
commit | dd439dcb42093fde4452e320f3a9c4bfa2066def (patch) | |
tree | 2c11e4241d60b61edddbc842f82cf6aeeed73b3e /src/test/java | |
parent | f605da3d9e6590351cb0bb26bb6ba5146952777c (diff) | |
download | volley-dd439dcb42093fde4452e320f3a9c4bfa2066def.tar.gz |
Retry more errors in Volley's BasicNetwork.
Always retry I/O errors while reading the HTTP response entity.
Furthermore, if a client opts in, retry HTTP 500 errors indicating
something went wrong on the server.
This resolves a longstanding TODO to only throw a ServerError on 5xx
errors by adding a ClientError for 4xx errors. For backwards
compatibility, this extends ServerError.
Note that Volley already may retry a request that reached the server
if the connection times out, which means that lack of idempotency
shouldn't be a concern here if it wasn't already. But if we wanted to
be even safer, we could require clients to opt into the additional
retry cases, at the cost of a somewhat more polluted API.
Add unit tests for most failure scenarios.
Bug: 23152983
Change-Id: I92cf35c66ccf98a1682adf41654afeb8634911db
Diffstat (limited to 'src/test/java')
-rw-r--r-- | src/test/java/com/android/volley/mock/MockHttpStack.java | 12 | ||||
-rw-r--r-- | src/test/java/com/android/volley/toolbox/BasicNetworkTest.java | 211 |
2 files changed, 214 insertions, 9 deletions
diff --git a/src/test/java/com/android/volley/mock/MockHttpStack.java b/src/test/java/com/android/volley/mock/MockHttpStack.java index 9594fde..91872d3 100644 --- a/src/test/java/com/android/volley/mock/MockHttpStack.java +++ b/src/test/java/com/android/volley/mock/MockHttpStack.java @@ -22,6 +22,7 @@ import com.android.volley.toolbox.HttpStack; import org.apache.http.HttpResponse; +import java.io.IOException; import java.util.HashMap; import java.util.Map; @@ -29,6 +30,8 @@ public class MockHttpStack implements HttpStack { private HttpResponse mResponseToReturn; + private IOException mExceptionToThrow; + private String mLastUrl; private Map<String, String> mLastHeaders; @@ -51,9 +54,16 @@ public class MockHttpStack implements HttpStack { mResponseToReturn = response; } + public void setExceptionToThrow(IOException exception) { + mExceptionToThrow = exception; + } + @Override public HttpResponse performRequest(Request<?> request, Map<String, String> additionalHeaders) - throws AuthFailureError { + throws IOException, AuthFailureError { + if (mExceptionToThrow != null) { + throw mExceptionToThrow; + } mLastUrl = request.getUrl(); mLastHeaders = new HashMap<String, String>(); if (request.getHeaders() != null) { diff --git a/src/test/java/com/android/volley/toolbox/BasicNetworkTest.java b/src/test/java/com/android/volley/toolbox/BasicNetworkTest.java index 89718b1..c01d9b0 100644 --- a/src/test/java/com/android/volley/toolbox/BasicNetworkTest.java +++ b/src/test/java/com/android/volley/toolbox/BasicNetworkTest.java @@ -16,29 +16,46 @@ package com.android.volley.toolbox; +import com.android.volley.AuthFailureError; import com.android.volley.NetworkResponse; import com.android.volley.Request; import com.android.volley.Response; +import com.android.volley.RetryPolicy; +import com.android.volley.ServerError; +import com.android.volley.TimeoutError; +import com.android.volley.VolleyError; import com.android.volley.mock.MockHttpStack; import org.apache.http.ProtocolVersion; +import org.apache.http.conn.ConnectTimeoutException; import org.apache.http.entity.StringEntity; import org.apache.http.message.BasicHttpResponse; - -import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; import org.robolectric.RobolectricTestRunner; -import static org.junit.Assert.*; - +import java.io.IOException; +import java.net.SocketTimeoutException; import java.util.HashMap; import java.util.Map; +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; +import static org.mockito.MockitoAnnotations.initMocks; + @RunWith(RobolectricTestRunner.class) public class BasicNetworkTest { + @Mock private Request<String> mMockRequest; + @Mock private RetryPolicy mMockRetryPolicy; + private BasicNetwork mNetwork; + + @Before public void setUp() throws Exception { + initMocks(this); + } + @Test public void headersAndPostParams() throws Exception { MockHttpStack mockHttpStack = new MockHttpStack(); BasicHttpResponse fakeResponse = new BasicHttpResponse(new ProtocolVersion("HTTP", 1, 1), @@ -46,7 +63,188 @@ public class BasicNetworkTest { fakeResponse.setEntity(new StringEntity("foobar")); mockHttpStack.setResponseToReturn(fakeResponse); BasicNetwork httpNetwork = new BasicNetwork(mockHttpStack); - Request<String> request = new Request<String>(Request.Method.GET, "http://foo", null) { + Request<String> request = buildRequest(); + httpNetwork.performRequest(request); + assertEquals("foo", mockHttpStack.getLastHeaders().get("requestheader")); + assertEquals("requestpost=foo&", new String(mockHttpStack.getLastPostBody())); + } + + @Test public void socketTimeout() throws Exception { + MockHttpStack mockHttpStack = new MockHttpStack(); + mockHttpStack.setExceptionToThrow(new SocketTimeoutException()); + BasicNetwork httpNetwork = new BasicNetwork(mockHttpStack); + Request<String> request = buildRequest(); + request.setRetryPolicy(mMockRetryPolicy); + doThrow(new VolleyError()).when(mMockRetryPolicy).retry(any(VolleyError.class)); + try { + httpNetwork.performRequest(request); + } catch (VolleyError e) { + // expected + } + // should retry socket timeouts + verify(mMockRetryPolicy).retry(any(TimeoutError.class)); + } + + @Test public void connectTimeout() throws Exception { + MockHttpStack mockHttpStack = new MockHttpStack(); + mockHttpStack.setExceptionToThrow(new ConnectTimeoutException()); + BasicNetwork httpNetwork = new BasicNetwork(mockHttpStack); + Request<String> request = buildRequest(); + request.setRetryPolicy(mMockRetryPolicy); + doThrow(new VolleyError()).when(mMockRetryPolicy).retry(any(VolleyError.class)); + try { + httpNetwork.performRequest(request); + } catch (VolleyError e) { + // expected + } + // should retry connection timeouts + verify(mMockRetryPolicy).retry(any(TimeoutError.class)); + } + + @Test public void noConnection() throws Exception { + MockHttpStack mockHttpStack = new MockHttpStack(); + mockHttpStack.setExceptionToThrow(new IOException()); + BasicNetwork httpNetwork = new BasicNetwork(mockHttpStack); + Request<String> request = buildRequest(); + request.setRetryPolicy(mMockRetryPolicy); + doThrow(new VolleyError()).when(mMockRetryPolicy).retry(any(VolleyError.class)); + try { + httpNetwork.performRequest(request); + } catch (VolleyError e) { + // expected + } + // should not retry when there is no connection + verify(mMockRetryPolicy, never()).retry(any(VolleyError.class)); + } + + @Test public void unauthorized() throws Exception { + MockHttpStack mockHttpStack = new MockHttpStack(); + BasicHttpResponse fakeResponse = new BasicHttpResponse(new ProtocolVersion("HTTP", 1, 1), + 401, "Unauthorized"); + mockHttpStack.setResponseToReturn(fakeResponse); + BasicNetwork httpNetwork = new BasicNetwork(mockHttpStack); + Request<String> request = buildRequest(); + request.setRetryPolicy(mMockRetryPolicy); + doThrow(new VolleyError()).when(mMockRetryPolicy).retry(any(VolleyError.class)); + try { + httpNetwork.performRequest(request); + } catch (VolleyError e) { + // expected + } + // should retry in case it's an auth failure. + verify(mMockRetryPolicy).retry(any(AuthFailureError.class)); + } + + @Test public void forbidden() throws Exception { + MockHttpStack mockHttpStack = new MockHttpStack(); + BasicHttpResponse fakeResponse = new BasicHttpResponse(new ProtocolVersion("HTTP", 1, 1), + 403, "Forbidden"); + mockHttpStack.setResponseToReturn(fakeResponse); + BasicNetwork httpNetwork = new BasicNetwork(mockHttpStack); + Request<String> request = buildRequest(); + request.setRetryPolicy(mMockRetryPolicy); + doThrow(new VolleyError()).when(mMockRetryPolicy).retry(any(VolleyError.class)); + try { + httpNetwork.performRequest(request); + } catch (VolleyError e) { + // expected + } + // should retry in case it's an auth failure. + verify(mMockRetryPolicy).retry(any(AuthFailureError.class)); + } + + @Test public void redirect() throws Exception { + for (int i = 300; i <= 399; i++) { + MockHttpStack mockHttpStack = new MockHttpStack(); + BasicHttpResponse fakeResponse = + new BasicHttpResponse(new ProtocolVersion("HTTP", 1, 1), i, ""); + mockHttpStack.setResponseToReturn(fakeResponse); + BasicNetwork httpNetwork = new BasicNetwork(mockHttpStack); + Request<String> request = buildRequest(); + request.setRetryPolicy(mMockRetryPolicy); + doThrow(new VolleyError()).when(mMockRetryPolicy).retry(any(VolleyError.class)); + try { + httpNetwork.performRequest(request); + } catch (VolleyError e) { + // expected + } + // should not retry 300 responses. + verify(mMockRetryPolicy, never()).retry(any(VolleyError.class)); + reset(mMockRetryPolicy); + } + } + + @Test public void otherClientError() throws Exception { + for (int i = 400; i <= 499; i++) { + if (i == 401 || i == 403) { + // covered above. + continue; + } + MockHttpStack mockHttpStack = new MockHttpStack(); + BasicHttpResponse fakeResponse = + new BasicHttpResponse(new ProtocolVersion("HTTP", 1, 1), i, ""); + mockHttpStack.setResponseToReturn(fakeResponse); + BasicNetwork httpNetwork = new BasicNetwork(mockHttpStack); + Request<String> request = buildRequest(); + request.setRetryPolicy(mMockRetryPolicy); + doThrow(new VolleyError()).when(mMockRetryPolicy).retry(any(VolleyError.class)); + try { + httpNetwork.performRequest(request); + } catch (VolleyError e) { + // expected + } + // should not retry other 400 errors. + verify(mMockRetryPolicy, never()).retry(any(VolleyError.class)); + reset(mMockRetryPolicy); + } + } + + @Test public void serverError_enableRetries() throws Exception { + for (int i = 500; i <= 599; i++) { + MockHttpStack mockHttpStack = new MockHttpStack(); + BasicHttpResponse fakeResponse = + new BasicHttpResponse(new ProtocolVersion("HTTP", 1, 1), i, ""); + mockHttpStack.setResponseToReturn(fakeResponse); + BasicNetwork httpNetwork = + new BasicNetwork(mockHttpStack, new ByteArrayPool(4096)); + Request<String> request = buildRequest(); + request.setRetryPolicy(mMockRetryPolicy); + request.setShouldRetryServerErrors(true); + doThrow(new VolleyError()).when(mMockRetryPolicy).retry(any(VolleyError.class)); + try { + httpNetwork.performRequest(request); + } catch (VolleyError e) { + // expected + } + // should retry all 500 errors + verify(mMockRetryPolicy).retry(any(ServerError.class)); + reset(mMockRetryPolicy); + } + } + + @Test public void serverError_disableRetries() throws Exception { + for (int i = 500; i <= 599; i++) { + MockHttpStack mockHttpStack = new MockHttpStack(); + BasicHttpResponse fakeResponse = + new BasicHttpResponse(new ProtocolVersion("HTTP", 1, 1), i, ""); + mockHttpStack.setResponseToReturn(fakeResponse); + BasicNetwork httpNetwork = new BasicNetwork(mockHttpStack); + Request<String> request = buildRequest(); + request.setRetryPolicy(mMockRetryPolicy); + doThrow(new VolleyError()).when(mMockRetryPolicy).retry(any(VolleyError.class)); + try { + httpNetwork.performRequest(request); + } catch (VolleyError e) { + // expected + } + // should not retry any 500 error w/ HTTP 500 retries turned off (the default). + verify(mMockRetryPolicy, never()).retry(any(VolleyError.class)); + reset(mMockRetryPolicy); + } + } + + private static Request<String> buildRequest() { + return new Request<String>(Request.Method.GET, "http://foo", null) { @Override protected Response<String> parseNetworkResponse(NetworkResponse response) { @@ -71,8 +269,5 @@ public class BasicNetworkTest { return result; } }; - httpNetwork.performRequest(request); - assertEquals("foo", mockHttpStack.getLastHeaders().get("requestheader")); - assertEquals("requestpost=foo&", new String(mockHttpStack.getLastPostBody())); } } |