Skip to content

Commit

Permalink
SONAR-7969 Fail when using key and deprecated in values WS
Browse files Browse the repository at this point in the history 
Display specific message when key and deprecated key are used at the same time
  • Loading branch information
@julienlancelot
julienlancelot committed Aug 26, 2016
1 parent aa5f425 commit 8de13ae
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 32 deletions.
6 changes: 5 additions & 1 deletion server/sonar-server/src/main/java/org/sonar/server/settings/ws/ValuesAction.java
View file
Expand Up @@ -47,6 +47,7 @@
import org.sonarqube.ws.Settings.ValuesWsResponse; import org.sonarqube.ws.Settings.ValuesWsResponse;
import org.sonarqube.ws.client.setting.ValuesRequest; import org.sonarqube.ws.client.setting.ValuesRequest;


import static java.lang.String.format;
import static org.elasticsearch.common.Strings.isNullOrEmpty; import static org.elasticsearch.common.Strings.isNullOrEmpty;
import static org.sonar.api.PropertyType.PROPERTY_SET; import static org.sonar.api.PropertyType.PROPERTY_SET;
import static org.sonar.server.component.ComponentFinder.ParamNames.ID_AND_KEY; import static org.sonar.server.component.ComponentFinder.ParamNames.ID_AND_KEY;
Expand Down Expand Up @@ -165,7 +166,10 @@ private List<Setting> loadDefaultSettings(Set<String> keys) {


private Map<String, String> getKeysToDisplayMap(Set<String> keys) { private Map<String, String> getKeysToDisplayMap(Set<String> keys) {
return keys.stream() return keys.stream()
.collect(Collectors.toMap(propertyDefinitions::validKey, Function.identity())); .collect(Collectors.toMap(propertyDefinitions::validKey, Function.identity(),
(u, v) -> {
throw new IllegalArgumentException(format("'%s' and '%s' cannot be used at the same time as they refer to the same setting", u, v));
}));
} }


private static class ValuesResponseBuilder { private static class ValuesResponseBuilder {
Expand Down
87 changes: 56 additions & 31 deletions server/sonar-server/src/test/java/org/sonar/server/settings/ws/ValuesActionTest.java
View file
Expand Up @@ -98,9 +98,9 @@ public void return_simple_value() throws Exception {
.build()); .build());
insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one")); insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one"));


ValuesWsResponse result = newRequestForGlobalProperties("foo"); ValuesWsResponse result = executeRequestForGlobalProperties("foo");
assertThat(result.getSettingsList()).hasSize(1);


assertThat(result.getSettingsList()).hasSize(1);
Settings.Setting value = result.getSettings(0); Settings.Setting value = result.getSettings(0);
assertThat(value.getKey()).isEqualTo("foo"); assertThat(value.getKey()).isEqualTo("foo");
assertThat(value.getValue()).isEqualTo("one"); assertThat(value.getValue()).isEqualTo("one");
Expand All @@ -125,7 +125,7 @@ public void return_multi_values() throws Exception {
.build()); .build());
insertProperties(newGlobalPropertyDto().setKey("global").setValue("three,four")); insertProperties(newGlobalPropertyDto().setKey("global").setValue("three,four"));


ValuesWsResponse result = newRequestForGlobalProperties("default", "global"); ValuesWsResponse result = executeRequestForGlobalProperties("default", "global");
assertThat(result.getSettingsList()).hasSize(2); assertThat(result.getSettingsList()).hasSize(2);


Settings.Setting foo = result.getSettings(0); Settings.Setting foo = result.getSettings(0);
Expand Down Expand Up @@ -157,9 +157,9 @@ public void return_property_set() throws Exception {
newGlobalPropertyDto().setKey("foo.1.size").setValue("size1"), newGlobalPropertyDto().setKey("foo.1.size").setValue("size1"),
newGlobalPropertyDto().setKey("foo.2.key").setValue("key2")); newGlobalPropertyDto().setKey("foo.2.key").setValue("key2"));


ValuesWsResponse result = newRequestForGlobalProperties("foo"); ValuesWsResponse result = executeRequestForGlobalProperties("foo");
assertThat(result.getSettingsList()).hasSize(1);


assertThat(result.getSettingsList()).hasSize(1);
Settings.Setting value = result.getSettings(0); Settings.Setting value = result.getSettings(0);
assertThat(value.getKey()).isEqualTo("foo"); assertThat(value.getKey()).isEqualTo("foo");
assertThat(value.hasValue()).isFalse(); assertThat(value.hasValue()).isFalse();
Expand All @@ -186,9 +186,9 @@ public void return_property_set_for_component() throws Exception {
newComponentPropertyDto(project).setKey("foo.1.size").setValue("size1"), newComponentPropertyDto(project).setKey("foo.1.size").setValue("size1"),
newComponentPropertyDto(project).setKey("foo.2.key").setValue("key2")); newComponentPropertyDto(project).setKey("foo.2.key").setValue("key2"));


ValuesWsResponse result = newRequestForProjectProperties("foo"); ValuesWsResponse result = executeRequestForProjectProperties("foo");
assertThat(result.getSettingsList()).hasSize(1);


assertThat(result.getSettingsList()).hasSize(1);
Settings.Setting value = result.getSettings(0); Settings.Setting value = result.getSettings(0);
assertThat(value.getKey()).isEqualTo("foo"); assertThat(value.getKey()).isEqualTo("foo");
assertThat(value.hasValue()).isFalse(); assertThat(value.hasValue()).isFalse();
Expand All @@ -207,7 +207,8 @@ public void return_default_values() throws Exception {
.defaultValue("default") .defaultValue("default")
.build()); .build());


ValuesWsResponse result = newRequestForGlobalProperties("foo"); ValuesWsResponse result = executeRequestForGlobalProperties("foo");

assertThat(result.getSettingsList()).hasSize(1); assertThat(result.getSettingsList()).hasSize(1);
assertSetting(result.getSettings(0), "foo", "default", true, false); assertSetting(result.getSettings(0), "foo", "default", true, false);
} }
Expand All @@ -220,7 +221,8 @@ public void return_global_values() throws Exception {
// The property is overriding default value // The property is overriding default value
newGlobalPropertyDto().setKey("property").setValue("one")); newGlobalPropertyDto().setKey("property").setValue("one"));


ValuesWsResponse result = newRequestForGlobalProperties("property"); ValuesWsResponse result = executeRequestForGlobalProperties("property");

assertThat(result.getSettingsList()).hasSize(1); assertThat(result.getSettingsList()).hasSize(1);
assertSetting(result.getSettings(0), "property", "one", false, false); assertSetting(result.getSettings(0), "property", "one", false, false);
} }
Expand All @@ -234,7 +236,8 @@ public void return_project_values() throws Exception {
// The property is overriding global value // The property is overriding global value
newComponentPropertyDto(project).setKey("property").setValue("two")); newComponentPropertyDto(project).setKey("property").setValue("two"));


ValuesWsResponse result = newRequestForProjectProperties("property"); ValuesWsResponse result = executeRequestForProjectProperties("property");

assertThat(result.getSettingsList()).hasSize(1); assertThat(result.getSettingsList()).hasSize(1);
assertSetting(result.getSettings(0), "property", "two", false, false); assertSetting(result.getSettings(0), "property", "two", false, false);
} }
Expand All @@ -246,7 +249,8 @@ public void return_is_inherited_to_true_when_property_is_defined_only_at_global_
// The property is not defined on project // The property is not defined on project
insertProperties(newGlobalPropertyDto().setKey("property").setValue("one")); insertProperties(newGlobalPropertyDto().setKey("property").setValue("one"));


ValuesWsResponse result = newRequestForProjectProperties("property"); ValuesWsResponse result = executeRequestForProjectProperties("property");

assertThat(result.getSettingsList()).hasSize(1); assertThat(result.getSettingsList()).hasSize(1);
assertSetting(result.getSettings(0), "property", "one", false, true); assertSetting(result.getSettings(0), "property", "one", false, true);
} }
Expand All @@ -256,7 +260,8 @@ public void return_values_even_if_no_property_definition() throws Exception {
setUserAsSystemAdmin(); setUserAsSystemAdmin();
insertProperties(newGlobalPropertyDto().setKey("globalPropertyWithoutDefinition").setValue("value")); insertProperties(newGlobalPropertyDto().setKey("globalPropertyWithoutDefinition").setValue("value"));


ValuesWsResponse result = newRequestForGlobalProperties("globalPropertyWithoutDefinition"); ValuesWsResponse result = executeRequestForGlobalProperties("globalPropertyWithoutDefinition");

Settings.Setting globalPropertyWithoutDefinitionValue = result.getSettings(0); Settings.Setting globalPropertyWithoutDefinitionValue = result.getSettings(0);
assertThat(globalPropertyWithoutDefinitionValue.getKey()).isEqualTo("globalPropertyWithoutDefinition"); assertThat(globalPropertyWithoutDefinitionValue.getKey()).isEqualTo("globalPropertyWithoutDefinition");
assertThat(globalPropertyWithoutDefinitionValue.getValue()).isEqualTo("value"); assertThat(globalPropertyWithoutDefinitionValue.getValue()).isEqualTo("value");
Expand All @@ -270,7 +275,8 @@ public void return_empty_when_property_def_exists_but_no_value() throws Exceptio
.builder("foo") .builder("foo")
.build()); .build());


ValuesWsResponse result = newRequestForGlobalProperties("foo"); ValuesWsResponse result = executeRequestForGlobalProperties("foo");

assertThat(result.getSettingsList()).isEmpty(); assertThat(result.getSettingsList()).isEmpty();
} }


Expand All @@ -283,22 +289,23 @@ public void does_return_nothing_when_unknown_keys() throws Exception {
.build()); .build());
insertProperties(newGlobalPropertyDto().setKey("bar").setValue("")); insertProperties(newGlobalPropertyDto().setKey("bar").setValue(""));


ValuesWsResponse result = newRequestForGlobalProperties("unknown"); ValuesWsResponse result = executeRequestForGlobalProperties("unknown");

assertThat(result.getSettingsList()).isEmpty(); assertThat(result.getSettingsList()).isEmpty();
} }


@Test @Test
public void return_module_values() throws Exception { public void return_module_values() throws Exception {
setUserAsSystemAdmin(); setUserAsSystemAdmin();
ComponentDto module = componentDb.insertComponent(newModuleDto(project)); ComponentDto module = componentDb.insertComponent(newModuleDto(project));

propertyDefinitions.addComponent(PropertyDefinition.builder("property").defaultValue("default").build()); propertyDefinitions.addComponent(PropertyDefinition.builder("property").defaultValue("default").build());
insertProperties( insertProperties(
newGlobalPropertyDto().setKey("property").setValue("one"), newGlobalPropertyDto().setKey("property").setValue("one"),
// The property is overriding global value // The property is overriding global value
newComponentPropertyDto(module).setKey("property").setValue("two")); newComponentPropertyDto(module).setKey("property").setValue("two"));


ValuesWsResponse result = newRequestForComponentProperties(module, "property"); ValuesWsResponse result = executeRequestForComponentProperties(module, "property");

assertThat(result.getSettingsList()).hasSize(1); assertThat(result.getSettingsList()).hasSize(1);
assertSetting(result.getSettings(0), "property", "two", false, false); assertSetting(result.getSettings(0), "property", "two", false, false);
} }
Expand All @@ -308,7 +315,6 @@ public void return_inherited_values_on_sub_module() throws Exception {
setUserAsSystemAdmin(); setUserAsSystemAdmin();
ComponentDto module = componentDb.insertComponent(newModuleDto(project)); ComponentDto module = componentDb.insertComponent(newModuleDto(project));
ComponentDto subModule = componentDb.insertComponent(newModuleDto(module)); ComponentDto subModule = componentDb.insertComponent(newModuleDto(module));

propertyDefinitions.addComponents(asList( propertyDefinitions.addComponents(asList(
PropertyDefinition.builder("defaultProperty").defaultValue("default").build(), PropertyDefinition.builder("defaultProperty").defaultValue("default").build(),
PropertyDefinition.builder("globalProperty").build(), PropertyDefinition.builder("globalProperty").build(),
Expand All @@ -319,7 +325,8 @@ public void return_inherited_values_on_sub_module() throws Exception {
newComponentPropertyDto(project).setKey("projectProperty").setValue("project"), newComponentPropertyDto(project).setKey("projectProperty").setValue("project"),
newComponentPropertyDto(module).setKey("moduleProperty").setValue("module")); newComponentPropertyDto(module).setKey("moduleProperty").setValue("module"));


ValuesWsResponse result = newRequestForComponentProperties(subModule, "defaultProperty", "globalProperty", "projectProperty", "moduleProperty"); ValuesWsResponse result = executeRequestForComponentProperties(subModule, "defaultProperty", "globalProperty", "projectProperty", "moduleProperty");

assertThat(result.getSettingsList()).hasSize(4); assertThat(result.getSettingsList()).hasSize(4);
assertSetting(result.getSettings(0), "defaultProperty", "default", true, false); assertSetting(result.getSettings(0), "defaultProperty", "default", true, false);
assertSetting(result.getSettings(1), "globalProperty", "global", false, true); assertSetting(result.getSettings(1), "globalProperty", "global", false, true);
Expand All @@ -336,9 +343,9 @@ public void return_value_of_deprecated_key() throws Exception {
.build()); .build());
insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one")); insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one"));


ValuesWsResponse result = newRequestForGlobalProperties("deprecated"); ValuesWsResponse result = executeRequestForGlobalProperties("deprecated");
assertThat(result.getSettingsList()).hasSize(1);


assertThat(result.getSettingsList()).hasSize(1);
Settings.Setting value = result.getSettings(0); Settings.Setting value = result.getSettings(0);
assertThat(value.getKey()).isEqualTo("deprecated"); assertThat(value.getKey()).isEqualTo("deprecated");
assertThat(value.getValue()).isEqualTo("one"); assertThat(value.getValue()).isEqualTo("one");
Expand All @@ -356,7 +363,6 @@ public void test_example_json_response() {
.multiValues(true) .multiValues(true)
.build()); .build());
insertProperties(newGlobalPropertyDto().setKey("sonar.autogenerated").setValue("val1,val2,val3")); insertProperties(newGlobalPropertyDto().setKey("sonar.autogenerated").setValue("val1,val2,val3"));

propertyDefinitions.addComponent(PropertyDefinition propertyDefinitions.addComponent(PropertyDefinition
.builder("sonar.demo") .builder("sonar.demo")
.type(PropertyType.PROPERTY_SET) .type(PropertyType.PROPERTY_SET)
Expand All @@ -375,6 +381,7 @@ public void test_example_json_response() {
.setMediaType(JSON) .setMediaType(JSON)
.execute() .execute()
.getInput(); .getInput();

JsonAssert.assertJson(ws.getDef().responseExampleAsString()).isSimilarTo(result); JsonAssert.assertJson(ws.getDef().responseExampleAsString()).isSimilarTo(result);
} }


Expand All @@ -383,7 +390,8 @@ public void fail_when_id_and_key_are_set() throws Exception {
setUserAsProjectAdmin(); setUserAsProjectAdmin();


expectedException.expect(IllegalArgumentException.class); expectedException.expect(IllegalArgumentException.class);
newRequest(project.uuid(), project.key());
executeRequest(project.uuid(), project.key());
} }


@Test @Test
Expand All @@ -392,7 +400,8 @@ public void fail_when_not_system_admin() throws Exception {
propertyDefinitions.addComponent(PropertyDefinition.builder("foo").build()); propertyDefinitions.addComponent(PropertyDefinition.builder("foo").build());


expectedException.expect(ForbiddenException.class); expectedException.expect(ForbiddenException.class);
newRequestForGlobalProperties("foo");
executeRequestForGlobalProperties("foo");
} }


@Test @Test
Expand All @@ -401,7 +410,23 @@ public void fail_when_not_project_admin() throws Exception {
propertyDefinitions.addComponent(PropertyDefinition.builder("foo").build()); propertyDefinitions.addComponent(PropertyDefinition.builder("foo").build());


expectedException.expect(ForbiddenException.class); expectedException.expect(ForbiddenException.class);
newRequest(project.uuid(), null, "foo");
executeRequest(project.uuid(), null, "foo");
}

@Test
public void fail_when_deprecated_key_and_new_key_are_used() throws Exception {
setUserAsSystemAdmin();
propertyDefinitions.addComponent(PropertyDefinition
.builder("foo")
.deprecatedKey("deprecated")
.build());
insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one"));

expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage("'foo' and 'deprecated' cannot be used at the same time as they refer to the same setting");

executeRequestForGlobalProperties("foo", "deprecated");
} }


@Test @Test
Expand All @@ -414,19 +439,19 @@ public void test_ws_definition() {
assertThat(action.params()).hasSize(3); assertThat(action.params()).hasSize(3);
} }


private ValuesWsResponse newRequestForComponentProperties(ComponentDto componentDto, String... keys) { private ValuesWsResponse executeRequestForComponentProperties(ComponentDto componentDto, String... keys) {
return newRequest(componentDto.uuid(), null, keys); return executeRequest(componentDto.uuid(), null, keys);
} }


private ValuesWsResponse newRequestForProjectProperties(String... keys) { private ValuesWsResponse executeRequestForProjectProperties(String... keys) {
return newRequest(project.uuid(), null, keys); return executeRequest(project.uuid(), null, keys);
} }


private ValuesWsResponse newRequestForGlobalProperties(String... keys) { private ValuesWsResponse executeRequestForGlobalProperties(String... keys) {
return newRequest(null, null, keys); return executeRequest(null, null, keys);
} }


private ValuesWsResponse newRequest(@Nullable String id, @Nullable String key, String... keys) { private ValuesWsResponse executeRequest(@Nullable String id, @Nullable String key, String... keys) {
TestRequest request = ws.newRequest() TestRequest request = ws.newRequest()
.setMediaType(MediaTypes.PROTOBUF) .setMediaType(MediaTypes.PROTOBUF)
.setParam("keys", COMMA_JOINER.join(keys)); .setParam("keys", COMMA_JOINER.join(keys));
Expand Down

0 comments on commit 8de13ae

Please sign in to comment.