From 655f9a3cd21557e7656de1c440cb2ad11895ef99 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 16 Jan 2018 16:04:19 +0100 Subject: [PATCH] SONAR-10288 Configuration supports empty fields and quotes in CE/Web --- .../server/config/ConfigurationProvider.java | 15 +- .../config/ConfigurationProviderTest.java | 336 ++++++++++++++++++ .../sonar/core/config/MultivalueProperty.java | 5 +- 3 files changed, 354 insertions(+), 2 deletions(-) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/config/ConfigurationProviderTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/config/ConfigurationProvider.java b/server/sonar-server/src/main/java/org/sonar/server/config/ConfigurationProvider.java index f91bcdfb38e..2094cea2ecc 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/config/ConfigurationProvider.java +++ b/server/sonar-server/src/main/java/org/sonar/server/config/ConfigurationProvider.java @@ -20,10 +20,16 @@ package org.sonar.server.config; import java.util.Optional; +import java.util.function.Function; +import org.apache.commons.lang.ArrayUtils; import org.picocontainer.injectors.ProviderAdapter; import org.sonar.api.config.Configuration; +import org.sonar.api.config.PropertyDefinition; import org.sonar.api.config.Settings; +import static java.util.function.Function.identity; +import static org.sonar.core.config.MultivalueProperty.parseAsCsv; + public class ConfigurationProvider extends ProviderAdapter { private Configuration configuration; @@ -36,6 +42,8 @@ public class ConfigurationProvider extends ProviderAdapter { } private static class ServerConfigurationAdapter implements Configuration { + private static final Function REPLACE_ENCODED_COMMAS = value -> value.replace("%2C", ","); + private final Settings settings; private ServerConfigurationAdapter(Settings settings) { @@ -54,7 +62,12 @@ public class ConfigurationProvider extends ProviderAdapter { @Override public String[] getStringArray(String key) { - return settings.getStringArray(key); + boolean multiValue = settings.getDefinition(key) + .map(PropertyDefinition::multiValues) + .orElse(false); + return get(key) + .map(v -> parseAsCsv(key, v, multiValue ? REPLACE_ENCODED_COMMAS : identity())) + .orElse(ArrayUtils.EMPTY_STRING_ARRAY); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/config/ConfigurationProviderTest.java b/server/sonar-server/src/test/java/org/sonar/server/config/ConfigurationProviderTest.java new file mode 100644 index 00000000000..72e6e4c791a --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/config/ConfigurationProviderTest.java @@ -0,0 +1,336 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.config; + +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; +import org.apache.commons.lang.RandomStringUtils; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.sonar.api.config.Configuration; +import org.sonar.api.config.PropertyDefinitions; +import org.sonar.api.config.internal.MapSettings; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.api.config.PropertyDefinition.builder; + +@RunWith(DataProviderRunner.class) +public class ConfigurationProviderTest { + private static final String[] EMPTY_STRING_ARRAY = {}; + private final String nonDeclaredKey = RandomStringUtils.randomAlphabetic(3); + private final String nonMultivalueKey = RandomStringUtils.randomAlphabetic(3); + private final String multivalueKey = RandomStringUtils.randomAlphabetic(3); + private final MapSettings settings = new MapSettings(new PropertyDefinitions( + builder(nonMultivalueKey).multiValues(false).build(), + builder(multivalueKey).multiValues(true).build())); + + private ConfigurationProvider underTest = new ConfigurationProvider(); + + @Test + @UseDataProvider("trimFieldsAndEncodedCommas") + public void getStringArray_split_on_comma_trim_and_support_encoded_comma_as_Settings_getStringArray(String idemUseCase) { + settings.setProperty(nonDeclaredKey, idemUseCase); + settings.setProperty(nonMultivalueKey, idemUseCase); + settings.setProperty(multivalueKey, idemUseCase); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorIsTheSame(configuration, nonDeclaredKey); + getStringArrayBehaviorIsTheSame(configuration, nonMultivalueKey); + getStringArrayBehaviorIsTheSame(configuration, multivalueKey); + } + + @DataProvider + public static Object[][] trimFieldsAndEncodedCommas() { + return new Object[][] { + {"a"}, + {" a"}, + {"a "}, + {" a "}, + {"a,b"}, + {" a,b"}, + {" a ,b"}, + {" a , b"}, + {"a ,b"}, + {" a, b"}, + {"a,b "}, + {" a , b "}, + {"a%2Cb"}, + {"a%2Cb,c"} + }; + } + + @Test + @UseDataProvider("emptyStrings") + public void getStringArray_parses_empty_string_the_same_Settings_if_unknown_or_non_multivalue_property(String emptyValue) { + settings.setProperty(nonDeclaredKey, emptyValue); + settings.setProperty(nonMultivalueKey, emptyValue); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorIsTheSame(configuration, nonDeclaredKey); + getStringArrayBehaviorIsTheSame(configuration, nonMultivalueKey); + } + + @Test + @UseDataProvider("emptyStrings") + public void getStringArray_parses_empty_string_differently_from_Settings_ifmultivalue_property(String emptyValue) { + settings.setProperty(multivalueKey, emptyValue); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorDiffers(configuration, multivalueKey, EMPTY_STRING_ARRAY); + } + + @DataProvider + public static Object[][] emptyStrings() { + return new Object[][] { + {""}, + {" "}, + {" "}, + }; + } + + @Test + @UseDataProvider("subsequentCommas1") + public void getStringArray_on_unknown_or_non_multivalue_properties_ignores_subsequent_commas_as_Settings(String subsequentCommas) { + settings.setProperty(nonDeclaredKey, subsequentCommas); + settings.setProperty(nonMultivalueKey, subsequentCommas); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorIsTheSame(configuration, nonDeclaredKey); + getStringArrayBehaviorIsTheSame(configuration, nonMultivalueKey); + } + + @DataProvider + public static Object[][] subsequentCommas1() { + return new Object[][] { + {",,a"}, + {",,,a"}, + {"a,,b"}, + {"a,,,b,c,,d"} + }; + } + + @Test + @UseDataProvider("subsequentCommas2") + public void getStringArray_on_unknown_or_non_multivalue_properties_ignores_subsequent_commas_differently_from_Settings(String subsequentCommas, String[] expected) { + settings.setProperty(nonDeclaredKey, subsequentCommas); + settings.setProperty(nonMultivalueKey, subsequentCommas); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorDiffers(configuration, nonDeclaredKey, expected); + getStringArrayBehaviorDiffers(configuration, nonMultivalueKey, expected); + } + + @DataProvider + public static Object[][] subsequentCommas2() { + return new Object[][] { + {",,", EMPTY_STRING_ARRAY}, + {",,,", EMPTY_STRING_ARRAY}, + {"a,,", arrayOf("a")}, + {"a,,,", arrayOf("a")} + }; + } + + @Test + @UseDataProvider("subsequentCommas3") + public void getStringArray_on_multivalue_properties_ignores_subsequent_commas_differently_from_Settings(String subsequentCommas, String[] expected) { + settings.setProperty(multivalueKey, subsequentCommas); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorDiffers(configuration, multivalueKey, expected); + } + + @DataProvider + public static Object[][] subsequentCommas3() { + return new Object[][] { + {",,", EMPTY_STRING_ARRAY}, + {",,,", EMPTY_STRING_ARRAY}, + {"a,,", arrayOf("a")}, + {"a,,,", arrayOf("a")}, + {",,a", arrayOf("a")}, + {",,,a", arrayOf("a")}, + {"a,,b", arrayOf("a", "b")}, + {"a,,,b", arrayOf("a", "b")}, + {"a,,,b,,", arrayOf("a", "b")}, + {",,a,,b", arrayOf("a", "b")}, + }; + } + + @Test + @UseDataProvider("emptyFields1") + public void getStringArray_on_unknown_or_non_multivalue_properties_ignores_empty_fields_same_as_settings(String emptyFields, String[] expected) { + settings.setProperty(nonDeclaredKey, emptyFields); + settings.setProperty(nonMultivalueKey, emptyFields); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorIsTheSame(configuration, nonDeclaredKey, expected); + getStringArrayBehaviorIsTheSame(configuration, nonMultivalueKey, expected); + } + + @DataProvider + public static Object[][] emptyFields1() { + return new Object[][] { + // these are inconsistent behaviors of StringUtils.splitByWholeSeparator used under the hood by Settings + {" , a", arrayOf("a")}, + {" ,a,b", arrayOf("a", "b")}, + {" ,,a,b", arrayOf("a", "b")} + }; + } + + @Test + @UseDataProvider("emptyFields2") + public void getStringArray_on_unknown_or_non_multivalue_properties_ignores_empty_fields_differently_from_settings(String emptyFields, String[] expected) { + settings.setProperty(nonDeclaredKey, emptyFields); + settings.setProperty(nonMultivalueKey, emptyFields); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorDiffers(configuration, nonDeclaredKey, expected); + getStringArrayBehaviorDiffers(configuration, nonMultivalueKey, expected); + } + + @DataProvider + public static Object[][] emptyFields2() { + return new Object[][] { + {", ", EMPTY_STRING_ARRAY}, + {" ,", EMPTY_STRING_ARRAY}, + {" , ", EMPTY_STRING_ARRAY}, + {" , \n ,, \t", EMPTY_STRING_ARRAY}, + {" , \t , , ", EMPTY_STRING_ARRAY}, + {"a, ", arrayOf("a")}, + {" a, ,", arrayOf("a")}, + {" , a, ,", arrayOf("a")}, + {"a,b, ", arrayOf("a", "b")}, + {"a, ,b", arrayOf("a", "b")}, + {" ,a, ,b", arrayOf("a", "b")}, + {" ,,a, ,b", arrayOf("a", "b")}, + {" ,a, ,,b", arrayOf("a", "b")}, + {" ,,a, ,,b", arrayOf("a", "b")}, + {"a, ,b, ", arrayOf("a", "b")}, + {"\t ,a, ,b, ", arrayOf("a", "b")}, + }; + } + + @Test + @UseDataProvider("emptyFields3") + public void getStringArray_on_multivalue_properties_ignores_empty_fields_differently_from_settings(String emptyFields, String[] expected) { + settings.setProperty(multivalueKey, emptyFields); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorDiffers(configuration, multivalueKey, expected); + } + + @DataProvider + public static Object[][] emptyFields3() { + return new Object[][] { + {", ", EMPTY_STRING_ARRAY}, + {" ,", EMPTY_STRING_ARRAY}, + {" , ", EMPTY_STRING_ARRAY}, + {" , \n ,, \t", EMPTY_STRING_ARRAY}, + {" , \t , , ", EMPTY_STRING_ARRAY}, + {"a, ", arrayOf("a")}, + {" a, ,", arrayOf("a")}, + {" , a, ,", arrayOf("a")}, + {"a,b, ", arrayOf("a", "b")}, + {" ,a,b", arrayOf("a", "b")}, + {" ,,a,b", arrayOf("a", "b")}, + {"a, ,b", arrayOf("a", "b")}, + {" ,a, ,b", arrayOf("a", "b")}, + {" ,,a, ,b", arrayOf("a", "b")}, + {" ,a, ,,b", arrayOf("a", "b")}, + {" ,,a, ,,b", arrayOf("a", "b")}, + {"a, ,b, ", arrayOf("a", "b")}, + {"\t ,a, ,b, ", arrayOf("a", "b")}, + }; + } + + @Test + @UseDataProvider("quotedStrings1") + public void getStringArray_supports_quoted_strings_when_settings_does_not(String str, + String[] configurationExpected, String[] settingsExpected) { + settings.setProperty(nonDeclaredKey, str); + settings.setProperty(nonMultivalueKey, str); + settings.setProperty(multivalueKey, str); + + Configuration configuration = underTest.provide(settings); + + getStringArrayBehaviorDiffers(configuration, nonDeclaredKey, configurationExpected, settingsExpected); + getStringArrayBehaviorDiffers(configuration, nonMultivalueKey, configurationExpected, settingsExpected); + getStringArrayBehaviorDiffers(configuration, multivalueKey, configurationExpected, settingsExpected); + } + + @DataProvider + public static Object[][] quotedStrings1() { + return new Object[][] { + {"\"\"", arrayOf(""), arrayOf("\"\"")}, + {" \"\"", arrayOf(""), arrayOf("\"\"")}, + {"\"\" ", arrayOf(""), arrayOf("\"\"")}, + {" \"\" ", arrayOf(""), arrayOf("\"\"")}, + {"\" \"", arrayOf(" "), arrayOf("\" \"")}, + {" \" \" ", arrayOf(" "), arrayOf("\" \"")}, + {"\"a \"", arrayOf("a "), arrayOf("\"a \"")}, + {"\" a\"", arrayOf(" a"), arrayOf("\" a\"")}, + {"\",\"", arrayOf(","), arrayOf("\"", "\"")}, + // escaped quote in quoted field + {"\"\"\"\"", arrayOf("\""), arrayOf("\"\"\"\"")}, + {"\"a\"\"\"", arrayOf("a\""), arrayOf("\"a\"\"\"")}, + {"\"\"\"b\"", arrayOf("\"b"), arrayOf("\"\"\"b\"")}, + {"\"a\"\"b\"", arrayOf("a\"b"), arrayOf("\"a\"\"b\"")}, + {"\",\",\"a\"", arrayOf(",", "a"), arrayOf("\"", "\"", "\"a\"")}, + }; + } + + private void getStringArrayBehaviorIsTheSame(Configuration configuration, String key) { + assertThat(configuration.getStringArray(key)) + .isEqualTo(settings.getStringArray(key)); + } + + private void getStringArrayBehaviorIsTheSame(Configuration configuration, String key, String[] expected) { + assertThat(configuration.getStringArray(key)) + .isEqualTo(expected) + .isEqualTo(settings.getStringArray(key)); + } + + private void getStringArrayBehaviorDiffers(Configuration configuration, String key, String[] expected) { + assertThat(configuration.getStringArray(key)) + .isEqualTo(expected) + .isNotEqualTo(settings.getStringArray(key)); + } + + private void getStringArrayBehaviorDiffers(Configuration configuration, String key, String[] configurationExpected, String[] settingsExpected) { + String[] conf = configuration.getStringArray(key); + String[] sett = settings.getStringArray(key); + assertThat(conf).isEqualTo(configurationExpected); + assertThat(sett).isEqualTo(settingsExpected); + assertThat(conf).isNotEqualTo(sett); + } + + private static String[] arrayOf(String... strs) { + return strs; + } +} diff --git a/sonar-core/src/main/java/org/sonar/core/config/MultivalueProperty.java b/sonar-core/src/main/java/org/sonar/core/config/MultivalueProperty.java index 17451ee0cb6..346f2250084 100644 --- a/sonar-core/src/main/java/org/sonar/core/config/MultivalueProperty.java +++ b/sonar-core/src/main/java/org/sonar/core/config/MultivalueProperty.java @@ -96,7 +96,10 @@ public class MultivalueProperty { } } } - it.forEachRemaining(s -> result.add(valueProcessor.apply(s))); + it.forEachRemaining(s -> { + String apply = valueProcessor.apply(s); + result.add(apply); + }); } } -- 2.39.5