Skip to content

Commit

Permalink
Updated changelog and fixed issue with FutureWriter.
Browse files Browse the repository at this point in the history
The character buffer used in the write method of the FutureWriter
was being modified by other threads which caused overwritten outputs.
  • Loading branch information
mbosecke committed Feb 8, 2014
1 parent d5371f7 commit 97a6d5e
Show file tree
Hide file tree
Showing 7 changed files with 153 additions and 64 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## v0.2.0-beta (2014-02-08)
- Implemented named arguments.
- Added dependency on google guava for template cache.
- Split the default loader class into multiple discrete loaders.
- Added the `title` filter.
- Fixed issue where compilation mutex might not have been released.
- Fixed parsing issues if variable names were prefixed with operator names.
- Fixed issue where included templates didn't have access to context.
- Fixed issue where `if` tag could not be used directly on a boolean variable.

## v0.1.5-beta (2014-01-27)
- Fixed major bug from v0.1.4 that prevented macros from being invoked more than once

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public NodePrint(NodeExpression expression, int lineNumber) {
@Override
public void compile(Compiler compiler) {

compiler.newline().write("writer.write(printVariable(").subcompile(expression).raw("));");
compiler.write("writer.write(printVariable(").subcompile(expression).raw("));").newline();
}

}
44 changes: 22 additions & 22 deletions src/main/java/com/mitchellbosecke/pebble/node/NodeRoot.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public class NodeRoot extends AbstractNode {

private final Map<String, NodeMacro> macros;

public NodeRoot(NodeBody body, String parentFileName, Map<String, NodeBlock> blocks,
Map<String, NodeMacro> macros, String filename) {
public NodeRoot(NodeBody body, String parentFileName, Map<String, NodeBlock> blocks, Map<String, NodeMacro> macros,
String filename) {
super(0);
this.body = body;
this.parentFileName = parentFileName;
Expand All @@ -47,7 +47,7 @@ public String getFilename() {
@Override
public void compile(Compiler compiler) {
String className = compiler.getEngine().getTemplateClassName(filename);

compileMetaInformationInComments(compiler);
compileClassHeader(compiler, className);
compileConstructor(compiler, className);
Expand All @@ -56,12 +56,13 @@ public void compile(Compiler compiler) {
compileMacros(compiler);
compileClassFooter(compiler);
}
private void compileMetaInformationInComments(Compiler compiler){

private void compileMetaInformationInComments(Compiler compiler) {
compiler.write("/*").newline();
compiler.write(" * Filename: ").raw(filename).newline();
compiler.write(" * Parent filename: ").raw(parentFileName).newline();
compiler.write(" * Compiled on: ").raw(new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date())).newline();
compiler.write(" * Compiled on: ").raw(new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date()))
.newline();
compiler.write(" */").newline();
}

Expand All @@ -76,16 +77,17 @@ private void compileClassHeader(Compiler compiler, String className) {

private void compileConstructor(Compiler compiler, String className) {
compiler.newline(2).write("public ").raw(className).raw(" (String javaCode, ")
.raw(PebbleEngine.class.getName()).raw(" engine, ").raw(PebbleTemplateImpl.class.getName()).raw(" parent) {").newline();
.raw(PebbleEngine.class.getName()).raw(" engine, ").raw(PebbleTemplateImpl.class.getName())
.raw(" parent) {").newline();

compiler.indent().write("super(javaCode, engine, parent);").newline();

compiler.outdent().write("}").newline(2);
}

private void compileBuildContentFunction(Compiler compiler) {
compiler.newline(2)
.write("public void buildContent(java.io.Writer writer, Context context) throws com.mitchellbosecke.pebble.error.PebbleException, java.io.IOException {")
compiler.write(
"public void buildContent(java.io.Writer writer, Context context) throws com.mitchellbosecke.pebble.error.PebbleException, java.io.IOException {")
.newline().indent();
if (this.parentFileName != null) {
compiler.write("context.pushInheritanceChain(this);").newline();
Expand All @@ -94,29 +96,27 @@ private void compileBuildContentFunction(Compiler compiler) {
body.compile(compiler);
}

compiler.outdent().newline().write("}");
}

private void compileClassFooter(Compiler compiler) {
compiler.outdent().newline(2).write("}");
compiler.outdent().write("}").newline(2);
}

private void compileBlocks(Compiler compiler) {
compiler.newline(2).write("public void initBlocks() {").newline().indent();
compiler.write("public void initBlocks() {").newline().indent();
for (NodeBlock block : blocks.values()) {
compiler.newline().subcompile(block);
compiler.subcompile(block).newline();
}
compiler.outdent().newline().write("}");
compiler.outdent().newline().write("}").newline(2);
}

private void compileMacros(Compiler compiler) {
compiler.newline(2).write("public void initMacros() {").newline().indent();

compiler.write("public void initMacros() {").newline().indent();
for (NodeMacro macro : macros.values()) {
compiler.newline(2).subcompile(macro);
compiler.subcompile(macro).newline();
}

compiler.outdent().newline().write("}");
compiler.outdent().newline().write("}").newline(2);
}

private void compileClassFooter(Compiler compiler) {
compiler.outdent().write("}");
}

public boolean hasParent() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public NodeText(String data, int lineNumber) {

@Override
public void compile(Compiler compiler) {
compiler.newline().write("writer.write(").string(getData()).raw(");");
compiler.write("writer.write(").string(getData()).raw(");").newline();
}

public String getData() {
Expand Down
13 changes: 11 additions & 2 deletions src/main/java/com/mitchellbosecke/pebble/utils/FutureWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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.concurrent.ExecutionException;
Expand Down Expand Up @@ -40,15 +41,23 @@ public void enqueue(Future<String> future) throws IOException {

@Override
public void write(final char[] cbuf, final int off, final int len) throws IOException {

/*
* We need to make a copy of the character buffer because the Writer
* class will continue to reuse it in other threads which might
* overwrite the contents.
*/
final char[] finalCharacterBuffer = Arrays.copyOf(cbuf, len);

if (orderedFutures.isEmpty()) {
internalWriter.write(cbuf, off, len);
internalWriter.write(finalCharacterBuffer, off, len);
} else {
Future<String> future = es.submit(new Callable<String>() {

@Override
public String call() throws Exception {
char[] chars = new char[len];
System.arraycopy(cbuf, off, chars, 0, len);
System.arraycopy(finalCharacterBuffer, off, chars, 0, len);
return new String(chars);
}

Expand Down
87 changes: 49 additions & 38 deletions src/test/java/com/mitchellbosecke/pebble/CoreTagsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,21 @@ public void testIf() throws PebbleException, IOException {
assertEquals("no", writer.toString());
}

@Test
public void testIfWithDirectProperty() throws PebbleException, IOException {
Loader loader = new StringLoader();
PebbleEngine pebble = new PebbleEngine(loader);
pebble.setStrictVariables(false);

String source = "{% if variable %}yes{% else %}no{% endif %}";
PebbleTemplate template = pebble.compile(source);
Map<String, Object> context = new HashMap<>();
context.put("variable", true);

Writer writer = new StringWriter();
template.evaluate(writer, context);
assertEquals("yes", writer.toString());
}
@Test
public void testIfWithDirectProperty() throws PebbleException, IOException {
Loader loader = new StringLoader();
PebbleEngine pebble = new PebbleEngine(loader);
pebble.setStrictVariables(false);

String source = "{% if variable %}yes{% else %}no{% endif %}";
PebbleTemplate template = pebble.compile(source);
Map<String, Object> context = new HashMap<>();
context.put("variable", true);

Writer writer = new StringWriter();
template.evaluate(writer, context);
assertEquals("yes", writer.toString());
}

@Test
public void testFlush() throws PebbleException, IOException {
Expand Down Expand Up @@ -99,7 +99,7 @@ public void testFor() throws PebbleException, IOException {
template.evaluate(writer, context);
assertEquals("[2]0Alex1Bob", writer.toString());
}

/**
* Issue #15
*
Expand All @@ -125,16 +125,15 @@ public void testForIteratingOverProperty() throws PebbleException, IOException {
template.evaluate(writer, context);
assertEquals("AlexBob", writer.toString());
}



@Test
public void testForWithNullIterable() throws PebbleException, IOException {
Loader loader = new StringLoader();
PebbleEngine pebble = new PebbleEngine(loader);

String source = "{% for user in users %}{{ loop.index }}{% endfor %}";
PebbleTemplate template = pebble.compile(source);

Map<String, Object> context = new HashMap<>();
context.put("users", null);

Expand Down Expand Up @@ -190,21 +189,22 @@ public void testMacro() throws PebbleException, IOException {
template.evaluate(writer);
assertEquals(" <input name=\"company\" value=\"google\" type=\"text\" />\n", writer.toString());
}
@Test(expected=ParserException.class)

@Test(expected = ParserException.class)
public void testMacrosWithSameName() throws PebbleException, IOException {
Loader loader = new StringLoader();
PebbleEngine pebble = new PebbleEngine(loader);
PebbleTemplate template = pebble.compile("{{ test() }}{% macro test(one) %}ONE{% endmacro %}{% macro test(one,two) %}TWO{% endmacro %}");
PebbleTemplate template = pebble
.compile("{{ test() }}{% macro test(one) %}ONE{% endmacro %}{% macro test(one,two) %}TWO{% endmacro %}");

Writer writer = new StringWriter();
template.evaluate(writer);
assertEquals(" <input name=\"company\" value=\"google\" type=\"text\" />\n", writer.toString());
}

/**
* There was an issue where the second invokation of a macro
* did not have access to the original arguments any more.
* There was an issue where the second invokation of a macro did not have
* access to the original arguments any more.
*
* @throws PebbleException
* @throws IOException
Expand All @@ -217,18 +217,19 @@ public void testMacroInvokedTwice() throws PebbleException, IOException {
template.evaluate(writer);
assertEquals("onetwo", writer.toString());
}

@Test
public void testMacroInvokationWithoutAllArguments() throws PebbleException, IOException {
Loader loader = new StringLoader();
PebbleEngine pebble = new PebbleEngine(loader);
PebbleTemplate template = pebble.compile("{{ test('1') }}{% macro test(one,two) %}{{ one }}{{ two }}{% endmacro %}");
PebbleTemplate template = pebble
.compile("{{ test('1') }}{% macro test(one,two) %}{{ one }}{{ two }}{% endmacro %}");

Writer writer = new StringWriter();
template.evaluate(writer);
assertEquals("1", writer.toString());
}

@Test(expected = PebbleException.class)
public void testDuplicateMacro() throws PebbleException, IOException {
PebbleTemplate template = pebble.compile("template.macroDuplicate.peb");
Expand Down Expand Up @@ -270,7 +271,7 @@ public void testInclude() throws PebbleException, IOException {
template.evaluate(writer);
assertEquals("TEMPLATE2\nTEMPLATE1\nTEMPLATE2\n", writer.toString());
}

/**
* Issue #16
*
Expand All @@ -286,10 +287,10 @@ public void testIncludePropagatesContext() throws PebbleException, IOException {
template.evaluate(writer, context);
assertEquals("Mitchell", writer.toString());
}

/**
* Ensures that when including a template it is safe to
* have conflicting block names.
* Ensures that when including a template it is safe to have conflicting
* block names.
*
* @throws PebbleException
* @throws IOException
Expand Down Expand Up @@ -318,27 +319,27 @@ public void testSet() throws PebbleException, IOException {
assertEquals("alex", writer.toString());
}

@Test(timeout = 4000)
@Test(timeout = 3000)
public void testParallel() throws PebbleException, IOException {
Loader loader = new StringLoader();
PebbleEngine pebble = new PebbleEngine(loader);
pebble.setExecutorService(Executors.newCachedThreadPool());

String source = "beginning {% parallel %}{{ slowObject.first }}{% endparallel %} middle {% parallel %}{{ slowObject.second }}{% endparallel %} end";
String source = "beginning {% parallel %}{{ slowObject.first }}{% endparallel %} middle {% parallel %}{{ slowObject.second }}{% endparallel %} end {% parallel %}{{ slowObject.third }}{% endparallel %}";
PebbleTemplate template = pebble.compile(source);

Writer writer = new StringWriter();
Map<String, Object> context = new HashMap<>();
context.put("slowObject", new SlowObject());
template.evaluate(writer, context);

assertEquals("beginning first middle second end", writer.toString());
assertEquals("beginning first middle second end third", writer.toString());

}

public class SlowObject {
public String first() {
try {
Thread.sleep(3000);
Thread.sleep(2000);
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
Expand All @@ -355,6 +356,16 @@ public String second() {
}
return "second";
}

public String third() {
try {
Thread.sleep(2000);
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
return "third";
}
}

public class User {
Expand All @@ -368,7 +379,7 @@ public String getUsername() {
return username;
}
}

public class Classroom {
private List<User> users = new ArrayList<>();

Expand Down
Loading

0 comments on commit 97a6d5e

Please sign in to comment.