SONAR-4488 Increase timeout for /batch_bootstrap/db

sonar-batch/src/main/java/org/sonar/batch/bootstrap/DryRunDatabase.java

@@ -32,6 +32,7 @@ import org.sonar.api.utils.HttpDownloader.HttpException;
 import org.sonar.api.utils.SonarException;
 
 import java.io.File;
+import java.net.SocketTimeoutException;
 
 /**
  * @since 3.4
@@ -45,6 +46,8 @@ public class DryRunDatabase implements BatchComponent {
   private static final String USER = "sonar";
   private static final String PASSWORD = "sonar";
 
+  private static final int DEFAULT_DRY_RUN_READ_TIMEOUT = 60 * 1000;
+
   private final Settings settings;
   private final ServerClient server;
   private final TempDirectories tempDirectories;
@@ -59,14 +62,19 @@ public class DryRunDatabase implements BatchComponent {
     if (settings.getBoolean(CoreProperties.DRY_RUN)) {
       LOG.info("Dry run");
       File databaseFile = tempDirectories.getFile("", "dryrun.h2.db");
-      downloadDatabase(databaseFile);
+
+      // SONAR-4488 Allow to increase dryRun timeout
+      int readTimeout = settings.getInt(CoreProperties.DRY_RUN_READ_TIMEOUT);
+      readTimeout = (readTimeout == 0) ? DEFAULT_DRY_RUN_READ_TIMEOUT : readTimeout;
+
+      downloadDatabase(databaseFile, readTimeout);
 
       String databasePath = StringUtils.removeEnd(databaseFile.getAbsolutePath(), ".h2.db");
       replaceSettings(databasePath);
     }
   }
 
-  private void downloadDatabase(File toFile) {
+  private void downloadDatabase(File toFile, int readTimeout) {
     String projectKey = null;
     try {
       projectKey = settings.getString(CoreProperties.PROJECT_KEY_PROPERTY);
@@ -75,15 +83,22 @@ public class DryRunDatabase implements BatchComponent {
         projectKey = String.format("%s:%s", projectKey, branch);
       }
       if (StringUtils.isBlank(projectKey)) {
-        server.download("/batch_bootstrap/db", toFile);
+        server.download("/batch_bootstrap/db", toFile, readTimeout);
       } else {
-        server.download("/batch_bootstrap/db?project=" + projectKey, toFile);
+        server.download("/batch_bootstrap/db?project=" + projectKey, toFile, readTimeout);
       }
       LOG.debug("Dry Run database size: {}", FileUtils.byteCountToDisplaySize(FileUtils.sizeOf(toFile)));
     } catch (SonarException e) {
       Throwable rootCause = Throwables.getRootCause(e);
+      if (rootCause instanceof SocketTimeoutException) {
+        // Pico will unwrap the first runtime exception
+        throw new SonarException(new SonarException(String.format("DryRun database read timed out after %s ms. You can try to increase read timeout with property -D"
+          + CoreProperties.DRY_RUN_READ_TIMEOUT,
+            readTimeout), e));
+      }
       if (projectKey != null && (rootCause instanceof HttpException) && (((HttpException) rootCause).getResponseCode() == 401)) {
-        throw new SonarException(String.format("You don't have access rights to project [%s]", projectKey), e);
+        // Pico will unwrap the first runtime exception
+        throw new SonarException(new SonarException(String.format("You don't have access rights to project [%s]", projectKey), e));
       }
       throw e;
     }

sonar-batch/src/main/java/org/sonar/batch/bootstrap/ServerClient.java

@@ -58,8 +58,12 @@ public class ServerClient implements BatchComponent {
   }
 
   public void download(String pathStartingWithSlash, File toFile) {
+    download(pathStartingWithSlash, toFile, null);
+  }
+
+  public void download(String pathStartingWithSlash, File toFile, Integer readTimeoutMillis) {
     try {
-      InputSupplier<InputStream> inputSupplier = doRequest(pathStartingWithSlash);
+      InputSupplier<InputStream> inputSupplier = doRequest(pathStartingWithSlash, readTimeoutMillis);
       Files.copy(inputSupplier, toFile);
     } catch (HttpDownloader.HttpException he) {
       throw handleHttpException(he);
@@ -73,7 +77,11 @@ public class ServerClient implements BatchComponent {
   }
 
   public String request(String pathStartingWithSlash, boolean wrapHttpException) {
-    InputSupplier<InputStream> inputSupplier = doRequest(pathStartingWithSlash);
+    return request(pathStartingWithSlash, wrapHttpException, null);
+  }
+
+  public String request(String pathStartingWithSlash, boolean wrapHttpException, Integer timeoutMillis) {
+    InputSupplier<InputStream> inputSupplier = doRequest(pathStartingWithSlash, timeoutMillis);
     try {
       return IOUtils.toString(inputSupplier.getInput(), "UTF-8");
     } catch (HttpDownloader.HttpException e) {
@@ -83,7 +91,7 @@ public class ServerClient implements BatchComponent {
     }
   }
 
-  private InputSupplier<InputStream> doRequest(String pathStartingWithSlash) {
+  private InputSupplier<InputStream> doRequest(String pathStartingWithSlash, Integer timeoutMillis) {
     Preconditions.checkArgument(pathStartingWithSlash.startsWith("/"), "Path must start with slash /");
     String path = StringEscapeUtils.escapeHtml(pathStartingWithSlash);
 
@@ -91,9 +99,9 @@ public class ServerClient implements BatchComponent {
     try {
       InputSupplier<InputStream> inputSupplier;
       if (Strings.isNullOrEmpty(getLogin())) {
-        inputSupplier = downloader.newInputSupplier(uri);
+        inputSupplier = downloader.newInputSupplier(uri, timeoutMillis);
       } else {
-        inputSupplier = downloader.newInputSupplier(uri, getLogin(), getPassword());
+        inputSupplier = downloader.newInputSupplier(uri, getLogin(), getPassword(), timeoutMillis);
       }
       return inputSupplier;
     } catch (Exception e) {

sonar-batch/src/test/java/org/sonar/batch/bootstrap/DryRunDatabaseTest.java

@@ -32,6 +32,7 @@ import org.sonar.api.utils.HttpDownloader;
 import org.sonar.api.utils.SonarException;
 
 import java.io.File;
+import java.net.SocketTimeoutException;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.mockito.Mockito.doThrow;
@@ -73,7 +74,15 @@ public class DryRunDatabaseTest {
   public void should_download_database() {
     new DryRunDatabase(settings, server, tempDirectories).start();
 
-    verify(server).download("/batch_bootstrap/db?project=group:project", databaseFile);
+    verify(server).download("/batch_bootstrap/db?project=group:project", databaseFile, 60000);
+  }
+
+  @Test
+  public void should_download_database_with_overriden_timeout() {
+    settings.setProperty(CoreProperties.DRY_RUN_READ_TIMEOUT, 80000);
+    new DryRunDatabase(settings, server, tempDirectories).start();
+
+    verify(server).download("/batch_bootstrap/db?project=group:project", databaseFile, 80000);
   }
 
   @Test
@@ -81,7 +90,7 @@ public class DryRunDatabaseTest {
     settings.setProperty(CoreProperties.PROJECT_BRANCH_PROPERTY, "mybranch");
     new DryRunDatabase(settings, server, tempDirectories).start();
 
-    verify(server).download("/batch_bootstrap/db?project=group:project:mybranch", databaseFile);
+    verify(server).download("/batch_bootstrap/db?project=group:project:mybranch", databaseFile, 60000);
   }
 
   @Test
@@ -97,7 +106,7 @@ public class DryRunDatabaseTest {
 
   @Test
   public void should_fail_on_invalid_role() {
-    doThrow(new SonarException(new HttpDownloader.HttpException(null, 401))).when(server).download("/batch_bootstrap/db?project=group:project", databaseFile);
+    doThrow(new SonarException(new HttpDownloader.HttpException(null, 401))).when(server).download("/batch_bootstrap/db?project=group:project", databaseFile, 60000);
 
     thrown.expect(SonarException.class);
     thrown.expectMessage("You don't have access rights to project [group:project]");
@@ -105,9 +114,19 @@ public class DryRunDatabaseTest {
     new DryRunDatabase(settings, server, tempDirectories).start();
   }
 
+  @Test
+  public void should_fail_on_read_timeout() {
+    doThrow(new SonarException(new SocketTimeoutException())).when(server).download("/batch_bootstrap/db?project=group:project", databaseFile, 60000);
+
+    thrown.expect(SonarException.class);
+    thrown.expectMessage("DryRun database read timed out after 60000 ms. You can try to increase read timeout with property -Dsonar.dryRun.readTimeout");
+
+    new DryRunDatabase(settings, server, tempDirectories).start();
+  }
+
   @Test
   public void should_fail() {
-    doThrow(new SonarException("BUG")).when(server).download("/batch_bootstrap/db?project=group:project", databaseFile);
+    doThrow(new SonarException("BUG")).when(server).download("/batch_bootstrap/db?project=group:project", databaseFile, 60000);
 
     thrown.expect(SonarException.class);
     thrown.expectMessage("BUG");
@@ -120,6 +139,6 @@ public class DryRunDatabaseTest {
     // on non-scan tasks
     settings.removeProperty(CoreProperties.PROJECT_KEY_PROPERTY);
     new DryRunDatabase(settings, server, tempDirectories).start();
-    verify(server).download("/batch_bootstrap/db", databaseFile);
+    verify(server).download("/batch_bootstrap/db", databaseFile, 60000);
   }
 }

sonar-plugin-api/src/main/java/org/sonar/api/CoreProperties.java

@@ -400,4 +400,9 @@ public interface CoreProperties {
    */
   @Deprecated
   String CORE_COVERAGE_PLUGIN_PROPERTY = "sonar.core.codeCoveragePlugin";
+
+  /**
+   * @since 3.7
+   */
+  String DRY_RUN_READ_TIMEOUT = "sonar.dryRun.readTimeout";
 }

sonar-plugin-api/src/main/java/org/sonar/api/utils/HttpDownloader.java

@@ -37,6 +37,8 @@ import org.sonar.api.ServerComponent;
 import org.sonar.api.config.Settings;
 import org.sonar.api.platform.Server;
 
+import javax.annotation.Nullable;
+
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -59,12 +61,23 @@ public class HttpDownloader extends UriReader.SchemeProcessor implements BatchCo
   public static final int TIMEOUT_MILLISECONDS = 20 * 1000;
 
   private final BaseHttpDownloader downloader;
+  private final Integer readTimeout;
 
   public HttpDownloader(Server server, Settings settings) {
+    this(server, settings, null);
+  }
+
+  public HttpDownloader(Server server, Settings settings, @Nullable Integer readTimeout) {
+    this.readTimeout = readTimeout;
     downloader = new BaseHttpDownloader(settings.getProperties(), server.getVersion());
   }
 
   public HttpDownloader(Settings settings) {
+    this(settings, null);
+  }
+
+  public HttpDownloader(Settings settings, @Nullable Integer readTimeout) {
+    this.readTimeout = readTimeout;
     downloader = new BaseHttpDownloader(settings.getProperties(), null);
   }
 
@@ -86,7 +99,7 @@ public class HttpDownloader extends UriReader.SchemeProcessor implements BatchCo
   @Override
   String readString(URI uri, Charset charset) {
     try {
-      return CharStreams.toString(CharStreams.newReaderSupplier(downloader.newInputSupplier(uri), charset));
+      return CharStreams.toString(CharStreams.newReaderSupplier(downloader.newInputSupplier(uri, this.readTimeout), charset));
     } catch (IOException e) {
       throw failToDownload(uri, e);
     }
@@ -98,7 +111,7 @@ public class HttpDownloader extends UriReader.SchemeProcessor implements BatchCo
 
   public byte[] download(URI uri) {
     try {
-      return ByteStreams.toByteArray(downloader.newInputSupplier(uri));
+      return ByteStreams.toByteArray(downloader.newInputSupplier(uri, this.readTimeout));
     } catch (IOException e) {
       throw failToDownload(uri, e);
     }
@@ -110,7 +123,7 @@ public class HttpDownloader extends UriReader.SchemeProcessor implements BatchCo
 
   public InputStream openStream(URI uri) {
     try {
-      return downloader.newInputSupplier(uri).getInput();
+      return downloader.newInputSupplier(uri, this.readTimeout).getInput();
     } catch (IOException e) {
       throw failToDownload(uri, e);
     }
@@ -118,7 +131,7 @@ public class HttpDownloader extends UriReader.SchemeProcessor implements BatchCo
 
   public void download(URI uri, File toFile) {
     try {
-      Files.copy(downloader.newInputSupplier(uri), toFile);
+      Files.copy(downloader.newInputSupplier(uri, this.readTimeout), toFile);
     } catch (IOException e) {
       FileUtils.deleteQuietly(toFile);
       throw failToDownload(uri, e);
@@ -193,11 +206,25 @@ public class HttpDownloader extends UriReader.SchemeProcessor implements BatchCo
     }
 
     public InputSupplier<InputStream> newInputSupplier(URI uri) {
-      return new HttpInputSupplier(uri, userAgent, null, null);
+      return new HttpInputSupplier(uri, userAgent, null, null, TIMEOUT_MILLISECONDS);
+    }
+
+    public InputSupplier<InputStream> newInputSupplier(URI uri, @Nullable Integer readTimeoutMillis) {
+      if (readTimeoutMillis != null) {
+        return new HttpInputSupplier(uri, userAgent, null, null, readTimeoutMillis);
+      }
+      return new HttpInputSupplier(uri, userAgent, null, null, TIMEOUT_MILLISECONDS);
     }
 
     public InputSupplier<InputStream> newInputSupplier(URI uri, String login, String password) {
-      return new HttpInputSupplier(uri, userAgent, login, password);
+      return new HttpInputSupplier(uri, userAgent, login, password, TIMEOUT_MILLISECONDS);
+    }
+
+    public InputSupplier<InputStream> newInputSupplier(URI uri, String login, String password, @Nullable Integer readTimeoutMillis) {
+      if (readTimeoutMillis != null) {
+        return new HttpInputSupplier(uri, userAgent, login, password, readTimeoutMillis);
+      }
+      return new HttpInputSupplier(uri, userAgent, login, password, TIMEOUT_MILLISECONDS);
     }
 
     private static class HttpInputSupplier implements InputSupplier<InputStream> {
@@ -205,12 +232,14 @@ public class HttpDownloader extends UriReader.SchemeProcessor implements BatchCo
       private final String password;
       private final URI uri;
       private final String userAgent;
+      private final int readTimeoutMillis;
 
-      HttpInputSupplier(URI uri, String userAgent, String login, String password) {
+      HttpInputSupplier(URI uri, String userAgent, String login, String password, int readTimeoutMillis) {
         this.uri = uri;
         this.userAgent = userAgent;
         this.login = login;
         this.password = password;
+        this.readTimeoutMillis = readTimeoutMillis;
       }
 
       public InputStream getInput() throws IOException {
@@ -222,7 +251,7 @@ public class HttpDownloader extends UriReader.SchemeProcessor implements BatchCo
           connection.setRequestProperty("Authorization", "Basic " + encoded);
         }
         connection.setConnectTimeout(TIMEOUT_MILLISECONDS);
-        connection.setReadTimeout(TIMEOUT_MILLISECONDS);
+        connection.setReadTimeout(readTimeoutMillis);
         connection.setUseCaches(true);
         connection.setInstanceFollowRedirects(true);
         connection.setRequestProperty("User-Agent", userAgent);

sonar-plugin-api/src/test/java/org/sonar/api/utils/HttpDownloaderTest.java

@@ -20,10 +20,13 @@
 package org.sonar.api.utils;
 
 import com.google.common.base.Charsets;
+import org.hamcrest.BaseMatcher;
+import org.hamcrest.Description;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
 import org.simpleframework.http.Request;
 import org.simpleframework.http.Response;
@@ -39,6 +42,7 @@ import java.net.InetSocketAddress;
 import java.net.Proxy;
 import java.net.ProxySelector;
 import java.net.SocketAddress;
+import java.net.SocketTimeoutException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.Arrays;
@@ -54,6 +58,9 @@ public class HttpDownloaderTest {
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
   private static SocketConnection socketConnection;
   private static String baseUrl;
 
@@ -65,7 +72,15 @@ public class HttpDownloaderTest {
           if (req.getPath().getPath().contains("/redirect/")) {
             resp.setCode(303);
             resp.add("Location", "/");
-          } else {
+          }
+          else {
+            if (req.getPath().getPath().contains("/timeout/")) {
+              try {
+                Thread.sleep(1000);
+              } catch (InterruptedException e) {
+                e.printStackTrace();
+              }
+            }
             resp.getPrintStream().append("agent=" + req.getValues("User-Agent").get(0));
           }
         } catch (IOException e) {
@@ -101,6 +116,33 @@ public class HttpDownloaderTest {
     assertThat(text.length()).isGreaterThan(10);
   }
 
+  @Test
+  public void readStringWithDefaultTimeout() throws URISyntaxException {
+    String text = new HttpDownloader(new Settings()).readString(new URI(baseUrl + "/timeout/"), Charsets.UTF_8);
+    assertThat(text.length()).isGreaterThan(10);
+  }
+
+  @Test
+  public void readStringWithTimeout() throws URISyntaxException {
+    String text = new HttpDownloader(new Settings(), 2000).readString(new URI(baseUrl + "/timeout/"), Charsets.UTF_8);
+    assertThat(text.length()).isGreaterThan(10);
+
+    thrown.expect(new BaseMatcher<Exception>() {
+      @Override
+      public boolean matches(Object ex) {
+        // TODO Auto-generated method stub
+        return ex instanceof SonarException && ((SonarException) ex).getCause() instanceof SocketTimeoutException;
+      }
+
+      @Override
+      public void describeTo(Description arg0) {
+        // TODO Auto-generated method stub
+
+      }
+    });
+    new HttpDownloader(new Settings(), 100).readString(new URI(baseUrl + "/timeout/"), Charsets.UTF_8);
+  }
+
   @Test(expected = SonarException.class)
   public void failIfServerDown() throws URISyntaxException {
     // I hope that the port 1 is not used !