diff --git a/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java b/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java index 6ba5900c7..314dffec0 100644 --- a/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java +++ b/src/main/java/com/microsoft/graph/httpcore/RetryHandler.java @@ -22,10 +22,8 @@ public class RetryHandler implements Interceptor{ */ private final String RETRY_ATTEMPT_HEADER = "Retry-Attempt"; private final String RETRY_AFTER = "Retry-After"; - private final String TRANSFER_ENCODING = "Transfer-Encoding"; - private final String TRANSFER_ENCODING_CHUNKED = "chunked"; - private final String APPLICATION_OCTET_STREAM = "application/octet-stream"; - private final String CONTENT_TYPE = "Content-Type"; + /** Content length request header value */ + private final String CONTENT_LENGTH = "Content-Length"; public static final int MSClientErrorCodeTooManyRequests = 429; public static final int MSClientErrorCodeServiceUnavailable = 503; @@ -66,7 +64,7 @@ boolean retryRequest(Response response, int executionCount, Request request, Ret // without any retry attempt. shouldRetry = (executionCount <= retryOptions.maxRetries()) - && checkStatus(statusCode) && isBuffered(response, request) + && checkStatus(statusCode) && isBuffered(request) && shouldRetryCallback != null && shouldRetryCallback.shouldRetry(retryOptions.delay(), executionCount, request, response); @@ -106,27 +104,22 @@ boolean checkStatus(int statusCode) { || statusCode == MSClientErrorCodeGatewayTimeout); } - boolean isBuffered(Response response, Request request) { - String methodName = request.method(); - if(methodName.equalsIgnoreCase("GET") || methodName.equalsIgnoreCase("DELETE") || methodName.equalsIgnoreCase("HEAD") || methodName.equalsIgnoreCase("OPTIONS")) - return true; + boolean isBuffered(Request request) { + final String methodName = request.method(); - boolean isHTTPMethodPutPatchOrPost = methodName.equalsIgnoreCase("POST") || + final boolean isHTTPMethodPutPatchOrPost = methodName.equalsIgnoreCase("POST") || methodName.equalsIgnoreCase("PUT") || methodName.equalsIgnoreCase("PATCH"); - if(isHTTPMethodPutPatchOrPost) { - boolean isStream = response.header(CONTENT_TYPE)!=null && response.header(CONTENT_TYPE).equalsIgnoreCase(APPLICATION_OCTET_STREAM); - if(!isStream) { - String transferEncoding = response.header(TRANSFER_ENCODING); - boolean isTransferEncodingChunked = (transferEncoding != null) && - transferEncoding.equalsIgnoreCase(TRANSFER_ENCODING_CHUNKED); - - if(request.body() != null && isTransferEncodingChunked) - return true; - } + if(isHTTPMethodPutPatchOrPost && request.body() != null) { + try { + return request.body().contentLength() != -1L; + } catch (IOException ex) { + // expected + return false; + } } - return false; + return true; } @Override diff --git a/src/test/java/com/microsoft/graph/httpcore/RetryHandlerTest.java b/src/test/java/com/microsoft/graph/httpcore/RetryHandlerTest.java index d5ddc3915..4f25ec097 100644 --- a/src/test/java/com/microsoft/graph/httpcore/RetryHandlerTest.java +++ b/src/test/java/com/microsoft/graph/httpcore/RetryHandlerTest.java @@ -4,6 +4,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import java.io.IOException; import java.net.HttpURLConnection; import org.junit.Test; @@ -16,6 +17,7 @@ import okhttp3.Request; import okhttp3.RequestBody; import okhttp3.Response; +import okio.BufferedSink; public class RetryHandlerTest { @@ -76,7 +78,7 @@ public void testRetryRequestWithMaxRetryAttempts() { // Default retry options with Number of maxretries default to 3 RetryOptions retryOptions = new RetryOptions(); // Try to execute one more than allowed default max retries - int executionCount = RetryOptions.DEFAULT_MAX_RETRIES + 1; + int executionCount = RetryOptions.DEFAULT_MAX_RETRIES + 1; assertFalse(retryhandler.retryRequest(response, executionCount, httpget, retryOptions)); } @@ -87,7 +89,7 @@ public void testRetryRequestForStatusCode() { Response response = new Response.Builder() .protocol(Protocol.HTTP_1_1) // For status code 500 which is not in (429 503 504), So NO retry - .code(HTTP_SERVER_ERROR) + .code(HTTP_SERVER_ERROR) .message( "Internal Server Error") .request(httpget) .build(); @@ -100,7 +102,7 @@ public void testRetryRequestWithTransferEncoding() { Request httppost = new Request.Builder().url(testmeurl).post(RequestBody.create(MediaType.parse("application/json"), "TEST")).build(); Response response = new Response.Builder() .protocol(Protocol.HTTP_1_1) - .code(HttpURLConnection.HTTP_GATEWAY_TIMEOUT) + .code(HttpURLConnection.HTTP_GATEWAY_TIMEOUT) .message( "gateway timeout") .request(httppost) .addHeader("Transfer-Encoding", "chunked") @@ -114,15 +116,15 @@ public void testRetryRequestWithExponentialBackOff() { Request httppost = new Request.Builder().url(testmeurl).post(RequestBody.create(MediaType.parse("application/json"), "TEST")).build(); Response response = new Response.Builder() .protocol(Protocol.HTTP_1_1) - .code(HttpURLConnection.HTTP_GATEWAY_TIMEOUT) + .code(HttpURLConnection.HTTP_GATEWAY_TIMEOUT) .message( "gateway timeout") .request(httppost) .addHeader("Transfer-Encoding", "chunked") .build(); - + assertTrue(retryhandler.retryRequest(response, 1, httppost, new RetryOptions())); } - + @Test public void testGetRetryAfterWithHeader() { RetryHandler retryHandler = new RetryHandler(); @@ -131,7 +133,7 @@ public void testGetRetryAfterWithHeader() { delay = retryHandler.getRetryAfter(TestResponse().newBuilder().addHeader("Retry-After", "1").build(), 2, 3); assertTrue(delay == 1000); } - + @Test public void testGetRetryAfterOnFirstExecution() { RetryHandler retryHandler = new RetryHandler(); @@ -140,14 +142,48 @@ public void testGetRetryAfterOnFirstExecution() { delay = retryHandler.getRetryAfter(TestResponse(), 3, 2); assertTrue(delay > 3100); } - + @Test public void testGetRetryAfterMaxExceed() { RetryHandler retryHandler = new RetryHandler(); long delay = retryHandler.getRetryAfter(TestResponse(), 190, 1); assertTrue(delay == 180000); } - + @Test + public void testIsBuffered() { + final RetryHandler retryHandler = new RetryHandler(); + Request request = new Request.Builder().url("https://localhost").method("GET", null).build(); + assertTrue("Get Request is buffered", retryHandler.isBuffered(request)); + + request = new Request.Builder().url("https://localhost").method("DELETE", null).build(); + assertTrue("Delete Request is buffered", retryHandler.isBuffered(request)); + + request = new Request.Builder().url("https://localhost") + .method("POST", + RequestBody.create(MediaType.parse("application/json"), + "{\"key\": 42 }")) + .build(); + assertTrue("Post Request is buffered", retryHandler.isBuffered(request)); + + request = new Request.Builder().url("https://localhost") + .method("POST", + new RequestBody() { + + @Override + public MediaType contentType() { + return MediaType.parse("application/octet-stream"); + } + + @Override + public void writeTo(BufferedSink sink) throws IOException { + // TODO Auto-generated method stub + + } + }) + .build(); + assertFalse("Post Stream Request is not buffered", retryHandler.isBuffered(request)); + } + Response TestResponse() { return new Response.Builder() .code(429)