From: Julien Lancelot Date: Tue, 28 Jun 2016 12:23:44 +0000 (+0200) Subject: SONAR-7776 Allow Java WS to stream the output X-Git-Tag: 6.0-RC1~228 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=0f42fe401f6f72873250f45cfee84ddd138b431c;p=sonarqube.git SONAR-7776 Allow Java WS to stream the output --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BackupAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BackupAction.java index 3814ecba6e8..faf3405a1c4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BackupAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/BackupAction.java @@ -30,9 +30,9 @@ import org.sonar.api.server.ws.WebService; import org.sonar.api.server.ws.WebService.NewAction; import org.sonar.db.DbClient; import org.sonar.db.DbSession; -import org.sonarqube.ws.MediaTypes; import org.sonar.server.qualityprofile.QProfileBackuper; import org.sonar.server.qualityprofile.QProfileFactory; +import org.sonarqube.ws.MediaTypes; public class BackupAction implements QProfileWsAction { @@ -66,12 +66,12 @@ public class BackupAction implements QProfileWsAction { public void handle(Request request, Response response) throws Exception { Stream stream = response.stream(); stream.setMediaType(MediaTypes.XML); - OutputStreamWriter writer = new OutputStreamWriter(stream.output(), StandardCharsets.UTF_8); DbSession dbSession = dbClient.openSession(false); + OutputStreamWriter writer = new OutputStreamWriter(stream.output(), StandardCharsets.UTF_8); try { String profileKey = QProfileIdentificationParamUtils.getProfileKeyFromParameters(request, profileFactory, dbSession); - backuper.backup(profileKey, writer); response.setHeader("Content-Disposition", String.format("attachment; filename=%s.xml", profileKey)); + backuper.backup(profileKey, writer); } finally { dbSession.close(); IOUtils.closeQuietly(writer); diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/LocalRequestAdapter.java b/server/sonar-server/src/main/java/org/sonar/server/ws/LocalRequestAdapter.java index 412b97df73e..f271938dcfb 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/LocalRequestAdapter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/LocalRequestAdapter.java @@ -52,6 +52,11 @@ public class LocalRequestAdapter extends ValidatingRequest { return localRequest.hasParam(key); } + @Override + public String getPath() { + return localRequest.getPath(); + } + @Override public String method() { return localRequest.getMethod(); diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java b/server/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java index c171df2a078..287f8379f20 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/ServletRequest.java @@ -100,4 +100,9 @@ public class ServletRequest extends ValidatingRequest { String formatSuffix = substringAfterLast(url, "."); return SUPPORTED_MEDIA_TYPES_BY_URL_SUFFIX.get(formatSuffix.toLowerCase(Locale.ENGLISH)); } + + @Override + public String getPath(){ + return source.getRequestURI().replaceFirst(source.getContextPath(), ""); + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java b/server/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java index 8a1fcfce4fc..5e11bf6c3bb 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/ServletResponse.java @@ -19,76 +19,78 @@ */ package org.sonar.server.ws; -import java.io.ByteArrayOutputStream; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.sonarqube.ws.MediaTypes.JSON; +import static org.sonarqube.ws.MediaTypes.XML; + +import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; -import java.nio.charset.StandardCharsets; import java.util.Collection; -import java.util.HashMap; -import java.util.Map; -import javax.annotation.CheckForNull; +import javax.servlet.http.HttpServletResponse; import org.sonar.api.server.ws.Response; import org.sonar.api.utils.text.JsonWriter; import org.sonar.api.utils.text.XmlWriter; -import org.sonarqube.ws.MediaTypes; public class ServletResponse implements Response { - private Map headers = new HashMap<>(); + private final ServletStream stream; - public static class ServletStream implements Stream { - private String mediaType; - private int httpStatus = 200; - private final ByteArrayOutputStream output = new ByteArrayOutputStream(); + public ServletResponse(HttpServletResponse response) { + stream = new ServletStream(response); + } - @CheckForNull - public String mediaType() { - return mediaType; - } + public static class ServletStream implements Stream { + private final HttpServletResponse response; - public int httpStatus() { - return httpStatus; + public ServletStream(HttpServletResponse response) { + this.response = response; + this.response.setStatus(200); + // SONAR-6964 WS should not be cached by browser + this.response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); } @Override public ServletStream setMediaType(String s) { - this.mediaType = s; + this.response.setContentType(s); return this; } @Override public ServletStream setStatus(int httpStatus) { - this.httpStatus = httpStatus; + this.response.setStatus(httpStatus); return this; } @Override public OutputStream output() { - return output; + try { + return response.getOutputStream(); + } catch (IOException e) { + throw new IllegalStateException(e); + } } - public String outputAsString() { - return new String(output.toByteArray(), StandardCharsets.UTF_8); + HttpServletResponse response() { + return response; } public ServletStream reset() { - output.reset(); + response.reset(); return this; } } - private final ServletStream stream = new ServletStream(); - @Override public JsonWriter newJsonWriter() { - stream.setMediaType(MediaTypes.JSON); - return JsonWriter.of(new OutputStreamWriter(stream.output(), StandardCharsets.UTF_8)); + stream.setMediaType(JSON); + return JsonWriter.of(new OutputStreamWriter(stream.output(), UTF_8)); } @Override public XmlWriter newXmlWriter() { - stream.setMediaType(MediaTypes.XML); - return XmlWriter.of(new OutputStreamWriter(stream.output(), StandardCharsets.UTF_8)); + stream.setMediaType(XML); + return XmlWriter.of(new OutputStreamWriter(stream.output(), UTF_8)); } @Override @@ -104,17 +106,17 @@ public class ServletResponse implements Response { @Override public Response setHeader(String name, String value) { - headers.put(name, value); + stream.response().setHeader(name, value); return this; } @Override public Collection getHeaderNames() { - return headers.keySet(); + return stream.response().getHeaderNames(); } @Override public String getHeader(String name) { - return headers.get(name); + return stream.response().getHeader(name); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java b/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java index fa1100c9c75..6d39e6d1fc5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceEngine.java @@ -19,12 +19,21 @@ */ package org.sonar.server.ws; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Strings.isNullOrEmpty; +import static java.lang.String.format; +import static org.apache.commons.lang.StringUtils.substring; +import static org.apache.commons.lang.StringUtils.substringAfterLast; +import static org.apache.commons.lang.StringUtils.substringBeforeLast; +import static org.sonar.server.ws.RequestVerifier.verifyRequest; +import static org.sonar.server.ws.ServletRequest.SUPPORTED_MEDIA_TYPES_BY_URL_SUFFIX; + import java.io.OutputStreamWriter; import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Locale; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; -import org.apache.commons.lang.StringUtils; import org.picocontainer.Startable; import org.sonar.api.i18n.I18n; import org.sonar.api.server.ServerSide; @@ -42,13 +51,6 @@ import org.sonar.server.exceptions.ServerException; import org.sonar.server.user.UserSession; import org.sonarqube.ws.MediaTypes; -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Strings.isNullOrEmpty; -import static java.lang.String.format; -import static org.apache.commons.lang.StringUtils.substringAfterLast; -import static org.sonar.server.ws.RequestVerifier.verifyRequest; -import static org.sonar.server.ws.ServletRequest.SUPPORTED_MEDIA_TYPES_BY_URL_SUFFIX; - /** * @since 4.2 */ @@ -79,30 +81,26 @@ public class WebServiceEngine implements LocalConnector, Startable { // nothing } - /** - * Used by Ruby on Rails to add ws routes. See WEB_INF/lib/java_ws_routing.rb - */ - public List controllers() { + List controllers() { return context.controllers(); } @Override public LocalResponse call(LocalRequest request) { - String controller = StringUtils.substringBeforeLast(request.getPath(), "/"); - String action = substringAfterLast(request.getPath(), "/"); DefaultLocalResponse localResponse = new DefaultLocalResponse(); - execute(new LocalRequestAdapter(request), localResponse, controller, action, null); + execute(new LocalRequestAdapter(request), localResponse); return localResponse; } - public void execute(Request request, Response response, String controllerPath, String actionKey, @Nullable String actionExtension) { + public void execute(Request request, Response response) { try { - WebService.Action action = getAction(controllerPath, actionKey); + ActionExtractor actionExtractor = new ActionExtractor(request.getPath()); + WebService.Action action = getAction(actionExtractor.getController(), actionExtractor.getAction()); if (request instanceof ValidatingRequest) { ((ValidatingRequest) request).setAction(action); ((ValidatingRequest) request).setLocalConnector(this); } - checkActionExtension(actionExtension); + checkActionExtension(actionExtractor.getExtension()); verifyRequest(action, request); action.handler().handle(request, response); } catch (IllegalArgumentException e) { @@ -156,4 +154,42 @@ public class WebServiceEngine implements LocalConnector, Startable { checkArgument(SUPPORTED_MEDIA_TYPES_BY_URL_SUFFIX.get(actionExtension.toLowerCase(Locale.ENGLISH)) != null, "Unknown action extension: %s", actionExtension); } + private static class ActionExtractor { + private static final String SLASH = "/"; + private static final String POINT = "."; + + private final String controller; + private final String action; + private final String extension; + + ActionExtractor(String path) { + String pathWithoutExtension = substringBeforeLast(path, POINT); + this.controller = extractController(pathWithoutExtension); + this.action = substringAfterLast(pathWithoutExtension, SLASH); + checkArgument(!action.isEmpty(), "Url is incorrect : '%s'", path); + this.extension = substringAfterLast(path, POINT); + } + + private static String extractController(String path) { + String controller = substringBeforeLast(path, SLASH); + if (controller.startsWith(SLASH)) { + return substring(controller, 1); + } + return controller; + } + + String getController() { + return controller; + } + + String getAction() { + return action; + } + + @CheckForNull + String getExtension() { + return extension; + } + } + } diff --git a/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceFilter.java b/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceFilter.java index 7bc8ce4e440..1c5e10b44d2 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceFilter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ws/WebServiceFilter.java @@ -20,21 +20,14 @@ package org.sonar.server.ws; -import static java.lang.String.format; - -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.OutputStream; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.commons.io.IOUtils; import org.sonar.api.server.ws.RailsHandler; import org.sonar.api.web.ServletFilter; @@ -47,18 +40,20 @@ import org.sonar.api.web.ServletFilter; public class WebServiceFilter extends ServletFilter { private final WebServiceEngine webServiceEngine; - private final Map wsUrls = new HashMap<>(); private final List includeUrls = new ArrayList<>(); + private final List excludeUrls = new ArrayList<>(); public WebServiceFilter(WebServiceEngine webServiceEngine) { this.webServiceEngine = webServiceEngine; webServiceEngine.controllers().stream() .forEach(controller -> controller.actions().stream() - .filter(action -> !(action.handler() instanceof RailsHandler) && !(action.handler() instanceof ServletFilterHandler)) .forEach(action -> { - String url = "/" + action.path(); - wsUrls.put(url, new WsUrl(controller.path(), action.key())); - includeUrls.add(url + "*"); + // Rails and servlet filter WS should not be executed by the web service engine + if (!(action.handler() instanceof RailsHandler) && !(action.handler() instanceof ServletFilterHandler)) { + includeUrls.add("/" + controller.path() + "/*"); + } else { + excludeUrls.add("/" + action.path() + "*"); + } })); } @@ -66,6 +61,7 @@ public class WebServiceFilter extends ServletFilter { public UrlPattern doGetPattern() { return UrlPattern.builder() .includes(includeUrls) + .excludes(excludeUrls) .build(); } @@ -73,45 +69,9 @@ public class WebServiceFilter extends ServletFilter { public void doFilter(javax.servlet.ServletRequest servletRequest, javax.servlet.ServletResponse servletResponse, FilterChain chain) throws IOException, ServletException { HttpServletRequest request = (HttpServletRequest) servletRequest; HttpServletResponse response = (HttpServletResponse) servletResponse; - String path = request.getRequestURI().replaceFirst(request.getContextPath(), ""); - - String[] pathWithExtension = getPathWithExtension(path); - WsUrl url = wsUrls.get(pathWithExtension[0]); - if (url == null) { - throw new IllegalStateException(format("Unknown path : %s", path)); - } ServletRequest wsRequest = new ServletRequest(request); - ServletResponse wsResponse = new ServletResponse(); - webServiceEngine.execute(wsRequest, wsResponse, url.getController(), url.getAction(), pathWithExtension[1]); - writeResponse(wsResponse, response); - } - - private static void writeResponse(ServletResponse wsResponse, HttpServletResponse response) throws IOException { - // SONAR-6964 WS should not be cached by browser - response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); - for (String header : wsResponse.getHeaderNames()) { - response.setHeader(header, wsResponse.getHeader(header)); - } - - response.setContentType(wsResponse.stream().mediaType()); - response.setStatus(wsResponse.stream().httpStatus()); - - OutputStream responseOutput = response.getOutputStream(); - ByteArrayOutputStream wsOutputStream = (ByteArrayOutputStream) wsResponse.stream().output(); - IOUtils.write(wsOutputStream.toByteArray(), responseOutput); - responseOutput.flush(); - responseOutput.close(); - } - - private static String[] getPathWithExtension(String fullPath) { - String path = fullPath; - String extension = null; - int semiColonPos = fullPath.lastIndexOf('.'); - if (semiColonPos > 0) { - path = fullPath.substring(0, semiColonPos); - extension = fullPath.substring(semiColonPos + 1); - } - return new String[] {path, extension}; + ServletResponse wsResponse = new ServletResponse(response); + webServiceEngine.execute(wsRequest, wsResponse); } @Override diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/DumbResponse.java b/server/sonar-server/src/test/java/org/sonar/server/ws/DumbResponse.java index 9c0fd057de9..ca7993cd77f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/DumbResponse.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/DumbResponse.java @@ -67,11 +67,15 @@ public class DumbResponse implements Response { this.status = i; return this; } + @Override public OutputStream output() { return output; } + public String outputAsString() { + return new String(output.toByteArray(), StandardCharsets.UTF_8); + } } @Override @@ -109,13 +113,11 @@ public class DumbResponse implements Response { return this; } - @Override public Collection getHeaderNames() { return headers.keySet(); } - @Override - public String getHeader(String name) { + public String getHeader(String name){ return headers.get(name); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/ServletRequestTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/ServletRequestTest.java index 0ea4f49addc..23f033eeefb 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/ServletRequestTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/ServletRequestTest.java @@ -112,6 +112,16 @@ public class ServletRequestTest { request.readInputStreamParam("param1"); } + @Test + public void getPath() throws Exception { + when(source.getRequestURI()).thenReturn("/sonar/path/to/resource/search"); + when(source.getContextPath()).thenReturn("/sonar"); + + ServletRequest request = new ServletRequest(source); + + assertThat(request.getPath()).isEqualTo("/path/to/resource/search"); + } + @Test public void to_string() { when(source.getRequestURL()).thenReturn(new StringBuffer("http:localhost:9000/api/issues")); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/ServletResponseTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/ServletResponseTest.java new file mode 100644 index 00000000000..fa3b8f3d87d --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/ServletResponseTest.java @@ -0,0 +1,128 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.ws; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.sonarqube.ws.MediaTypes.JSON; +import static org.sonarqube.ws.MediaTypes.XML; + +import javax.servlet.ServletOutputStream; +import javax.servlet.http.HttpServletResponse; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +public class ServletResponseTest { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + ServletOutputStream output = mock(ServletOutputStream.class); + + HttpServletResponse response = mock(HttpServletResponse.class); + + ServletResponse underTest = new ServletResponse(response); + + @Before + public void setUp() throws Exception { + when(response.getOutputStream()).thenReturn(output); + } + + @Test + public void test_default_header() throws Exception { + verify(response).setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); + } + + @Test + public void set_header() throws Exception { + underTest.setHeader("header", "value"); + + verify(response).setHeader("header", "value"); + } + + @Test + public void get_header() throws Exception { + underTest.getHeader("header"); + + verify(response).getHeader("header"); + } + + @Test + public void get_header_names() throws Exception { + underTest.getHeaderNames(); + + verify(response).getHeaderNames(); + } + + @Test + public void test_default_status() throws Exception { + verify(response).setStatus(200); + } + + @Test + public void set_status() throws Exception { + underTest.stream().setStatus(404); + + verify(response).setStatus(404); + } + + @Test + public void test_output() throws Exception { + assertThat(underTest.stream().output()).isEqualTo(output); + } + + + @Test + public void test_reset() throws Exception { + underTest.stream().reset(); + + verify(response).reset(); + } + + @Test + public void test_newJsonWriter() throws Exception { + underTest.newJsonWriter(); + + verify(response).setContentType(JSON); + verify(response).getOutputStream(); + } + + @Test + public void test_newXmlWriter() throws Exception { + underTest.newXmlWriter(); + + verify(response).setContentType(XML); + verify(response).getOutputStream(); + } + + @Test + public void test_noContent() throws Exception { + underTest.noContent(); + + verify(response).setStatus(204); + verify(response, never()).getOutputStream(); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/TestRequest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/TestRequest.java index bab926bf93b..e02e2c04a5a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/TestRequest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/TestRequest.java @@ -19,6 +19,8 @@ */ package org.sonar.server.ws; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.base.Throwables; import java.io.InputStream; import java.util.HashMap; @@ -26,13 +28,12 @@ import java.util.Map; import org.apache.commons.io.IOUtils; import org.sonar.api.server.ws.internal.ValidatingRequest; -import static com.google.common.base.Preconditions.checkNotNull; - public class TestRequest extends ValidatingRequest { private final Map params = new HashMap<>(); private String method = "GET"; private String mimeType = "application/octet-stream"; + private String path; @Override protected String readParam(String key) { @@ -58,6 +59,16 @@ public class TestRequest extends ValidatingRequest { return params.containsKey(key); } + @Override + public String getPath() { + return path; + } + + public TestRequest setPath(String path) { + this.path = path; + return this; + } + public TestRequest setMethod(String method) { checkNotNull(method); this.method = method; diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java index b586eb8c961..5f30364e2b8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceEngineTest.java @@ -19,6 +19,10 @@ */ package org.sonar.server.ws; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import com.google.common.collect.Maps; import java.io.IOException; import java.io.InputStream; @@ -42,14 +46,11 @@ import org.sonar.server.exceptions.Message; import org.sonar.server.tester.UserSessionRule; import org.sonarqube.ws.MediaTypes; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - public class WebServiceEngineTest { @Rule public UserSessionRule userSessionRule = UserSessionRule.standalone(); + I18n i18n = mock(I18n.class); WebServiceEngine underTest = new WebServiceEngine(new WebService[] {new SystemWs()}, i18n, userSessionRule); @@ -72,177 +73,186 @@ public class WebServiceEngineTest { @Test public void execute_request() { - ValidatingRequest request = new SimpleRequest("GET"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "health", null); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/health"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); + + assertThat(response.stream().outputAsString()).isEqualTo("good"); + } + + @Test + public void execute_request_when_path_does_not_begin_with_slash() { + ValidatingRequest request = new SimpleRequest("GET", "api/system/health"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("good"); } @Test public void execute_request_with_action_suffix() { - ValidatingRequest request = new SimpleRequest("GET"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "health", "PROTOBUF"); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/health"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("good"); } @Test public void bad_request_if_action_suffix_is_not_supported() { - ValidatingRequest request = new SimpleRequest("GET"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "health", "bat"); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/health.bat"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); - assertThat(response.stream().httpStatus()).isEqualTo(400); + assertThat(response.stream().status()).isEqualTo(400); assertThat(response.stream().mediaType()).isEqualTo(MediaTypes.JSON); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Unknown action extension: bat\"}]}"); } @Test public void no_content() { - ValidatingRequest request = new SimpleRequest("GET"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "alive", null); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/alive"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEmpty(); } @Test public void bad_controller() { - ValidatingRequest request = new SimpleRequest("GET"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/xxx", "health", null); + ValidatingRequest request = new SimpleRequest("GET", "/api/xxx/health"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Unknown web service: api/xxx\"}]}"); } @Test public void bad_action() { - ValidatingRequest request = new SimpleRequest("GET"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "xxx", null); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/xxx"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Unknown action: api/system/xxx\"}]}"); } @Test public void method_get_not_allowed() { - ValidatingRequest request = new SimpleRequest("GET"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "ping", null); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/ping"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"HTTP method POST is required\"}]}"); } @Test public void method_post_required() { - ValidatingRequest request = new SimpleRequest("POST"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "ping", null); + ValidatingRequest request = new SimpleRequest("POST", "/api/system/ping"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("pong"); } @Test public void unknown_parameter_is_set() { - ValidatingRequest request = new SimpleRequest("GET").setParam("unknown", "Unknown"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "fail_with_undeclared_parameter", null); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/fail_with_undeclared_parameter").setParam("unknown", "Unknown"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"BUG - parameter 'unknown' is undefined for action 'fail_with_undeclared_parameter'\"}]}"); } @Test public void required_parameter_is_not_set() { - ValidatingRequest request = new SimpleRequest("GET"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "print", null); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/print"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"The 'message' parameter is missing\"}]}"); } @Test public void optional_parameter_is_not_set() { - ValidatingRequest request = new SimpleRequest("GET").setParam("message", "Hello World"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "print", null); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/print").setParam("message", "Hello World"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("Hello World by -"); } @Test public void optional_parameter_is_set() { - ValidatingRequest request = new SimpleRequest("GET") + ValidatingRequest request = new SimpleRequest("GET", "/api/system/print") .setParam("message", "Hello World") .setParam("author", "Marcel"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "print", null); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("Hello World by Marcel"); } @Test public void param_value_is_in_possible_values() { - ValidatingRequest request = new SimpleRequest("GET") + ValidatingRequest request = new SimpleRequest("GET", "/api/system/print") .setParam("message", "Hello World") .setParam("format", "json"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "print", null); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("Hello World by -"); } @Test public void param_value_is_not_in_possible_values() { - ValidatingRequest request = new SimpleRequest("GET") + ValidatingRequest request = new SimpleRequest("GET", "api/system/print") .setParam("message", "Hello World") .setParam("format", "html"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "print", null); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Value of parameter 'format' (html) must be one of: [json, xml]\"}]}"); } @Test public void internal_error() { - ValidatingRequest request = new SimpleRequest("GET"); - ServletResponse response = new ServletResponse(); - underTest.execute(request, response, "api/system", "fail", null); + ValidatingRequest request = new SimpleRequest("GET", "/api/system/fail"); + DumbResponse response = new DumbResponse(); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[{\"msg\":\"Unexpected\"}]}"); - assertThat(response.stream().httpStatus()).isEqualTo(500); + assertThat(response.stream().status()).isEqualTo(500); assertThat(response.stream().mediaType()).isEqualTo(MediaTypes.JSON); } @Test public void bad_request_with_i18n_message() { userSessionRule.setLocale(Locale.ENGLISH); - ValidatingRequest request = new SimpleRequest("GET").setParam("count", "3"); - ServletResponse response = new ServletResponse(); + ValidatingRequest request = new SimpleRequest("GET", "api/system/fail_with_i18n_message").setParam("count", "3"); + DumbResponse response = new DumbResponse(); when(i18n.message(Locale.ENGLISH, "bad.request.reason", "bad.request.reason", 0)).thenReturn("reason #0"); - underTest.execute(request, response, "api/system", "fail_with_i18n_message", null); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo( "{\"errors\":[{\"msg\":\"reason #0\"}]}"); - assertThat(response.stream().httpStatus()).isEqualTo(400); + assertThat(response.stream().status()).isEqualTo(400); assertThat(response.stream().mediaType()).isEqualTo(MediaTypes.JSON); } @Test public void bad_request_with_multiple_messages() { - ValidatingRequest request = new SimpleRequest("GET").setParam("count", "3"); - ServletResponse response = new ServletResponse(); + ValidatingRequest request = new SimpleRequest("GET", "api/system/fail_with_multiple_messages").setParam("count", "3"); + DumbResponse response = new DumbResponse(); - underTest.execute(request, response, "api/system", "fail_with_multiple_messages", null); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[" + "{\"msg\":\"Bad request reason #0\"}," + "{\"msg\":\"Bad request reason #1\"}," + "{\"msg\":\"Bad request reason #2\"}" + "]}"); - assertThat(response.stream().httpStatus()).isEqualTo(400); + assertThat(response.stream().status()).isEqualTo(400); assertThat(response.stream().mediaType()).isEqualTo(MediaTypes.JSON); } @@ -250,25 +260,25 @@ public class WebServiceEngineTest { public void bad_request_with_multiple_i18n_messages() { userSessionRule.setLocale(Locale.ENGLISH); - ValidatingRequest request = new SimpleRequest("GET").setParam("count", "3"); - ServletResponse response = new ServletResponse(); + ValidatingRequest request = new SimpleRequest("GET", "api/system/fail_with_multiple_i18n_messages").setParam("count", "3"); + DumbResponse response = new DumbResponse(); when(i18n.message(Locale.ENGLISH, "bad.request.reason", "bad.request.reason", 0)).thenReturn("reason #0"); when(i18n.message(Locale.ENGLISH, "bad.request.reason", "bad.request.reason", 1)).thenReturn("reason #1"); when(i18n.message(Locale.ENGLISH, "bad.request.reason", "bad.request.reason", 2)).thenReturn("reason #2"); - underTest.execute(request, response, "api/system", "fail_with_multiple_i18n_messages", null); + underTest.execute(request, response); assertThat(response.stream().outputAsString()).isEqualTo("{\"errors\":[" + "{\"msg\":\"reason #0\"}," + "{\"msg\":\"reason #1\"}," + "{\"msg\":\"reason #2\"}]}"); - assertThat(response.stream().httpStatus()).isEqualTo(400); + assertThat(response.stream().status()).isEqualTo(400); assertThat(response.stream().mediaType()).isEqualTo(MediaTypes.JSON); } @Test public void should_handle_headers() { - ServletResponse response = new ServletResponse(); + DumbResponse response = new DumbResponse(); String name = "Content-Disposition"; String value = "attachment; filename=sonarqube.zip"; response.setHeader(name, value); @@ -278,10 +288,12 @@ public class WebServiceEngineTest { private static class SimpleRequest extends ValidatingRequest { private final String method; + private String path; private Map params = Maps.newHashMap(); - private SimpleRequest(String method) { + private SimpleRequest(String method, String path) { this.method = method; + this.path = path; } @Override @@ -299,6 +311,11 @@ public class WebServiceEngineTest { return params.keySet().contains(key); } + @Override + public String getPath() { + return path; + } + @Override protected String readParam(String key) { return params.get(key); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceFilterTest.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceFilterTest.java index 8d3f01133a1..8e652c4c40f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceFilterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WebServiceFilterTest.java @@ -22,10 +22,8 @@ package org.sonar.server.ws; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import static org.sonar.server.ws.WebServiceFilterTest.WsUrl.newWsUrl; @@ -71,7 +69,8 @@ public class WebServiceFilterTest { newWsUrl("api/issues", "search"), newWsUrl("batch", "index"), newWsUrl("api/authentication", "login").setHandler(ServletFilterHandler.INSTANCE), - newWsUrl("api/resources", "index").setHandler(RailsHandler.INSTANCE)); + newWsUrl("api/resources", "index").setHandler(RailsHandler.INSTANCE), + newWsUrl("api/issues", "deprecatedSearch").setHandler(RailsHandler.INSTANCE)); assertThat(underTest.doGetPattern().matches("/api/issues/search")).isTrue(); assertThat(underTest.doGetPattern().matches("/api/issues/search.protobuf")).isTrue(); @@ -79,44 +78,17 @@ public class WebServiceFilterTest { assertThat(underTest.doGetPattern().matches("/api/resources/index")).isFalse(); assertThat(underTest.doGetPattern().matches("/api/authentication/login")).isFalse(); + assertThat(underTest.doGetPattern().matches("/api/issues/deprecatedSearch")).isFalse(); assertThat(underTest.doGetPattern().matches("/foo")).isFalse(); } @Test - public void execute() throws Exception { - initWebServiceEngine(newWsUrl("api/issues", "search")); - when(request.getRequestURI()).thenReturn("/api/issues/search"); - - underTest.doFilter(request, response, chain); - - verify(webServiceEngine).execute(any(ServletRequest.class), any(org.sonar.server.ws.ServletResponse.class), eq("api/issues"), eq("search"), eq(null)); - - verify(response).setStatus(200); - verify(response).setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); - - verify(responseOutput).flush(); - verify(responseOutput).close(); - verifyZeroInteractions(chain); - } - - @Test - public void execute_with_extension() throws Exception { - initWebServiceEngine(newWsUrl("api/issues", "search")); - when(request.getRequestURI()).thenReturn("/api/issues/search.protobuff"); + public void execute_ws() throws Exception { + underTest = new WebServiceFilter(webServiceEngine); underTest.doFilter(request, response, chain); - verify(webServiceEngine).execute(any(ServletRequest.class), any(org.sonar.server.ws.ServletResponse.class), eq("api/issues"), eq("search"), eq("protobuff")); - } - - @Test - public void fail_on_unknown_path() throws Exception { - initWebServiceEngine(newWsUrl("api/issues", "search")); - when(request.getRequestURI()).thenReturn("/api/resources/index"); - - expectedException.expect(IllegalStateException.class); - expectedException.expectMessage("Unknown path : /api/resources/index"); - underTest.doFilter(request, response, chain); + verify(webServiceEngine).execute(any(ServletRequest.class), any(org.sonar.server.ws.ServletResponse.class)); } private void initWebServiceEngine(WsUrl... wsUrls) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/ws/WsTester.java b/server/sonar-server/src/test/java/org/sonar/server/ws/WsTester.java index 446f0baee8f..7b0e580cdb0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ws/WsTester.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ws/WsTester.java @@ -19,6 +19,9 @@ */ package org.sonar.server.ws; +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.server.ws.RequestVerifier.verifyRequest; + import com.google.common.collect.Maps; import java.io.ByteArrayOutputStream; import java.io.InputStream; @@ -41,9 +44,6 @@ import org.sonar.server.ws.WsTester.TestResponse.TestStream; import org.sonar.test.JsonAssert; import org.sonarqube.ws.MediaTypes; -import static org.assertj.core.api.Assertions.assertThat; -import static org.sonar.server.ws.RequestVerifier.verifyRequest; - /** * @since 4.2 */ @@ -52,6 +52,7 @@ public class WsTester { public static class TestRequest extends ValidatingRequest { private final String method; + private String path; private Map params = Maps.newHashMap(); @@ -74,6 +75,16 @@ public class WsTester { return params.keySet().contains(key); } + @Override + public String getPath() { + return path; + } + + public TestRequest setPath(String path) { + this.path = path; + return this; + } + public TestRequest setParams(Map m) { this.params = m; return this; diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java index c746da58dfa..50dbabfdc58 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java @@ -264,4 +264,10 @@ public abstract class Request { */ @Beta public abstract LocalConnector localConnector(); + + /** + * Return path of the request + * @since 6.0 + */ + public abstract String getPath(); } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java index e9d2c0827d1..3e583f691a1 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java @@ -19,6 +19,8 @@ */ package org.sonar.api.server.ws.internal; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.collect.Maps; import java.io.InputStream; import java.util.Map; @@ -27,8 +29,6 @@ import org.apache.commons.io.IOUtils; import org.sonar.api.server.ws.LocalConnector; import org.sonar.api.server.ws.Request; -import static com.google.common.base.Preconditions.checkNotNull; - /** * Fake implementation of {@link org.sonar.api.server.ws.Request} used * for testing. Call the method {@link #setParam(String, String)} to @@ -38,6 +38,7 @@ public class SimpleGetRequest extends Request { private final Map params = Maps.newHashMap(); private String mediaType = "application/json"; + private String path; @Override public String method() { @@ -85,4 +86,14 @@ public class SimpleGetRequest extends Request { public LocalConnector localConnector() { throw new UnsupportedOperationException(); } + + @Override + public String getPath() { + return path; + } + + public SimpleGetRequest setPath(String path) { + this.path = path; + return this; + } } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java index a25365196a7..a8b939ee116 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java @@ -19,6 +19,9 @@ */ package org.sonar.api.server.ws; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + import com.google.common.collect.Maps; import java.io.InputStream; import java.util.Map; @@ -32,9 +35,6 @@ import org.sonar.api.rule.RuleStatus; import org.sonar.api.server.ws.internal.ValidatingRequest; import org.sonar.api.utils.DateUtils; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; - public class RequestTest { @Rule @@ -278,6 +278,11 @@ public class RequestTest { return params.keySet().contains(key); } + @Override + public String getPath() { + return null; + } + public FakeRequest setParam(String key, @Nullable String value) { if (value != null) { params.put(key, value);