diff options
6 files changed, 69 insertions, 32 deletions
diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java index b3f38531604..baea3587c53 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/ResetAction.java @@ -40,6 +40,7 @@ import org.sonar.server.setting.ws.SettingValidations.SettingData; import org.sonar.server.user.UserSession; import org.sonarqube.ws.client.setting.ResetRequest; +import static java.util.Collections.emptyList; import static org.sonar.server.setting.ws.SettingsWsComponentParameters.addComponentParameters; import static org.sonarqube.ws.client.ce.CeWsParameters.PARAM_COMPONENT_KEY; import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_RESET; @@ -95,7 +96,7 @@ public class ResetAction implements SettingsWsAction { Optional<ComponentDto> component = getComponent(dbSession, resetRequest); checkPermissions(component); resetRequest.getKeys().forEach(key -> { - SettingData data = new SettingData(key, component.orElse(null)); + SettingData data = new SettingData(key, emptyList(), component.orElse(null)); ImmutableList.of(validations.scope(), validations.qualifier()) .forEach(validation -> validation.validate(data)); }); 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 index bc9e212bb5e..0263760be91 100644 --- 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 @@ -29,7 +29,6 @@ import com.google.gson.reflect.TypeToken; import java.lang.reflect.Type; import java.util.Collections; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -159,7 +158,7 @@ public class SetAction implements SettingsWsAction { String value; - commonChecks(request, definition, component); + commonChecks(request, component); if (!request.getFieldValues().isEmpty()) { value = doHandlePropertySet(dbSession, request, definition, component); @@ -191,7 +190,7 @@ public class SetAction implements SettingsWsAction { List<String> fieldValues = request.getFieldValues(); IntStream.of(fieldIds).boxed() .flatMap(i -> readOneFieldValues(fieldValues.get(i - 1), request.getKey()).entrySet().stream() - .map(entry -> new KeyValue(key + "." + i + "." + entry.getKey(), entry.getValue()))) + .map(entry -> new KeyValue(key + "." + i + "." + entry.getKey(), entry.getValue()))) .forEach(keyValue -> dbClient.propertiesDao().saveProperty(dbSession, toFieldProperty(keyValue, componentId))); return inlinedFieldKeys; @@ -205,10 +204,10 @@ public class SetAction implements SettingsWsAction { } } - private void commonChecks(SetRequest request, @Nullable PropertyDefinition definition, Optional<ComponentDto> component) { + private void commonChecks(SetRequest request, Optional<ComponentDto> component) { checkValueIsSet(request); - SettingData settingData = new SettingData(request.getKey(), component.orElse(null)); - ImmutableList.of(validations.scope(), validations.qualifier()).stream() + SettingData settingData = new SettingData(request.getKey(), valuesFromRequest(request), component.orElse(null)); + ImmutableList.of(validations.scope(), validations.qualifier(), validations.valueType()) .forEach(validation -> validation.validate(settingData)); } @@ -237,7 +236,6 @@ public class SetAction implements SettingsWsAction { return; } - checkType(request, definition); checkSingleOrMultiValue(request, definition); } @@ -257,18 +255,6 @@ public class SetAction implements SettingsWsAction { "Parameter '%s' must be used for single value setting. Parameter '%s' must be used for multi value setting.", PARAM_VALUE, PARAM_VALUES); } - private void checkType(SetRequest request, PropertyDefinition definition) { - List<String> values = valuesFromRequest(request); - Optional<PropertyDefinition.Result> failingResult = values.stream() - .map(definition::validate) - .filter(result -> !result.isValid()) - .findAny(); - String errorKey = failingResult.isPresent() ? failingResult.get().getErrorKey() : null; - checkRequest(errorKey == null, - i18n.message(Locale.ENGLISH, "property.error." + errorKey, "Error when validating setting with key '%s' and value '%s'"), - request.getKey(), request.getValue()); - } - private static void checkValueIsSet(SetRequest request) { checkRequest(!isNullOrEmpty(request.getValue()) ^ !request.getValues().isEmpty() diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidations.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidations.java index daa4d186abb..0f70613f543 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidations.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingValidations.java @@ -20,14 +20,20 @@ package org.sonar.server.setting.ws; +import java.util.Collections; +import java.util.List; import java.util.Locale; +import java.util.Optional; +import java.util.stream.Collectors; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.config.PropertyDefinition; import org.sonar.api.config.PropertyDefinitions; import org.sonar.api.i18n.I18n; import org.sonar.db.component.ComponentDto; +import org.sonarqube.ws.client.setting.SetRequest; +import static java.util.Objects.requireNonNull; import static org.sonar.server.ws.WsUtils.checkRequest; public class SettingValidations { @@ -56,6 +62,27 @@ public class SettingValidations { }; } + public SettingValidation valueType() { + return data -> { + PropertyDefinition definition = definitions.get(data.key); + if (definition == null) { + return; + } + Optional<PropertyDefinition.Result> failingResult = data.values.stream() + .map(definition::validate) + .filter(result -> !result.isValid()) + .findAny(); + String errorKey = failingResult.isPresent() ? failingResult.get().getErrorKey() : null; + checkRequest(errorKey == null, + i18n.message(Locale.ENGLISH, "property.error." + errorKey, "Error when validating setting with key '%s' and value [%s]"), + data.key, data.values.stream().collect(Collectors.joining(", "))); + }; + } + + private static List<String> valuesFromRequest(SetRequest request) { + return request.getValue() == null ? request.getValues() : Collections.singletonList(request.getValue()); + } + private static boolean isGlobal(PropertyDefinition definition) { return !definition.global() && definition.qualifiers().isEmpty(); } @@ -67,11 +94,13 @@ public class SettingValidations { public static class SettingData { private final String key; + private final List<String> values; @CheckForNull private final ComponentDto component; - public SettingData(String key, @Nullable ComponentDto component) { - this.key = key; + public SettingData(String key, List<String> values, @Nullable ComponentDto component) { + this.key = requireNonNull(key); + this.values = requireNonNull(values); this.component = component; } diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java index 91eba042d89..f808060d76b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java @@ -403,9 +403,9 @@ public class SetActionTest { .defaultValue("default") .build()); expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Error when validating setting with key 'my.key' and value 'My Value'"); + expectedException.expectMessage("Error when validating setting with key 'my.key' and value [My Value, My Other Value]"); - callForGlobalSetting("my.key", "My Value"); + callForMultiValueGlobalSetting("my.key", newArrayList("My Value", "My Other Value")); } @Test 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 0100cf61d36..c6ef17c80af 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 @@ -38,6 +38,7 @@ import org.sonar.api.server.ServerSide; import org.sonarsource.api.sonarlint.SonarLintSide; import static com.google.common.base.Preconditions.checkArgument; +import static org.apache.commons.lang.StringUtils.isBlank; import static org.sonar.api.PropertyType.PROPERTY_SET; /** @@ -150,18 +151,25 @@ public final class PropertyDefinition { } public static Result validate(PropertyType type, @Nullable String value, List<String> options) { - if (StringUtils.isNotBlank(value)) { - if (type == PropertyType.BOOLEAN) { + if (isBlank(value)) { + return Result.SUCCESS; + } + + switch (type) { + case BOOLEAN: return validateBoolean(value); - } else if (type == PropertyType.INTEGER) { + case INTEGER: + case LONG: return validateInteger(value); - } else if (type == PropertyType.FLOAT) { + case FLOAT: return validateFloat(value); - } else if (type == PropertyType.SINGLE_SELECT_LIST && !options.contains(value)) { - return Result.newError("notInOptions"); - } + case SINGLE_SELECT_LIST: + if (!options.contains(value)) { + return Result.newError("notInOptions"); + } + default: + return Result.SUCCESS; } - return Result.SUCCESS; } private static Result validateBoolean(String value) { 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 cb4b67f242f..4f7a860a1c4 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 @@ -227,6 +227,19 @@ public class PropertyDefinitionTest { } @Test + public void should_validate_long() { + PropertyDefinition def = PropertyDefinition.builder("foo").name("foo").type(PropertyType.LONG).build(); + + assertThat(def.validate(null).isValid()).isTrue(); + assertThat(def.validate("").isValid()).isTrue(); + assertThat(def.validate(" ").isValid()).isTrue(); + assertThat(def.validate("123456").isValid()).isTrue(); + + assertThat(def.validate("foo").isValid()).isFalse(); + assertThat(def.validate("foo").getErrorKey()).isEqualTo("notInteger"); + } + + @Test public void should_validate_float() { PropertyDefinition def = PropertyDefinition.builder("foo").name("foo").type(PropertyType.FLOAT).build(); |