]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-14498 Support JSON property type
authorJacek <jacek.poreda@sonarsource.com>
Fri, 19 Feb 2021 15:20:34 +0000 (16:20 +0100)
committersonartech <sonartech@sonarsource.com>
Fri, 26 Feb 2021 20:07:39 +0000 (20:07 +0000)
server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/ListDefinitionsActionTest.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/SetActionTest.java
sonar-plugin-api/src/main/java/org/sonar/api/PropertyType.java
sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java
sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java
sonar-ws/src/main/protobuf/ws-settings.proto

index 774a7e3ffcf20eb1b139efa50b252d2f407712b9..bf2ce289c4caff29a0fd055b13144320d29a5044 100644 (file)
 package org.sonar.server.setting.ws;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.gson.Gson;
+import com.google.gson.JsonElement;
+import java.io.IOException;
 import java.util.List;
 import java.util.Locale;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
@@ -121,6 +125,8 @@ public class SettingValidations {
         validateMetric(data);
       } else if (definition.type() == PropertyType.USER_LOGIN) {
         validateLogin(data);
+      } else if (definition.type() == PropertyType.JSON) {
+        validateJson(data);
       } else {
         validateOtherTypes(data, definition);
       }
@@ -152,5 +158,16 @@ public class SettingValidations {
           data.key, data.values.stream().collect(Collectors.joining(", ")));
       }
     }
+
+    private void validateJson(SettingData data) {
+      Optional<String> jsonContent = data.values.stream().findFirst();
+      if (jsonContent.isPresent()) {
+        try {
+          new Gson().getAdapter(JsonElement.class).fromJson(jsonContent.get());
+        } catch (IOException e) {
+          throw new IllegalArgumentException("Provided JSON is invalid");
+        }
+      }
+    }
   }
 }
index c5c287b407906f681504fc92e3b1366d7d58fb25..b1009264049c125a9115840348e87fa29771f775 100644 (file)
@@ -191,6 +191,21 @@ public class ListDefinitionsActionTest {
     assertThat(definition.getOptionsList()).containsExactly("one", "two");
   }
 
+  @Test
+  public void return_JSON_property() {
+    logIn();
+    propertyDefinitions.addComponent(PropertyDefinition
+        .builder("foo")
+        .type(PropertyType.JSON)
+        .build());
+
+    ListDefinitionsWsResponse result = executeRequest();
+
+    assertThat(result.getDefinitionsList()).hasSize(1);
+    Definition definition = result.getDefinitions(0);
+    assertThat(definition.getType()).isEqualTo(Settings.Type.JSON);
+  }
+
   @Test
   public void return_property_set() {
     logIn();
index f2004c3bfbc9492635f896216e6acad518258661..7f9e3adbb14a7530e45ba611b32283e2d207f0cd 100644 (file)
@@ -64,6 +64,7 @@ import static com.google.common.collect.Lists.newArrayList;
 import static java.lang.String.format;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.groups.Tuple.tuple;
 import static org.sonar.db.component.ComponentTesting.newView;
 import static org.sonar.db.metric.MetricTesting.newMetricDto;
@@ -388,6 +389,46 @@ public class SetActionTest {
     assertGlobalSetting("my.key", "My Value");
   }
 
+  @Test
+  public void persist_JSON_property() {
+    definitions.addComponent(PropertyDefinition
+      .builder("my.key")
+      .name("foo")
+      .description("desc")
+      .category("cat")
+      .subCategory("subCat")
+      .type(PropertyType.JSON)
+      .build());
+
+    callForGlobalSetting("my.key", "{\"test\":\"value\"}");
+
+    assertGlobalSetting("my.key", "{\"test\":\"value\"}");
+  }
+
+  @Test
+  public void fail_if_JSON_invalid_for_JSON_property() {
+    definitions.addComponent(PropertyDefinition
+      .builder("my.key")
+      .name("foo")
+      .description("desc")
+      .category("cat")
+      .subCategory("subCat")
+      .type(PropertyType.JSON)
+      .build());
+
+    assertThatThrownBy(() -> callForGlobalSetting("my.key", "{\"test\":\"value\""))
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Provided JSON is invalid");
+
+    assertThatThrownBy(() -> callForGlobalSetting("my.key", "{\"test\":\"value\",}"))
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Provided JSON is invalid");
+
+    assertThatThrownBy(() -> callForGlobalSetting("my.key", "{\"test\":[\"value\",]}"))
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Provided JSON is invalid");
+  }
+
   @Test
   public void persist_global_setting_with_non_ascii_characters() {
     callForGlobalSetting("my.key", "fi±∞…");
index d9df6c4cd6d7fbb6044332c8fa93fbbef8d4f287..604165fc5d4d092227bb71404e56a2395b0ba079 100644 (file)
@@ -107,5 +107,11 @@ public enum PropertyType {
    * @deprecated since 6.3, this type is useless as Dashboards have been removed
    */
   @Deprecated
-  LONG
+  LONG,
+
+  /**
+   * JSON property type
+   * @since 8.8
+   */
+  JSON
 }
index d6fac0aaf3794de2eaab1ddb64991170c82bdbcc..da3f1cc1c8c8e0e2bd5bf188181d50ae1a602517 100644 (file)
@@ -50,6 +50,7 @@ import static org.apache.commons.lang.StringUtils.isEmpty;
 import static org.sonar.api.PropertyType.BOOLEAN;
 import static org.sonar.api.PropertyType.FLOAT;
 import static org.sonar.api.PropertyType.INTEGER;
+import static org.sonar.api.PropertyType.JSON;
 import static org.sonar.api.PropertyType.LONG;
 import static org.sonar.api.PropertyType.PROPERTY_SET;
 import static org.sonar.api.PropertyType.REGULAR_EXPRESSION;
@@ -599,6 +600,7 @@ public final class PropertyDefinition {
       fixType(key, type);
       checkArgument(onQualifiers.isEmpty() || onlyOnQualifiers.isEmpty(), "Cannot define both onQualifiers and onlyOnQualifiers");
       checkArgument(!hidden || (onQualifiers.isEmpty() && onlyOnQualifiers.isEmpty()), "Cannot be hidden and defining qualifiers on which to display");
+      checkArgument(!JSON.equals(type) || !multiValues, "Multivalues are not allowed to be defined for JSON-type property.");
       if (hidden) {
         global = false;
       }
index 5333d694d658b6972025249eb65cde7e544fa95f..61aa1a20882ba21354a94550a6a86d3f5af5f701 100644 (file)
@@ -25,23 +25,22 @@ import java.util.Collections;
 import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 import org.sonar.api.Properties;
 import org.sonar.api.Property;
 import org.sonar.api.PropertyField;
 import org.sonar.api.PropertyType;
+import org.sonar.api.config.PropertyDefinition.Builder;
 import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.utils.AnnotationUtils;
 
 import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.fail;
 
 public class PropertyDefinitionTest {
-  @Rule
-  public ExpectedException thrown = ExpectedException.none();
 
   @Test
   public void should_override_toString() {
@@ -304,36 +303,52 @@ public class PropertyDefinitionTest {
     assertThat(def.type()).isEqualTo(PropertyType.LICENSE);
   }
 
+  @Test
+  public void should_create_json_property_type() {
+    Builder builder = PropertyDefinition.builder("json-prop").type(PropertyType.JSON).multiValues(false);
+    assertThatCode(builder::build)
+      .doesNotThrowAnyException();
+  }
+
   @Test
   public void should_not_authorise_empty_key() {
-    thrown.expect(IllegalArgumentException.class);
-    thrown.expectMessage("Key must be set");
+    Builder builder = PropertyDefinition.builder(null);
+    assertThatThrownBy(builder::build)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Key must be set");
+  }
 
-    PropertyDefinition.builder(null).build();
+  @Test
+  public void should_not_create_json_multivalue() {
+    Builder builder = PropertyDefinition.builder("json-prop").type(PropertyType.JSON).multiValues(true);
+    assertThatThrownBy(builder::build)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Multivalues are not allowed to be defined for JSON-type property.");
   }
 
   @Test
   public void should_not_authorize_defining_on_qualifiers_and_hidden() {
-    thrown.expect(IllegalArgumentException.class);
-    thrown.expectMessage("Cannot be hidden and defining qualifiers on which to display");
-
-    PropertyDefinition.builder("foo").name("foo").onQualifiers(Qualifiers.PROJECT).hidden().build();
+    Builder builder = PropertyDefinition.builder("foo").name("foo").onQualifiers(Qualifiers.PROJECT).hidden();
+    assertThatThrownBy(builder::build)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Cannot be hidden and defining qualifiers on which to display");
   }
 
   @Test
   public void should_not_authorize_defining_ony_on_qualifiers_and_hidden() {
-    thrown.expect(IllegalArgumentException.class);
-    thrown.expectMessage("Cannot be hidden and defining qualifiers on which to display");
-
-    PropertyDefinition.builder("foo").name("foo").onlyOnQualifiers(Qualifiers.PROJECT).hidden().build();
+    Builder builder = PropertyDefinition.builder("foo").name("foo").onlyOnQualifiers(Qualifiers.PROJECT).hidden();
+    assertThatThrownBy(builder::build)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Cannot be hidden and defining qualifiers on which to display");
   }
 
   @Test
   public void should_not_authorize_defining_on_qualifiers_and_only_on_qualifiers() {
-    thrown.expect(IllegalArgumentException.class);
-    thrown.expectMessage("Cannot define both onQualifiers and onlyOnQualifiers");
-
-    PropertyDefinition.builder("foo").name("foo").onQualifiers(Qualifiers.MODULE).onlyOnQualifiers(Qualifiers.PROJECT).build();
+    Builder builder = PropertyDefinition.builder("foo").name("foo").onQualifiers(Qualifiers.MODULE)
+      .onlyOnQualifiers(Qualifiers.PROJECT);
+    assertThatThrownBy(builder::build)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Cannot define both onQualifiers and onlyOnQualifiers");
   }
 
   private static final Set<String> ALLOWED_QUALIFIERS = ImmutableSet.of("TRK", "VW", "BRC", "SVW");
index db185b54446b29c06e47bbb62e95847135e17853..69e8f6d93d937ac93d711d30e2dfbc4bf18ec8ab 100644 (file)
@@ -81,6 +81,7 @@ enum Type {
   SINGLE_SELECT_LIST = 11;
   PROPERTY_SET = 12;
   LICENSE = 13;
+  JSON = 14;
 }
 
 // Response of GET api/settings/values