From 60d2efc304295f73ffba36a64a57e051639d1d12 Mon Sep 17 00:00:00 2001 From: Mitchell Bosecke Date: Mon, 27 Oct 2014 20:48:46 -0600 Subject: [PATCH] Fixed bug where nested "parallel" tags would fail with a casting error --- .../pebble/node/ParallelNode.java | 9 ++- .../pebble/template/PebbleTemplateImpl.java | 2 +- .../pebble/utils/FutureWriter.java | 61 ++++++++++++------- .../mitchellbosecke/pebble/CoreTagsTest.java | 38 ++++++++++++ 4 files changed, 84 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/mitchellbosecke/pebble/node/ParallelNode.java b/src/main/java/com/mitchellbosecke/pebble/node/ParallelNode.java index 2361d7eb8..fc04f9350 100644 --- a/src/main/java/com/mitchellbosecke/pebble/node/ParallelNode.java +++ b/src/main/java/com/mitchellbosecke/pebble/node/ParallelNode.java @@ -69,14 +69,17 @@ public void render(final PebbleTemplateImpl self, Writer writer, final Evaluatio final EvaluationContext contextCopy = context.deepCopy(self); - final Writer stringWriter = new StringWriter(); + final StringWriter newStringWriter = new StringWriter(); + final Writer newFutureWriter = new FutureWriter(newStringWriter); Future future = es.submit(new Callable() { @Override public String call() throws PebbleException, IOException { - body.render(self, stringWriter, contextCopy); - return stringWriter.toString(); + body.render(self, newFutureWriter, contextCopy); + newFutureWriter.flush(); + newFutureWriter.close(); + return newStringWriter.toString(); } }); ((FutureWriter) writer).enqueue(future); diff --git a/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java b/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java index 6de93eb4c..fdb013673 100644 --- a/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java +++ b/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java @@ -107,7 +107,7 @@ public void evaluate(Writer writer, Map map, Locale locale) thro */ public void evaluate(Writer writer, EvaluationContext context) throws PebbleException, IOException { if (context.getExecutorService() != null) { - writer = new FutureWriter(writer, context.getExecutorService()); + writer = new FutureWriter(writer); } buildContent(writer, context); writer.flush(); diff --git a/src/main/java/com/mitchellbosecke/pebble/utils/FutureWriter.java b/src/main/java/com/mitchellbosecke/pebble/utils/FutureWriter.java index f0df54e0e..ac32a52d1 100644 --- a/src/main/java/com/mitchellbosecke/pebble/utils/FutureWriter.java +++ b/src/main/java/com/mitchellbosecke/pebble/utils/FutureWriter.java @@ -10,34 +10,33 @@ import java.io.IOException; import java.io.Writer; -import java.util.Arrays; -import java.util.concurrent.Callable; -import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.LinkedList; import java.util.concurrent.ExecutionException; -import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; /** * A Writer that will wrap around the user-provided writer if the user also * provided an ExecutorService to the main PebbleEngine. A FutureWriter is * capable of handling Futures that will return a string. * + * It is not thread safe but that is okay. Each thread will have it's own + * writer, provided by the "parallel" node; i.e. they will never share writers. + * * @author Mitchell * */ public class FutureWriter extends Writer { - private final ConcurrentLinkedQueue> orderedFutures = new ConcurrentLinkedQueue<>(); - - private final ExecutorService es; + private final LinkedList> orderedFutures = new LinkedList<>(); private final Writer internalWriter; private boolean closed = false; - public FutureWriter(Writer writer, ExecutorService es) { + public FutureWriter(Writer writer) { this.internalWriter = writer; - this.es = es; } public void enqueue(Future future) throws IOException { @@ -50,26 +49,44 @@ public void enqueue(Future future) throws IOException { @Override public void write(final char[] cbuf, final int off, final int len) throws IOException { - /* - * We need to make a defensive copy of the character buffer because this - * class will continue to reuse the same buffer with future invocations - * of this write method. - */ - final char[] finalCharacterBuffer = Arrays.copyOf(cbuf, len); + if (closed) { + throw new IOException("Writer is closed"); + } + + final String result = new String(cbuf, off, len); if (orderedFutures.isEmpty()) { - internalWriter.write(finalCharacterBuffer, off, len); + internalWriter.write(result); } else { - Future future = es.submit(new Callable() { + Future future = new Future() { + + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + return false; + } + + @Override + public boolean isCancelled() { + return false; + } + + @Override + public boolean isDone() { + return true; + } + + @Override + public String get() throws InterruptedException, ExecutionException { + return result; + } @Override - public String call() throws Exception { - char[] chars = new char[len]; - System.arraycopy(finalCharacterBuffer, off, chars, 0, len); - return new String(chars); + public String get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, + TimeoutException { + return null; } - }); + }; orderedFutures.add(future); } diff --git a/src/test/java/com/mitchellbosecke/pebble/CoreTagsTest.java b/src/test/java/com/mitchellbosecke/pebble/CoreTagsTest.java index 311c91c59..cf15a64d0 100644 --- a/src/test/java/com/mitchellbosecke/pebble/CoreTagsTest.java +++ b/src/test/java/com/mitchellbosecke/pebble/CoreTagsTest.java @@ -610,6 +610,34 @@ public void testParallelTagWhileEvaluationContextIsChanging() throws PebbleExcep assertEquals("0123456789", writer.toString()); } + /** + * Nested parallel tags were throwing an error. + * + * @throws PebbleException + * @throws IOException + */ + @Test(timeout = 500) + public void testNestedParallel() throws PebbleException, IOException { + Loader loader = new StringLoader(); + PebbleEngine pebble = new PebbleEngine(loader); + pebble.setExecutorService(Executors.newCachedThreadPool()); + // @formatter:off + String source = "{% parallel %}" + + "{% parallel %}{{ slowObject.fourth() }}{% endparallel %} {% parallel %}{{ slowObject.first() }}{% endparallel %} " + + "{% parallel %}{{ slowObject.fourth() }}{% endparallel %} {% parallel %}{{ slowObject.first() }}{% endparallel %}" + + "{% endparallel %}"; + // @formatter:on + PebbleTemplate template = pebble.getTemplate(source); + + Writer writer = new StringWriter(); + Map context = new HashMap<>(); + + context.put("slowObject", new SlowObject()); + template.evaluate(writer, context); + + assertEquals("fourth first fourth first", writer.toString()); + } + @Test(timeout = 300) public void testIncludeWithinParallelTag() throws PebbleException, IOException { PebbleEngine pebble = new PebbleEngine(); @@ -672,6 +700,16 @@ public String third() { } return "third"; } + + public String fourth() { + try { + Thread.sleep(400); + } catch (InterruptedException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } + return "fourth"; + } } public class User {