diff --git a/src/main/java/com/microsoft/graph/http/BaseCollectionRequest.java b/src/main/java/com/microsoft/graph/http/BaseCollectionRequest.java index 694703cb2..f973f83df 100644 --- a/src/main/java/com/microsoft/graph/http/BaseCollectionRequest.java +++ b/src/main/java/com/microsoft/graph/http/BaseCollectionRequest.java @@ -357,7 +357,7 @@ public void setShouldRedirect(@Nonnull IShouldRedirect shouldRedirect) { * * @return Callback which is called before redirect */ - @Nullable + @Nonnull public IShouldRedirect getShouldRedirect() { return baseRequest.getShouldRedirect(); } @@ -377,7 +377,7 @@ public void setShouldRetry(@Nonnull IShouldRetry shouldretry) { * * @return Callback called before retry */ - @Nullable + @Nonnull public IShouldRetry getShouldRetry() { return baseRequest.getShouldRetry(); } diff --git a/src/main/java/com/microsoft/graph/http/BaseRequest.java b/src/main/java/com/microsoft/graph/http/BaseRequest.java index 436d7173d..8bc323005 100644 --- a/src/main/java/com/microsoft/graph/http/BaseRequest.java +++ b/src/main/java/com/microsoft/graph/http/BaseRequest.java @@ -537,7 +537,7 @@ public void setShouldRedirect(@Nonnull IShouldRedirect shouldRedirect) { * * @return Callback which is called before redirect */ - @Nullable + @Nonnull public IShouldRedirect getShouldRedirect() { return shouldRedirect; } @@ -557,7 +557,7 @@ public void setShouldRetry(@Nonnull IShouldRetry shouldretry) { * * @return Callback called before retry */ - @Nullable + @Nonnull public IShouldRetry getShouldRetry() { return shouldRetry; } diff --git a/src/main/java/com/microsoft/graph/http/BaseStreamRequest.java b/src/main/java/com/microsoft/graph/http/BaseStreamRequest.java index ae5f69984..0be8db3be 100644 --- a/src/main/java/com/microsoft/graph/http/BaseStreamRequest.java +++ b/src/main/java/com/microsoft/graph/http/BaseStreamRequest.java @@ -224,7 +224,7 @@ public void setShouldRedirect(@Nonnull final IShouldRedirect shouldRedirect) { * * @return Callback which is called before redirect */ - @Nullable + @Nonnull public IShouldRedirect getShouldRedirect() { return baseRequest.getShouldRedirect(); } @@ -244,7 +244,7 @@ public void setShouldRetry(@Nonnull final IShouldRetry shouldretry) { * * @return Callback called before retry */ - @Nullable + @Nonnull public IShouldRetry getShouldRetry() { return baseRequest.getShouldRetry(); } diff --git a/src/main/java/com/microsoft/graph/http/CoreHttpProvider.java b/src/main/java/com/microsoft/graph/http/CoreHttpProvider.java index 3fb14b462..8be1022b7 100644 --- a/src/main/java/com/microsoft/graph/http/CoreHttpProvider.java +++ b/src/main/java/com/microsoft/graph/http/CoreHttpProvider.java @@ -57,6 +57,12 @@ import okhttp3.ResponseBody; import okio.BufferedSink; +import static com.microsoft.graph.httpcore.middlewareoption.RedirectOptions.DEFAULT_MAX_REDIRECTS; +import static com.microsoft.graph.httpcore.middlewareoption.RedirectOptions.DEFAULT_SHOULD_REDIRECT; +import static com.microsoft.graph.httpcore.middlewareoption.RetryOptions.DEFAULT_DELAY; +import static com.microsoft.graph.httpcore.middlewareoption.RetryOptions.DEFAULT_MAX_RETRIES; +import static com.microsoft.graph.httpcore.middlewareoption.RetryOptions.DEFAULT_SHOULD_RETRY; + /** * HTTP provider based off of OkHttp and msgraph-sdk-java-core library */ @@ -236,8 +242,14 @@ public Request getHttpRequest(@Nonnull final IHttpRequest request logger.logDebug("Starting to send request, URL " + requestUrl.toString()); // Request level middleware options - final RedirectOptions redirectOptions = request.getMaxRedirects() <= 0 ? null : new RedirectOptions(request.getMaxRedirects(), request.getShouldRedirect()); - final RetryOptions retryOptions = request.getShouldRetry() == null ? null : new RetryOptions(request.getShouldRetry(), request.getMaxRetries(), request.getDelay()); + final RedirectOptions redirectOptions = + request.getMaxRedirects() == DEFAULT_MAX_REDIRECTS && request.getShouldRedirect().equals(DEFAULT_SHOULD_REDIRECT) + ? null + : new RedirectOptions(request.getMaxRedirects(), request.getShouldRedirect()); + final RetryOptions retryOptions = + request.getMaxRetries() == DEFAULT_MAX_RETRIES && request.getDelay() == DEFAULT_DELAY && request.getShouldRetry().equals(DEFAULT_SHOULD_RETRY) + ? null + : new RetryOptions(request.getShouldRetry(), request.getMaxRetries(), request.getDelay()); final Request coreHttpRequest = convertIHttpRequestToOkHttpRequest(request); Request.Builder corehttpRequestBuilder = coreHttpRequest diff --git a/src/main/java/com/microsoft/graph/http/IHttpRequest.java b/src/main/java/com/microsoft/graph/http/IHttpRequest.java index c032d4096..62969d9bd 100644 --- a/src/main/java/com/microsoft/graph/http/IHttpRequest.java +++ b/src/main/java/com/microsoft/graph/http/IHttpRequest.java @@ -119,7 +119,7 @@ public interface IHttpRequest { * * @return Callback which is called before redirect */ - @Nullable + @Nonnull IShouldRedirect getShouldRedirect(); /** @@ -134,7 +134,7 @@ public interface IHttpRequest { * * @return Callback called before retry */ - @Nullable + @Nonnull IShouldRetry getShouldRetry(); /** diff --git a/src/main/java/com/microsoft/graph/httpcore/middlewareoption/RetryOptions.java b/src/main/java/com/microsoft/graph/httpcore/middlewareoption/RetryOptions.java index 8fa390a48..ba9418ad8 100644 --- a/src/main/java/com/microsoft/graph/httpcore/middlewareoption/RetryOptions.java +++ b/src/main/java/com/microsoft/graph/httpcore/middlewareoption/RetryOptions.java @@ -68,7 +68,7 @@ public RetryOptions(@Nullable final IShouldRetry shouldRetry, int maxRetries, lo if(maxRetries < 0) throw new IllegalArgumentException("Max retries cannot be negative"); - this.mShouldretry = shouldRetry != null ? shouldRetry : DEFAULT_SHOULD_RETRY; + this.mShouldretry = shouldRetry == null ? DEFAULT_SHOULD_RETRY : shouldRetry; this.mMaxRetries = maxRetries; this.mDelay = delay; } diff --git a/src/test/java/com/microsoft/graph/http/CoreHttpProviderTests.java b/src/test/java/com/microsoft/graph/http/CoreHttpProviderTests.java index 2a3c931e0..001d9a080 100644 --- a/src/test/java/com/microsoft/graph/http/CoreHttpProviderTests.java +++ b/src/test/java/com/microsoft/graph/http/CoreHttpProviderTests.java @@ -9,6 +9,10 @@ import com.microsoft.graph.core.GraphErrorCodes; import com.microsoft.graph.core.IBaseClient; +import com.microsoft.graph.httpcore.middlewareoption.IShouldRedirect; +import com.microsoft.graph.httpcore.middlewareoption.IShouldRetry; +import com.microsoft.graph.httpcore.middlewareoption.RedirectOptions; +import com.microsoft.graph.httpcore.middlewareoption.RetryOptions; import com.microsoft.graph.logger.ILogger; import com.microsoft.graph.logger.LoggerLevel; import com.microsoft.graph.options.HeaderOption; @@ -21,6 +25,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.MalformedURLException; import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -37,6 +42,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -198,5 +204,88 @@ private void setCoreHttpProvider(final Object toSerialize) throws IOException { mock(ILogger.class), mClient); } + @Test + public void getHttpRequestDoesntSetRetryOrRedirectOptionsOnDefaultValues() throws MalformedURLException { + final IHttpRequest absRequest = mock(IHttpRequest.class); + when(absRequest.getRequestUrl()).thenReturn(new URL("https://graph.microsoft.com/v1.0/me")); + when(absRequest.getHttpMethod()).thenReturn(HttpMethod.GET); + final ISerializer serializer = mock(ISerializer.class); + final ILogger logger = mock(ILogger.class); + + mProvider = new CoreHttpProvider(serializer, + logger, + new OkHttpClient.Builder().build()); + + when(absRequest.getMaxRedirects()).thenReturn(RedirectOptions.DEFAULT_MAX_REDIRECTS); + when(absRequest.getShouldRedirect()).thenReturn(RedirectOptions.DEFAULT_SHOULD_REDIRECT); + Request request = mProvider.getHttpRequest(absRequest, String.class, null); + RedirectOptions redirectOptions = request.tag(RedirectOptions.class); + + assertNull(redirectOptions); + + when(absRequest.getShouldRetry()).thenReturn(RetryOptions.DEFAULT_SHOULD_RETRY); + when(absRequest.getMaxRetries()).thenReturn(RetryOptions.DEFAULT_MAX_RETRIES); + when(absRequest.getDelay()).thenReturn(RetryOptions.DEFAULT_DELAY); + + request = mProvider.getHttpRequest(absRequest, String.class, null); + RetryOptions retryOptions = request.tag(RetryOptions.class); + + assertNull(retryOptions); + } + + @Test + public void getHttpRequestSetsRetryOrRedirectOptionsOnNonDefaultValues() throws MalformedURLException { + final IHttpRequest absRequest = mock(IHttpRequest.class); + when(absRequest.getRequestUrl()).thenReturn(new URL("https://graph.microsoft.com/v1.0/me")); + when(absRequest.getHttpMethod()).thenReturn(HttpMethod.GET); + final ISerializer serializer = mock(ISerializer.class); + final ILogger logger = mock(ILogger.class); + mProvider = new CoreHttpProvider(serializer, + logger, + new OkHttpClient.Builder().build()); + + // testing all pairs to cover all branches + when(absRequest.getMaxRedirects()).thenReturn(RedirectOptions.DEFAULT_MAX_REDIRECTS -1); + when(absRequest.getShouldRedirect()).thenReturn(mock(IShouldRedirect.class)); + Request request = mProvider.getHttpRequest(absRequest, String.class, null); + RedirectOptions redirectOptions = request.tag(RedirectOptions.class); + + assertNotNull(redirectOptions); + + when(absRequest.getMaxRedirects()).thenReturn(RedirectOptions.DEFAULT_MAX_REDIRECTS); + when(absRequest.getShouldRedirect()).thenReturn(mock(IShouldRedirect.class)); + request = mProvider.getHttpRequest(absRequest, String.class, null); + redirectOptions = request.tag(RedirectOptions.class); + + assertNotNull(redirectOptions); + + // testing all pairs to cover all branches + when(absRequest.getShouldRetry()).thenReturn(mock(IShouldRetry.class)); + when(absRequest.getMaxRetries()).thenReturn(RetryOptions.DEFAULT_MAX_RETRIES-1); + when(absRequest.getDelay()).thenReturn(RetryOptions.DEFAULT_DELAY-1); + + request = mProvider.getHttpRequest(absRequest, String.class, null); + RetryOptions retryOptions = request.tag(RetryOptions.class); + + assertNotNull(retryOptions); + + when(absRequest.getShouldRetry()).thenReturn(mock(IShouldRetry.class)); + when(absRequest.getMaxRetries()).thenReturn(RetryOptions.DEFAULT_MAX_RETRIES); + when(absRequest.getDelay()).thenReturn(RetryOptions.DEFAULT_DELAY-1); + + request = mProvider.getHttpRequest(absRequest, String.class, null); + retryOptions = request.tag(RetryOptions.class); + + assertNotNull(retryOptions); + + when(absRequest.getShouldRetry()).thenReturn(mock(IShouldRetry.class)); + when(absRequest.getMaxRetries()).thenReturn(RetryOptions.DEFAULT_MAX_RETRIES); + when(absRequest.getDelay()).thenReturn(RetryOptions.DEFAULT_DELAY); + + request = mProvider.getHttpRequest(absRequest, String.class, null); + retryOptions = request.tag(RetryOptions.class); + + assertNotNull(retryOptions); + } } diff --git a/src/test/java/com/microsoft/graph/httpcore/RedirectHandlerTest.java b/src/test/java/com/microsoft/graph/httpcore/RedirectHandlerTest.java index 326d28d62..7acfddaae 100644 --- a/src/test/java/com/microsoft/graph/httpcore/RedirectHandlerTest.java +++ b/src/test/java/com/microsoft/graph/httpcore/RedirectHandlerTest.java @@ -1,5 +1,6 @@ package com.microsoft.graph.httpcore; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -262,5 +263,20 @@ public void testGetRedirectRelativeLocationRequestURLwithSlash() throws Protocol String expected = "https://graph.microsoft.com/v1.0/me/testrelativeurl"; assertTrue(request.url().toString().compareTo(expected) == 0); } + @Test + public void testIsRedirectedIsFalseIfExceedsMaxRedirects() throws ProtocolException, IOException { + RedirectOptions options = new RedirectOptions(0, null); + RedirectHandler redirectHandler = new RedirectHandler(options); + Request httppost = new Request.Builder().url(testmeurl).build(); + Response response = new Response.Builder() + .protocol(Protocol.HTTP_1_1) + .code(HttpURLConnection.HTTP_SEE_OTHER) + .message("See Other") + .addHeader("location", "/testrelativeurl") + .request(httppost) + .build(); + boolean isRedirected = redirectHandler.isRedirected(httppost, response, 1, options); + assertFalse(isRedirected); + } } diff --git a/src/test/java/com/microsoft/graph/httpcore/middlewareoption/RedirectOptionsTest.java b/src/test/java/com/microsoft/graph/httpcore/middlewareoption/RedirectOptionsTest.java new file mode 100644 index 000000000..81be8c023 --- /dev/null +++ b/src/test/java/com/microsoft/graph/httpcore/middlewareoption/RedirectOptionsTest.java @@ -0,0 +1,33 @@ +package com.microsoft.graph.httpcore.middlewareoption; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import org.junit.jupiter.api.Test; + +import okhttp3.Response; + +public class RedirectOptionsTest { + @Test + public void constructorDefensiveProgramming() { + assertThrows(IllegalArgumentException.class, () -> { + new RedirectOptions(RedirectOptions.MAX_REDIRECTS +1, null); + }); + + assertThrows(IllegalArgumentException.class, () -> { + new RedirectOptions(-1, null); + }); + } + @Test + public void defaultShouldRedirectValue() { + RedirectOptions options = new RedirectOptions(); + assertEquals(options.shouldRedirect(), RedirectOptions.DEFAULT_SHOULD_REDIRECT); + } + @Test + public void defaultShouldRedirectIsTrue() { + Response response = mock(Response.class); + assertTrue(RedirectOptions.DEFAULT_SHOULD_REDIRECT.shouldRedirect(response)); + } +} diff --git a/src/test/java/com/microsoft/graph/httpcore/middlewareoption/RetryOptionsTest.java b/src/test/java/com/microsoft/graph/httpcore/middlewareoption/RetryOptionsTest.java new file mode 100644 index 000000000..91502144d --- /dev/null +++ b/src/test/java/com/microsoft/graph/httpcore/middlewareoption/RetryOptionsTest.java @@ -0,0 +1,32 @@ +package com.microsoft.graph.httpcore.middlewareoption; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +public class RetryOptionsTest { + @Test + public void constructorDefensiveProgramming() { + assertThrows(IllegalArgumentException.class, () -> { + new RetryOptions(null, RetryOptions.MAX_RETRIES +1, 0); + }); + + assertThrows(IllegalArgumentException.class, () -> { + new RetryOptions(null, RetryOptions.MAX_RETRIES, RetryOptions.MAX_DELAY +1); + }); + + assertThrows(IllegalArgumentException.class, () -> { + new RetryOptions(null, RetryOptions.MAX_RETRIES, -1); + }); + + assertThrows(IllegalArgumentException.class, () -> { + new RetryOptions(null, -1, RetryOptions.MAX_DELAY); + }); + } + @Test + public void defaultShouldRetryValue() { + RetryOptions options = new RetryOptions(); + assertEquals(options.shouldRetry(), RetryOptions.DEFAULT_SHOULD_RETRY); + } +}