From: Julien Lancelot Date: Fri, 2 Sep 2016 08:04:35 +0000 (+0200) Subject: SONAR-7986 Accept multiple keys in api/settings/rest X-Git-Tag: 6.1-RC1~234 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=1e2114b913d160025f5d643fda30ffddf25ced45;p=sonarqube.git SONAR-7986 Accept multiple keys in api/settings/rest --- diff --git a/it/it-tests/src/test/java/it/settings/PropertySetsTest.java b/it/it-tests/src/test/java/it/settings/PropertySetsTest.java index 0dca1cf94b5..57816603844 100644 --- a/it/it-tests/src/test/java/it/settings/PropertySetsTest.java +++ b/it/it-tests/src/test/java/it/settings/PropertySetsTest.java @@ -128,8 +128,8 @@ public class PropertySetsTest { Settings.Setting setting = getSetting(baseSettingKey); assertThat(setting.getFieldValues().getFieldValuesList()).hasSize(fieldsValues.length); int index = 0; - for (Settings.FieldValues.Value fieldsValue : setting.getFieldValues().getFieldValuesList()) { - assertThat(fieldsValue.getValue()).containsOnly(fieldsValues[index].toArray(new Map.Entry[]{})); + for (Settings.FieldValues.Value fieldValue : setting.getFieldValues().getFieldValuesList()) { + assertThat(fieldValue.getValue()).containsOnly(fieldsValues[index].toArray(new Map.Entry[] {})); index++; } } @@ -142,7 +142,7 @@ public class PropertySetsTest { } static void resetSetting(String... keys) { - stream(keys).forEach(key -> SETTINGS.reset(ResetRequest.builder().setKey(key).build())); + stream(keys).forEach(key -> SETTINGS.reset(ResetRequest.builder().setKeys(keys).build())); } } diff --git a/it/it-tests/src/test/java/it/settings/SettingsTest.java b/it/it-tests/src/test/java/it/settings/SettingsTest.java index 6fc55ded699..36d0eff5000 100644 --- a/it/it-tests/src/test/java/it/settings/SettingsTest.java +++ b/it/it-tests/src/test/java/it/settings/SettingsTest.java @@ -24,6 +24,7 @@ import com.sonar.orchestrator.selenium.Selenese; import it.Category1Suite; import java.io.IOException; import java.util.List; +import javax.annotation.CheckForNull; import org.apache.commons.io.FileUtils; import org.junit.After; import org.junit.BeforeClass; @@ -87,7 +88,7 @@ public class SettingsTest { } @Test - public void set_value() throws Exception { + public void set_setting() throws Exception { SETTINGS.set(SetRequest.builder().setKey(PLUGIN_SETTING_KEY).setValue("some value").build()); String value = getSetting(PLUGIN_SETTING_KEY).getValue(); @@ -95,16 +96,20 @@ public class SettingsTest { } @Test - public void remove_value() throws Exception { + public void remove_setting() throws Exception { SETTINGS.set(SetRequest.builder().setKey(PLUGIN_SETTING_KEY).setValue("some value").build()); - SETTINGS.reset(ResetRequest.builder().setKey(PLUGIN_SETTING_KEY).build()); + SETTINGS.set(SetRequest.builder().setKey("sonar.links.ci").setValue("http://localhost").build()); + + SETTINGS.reset(ResetRequest.builder().setKeys(PLUGIN_SETTING_KEY, "sonar.links.ci").build()); assertThat(getSetting(PLUGIN_SETTING_KEY).getValue()).isEqualTo("aDefaultValue"); + assertThat(getSetting("sonar.links.ci")).isNull(); } + @CheckForNull private Settings.Setting getSetting(String key) { Settings.ValuesWsResponse response = SETTINGS.values(ValuesRequest.builder().setKeys(key).build()); List settings = response.getSettingsList(); - assertThat(settings).hasSize(1); - return settings.get(0); + return settings.isEmpty() ? null : settings.get(0); } + } 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 87dda71655a..7d1c636a5dd 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 @@ -20,6 +20,8 @@ package org.sonar.server.setting.ws; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import org.sonar.api.config.PropertyDefinition; import org.sonar.api.config.PropertyDefinitions; @@ -28,6 +30,7 @@ import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.web.UserRole; import org.sonar.core.permission.GlobalPermissions; +import org.sonar.core.util.stream.Collectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; @@ -39,7 +42,7 @@ import static org.sonar.server.setting.ws.SettingsWsComponentParameters.addCompo import static org.sonarqube.ws.client.ce.CeWsParameters.PARAM_COMPONENT_KEY; import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_RESET; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONENT_ID; -import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEY; +import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEYS; public class ResetAction implements SettingsWsAction { @@ -72,9 +75,9 @@ public class ResetAction implements SettingsWsAction { .setPost(true) .setHandler(this); - action.createParam(PARAM_KEY) - .setDescription("Setting key") - .setExampleValue("sonar.links.scm") + action.createParam(PARAM_KEYS) + .setDescription("Setting keys") + .setExampleValue("sonar.links.scm,sonar.debt.hoursInDay") .setRequired(true); addComponentParameters(action); } @@ -87,12 +90,11 @@ public class ResetAction implements SettingsWsAction { Optional component = getComponent(dbSession, resetRequest); checkPermissions(component); - PropertyDefinition definition = definitions.get(resetRequest.getKey()); - String key = definition != null ? definition.key() : resetRequest.getKey(); + List keys = getKeys(resetRequest); if (component.isPresent()) { - settingsUpdater.deleteComponentSetting(dbSession, key, component.get()); + settingsUpdater.deleteComponentSettings(dbSession, component.get(), keys); } else { - settingsUpdater.deleteGlobalSetting(dbSession, key); + settingsUpdater.deleteGlobalSettings(dbSession, keys); } dbSession.commit(); response.noContent(); @@ -101,9 +103,18 @@ public class ResetAction implements SettingsWsAction { } } + private List getKeys(ResetRequest request) { + return new ArrayList<>(request.getKeys().stream() + .map(key -> { + PropertyDefinition definition = definitions.get(key); + return definition != null ? definition.key() : key; + }) + .collect(Collectors.toSet())); + } + private static ResetRequest toWsRequest(Request request) { return ResetRequest.builder() - .setKey(request.mandatoryParam(PARAM_KEY)) + .setKeys(request.paramAsStrings(PARAM_KEYS)) .setComponentId(request.param(PARAM_COMPONENT_ID)) .setComponentKey(request.param(PARAM_COMPONENT_KEY)) .build(); 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 f99b1e24aaa..5f32087103b 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 @@ -200,9 +200,9 @@ public class SetAction implements SettingsWsAction { private void deleteSettings(DbSession dbSession, Optional component, String key) { if (component.isPresent()) { - settingsUpdater.deleteComponentSetting(dbSession, key, component.get()); + settingsUpdater.deleteComponentSettings(dbSession, component.get(), key); } else { - settingsUpdater.deleteGlobalSetting(dbSession, key); + settingsUpdater.deleteGlobalSettings(dbSession, key); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsUpdater.java index c88893e38c7..45e7be70b0e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsUpdater.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SettingsUpdater.java @@ -20,6 +20,7 @@ package org.sonar.server.setting.ws; +import java.util.List; import java.util.Optional; import java.util.Set; import org.sonar.api.PropertyType; @@ -30,6 +31,8 @@ import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.db.property.PropertyDto; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Arrays.asList; import static org.sonar.server.setting.ws.PropertySetExtractor.extractPropertySetKeys; public class SettingsUpdater { @@ -42,49 +45,61 @@ public class SettingsUpdater { this.definitions = definitions; } - public void deleteGlobalSetting(DbSession dbSession, String propertyKey) { - delete(dbSession, propertyKey, Optional.empty()); + public void deleteGlobalSettings(DbSession dbSession, String... settingKeys) { + deleteGlobalSettings(dbSession, asList(settingKeys)); } - public void deleteComponentSetting(DbSession dbSession, String propertyKey, ComponentDto componentDto) { - delete(dbSession, propertyKey, Optional.of(componentDto)); + public void deleteGlobalSettings(DbSession dbSession, List settingKeys) { + checkArgument(!settingKeys.isEmpty(), "At least one setting key is required"); + settingKeys.forEach(key -> delete(dbSession, key, Optional.empty())); } - private void delete(DbSession dbSession, String propertyKey, Optional componentDto) { - PropertyDefinition definition = definitions.get(propertyKey); + public void deleteComponentSettings(DbSession dbSession, ComponentDto componentDto, String... settingKeys) { + deleteComponentSettings(dbSession, componentDto, asList(settingKeys)); + } + + public void deleteComponentSettings(DbSession dbSession, ComponentDto componentDto, List settingKeys) { + checkArgument(!settingKeys.isEmpty(), "At least one setting key is required"); + for (String propertyKey : settingKeys) { + delete(dbSession, propertyKey, Optional.of(componentDto)); + } + } + + private void delete(DbSession dbSession, String settingKey, Optional componentDto) { + PropertyDefinition definition = definitions.get(settingKey); if (definition == null || !definition.type().equals(PropertyType.PROPERTY_SET)) { - deleteSetting(dbSession, propertyKey, componentDto); + deleteSetting(dbSession, settingKey, componentDto); } else { - deletePropertySet(dbSession, propertyKey, definition, componentDto); + deletePropertySet(dbSession, settingKey, definition, componentDto); } } - private void deleteSetting(DbSession dbSession, String propertyKey, Optional componentDto) { + private void deleteSetting(DbSession dbSession, String settingKey, Optional componentDto) { if (componentDto.isPresent()) { - dbClient.propertiesDao().deleteProjectProperty(propertyKey, componentDto.get().getId(), dbSession); + dbClient.propertiesDao().deleteProjectProperty(settingKey, componentDto.get().getId(), dbSession); } else { - dbClient.propertiesDao().deleteGlobalProperty(propertyKey, dbSession); + dbClient.propertiesDao().deleteGlobalProperty(settingKey, dbSession); } } - private void deletePropertySet(DbSession dbSession, String propertyKey, PropertyDefinition definition, Optional componentDto) { - Optional propertyDto = selectPropertyDto(dbSession, propertyKey, componentDto); + private void deletePropertySet(DbSession dbSession, String settingKey, PropertyDefinition definition, Optional componentDto) { + Optional propertyDto = selectPropertyDto(dbSession, settingKey, componentDto); if (!propertyDto.isPresent()) { // Setting doesn't exist, nothing to do return; } - Set propertySetKeys = extractPropertySetKeys(propertyDto.get(), definition); - for (String key : propertySetKeys) { + Set settingSetKeys = extractPropertySetKeys(propertyDto.get(), definition); + for (String key : settingSetKeys) { deleteSetting(dbSession, key, componentDto); } - deleteSetting(dbSession, propertyKey, componentDto); + deleteSetting(dbSession, settingKey, componentDto); } - private Optional selectPropertyDto(DbSession dbSession, String propertyKey, Optional componentDto) { + private Optional selectPropertyDto(DbSession dbSession, String settingKey, Optional componentDto) { if (componentDto.isPresent()) { - return Optional.ofNullable(dbClient.propertiesDao().selectProjectProperty(dbSession, componentDto.get().getId(), propertyKey)); + return Optional.ofNullable(dbClient.propertiesDao().selectProjectProperty(dbSession, componentDto.get().getId(), settingKey)); } else { - return Optional.ofNullable(dbClient.propertiesDao().selectGlobalProperty(dbSession, propertyKey)); + return Optional.ofNullable(dbClient.propertiesDao().selectGlobalProperty(dbSession, settingKey)); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java index 76f7b9c7ac5..7ba924ec5a1 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/ResetActionTest.java @@ -169,7 +169,7 @@ public class ResetActionTest { public void empty_204_response() { setUserAsSystemAdmin(); TestResponse result = ws.newRequest() - .setParam("key", "my.key") + .setParam("keys", "my.key") .execute(); assertThat(result.getStatus()).isEqualTo(HTTP_NO_CONTENT); @@ -221,7 +221,7 @@ public class ResetActionTest { private void executeRequest(String key, @Nullable String componentId, @Nullable String componentKey) { TestRequest request = ws.newRequest() .setMediaType(MediaTypes.PROTOBUF) - .setParam("key", key); + .setParam("keys", key); if (componentId != null) { request.setParam("componentId", componentId); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsUpdaterTest.java index 372691634cc..db0d0634bd0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsUpdaterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SettingsUpdaterTest.java @@ -23,6 +23,7 @@ package org.sonar.server.setting.ws; import org.junit.Before; 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; @@ -47,6 +48,9 @@ import static org.sonar.db.property.PropertyTesting.newUserPropertyDto; public class SettingsUpdaterTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Rule public DbTester db = DbTester.create(System2.INSTANCE); @@ -67,26 +71,30 @@ public class SettingsUpdaterTest { } @Test - public void delete_global_setting() throws Exception { + public void delete_global_settings() throws Exception { definitions.addComponent(PropertyDefinition.builder("foo").build()); - propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one")); propertyDb.insertProperties(newComponentPropertyDto(project).setKey("foo").setValue("value")); + propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one")); + propertyDb.insertProperties(newGlobalPropertyDto().setKey("bar").setValue("two")); - underTest.deleteGlobalSetting(dbSession, "foo"); + underTest.deleteGlobalSettings(dbSession, "foo", "bar"); assertGlobalPropertyDoesNotExist("foo"); + assertGlobalPropertyDoesNotExist("bar"); assertProjectPropertyExists("foo"); } @Test - public void delete_component_setting() throws Exception { + public void delete_component_settings() throws Exception { definitions.addComponent(PropertyDefinition.builder("foo").build()); - propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one")); - propertyDb.insertProperties(newComponentPropertyDto(project).setKey("foo").setValue("value")); + propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("value")); + propertyDb.insertProperties(newComponentPropertyDto(project).setKey("foo").setValue("one")); + propertyDb.insertProperties(newComponentPropertyDto(project).setKey("bar").setValue("two")); - underTest.deleteComponentSetting(dbSession, "foo", project); + underTest.deleteComponentSettings(dbSession, project, "foo", "bar"); assertProjectPropertyDoesNotExist("foo"); + assertProjectPropertyDoesNotExist("bar"); assertGlobalPropertyExists("foo"); } @@ -94,7 +102,7 @@ public class SettingsUpdaterTest { public void does_not_fail_when_deleting_unknown_setting() throws Exception { propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one")); - underTest.deleteGlobalSetting(dbSession, "unknown"); + underTest.deleteGlobalSettings(dbSession, "unknown"); assertGlobalPropertyExists("foo"); } @@ -105,7 +113,7 @@ public class SettingsUpdaterTest { propertyDb.insertProperties(newUserPropertyDto("foo", "one", user)); propertyDb.insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one")); - underTest.deleteGlobalSetting(dbSession, "foo"); + underTest.deleteGlobalSettings(dbSession, "foo"); assertUserPropertyExists("foo", user); } @@ -125,7 +133,7 @@ public class SettingsUpdaterTest { newGlobalPropertyDto().setKey("foo.1.size").setValue("size1"), newGlobalPropertyDto().setKey("foo.2.key").setValue("key2")); - underTest.deleteGlobalSetting(dbSession, "foo"); + underTest.deleteGlobalSettings(dbSession, "foo"); assertGlobalPropertyDoesNotExist("foo"); assertGlobalPropertyDoesNotExist("foo.1.key"); @@ -148,7 +156,7 @@ public class SettingsUpdaterTest { newComponentPropertyDto(project).setKey("foo.1.size").setValue("size1"), newComponentPropertyDto(project).setKey("foo.2.key").setValue("key2")); - underTest.deleteComponentSetting(dbSession, "foo", project); + underTest.deleteComponentSettings(dbSession, project, "foo"); assertProjectPropertyDoesNotExist("foo"); assertProjectPropertyDoesNotExist("foo.1.key"); @@ -169,11 +177,27 @@ public class SettingsUpdaterTest { newComponentPropertyDto(project).setKey("other").setValue("1,2"), newComponentPropertyDto(project).setKey("other.1.key").setValue("key1")); - underTest.deleteComponentSetting(dbSession, "foo", project); + underTest.deleteComponentSettings(dbSession, project, "foo"); assertProjectPropertyExists("other"); } + @Test + public void fail_to_delete_global_setting_when_no_setting_key() throws Exception { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("At least one setting key is required"); + + underTest.deleteGlobalSettings(dbSession); + } + + @Test + public void fail_to_delete_component_setting_when_no_setting_key() throws Exception { + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("At least one setting key is required"); + + underTest.deleteComponentSettings(dbSession, project); + } + private void assertGlobalPropertyDoesNotExist(String key) { assertThat(dbClient.propertiesDao().selectGlobalProperty(dbSession, key)).isNull(); } diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/ResetRequest.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/ResetRequest.java index 184d9fb9731..e999dde79b9 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/ResetRequest.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/ResetRequest.java @@ -20,24 +20,26 @@ package org.sonarqube.ws.client.setting; +import java.util.List; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Arrays.asList; public class ResetRequest { - private final String key; + private final List keys; private final String componentId; private final String componentKey; public ResetRequest(Builder builder) { - this.key = builder.key; + this.keys = builder.keys; this.componentId = builder.componentId; this.componentKey = builder.componentKey; } - public String getKey() { - return key; + public List getKeys() { + return keys; } @CheckForNull @@ -55,7 +57,7 @@ public class ResetRequest { } public static class Builder { - private String key; + private List keys; private String componentId; private String componentKey; @@ -63,8 +65,13 @@ public class ResetRequest { // enforce factory method use } - public Builder setKey(String key) { - this.key = key; + public Builder setKeys(List keys) { + this.keys = keys; + return this; + } + + public Builder setKeys(String... keys) { + setKeys(asList(keys)); return this; } @@ -79,7 +86,7 @@ public class ResetRequest { } public ResetRequest build() { - checkArgument(key != null && !key.isEmpty(), "Setting key is mandatory and must not be empty."); + checkArgument(keys != null && !keys.isEmpty(), "Setting keys is mandatory and must not be empty."); return new ResetRequest(this); } } diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SettingsService.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SettingsService.java index 388bed8616d..bdc9453aba1 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SettingsService.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/setting/SettingsService.java @@ -71,7 +71,7 @@ public class SettingsService extends BaseService { public void reset(ResetRequest request) { call(new PostRequest(path(ACTION_RESET)) - .setParam(PARAM_KEY, request.getKey()) + .setParam(PARAM_KEYS, inlineMultipleParamValue(request.getKeys())) .setParam(PARAM_COMPONENT_ID, request.getComponentId()) .setParam(PARAM_COMPONENT_KEY, request.getComponentKey())); } diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/ResetRequestTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/ResetRequestTest.java index edf7c333885..994af2a0756 100644 --- a/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/ResetRequestTest.java +++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/ResetRequestTest.java @@ -35,15 +35,15 @@ public class ResetRequestTest { @Test public void create_set_request() { - ResetRequest result = underTest.setKey("my.key").build(); + ResetRequest result = underTest.setKeys("my.key").build(); - assertThat(result.getKey()).isEqualTo("my.key"); + assertThat(result.getKeys()).containsOnly("my.key"); } @Test - public void fail_when_empty_key() { + public void fail_when_empty_keys() { expectedException.expect(IllegalArgumentException.class); - underTest.setKey("").build(); + underTest.setKeys().build(); } } 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 82cfba4d457..1faf51ef43d 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 @@ -98,12 +98,12 @@ public class SettingsServiceTest { @Test public void reset() { underTest.reset(ResetRequest.builder() - .setKey("sonar.debt") + .setKeys("sonar.debt") .setComponentKey("KEY") .build()); serviceTester.assertThat(serviceTester.getPostRequest()) - .hasParam(PARAM_KEY, "sonar.debt") + .hasParam(PARAM_KEYS, "sonar.debt") .hasParam(PARAM_COMPONENT_KEY, "KEY") .andNoOtherParam(); }