From: Teryk Bellahsene Date: Mon, 29 Aug 2016 16:44:19 +0000 (+0200) Subject: SONAR-8004 WS settings/set handles property set X-Git-Tag: 6.1-RC1~258 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fpull%2F1190%2Fhead;p=sonarqube.git SONAR-8004 WS settings/set handles property set --- 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 25136c0a62b..629c520de22 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,14 +20,25 @@ package org.sonar.server.settings.ws; -import com.google.common.base.Joiner; +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ListMultimap; +import com.google.gson.JsonParseException; +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; +import java.util.stream.Collector; import java.util.stream.Collectors; +import java.util.stream.IntStream; +import javax.annotation.Nullable; +import org.sonar.api.PropertyType; import org.sonar.api.config.PropertyDefinition; import org.sonar.api.config.PropertyDefinitions; +import org.sonar.api.config.PropertyFieldDefinition; import org.sonar.api.i18n.I18n; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; @@ -39,7 +50,10 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.db.property.PropertyDto; +import org.sonar.scanner.protocol.GsonHelper; import org.sonar.server.component.ComponentFinder; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.platform.SettingsChangeNotifier; import org.sonar.server.user.UserSession; import org.sonar.server.ws.KeyExamples; import org.sonarqube.ws.client.setting.SetRequest; @@ -49,24 +63,31 @@ import static org.sonar.server.ws.WsUtils.checkRequest; import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_SET; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONENT_ID; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONENT_KEY; +import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_FIELD_VALUES; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEY; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_VALUE; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_VALUES; public class SetAction implements SettingsWsAction { - private static final Joiner COMMA_JOINER = Joiner.on(","); + private static final Collector COMMA_JOINER = Collectors.joining(","); + private final PropertyDefinitions propertyDefinitions; private final I18n i18n; private final DbClient dbClient; private final ComponentFinder componentFinder; private final UserSession userSession; + private final SettingsUpdater settingsUpdater; + private final SettingsChangeNotifier settingsChangeNotifier; - public SetAction(PropertyDefinitions propertyDefinitions, I18n i18n, DbClient dbClient, ComponentFinder componentFinder, UserSession userSession) { + public SetAction(PropertyDefinitions propertyDefinitions, I18n i18n, DbClient dbClient, ComponentFinder componentFinder, UserSession userSession, + SettingsUpdater settingsUpdater, SettingsChangeNotifier settingsChangeNotifier) { this.propertyDefinitions = propertyDefinitions; this.i18n = i18n; this.dbClient = dbClient; this.componentFinder = componentFinder; this.userSession = userSession; + this.settingsUpdater = settingsUpdater; + this.settingsChangeNotifier = settingsChangeNotifier; } @Override @@ -99,6 +120,10 @@ public class SetAction implements SettingsWsAction { .setDescription("Setting multi value. To set several values, the parameter must be called once for each value.") .setExampleValue("values=firstValue&values=secondValue&values=thirdValue"); + action.createParam(PARAM_FIELD_VALUES) + .setDescription("Setting field values. To set several values, the parameter must be called once for each value.") + .setExampleValue(PARAM_FIELD_VALUES + "={\"firstField\":\"first value\", \"secondField\":\"second value\", \"thirdField\":\"third value\"}"); + action.createParam(PARAM_COMPONENT_ID) .setDescription("Component id") .setExampleValue(Uuids.UUID_EXAMPLE_01); @@ -112,12 +137,7 @@ public class SetAction implements SettingsWsAction { public void handle(Request request, Response response) throws Exception { DbSession dbSession = dbClient.openSession(false); try { - SetRequest setRequest = toWsRequest(request); - Optional component = searchComponent(dbSession, setRequest); - checkPermissions(component); - - validate(setRequest, component); - dbClient.propertiesDao().insertProperty(dbSession, toProperty(setRequest, component)); + doHandle(dbSession, toWsRequest(request)); dbSession.commit(); } finally { dbClient.closeSession(dbSession); @@ -126,7 +146,79 @@ public class SetAction implements SettingsWsAction { response.noContent(); } - private void validate(SetRequest request, Optional component) { + private void doHandle(DbSession dbSession, SetRequest request) { + Optional component = searchComponent(dbSession, request); + checkPermissions(component); + + PropertyDefinition definition = propertyDefinitions.get(request.getKey()); + + String value; + + commonChecks(request, definition, component); + + if (!request.getFieldValues().isEmpty()) { + value = doHandlePropertySet(dbSession, request, definition, component); + } else { + validate(request); + PropertyDto property = toProperty(request, component); + value = property.getValue(); + dbClient.propertiesDao().insertProperty(dbSession, property); + } + + if (!component.isPresent()) { + settingsChangeNotifier.onGlobalPropertyChange(request.getKey(), value); + } + } + + private void commonChecks(SetRequest request, @Nullable PropertyDefinition definition, Optional component) { + checkValueIsSet(request); + checkGlobalOrProject(request, definition, component); + checkComponentQualifier(request, definition, component); + } + + private String doHandlePropertySet(DbSession dbSession, SetRequest request, @Nullable PropertyDefinition definition, Optional component) { + validatePropertySet(request, definition, component); + + int[] fieldIds = IntStream.rangeClosed(1, request.getFieldValues().size()).toArray(); + String inlinedFieldKeys = IntStream.of(fieldIds).mapToObj(String::valueOf).collect(COMMA_JOINER); + String key = persistedKey(request); + Long componentId = component.isPresent() ? component.get().getId() : null; + + deleteSettings(dbSession, component, key); + dbClient.propertiesDao().insertProperty(dbSession, new PropertyDto().setKey(key).setValue(inlinedFieldKeys).setResourceId(componentId)); + + List> fieldValues = request.getFieldValues(); + IntStream.of(fieldIds).boxed() + .flatMap(i -> fieldValues.get(i - 1).entrySet().stream().map(entry -> new KeyValue(key + "." + i + "." + entry.getKey(), entry.getValue()))) + .forEach(keyValue -> dbClient.propertiesDao().insertProperty(dbSession, toFieldProperty(keyValue, componentId))); + + return inlinedFieldKeys; + } + + private void deleteSettings(DbSession dbSession, Optional component, String key) { + if (component.isPresent()) { + settingsUpdater.deleteComponentSetting(dbSession, key, component.get()); + } else { + settingsUpdater.deleteGlobalSetting(dbSession, key); + } + } + + private void validatePropertySet(SetRequest request, @Nullable PropertyDefinition definition, Optional component) { + checkRequest(definition != null, "Setting '%s' is undefined", request.getKey()); + checkRequest(PropertyType.PROPERTY_SET.equals(definition.type()), "Parameter '%s' is used for setting of property set type only", PARAM_FIELD_VALUES); + + Set fieldKeys = definition.fields().stream().map(PropertyFieldDefinition::key).collect(Collectors.toSet()); + ListMultimap valuesByFieldKeys = ArrayListMultimap.create(fieldKeys.size(), request.getFieldValues().size() * fieldKeys.size()); + + request.getFieldValues().stream() + .flatMap(map -> map.entrySet().stream()) + .peek(entry -> valuesByFieldKeys.put(entry.getKey(), entry.getValue())) + .forEach(entry -> checkRequest(fieldKeys.contains(entry.getKey()), "Unknown field key '%s' for setting '%s'", entry.getKey(), request.getKey())); + + checkFieldType(request, definition, valuesByFieldKeys); + } + + private void validate(SetRequest request) { PropertyDefinition definition = propertyDefinitions.get(request.getKey()); if (definition == null) { return; @@ -134,8 +226,17 @@ public class SetAction implements SettingsWsAction { checkType(request, definition); checkSingleOrMultiValue(request, definition); - checkGlobalOrProject(request, definition, component); - checkComponentQualifier(request, definition, component); + } + + private static void checkFieldType(SetRequest request, PropertyDefinition definition, ListMultimap valuesByFieldKeys) { + for (PropertyFieldDefinition fieldDefinition : definition.fields()) { + for (String value : valuesByFieldKeys.get(fieldDefinition.key())) { + PropertyDefinition.Result result = fieldDefinition.validate(value); + checkRequest(result.isValid(), + "Error when validating setting with key '%s'. Field '%s' has incorrect value '%s'.", + request.getKey(), fieldDefinition.key(), value); + } + } } private static void checkSingleOrMultiValue(SetRequest request, PropertyDefinition definition) { @@ -143,14 +244,15 @@ 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 static void checkGlobalOrProject(SetRequest request, PropertyDefinition definition, Optional component) { - checkRequest(component.isPresent() || definition.global(), "Setting '%s' cannot be global", request.getKey()); + private static void checkGlobalOrProject(SetRequest request, @Nullable PropertyDefinition definition, Optional component) { + checkRequest(component.isPresent() || definition == null || definition.global(), "Setting '%s' cannot be global", request.getKey()); } - private void checkComponentQualifier(SetRequest request, PropertyDefinition definition, Optional component) { + private void checkComponentQualifier(SetRequest request, @Nullable PropertyDefinition definition, Optional component) { String qualifier = component.isPresent() ? component.get().qualifier() : ""; checkRequest(!component.isPresent() - || definition.qualifiers().contains(component.get().qualifier()), + || definition == null + || definition.qualifiers().contains(component.get().qualifier()), "Setting '%s' cannot be set on a %s", request.getKey(), i18n.message(Locale.ENGLISH, "qualifier." + qualifier, null)); } @@ -167,17 +269,25 @@ public class SetAction implements SettingsWsAction { } private static void checkValueIsSet(SetRequest request) { - checkRequest(isNullOrEmpty(request.getValue()) ^ request.getValues().isEmpty(), - "Either '%s' or '%s' must be provided, not both", PARAM_VALUE, PARAM_VALUES); + checkRequest(!isNullOrEmpty(request.getValue()) + ^ !request.getValues().isEmpty() + ^ !request.getFieldValues().isEmpty(), + "One and only one of '%s', '%s', '%s' must be provided", PARAM_VALUE, PARAM_VALUES, PARAM_FIELD_VALUES); } private static List valuesFromRequest(SetRequest request) { return request.getValue() == null ? request.getValues() : Collections.singletonList(request.getValue()); } + private String persistedKey(SetRequest request) { + PropertyDefinition definition = propertyDefinitions.get(request.getKey()); + // handles deprecated key but persist the new key + return definition == null ? request.getKey() : definition.key(); + } + private static String persistedValue(SetRequest request) { return request.getValue() == null - ? COMMA_JOINER.join(request.getValues().stream().map(value -> value.replace(",", "%2C")).collect(Collectors.toList())) + ? request.getValues().stream().map(value -> value.replace(",", "%2C")).collect(COMMA_JOINER) : request.getValue(); } @@ -190,17 +300,32 @@ public class SetAction implements SettingsWsAction { } private static SetRequest toWsRequest(Request request) { - SetRequest setRequest = SetRequest.builder() + return SetRequest.builder() .setKey(request.mandatoryParam(PARAM_KEY)) .setValue(request.param(PARAM_VALUE)) .setValues(request.multiParam(PARAM_VALUES)) + .setFieldValues(readFieldValues(request)) .setComponentId(request.param(PARAM_COMPONENT_ID)) .setComponentKey(request.param(PARAM_COMPONENT_KEY)) .build(); + } - checkValueIsSet(setRequest); + private static List> readFieldValues(Request request) { + String key = request.mandatoryParam(PARAM_KEY); - return setRequest; + return request.multiParam(PARAM_FIELD_VALUES).stream() + .map(json -> readOneFieldValues(json, key)) + .collect(Collectors.toList()); + } + + private static Map readOneFieldValues(String json, String key) { + Type type = new TypeToken>() { + }.getType(); + try { + return (Map) GsonHelper.create().fromJson(json, type); + } catch (JsonParseException e) { + throw new BadRequestException(String.format("Invalid JSON '%s' for setting '%s'", json, key)); + } } private Optional searchComponent(DbSession dbSession, SetRequest request) { @@ -214,9 +339,7 @@ public class SetAction implements SettingsWsAction { } private PropertyDto toProperty(SetRequest request, Optional component) { - PropertyDefinition definition = propertyDefinitions.get(request.getKey()); - // handles deprecated key but persist the new key - String key = definition == null ? request.getKey() : definition.key(); + String key = persistedKey(request); String value = persistedValue(request); PropertyDto property = new PropertyDto() @@ -229,4 +352,18 @@ public class SetAction implements SettingsWsAction { return property; } + + private static PropertyDto toFieldProperty(KeyValue keyValue, @Nullable Long componentId) { + return new PropertyDto().setKey(keyValue.key).setValue(keyValue.value).setResourceId(componentId); + } + + private static class KeyValue { + private final String key; + private final String value; + + private KeyValue(String key, String value) { + this.key = key; + this.value = value; + } + } } 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 afb06287fc5..1e7352643d8 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 @@ -20,6 +20,8 @@ package org.sonar.server.settings.ws; +import com.google.common.collect.ImmutableMap; +import com.google.gson.Gson; import java.net.HttpURLConnection; import java.util.List; import javax.annotation.Nullable; @@ -29,6 +31,7 @@ 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.config.PropertyFieldDefinition; import org.sonar.api.resources.Qualifiers; import org.sonar.api.server.ws.WebService; import org.sonar.api.server.ws.WebService.Param; @@ -43,24 +46,32 @@ import org.sonar.db.component.ComponentDto; import org.sonar.db.property.PropertyDbTester; import org.sonar.db.property.PropertyDto; import org.sonar.db.property.PropertyQuery; +import org.sonar.scanner.protocol.GsonHelper; 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.platform.SettingsChangeNotifier; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.TestResponse; import org.sonar.server.ws.WsActionTester; import static com.google.common.collect.Lists.newArrayList; +import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.groups.Tuple.tuple; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.sonar.db.component.ComponentTesting.newView; import static org.sonar.db.property.PropertyTesting.newComponentPropertyDto; import static org.sonar.db.property.PropertyTesting.newGlobalPropertyDto; public class SetActionTest { + private static final Gson GSON = GsonHelper.create(); + @Rule public ExpectedException expectedException = ExpectedException.none(); @Rule @@ -76,8 +87,10 @@ public class SetActionTest { I18nRule i18n = new I18nRule(); PropertyDefinitions propertyDefinitions = new PropertyDefinitions(); + SettingsChangeNotifier settingsChangeNotifier = mock(SettingsChangeNotifier.class); + SettingsUpdater settingsUpdater = new SettingsUpdater(dbClient, propertyDefinitions); - SetAction underTest = new SetAction(propertyDefinitions, i18n, dbClient, componentFinder, userSession); + SetAction underTest = new SetAction(propertyDefinitions, i18n, dbClient, componentFinder, userSession, settingsUpdater, settingsChangeNotifier); WsActionTester ws = new WsActionTester(underTest); @@ -93,31 +106,34 @@ public class SetActionTest { } @Test - public void persist_new_global_property() { - callForGlobalProperty("my.key", "my,value"); + public void persist_new_global_setting() { + callForGlobalSetting("my.key", "my,value"); - assertGlobalProperty("my.key", "my,value"); + assertGlobalSetting("my.key", "my,value"); + verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", "my,value"); } @Test - public void update_existing_global_property() { + public void update_existing_global_setting() { propertyDb.insertProperty(newGlobalPropertyDto("my.key", "my value")); - assertGlobalProperty("my.key", "my value"); + assertGlobalSetting("my.key", "my value"); - callForGlobalProperty("my.key", "my new value"); + callForGlobalSetting("my.key", "my new value"); - assertGlobalProperty("my.key", "my new value"); + assertGlobalSetting("my.key", "my new value"); + verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", "my new value"); } @Test - public void persist_new_project_property() { + public void persist_new_project_setting() { propertyDb.insertProperty(newGlobalPropertyDto("my.key", "my global value")); ComponentDto project = componentDb.insertProject(); - callForProjectPropertyByUuid("my.key", "my project value", project.uuid()); + callForProjectSettingByUuid("my.key", "my project value", project.uuid()); - assertGlobalProperty("my.key", "my global value"); - assertProjectProperty("my.key", "my project value", project.getId()); + assertGlobalSetting("my.key", "my global value"); + assertComponentSetting("my.key", "my project value", project.getId()); + verifyZeroInteractions(settingsChangeNotifier); } @Test @@ -125,46 +141,181 @@ public class SetActionTest { ComponentDto project = componentDb.insertProject(); userSession.anonymous().addProjectUuidPermissions(UserRole.ADMIN, project.uuid()); - callForProjectPropertyByUuid("my.key", "my value", project.uuid()); + callForProjectSettingByUuid("my.key", "my value", project.uuid()); - assertProjectProperty("my.key", "my value", project.getId()); + assertComponentSetting("my.key", "my value", project.getId()); } @Test - public void update_existing_project_property() { + public void update_existing_project_setting() { propertyDb.insertProperty(newGlobalPropertyDto("my.key", "my global value")); ComponentDto project = componentDb.insertProject(); propertyDb.insertProperty(newComponentPropertyDto("my.key", "my project value", project)); - assertProjectProperty("my.key", "my project value", project.getId()); + assertComponentSetting("my.key", "my project value", project.getId()); - callForProjectPropertyByKey("my.key", "my new project value", project.key()); + callForProjectSettingByKey("my.key", "my new project value", project.key()); - assertProjectProperty("my.key", "my new project value", project.getId()); + assertComponentSetting("my.key", "my new project value", project.getId()); } @Test - public void persist_several_multi_value_property() { - callForMultiValueGlobalProperty("my.key", newArrayList("first,Value", "second,Value", "third,Value")); + public void persist_several_multi_value_setting() { + callForMultiValueGlobalSetting("my.key", newArrayList("first,Value", "second,Value", "third,Value")); - assertGlobalProperty("my.key", "first%2CValue,second%2CValue,third%2CValue"); + String expectedValue = "first%2CValue,second%2CValue,third%2CValue"; + assertGlobalSetting("my.key", expectedValue); + verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", expectedValue); } @Test - public void persist_one_multi_value_property() { - callForMultiValueGlobalProperty("my.key", newArrayList("first,Value")); + public void persist_one_multi_value_setting() { + callForMultiValueGlobalSetting("my.key", newArrayList("first,Value")); + + assertGlobalSetting("my.key", "first%2CValue"); + } + + @Test + public void persist_property_set_setting() { + propertyDefinitions.addComponent(PropertyDefinition + .builder("my.key") + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.PROPERTY_SET) + .defaultValue("default") + .fields(newArrayList( + PropertyFieldDefinition.build("firstField") + .name("First Field") + .type(PropertyType.STRING) + .build(), + PropertyFieldDefinition.build("secondField") + .name("Second Field") + .type(PropertyType.STRING) + .build())) + .build()); - assertGlobalProperty("my.key", "first%2CValue"); + callForGlobalPropertySet("my.key", newArrayList( + GSON.toJson(ImmutableMap.of("firstField", "firstValue", "secondField", "secondValue")), + GSON.toJson(ImmutableMap.of("firstField", "anotherFirstValue", "secondField", "anotherSecondValue")), + GSON.toJson(ImmutableMap.of("firstField", "yetFirstValue", "secondField", "yetSecondValue")))); + + assertThat(dbClient.propertiesDao().selectGlobalProperties(dbSession)).hasSize(7); + assertGlobalSetting("my.key", "1,2,3"); + assertGlobalSetting("my.key.1.firstField", "firstValue"); + assertGlobalSetting("my.key.1.secondField", "secondValue"); + assertGlobalSetting("my.key.2.firstField", "anotherFirstValue"); + assertGlobalSetting("my.key.2.secondField", "anotherSecondValue"); + assertGlobalSetting("my.key.3.firstField", "yetFirstValue"); + assertGlobalSetting("my.key.3.secondField", "yetSecondValue"); + verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", "1,2,3"); } @Test - public void user_property_is_not_updated() { + public void update_property_set_setting() { + propertyDefinitions.addComponent(PropertyDefinition + .builder("my.key") + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.PROPERTY_SET) + .defaultValue("default") + .fields(newArrayList( + PropertyFieldDefinition.build("firstField") + .name("First Field") + .type(PropertyType.STRING) + .build(), + PropertyFieldDefinition.build("secondField") + .name("Second Field") + .type(PropertyType.STRING) + .build())) + .build()); + propertyDb.insertProperties( + newGlobalPropertyDto("my.key", "1,2,3,4"), + newGlobalPropertyDto("my.key.1.firstField", "oldFirstValue"), + newGlobalPropertyDto("my.key.1.secondField", "oldSecondValue"), + newGlobalPropertyDto("my.key.2.firstField", "anotherOldFirstValue"), + newGlobalPropertyDto("my.key.2.secondField", "anotherOldSecondValue"), + newGlobalPropertyDto("my.key.3.firstField", "oldFirstValue"), + newGlobalPropertyDto("my.key.3.secondField", "oldSecondValue"), + newGlobalPropertyDto("my.key.4.firstField", "anotherOldFirstValue"), + newGlobalPropertyDto("my.key.4.secondField", "anotherOldSecondValue")); + + callForGlobalPropertySet("my.key", newArrayList( + GSON.toJson(ImmutableMap.of("firstField", "firstValue", "secondField", "secondValue")), + GSON.toJson(ImmutableMap.of("firstField", "anotherFirstValue", "secondField", "anotherSecondValue")), + GSON.toJson(ImmutableMap.of("firstField", "yetFirstValue", "secondField", "yetSecondValue")))); + + assertThat(dbClient.propertiesDao().selectGlobalProperties(dbSession)).hasSize(7); + assertGlobalSetting("my.key", "1,2,3"); + assertGlobalSetting("my.key.1.firstField", "firstValue"); + assertGlobalSetting("my.key.1.secondField", "secondValue"); + assertGlobalSetting("my.key.2.firstField", "anotherFirstValue"); + assertGlobalSetting("my.key.2.secondField", "anotherSecondValue"); + assertGlobalSetting("my.key.3.firstField", "yetFirstValue"); + assertGlobalSetting("my.key.3.secondField", "yetSecondValue"); + verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", "1,2,3"); + } + + @Test + public void update_property_set_on_component_setting() { + propertyDefinitions.addComponent(PropertyDefinition + .builder("my.key") + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.PROPERTY_SET) + .defaultValue("default") + .onQualifiers(Qualifiers.PROJECT) + .fields(newArrayList( + PropertyFieldDefinition.build("firstField") + .name("First Field") + .type(PropertyType.STRING) + .build(), + PropertyFieldDefinition.build("secondField") + .name("Second Field") + .type(PropertyType.STRING) + .build())) + .build()); + ComponentDto project = componentDb.insertProject(); + propertyDb.insertProperties( + newGlobalPropertyDto("my.key", "1"), + newGlobalPropertyDto("my.key.1.firstField", "oldFirstValue"), + newGlobalPropertyDto("my.key.1.secondField", "oldSecondValue"), + newComponentPropertyDto("my.key", "1", project), + newComponentPropertyDto("my.key.1.firstField", "componentFirstValue", project), + newComponentPropertyDto("my.key.1.firstField", "componentSecondValue", project)); + + callForComponentPropertySetByUuid("my.key", newArrayList( + GSON.toJson(ImmutableMap.of("firstField", "firstValue", "secondField", "secondValue")), + GSON.toJson(ImmutableMap.of("firstField", "anotherFirstValue", "secondField", "anotherSecondValue"))), + project.uuid()); + + assertThat(dbClient.propertiesDao().selectGlobalProperties(dbSession)).hasSize(3); + assertThat(dbClient.propertiesDao().selectProjectProperties(dbSession, project.key())).hasSize(5); + assertGlobalSetting("my.key", "1"); + assertGlobalSetting("my.key.1.firstField", "oldFirstValue"); + assertGlobalSetting("my.key.1.secondField", "oldSecondValue"); + Long projectId = project.getId(); + assertComponentSetting("my.key", "1,2", projectId); + assertComponentSetting("my.key.1.firstField", "firstValue", projectId); + assertComponentSetting("my.key.1.secondField", "secondValue", projectId); + assertComponentSetting("my.key.2.firstField", "anotherFirstValue", projectId); + assertComponentSetting("my.key.2.secondField", "anotherSecondValue", projectId); + verifyZeroInteractions(settingsChangeNotifier); + } + + @Test + public void user_setting_is_not_updated() { propertyDb.insertProperty(newGlobalPropertyDto("my.key", "my user value").setUserId(42L)); propertyDb.insertProperty(newGlobalPropertyDto("my.key", "my global value")); - callForGlobalProperty("my.key", "my new global value"); + callForGlobalSetting("my.key", "my new global value"); - assertGlobalProperty("my.key", "my new global value"); - assertUserProperty("my.key", "my user value", 42L); + assertGlobalSetting("my.key", "my new global value"); + assertUserSetting("my.key", "my user value", 42L); } @Test @@ -180,16 +331,16 @@ public class SetActionTest { .defaultValue("default") .build()); - callForGlobalProperty("my.old.key", "My Value"); + callForGlobalSetting("my.old.key", "My Value"); - assertGlobalProperty("my.key", "My Value"); + assertGlobalSetting("my.key", "My Value"); } @Test public void fail_when_no_key() { expectedException.expect(IllegalArgumentException.class); - callForGlobalProperty(null, "my value"); + callForGlobalSetting(null, "my value"); } @Test @@ -197,23 +348,23 @@ public class SetActionTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Setting key is mandatory and must not be empty"); - callForGlobalProperty(" ", "my value"); + callForGlobalSetting(" ", "my value"); } @Test public void fail_when_no_value() { expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Either 'value' or 'values' must be provided, not both"); + expectedException.expectMessage("One and only one of 'value', 'values', 'fieldValues' must be provided"); - callForGlobalProperty("my.key", null); + callForGlobalSetting("my.key", null); } @Test public void fail_when_empty_value() { expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Either 'value' or 'values' must be provided, not both"); + expectedException.expectMessage("One and only one of 'value', 'values', 'fieldValues' must be provided"); - callForGlobalProperty("my.key", ""); + callForGlobalSetting("my.key", ""); } @Test @@ -221,7 +372,7 @@ public class SetActionTest { userSession.anonymous().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); expectedException.expect(ForbiddenException.class); - callForGlobalProperty("my.key", "my value"); + callForGlobalSetting("my.key", "my value"); } @Test @@ -239,7 +390,7 @@ public class SetActionTest { expectedException.expect(BadRequestException.class); expectedException.expectMessage("Not an integer error message"); - callForGlobalProperty("my.key", "My Value"); + callForGlobalSetting("my.key", "My Value"); } @Test @@ -256,7 +407,7 @@ public class SetActionTest { expectedException.expect(BadRequestException.class); expectedException.expectMessage("Error when validating setting with key 'my.key' and value 'My Value'"); - callForGlobalProperty("my.key", "My Value"); + callForGlobalSetting("my.key", "My Value"); } @Test @@ -274,7 +425,7 @@ public class SetActionTest { expectedException.expect(BadRequestException.class); expectedException.expectMessage("Setting 'my.key' cannot be global"); - callForGlobalProperty("my.key", "42"); + callForGlobalSetting("my.key", "42"); } @Test @@ -294,15 +445,15 @@ public class SetActionTest { expectedException.expect(BadRequestException.class); expectedException.expectMessage("Setting 'my.key' cannot be set on a View"); - callForProjectPropertyByUuid("my.key", "My Value", view.uuid()); + callForProjectSettingByUuid("my.key", "My Value", view.uuid()); } @Test public void fail_when_single_and_multi_value_provided() { expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Either 'value' or 'values' must be provided, not both"); + expectedException.expectMessage("One and only one of 'value', 'values', 'fieldValues' must be provided"); - call("my.key", "My Value", newArrayList("Another Value"), null, null); + call("my.key", "My Value", newArrayList("Another Value"), null, null, null); } @Test @@ -319,7 +470,7 @@ public class SetActionTest { expectedException.expect(BadRequestException.class); expectedException.expectMessage("Parameter 'value' must be used for single value setting. Parameter 'values' must be used for multi value setting."); - callForGlobalProperty("my.key", "My Value"); + callForGlobalSetting("my.key", "My Value"); } @Test @@ -336,7 +487,106 @@ public class SetActionTest { expectedException.expect(BadRequestException.class); expectedException.expectMessage("Parameter 'value' must be used for single value setting. Parameter 'values' must be used for multi value setting."); - callForMultiValueGlobalProperty("my.key", newArrayList("My Value")); + callForMultiValueGlobalSetting("my.key", newArrayList("My Value")); + } + + @Test + public void fail_when_property_set_setting_is_not_defined() { + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Setting 'my.key' is undefined"); + + callForGlobalPropertySet("my.key", singletonList("{\"field\":\"value\"}")); + } + + @Test + public void fail_when_property_set_with_unknown_field() { + propertyDefinitions.addComponent(PropertyDefinition + .builder("my.key") + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.PROPERTY_SET) + .defaultValue("default") + .fields(newArrayList( + PropertyFieldDefinition.build("field") + .name("Field") + .type(PropertyType.STRING) + .build())) + .build()); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Unknown field key 'unknownField' for setting 'my.key'"); + + callForGlobalPropertySet("my.key", newArrayList(GSON.toJson(ImmutableMap.of("field", "value", "unknownField", "anotherValue")))); + } + + @Test + public void fail_when_property_set_has_field_with_incorrect_type() { + propertyDefinitions.addComponent(PropertyDefinition + .builder("my.key") + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.PROPERTY_SET) + .defaultValue("default") + .fields(newArrayList( + PropertyFieldDefinition.build("field") + .name("Field") + .type(PropertyType.INTEGER) + .build())) + .build()); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Error when validating setting with key 'my.key'. Field 'field' has incorrect value 'notAnInt'."); + + callForGlobalPropertySet("my.key", newArrayList(GSON.toJson(ImmutableMap.of("field", "notAnInt")))); + } + + @Test + public void fail_when_property_set_with_invalid_json() { + propertyDefinitions.addComponent(PropertyDefinition + .builder("my.key") + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.PROPERTY_SET) + .defaultValue("default") + .fields(newArrayList( + PropertyFieldDefinition.build("field") + .name("Field") + .type(PropertyType.STRING) + .build())) + .build()); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Invalid JSON 'incorrectJson:incorrectJson' for setting 'my.key'"); + + callForGlobalPropertySet("my.key", newArrayList("incorrectJson:incorrectJson")); + } + + @Test + public void fail_when_property_set_on_component_of_global_setting() { + propertyDefinitions.addComponent(PropertyDefinition + .builder("my.key") + .name("foo") + .description("desc") + .category("cat") + .subCategory("subCat") + .type(PropertyType.PROPERTY_SET) + .defaultValue("default") + .fields(newArrayList(PropertyFieldDefinition.build("firstField").name("First Field").type(PropertyType.STRING).build())) + .build()); + i18n.put("qualifier." + Qualifiers.PROJECT, "Project"); + ComponentDto project = componentDb.insertProject(); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Setting 'my.key' cannot be set on a Project"); + + callForComponentPropertySetByUuid("my.key", newArrayList( + GSON.toJson(ImmutableMap.of("firstField", "firstValue"))), project.uuid()); } @Test @@ -347,10 +597,10 @@ public class SetActionTest { assertThat(definition.isPost()).isTrue(); assertThat(definition.since()).isEqualTo("6.1"); assertThat(definition.params()).extracting(Param::key) - .containsOnly("key", "value", "values", "componentId", "componentKey"); + .containsOnly("key", "value", "values", "fieldValues", "componentId", "componentKey"); } - private void assertGlobalProperty(String key, String value) { + private void assertGlobalSetting(String key, String value) { PropertyDto result = dbClient.propertiesDao().selectGlobalProperty(key); assertThat(result) @@ -358,7 +608,7 @@ public class SetActionTest { .containsExactly(key, value, null); } - private void assertUserProperty(String key, String value, long userId) { + private void assertUserSetting(String key, String value, long userId) { List result = dbClient.propertiesDao().selectByQuery(PropertyQuery.builder().setKey(key).setUserId((int) userId).build(), dbSession); assertThat(result).hasSize(1) @@ -366,7 +616,7 @@ public class SetActionTest { .containsExactly(tuple(key, value, userId)); } - private void assertProjectProperty(String key, String value, long componentId) { + private void assertComponentSetting(String key, String value, long componentId) { PropertyDto result = dbClient.propertiesDao().selectProjectProperty(componentId, key); assertThat(result) @@ -374,23 +624,32 @@ public class SetActionTest { .containsExactly(key, value, componentId); } - private void callForGlobalProperty(@Nullable String key, @Nullable String value) { - call(key, value, null, null, null); + private void callForGlobalSetting(@Nullable String key, @Nullable String value) { + call(key, value, null, null, null, null); } - private void callForMultiValueGlobalProperty(@Nullable String key, @Nullable List values) { - call(key, null, values, null, null); + private void callForMultiValueGlobalSetting(@Nullable String key, @Nullable List values) { + call(key, null, values, null, null, null); } - private void callForProjectPropertyByUuid(@Nullable String key, @Nullable String value, @Nullable String componentUuid) { - call(key, value, null, componentUuid, null); + private void callForGlobalPropertySet(@Nullable String key, @Nullable List fieldValues) { + call(key, null, null, fieldValues, null, null); } - private void callForProjectPropertyByKey(@Nullable String key, @Nullable String value, @Nullable String componentKey) { - call(key, value, null, null, componentKey); + private void callForComponentPropertySetByUuid(@Nullable String key, @Nullable List fieldValues, @Nullable String componentUuid) { + call(key, null, null, fieldValues, componentUuid, null); } - private void call(@Nullable String key, @Nullable String value, @Nullable List values, @Nullable String componentUuid, @Nullable String componentKey) { + private void callForProjectSettingByUuid(@Nullable String key, @Nullable String value, @Nullable String componentUuid) { + call(key, value, null, null, componentUuid, null); + } + + private void callForProjectSettingByKey(@Nullable String key, @Nullable String value, @Nullable String componentKey) { + call(key, value, null, null, null, componentKey); + } + + private void call(@Nullable String key, @Nullable String value, @Nullable List values, @Nullable List fieldValues, @Nullable String componentUuid, + @Nullable String componentKey) { TestRequest request = ws.newRequest(); if (key != null) { @@ -405,6 +664,10 @@ public class SetActionTest { request.setMultiParam("values", values); } + if (fieldValues != null) { + request.setMultiParam("fieldValues", fieldValues); + } + if (componentUuid != null) { request.setParam("componentId", componentUuid); } diff --git a/sonar-db/src/main/java/org/sonar/db/property/PropertyDto.java b/sonar-db/src/main/java/org/sonar/db/property/PropertyDto.java index cffe1e84c02..404be7e30ad 100644 --- a/sonar-db/src/main/java/org/sonar/db/property/PropertyDto.java +++ b/sonar-db/src/main/java/org/sonar/db/property/PropertyDto.java @@ -21,6 +21,8 @@ package org.sonar.db.property; import com.google.common.base.MoreObjects; import java.util.Objects; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import static com.google.common.base.Preconditions.checkArgument; @@ -61,20 +63,22 @@ public class PropertyDto { return this; } + @CheckForNull public Long getResourceId() { return resourceId; } - public PropertyDto setResourceId(Long resourceId) { + public PropertyDto setResourceId(@Nullable Long resourceId) { this.resourceId = resourceId; return this; } + @CheckForNull public Long getUserId() { return userId; } - public PropertyDto setUserId(Long userId) { + public PropertyDto setUserId(@Nullable Long userId) { this.userId = userId; return this; } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java index c9d23cb6386..c0b7ec7d618 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java @@ -38,6 +38,8 @@ import static com.google.common.base.Preconditions.checkArgument; */ public abstract class Request { + private static final String MSG_PARAMETER_MISSING = "The '%s' parameter is missing"; + /** * Returns the name of the HTTP method with which this request was made. Possible * values are GET and POST. Others are not supported. @@ -62,7 +64,7 @@ public abstract class Request { public String mandatoryParam(String key) { String value = param(key); if (value == null) { - throw new IllegalArgumentException(String.format("The '%s' parameter is missing", key)); + throw new IllegalArgumentException(String.format(MSG_PARAMETER_MISSING, key)); } return value; } @@ -104,14 +106,14 @@ public abstract class Request { public List mandatoryParamAsStrings(String key) { List values = paramAsStrings(key); if (values == null) { - throw new IllegalArgumentException(String.format("The '%s' parameter is missing", key)); + throw new IllegalArgumentException(String.format(MSG_PARAMETER_MISSING, key)); } return values; } public List mandatoryMultiParam(String key) { List values = multiParam(key); - checkArgument(!values.isEmpty(), "The '%s' parameter is missing", key); + checkArgument(!values.isEmpty(), MSG_PARAMETER_MISSING, key); return values; } @@ -138,7 +140,7 @@ public abstract class Request { public Part mandatoryParamAsPart(String key) { Part part = paramAsPart(key); - checkArgument(part != null, "The '%s' parameter is missing", key); + checkArgument(part != null, MSG_PARAMETER_MISSING, key); return part; } diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SetRequest.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SetRequest.java index ab5c1bd340e..2c09dbd107a 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SetRequest.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SetRequest.java @@ -21,6 +21,7 @@ package org.sonarqube.ws.client.setting; import java.util.List; +import java.util.Map; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -31,6 +32,7 @@ public class SetRequest { private final String key; private final String value; private final List values; + private final List> fieldValues; private final String componentId; private final String componentKey; @@ -38,6 +40,7 @@ public class SetRequest { this.key = builder.key; this.value = builder.value; this.values = builder.values; + this.fieldValues = builder.fieldValues; this.componentId = builder.componentId; this.componentKey = builder.componentKey; } @@ -55,6 +58,10 @@ public class SetRequest { return values; } + public List> getFieldValues() { + return fieldValues; + } + @CheckForNull public String getComponentId() { return componentId; @@ -73,6 +80,7 @@ public class SetRequest { private String key; private String value; private List values = emptyList(); + private List> fieldValues = emptyList(); private String componentId; private String componentKey; @@ -95,6 +103,11 @@ public class SetRequest { return this; } + public Builder setFieldValues(List> fieldValues) { + this.fieldValues = fieldValues; + return this; + } + public Builder setComponentId(@Nullable String componentId) { this.componentId = componentId; return this; @@ -108,6 +121,7 @@ public class SetRequest { public SetRequest build() { checkArgument(key != null && !key.isEmpty(), "Setting key is mandatory and must not be empty"); checkArgument(values != null, "Setting values must not be null"); + checkArgument(fieldValues != null, "Setting fields values must not be null"); return new SetRequest(this); } } diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SettingsWsParameters.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SettingsWsParameters.java index 5c6e35d243d..85e4da4e300 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SettingsWsParameters.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SettingsWsParameters.java @@ -34,6 +34,7 @@ public class SettingsWsParameters { public static final String PARAM_KEY = "key"; public static final String PARAM_VALUE = "value"; public static final String PARAM_VALUES = "values"; + public static final String PARAM_FIELD_VALUES = "fieldValues"; private SettingsWsParameters() { // Only static stuff diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/ServiceTester.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/ServiceTester.java index 13d633ec1ec..89758a70d9d 100644 --- a/sonar-ws/src/test/java/org/sonarqube/ws/client/ServiceTester.java +++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/ServiceTester.java @@ -19,6 +19,7 @@ */ package org.sonarqube.ws.client; +import com.google.common.base.Joiner; import com.google.protobuf.Parser; import java.util.ArrayList; import java.util.List; @@ -88,6 +89,8 @@ import static org.mockito.Mockito.spy; * */ public class ServiceTester extends ExternalResource { + private static final Joiner COMMA_JOINER = Joiner.on(","); + private final T underTest; private final List getCalls = new ArrayList<>(); private final List postCalls = new ArrayList<>(); @@ -318,6 +321,16 @@ public class ServiceTester extends ExternalResource { return this; } + public RequestAssert hasParam(String key, List values) { + isNotNull(); + + MapEntry entry = MapEntry.entry(key, values.toString()); + Assertions.assertThat(actual.getParams()).contains(entry); + this.assertedParams.add(entry); + + return this; + } + public RequestAssert andNoOtherParam() { isNotNull(); diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/SettingsServiceTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/SettingsServiceTest.java index 41b403e82c8..9fe11418c09 100644 --- a/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/SettingsServiceTest.java +++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/SettingsServiceTest.java @@ -30,6 +30,7 @@ import org.sonarqube.ws.client.WsConnector; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONENT_ID; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONENT_KEY; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEY; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEYS; @@ -75,12 +76,15 @@ public class SettingsServiceTest { underTest.set(SetRequest.builder() .setKey("sonar.debt") .setValue("8h") + // TODO WS Client must handle multi value param + .setComponentId("UUID") .setComponentKey("KEY") .build()); serviceTester.assertThat(serviceTester.getPostRequest()) .hasParam(PARAM_KEY, "sonar.debt") .hasParam(PARAM_VALUE, "8h") + .hasParam(PARAM_COMPONENT_ID, "UUID") .hasParam(PARAM_COMPONENT_KEY, "KEY") .andNoOtherParam(); }