SONAR-11113 Increase timeout for downloading plugins

5 files changed, 75 insertions, 8 deletions

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/PluginFiles.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/PluginFiles.java
index 697908b40db..92dcb395f94 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/PluginFiles.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/PluginFiles.java
@@ -92,7 +92,8 @@ public class PluginFiles {
   private Optional<File> download(InstalledPlugin plugin) {
     GetRequest request = new GetRequest("api/plugins/download")
       .setParam("plugin", plugin.key)
-      .setParam("acceptCompressions", PACK200);
+      .setParam("acceptCompressions", PACK200)
+      .setTimeOutInMs(5 * 60_000);
 
     File downloadedFile = newTempFile();
     LOGGER.debug("Download plugin '{}' to '{}'", plugin.key, downloadedFile);
diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/BaseRequest.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/BaseRequest.java
index c0a308c3578..960aff5dbb7 100644
--- a/sonar-ws/src/main/java/org/sonarqube/ws/client/BaseRequest.java
+++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/BaseRequest.java
@@ -28,6 +28,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.OptionalInt;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -49,6 +50,7 @@ abstract class BaseRequest<SELF extends BaseRequest> implements WsRequest {
 
   private final DefaultParameters parameters = new DefaultParameters();
   private final DefaultHeaders headers = new DefaultHeaders();
+  private OptionalInt timeOutInMs = OptionalInt.empty();
 
   BaseRequest(String path) {
     this.path = path;
@@ -64,6 +66,16 @@ abstract class BaseRequest<SELF extends BaseRequest> implements WsRequest {
     return mediaType;
   }
 
+  @Override
+  public OptionalInt getTimeOutInMs() {
+    return timeOutInMs;
+  }
+
+  public SELF setTimeOutInMs(int timeOutInMs) {
+    this.timeOutInMs = OptionalInt.of(timeOutInMs);
+    return (SELF) this;
+  }
+
   /**
    * Expected media type of response. Default is {@link MediaTypes#JSON}.
    */
diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java
index 5b12bd95409..031f44c60c2 100644
--- a/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java
+++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java
@@ -22,6 +22,7 @@ package org.sonarqube.ws.client;
 import java.io.IOException;
 import java.net.Proxy;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 import javax.annotation.Nullable;
 import javax.net.ssl.SSLSocketFactory;
 import javax.net.ssl.X509TrustManager;
@@ -121,7 +122,7 @@ public class HttpConnector implements WsConnector {
     completeUrlQueryParameters(getRequest, urlBuilder);
 
     Request.Builder okRequestBuilder = prepareOkRequestBuilder(getRequest, urlBuilder).get();
-    return new OkHttpResponse(doCall(okHttpClient, okRequestBuilder.build()));
+    return new OkHttpResponse(doCall(prepareOkHttpClient(okHttpClient, getRequest), okRequestBuilder.build()));
   }
 
   private WsResponse post(PostRequest postRequest) {
@@ -152,8 +153,8 @@ public class HttpConnector implements WsConnector {
       body = bodyBuilder.build();
     }
     Request.Builder okRequestBuilder = prepareOkRequestBuilder(postRequest, urlBuilder).post(body);
-    Response response = doCall(noRedirectOkHttpClient, okRequestBuilder.build());
-    response = checkRedirect(response);
+    Response response = doCall(prepareOkHttpClient(noRedirectOkHttpClient, postRequest), okRequestBuilder.build());
+    response = checkRedirect(response, postRequest);
     return new OkHttpResponse(response);
   }
 
@@ -164,6 +165,16 @@ public class HttpConnector implements WsConnector {
       .newBuilder();
   }
 
+  private static OkHttpClient prepareOkHttpClient(OkHttpClient okHttpClient, WsRequest wsRequest) {
+    if (!wsRequest.getTimeOutInMs().isPresent()) {
+      return okHttpClient;
+    }
+
+    return okHttpClient.newBuilder()
+      .readTimeout(wsRequest.getTimeOutInMs().getAsInt(), TimeUnit.MILLISECONDS)
+      .build();
+  }
+
   private static void completeUrlQueryParameters(BaseRequest<?> request, HttpUrl.Builder urlBuilder) {
     request.getParameters().getKeys()
       .forEach(key -> request.getParameters().getValues(key)
@@ -191,7 +202,7 @@ public class HttpConnector implements WsConnector {
     }
   }
 
-  private Response checkRedirect(Response response) {
+  private Response checkRedirect(Response response, PostRequest postRequest) {
     switch (response.code()) {
       case HTTP_MOVED_PERM:
       case HTTP_MOVED_TEMP:
@@ -202,13 +213,13 @@ public class HttpConnector implements WsConnector {
         // See:
         // https://github.com/square/okhttp/blob/07309c1c7d9e296014268ebd155ebf7ef8679f6c/okhttp/src/main/java/okhttp3/internal/http/RetryAndFollowUpInterceptor.java#L316
         // https://github.com/square/okhttp/issues/936#issuecomment-266430151
-        return followPostRedirect(response);
+        return followPostRedirect(response, postRequest);
       default:
         return response;
     }
   }
 
-  private Response followPostRedirect(Response response) {
+  private Response followPostRedirect(Response response, PostRequest postRequest) {
     String location = response.header("Location");
     if (location == null) {
       throw new IllegalStateException(format("Missing HTTP header 'Location' in redirect of %s", response.request().url()));
@@ -223,7 +234,7 @@ public class HttpConnector implements WsConnector {
     Request.Builder redirectRequest = response.request().newBuilder();
     redirectRequest.post(response.request().body());
     response.body().close();
-    return doCall(noRedirectOkHttpClient, redirectRequest.url(url).build());
+    return doCall(prepareOkHttpClient(noRedirectOkHttpClient, postRequest), redirectRequest.url(url).build());
   }
 
   /**
diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/WsRequest.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/WsRequest.java
index 62e16c3fccb..ebe6cfb221b 100644
--- a/sonar-ws/src/main/java/org/sonarqube/ws/client/WsRequest.java
+++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/WsRequest.java
@@ -20,6 +20,7 @@
 package org.sonarqube.ws.client;
 
 import java.util.Map;
+import java.util.OptionalInt;
 
 /**
  * @since 5.3
@@ -32,6 +33,8 @@ public interface WsRequest {
 
   String getMediaType();
 
+  OptionalInt getTimeOutInMs();
+
   /**
    *
    * In case of multi value parameters, returns the first value
diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java
index 983fa4553a3..9af677135b5 100644
--- a/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java
+++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java
@@ -23,9 +23,11 @@ import java.io.File;
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.Proxy;
+import java.net.SocketTimeoutException;
 import java.util.Base64;
 import java.util.List;
 import java.util.Random;
+import java.util.concurrent.TimeUnit;
 import javax.net.ssl.SSLSocketFactory;
 import okhttp3.ConnectionSpec;
 import okhttp3.mockwebserver.MockResponse;
@@ -35,6 +37,7 @@ import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.RandomStringUtils;
 import org.apache.commons.lang.StringUtils;
+import org.hamcrest.core.IsInstanceOf;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -425,6 +428,43 @@ public class HttpConnectorTest {
     assertThat(underTest.okHttpClient().sslSocketFactory()).isInstanceOf(SSLSocketFactory.getDefault().getClass());
   }
 
+  @Test
+  public void override_timeout_on_get() {
+    underTest = HttpConnector.newBuilder().url(serverUrl).build();
+    server.enqueue(new MockResponse().setBodyDelay(100, TimeUnit.MILLISECONDS).setBody("Hello delayed"));
+
+    expectedException.expect(IllegalStateException.class);
+    expectedException.expectCause(IsInstanceOf.instanceOf(SocketTimeoutException.class));
+
+    WsResponse call = underTest.call(new GetRequest("/").setTimeOutInMs(5));
+    assertThat(call.content()).equals("Hello delayed");
+  }
+
+  @Test
+  public void override_timeout_on_post() {
+    underTest = HttpConnector.newBuilder().url(serverUrl).build();
+    // Headers are not affected by setBodyDelay, let's throttle the answer
+    server.enqueue(new MockResponse().throttleBody(1,100, TimeUnit.MILLISECONDS).setBody("Hello delayed"));
+
+    expectedException.expect(IllegalStateException.class);
+    expectedException.expectCause(IsInstanceOf.instanceOf(SocketTimeoutException.class));
+    WsResponse call = underTest.call(new PostRequest("/").setTimeOutInMs(5));
+    assertThat(call.content()).equals("Hello delayed");
+  }
+
+  @Test
+  public void override_timeout_on_post_with_redirect() {
+    underTest = HttpConnector.newBuilder().url(serverUrl).build();
+    server.enqueue(new MockResponse().setResponseCode(301).setHeader("Location:", "/redirect"));
+    // Headers are not affected by setBodyDelay, let's throttle the answer
+    server.enqueue(new MockResponse().throttleBody(1,100, TimeUnit.MILLISECONDS).setBody("Hello delayed"));
+
+    expectedException.expect(IllegalStateException.class);
+    expectedException.expectCause(IsInstanceOf.instanceOf(SocketTimeoutException.class));
+    WsResponse call = underTest.call(new PostRequest("/").setTimeOutInMs(5));
+    assertThat(call.content()).equals("Hello delayed");
+  }
+
   private void assertTlsAndClearTextSpecifications(HttpConnector underTest) {
     List<ConnectionSpec> connectionSpecs = underTest.okHttpClient().connectionSpecs();
     assertThat(connectionSpecs).hasSize(2);