From 8de13aed2d978699d7f8d379f6af8610c173c29e Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 26 Aug 2016 13:42:32 +0200 Subject: [PATCH] SONAR-7969 Fail when using key and deprecated in values WS Display specific message when key and deprecated key are used at the same time --- .../server/settings/ws/ValuesAction.java | 6 +- .../server/settings/ws/ValuesActionTest.java | 87 ++++++++++++------- 2 files changed, 61 insertions(+), 32 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/settings/ws/ValuesAction.java b/server/sonar-server/src/main/java/org/sonar/server/settings/ws/ValuesAction.java index a6303dc05985..a82fe4db3e9d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/settings/ws/ValuesAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/settings/ws/ValuesAction.java @@ -47,6 +47,7 @@ import org.sonarqube.ws.Settings.ValuesWsResponse; import org.sonarqube.ws.client.setting.ValuesRequest; +import static java.lang.String.format; import static org.elasticsearch.common.Strings.isNullOrEmpty; import static org.sonar.api.PropertyType.PROPERTY_SET; import static org.sonar.server.component.ComponentFinder.ParamNames.ID_AND_KEY; @@ -165,7 +166,10 @@ private List loadDefaultSettings(Set keys) { private Map getKeysToDisplayMap(Set keys) { 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 { diff --git a/server/sonar-server/src/test/java/org/sonar/server/settings/ws/ValuesActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/settings/ws/ValuesActionTest.java index 081d5adaa96b..878af1640597 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/settings/ws/ValuesActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/settings/ws/ValuesActionTest.java @@ -98,9 +98,9 @@ public void return_simple_value() throws Exception { .build()); insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one")); - ValuesWsResponse result = newRequestForGlobalProperties("foo"); - assertThat(result.getSettingsList()).hasSize(1); + ValuesWsResponse result = executeRequestForGlobalProperties("foo"); + assertThat(result.getSettingsList()).hasSize(1); Settings.Setting value = result.getSettings(0); assertThat(value.getKey()).isEqualTo("foo"); assertThat(value.getValue()).isEqualTo("one"); @@ -125,7 +125,7 @@ public void return_multi_values() throws Exception { .build()); insertProperties(newGlobalPropertyDto().setKey("global").setValue("three,four")); - ValuesWsResponse result = newRequestForGlobalProperties("default", "global"); + ValuesWsResponse result = executeRequestForGlobalProperties("default", "global"); assertThat(result.getSettingsList()).hasSize(2); Settings.Setting foo = result.getSettings(0); @@ -157,9 +157,9 @@ public void return_property_set() throws Exception { newGlobalPropertyDto().setKey("foo.1.size").setValue("size1"), newGlobalPropertyDto().setKey("foo.2.key").setValue("key2")); - ValuesWsResponse result = newRequestForGlobalProperties("foo"); - assertThat(result.getSettingsList()).hasSize(1); + ValuesWsResponse result = executeRequestForGlobalProperties("foo"); + assertThat(result.getSettingsList()).hasSize(1); Settings.Setting value = result.getSettings(0); assertThat(value.getKey()).isEqualTo("foo"); assertThat(value.hasValue()).isFalse(); @@ -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.2.key").setValue("key2")); - ValuesWsResponse result = newRequestForProjectProperties("foo"); - assertThat(result.getSettingsList()).hasSize(1); + ValuesWsResponse result = executeRequestForProjectProperties("foo"); + assertThat(result.getSettingsList()).hasSize(1); Settings.Setting value = result.getSettings(0); assertThat(value.getKey()).isEqualTo("foo"); assertThat(value.hasValue()).isFalse(); @@ -207,7 +207,8 @@ public void return_default_values() throws Exception { .defaultValue("default") .build()); - ValuesWsResponse result = newRequestForGlobalProperties("foo"); + ValuesWsResponse result = executeRequestForGlobalProperties("foo"); + assertThat(result.getSettingsList()).hasSize(1); assertSetting(result.getSettings(0), "foo", "default", true, false); } @@ -220,7 +221,8 @@ public void return_global_values() throws Exception { // The property is overriding default value newGlobalPropertyDto().setKey("property").setValue("one")); - ValuesWsResponse result = newRequestForGlobalProperties("property"); + ValuesWsResponse result = executeRequestForGlobalProperties("property"); + assertThat(result.getSettingsList()).hasSize(1); assertSetting(result.getSettings(0), "property", "one", false, false); } @@ -234,7 +236,8 @@ public void return_project_values() throws Exception { // The property is overriding global value newComponentPropertyDto(project).setKey("property").setValue("two")); - ValuesWsResponse result = newRequestForProjectProperties("property"); + ValuesWsResponse result = executeRequestForProjectProperties("property"); + assertThat(result.getSettingsList()).hasSize(1); assertSetting(result.getSettings(0), "property", "two", false, false); } @@ -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 insertProperties(newGlobalPropertyDto().setKey("property").setValue("one")); - ValuesWsResponse result = newRequestForProjectProperties("property"); + ValuesWsResponse result = executeRequestForProjectProperties("property"); + assertThat(result.getSettingsList()).hasSize(1); assertSetting(result.getSettings(0), "property", "one", false, true); } @@ -256,7 +260,8 @@ public void return_values_even_if_no_property_definition() throws Exception { setUserAsSystemAdmin(); insertProperties(newGlobalPropertyDto().setKey("globalPropertyWithoutDefinition").setValue("value")); - ValuesWsResponse result = newRequestForGlobalProperties("globalPropertyWithoutDefinition"); + ValuesWsResponse result = executeRequestForGlobalProperties("globalPropertyWithoutDefinition"); + Settings.Setting globalPropertyWithoutDefinitionValue = result.getSettings(0); assertThat(globalPropertyWithoutDefinitionValue.getKey()).isEqualTo("globalPropertyWithoutDefinition"); assertThat(globalPropertyWithoutDefinitionValue.getValue()).isEqualTo("value"); @@ -270,7 +275,8 @@ public void return_empty_when_property_def_exists_but_no_value() throws Exceptio .builder("foo") .build()); - ValuesWsResponse result = newRequestForGlobalProperties("foo"); + ValuesWsResponse result = executeRequestForGlobalProperties("foo"); + assertThat(result.getSettingsList()).isEmpty(); } @@ -283,7 +289,8 @@ public void does_return_nothing_when_unknown_keys() throws Exception { .build()); insertProperties(newGlobalPropertyDto().setKey("bar").setValue("")); - ValuesWsResponse result = newRequestForGlobalProperties("unknown"); + ValuesWsResponse result = executeRequestForGlobalProperties("unknown"); + assertThat(result.getSettingsList()).isEmpty(); } @@ -291,14 +298,14 @@ public void does_return_nothing_when_unknown_keys() throws Exception { public void return_module_values() throws Exception { setUserAsSystemAdmin(); ComponentDto module = componentDb.insertComponent(newModuleDto(project)); - propertyDefinitions.addComponent(PropertyDefinition.builder("property").defaultValue("default").build()); insertProperties( newGlobalPropertyDto().setKey("property").setValue("one"), // The property is overriding global value newComponentPropertyDto(module).setKey("property").setValue("two")); - ValuesWsResponse result = newRequestForComponentProperties(module, "property"); + ValuesWsResponse result = executeRequestForComponentProperties(module, "property"); + assertThat(result.getSettingsList()).hasSize(1); assertSetting(result.getSettings(0), "property", "two", false, false); } @@ -308,7 +315,6 @@ public void return_inherited_values_on_sub_module() throws Exception { setUserAsSystemAdmin(); ComponentDto module = componentDb.insertComponent(newModuleDto(project)); ComponentDto subModule = componentDb.insertComponent(newModuleDto(module)); - propertyDefinitions.addComponents(asList( PropertyDefinition.builder("defaultProperty").defaultValue("default").build(), PropertyDefinition.builder("globalProperty").build(), @@ -319,7 +325,8 @@ public void return_inherited_values_on_sub_module() throws Exception { newComponentPropertyDto(project).setKey("projectProperty").setValue("project"), 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); assertSetting(result.getSettings(0), "defaultProperty", "default", true, false); assertSetting(result.getSettings(1), "globalProperty", "global", false, true); @@ -336,9 +343,9 @@ public void return_value_of_deprecated_key() throws Exception { .build()); insertProperties(newGlobalPropertyDto().setKey("foo").setValue("one")); - ValuesWsResponse result = newRequestForGlobalProperties("deprecated"); - assertThat(result.getSettingsList()).hasSize(1); + ValuesWsResponse result = executeRequestForGlobalProperties("deprecated"); + assertThat(result.getSettingsList()).hasSize(1); Settings.Setting value = result.getSettings(0); assertThat(value.getKey()).isEqualTo("deprecated"); assertThat(value.getValue()).isEqualTo("one"); @@ -356,7 +363,6 @@ public void test_example_json_response() { .multiValues(true) .build()); insertProperties(newGlobalPropertyDto().setKey("sonar.autogenerated").setValue("val1,val2,val3")); - propertyDefinitions.addComponent(PropertyDefinition .builder("sonar.demo") .type(PropertyType.PROPERTY_SET) @@ -375,6 +381,7 @@ public void test_example_json_response() { .setMediaType(JSON) .execute() .getInput(); + JsonAssert.assertJson(ws.getDef().responseExampleAsString()).isSimilarTo(result); } @@ -383,7 +390,8 @@ public void fail_when_id_and_key_are_set() throws Exception { setUserAsProjectAdmin(); expectedException.expect(IllegalArgumentException.class); - newRequest(project.uuid(), project.key()); + + executeRequest(project.uuid(), project.key()); } @Test @@ -392,7 +400,8 @@ public void fail_when_not_system_admin() throws Exception { propertyDefinitions.addComponent(PropertyDefinition.builder("foo").build()); expectedException.expect(ForbiddenException.class); - newRequestForGlobalProperties("foo"); + + executeRequestForGlobalProperties("foo"); } @Test @@ -401,7 +410,23 @@ public void fail_when_not_project_admin() throws Exception { propertyDefinitions.addComponent(PropertyDefinition.builder("foo").build()); 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 @@ -414,19 +439,19 @@ public void test_ws_definition() { assertThat(action.params()).hasSize(3); } - private ValuesWsResponse newRequestForComponentProperties(ComponentDto componentDto, String... keys) { - return newRequest(componentDto.uuid(), null, keys); + private ValuesWsResponse executeRequestForComponentProperties(ComponentDto componentDto, String... keys) { + return executeRequest(componentDto.uuid(), null, keys); } - private ValuesWsResponse newRequestForProjectProperties(String... keys) { - return newRequest(project.uuid(), null, keys); + private ValuesWsResponse executeRequestForProjectProperties(String... keys) { + return executeRequest(project.uuid(), null, keys); } - private ValuesWsResponse newRequestForGlobalProperties(String... keys) { - return newRequest(null, null, keys); + private ValuesWsResponse executeRequestForGlobalProperties(String... 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() .setMediaType(MediaTypes.PROTOBUF) .setParam("keys", COMMA_JOINER.join(keys));