SONAR-7970 WS settings/set validates data

2 files changed, 138 insertions, 4 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/settings/ws/SetAction.java b/server/sonar-server/src/main/java/org/sonar/server/settings/ws/SetAction.java
index a5b7e060a47..c7fb7ac44e7 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/settings/ws/SetAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/settings/ws/SetAction.java
@@ -20,7 +20,11 @@
 
 package org.sonar.server.settings.ws;
 
+import java.util.Locale;
 import java.util.Optional;
+import org.sonar.api.config.PropertyDefinition;
+import org.sonar.api.config.PropertyDefinitions;
+import org.sonar.api.i18n.I18n;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
@@ -42,12 +46,18 @@ import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONE
 import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEY;
 import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_VALUE;
 
+import static org.sonar.server.ws.WsUtils.checkRequest;
+
 public class SetAction implements SettingsWsAction {
+  private final PropertyDefinitions propertyDefinitions;
+  private final I18n i18n;
   private final DbClient dbClient;
   private final ComponentFinder componentFinder;
   private final UserSession userSession;
 
-  public SetAction(DbClient dbClient, ComponentFinder componentFinder, UserSession userSession) {
+  public SetAction(PropertyDefinitions propertyDefinitions, I18n i18n, DbClient dbClient, ComponentFinder componentFinder, UserSession userSession) {
+    this.propertyDefinitions = propertyDefinitions;
+    this.i18n = i18n;
     this.dbClient = dbClient;
     this.componentFinder = componentFinder;
     this.userSession = userSession;
@@ -94,6 +104,7 @@ public class SetAction implements SettingsWsAction {
       Optional<ComponentDto> component = searchComponent(dbSession, setRequest);
       checkPermissions(component);
 
+      validate(setRequest, component);
       dbClient.propertiesDao().insertProperty(dbSession, toProperty(setRequest, component));
       dbSession.commit();
     } finally {
@@ -103,6 +114,25 @@ public class SetAction implements SettingsWsAction {
     response.noContent();
   }
 
+  private void validate(SetRequest request, Optional<ComponentDto> component) {
+    PropertyDefinition definition = propertyDefinitions.get(request.getKey());
+    if (definition == null) {
+      return;
+    }
+
+    PropertyDefinition.Result result = definition.validate(request.getValue());
+
+    checkRequest(result.isValid(),
+      i18n.message(Locale.ENGLISH, "property.error." + result.getErrorKey(), "Error when validating setting with key '%s' and value '%s'"),
+      request.getKey(), request.getValue());
+
+    checkRequest(component.isPresent() || definition.global(), "Setting '%s' cannot be global", request.getKey());
+    String qualifier = component.isPresent() ? component.get().qualifier() : "";
+    checkRequest(!component.isPresent()
+      || definition.qualifiers().contains(component.get().qualifier()),
+      "Setting '%s' cannot be set on a %s", request.getKey(), i18n.message(Locale.ENGLISH, "qualifier." + qualifier, null));
+  }
+
   private void checkPermissions(Optional<ComponentDto> component) {
     if (component.isPresent()) {
       userSession.checkComponentUuidPermission(UserRole.ADMIN, component.get().uuid());
@@ -130,9 +160,12 @@ public class SetAction implements SettingsWsAction {
     return Optional.of(project);
   }
 
-  private static PropertyDto toProperty(SetRequest request, Optional<ComponentDto> component) {
+  private PropertyDto toProperty(SetRequest request, Optional<ComponentDto> component) {
+    PropertyDefinition definition = propertyDefinitions.get(request.getKey());
+    // handles deprecated key but persist the new key
+    String key = definition == null ? request.getKey() : definition.key();
     PropertyDto property = new PropertyDto()
-      .setKey(request.getKey())
+      .setKey(key)
       .setValue(request.getValue());
 
     if (component.isPresent()) {
diff --git a/server/sonar-server/src/test/java/org/sonar/server/settings/ws/SetActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/settings/ws/SetActionTest.java
index aa8cb63e548..6c1e01e85cd 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/settings/ws/SetActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/settings/ws/SetActionTest.java
@@ -26,6 +26,10 @@ import javax.annotation.Nullable;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.sonar.api.PropertyType;
+import org.sonar.api.config.PropertyDefinition;
+import org.sonar.api.config.PropertyDefinitions;
+import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.server.ws.WebService.Param;
 import org.sonar.api.utils.System2;
@@ -40,7 +44,9 @@ import org.sonar.db.property.PropertyDbTester;
 import org.sonar.db.property.PropertyDto;
 import org.sonar.db.property.PropertyQuery;
 import org.sonar.server.component.ComponentFinder;
+import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
+import org.sonar.server.i18n.I18nRule;
 import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.TestResponse;
@@ -48,6 +54,7 @@ import org.sonar.server.ws.WsActionTester;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.groups.Tuple.tuple;
+import static org.sonar.db.component.ComponentTesting.newView;
 import static org.sonar.db.property.PropertyTesting.newComponentPropertyDto;
 import static org.sonar.db.property.PropertyTesting.newGlobalPropertyDto;
 
@@ -66,7 +73,10 @@ public class SetActionTest {
   DbSession dbSession = db.getSession();
   ComponentFinder componentFinder = new ComponentFinder(dbClient);
 
-  SetAction underTest = new SetAction(dbClient, componentFinder, userSession);
+  I18nRule i18n = new I18nRule();
+  PropertyDefinitions propertyDefinitions = new PropertyDefinitions();
+
+  SetAction underTest = new SetAction(propertyDefinitions, i18n, dbClient, componentFinder, userSession);
 
   WsActionTester ws = new WsActionTester(underTest);
 
@@ -143,6 +153,24 @@ public class SetActionTest {
   }
 
   @Test
+  public void persist_global_property_with_deprecated_key() {
+    propertyDefinitions.addComponent(PropertyDefinition
+      .builder("my.key")
+      .deprecatedKey("my.old.key")
+      .name("foo")
+      .description("desc")
+      .category("cat")
+      .subCategory("subCat")
+      .type(PropertyType.STRING)
+      .defaultValue("default")
+      .build());
+
+    callForGlobalProperty("my.old.key", "My Value");
+
+    assertGlobalProperty("my.key", "My Value");
+  }
+
+  @Test
   public void fail_when_no_key() {
     expectedException.expect(IllegalArgumentException.class);
 
@@ -181,6 +209,79 @@ public class SetActionTest {
   }
 
   @Test
+  public void fail_when_data_and_type_do_not_match() {
+    propertyDefinitions.addComponent(PropertyDefinition
+      .builder("my.key")
+      .name("foo")
+      .description("desc")
+      .category("cat")
+      .subCategory("subCat")
+      .type(PropertyType.INTEGER)
+      .defaultValue("default")
+      .build());
+    i18n.put("property.error.notInteger", "Not an integer error message");
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("Not an integer error message");
+
+    callForGlobalProperty("my.key", "My Value");
+  }
+
+  @Test
+  public void fail_when_data_and_type_do_not_match_with_unknown_error_key() {
+    propertyDefinitions.addComponent(PropertyDefinition
+      .builder("my.key")
+      .name("foo")
+      .description("desc")
+      .category("cat")
+      .subCategory("subCat")
+      .type(PropertyType.INTEGER)
+      .defaultValue("default")
+      .build());
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("Error when validating setting with key 'my.key' and value 'My Value'");
+
+    callForGlobalProperty("my.key", "My Value");
+  }
+
+  @Test
+  public void fail_when_global_with_property_only_on_projects() {
+    propertyDefinitions.addComponent(PropertyDefinition
+      .builder("my.key")
+      .name("foo")
+      .description("desc")
+      .category("cat")
+      .subCategory("subCat")
+      .type(PropertyType.INTEGER)
+      .defaultValue("default")
+      .onlyOnQualifiers(Qualifiers.PROJECT)
+      .build());
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("Setting 'my.key' cannot be global");
+
+    callForGlobalProperty("my.key", "42");
+  }
+
+  @Test
+  public void fail_when_view_property_when_on_projects_only() {
+    propertyDefinitions.addComponent(PropertyDefinition
+      .builder("my.key")
+      .name("foo")
+      .description("desc")
+      .category("cat")
+      .subCategory("subCat")
+      .type(PropertyType.STRING)
+      .defaultValue("default")
+      .onQualifiers(Qualifiers.PROJECT)
+      .build());
+    ComponentDto view = componentDb.insertComponent(newView("view-uuid"));
+    i18n.put("qualifier." + Qualifiers.VIEW, "View");
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("Setting 'my.key' cannot be set on a View");
+
+    callForProjectPropertyByUuid("my.key", "My Value", view.uuid());
+  }
+
+  @Test
   public void definition() {
     WebService.Action definition = ws.getDef();