SONAR-8050 Validate settings of type LONG

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 b3f38531604b6bb55436c8f31871f7b5cb39838a..baea3587c534da9785ea0996fd464f2a0410247e 100644 (file)
--- 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 bc9e212bb5e906ca882c1342697bd607354dbfec..0263760be91365e77a7fbf89101ada4e077730b5 100644 (file)
--- 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 daa4d186abbeb67a758c0f419ab1609c1f620abe..0f70613f5438310aa468fb653018a76cf8dca2dd 100644 (file)
--- 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 91eba042d89291e1c62d62b1ff023da1576351f1..f808060d76bcd2f02fe027b9017c57bdaf15856b 100644 (file)
--- 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 0100cf61d360b4c4f3310ff4bf481ac1ed72ce36..c6ef17c80afec9bf3b459c4e67ed067760329eb3 100644 (file)
--- 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 cb4b67f242f5a7bed66f7c25be71de0e127c6edf..4f7a860a1c417178aaf0fff94bbfd36ffba2b887 100644 (file)
--- 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
@@ -226,6 +226,19 @@ public class PropertyDefinitionTest {
     assertThat(def.validate("foo").getErrorKey()).isEqualTo("notInteger");
   }
 
+  @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();