From bfaf20d59999270bf917ab29c5cb2742fcf9fbfd Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Tue, 6 Oct 2015 21:38:05 +0200 Subject: [PATCH] SONAR-6837 Increase the read-timeout of any call to a SQ WS to 1 minute and make this timeout configurable --- .../sonar/batch/bootstrap/ServerClient.java | 29 +++++++----- .../batch/bootstrap/ServerClientTest.java | 45 +++++++++++++++---- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/ServerClient.java b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/ServerClient.java index 0fcc09bcf3d..f3014a17df1 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/bootstrap/ServerClient.java +++ b/sonar-batch/src/main/java/org/sonar/batch/bootstrap/ServerClient.java @@ -23,6 +23,13 @@ import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.io.Files; import com.google.common.io.InputSupplier; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.UnsupportedEncodingException; +import java.net.URI; +import java.net.URLEncoder; +import javax.annotation.Nullable; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringEscapeUtils; import org.apache.commons.lang.StringUtils; @@ -31,15 +38,6 @@ import org.sonar.api.CoreProperties; import org.sonar.api.utils.HttpDownloader; import org.sonar.batch.bootstrapper.EnvironmentInformation; -import javax.annotation.Nullable; - -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.io.UnsupportedEncodingException; -import java.net.URI; -import java.net.URLEncoder; - /** * Replace the deprecated org.sonar.batch.ServerMetadata * TODO extends Server when removing the deprecated org.sonar.batch.ServerMetadata @@ -48,6 +46,8 @@ import java.net.URLEncoder; */ public class ServerClient implements BatchComponent { + private static final int WS_TIMEOUT_DEFAULT = 60 * 1000; + static final String SONAR_WS_TIMEOUT_PROP = "sonar.ws.timeout"; private BootstrapProperties props; private HttpDownloader.BaseHttpDownloader downloader; @@ -98,13 +98,20 @@ public class ServerClient implements BatchComponent { Preconditions.checkArgument(pathStartingWithSlash.startsWith("/"), "Path must start with slash /"); String path = StringEscapeUtils.escapeHtml(pathStartingWithSlash); + int readTimeout; + if (timeoutMillis != null) { + readTimeout = timeoutMillis.intValue(); + } else { + readTimeout = props.properties().containsKey(SONAR_WS_TIMEOUT_PROP) ? Integer.parseInt(props.property(SONAR_WS_TIMEOUT_PROP)) * 1000 : WS_TIMEOUT_DEFAULT; + } + URI uri = URI.create(getURL() + path); try { InputSupplier inputSupplier; if (Strings.isNullOrEmpty(getLogin())) { - inputSupplier = downloader.newInputSupplier(uri, timeoutMillis); + inputSupplier = downloader.newInputSupplier(uri, readTimeout); } else { - inputSupplier = downloader.newInputSupplier(uri, getLogin(), getPassword(), timeoutMillis); + inputSupplier = downloader.newInputSupplier(uri, getLogin(), getPassword(), readTimeout); } return inputSupplier; } catch (Exception e) { diff --git a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/ServerClientTest.java b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/ServerClientTest.java index 9c9b930f99d..9e14f890056 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/bootstrap/ServerClientTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/bootstrap/ServerClientTest.java @@ -20,26 +20,27 @@ package org.sonar.batch.bootstrap; import com.google.common.base.Charsets; +import com.google.common.collect.ImmutableMap; import com.google.common.io.Files; +import java.io.File; +import java.io.IOException; +import java.net.SocketTimeoutException; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.io.IOUtils; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.AbstractHandler; import org.junit.After; +import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; import org.sonar.batch.bootstrapper.EnvironmentInformation; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import java.io.File; -import java.io.IOException; - import static javax.servlet.http.HttpServletResponse.SC_OK; import static org.apache.commons.io.IOUtils.write; import static org.fest.assertions.Assertions.assertThat; @@ -82,13 +83,31 @@ public class ServerClientTest { assertThat(newServerClient().request("/foo")).isEqualTo("this is the content"); } + @Test + public void should_timeout() throws Exception { + server = new MockHttpServer(); + server.start(); + server.setMockResponseData("this is the content"); + server.setDelay(3 * 1000); + + when(bootstrapProps.properties()).thenReturn(ImmutableMap.of(ServerClient.SONAR_WS_TIMEOUT_PROP, "1")); + when(bootstrapProps.property(ServerClient.SONAR_WS_TIMEOUT_PROP)).thenReturn("1"); + + try { + newServerClient().request("/"); + Assert.fail("Should timeout"); + } catch (Exception e) { + assertThat(e.getCause()).isExactlyInstanceOf(SocketTimeoutException.class); + } + } + @Test public void should_escape_html_from_url() throws Exception { server = new MockHttpServer(); server.start(); server.setMockResponseData("this is the content"); - assertThat(newServerClient().request("/")).isEqualTo("this is the content"); + assertThat(newServerClient().request("/foo")).isEqualTo("this is the content"); } @Test @@ -151,6 +170,7 @@ public class ServerClientTest { private String requestBody; private String mockResponseData; private int mockResponseStatus = SC_OK; + private int delay = 0; public void start() throws Exception { server = new Server(0); @@ -171,6 +191,11 @@ public class ServerClientTest { setRequestBody(IOUtils.toString(baseRequest.getInputStream())); response.setStatus(mockResponseStatus); response.setContentType("text/xml;charset=utf-8"); + try { + Thread.sleep(delay); + } catch (InterruptedException e) { + // Ignore + } write(getResponseBody(), response.getOutputStream()); baseRequest.setHandled(true); } @@ -208,6 +233,10 @@ public class ServerClientTest { this.mockResponseStatus = status; } + public void setDelay(int delay) { + this.delay = delay; + } + public String getMockResponseData() { return mockResponseData; } -- 2.39.5