From 5e04bd807b2b6f0ab6e213b5f786a10c35743166 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Thu, 25 Aug 2016 16:13:59 +0200 Subject: [PATCH] SONAR-8003 WS API handles multi param --- .../sonar/server/ws/LocalRequestAdapter.java | 6 ++ .../org/sonar/server/ws/ServletRequest.java | 19 +++- .../sonar/server/ws/ServletRequestTest.java | 94 ++++++++++++------- .../java/org/sonar/server/ws/TestRequest.java | 13 ++- .../java/org/sonar/server/ws/WsTester.java | 15 ++- .../java/org/sonar/api/server/ws/Request.java | 15 ++- .../server/ws/internal/SimpleGetRequest.java | 13 ++- .../server/ws/internal/ValidatingRequest.java | 40 +++++++- .../org/sonar/api/server/ws/RequestTest.java | 46 +++++++-- 9 files changed, 204 insertions(+), 57 deletions(-) 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 98e452668ec..7b24df8dea7 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 @@ -22,6 +22,7 @@ package org.sonar.server.ws; import java.io.ByteArrayInputStream; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.util.List; import org.sonar.api.server.ws.LocalConnector; import org.sonar.api.server.ws.internal.ValidatingRequest; @@ -38,6 +39,11 @@ public class LocalRequestAdapter extends ValidatingRequest { return localRequest.getParam(key); } + @Override + protected List readMultiParam(String key) { + throw new UnsupportedOperationException("reading multi value param is not supported yet by local WS calls"); + } + @Override protected InputStream readInputStreamParam(String key) { String value = readParam(key); 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 75efe64c1f5..4513d249f54 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 @@ -19,14 +19,11 @@ */ package org.sonar.server.ws; -import static com.google.common.base.MoreObjects.firstNonNull; -import static java.util.Locale.ENGLISH; -import static org.apache.commons.lang.StringUtils.substringAfterLast; -import static org.apache.tomcat.util.http.fileupload.FileUploadBase.MULTIPART; - +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.net.HttpHeaders; import java.io.InputStream; +import java.util.List; import java.util.Map; import javax.annotation.CheckForNull; import javax.servlet.http.HttpServletRequest; @@ -34,6 +31,12 @@ import org.sonar.api.server.ws.internal.PartImpl; import org.sonar.api.server.ws.internal.ValidatingRequest; import org.sonarqube.ws.MediaTypes; +import static com.google.common.base.MoreObjects.firstNonNull; +import static java.util.Collections.emptyList; +import static java.util.Locale.ENGLISH; +import static org.apache.commons.lang.StringUtils.substringAfterLast; +import static org.apache.tomcat.util.http.fileupload.FileUploadBase.MULTIPART; + public class ServletRequest extends ValidatingRequest { private final HttpServletRequest source; @@ -71,6 +74,12 @@ public class ServletRequest extends ValidatingRequest { return source.getParameter(key); } + @Override + protected List readMultiParam(String key) { + String[] values = source.getParameterValues(key); + return values == null ? emptyList() : ImmutableList.copyOf(values); + } + @Override protected InputStream readInputStreamParam(String key) { Part part = readPart(key); 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 9b7f108fdd4..cb21c225c30 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 @@ -19,15 +19,10 @@ */ package org.sonar.server.ws; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - import com.google.common.collect.ImmutableMap; import com.google.common.net.HttpHeaders; import java.io.InputStream; +import java.util.List; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.Part; import org.junit.Rule; @@ -35,6 +30,12 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonarqube.ws.MediaTypes; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + public class ServletRequestTest { @Rule @@ -42,10 +43,12 @@ public class ServletRequestTest { HttpServletRequest source = mock(HttpServletRequest.class); + ServletRequest underTest = new ServletRequest(source); + @Test public void call_method() { - ServletRequest request = new ServletRequest(source); - request.method(); + underTest.method(); + verify(source).getMethod(); } @@ -53,23 +56,23 @@ public class ServletRequestTest { public void getMediaType() throws Exception { when(source.getHeader(HttpHeaders.ACCEPT)).thenReturn(MediaTypes.JSON); when(source.getRequestURI()).thenReturn("/path/to/resource/search"); - ServletRequest request = new ServletRequest(source); - assertThat(request.getMediaType()).isEqualTo(MediaTypes.JSON); + + assertThat(underTest.getMediaType()).isEqualTo(MediaTypes.JSON); } @Test public void default_media_type_is_octet_stream() throws Exception { - ServletRequest request = new ServletRequest(source); when(source.getRequestURI()).thenReturn("/path/to/resource/search"); - assertThat(request.getMediaType()).isEqualTo(MediaTypes.DEFAULT); + + assertThat(underTest.getMediaType()).isEqualTo(MediaTypes.DEFAULT); } @Test public void media_type_taken_in_url_first() throws Exception { - ServletRequest request = new ServletRequest(source); when(source.getHeader(HttpHeaders.ACCEPT)).thenReturn(MediaTypes.JSON); when(source.getRequestURI()).thenReturn("/path/to/resource/search.protobuf"); - assertThat(request.getMediaType()).isEqualTo(MediaTypes.PROTOBUF); + + assertThat(underTest.getMediaType()).isEqualTo(MediaTypes.PROTOBUF); } @Test @@ -82,8 +85,35 @@ public class ServletRequestTest { @Test public void read_param_from_source() { when(source.getParameter("param")).thenReturn("value"); - ServletRequest request = new ServletRequest(source); - assertThat(request.readParam("param")).isEqualTo("value"); + + assertThat(underTest.readParam("param")).isEqualTo("value"); + } + + @Test + public void read_multi_param_from_source_with_values() { + when(source.getParameterValues("param")).thenReturn(new String[]{"firstValue", "secondValue", "thirdValue"}); + + List result = underTest.readMultiParam("param"); + + assertThat(result).containsExactly("firstValue", "secondValue", "thirdValue"); + } + + @Test + public void read_multi_param_from_source_with_one_value() { + when(source.getParameterValues("param")).thenReturn(new String[]{"firstValue"}); + + List result = underTest.readMultiParam("param"); + + assertThat(result).containsExactly("firstValue"); + } + + @Test + public void read_multi_param_from_source_without_value() { + when(source.getParameterValues("param")).thenReturn(null); + + List result = underTest.readMultiParam("param"); + + assertThat(result).isEmpty(); } @Test @@ -95,10 +125,8 @@ public class ServletRequestTest { when(part.getSize()).thenReturn(10L); when(source.getPart("param1")).thenReturn(part); - ServletRequest request = new ServletRequest(source); - assertThat(request.readInputStreamParam("param1")).isEqualTo(file); - - assertThat(request.readInputStreamParam("param2")).isNull(); + assertThat(underTest.readInputStreamParam("param1")).isEqualTo(file); + assertThat(underTest.readInputStreamParam("param2")).isNull(); } @Test @@ -110,22 +138,21 @@ public class ServletRequestTest { when(part.getSize()).thenReturn(0L); when(source.getPart("param1")).thenReturn(part); - ServletRequest request = new ServletRequest(source); - assertThat(request.readInputStreamParam("param1")).isNull(); + assertThat(underTest.readInputStreamParam("param1")).isNull(); } @Test public void return_no_input_stream_when_content_type_is_not_multipart() throws Exception { when(source.getContentType()).thenReturn("multipart/form-data"); - ServletRequest request = new ServletRequest(source); - assertThat(request.readInputStreamParam("param1")).isNull(); + + assertThat(underTest.readInputStreamParam("param1")).isNull(); } @Test public void return_no_input_stream_when_content_type_is_null() throws Exception { when(source.getContentType()).thenReturn(null); - ServletRequest request = new ServletRequest(source); - assertThat(request.readInputStreamParam("param1")).isNull(); + + assertThat(underTest.readInputStreamParam("param1")).isNull(); } @Test @@ -136,11 +163,11 @@ public class ServletRequestTest { when(part.getSize()).thenReturn(0L); when(part.getInputStream()).thenReturn(file); doThrow(IllegalArgumentException.class).when(source).getPart("param1"); - ServletRequest request = new ServletRequest(source); expectedException.expect(IllegalStateException.class); expectedException.expectMessage("Can't read file part"); - request.readInputStreamParam("param1"); + + underTest.readInputStreamParam("param1"); } @Test @@ -148,19 +175,16 @@ public class ServletRequestTest { 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"); + assertThat(underTest.getPath()).isEqualTo("/path/to/resource/search"); } @Test public void to_string() { when(source.getRequestURL()).thenReturn(new StringBuffer("http:localhost:9000/api/issues")); - ServletRequest request = new ServletRequest(source); - assertThat(request.toString()).isEqualTo("http:localhost:9000/api/issues"); + assertThat(underTest.toString()).isEqualTo("http:localhost:9000/api/issues"); when(source.getQueryString()).thenReturn("components=sonar"); - request = new ServletRequest(source); - assertThat(request.toString()).isEqualTo("http:localhost:9000/api/issues?components=sonar"); + + assertThat(underTest.toString()).isEqualTo("http:localhost:9000/api/issues?components=sonar"); } } 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 ac7e439c5a2..69a49043eb0 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,17 +19,20 @@ */ package org.sonar.server.ws; -import static com.google.common.base.Preconditions.checkNotNull; - import com.google.common.base.Throwables; import com.google.common.collect.Maps; import java.io.InputStream; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.commons.io.IOUtils; import org.sonar.api.server.ws.internal.PartImpl; import org.sonar.api.server.ws.internal.ValidatingRequest; +import static com.google.common.base.Preconditions.checkNotNull; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; + public class TestRequest extends ValidatingRequest { private final Map params = new HashMap<>(); @@ -43,6 +46,12 @@ public class TestRequest extends ValidatingRequest { return params.get(key); } + @Override + protected List readMultiParam(String key) { + String value = params.get(key); + return value == null ? emptyList() : singletonList(value); + } + @Override protected InputStream readInputStreamParam(String key) { String value = readParam(key); 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 0d4036364de..4f826bebdb0 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,9 +19,6 @@ */ 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; @@ -31,6 +28,7 @@ import java.net.HttpURLConnection; import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Collection; +import java.util.List; import java.util.Map; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -45,6 +43,11 @@ import org.sonar.server.ws.WsTester.TestResponse.TestStream; import org.sonar.test.JsonAssert; import org.sonarqube.ws.MediaTypes; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.server.ws.RequestVerifier.verifyRequest; + /** * @since 4.2 */ @@ -104,6 +107,12 @@ public class WsTester { return params.get(key); } + @Override + protected List readMultiParam(String key) { + String value = params.get(key); + return value == null ? emptyList() : singletonList(value); + } + @Override protected InputStream readInputStreamParam(String key) { String param = readParam(key); 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 48d9f427b2e..bf32ff65ad8 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 @@ -19,8 +19,6 @@ */ package org.sonar.api.server.ws; -import static com.google.common.base.Preconditions.checkArgument; - import com.google.common.annotations.Beta; import com.google.common.base.Splitter; import com.google.common.collect.Lists; @@ -33,6 +31,8 @@ import org.apache.commons.lang.StringUtils; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.SonarException; +import static com.google.common.base.Preconditions.checkArgument; + /** * @since 4.2 */ @@ -109,6 +109,15 @@ public abstract class Request { return values; } + public List mandatoryMultiParam(String key) { + List values = multiParam(key); + if (values.isEmpty()) { + throw new IllegalArgumentException(String.format("The '%s' parameter is missing", key)); + } + + return values; + } + @CheckForNull public List paramAsStrings(String key) { String value = param(key); @@ -121,6 +130,8 @@ public abstract class Request { @CheckForNull public abstract String param(String key); + public abstract List multiParam(String key); + @CheckForNull public abstract InputStream paramAsInputStream(String key); 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 9b2348f48d2..e207c01cedb 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,16 +19,19 @@ */ 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.List; import java.util.Map; import javax.annotation.Nullable; 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; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; + /** * Fake implementation of {@link org.sonar.api.server.ws.Request} used * for testing. Call the method {@link #setParam(String, String)} to @@ -67,6 +70,12 @@ public class SimpleGetRequest extends Request { return params.get(key); } + @Override + public List multiParam(String key) { + String value = params.get(key); + return value == null ? emptyList() : singletonList(value); + } + @Override public InputStream paramAsInputStream(String key) { return IOUtils.toInputStream(param(key)); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java index fd1c2e2270d..389c6e62d3a 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java @@ -19,8 +19,6 @@ */ package org.sonar.api.server.ws.internal; -import static com.google.common.base.Preconditions.checkNotNull; - import com.google.common.base.CharMatcher; import com.google.common.base.Splitter; import com.google.common.collect.Lists; @@ -36,6 +34,10 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.log.Loggers; +import static com.google.common.base.Preconditions.checkNotNull; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; + /** * @since 4.2 */ @@ -68,6 +70,16 @@ public abstract class ValidatingRequest extends Request { return param(key, true); } + @Override + public List multiParam(String key) { + WebService.Param definition = action.param(key); + List values = readMultiParamOrDefaultValue(key, definition); + + values.forEach(value -> validate(value, definition)); + + return values; + } + @Override @CheckForNull public InputStream paramAsInputStream(String key) { @@ -139,9 +151,33 @@ public abstract class ValidatingRequest extends Request { return value; } + private List readMultiParamOrDefaultValue(String key, @Nullable WebService.Param definition) { + if (definition == null) { + String message = String.format("BUG - parameter '%s' is undefined for action '%s'", key, action.key()); + Loggers.get(getClass()).error(message); + throw new IllegalArgumentException(message); + } + + List keyValues = readMultiParam(key); + if (!keyValues.isEmpty()) { + return keyValues; + } + + String deprecatedKey = definition.deprecatedKey(); + List deprecatedKeyValues = deprecatedKey == null ? emptyList() : readMultiParam(deprecatedKey); + if (!deprecatedKeyValues.isEmpty()) { + return deprecatedKeyValues; + } + + String defaultValue = definition.defaultValue(); + return defaultValue == null ? emptyList() : singletonList(defaultValue); + } + @CheckForNull protected abstract String readParam(String key); + protected abstract List readMultiParam(String key); + @CheckForNull protected abstract InputStream readInputStreamParam(String key); 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 8f6613176c5..d5580b1681a 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,11 +19,11 @@ */ 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 com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; import java.io.InputStream; +import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.annotation.Nullable; import org.apache.commons.io.IOUtils; @@ -36,6 +36,10 @@ import org.sonar.api.server.ws.internal.PartImpl; import org.sonar.api.server.ws.internal.ValidatingRequest; import org.sonar.api.utils.DateUtils; +import static com.google.common.collect.Lists.newArrayList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + public class RequestTest { @Rule @@ -96,6 +100,23 @@ public class RequestTest { underTest.mandatoryParamAsStrings("a_required_string"); } + @Test + public void mandatory_multi_param() { + underTest.setMultiParam("a_required_multi_param", newArrayList("firstValue", "secondValue", "thirdValue")); + + List result = underTest.mandatoryMultiParam("a_required_multi_param"); + + assertThat(result).containsExactly("firstValue", "secondValue", "thirdValue"); + } + + @Test + public void fail_when_no_multi_param() { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("The 'a_required_multi_param' parameter is missing"); + + underTest.mandatoryMultiParam("a_required_multi_param"); + } + @Test public void default_value_of_optional_param() { assertThat(underTest.param("has_default_string")).isEqualTo("the_default_string"); @@ -282,8 +303,9 @@ public class RequestTest { private static class FakeRequest extends ValidatingRequest { - private final Map params = Maps.newHashMap(); - private final Map parts = Maps.newHashMap(); + private final ListMultimap multiParams = ArrayListMultimap.create(); + private final Map params = new HashMap<>(); + private final Map parts = new HashMap<>(); @Override public String method() { @@ -312,11 +334,22 @@ public class RequestTest { return this; } + public FakeRequest setMultiParam(String key, List values) { + multiParams.putAll(key, values); + + return this; + } + @Override protected String readParam(String key) { return params.get(key); } + @Override + protected List readMultiParam(String key) { + return multiParams.get(key); + } + @Override protected InputStream readInputStreamParam(String key) { String param = readParam(key); @@ -360,6 +393,7 @@ public class RequestTest { action.createParam("a_required_boolean").setRequired(true); action.createParam("a_required_number").setRequired(true); action.createParam("a_required_enum").setRequired(true); + action.createParam("a_required_multi_param").setRequired(true); action.createParam("has_default_string").setDefaultValue("the_default_string"); action.createParam("has_default_number").setDefaultValue("10"); -- 2.39.5