From a123fec717a9a463e9e4c243b1953076660735c7 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 25 Aug 2017 09:24:27 +0200 Subject: [PATCH] SONAR-9616 Handle branch in api/settings/set --- .../sonar/server/setting/ws/SetAction.java | 20 ++++- .../server/setting/ws/SetActionTest.java | 79 ++++++++++++++++++- .../ws/client/setting/SetRequest.java | 13 +++ .../ws/client/setting/SettingsService.java | 5 +- .../client/setting/SettingsWsParameters.java | 1 + .../ws/client/setting/SetRequestTest.java | 12 +++ .../client/setting/SettingsServiceTest.java | 3 + 7 files changed, 129 insertions(+), 4 deletions(-) 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 9935159b9d9..910be41d473 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 @@ -21,6 +21,7 @@ package org.sonar.server.setting.ws; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ListMultimap; import com.google.gson.Gson; import com.google.gson.JsonSyntaxException; @@ -56,9 +57,14 @@ import org.sonar.server.setting.ws.SettingValidations.SettingData; import org.sonar.server.user.UserSession; import org.sonarqube.ws.client.setting.SetRequest; +import static com.google.common.base.Preconditions.checkArgument; +import static java.lang.String.format; +import static org.sonar.core.config.CorePropertyDefinitions.LEAK_PERIOD; +import static org.sonar.server.ws.KeyExamples.KEY_BRANCH_EXAMPLE_001; import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001; 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_BRANCH; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONENT; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_FIELD_VALUES; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEY; @@ -68,6 +74,7 @@ import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_VALUES; public class SetAction implements SettingsWsAction { private static final Collector COMMA_JOINER = Collectors.joining(","); private static final String MSG_NO_EMPTY_VALUE = "A non empty value must be provided"; + public static final Set SETTING_ON_BRANCHES = ImmutableSet.of(LEAK_PERIOD); private final PropertyDefinitions propertyDefinitions; private final DbClient dbClient; @@ -124,6 +131,12 @@ public class SetAction implements SettingsWsAction { .setDescription("Component key") .setDeprecatedKey("componentKey", "6.3") .setExampleValue(KEY_PROJECT_EXAMPLE_001); + + action.createParam(PARAM_BRANCH) + .setDescription("Branch key. Only available on following settings : %s", SETTING_ON_BRANCHES.stream().collect(COMMA_JOINER)) + .setExampleValue(KEY_BRANCH_EXAMPLE_001) + .setInternal(true) + .setSince("6.6"); } @Override @@ -190,9 +203,11 @@ public class SetAction implements SettingsWsAction { private void commonChecks(SetRequest request, Optional component) { checkValueIsSet(request); - SettingData settingData = new SettingData(request.getKey(), valuesFromRequest(request), component.orElse(null)); + String settingKey = request.getKey(); + SettingData settingData = new SettingData(settingKey, valuesFromRequest(request), component.orElse(null)); ImmutableList.of(validations.scope(), validations.qualifier(), validations.valueType()) .forEach(validation -> validation.accept(settingData)); + component.map(ComponentDto::getBranch).ifPresent(b -> checkArgument(SETTING_ON_BRANCHES.contains(settingKey), format("Setting '%s' cannot be set on a branch", settingKey))); } private static void validatePropertySet(SetRequest request, @Nullable PropertyDefinition definition) { @@ -278,6 +293,7 @@ public class SetAction implements SettingsWsAction { .setValues(request.multiParam(PARAM_VALUES)) .setFieldValues(request.multiParam(PARAM_FIELD_VALUES)) .setComponent(request.param(PARAM_COMPONENT)) + .setBranch(request.param(PARAM_BRANCH)) .build(); } @@ -297,7 +313,7 @@ public class SetAction implements SettingsWsAction { if (componentKey == null) { return Optional.empty(); } - return Optional.of(componentFinder.getByKey(dbSession, componentKey)); + return Optional.of(componentFinder.getByKeyAndOptionalBranch(dbSession, componentKey, request.getBranch())); } private PropertyDto toProperty(SetRequest request, Optional 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 cb7bb364de7..7f93922865b 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 @@ -396,6 +396,29 @@ public class SetActionTest { assertThat(settingsChangeNotifier.wasCalled).isTrue(); } + @Test + public void set_leak_on_branch() { + ComponentDto project = db.components().insertMainBranch(); + logInAsProjectAdministrator(project); + ComponentDto branch = db.components().insertProjectBranch(project); + String leakKey = "sonar.leak.period"; + definitions.addComponent(PropertyDefinition.builder(leakKey) + .name("Leak") + .description("desc") + .onQualifiers(Qualifiers.PROJECT) + .build()); + propertyDb.insertProperties(newComponentPropertyDto(leakKey, "1", branch)); + + ws.newRequest() + .setParam("key", leakKey) + .setParam("value", "2") + .setParam("component", branch.getKey()) + .setParam("branch", branch.getBranch()) + .execute(); + + assertComponentSetting(leakKey, "2", branch.getId()); + } + @Test public void fail_when_no_key() { expectedException.expect(IllegalArgumentException.class); @@ -914,6 +937,54 @@ public class SetActionTest { callForProjectSettingByKey("my.key", "My Value", branch.getDbKey()); } + @Test + public void fail_when_component_not_found() { + expectedException.expect(NotFoundException.class); + expectedException.expectMessage("Component key 'unknown' not found"); + + ws.newRequest() + .setParam("key", "foo") + .setParam("value", "2") + .setParam("component", "unknown") + .execute(); + } + + @Test + public void fail_when_branch_not_found() { + ComponentDto project = db.components().insertMainBranch(); + logInAsProjectAdministrator(project); + ComponentDto branch = db.components().insertProjectBranch(project); + String settingKey = "not_allowed_on_branch"; + + expectedException.expect(NotFoundException.class); + expectedException.expectMessage(format("Component '%s' on branch 'unknown' not found", branch.getKey())); + + ws.newRequest() + .setParam("key", settingKey) + .setParam("value", "2") + .setParam("component", branch.getKey()) + .setParam("branch", "unknown") + .execute(); + } + + @Test + public void fail_when_setting_not_allowed_setting_on_branch() { + ComponentDto project = db.components().insertMainBranch(); + logInAsProjectAdministrator(project); + ComponentDto branch = db.components().insertProjectBranch(project); + String settingKey = "not_allowed_on_branch"; + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage(format("Setting '%s' cannot be set on a branch", settingKey)); + + ws.newRequest() + .setParam("key", settingKey) + .setParam("value", "2") + .setParam("component", branch.getKey()) + .setParam("branch", branch.getBranch()) + .execute(); + } + @Test public void definition() { WebService.Action definition = ws.getDef(); @@ -923,7 +994,12 @@ public class SetActionTest { assertThat(definition.isInternal()).isFalse(); assertThat(definition.since()).isEqualTo("6.1"); assertThat(definition.params()).extracting(Param::key) - .containsOnly("key", "value", "values", "fieldValues", "component"); + .containsOnly("key", "value", "values", "fieldValues", "component", "branch"); + + Param branch = definition.param("branch"); + assertThat(branch.isInternal()).isTrue(); + assertThat(branch.since()).isEqualTo("6.6"); + assertThat(branch.description()).isEqualTo("Branch key. Only available on following settings : sonar.leak.period"); } private void assertGlobalSetting(String key, String value) { @@ -946,6 +1022,7 @@ public class SetActionTest { PropertyDto result = dbClient.propertiesDao().selectProjectProperty(componentId, key); assertThat(result) + .isNotNull() .extracting(PropertyDto::getKey, PropertyDto::getValue, PropertyDto::getResourceId) .containsExactly(key, value, componentId); } 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 5c56a4fc648..70834435f46 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 @@ -32,6 +32,7 @@ public class SetRequest { private final List values; private final List fieldValues; private final String component; + private final String branch; private SetRequest(Builder builder) { this.key = builder.key; @@ -39,6 +40,7 @@ public class SetRequest { this.values = builder.values; this.fieldValues = builder.fieldValues; this.component = builder.component; + this.branch = builder.branch; } public String getKey() { @@ -63,6 +65,11 @@ public class SetRequest { return component; } + @CheckForNull + public String getBranch() { + return branch; + } + public static Builder builder() { return new Builder(); } @@ -73,6 +80,7 @@ public class SetRequest { private List values = emptyList(); private List fieldValues = emptyList(); private String component; + private String branch; private Builder() { // enforce factory method use @@ -103,6 +111,11 @@ public class SetRequest { return this; } + public Builder setBranch(@Nullable String branch) { + this.branch = branch; + return this; + } + 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"); 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 c29d1ad7c0c..f0ec6e354c1 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 @@ -31,6 +31,7 @@ import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_RESET; import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_SET; import static org.sonarqube.ws.client.setting.SettingsWsParameters.ACTION_VALUES; import static org.sonarqube.ws.client.setting.SettingsWsParameters.CONTROLLER_SETTINGS; +import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_BRANCH; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONENT; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_FIELD_VALUES; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEY; @@ -62,7 +63,9 @@ public class SettingsService extends BaseService { .setParam(PARAM_VALUE, request.getValue()) .setParam(PARAM_VALUES, request.getValues()) .setParam(PARAM_FIELD_VALUES, request.getFieldValues()) - .setParam(PARAM_COMPONENT, request.getComponent())); + .setParam(PARAM_COMPONENT, request.getComponent()) + .setParam(PARAM_BRANCH, request.getBranch()) + ); } public void reset(ResetRequest request) { 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 528cd344fef..a4ccc5c8926 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 @@ -28,6 +28,7 @@ public class SettingsWsParameters { public static final String ACTION_RESET = "reset"; public static final String PARAM_COMPONENT = "component"; + public static final String PARAM_BRANCH = "branch"; public static final String PARAM_KEYS = "keys"; public static final String PARAM_KEY = "key"; public static final String PARAM_VALUE = "value"; diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/SetRequestTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/SetRequestTest.java index e5dda634f34..fb2a2658ef8 100644 --- a/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/SetRequestTest.java +++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/setting/SetRequestTest.java @@ -40,6 +40,7 @@ public class SetRequestTest { assertThat(result.getValue()).isEqualTo("my value"); assertThat(result.getValues()).isNotNull().isEmpty(); assertThat(result.getComponent()).isNull(); + assertThat(result.getBranch()).isNull(); } @Test @@ -49,6 +50,17 @@ public class SetRequestTest { assertThat(result.getKey()).isEqualTo("my.key"); assertThat(result.getValue()).isEqualTo("my value"); assertThat(result.getComponent()).isEqualTo("projectKey"); + assertThat(result.getBranch()).isNull(); + } + + @Test + public void create_request_with_component_and_branch() { + SetRequest result = underTest.setKey("my.key").setValue("my value").setComponent("projectKey").setBranch("my_branch").build(); + + assertThat(result.getKey()).isEqualTo("my.key"); + assertThat(result.getValue()).isEqualTo("my value"); + assertThat(result.getComponent()).isEqualTo("projectKey"); + assertThat(result.getBranch()).isEqualTo("my_branch"); } @Test 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 854a07d5972..d0b84d18afa 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 com.google.common.collect.Lists.newArrayList; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_BRANCH; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_COMPONENT; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_FIELD_VALUES; import static org.sonarqube.ws.client.setting.SettingsWsParameters.PARAM_KEY; @@ -80,6 +81,7 @@ public class SettingsServiceTest { .setValues(newArrayList("v1", "v2", "v3")) .setFieldValues(newArrayList("json1", "json2", "json3")) .setComponent("KEY") + .setBranch("BRANCH") .build()); serviceTester.assertThat(serviceTester.getPostRequest()) @@ -88,6 +90,7 @@ public class SettingsServiceTest { .hasParam(PARAM_VALUES, newArrayList("v1", "v2", "v3")) .hasParam(PARAM_FIELD_VALUES, newArrayList("json1", "json2", "json3")) .hasParam(PARAM_COMPONENT, "KEY") + .hasParam(PARAM_BRANCH, "BRANCH") .andNoOtherParam(); } -- 2.39.5