SONAR-8004 WS settings/set with a proper WS client

SonarSource · Sep 1, 2016 · 3373145 · 3373145
1 parent a9dcddb
commit 3373145
Show file tree

Hide file tree

Showing 14 changed files with 209 additions and 50 deletions.
diff --git a/it/it-tests/src/test/java/it/settings/PropertySetsTest.java b/it/it-tests/src/test/java/it/settings/PropertySetsTest.java
@@ -28,13 +28,14 @@
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Test;
-import org.sonar.wsclient.services.PropertyUpdateQuery;
 import org.sonarqube.ws.Settings;
 import org.sonarqube.ws.client.setting.ResetRequest;
+import org.sonarqube.ws.client.setting.SetRequest;
 import org.sonarqube.ws.client.setting.SettingsService;
 import org.sonarqube.ws.client.setting.ValuesRequest;
 import util.selenium.SeleneseTest;
 
+import static com.google.common.collect.Lists.newArrayList;
 import static java.util.Arrays.asList;
 import static java.util.Arrays.stream;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -94,13 +95,12 @@ public void support_property_sets_with_auto_generated_keys() {
 
   @Test
   public void edit_property_set() {
-    setProperty("sonar.test.jira.servers", "jira1,jira2");
-    setProperty("sonar.test.jira.servers.jira1.key", "jira1");
-    setProperty("sonar.test.jira.servers.jira1.url", "http://jira1");
-    setProperty("sonar.test.jira.servers.jira1.port", "12345");
-    setProperty("sonar.test.jira.servers.jira2.key", "jira2");
-    setProperty("sonar.test.jira.servers.jira2.url", "http://jira2");
-    setProperty("sonar.test.jira.servers.jira2.port", "54321");
+    SETTINGS.set(SetRequest.builder()
+      .setKey("sonar.test.jira.servers")
+      .setFieldValues(newArrayList(
+        "{\"key\":\"jira1\", \"url\":\"http://jira1\", \"port\":\"12345\"}",
+        "{\"key\":\"jira2\", \"url\":\"http://jira2\", \"port\":\"54321\"}"))
+      .build());
 
     assertPropertySet("sonar.test.jira.servers",
       asList(entry("key", "jira1"), entry("url", "http://jira1"), entry("port", "12345")),
@@ -109,9 +109,10 @@ public void edit_property_set() {
 
   @Test
   public void delete_property_set() throws Exception {
-    setProperty("sonar.test.jira.servers", "jira1");
-    setProperty("sonar.test.jira.servers.jira1.url", "http://jira1");
-    setProperty("sonar.test.jira.servers.jira1.port", "12345");
+    SETTINGS.set(SetRequest.builder()
+      .setKey("sonar.test.jira.servers")
+      .setFieldValues(newArrayList("{\"url\":\"http://jira1\"}", "{\"port\":\"12345\"}"))
+      .build());
 
     resetSetting("sonar.test.jira.servers");
 
@@ -135,14 +136,6 @@ private Settings.Setting getSetting(String key) {
     return settings.get(0);
   }
 
-  /**
-   * @deprecated Replace with api/settings/set WS when setting property set will be possible in the WS
-   */
-  @Deprecated
-  static void setProperty(String key, String value) {
-    orchestrator.getServer().getAdminWsClient().update(new PropertyUpdateQuery(key, value));
-  }
-
   static void resetSetting(String... keys) {
     stream(keys).forEach(key -> SETTINGS.reset(ResetRequest.builder().setKey(key).build()));
   }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java
@@ -178,7 +178,7 @@ private void commonChecks(SetRequest request, @Nullable PropertyDefinition defin
   }
 
   private String doHandlePropertySet(DbSession dbSession, SetRequest request, @Nullable PropertyDefinition definition, Optional<ComponentDto> component) {
-    validatePropertySet(request, definition, component);
+    validatePropertySet(request, definition);
 
     int[] fieldIds = IntStream.rangeClosed(1, request.getFieldValues().size()).toArray();
     String inlinedFieldKeys = IntStream.of(fieldIds).mapToObj(String::valueOf).collect(COMMA_JOINER);
@@ -188,9 +188,10 @@ private String doHandlePropertySet(DbSession dbSession, SetRequest request, @Nul
     deleteSettings(dbSession, component, key);
     dbClient.propertiesDao().insertProperty(dbSession, new PropertyDto().setKey(key).setValue(inlinedFieldKeys).setResourceId(componentId));
 
-    List<Map<String, String>> fieldValues = request.getFieldValues();
+    List<String> fieldValues = request.getFieldValues();
     IntStream.of(fieldIds).boxed()
-      .flatMap(i -> fieldValues.get(i - 1).entrySet().stream().map(entry -> new KeyValue(key + "." + i + "." + entry.getKey(), entry.getValue())))
+      .flatMap(i -> readOneFieldValues(fieldValues.get(i - 1), request.getKey()).entrySet().stream()
+        .map(entry -> new KeyValue(key + "." + i + "." + entry.getKey(), entry.getValue())))
       .forEach(keyValue -> dbClient.propertiesDao().insertProperty(dbSession, toFieldProperty(keyValue, componentId)));
 
     return inlinedFieldKeys;
@@ -204,14 +205,15 @@ private void deleteSettings(DbSession dbSession, Optional<ComponentDto> componen
     }
   }
 
-  private void validatePropertySet(SetRequest request, @Nullable PropertyDefinition definition, Optional<ComponentDto> component) {
+  private void validatePropertySet(SetRequest request, @Nullable PropertyDefinition definition) {
     checkRequest(definition != null, "Setting '%s' is undefined", request.getKey());
     checkRequest(PropertyType.PROPERTY_SET.equals(definition.type()), "Parameter '%s' is used for setting of property set type only", PARAM_FIELD_VALUES);
 
     Set<String> fieldKeys = definition.fields().stream().map(PropertyFieldDefinition::key).collect(Collectors.toSet());
     ListMultimap<String, String> valuesByFieldKeys = ArrayListMultimap.create(fieldKeys.size(), request.getFieldValues().size() * fieldKeys.size());
 
     request.getFieldValues().stream()
+      .map(oneFieldValues -> readOneFieldValues(oneFieldValues, request.getKey()))
       .flatMap(map -> map.entrySet().stream())
       .peek(entry -> valuesByFieldKeys.put(entry.getKey(), entry.getValue()))
       .forEach(entry -> checkRequest(fieldKeys.contains(entry.getKey()), "Unknown field key '%s' for setting '%s'", entry.getKey(), request.getKey()));
@@ -305,20 +307,12 @@ private static SetRequest toWsRequest(Request request) {
       .setKey(request.mandatoryParam(PARAM_KEY))
       .setValue(request.param(PARAM_VALUE))
       .setValues(request.multiParam(PARAM_VALUES))
-      .setFieldValues(readFieldValues(request))
+      .setFieldValues(request.multiParam(PARAM_FIELD_VALUES))
       .setComponentId(request.param(PARAM_COMPONENT_ID))
       .setComponentKey(request.param(PARAM_COMPONENT_KEY))
       .build();
   }
 
-  private static List<Map<String, String>> readFieldValues(Request request) {
-    String key = request.mandatoryParam(PARAM_KEY);
-
-    return request.multiParam(PARAM_FIELD_VALUES).stream()
-      .map(json -> readOneFieldValues(json, key))
-      .collect(Collectors.toList());
-  }
-
   private static Map<String, String> readOneFieldValues(String json, String key) {
     Type type = new TypeToken<Map<String, String>>() {
     }.getType();

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
@@ -41,7 +41,7 @@ protected String readParam(String key) {
 
   @Override
   protected List<String> readMultiParam(String key) {
-    throw new UnsupportedOperationException("reading multi value param is not supported yet by local WS calls");
+    return localRequest.getMultiParam(key);
   }
 
   @Override

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/LocalConnector.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/LocalConnector.java
@@ -21,6 +21,7 @@
 
 import com.google.common.annotations.Beta;
 import java.util.Collection;
+import java.util.List;
 import javax.annotation.CheckForNull;
 
 /**
@@ -86,6 +87,11 @@ interface LocalRequest {
      */
     @CheckForNull
     String getParam(String key);
+
+    /**
+     * @see Request#multiParam(String)
+     */
+    List<String> getMultiParam(String key);
   }
 
   interface LocalResponse {

diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/BaseRequest.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/BaseRequest.java
@@ -19,13 +19,22 @@
  */
 package org.sonarqube.ws.client;
 
+import com.google.common.collect.LinkedListMultimap;
+import com.google.common.collect.ListMultimap;
+import java.util.Collection;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.sonarqube.ws.MediaTypes;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.util.Collections.singletonList;
 import static java.util.Objects.requireNonNull;
 
 abstract class BaseRequest<SELF extends BaseRequest> implements WsRequest {
@@ -34,8 +43,7 @@ abstract class BaseRequest<SELF extends BaseRequest> implements WsRequest {
 
   private String mediaType = MediaTypes.JSON;
 
-  // keep the same order -> do not use HashMap
-  private final Map<String, String> params = new LinkedHashMap<>();
+  private final DefaultParameters parameters = new DefaultParameters();
 
   BaseRequest(String path) {
     this.path = path;
@@ -60,16 +68,79 @@ public SELF setMediaType(String s) {
     return (SELF) this;
   }
 
+  /**
+   * To set a multi value parameters, provide a Collection with the values
+   */
   public SELF setParam(String key, @Nullable Object value) {
     checkArgument(!isNullOrEmpty(key), "a WS parameter key cannot be null");
-    if (value != null) {
-      this.params.put(key, value.toString());
+    if (value == null) {
+      return (SELF) this;
+    }
+
+    if (value instanceof Collection) {
+      Collection<Object> values = (Collection<Object>) value;
+      if (values.isEmpty()) {
+        return (SELF) this;
+      }
+      parameters.setValues(key, values.stream().map(Object::toString).collect(Collectors.toList()));
+    } else {
+      parameters.setValue(key, value.toString());
     }
     return (SELF) this;
   }
 
   @Override
   public Map<String, String> getParams() {
-    return params;
+    return parameters.keyValues.keySet().stream()
+      .collect(Collectors.toMap(
+        Function.identity(),
+        key -> parameters.keyValues.get(key).get(0),
+        (v1, v2) -> {
+          throw new IllegalStateException(String.format("Duplicate key '%s' in request", v1));
+        },
+        LinkedHashMap::new));
+  }
+
+  @Override
+  public Parameters getParameters() {
+    return parameters;
+  }
+
+  private static class DefaultParameters implements Parameters {
+    // preserve insertion order
+    private final ListMultimap<String, String> keyValues = LinkedListMultimap.create();
+
+    @Override
+    @CheckForNull
+    public String getValue(String key) {
+      return keyValues.containsKey(key) ? keyValues.get(key).get(0) : null;
+    }
+
+    @Override
+    public List<String> getValues(String key) {
+      return keyValues.get(key);
+    }
+
+    @Override
+    public Set<String> getKeys() {
+      return keyValues.keySet();
+    }
+
+    private DefaultParameters setValue(String key, String value) {
+      checkArgument(!isNullOrEmpty(key));
+      checkArgument(!isNullOrEmpty(value));
+
+      keyValues.putAll(key, singletonList(value));
+      return this;
+    }
+
+    private DefaultParameters setValues(String key, Collection<String> values) {
+      checkArgument(!isNullOrEmpty(key));
+      checkArgument(values != null && !values.isEmpty());
+
+      this.keyValues.putAll(key, values.stream().map(Object::toString).collect(Collectors.toList()));
+
+      return this;
+    }
   }
 }
diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java
@@ -162,9 +162,10 @@ private HttpUrl.Builder prepareUrlBuilder(WsRequest wsRequest) {
     HttpUrl.Builder urlBuilder = baseUrl
       .resolve(path.startsWith("/") ? path.replaceAll("^(/)+", "") : path)
       .newBuilder();
-    for (Map.Entry<String, String> param : wsRequest.getParams().entrySet()) {
-      urlBuilder.addQueryParameter(param.getKey(), param.getValue());
-    }
+    wsRequest.getParameters().getKeys()
+      .forEach(key -> wsRequest.getParameters().getValues(key)
+        .forEach(value -> urlBuilder.addQueryParameter(key, value)));
+
     return urlBuilder;
   }
 

diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/LocalWsConnector.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/LocalWsConnector.java
@@ -24,6 +24,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
+import java.util.List;
 import org.sonar.api.server.ws.LocalConnector;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -55,9 +56,11 @@ public WsResponse call(WsRequest wsRequest) {
 
   private static class DefaultLocalRequest implements LocalConnector.LocalRequest {
     private final WsRequest wsRequest;
+    private final Parameters parameters;
 
     public DefaultLocalRequest(WsRequest wsRequest) {
       this.wsRequest = wsRequest;
+      this.parameters = wsRequest.getParameters();
     }
 
     @Override
@@ -77,12 +80,17 @@ public String getMethod() {
 
     @Override
     public boolean hasParam(String key) {
-      return wsRequest.getParams().containsKey(key);
+      return !parameters.getValues(key).isEmpty();
     }
 
     @Override
     public String getParam(String key) {
-      return wsRequest.getParams().get(key);
+      return parameters.getValue(key);
+    }
+
+    @Override
+    public List<String> getMultiParam(String key) {
+      return parameters.getValues(key);
     }
   }
 

diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/Parameters.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/Parameters.java
@@ -0,0 +1,37 @@
+/*
+ * 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.sonarqube.ws.client;
+
+import java.util.List;
+import java.util.Set;
+import javax.annotation.CheckForNull;
+
+public interface Parameters {
+  /**
+   * In the case of a multi value parameter, returns the first element
+   */
+  @CheckForNull
+  String getValue(String key);
+
+  List<String> getValues(String key);
+
+  Set<String> getKeys();
+}
diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/WsRequest.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/WsRequest.java
@@ -32,8 +32,17 @@ public interface WsRequest {
 
   String getMediaType();
 
+  /**
+   *
+   * In case of multi value parameters, returns the first value
+   *
+   * @deprecated since 6.1. Use {@link #getParameters()} instead
+   */
+  @Deprecated
   Map<String, String> getParams();
 
+  Parameters getParameters();
+
   enum Method {
     GET, POST
   }