Refactors ScriptQueryBuilder and Parser

Relates to #10217
Closes #12115

This PR is against the query-refactoring branch.

core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java

@@ -19,24 +19,44 @@
 
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.RandomAccessWeight;
+import org.apache.lucene.search.Weight;
+import org.apache.lucene.util.Bits;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.script.Script;
+import org.elasticsearch.script.*;
 import org.elasticsearch.script.Script.ScriptField;
+import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.io.IOException;
+import java.util.Objects;
 
 public class ScriptQueryBuilder extends AbstractQueryBuilder<ScriptQueryBuilder> {
 
     public static final String NAME = "script";
 
-    static final ScriptQueryBuilder PROTOTYPE = new ScriptQueryBuilder((Script) null);
+    static final ScriptQueryBuilder PROTOTYPE = new ScriptQueryBuilder(null);
-
-    private Script script;
 
+    private final Script script;
+    
     public ScriptQueryBuilder(Script script) {
         this.script = script;
     }
 
+    public Script script() {
+        return this.script;
+    }
+
+    @Override
+    public String getName() {
+        return NAME;
+    }
+
     @Override
     protected void doXContent(XContentBuilder builder, Params builderParams) throws IOException {
         builder.startObject(NAME);
@@ -46,7 +66,109 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder<ScriptQueryBuilder>
     }
 
     @Override
-    public String getName() {
+    protected Query doToQuery(QueryParseContext parseContext) throws IOException {
-        return NAME;
+        return new ScriptQuery(script, parseContext.scriptService(), parseContext.lookup());
+    }
+
+    @Override
+    public QueryValidationException validate() {
+        QueryValidationException validationException = null;
+        if (this.script == null) {
+            validationException = addValidationError("script cannot be null", validationException);
+        }
+        return validationException;
+    }
+
+    static class ScriptQuery extends Query {
+
+        private final Script script;
+
+        private final SearchScript searchScript;
+
+        public ScriptQuery(Script script, ScriptService scriptService, SearchLookup searchLookup) {
+            this.script = script;
+            this.searchScript = scriptService.search(searchLookup, script, ScriptContext.Standard.SEARCH);
+        }
+
+        @Override
+        public String toString(String field) {
+            StringBuilder buffer = new StringBuilder();
+            buffer.append("ScriptFilter(");
+            buffer.append(script);
+            buffer.append(")");
+            return buffer.toString();
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj)
+                return true;
+            if (!super.equals(obj))
+                return false;
+            ScriptQuery other = (ScriptQuery) obj;
+            return Objects.equals(script, other.script);
+        }
+
+        @Override
+        public int hashCode() {
+            final int prime = 31;
+            int result = super.hashCode();
+            result = prime * result + Objects.hashCode(script);
+            return result;
+        }
+
+        @Override
+        public Weight createWeight(IndexSearcher searcher, boolean needsScores) throws IOException {
+            return new RandomAccessWeight(this) {
+                @Override
+                protected Bits getMatchingDocs(final LeafReaderContext context) throws IOException {
+                    final LeafSearchScript leafScript = searchScript.getLeafSearchScript(context);
+                    return new Bits() {
+
+                        @Override
+                        public boolean get(int doc) {
+                            leafScript.setDocument(doc);
+                            Object val = leafScript.run();
+                            if (val == null) {
+                                return false;
+                            }
+                            if (val instanceof Boolean) {
+                                return (Boolean) val;
+                            }
+                            if (val instanceof Number) {
+                                return ((Number) val).longValue() != 0;
+                            }
+                            throw new IllegalArgumentException("Can't handle type [" + val + "] in script filter");
+                        }
+
+                        @Override
+                        public int length() {
+                            return context.reader().maxDoc();
+                        }
+
+                    };
+                }
+            };
+        }
+    }
+    
+    @Override
+    protected ScriptQueryBuilder doReadFrom(StreamInput in) throws IOException {
+        return new ScriptQueryBuilder(Script.readScript(in));
+    }
+
+    @Override
+    protected void doWriteTo(StreamOutput out) throws IOException {
+        script.writeTo(out);
+    }
+
+    @Override
+    protected int doHashCode() {
+        return Objects.hash(script);
+    }
+
+    @Override
+    protected boolean doEquals(ScriptQueryBuilder other) {
+        return Objects.equals(script, other.script);
     }
 }

core/src/main/java/org/elasticsearch/index/query/ScriptQueryParser.java

@@ -19,20 +19,12 @@
 
 package org.elasticsearch.index.query;
 
-import com.google.common.base.Objects;
-import org.apache.lucene.index.LeafReaderContext;
-import org.apache.lucene.search.IndexSearcher;
-import org.apache.lucene.search.Query;
-import org.apache.lucene.search.RandomAccessWeight;
-import org.apache.lucene.search.Weight;
-import org.apache.lucene.util.Bits;
-import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.script.*;
+import org.elasticsearch.script.Script;
 import org.elasticsearch.script.Script.ScriptField;
+import org.elasticsearch.script.ScriptParameterParser;
 import org.elasticsearch.script.ScriptParameterParser.ScriptParameterValue;
-import org.elasticsearch.search.lookup.SearchLookup;
 
 import java.io.IOException;
 import java.util.Map;
@@ -42,7 +34,7 @@ import static com.google.common.collect.Maps.newHashMap;
 /**
  *
  */
-public class ScriptQueryParser extends BaseQueryParserTemp {
+public class ScriptQueryParser extends BaseQueryParser {
 
     @Inject
     public ScriptQueryParser() {
@@ -54,20 +46,19 @@ public class ScriptQueryParser extends BaseQueryParserTemp {
     }
 
     @Override
-    public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
+    public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
         XContentParser parser = parseContext.parser();
         ScriptParameterParser scriptParameterParser = new ScriptParameterParser();
-
+        
-        XContentParser.Token token;
-
         // also, when caching, since its isCacheable is false, will result in loading all bit set...
         Script script = null;
         Map<String, Object> params = null;
 
         float boost = AbstractQueryBuilder.DEFAULT_BOOST;
         String queryName = null;
-        String currentFieldName = null;
 
+        XContentParser.Token token;
+        String currentFieldName = null;
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
             if (token == XContentParser.Token.FIELD_NAME) {
                 currentFieldName = parser.currentName();
@@ -108,85 +99,9 @@ public class ScriptQueryParser extends BaseQueryParserTemp {
             throw new QueryParsingException(parseContext, "script must be provided with a [script] filter");
         }
 
-        Query query = new ScriptQuery(script, parseContext.scriptService(), parseContext.lookup());
+        return new ScriptQueryBuilder(script)
-        if (queryName != null) {
+                .boost(boost)
-            parseContext.addNamedQuery(queryName, query);
+                .queryName(queryName);
-        }
-        query.setBoost(boost);
-        return query;
-    }
-
-    static class ScriptQuery extends Query {
-
-        private final Script script;
-
-        private final SearchScript searchScript;
-
-        public ScriptQuery(Script script, ScriptService scriptService, SearchLookup searchLookup) {
-            this.script = script;
-            this.searchScript = scriptService.search(searchLookup, script, ScriptContext.Standard.SEARCH);
-        }
-
-        @Override
-        public String toString(String field) {
-            StringBuilder buffer = new StringBuilder();
-            buffer.append("ScriptFilter(");
-            buffer.append(script);
-            buffer.append(")");
-            return buffer.toString();
-        }
-
-        @Override
-        public boolean equals(Object obj) {
-            if (this == obj)
-                return true;
-            if (!super.equals(obj))
-                return false;
-            ScriptQuery other = (ScriptQuery) obj;
-            return Objects.equal(script, other.script);
-        }
-
-        @Override
-        public int hashCode() {
-            final int prime = 31;
-            int result = super.hashCode();
-            result = prime * result + Objects.hashCode(script);
-            return result;
-        }
-
-        @Override
-        public Weight createWeight(IndexSearcher searcher, boolean needsScores) throws IOException {
-            return new RandomAccessWeight(this) {
-                @Override
-                protected Bits getMatchingDocs(final LeafReaderContext context) throws IOException {
-                    final LeafSearchScript leafScript = searchScript.getLeafSearchScript(context);
-                    return new Bits() {
-
-                        @Override
-                        public boolean get(int doc) {
-                            leafScript.setDocument(doc);
-                            Object val = leafScript.run();
-                            if (val == null) {
-                                return false;
-                            }
-                            if (val instanceof Boolean) {
-                                return (Boolean) val;
-                            }
-                            if (val instanceof Number) {
-                                return ((Number) val).longValue() != 0;
-                            }
-                            throw new IllegalArgumentException("Can't handle type [" + val + "] in script filter");
-                        }
-
-                        @Override
-                        public int length() {
-                            return context.reader().maxDoc();
-                        }
-
-                    };
-                }
-            };
-        }
     }
 
     @Override

core/src/test/java/org/elasticsearch/index/query/ScriptQueryBuilderTest.java

@@ -0,0 +1,59 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.index.query;
+
+import org.apache.lucene.search.Query;
+import org.elasticsearch.script.Script;
+import org.elasticsearch.script.ScriptService.ScriptType;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.hamcrest.Matchers.is;
+
+public class ScriptQueryBuilderTest extends BaseQueryTestCase<ScriptQueryBuilder> {
+
+    @Override
+    protected ScriptQueryBuilder doCreateTestQueryBuilder() {
+        String script;
+        Map<String, Object> params = null;
+        if (randomBoolean()) {
+            script = "5 * 2 > param";
+            params = new HashMap<>();
+            params.put("param", 1);
+        } else {
+            script = "5 * 2 > 2";
+        }
+        return new ScriptQueryBuilder(new Script(script, ScriptType.INLINE, "expression", params));
+    }
+
+    @Override
+    protected Query doCreateExpectedQuery(ScriptQueryBuilder queryBuilder, QueryParseContext context) throws IOException {
+        return new ScriptQueryBuilder.ScriptQuery(queryBuilder.script(), context.scriptService(), context.lookup());
+    }
+
+    @Test
+    public void testValidate() {
+        ScriptQueryBuilder scriptQueryBuilder = new ScriptQueryBuilder(null);
+        assertThat(scriptQueryBuilder.validate().validationErrors().size(), is(1));
+    }
+}