Fix more code review comments.

Relates to elastic#4879
MaineC · Feb 17, 2014 · a2d1ba3 · a2d1ba3
1 parent 5abf744
commit a2d1ba3
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 32 deletions.
diff --git a/docs/reference/query-dsl/queries/template-query.asciidoc b/docs/reference/query-dsl/queries/template-query.asciidoc
@@ -1,6 +1,8 @@
 [[query-dsl-template-query]]
 === Template Query
 
+coming[1.1.0]
+
 A query that accepts a query template and a map of key/value pairs to fill in
 template parameters.
 

diff --git a/pom.xml b/pom.xml
@@ -506,7 +506,6 @@
                             <include>org.joda:joda-convert</include>
                             <include>io.netty:netty</include>
                             <include>com.ning:compress-lzf</include>
-                            <include>com.github.spullara.mustache.java:compiler</include>
                         </includes>
                     </artifactSet>
                     <relocations>

diff --git a/src/main/assemblies/common-bin.xml b/src/main/assemblies/common-bin.xml
@@ -9,6 +9,7 @@
                 <include>net.java.dev.jna:jna</include>
                 <include>com.spatial4j:spatial4j</include>
                 <include>com.vividsolutions:jts</include>
+                <include>com.github.spullara.mustache.java:compiler</include>
             </includes>
         </dependencySet>
         <dependencySet>
@@ -42,4 +43,4 @@
             <outputDirectory>/</outputDirectory>
         </file>
     </files>
-</component>
+</component>
diff --git a/src/main/java/org/elasticsearch/index/query/TemplateQueryParser.java b/src/main/java/org/elasticsearch/index/query/TemplateQueryParser.java
@@ -20,6 +20,7 @@
 
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
@@ -107,7 +108,7 @@ public Query parse(QueryParseContext parseContext) throws IOException {
         }
 
         ExecutableScript executable = this.scriptService.executable("mustache", template, vars);
-        String querySource = (String) executable.run();
+        BytesReference querySource = (BytesReference) executable.run();
 
         XContentParser qSourceParser = XContentFactory.xContent(querySource).createParser(querySource);
         try {

diff --git a/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngineService.java b/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngineService.java
@@ -18,22 +18,22 @@
  */
 package org.elasticsearch.script.mustache;
 
-import com.fasterxml.jackson.core.io.SegmentedStringWriter;
-import com.fasterxml.jackson.core.util.BufferRecycler;
 import com.github.mustachejava.DefaultMustacheFactory;
 import com.github.mustachejava.Mustache;
-import com.github.mustachejava.MustacheFactory;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.io.FastStringReader;
+import org.elasticsearch.common.io.UTF8StreamWriter;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.ScriptEngineService;
 import org.elasticsearch.script.SearchScript;
 import org.elasticsearch.search.lookup.SearchLookup;
 
-import java.io.StringWriter;
+import java.io.IOException;
+import java.lang.ref.SoftReference;
 import java.util.Map;
 
 /**
@@ -46,8 +46,20 @@
  */
 public class MustacheScriptEngineService extends AbstractComponent implements ScriptEngineService {
 
-    /** Factory to generate Mustache objects from. */
-    private static final MustacheFactory MFACTORY = new DefaultMustacheFactory();
+    /** Thread local UTF8StreamWriter to store template execution results in, thread local to save object creation.*/
+    private static ThreadLocal<SoftReference<UTF8StreamWriter>> utf8StreamWriter = new ThreadLocal<SoftReference<UTF8StreamWriter>>();
+
+    /** If exists, reset and return, otherwise create, reset and return a writer.*/
+    private static UTF8StreamWriter utf8StreamWriter() {
+        SoftReference<UTF8StreamWriter> ref = utf8StreamWriter.get();
+        UTF8StreamWriter writer = (ref == null) ? null : ref.get();
+        if (writer == null) {
+            writer = new UTF8StreamWriter(1024 * 4);
+            utf8StreamWriter.set(new SoftReference<UTF8StreamWriter>(writer));
+        }
+        writer.reset();
+        return writer;
+    }
 
     /**
      * @param settings automatically wired by Guice.
@@ -66,7 +78,8 @@ public MustacheScriptEngineService(Settings settings) {
      * @return a compiled template object for later execution.
      * */
     public Object compile(String template) {
-        return MFACTORY.compile(new FastStringReader(template), "query-template");
+        /** Factory to generate Mustache objects from. */
+        return (new DefaultMustacheFactory()).compile(new FastStringReader(template), "query-template");
     }
 
     /**
@@ -81,9 +94,16 @@ public Object compile(String template) {
      * @return the processed string with all given variables substitued.
      * */
     public Object execute(Object template, Map<String, Object> vars) {
-        SegmentedStringWriter result = new SegmentedStringWriter(new BufferRecycler());
-        ((Mustache) template).execute(result, vars);
-        return result.getAndClear();
+        BytesStreamOutput result = new BytesStreamOutput();
+        UTF8StreamWriter writer = utf8StreamWriter().setOutput(result);
+        ((Mustache) template).execute(writer, vars);
+        try {
+            writer.flush();
+            writer.close();
+        } catch (IOException e) {
+            logger.error("Could not execute query template: ", e);
+        }
+        return result.bytes();
     }
 
     @Override
@@ -144,9 +164,16 @@ public void setNextVar(String name, Object value) {
 
         @Override
         public Object run() {
-            StringWriter result = new StringWriter();
-            ((Mustache) mustache).execute(result, vars);
-            return result.toString();
+            BytesStreamOutput result = new BytesStreamOutput();
+            UTF8StreamWriter writer = utf8StreamWriter().setOutput(result);
+            ((Mustache) mustache).execute(writer, vars);
+            try {
+                writer.flush();
+                writer.close();
+            } catch (IOException e) {
+                logger.error("Could not execute query template: ", e);
+            }
+            return result.bytes();
         }
 
         @Override

diff --git a/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTest.java b/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTest.java
@@ -18,19 +18,20 @@
  */
 package org.elasticsearch.script.mustache;
 
-import static org.junit.Assert.*;
-
-import java.util.HashMap;
-import java.util.Map;
-
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.test.ElasticsearchTestCase;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.nio.charset.Charset;
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * Mustache based templating test
  * */
-public class MustacheScriptEngineTest {
+public class MustacheScriptEngineTest extends ElasticsearchTestCase {
     private MustacheScriptEngineService qe;
 
     private static String TEMPLATE = "GET _search {\"query\": " + "{\"boosting\": {" + "\"positive\": {\"match\": {\"body\": \"gift\"}},"
@@ -45,9 +46,10 @@ public void setup() {
     public void testSimpleParameterReplace() {
         Map<String, Object> vars = new HashMap<String, Object>();
         vars.put("boost_val", "0.3");
-        Object o = qe.execute(qe.compile(TEMPLATE), vars);
+        BytesReference o = (BytesReference) qe.execute(qe.compile(TEMPLATE), vars);
         assertEquals("GET _search {\"query\": {\"boosting\": {\"positive\": {\"match\": {\"body\": \"gift\"}},"
-                + "\"negative\": {\"term\": {\"body\": {\"value\": \"solr\"}}}, \"negative_boost\": 0.3 } }}", ((String) o));
+                + "\"negative\": {\"term\": {\"body\": {\"value\": \"solr\"}}}, \"negative_boost\": 0.3 } }}",
+                new String(o.toBytes(), Charset.forName("UTF-8")));
     }
 
 }
diff --git a/src/test/java/org/elasticsearch/script/mustache/MustacheTest.java b/src/test/java/org/elasticsearch/script/mustache/MustacheTest.java
@@ -18,22 +18,20 @@
  */
 package org.elasticsearch.script.mustache;
 
-import static org.junit.Assert.*;
+import com.github.mustachejava.DefaultMustacheFactory;
+import com.github.mustachejava.Mustache;
+import com.github.mustachejava.MustacheFactory;
+import org.elasticsearch.test.ElasticsearchTestCase;
+import org.junit.Test;
 
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.util.HashMap;
 
-import org.junit.Test;
-
-import com.github.mustachejava.DefaultMustacheFactory;
-import com.github.mustachejava.Mustache;
-import com.github.mustachejava.MustacheFactory;
-
 /**
  * Figure out how Mustache works for the simplest use case. Leaving in here for now for reference.
  * */
-public class MustacheTest {
+public class MustacheTest extends ElasticsearchTestCase {
 
     @Test
     public void test() {