Merge pull request #183 from Asky-GH/dev

- fixes retry/redirect options customization

Author: baywet

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();
     }

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;
     }

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();
     }

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 <Result, Body> 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

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();
 
     /**

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;
     }

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);
+    }
 }

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);
+    }
 }

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));
+    }
+}

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);
+    }
+}