From 34f43848de985a6e4395c0b138c74ab9d9397620 Mon Sep 17 00:00:00 2001 From: Guille Date: Mon, 9 Dec 2019 12:13:05 +0100 Subject: [PATCH 1/3] Replace \ chars in Windows paths with / to make them portable --- src/org/opendatakit/briefcase/model/form/FormMetadata.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/org/opendatakit/briefcase/model/form/FormMetadata.java b/src/org/opendatakit/briefcase/model/form/FormMetadata.java index 51fca1000..d67be0324 100644 --- a/src/org/opendatakit/briefcase/model/form/FormMetadata.java +++ b/src/org/opendatakit/briefcase/model/form/FormMetadata.java @@ -1,6 +1,7 @@ package org.opendatakit.briefcase.model.form; import static org.opendatakit.briefcase.model.form.AsJson.getJson; +import static org.opendatakit.briefcase.util.Host.isWindows; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; @@ -110,9 +111,12 @@ FormMetadata withLastExportedSubmission(String instanceId, OffsetDateTime submis @Override public ObjectNode asJson(ObjectMapper mapper) { + String portableFormDir = isWindows() + ? formDir.toString().replace("\\", "/") + : formDir.toString(); ObjectNode root = mapper.createObjectNode(); root.putObject("key").setAll(key.asJson(mapper)); - root.put("formDir", formDir.toString()); + root.put("formDir", portableFormDir); root.put("hasBeenPulled", hasBeenPulled); root.putObject("cursor").setAll(cursor.asJson(mapper)); lastExportedSubmission.ifPresent(o -> root.putObject("lastExportedSubmission").setAll(o.asJson(mapper))); From ef93a07d22a12ce4d428d2c0daf9c559fc81f60e Mon Sep 17 00:00:00 2001 From: Guille Date: Mon, 9 Dec 2019 13:12:06 +0100 Subject: [PATCH 2/3] Ensure we won't have form dirs with chars that might be interpreted as directory separators in other platforms --- .../org/opendatakit/briefcase/util/StringUtilsTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/java/org/opendatakit/briefcase/util/StringUtilsTest.java b/test/java/org/opendatakit/briefcase/util/StringUtilsTest.java index fcad77566..cc88ccabd 100644 --- a/test/java/org/opendatakit/briefcase/util/StringUtilsTest.java +++ b/test/java/org/opendatakit/briefcase/util/StringUtilsTest.java @@ -23,6 +23,13 @@ public void stripIllegalChars_shouldRemovePunctuationChars() { Assert.assertEquals("abcdef___", output); } + @Test + public void stripIllegalChars_shouldRemoveDirectorySeparatorChars() { + String input = "a\\b/c"; + String output = StringUtils.stripIllegalChars(input); + Assert.assertEquals("a_b_c", output); + } + @Test public void stripIllegalChars_shouldReplaceWhitespaceWithSpace() { String input = "abcdef\t\n\r"; From f2a3a48aca54ad4196f18eaf5e9bacf285eef4f1 Mon Sep 17 00:00:00 2001 From: Guille Date: Mon, 9 Dec 2019 13:25:33 +0100 Subject: [PATCH 3/3] Simplify tests with parameterized examples --- .../aggregate/form/XFormParametersTest.java | 2 +- .../briefcase/util/StringUtilsTest.java | 80 +++++++------------ 2 files changed, 31 insertions(+), 51 deletions(-) diff --git a/test/java/org/opendatakit/aggregate/form/XFormParametersTest.java b/test/java/org/opendatakit/aggregate/form/XFormParametersTest.java index fe012df68..3410ae356 100644 --- a/test/java/org/opendatakit/aggregate/form/XFormParametersTest.java +++ b/test/java/org/opendatakit/aggregate/form/XFormParametersTest.java @@ -87,4 +87,4 @@ public Matcher apply(XFormParameters value) { return matcherProvider.apply(value); } } -} \ No newline at end of file +} diff --git a/test/java/org/opendatakit/briefcase/util/StringUtilsTest.java b/test/java/org/opendatakit/briefcase/util/StringUtilsTest.java index cc88ccabd..b111ed1f0 100644 --- a/test/java/org/opendatakit/briefcase/util/StringUtilsTest.java +++ b/test/java/org/opendatakit/briefcase/util/StringUtilsTest.java @@ -1,60 +1,40 @@ package org.opendatakit.briefcase.util; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import org.junit.Assert; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; +import static org.opendatakit.briefcase.util.StringUtils.stripIllegalChars; + +import java.util.Arrays; +import java.util.Collection; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +@RunWith(value = Parameterized.class) public class StringUtilsTest { - - @Test - public void stripIllegalChars_shouldReturnNullForNullInput() { - Assert.assertNull(StringUtils.stripIllegalChars(null)); - } - - @Test - public void stripIllegalChars_shouldRemovePunctuationChars() { - String input = "abcdef.:;"; - String output = StringUtils.stripIllegalChars(input); - - Pattern pattern = Pattern.compile("\\p{Punct}"); - Matcher matcher = pattern.matcher(output); - Assert.assertTrue(!matcher.matches()); - Assert.assertEquals("abcdef___", output); - } - - @Test - public void stripIllegalChars_shouldRemoveDirectorySeparatorChars() { - String input = "a\\b/c"; - String output = StringUtils.stripIllegalChars(input); - Assert.assertEquals("a_b_c", output); + @Parameterized.Parameter(value = 0) + public String testCase; + + @Parameterized.Parameter(value = 1) + public String input; + + @Parameterized.Parameter(value = 2) + public String expectedOutput; + + @Parameterized.Parameters(name = "{0}") + public static Collection data() { + return Arrays.asList(new Object[][]{ + {"null input produces null output", null, null}, + {"Common punctuation chars get replaced by underscore", "abcdef.:;", "abcdef___"}, + {"Windows and *nix dir separators get replaced by underscore", "a\\b/c", "a_b_c"}, + {"Whitespace chars get normalized", "abcdef\t\n\r ", "abcdef "}, + {"Unicode chars should get through", "シャンプー", "シャンプー"}, + }); } @Test - public void stripIllegalChars_shouldReplaceWhitespaceWithSpace() { - String input = "abcdef\t\n\r"; - String output = StringUtils.stripIllegalChars(input); - - Pattern pattern = Pattern.compile("[\\t\\n\\x0B\\f\\r]"); - Matcher matcher = pattern.matcher(output); - Assert.assertTrue(String.format("Input: %s, output: %s", input, output), !matcher.matches()); - Assert.assertEquals("abcdef ", output); + public void test_stripIllegalChars() { + assertThat(stripIllegalChars(input), expectedOutput == null ? is(nullValue()) : is(expectedOutput)); } - - @Test - public void stripIllegalChars_shouldWorkWithUnicodeCharacters() { - String input = ".:;%&シャンプー \n\r"; - String output = StringUtils.stripIllegalChars(input); - - Pattern pattern = Pattern.compile("\\p{Punct}"); - Matcher matcher = pattern.matcher(output); - Assert.assertTrue(String.format("Input: %s, output: %s", input, output), !matcher.matches()); - - pattern = Pattern.compile("[\\t\\n\\x0B\\f\\r]"); - matcher = pattern.matcher(output); - Assert.assertTrue(String.format("Input: %s, output: %s", input, output), !matcher.matches()); - - Assert.assertEquals("_____シャンプー ", output); - } - }