From dedb3fce26e561a0c51602a0087adb26b91888cc Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 28 Jan 2016 10:38:44 +0100 Subject: [PATCH] #450 Add integration test for potentially difficult values - Ensure that edge cases such as backslashes, apostrophes and new lines are handled properly by the YAML writer & reader --- .../fr/xephi/authme/settings/NewSetting.java | 22 +++-- .../settings/NewSettingIntegrationTest.java | 96 +++++++++++++++---- .../resources/config-difficult-values.yml | 29 ++++++ 3 files changed, 122 insertions(+), 25 deletions(-) create mode 100644 src/test/resources/config-difficult-values.yml diff --git a/src/main/java/fr/xephi/authme/settings/NewSetting.java b/src/main/java/fr/xephi/authme/settings/NewSetting.java index c48f1c469..8aa46b84f 100644 --- a/src/main/java/fr/xephi/authme/settings/NewSetting.java +++ b/src/main/java/fr/xephi/authme/settings/NewSetting.java @@ -76,16 +76,10 @@ public class NewSetting { public void save(PropertyMap propertyMap) { try (FileWriter writer = new FileWriter(file)) { + Yaml simpleYaml = newYaml(false); + Yaml singleQuoteYaml = newYaml(true); + writer.write(""); - - DumperOptions simpleOptions = new DumperOptions(); - simpleOptions.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK); - Yaml simpleYaml = new Yaml(simpleOptions); - DumperOptions singleQuoteOptions = new DumperOptions(); - singleQuoteOptions.setDefaultScalarStyle(DumperOptions.ScalarStyle.SINGLE_QUOTED); - singleQuoteOptions.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK); - Yaml singleQuoteYaml = new Yaml(singleQuoteOptions); - // Contains all but the last node of the setting, e.g. [DataSource, mysql] for "DataSource.mysql.username" List currentPath = new ArrayList<>(); for (Map.Entry, String[]> entry : propertyMap.entrySet()) { @@ -150,6 +144,16 @@ public class NewSetting { return join("\n" + indent(indent), representation.split("\\n")); } + private static Yaml newYaml(boolean useSingleQuotes) { + DumperOptions options = new DumperOptions(); + options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK); + options.setAllowUnicode(true); + if (useSingleQuotes) { + options.setDefaultScalarStyle(DumperOptions.ScalarStyle.SINGLE_QUOTED); + } + return new Yaml(options); + } + private static String join(String delimiter, String[] items) { StringBuilder sb = new StringBuilder(); String delim = ""; diff --git a/src/test/java/fr/xephi/authme/settings/NewSettingIntegrationTest.java b/src/test/java/fr/xephi/authme/settings/NewSettingIntegrationTest.java index 491d11f68..41b2cf2bd 100644 --- a/src/test/java/fr/xephi/authme/settings/NewSettingIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/settings/NewSettingIntegrationTest.java @@ -7,7 +7,6 @@ import fr.xephi.authme.settings.properties.TestConfiguration; import fr.xephi.authme.settings.propertymap.PropertyMap; import fr.xephi.authme.util.WrapperMock; import org.bukkit.configuration.file.YamlConfiguration; -import org.junit.BeforeClass; import org.junit.Test; import java.io.File; @@ -15,8 +14,10 @@ import java.lang.reflect.Field; import java.net.URL; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; +import static fr.xephi.authme.settings.domain.Property.newProperty; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.junit.Assume.assumeThat; @@ -30,22 +31,10 @@ public class NewSettingIntegrationTest { private static final String COMPLETE_FILE = "config-sample-values.yml"; /** File name of the sample config missing certain {@link TestConfiguration} values. */ private static final String INCOMPLETE_FILE = "config-incomplete-sample.yml"; + /** File name for testing difficult values. */ + private static final String DIFFICULT_FILE = "config-difficult-values.yml"; - private static PropertyMap propertyMap; - - @BeforeClass - public static void generatePropertyMap() { - WrapperMock.createInstance(); - propertyMap = new PropertyMap(); - for (Field field : TestConfiguration.class.getDeclaredFields()) { - Object fieldValue = ReflectionTestUtils.getFieldValue(TestConfiguration.class, null, field.getName()); - if (fieldValue instanceof Property) { - Property property = (Property) fieldValue; - String[] comments = new String[]{"Comment for '" + property.getPath() + "'"}; - propertyMap.put(property, comments); - } - } - } + private static PropertyMap propertyMap = generatePropertyMap(); @Test public void shouldLoadAndReadAllProperties() { @@ -107,6 +96,62 @@ public class NewSettingIntegrationTest { } } + /** Verify that "difficult cases" such as apostrophes in strings etc. are handled properly. */ + @Test + public void shouldProperlyExportAnyValues() { + // given + File file = getConfigFile(DIFFICULT_FILE); + YamlConfiguration configuration = YamlConfiguration.loadConfiguration(file); + assumeThat(configuration.contains(TestConfiguration.DUST_LEVEL.getPath()), equalTo(false)); + + // Additional string properties + List> additionalProperties = Arrays.asList( + newProperty("more.string1", "it's a text with some \\'apostrophes'"), + newProperty("more.string2", "\tthis one\nhas some\nnew '' lines-test") + ); + PropertyMap propertyMap = generatePropertyMap(); + for (Property property : additionalProperties) { + propertyMap.put(property, new String[0]); + } + + // when + new NewSetting(configuration, file, propertyMap); + // reload the file as settings should hav been rewritten + configuration = YamlConfiguration.loadConfiguration(file); + + // then + // assert that we won't rewrite the settings again! One rewrite should produce a valid, complete configuration + File unusedFile = new File("config-difficult-values.unused.yml"); + NewSetting settings = new NewSetting(configuration, unusedFile, propertyMap); + assertThat(unusedFile.exists(), equalTo(false)); + assertThat(configuration.contains(TestConfiguration.DUST_LEVEL.getPath()), equalTo(true)); + + Map, Object> expectedValues = ImmutableMap., Object>builder() + .put(TestConfiguration.DURATION_IN_SECONDS, 20) + .put(TestConfiguration.SYSTEM_NAME, "A 'test' name") + .put(TestConfiguration.RATIO_LIMIT, -41.8) + .put(TestConfiguration.RATIO_FIELDS, Arrays.asList("Australia\\", "\tBurundi'", "Colombia?\n''")) + .put(TestConfiguration.VERSION_NUMBER, -1337) + .put(TestConfiguration.SKIP_BORING_FEATURES, false) + .put(TestConfiguration.BORING_COLORS, Arrays.asList("it's a difficult string!", "gray\nwith new lines\n")) + .put(TestConfiguration.DUST_LEVEL, 0.2) + .put(TestConfiguration.USE_COOL_FEATURES, true) + .put(TestConfiguration.COOL_OPTIONS, Collections.EMPTY_LIST) + .put(additionalProperties.get(0), additionalProperties.get(0).getDefaultValue()) + .put(additionalProperties.get(1), additionalProperties.get(1).getDefaultValue()) + .build(); + for (Map.Entry, Object> entry : expectedValues.entrySet()) { + assertThat("Property '" + entry.getKey().getPath() + "' has expected value" + + entry.getValue() + " but found " + settings.getProperty(entry.getKey()), + settings.getProperty(entry.getKey()), equalTo(entry.getValue())); + } + } + + /** + * Return a {@link File} instance to an existing file in the target/test-classes folder. + * + * @return The generated File + */ private File getConfigFile(String file) { URL url = getClass().getClassLoader().getResource(file); if (url == null) { @@ -115,4 +160,23 @@ public class NewSettingIntegrationTest { return new File(url.getFile()); } + /** + * Generate a property map with all properties in {@link TestConfiguration}. + * + * @return The generated property map + */ + private static PropertyMap generatePropertyMap() { + WrapperMock.createInstance(); + PropertyMap propertyMap = new PropertyMap(); + for (Field field : TestConfiguration.class.getDeclaredFields()) { + Object fieldValue = ReflectionTestUtils.getFieldValue(TestConfiguration.class, null, field.getName()); + if (fieldValue instanceof Property) { + Property property = (Property) fieldValue; + String[] comments = new String[]{"Comment for '" + property.getPath() + "'"}; + propertyMap.put(property, comments); + } + } + return propertyMap; + } + } diff --git a/src/test/resources/config-difficult-values.yml b/src/test/resources/config-difficult-values.yml new file mode 100644 index 000000000..157971bc5 --- /dev/null +++ b/src/test/resources/config-difficult-values.yml @@ -0,0 +1,29 @@ +# Test config file with some "difficult" values + +test: + duration: 20.102 + systemName: 'A ''test'' name' +sample: + ratio: + limit: -41.8 + fields: + - Australia\ + - ' Burundi''' + - 'Colombia? + + ''''' + # The last element above represents "Colombia?\n''" +version: -1337 +features: + boring: + # YAML allows both "yes"/"no" and "true"/"false" for expressing booleans + skip: no + colors: + - 'it''s a difficult string!' + - | + gray + with new lines + # dustLevel: 0.81 <-- missing property triggering rewrite + cool: + enabled: yes + options: []