Skip to content

Commit

Permalink
Merge pull request #139 from microsoftgraph/bugfix/chunk-handler-v1
Browse files Browse the repository at this point in the history
bugfix/chunk handler v1
  • Loading branch information
baywet authored Feb 9, 2021
2 parents 511f3a6 + da0468c commit 973bc01
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public static Builder custom() {
return new OkHttpClient.Builder()
.addInterceptor(new TelemetryHandler())
.followRedirects(false)
.followSslRedirects(false)
.protocols(Arrays.asList(Protocol.HTTP_1_1)); //https://stackoverflow.com/questions/62031298/sockettimeout-on-java-11-but-not-on-java-8
}

Expand Down
55 changes: 28 additions & 27 deletions src/main/java/com/microsoft/graph/httpcore/RedirectHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@
import okhttp3.Response;

public class RedirectHandler implements Interceptor{

public final MiddlewareType MIDDLEWARE_TYPE = MiddlewareType.REDIRECT;

private RedirectOptions mRedirectOptions;

/*
* Initialize using default redirect options, default IShouldRedirect and max redirect value
*/
public RedirectHandler() {
this(null);
}

/*
* @param redirectOptions pass instance of redirect options to be used
*/
Expand All @@ -40,16 +40,16 @@ public RedirectHandler(RedirectOptions redirectOptions) {
this.mRedirectOptions = new RedirectOptions();
}
}

boolean isRedirected(Request request, Response response, int redirectCount, RedirectOptions redirectOptions) throws IOException {
// Check max count of redirects reached
if(redirectCount > redirectOptions.maxRedirects()) return false;

// Location header empty then don't redirect
final String locationHeader = response.header("location");
if(locationHeader == null)
return false;

// If any of 301,302,303,307,308 then redirect
final int statusCode = response.code();
if(statusCode == HTTP_PERM_REDIRECT || //308
Expand All @@ -58,31 +58,31 @@ boolean isRedirected(Request request, Response response, int redirectCount, Redi
statusCode == HTTP_SEE_OTHER || //303
statusCode == HTTP_MOVED_TEMP) //302
return true;

return false;
}

Request getRedirect(
final Request request,
final Response userResponse) throws ProtocolException {
final Response userResponse) throws ProtocolException {
String location = userResponse.header("Location");
if (location == null || location.length() == 0) return null;

// For relative URL in location header, the new url to redirect is relative to original request
if(location.startsWith("/")) {
if(request.url().toString().endsWith("/")) {
location = location.substring(1);
}
location = request.url() + location;
}

HttpUrl requestUrl = userResponse.request().url();

HttpUrl locationUrl = userResponse.request().url().resolve(location);

// Don't follow redirects to unsupported protocols.
if (locationUrl == null) return null;

// Most redirects don't include a request body.
Request.Builder requestBuilder = userResponse.request().newBuilder();

Expand All @@ -94,7 +94,7 @@ Request getRedirect(
if (!sameScheme || !sameHost) {
requestBuilder.removeHeader("Authorization");
}

// Response status code 303 See Other then POST changes to GET
if(userResponse.code() == HTTP_SEE_OTHER) {
requestBuilder.method("GET", null);
Expand All @@ -107,29 +107,30 @@ Request getRedirect(
@Override
public Response intercept(Chain chain) throws IOException {
Request request = chain.request();

if(request.tag(TelemetryOptions.class) == null)
request = request.newBuilder().tag(TelemetryOptions.class, new TelemetryOptions()).build();
request.tag(TelemetryOptions.class).setFeatureUsage(TelemetryOptions.REDIRECT_HANDLER_ENABLED_FLAG);

Response response = null;
int requestsCount = 1;

// Use should retry pass along with this request
RedirectOptions redirectOptions = request.tag(RedirectOptions.class);
redirectOptions = redirectOptions != null ? redirectOptions : this.mRedirectOptions;

while(true) {
response = chain.proceed(request);
boolean shouldRedirect = isRedirected(request, response, requestsCount, redirectOptions)
final boolean shouldRedirect = isRedirected(request, response, requestsCount, redirectOptions)
&& redirectOptions.shouldRedirect().shouldRedirect(response);
if(!shouldRedirect) break;

Request followup = getRedirect(request, response);
if(followup == null) break;
request = followup;

requestsCount++;

final Request followup = getRedirect(request, response);
if(followup != null) {
response.close();
request = followup;
requestsCount++;
}
}
return response;
}
Expand Down
62 changes: 32 additions & 30 deletions src/main/java/com/microsoft/graph/httpcore/RetryHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
import okhttp3.Response;

public class RetryHandler implements Interceptor{

public final MiddlewareType MIDDLEWARE_TYPE = MiddlewareType.RETRY;

private RetryOptions mRetryOption;

/*
* constant string being used
*/
Expand All @@ -26,13 +26,13 @@ public class RetryHandler implements Interceptor{
private final String TRANSFER_ENCODING_CHUNKED = "chunked";
private final String APPLICATION_OCTET_STREAM = "application/octet-stream";
private final String CONTENT_TYPE = "Content-Type";

public static final int MSClientErrorCodeTooManyRequests = 429;
public static final int MSClientErrorCodeServiceUnavailable = 503;
public static final int MSClientErrorCodeGatewayTimeout = 504;

private final long DELAY_MILLISECONDS = 1000;

/*
* @retryOption Create Retry handler using retry option
*/
Expand All @@ -48,28 +48,28 @@ public RetryHandler(RetryOptions retryOption) {
public RetryHandler() {
this(null);
}

boolean retryRequest(Response response, int executionCount, Request request, RetryOptions retryOptions) {

// Should retry option
// Use should retry common for all requests
IShouldRetry shouldRetryCallback = null;
if(retryOptions != null) {
shouldRetryCallback = retryOptions.shouldRetry();
}

boolean shouldRetry = false;
// Status codes 429 503 504
int statusCode = response.code();
// Only requests with payloads that are buffered/rewindable are supported.
// Payloads with forward only streams will be have the responses returned
// Only requests with payloads that are buffered/rewindable are supported.
// Payloads with forward only streams will be have the responses returned
// without any retry attempt.
shouldRetry =
(executionCount <= retryOptions.maxRetries())
shouldRetry =
(executionCount <= retryOptions.maxRetries())
&& checkStatus(statusCode) && isBuffered(response, request)
&& shouldRetryCallback != null
&& shouldRetryCallback != null
&& shouldRetryCallback.shouldRetry(retryOptions.delay(), executionCount, request, response);

if(shouldRetry) {
long retryInterval = getRetryAfter(response, retryOptions.delay(), executionCount);
try {
Expand All @@ -80,7 +80,7 @@ && checkStatus(statusCode) && isBuffered(response, request)
}
return shouldRetry;
}

/**
* Get retry after in milliseconds
* @param response Response
Expand All @@ -100,28 +100,28 @@ long getRetryAfter(Response response, long delay, int executionCount) {
}
return (long)Math.min(retryDelay, RetryOptions.MAX_DELAY * DELAY_MILLISECONDS);
}

boolean checkStatus(int statusCode) {
return (statusCode == MSClientErrorCodeTooManyRequests || statusCode == MSClientErrorCodeServiceUnavailable
return (statusCode == MSClientErrorCodeTooManyRequests || statusCode == MSClientErrorCodeServiceUnavailable
|| 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"))
if(methodName.equalsIgnoreCase("GET") || methodName.equalsIgnoreCase("DELETE") || methodName.equalsIgnoreCase("HEAD") || methodName.equalsIgnoreCase("OPTIONS"))
return true;

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) &&
boolean isTransferEncodingChunked = (transferEncoding != null) &&
transferEncoding.equalsIgnoreCase(TRANSFER_ENCODING_CHUNKED);

if(request.body() != null && isTransferEncodingChunked)
return true;
}
Expand All @@ -132,24 +132,26 @@ boolean isBuffered(Response response, Request request) {
@Override
public Response intercept(Chain chain) throws IOException {
Request request = chain.request();

if(request.tag(TelemetryOptions.class) == null)
request = request.newBuilder().tag(TelemetryOptions.class, new TelemetryOptions()).build();
request.tag(TelemetryOptions.class).setFeatureUsage(TelemetryOptions.RETRY_HANDLER_ENABLED_FLAG);

Response response = chain.proceed(request);

// Use should retry pass along with this request
RetryOptions retryOption = request.tag(RetryOptions.class);
retryOption = retryOption != null ? retryOption : mRetryOption;

int executionCount = 1;
while(retryRequest(response, executionCount, request, retryOption)) {
request = request.newBuilder().addHeader(RETRY_ATTEMPT_HEADER, String.valueOf(executionCount)).build();
executionCount++;
if(response != null && response.body() != null) {
response.body().close();
}
if(response != null) {
if(response.body() != null)
response.body().close();
response.close();
}
response = chain.proceed(request);
}
return response;
Expand Down

0 comments on commit 973bc01

Please sign in to comment.