]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-7300 Handle multi values in PUT|POST api/properties
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Wed, 11 Jan 2017 16:42:26 +0000 (17:42 +0100)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 12 Jan 2017 13:53:56 +0000 (14:53 +0100)
it/it-tests/src/test/java/it/settings/DeprecatedPropertiesWsTest.java
server/sonar-server/src/main/java/org/sonar/server/property/ws/IndexAction.java
server/sonar-server/src/main/java/org/sonar/server/ws/DeprecatedRestWebServiceFilter.java
server/sonar-server/src/test/java/org/sonar/server/ws/DeprecatedRestWebServiceFilterTest.java

index 80deea8e407a8792859a4af978c762ba0e2ea648..a6d02dbf2326d4f064c4df95c330c8bf9c4e599a 100644 (file)
@@ -102,7 +102,7 @@ public class DeprecatedPropertiesWsTest {
   }
 
   private static void doResetSettings() {
-    resetSettings(orchestrator, null, "some-property", "custom-property", "int", "multi", "boolean", "hidden", "not_defined", "setting.secured", "setting.license.secured", "list");
+    resetSettings(orchestrator, null, "some-property", "custom-property", "int", "multi", "boolean", "hidden", "not_defined", "setting.secured", "setting.license.secured", "list", "undefined");
     resetSettings(orchestrator, PROJECT_KEY, PROJECT_SETTING_KEY, "sonar.coverage.exclusions", "project.setting");
   }
 
@@ -120,11 +120,11 @@ public class DeprecatedPropertiesWsTest {
 
   @Test
   public void get_multi_values() throws Exception {
-    setProperty("multi", asList("value1", "value2"), null);
+    setProperty("multi", asList("value1", "value2", "value,3"), null);
 
     Properties.Property setting = getProperty("multi", null);
-    assertThat(setting.getValue()).isEqualTo("value1,value2");
-    assertThat(setting.getValues()).containsOnly("value1", "value2");
+    assertThat(setting.getValue()).isEqualTo("value1,value2,value%2C3");
+    assertThat(setting.getValues()).containsOnly("value1", "value2", "value,3");
   }
 
   @Test
@@ -235,6 +235,22 @@ public class DeprecatedPropertiesWsTest {
     assertThat(getProperty("project.setting", PROJECT_KEY).getValue()).isEqualTo("some-value");
   }
 
+  @Test
+  public void put_property_for_undefined_setting() throws Exception {
+    putProperty("undefined", "some-value", null);
+
+    assertThat(getProperty("undefined", null).getValue()).isEqualTo("some-value");
+  }
+
+  @Test
+  public void put_property_multi_values() throws Exception {
+    putProperty("multi", "value1,value2,value3", null);
+
+    Properties.Property setting = getProperty("multi", null);
+    assertThat(setting.getValue()).isEqualTo("value1,value2,value3");
+    assertThat(setting.getValues()).containsOnly("value1", "value2", "value3");
+  }
+
   @Test
   public void delete_property() throws Exception {
     setProperty("custom-property", "value", null);
index 4d171d7244640500aeaffa7324b04317a205a73a..d4626462d925e22957b06a98cd07b6058c46bb41 100644 (file)
@@ -64,7 +64,7 @@ public class IndexAction implements WsAction {
   private static final String COMMA_ENCODED_VALUE = "%2C";
 
   public static final String PARAM_KEY = "key";
-  private static final String PARAM_COMPONENT = "resource";
+  public static final String PARAM_COMPONENT = "resource";
 
   private final DbClient dbClient;
   private final ComponentFinder componentFinder;
index 10a43508b102ba6f6d36298fa9bbb00241e99ce1..d4e106f1f9708aa27d804478eeab7f573420459f 100644 (file)
 
 package org.sonar.server.ws;
 
+import com.google.common.base.Splitter;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Multimap;
 import java.io.IOException;
 import java.io.StringWriter;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import javax.servlet.FilterChain;
@@ -33,9 +38,11 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.apache.commons.io.IOUtils;
 import org.sonar.api.web.ServletFilter;
+import org.sonar.server.property.ws.IndexAction;
 
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.sonar.api.server.ws.RailsHandler.PARAM_FORMAT;
 import static org.sonar.server.property.ws.PropertiesWs.CONTROLLER_PROPERTIES;
 import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_RESET;
 import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_SET;
@@ -44,12 +51,15 @@ import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONE
 import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEY;
 import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEYS;
 import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_VALUE;
+import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_VALUES;
 
 /**
  * This filter is used to execute some deprecated Java WS, that were using REST
  */
 public class DeprecatedRestWebServiceFilter extends ServletFilter {
 
+  private static final Splitter VALUE_SPLITTER = Splitter.on(",").omitEmptyStrings();
+
   private final WebServiceEngine webServiceEngine;
 
   public DeprecatedRestWebServiceFilter(WebServiceEngine webServiceEngine) {
@@ -93,21 +103,22 @@ public class DeprecatedRestWebServiceFilter extends ServletFilter {
 
     @Override
     public String getPath() {
-      return restResponse.getRedirectedPath();
+      return restResponse.redirectedPath;
     }
 
     @Override
     public boolean hasParam(String key) {
-      return restResponse.getAdditionalParams().containsKey(key) || super.hasParam(key);
+      return restResponse.additionalParams.containsKey(key) || restResponse.additionalMultiParams.containsKey(key);
     }
 
     @Override
     protected String readParam(String key) {
-      String param = restResponse.getAdditionalParams().get(key);
-      if (param != null) {
-        return param;
-      }
-      return super.readParam(key);
+      return restResponse.additionalParams.get(key);
+    }
+
+    @Override
+    protected List<String> readMultiParam(String key) {
+      return new ArrayList<>(restResponse.additionalMultiParams.get(key));
     }
 
     @Override
@@ -119,6 +130,7 @@ public class DeprecatedRestWebServiceFilter extends ServletFilter {
   private static class Response {
     private final HttpServletRequest request;
     private final Map<String, String> additionalParams = new HashMap<>();
+    private final Multimap<String, String> additionalMultiParams = ArrayListMultimap.create();
     private final String originalPath;
     private String redirectedPath;
     private String redirectedMethod;
@@ -132,31 +144,39 @@ public class DeprecatedRestWebServiceFilter extends ServletFilter {
     void init() {
       String method = request.getMethod();
       Optional<String> key = getKeyOrId();
-      Optional<String> value = getValue();
-      Optional<String> component = getComponent();
       switch (method) {
         case "POST":
         case "PUT":
-          if (value.isPresent()) {
-            addParameterIfPresent(PARAM_KEY, key);
-            addParameterIfPresent(PARAM_VALUE, value);
-            addParameterIfPresent(PARAM_COMPONENT, component);
-            redirectedPath = CONTROLLER_SETTINGS + "/" + ACTION_SET;
-            redirectedMethod = "POST";
-          } else {
-            redirectToReset(key, component);
-          }
+          handlePutAndPost(key, getValues(), getComponent());
           break;
         case "DELETE":
-          redirectToReset(key, component);
+          handleDelete(key, getComponent());
           break;
         default:
-          addParameterIfPresent(PARAM_KEY, key);
-          redirectedPath = CONTROLLER_PROPERTIES + "/index";
-          redirectedMethod = "GET";
+          handleGet(key, getComponent());
       }
     }
 
+    private void handlePutAndPost(Optional<String> key, List<String> values, Optional<String> component) {
+      if (values.isEmpty()) {
+        redirectToReset(key, component);
+      } else {
+        redirectToSet(key, values, component);
+      }
+    }
+
+    private void handleDelete(Optional<String> key, Optional<String> component) {
+      redirectToReset(key, component);
+    }
+
+    private void handleGet(Optional<String> key, Optional<String> component) {
+      addParameterIfPresent(PARAM_KEY, key);
+      addParameterIfPresent(IndexAction.PARAM_COMPONENT, component);
+      addParameterIfPresent(PARAM_FORMAT, readParam(PARAM_FORMAT));
+      redirectedPath = CONTROLLER_PROPERTIES + "/index";
+      redirectedMethod = "GET";
+    }
+
     private Optional<String> getKeyOrId() {
       Optional<String> key = getKey();
       return key.isPresent() ? key : readParam("id");
@@ -171,12 +191,17 @@ public class DeprecatedRestWebServiceFilter extends ServletFilter {
     }
 
     private Optional<String> getComponent() {
-      return readParam("resource");
+      return readParam(IndexAction.PARAM_COMPONENT);
     }
 
-    private Optional<String> getValue() {
+    private List<String> getValues() {
       Optional<String> value = readParam("value");
-      return value.isPresent() ? value : readBody();
+      if (!value.isPresent()) {
+        List<String> values = new ArrayList<>();
+        readBody().ifPresent(values::add);
+        return values;
+      }
+      return VALUE_SPLITTER.splitToList(value.get());
     }
 
     private Optional<String> readParam(String paramKey) {
@@ -198,6 +223,18 @@ public class DeprecatedRestWebServiceFilter extends ServletFilter {
       }
     }
 
+    private void redirectToSet(Optional<String> key, List<String> values, Optional<String> component) {
+      addParameterIfPresent(PARAM_KEY, key);
+      if (values.size() == 1) {
+        additionalParams.put(PARAM_VALUE, values.get(0));
+      } else {
+        additionalMultiParams.putAll(PARAM_VALUES, values);
+      }
+      addParameterIfPresent(PARAM_COMPONENT, component);
+      redirectedPath = CONTROLLER_SETTINGS + "/" + ACTION_SET;
+      redirectedMethod = "POST";
+    }
+
     private void redirectToReset(Optional<String> key, Optional<String> component) {
       addParameterIfPresent(PARAM_KEYS, key);
       addParameterIfPresent(PARAM_COMPONENT, component);
@@ -209,13 +246,6 @@ public class DeprecatedRestWebServiceFilter extends ServletFilter {
       value.ifPresent(s -> additionalParams.put(parameterKey, s));
     }
 
-    String getRedirectedPath() {
-      return redirectedPath;
-    }
-
-    Map<String, String> getAdditionalParams() {
-      return additionalParams;
-    }
   }
 
 }
index 8691e8dad85e3dbc895e990b26494bac88c2f2be..65cbf7f8c780154bbefa2621235688cca5498da7 100644 (file)
@@ -22,6 +22,7 @@ package org.sonar.server.ws;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.util.Arrays;
 import javax.servlet.FilterChain;
 import javax.servlet.ReadListener;
 import javax.servlet.ServletInputStream;
@@ -78,9 +79,8 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/properties/index");
-    assertThat(servletRequestCaptor.getValue().hasParam("key")).isFalse();
+    assertRedirection("api/properties/index", "GET");
+    assertNoParam("key", "component", "value", "values");
   }
 
   @Test
@@ -90,10 +90,8 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/properties/index");
-    assertNoParam("key");
-    assertNoParam("component");
+    assertRedirection("api/properties/index", "GET");
+    assertNoParam("key", "component", "value", "values");
   }
 
   @Test
@@ -103,11 +101,9 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/properties/index");
-    assertThat(servletRequestCaptor.getValue().method()).isEqualTo("GET");
+    assertRedirection("api/properties/index", "GET");
     assertParam("key", "my.property");
-    assertNoParam("component");
+    assertNoParam("component", "value", "values");
   }
 
   @Test
@@ -118,11 +114,9 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/properties/index");
-    assertThat(servletRequestCaptor.getValue().method()).isEqualTo("GET");
+    assertRedirection("api/properties/index", "GET");
     assertParam("key", "my.property");
-    assertNoParam("component");
+    assertNoParam("component", "value", "values");
   }
 
   @Test
@@ -133,11 +127,37 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/properties/index");
-    assertThat(servletRequestCaptor.getValue().method()).isEqualTo("GET");
+    assertRedirection("api/properties/index", "GET");
     assertParam("key", "my.property");
-    assertNoParam("component");
+    assertNoParam("component", "value", "values");
+  }
+
+  @Test
+  public void redirect_api_properties_when_resource() throws Exception {
+    when(request.getRequestURI()).thenReturn("/api/properties/my.property");
+    when(request.getParameter("resource")).thenReturn("my_project");
+    when(request.getMethod()).thenReturn("GET");
+
+    underTest.doFilter(request, response, chain);
+
+    assertRedirection("api/properties/index", "GET");
+    assertParam("key", "my.property");
+    assertParam("resource", "my_project");
+    assertNoParam("component", "value", "values");
+  }
+
+  @Test
+  public void redirect_api_properties_when_format() throws Exception {
+    when(request.getRequestURI()).thenReturn("/api/properties/my.property");
+    when(request.getParameter("format")).thenReturn("json");
+    when(request.getMethod()).thenReturn("GET");
+
+    underTest.doFilter(request, response, chain);
+
+    assertRedirection("api/properties/index", "GET");
+    assertParam("key", "my.property");
+    assertParam("format", "json");
+    assertNoParam("component", "value", "values");
   }
 
   @Test
@@ -149,12 +169,11 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/settings/set");
-    assertThat(servletRequestCaptor.getValue().method()).isEqualTo("POST");
+    assertRedirection("api/settings/set", "POST");
     assertParam("key", "my.property");
     assertParam("value", "my_value");
     assertParam("component", "my_project");
+    assertNoParam("values");
   }
 
   @Test
@@ -166,12 +185,11 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/settings/set");
-    assertThat(servletRequestCaptor.getValue().method()).isEqualTo("POST");
+    assertRedirection("api/settings/set", "POST");
     assertParam("key", "my.property");
     assertParam("value", "my_value");
     assertParam("component", "my_project");
+    assertNoParam("values");
   }
 
   @Test
@@ -182,11 +200,25 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/settings/set");
-    assertThat(servletRequestCaptor.getValue().method()).isEqualTo("POST");
+    assertRedirection("api/settings/set", "POST");
     assertParam("key", "my.property");
     assertParam("value", "my_value");
+    assertNoParam("values", "component");
+  }
+
+  @Test
+  public void redirect_post_api_properties_to_api_settings_set_when_multi_values() throws Exception {
+    when(request.getRequestURI()).thenReturn("/api/properties/my.property");
+    when(request.getParameter("value")).thenReturn("value1,value2,value3");
+    when(request.getMethod()).thenReturn("POST");
+
+    underTest.doFilter(request, response, chain);
+
+    assertRedirection("api/settings/set", "POST");
+    assertParam("key", "my.property");
+    assertNoParam("value");
+    assertThat(servletRequestCaptor.getValue().hasParam("values")).as("Parameter '%s' hasn't been found", "values").isTrue();
+    assertThat(servletRequestCaptor.getValue().readMultiParam("values")).containsOnly("value1", "value2", "value3");
   }
 
   @Test
@@ -197,12 +229,10 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/settings/reset");
-    assertThat(servletRequestCaptor.getValue().method()).isEqualTo("POST");
+    assertRedirection("api/settings/reset", "POST");
     assertParam("keys", "my.property");
-    assertNoParam("value");
     assertParam("component", "my_project");
+    assertNoParam("value", "values");
   }
 
   @Test
@@ -214,12 +244,10 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/settings/reset");
-    assertThat(servletRequestCaptor.getValue().method()).isEqualTo("POST");
+    assertRedirection("api/settings/reset", "POST");
     assertParam("keys", "my.property");
-    assertNoParam("value");
     assertParam("component", "my_project");
+    assertNoParam("value", "values");
   }
 
   @Test
@@ -230,11 +258,16 @@ public class DeprecatedRestWebServiceFilterTest {
 
     underTest.doFilter(request, response, chain);
 
-    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
-    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo("api/settings/reset");
-    assertThat(servletRequestCaptor.getValue().method()).isEqualTo("POST");
+    assertRedirection("api/settings/reset", "POST");
     assertParam("keys", "my.property");
     assertParam("component", "my_project");
+    assertNoParam("value", "values");
+  }
+
+  private void assertRedirection(String path, String method) {
+    verify(webServiceEngine).execute(servletRequestCaptor.capture(), any(ServletResponse.class));
+    assertThat(servletRequestCaptor.getValue().getPath()).isEqualTo(path);
+    assertThat(servletRequestCaptor.getValue().method()).isEqualTo(method);
   }
 
   private void assertParam(String key, String value) {
@@ -242,8 +275,11 @@ public class DeprecatedRestWebServiceFilterTest {
     assertThat(servletRequestCaptor.getValue().readParam(key)).isEqualTo(value);
   }
 
-  private void assertNoParam(String key) {
-    assertThat(servletRequestCaptor.getValue().hasParam(key)).isFalse();
+  private void assertNoParam(String... keys) {
+    Arrays.stream(keys).forEach(key -> {
+      assertThat(servletRequestCaptor.getValue().hasParam(key)).as(key).isFalse();
+      assertThat(servletRequestCaptor.getValue().readParam(key)).as(key).isNull();
+    });
   }
 
   private static class TestInputStream extends ServletInputStream {