From c95856f14e4523a6f9fbc5a425cc65189d4114dc Mon Sep 17 00:00:00 2001 From: Jan Eglinger Date: Fri, 21 Jul 2017 10:38:31 +0200 Subject: [PATCH 1/2] Use ConvertService to get String representation of input value This is required to correctly persist values for File[] inputs etc. --- src/main/java/org/scijava/module/DefaultModuleService.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/org/scijava/module/DefaultModuleService.java b/src/main/java/org/scijava/module/DefaultModuleService.java index 2bcf13d24..bdad241f5 100644 --- a/src/main/java/org/scijava/module/DefaultModuleService.java +++ b/src/main/java/org/scijava/module/DefaultModuleService.java @@ -297,9 +297,7 @@ public void save(final ModuleItem item, final T value) { return; } - // FIXME: Convert to string, instead of just calling toString. - // Otherwise many things (e.g. File[]) are persisted improperly. - final String sValue = value == null ? "" : value.toString(); + final String sValue = value == null ? "" : convertService.convert(value, String.class); // do not persist if object cannot be converted back from a string if (!convertService.supports(sValue, item.getType())) return; From 58acc0b9a28ede80d0df4edd7dd8acba76abb437 Mon Sep 17 00:00:00 2001 From: Jan Eglinger Date: Fri, 21 Jul 2017 10:39:22 +0200 Subject: [PATCH 2/2] Add File list converters and tests This introduces a dependency on Google Guava --- pom.xml | 6 +- .../scijava/convert/FileListConverters.java | 125 ++++++++++++++++++ .../convert/FileListConverterTest.java | 71 ++++++++++ 3 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/scijava/convert/FileListConverters.java create mode 100644 src/test/java/org/scijava/convert/FileListConverterTest.java diff --git a/pom.xml b/pom.xml index 91b5b993f..0c9add870 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.scijava pom-scijava - 13.1.0 + 16.2.0 @@ -172,6 +172,10 @@ Konstanz, and KNIME GmbH. + + com.google.guava + guava + com.googlecode.gentyref gentyref diff --git a/src/main/java/org/scijava/convert/FileListConverters.java b/src/main/java/org/scijava/convert/FileListConverters.java new file mode 100644 index 000000000..2138b747a --- /dev/null +++ b/src/main/java/org/scijava/convert/FileListConverters.java @@ -0,0 +1,125 @@ +package org.scijava.convert; + +import com.google.common.base.Splitter; + +import java.io.File; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + +import org.scijava.Priority; +import org.scijava.plugin.Plugin; + +public class FileListConverters { + // -- String to File (list) converters -- + + @Plugin(type = Converter.class, priority = Priority.NORMAL) + public static class StringToFileConverter extends + AbstractConverter { + + @SuppressWarnings("unchecked") + @Override + public T convert(Object src, Class dest) { + return (T) new File((String) src); + } + + @Override + public Class getOutputType() { + return File.class; + } + + @Override + public Class getInputType() { + return String.class; + } + + } + + @Plugin(type = Converter.class, priority = Priority.NORMAL) + public static class StringToFileArrayConverter extends + AbstractConverter { + + @SuppressWarnings("unchecked") + @Override + public T convert(Object src, Class dest) { + // split string src only at non-quoted commas + // see https://stackoverflow.com/a/1757107/1919049 + Iterable list = Splitter.onPattern( + ",(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)").split((String) src); + List fileList = new ArrayList<>(); + for (String filePath : list) { + fileList.add(new File(filePath.replaceAll("^\"|\"$", ""))); + } + return (T) fileList.toArray(new File[fileList.size()]); + } + + @Override + public Class getOutputType() { + return File[].class; + } + + @Override + public Class getInputType() { + return String.class; + } + + } + + // TODO add StringToFileListConverter + + // -- File (list) to String converters -- + + @Plugin(type = Converter.class, priority = Priority.NORMAL) + public static class FileToStringConverter extends + AbstractConverter { + + @SuppressWarnings("unchecked") + @Override + public T convert(Object src, Class dest) { + return (T) ((File) src).getAbsolutePath(); + } + + @Override + public Class getOutputType() { + return String.class; + } + + @Override + public Class getInputType() { + return File.class; + } + + } + + @Plugin(type = Converter.class, priority = Priority.NORMAL) + public static class FileArrayToStringConverter extends + AbstractConverter { + + @SuppressWarnings("unchecked") + @Override + public T convert(Object src, Class dest) { + List result = Arrays.asList((File[]) src) + .stream() + .map(f -> { + String path = f.getAbsolutePath(); + return path.contains(",") ? "\"" + path + "\"": path; + }) + .collect(Collectors.toList()); + return (T) String.join(",", result); + } + + @Override + public Class getOutputType() { + return String.class; + } + + @Override + public Class getInputType() { + return File[].class; + } + + } + + // TODO add FileListToStringConverter +} diff --git a/src/test/java/org/scijava/convert/FileListConverterTest.java b/src/test/java/org/scijava/convert/FileListConverterTest.java new file mode 100644 index 000000000..46eae2f3e --- /dev/null +++ b/src/test/java/org/scijava/convert/FileListConverterTest.java @@ -0,0 +1,71 @@ +package org.scijava.convert; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +import java.io.File; + +import org.junit.Test; +import org.scijava.convert.FileListConverters.FileArrayToStringConverter; +import org.scijava.convert.FileListConverters.FileToStringConverter; +import org.scijava.convert.FileListConverters.StringToFileArrayConverter; +import org.scijava.convert.FileListConverters.StringToFileConverter; + +public class FileListConverterTest { + + @Test + public void testStringToFileConverter() { + final StringToFileConverter conv = new StringToFileConverter(); + final String path = "C:\\temp\\f,i;l-ename.txt"; + assertTrue("Cannot convert from String to File", + conv.canConvert(String.class, File.class)); + assertFalse("Can erroneously convert from String to File[]", + conv.canConvert(String.class, File[].class)); + assertEquals(new File(path), + conv.convert(path, File.class)); + } + + @Test + public void testStringToFileArrayConverter() { + final StringToFileArrayConverter conv = new StringToFileArrayConverter(); + final String path = "\"C:\\temp\\f,i;l-ename.txt\",C:\\temp"; + assertTrue("Cannot convert from String to File[]", + conv.canConvert(String.class, File[].class)); + assertFalse("Can erroneously convert from String to File", + conv.canConvert(String.class, File.class)); + assertEquals("Wrong array length", 2, + conv.convert(path, File[].class).length); + assertEquals("Wrong file name", new File("C:\\temp\\f,i;l-ename.txt"), + conv.convert(path, File[].class)[0]); + assertEquals("Wrong file name", new File("C:\\temp"), + conv.convert(path, File[].class)[1]); + } + + @Test + public void testFileToStringConverter() { + final FileToStringConverter conv = new FileToStringConverter(); + final File file = new File("C:\\temp\\f,i;l-ename.txt"); + assertTrue("Cannot convert from File to String", + conv.canConvert(File.class, String.class)); + assertFalse("Can erroneously convert from File[] to String", + conv.canConvert(File[].class, String.class)); + assertEquals(file.getAbsolutePath(), + conv.convert(file, String.class)); + } + + @Test + public void testFileArrayToStringConverter() { + final FileArrayToStringConverter conv = new FileArrayToStringConverter(); + final File[] fileArray = new File[2]; + fileArray[0] = new File("C:\\temp\\f,i;l-ename.txt"); + fileArray[1] = new File("C:\\temp"); + final String expected = "\"" + fileArray[0].getAbsolutePath() + "\"," + fileArray[1].getAbsolutePath(); + assertTrue("Cannot convert from File[] to String", + conv.canConvert(File[].class, String.class)); + assertFalse("Can erroneously convert from File to String", + conv.canConvert(File.class, String.class)); + assertEquals("Wrong output string", expected, + conv.convert(fileArray, String.class)); + } +}