From fb7ed5d71f119adc9920c5630d431cd04c889b84 Mon Sep 17 00:00:00 2001 From: Squareys Date: Thu, 8 Oct 2015 09:35:43 +0200 Subject: [PATCH 1/2] Fix issues with Readers of ScriptInfo used multiple times without reset ScriptModule, for example, does not reset the Reader aquired via `ScriptInfo.getReader()`. While it is definitely an option to fix this issue by resetting it there, this way we can ensure that the behaviour of `ScriptInfo.getReader()` stays consistent. Also, this solution makes way to some day maybe run ScriptModules in parallel. Signed-off-by: Squareys --- .../java/org/scijava/script/ScriptInfo.java | 54 +++++++++++++++---- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/scijava/script/ScriptInfo.java b/src/main/java/org/scijava/script/ScriptInfo.java index 2ac670db6..67f67ea62 100644 --- a/src/main/java/org/scijava/script/ScriptInfo.java +++ b/src/main/java/org/scijava/script/ScriptInfo.java @@ -36,6 +36,7 @@ import java.io.FileReader; import java.io.IOException; import java.io.Reader; +import java.io.StringReader; import java.text.SimpleDateFormat; import java.util.Date; import java.util.HashMap; @@ -73,7 +74,7 @@ public class ScriptInfo extends AbstractModuleInfo implements Contextual { private static final int PARAM_CHAR_MAX = 640 * 1024; // should be enough ;-) private final String path; - private final BufferedReader reader; + private final String script; @Parameter private Context context; @@ -128,8 +129,17 @@ public ScriptInfo(final Context context, final String path, { setContext(context); this.path = path; - this.reader = - reader == null ? null : new BufferedReader(reader, PARAM_CHAR_MAX); + + String script = null; + if (reader != null) { + try { + script = getReaderContentsAsString(reader); + } + catch (final IOException exc) { + log.error("Error reading script: " + path, exc); + } + } + this.script = script; } // -- ScriptInfo methods -- @@ -148,14 +158,17 @@ public String getPath() { } /** - * Gets the reader which delivers the script's content. + * Gets a reader which delivers the script's content. *

* This might be null, in which case the content is stored in a file on disk * given by {@link #getPath()}. *

*/ public BufferedReader getReader() { - return reader; + if (script == null) { + return null; + } + return new BufferedReader(new StringReader(script), PARAM_CHAR_MAX); } /** @@ -214,12 +227,11 @@ public void parseParameters() { try { final BufferedReader in; - if (reader == null) { + if (script == null) { in = new BufferedReader(new FileReader(getPath())); } else { - in = reader; - in.mark(PARAM_CHAR_MAX); + in = getReader(); } while (true) { final String line = in.readLine(); @@ -234,8 +246,7 @@ public void parseParameters() { } else if (line.matches(".*\\w.*")) break; } - if (reader == null) in.close(); - else in.reset(); + in.close(); if (!returnValueDeclared) addReturnValue(); } @@ -478,4 +489,27 @@ else if ("value".equalsIgnoreCase(key)) { } } + /** + * Read entire contents of a Reader and return as String. + * + * @param reader {@link Reader} whose contents should be returned as String. + * Expected to never be null. + * @return contents of reader as String. + * @throws IOException If an I/O error occurs + * @throws NullPointerException If reader is null + */ + private static String getReaderContentsAsString(final Reader reader) + throws IOException, NullPointerException + { + final char[] buffer = new char[8192]; + final StringBuilder builder = new StringBuilder(); + + int read; + while ((read = reader.read(buffer)) != -1) { + builder.append(buffer, 0, read); + } + + return builder.toString(); + } + } From 957e7a191d93e7d407903e6b4a121f55ba9b1cf6 Mon Sep 17 00:00:00 2001 From: Squareys Date: Thu, 8 Oct 2015 09:36:53 +0200 Subject: [PATCH 2/2] Test independence of Readers returned by `ScriptInfo.getReader()` Signed-off-by: Squareys --- .../org/scijava/script/ScriptInfoTest.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/test/java/org/scijava/script/ScriptInfoTest.java b/src/test/java/org/scijava/script/ScriptInfoTest.java index 74de8d8af..c36b4abb8 100644 --- a/src/test/java/org/scijava/script/ScriptInfoTest.java +++ b/src/test/java/org/scijava/script/ScriptInfoTest.java @@ -35,6 +35,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.io.BufferedReader; import java.io.File; import java.io.IOException; import java.io.Reader; @@ -124,6 +125,26 @@ public void testVersion() throws IOException { FileUtils.deleteRecursively(tmpDir); } + /** + * Ensures the ScriptInfos Reader can be reused for multiple executions of the + * script. + */ + @Test + public void testReaderSanity() throws Exception { + final String script = "" + // + "% @LogService log\n" + // + "% @OUTPUT Integer output"; + + ScriptInfo info = new ScriptInfo(context, "hello.bsizes", new StringReader( + script)); + BufferedReader reader1 = info.getReader(); + BufferedReader reader2 = info.getReader(); + + assertEquals("Readers are not independent.", reader1.read(), reader2 + .read()); + + } + @Plugin(type = ScriptLanguage.class) public static class BindingSizes extends AbstractScriptLanguage {