diff options
7 files changed, 117 insertions, 20 deletions
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java index 774a7e3ffcf..bf2ce289c4c 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/setting/ws/SettingValidations.java @@ -20,8 +20,12 @@ 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"); + } + } + } } } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/ListDefinitionsActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/ListDefinitionsActionTest.java index c5c287b4079..b1009264049 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/ListDefinitionsActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/ListDefinitionsActionTest.java @@ -192,6 +192,21 @@ public class ListDefinitionsActionTest { } @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(); propertyDefinitions.addComponent(PropertyDefinition diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/SetActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/SetActionTest.java index f2004c3bfbc..7f9e3adbb14 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/SetActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/setting/ws/SetActionTest.java @@ -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; @@ -389,6 +390,46 @@ public class SetActionTest { } @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±∞…"); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/PropertyType.java b/sonar-plugin-api/src/main/java/org/sonar/api/PropertyType.java index d9df6c4cd6d..604165fc5d4 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/PropertyType.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/PropertyType.java @@ -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 } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java b/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java index d6fac0aaf37..da3f1cc1c8c 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/config/PropertyDefinition.java @@ -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; } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java index 5333d694d65..61aa1a20882 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/config/PropertyDefinitionTest.java @@ -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() { @@ -305,35 +304,51 @@ public class PropertyDefinitionTest { } @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"); diff --git a/sonar-ws/src/main/protobuf/ws-settings.proto b/sonar-ws/src/main/protobuf/ws-settings.proto index db185b54446..69e8f6d93d9 100644 --- a/sonar-ws/src/main/protobuf/ws-settings.proto +++ b/sonar-ws/src/main/protobuf/ws-settings.proto @@ -81,6 +81,7 @@ enum Type { SINGLE_SELECT_LIST = 11; PROPERTY_SET = 12; LICENSE = 13; + JSON = 14; } // Response of GET api/settings/values |