Merge pull request #11274 from MaineC/feature/simple-query-string-refactoring

Refactors SimpleQueryStringBuilder/Parser

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

@@ -29,6 +29,7 @@ import org.apache.lucene.util.BytesRef;
 import java.io.IOException;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 
 /**
  * Wrapper class for Lucene's SimpleQueryParser that allows us to redefine
@@ -202,51 +203,102 @@ public class SimpleQueryParser extends org.apache.lucene.queryparser.simple.Simp
             return new PrefixQuery(new Term(field, termStr));
         }
     }
-
     /**
      * Class encapsulating the settings for the SimpleQueryString query, with
      * their default values
      */
-    public static class Settings {
+    static class Settings {
-        private Locale locale = Locale.ROOT;
+        /** Locale to use for parsing. */
-        private boolean lowercaseExpandedTerms = true;
+        private Locale locale = SimpleQueryStringBuilder.DEFAULT_LOCALE;
-        private boolean lenient = false;
+        /** Specifies whether parsed terms should be lowercased. */
-        private boolean analyzeWildcard = false;
+        private boolean lowercaseExpandedTerms = SimpleQueryStringBuilder.DEFAULT_LOWERCASE_EXPANDED_TERMS;
+        /** Specifies whether lenient query parsing should be used. */
+        private boolean lenient = SimpleQueryStringBuilder.DEFAULT_LENIENT;
+        /** Specifies whether wildcards should be analyzed. */
+        private boolean analyzeWildcard = SimpleQueryStringBuilder.DEFAULT_ANALYZE_WILDCARD;
 
+        /**
+         * Generates default {@link Settings} object (uses ROOT locale, does
+         * lowercase terms, no lenient parsing, no wildcard analysis).
+         * */
         public Settings() {
-
         }
 
-        public void locale(Locale locale) {
+        public Settings(Locale locale, Boolean lowercaseExpandedTerms, Boolean lenient, Boolean analyzeWildcard) {
             this.locale = locale;
+            this.lowercaseExpandedTerms = lowercaseExpandedTerms;
+            this.lenient = lenient;
+            this.analyzeWildcard = analyzeWildcard;
         }
 
+        /** Specifies the locale to use for parsing, Locale.ROOT by default. */
+        public void locale(Locale locale) {
+            this.locale = (locale != null) ? locale : SimpleQueryStringBuilder.DEFAULT_LOCALE;
+        }
+
+        /** Returns the locale to use for parsing. */
         public Locale locale() {
             return this.locale;
         }
 
+        /**
+         * Specifies whether to lowercase parse terms, defaults to true if
+         * unset.
+         */
         public void lowercaseExpandedTerms(boolean lowercaseExpandedTerms) {
             this.lowercaseExpandedTerms = lowercaseExpandedTerms;
         }
 
+        /** Returns whether to lowercase parse terms. */
         public boolean lowercaseExpandedTerms() {
             return this.lowercaseExpandedTerms;
         }
 
+        /** Specifies whether to use lenient parsing, defaults to false. */
         public void lenient(boolean lenient) {
             this.lenient = lenient;
         }
 
+        /** Returns whether to use lenient parsing. */
         public boolean lenient() {
             return this.lenient;
         }
 
+        /** Specifies whether to analyze wildcards. Defaults to false if unset. */
         public void analyzeWildcard(boolean analyzeWildcard) {
             this.analyzeWildcard = analyzeWildcard;
         }
 
+        /** Returns whether to analyze wildcards. */
         public boolean analyzeWildcard() {
             return analyzeWildcard;
         }
+
+        @Override
+        public int hashCode() {
+            // checking the return value of toLanguageTag() for locales only.
+            // For further reasoning see
+            // https://issues.apache.org/jira/browse/LUCENE-4021
+            return Objects.hash(locale.toLanguageTag(), lowercaseExpandedTerms, lenient, analyzeWildcard);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj == null || getClass() != obj.getClass()) {
+                return false;
+            }
+            Settings other = (Settings) obj;
+
+            // checking the return value of toLanguageTag() for locales only.
+            // For further reasoning see
+            // https://issues.apache.org/jira/browse/LUCENE-4021
+            return (Objects.equals(locale.toLanguageTag(), other.locale.toLanguageTag())
+                    && Objects.equals(lowercaseExpandedTerms, other.lowercaseExpandedTerms) 
+                    && Objects.equals(lenient, other.lenient)
+                    && Objects.equals(analyzeWildcard, other.analyzeWildcard));
+        }
     }
 }

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

@@ -19,151 +19,352 @@
 
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.index.query.SimpleQueryParser.Settings;
 
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
+import java.util.TreeMap;
 
 /**
- * SimpleQuery is a query parser that acts similar to a query_string
+ * SimpleQuery is a query parser that acts similar to a query_string query, but
- * query, but won't throw exceptions for any weird string syntax.
+ * won't throw exceptions for any weird string syntax.
+ * 
+ * For more detailed explanation of the query string syntax see also the <a
+ * href=
+ * "https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-simple-query-string-query.html"
+ * > online documentation</a>.
  */
 public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQueryStringBuilder> implements BoostableQueryBuilder<SimpleQueryStringBuilder> {
+    /** Default locale used for parsing.*/
+    public static final Locale DEFAULT_LOCALE = Locale.ROOT;
+    /** Default for lowercasing parsed terms.*/
+    public static final boolean DEFAULT_LOWERCASE_EXPANDED_TERMS = true;
+    /** Default for using lenient query parsing.*/
+    public static final boolean DEFAULT_LENIENT = false;
+    /** Default for wildcard analysis.*/
+    public static final boolean DEFAULT_ANALYZE_WILDCARD = false;
+    /** Default for boost to apply to resulting Lucene query. Defaults to 1.0*/
+    public static final float DEFAULT_BOOST = 1.0f; 
+    /** Default for default operator to use for linking boolean clauses.*/
+    public static final Operator DEFAULT_OPERATOR = Operator.OR;
+    /** Default for search flags to use. */
+    public static final int DEFAULT_FLAGS = SimpleQueryStringFlag.ALL.value;
+    /** Name for (de-)serialization. */
     public static final String NAME = "simple_query_string";
-    private Map<String, Float> fields = new HashMap<>();
+    /** Query text to parse. */
-    private String analyzer;
-    private Operator operator;
     private final String queryText;
+    /** Boost to apply to resulting Lucene query. Defaults to 1.0*/
+    private float boost = DEFAULT_BOOST;
+    /**
+     * Fields to query against. If left empty will query default field,
+     * currently _ALL. Uses a TreeMap to hold the fields so boolean clauses are
+     * always sorted in same order for generated Lucene query for easier
+     * testing.
+     * 
+     * Can be changed back to HashMap once https://issues.apache.org/jira/browse/LUCENE-6305 is fixed.
+     */
+    private final Map<String, Float> fieldsAndWeights = new TreeMap<>();
+    /** If specified, analyzer to use to parse the query text, defaults to registered default in toQuery. */
+    private String analyzer;
+    /** Name of the query. Optional.*/
     private String queryName;
+    /** Default operator to use for linking boolean clauses. Defaults to OR according to docs. */
+    private Operator defaultOperator = DEFAULT_OPERATOR;
+    /** If result is a boolean query, minimumShouldMatch parameter to apply. Ignored otherwise. */
     private String minimumShouldMatch;
-    private int flags = -1;
+    /** Any search flags to be used, ALL by default. */
-    private float boost = -1.0f;
+    private int flags = DEFAULT_FLAGS;
-    private Boolean lowercaseExpandedTerms;
+
-    private Boolean lenient;
+    /** Further search settings needed by the ES specific query string parser only. */
-    private Boolean analyzeWildcard;
+    private Settings settings = new Settings();
-    private Locale locale;
+
     static final SimpleQueryStringBuilder PROTOTYPE = new SimpleQueryStringBuilder(null);
 
-    /**
+    /** Operators available for linking boolean clauses. */
-     * Operators for the default_operator
+    // Move out after #11345 is in.
-     */
     public static enum Operator {
         AND,
-        OR
+        OR;
+
+        public static Operator parseFromInt(int ordinal) {
+            switch (ordinal) {
+                case 0:
+                    return AND;
+                case 1:
+                    return OR;
+                default:
+                    throw new IllegalArgumentException("cannot parse Operator from ordinal " + ordinal);
+            }
+
+        }
     }
 
-    /**
+    /** Construct a new simple query with this query string. */
-     * Construct a new simple query with the given text
+    public SimpleQueryStringBuilder(String queryText) {
-     */
+        this.queryText = queryText;
-    public SimpleQueryStringBuilder(String text) {
-        this.queryText = text;
     }
 
-    /** Set the boost of this query. */
     @Override
     public SimpleQueryStringBuilder boost(float boost) {
         this.boost = boost;
         return this;
     }
-    
+
-    /** Returns the boost of this query. */
+    /** Returns the boost to apply to resulting Lucene query.*/
     public float boost() {
         return this.boost;
     }
+    /** Returns the text to parse the query from. */
+    public String text() {
+        return this.queryText;
+    }
 
-    /**
+    /** Add a field to run the query against. */
-     * Add a field to run the query against
-     */
     public SimpleQueryStringBuilder field(String field) {
-        this.fields.put(field, null);
+        if (Strings.isEmpty(field)) {
+            throw new IllegalArgumentException("supplied field is null or empty.");
+        }
+        this.fieldsAndWeights.put(field, 1.0f);
         return this;
     }
 
-    /**
+    /** Add a field to run the query against with a specific boost. */
-     * Add a field to run the query against with a specific boost
-     */
     public SimpleQueryStringBuilder field(String field, float boost) {
-        this.fields.put(field, boost);
+        if (Strings.isEmpty(field)) {
+            throw new IllegalArgumentException("supplied field is null or empty.");
+        }
+        this.fieldsAndWeights.put(field, boost);
         return this;
     }
 
-    /**
+    /** Add several fields to run the query against with a specific boost. */
-     * Specify a name for the query
+    public SimpleQueryStringBuilder fields(Map<String, Float> fields) {
-     */
+        this.fieldsAndWeights.putAll(fields);
-    public SimpleQueryStringBuilder queryName(String name) {
-        this.queryName = name;
         return this;
     }
 
-    /**
+    /** Returns the fields including their respective boosts to run the query against. */
-     * Specify an analyzer to use for the query
+    public Map<String, Float> fields() {
-     */
+        return this.fieldsAndWeights;
+    }
+
+    /** Specify an analyzer to use for the query. */
     public SimpleQueryStringBuilder analyzer(String analyzer) {
         this.analyzer = analyzer;
         return this;
     }
 
-    /**
+    /** Returns the analyzer to use for the query. */
-     * Specify the default operator for the query. Defaults to "OR" if no
+    public String analyzer() {
-     * operator is specified
+        return this.analyzer;
-     */
-    public SimpleQueryStringBuilder defaultOperator(Operator defaultOperator) {
-        this.operator = defaultOperator;
-        return this;
     }
 
     /**
-     * Specify the enabled features of the SimpleQueryString.
+     * Specify the default operator for the query. Defaults to "OR" if no
+     * operator is specified.
+     */
+    public SimpleQueryStringBuilder defaultOperator(Operator defaultOperator) {
+        this.defaultOperator = (defaultOperator != null) ? defaultOperator : DEFAULT_OPERATOR;
+        return this;
+    }
+
+    /** Returns the default operator for the query. */
+    public Operator defaultOperator() {
+        return this.defaultOperator;
+    }
+
+    /**
+     * Specify the enabled features of the SimpleQueryString. Defaults to ALL if
+     * none are specified.
      */
     public SimpleQueryStringBuilder flags(SimpleQueryStringFlag... flags) {
-        int value = 0;
+        if (flags != null && flags.length > 0) {
-        if (flags.length == 0) {
+            int value = 0;
-            value = SimpleQueryStringFlag.ALL.value;
-        } else {
             for (SimpleQueryStringFlag flag : flags) {
                 value |= flag.value;
             }
+            this.flags = value;
+        } else {
+            this.flags = DEFAULT_FLAGS;
         }
-        this.flags = value;
+
         return this;
     }
 
+    /** For testing and serialisation only. */
+    SimpleQueryStringBuilder flags(int flags) {
+        this.flags = flags;
+        return this;
+    }
+
+    /** For testing only: Return the flags set for this query. */
+    int flags() {
+        return this.flags;
+    }
+
+    /** Set the name for this query. */
+    public SimpleQueryStringBuilder queryName(String queryName) {
+        this.queryName = queryName;
+        return this;
+    }
+
+    /** Returns the name for this query. */
+    public String queryName() {
+        return queryName;
+    }
+
+    /**
+     * Specifies whether parsed terms for this query should be lower-cased.
+     * Defaults to true if not set.
+     */
     public SimpleQueryStringBuilder lowercaseExpandedTerms(boolean lowercaseExpandedTerms) {
-        this.lowercaseExpandedTerms = lowercaseExpandedTerms;
+        this.settings.lowercaseExpandedTerms(lowercaseExpandedTerms);
         return this;
     }
 
+    /** Returns whether parsed terms should be lower cased for this query. */
+    public boolean lowercaseExpandedTerms() {
+        return this.settings.lowercaseExpandedTerms();
+    }
+
+    /** Specifies the locale for parsing terms. Defaults to ROOT if none is set. */
     public SimpleQueryStringBuilder locale(Locale locale) {
-        this.locale = locale;
+        this.settings.locale(locale);
         return this;
     }
 
+    /** Returns the locale for parsing terms for this query. */
+    public Locale locale() {
+        return this.settings.locale();
+    }
+
+    /** Specifies whether query parsing should be lenient. Defaults to false. */
     public SimpleQueryStringBuilder lenient(boolean lenient) {
-        this.lenient = lenient;
+        this.settings.lenient(lenient);
         return this;
     }
 
+    /** Returns whether query parsing should be lenient. */
+    public boolean lenient() {
+        return this.settings.lenient();
+    }
+
+    /** Specifies whether wildcards should be analyzed. Defaults to false. */
     public SimpleQueryStringBuilder analyzeWildcard(boolean analyzeWildcard) {
-        this.analyzeWildcard = analyzeWildcard;
+        this.settings.analyzeWildcard(DEFAULT_ANALYZE_WILDCARD);
         return this;
     }
 
+    /** Returns whether wildcards should by analyzed. */
+    public boolean analyzeWildcard() {
+        return this.settings.analyzeWildcard();
+    }
+
+    /**
+     * Specifies the minimumShouldMatch to apply to the resulting query should
+     * that be a Boolean query.
+     */
     public SimpleQueryStringBuilder minimumShouldMatch(String minimumShouldMatch) {
         this.minimumShouldMatch = minimumShouldMatch;
         return this;
     }
 
+    /**
+     * Returns the minimumShouldMatch to apply to the resulting query should
+     * that be a Boolean query.
+     */
+    public String minimumShouldMatch() {
+        return minimumShouldMatch;
+    }
+
+    /** 
+     * {@inheritDoc} 
+     * 
+     * Checks that mandatory queryText is neither null nor empty.
+     * */
+    @Override
+    public QueryValidationException validate() {
+        QueryValidationException validationException = null;
+
+        // Query text is required
+        if (queryText == null) {
+            validationException = QueryValidationException.addValidationError("[" + SimpleQueryStringBuilder.NAME + "] query text missing",
+                    validationException);
+        }
+
+        return validationException;
+    }
+
+    @Override
+    public Query toQuery(QueryParseContext parseContext) {
+        // Use the default field (_all) if no fields specified
+        if (fieldsAndWeights.isEmpty()) {
+            String field = parseContext.defaultField();
+            fieldsAndWeights.put(field, 1.0F);
+        }
+
+        // Use standard analyzer by default if none specified
+        Analyzer luceneAnalyzer;
+        if (analyzer == null) {
+            luceneAnalyzer = parseContext.mapperService().searchAnalyzer();
+        } else {
+            luceneAnalyzer = parseContext.analysisService().analyzer(analyzer);
+            if (luceneAnalyzer == null) {
+                throw new QueryParsingException(parseContext, "[" + SimpleQueryStringBuilder.NAME + "] analyzer [" + analyzer
+                        + "] not found");
+            }
+
+        }
+        SimpleQueryParser sqp = new SimpleQueryParser(luceneAnalyzer, fieldsAndWeights, flags, settings);
+
+        if (defaultOperator != null) {
+            switch (defaultOperator) {
+                case OR:
+                    sqp.setDefaultOperator(Occur.SHOULD);
+                    break;
+                case AND:
+                    sqp.setDefaultOperator(Occur.MUST);
+                    break;
+            }
+        }
+
+        Query query = sqp.parse(queryText);
+        if (queryName != null) {
+            parseContext.addNamedQuery(queryName, query);
+        }
+
+        if (minimumShouldMatch != null && query instanceof BooleanQuery) {
+            Queries.applyMinimumShouldMatch((BooleanQuery) query, minimumShouldMatch);
+        }
+
+        // safety check - https://github.com/elastic/elasticsearch/pull/11696#discussion-diff-32532468 
+        if (query != null) {
+            query.setBoost(boost);
+        }
+        return query;
+    }
+
     @Override
     public void doXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject(NAME);
 
         builder.field("query", queryText);
 
-        if (fields.size() > 0) {
+        if (fieldsAndWeights.size() > 0) {
             builder.startArray("fields");
-            for (Map.Entry<String, Float> entry : fields.entrySet()) {
+            for (Map.Entry<String, Float> entry : fieldsAndWeights.entrySet()) {
                 String field = entry.getKey();
                 Float boost = entry.getValue();
                 if (boost != null) {
@@ -175,33 +376,16 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
             builder.endArray();
         }
 
-        if (flags != -1) {
-            builder.field("flags", flags);
-        }
-
         if (analyzer != null) {
             builder.field("analyzer", analyzer);
         }
 
-        if (operator != null) {
+        builder.field("flags", flags);
-            builder.field("default_operator", operator.name().toLowerCase(Locale.ROOT));
+        builder.field("default_operator", defaultOperator.name().toLowerCase(Locale.ROOT));
-        }
+        builder.field("lowercase_expanded_terms", settings.lowercaseExpandedTerms());
-
+        builder.field("lenient", settings.lenient());
-        if (lowercaseExpandedTerms != null) {
+        builder.field("analyze_wildcard", settings.analyzeWildcard());
-            builder.field("lowercase_expanded_terms", lowercaseExpandedTerms);
+        builder.field("locale", (settings.locale().toLanguageTag()));
-        }
-
-        if (lenient != null) {
-            builder.field("lenient", lenient);
-        }
-
-        if (analyzeWildcard != null) {
-            builder.field("analyze_wildcard", analyzeWildcard);
-        }
-
-        if (locale != null) {
-            builder.field("locale", locale.toString());
-        }
 
         if (queryName != null) {
             builder.field("_name", queryName);
@@ -210,7 +394,7 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
         if (minimumShouldMatch != null) {
             builder.field("minimum_should_match", minimumShouldMatch);
         }
-        
+
         if (boost != -1.0f) {
             builder.field("boost", boost);
         }
@@ -222,4 +406,76 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
     public String getName() {
         return NAME;
     }
+
+    @Override
+    public SimpleQueryStringBuilder readFrom(StreamInput in) throws IOException {
+        SimpleQueryStringBuilder result = new SimpleQueryStringBuilder(in.readString());
+        result.boost = in.readFloat();
+        int size = in.readInt();
+        Map<String, Float> fields = new HashMap<>();
+        for (int i = 0; i < size; i++) {
+            String field = in.readString();
+            Float weight = in.readFloat();
+            fields.put(field, weight);
+        }
+        result.fieldsAndWeights.putAll(fields);
+
+        result.flags = in.readInt();
+        result.analyzer = in.readOptionalString();
+
+        result.defaultOperator = Operator.parseFromInt(in.readInt());
+        result.settings.lowercaseExpandedTerms(in.readBoolean());
+        result.settings.lenient(in.readBoolean());
+        result.settings.analyzeWildcard(in.readBoolean());
+
+        String localeStr = in.readString();
+        result.settings.locale(Locale.forLanguageTag(localeStr));
+
+        result.queryName = in.readOptionalString();
+        result.minimumShouldMatch = in.readOptionalString();
+
+        return result;
+    }
+
+    @Override
+    public void writeTo(StreamOutput out) throws IOException {
+        out.writeString(queryText);
+        out.writeFloat(boost);
+        out.writeInt(fieldsAndWeights.size());
+        for (Map.Entry<String, Float> entry : fieldsAndWeights.entrySet()) {
+            out.writeString(entry.getKey());
+            out.writeFloat(entry.getValue());
+        }
+        out.writeInt(flags);
+        out.writeOptionalString(analyzer);
+        out.writeInt(defaultOperator.ordinal());
+        out.writeBoolean(settings.lowercaseExpandedTerms());
+        out.writeBoolean(settings.lenient());
+        out.writeBoolean(settings.analyzeWildcard());
+        out.writeString(settings.locale().toLanguageTag());
+
+        out.writeOptionalString(queryName);
+        out.writeOptionalString(minimumShouldMatch);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(fieldsAndWeights, analyzer, defaultOperator, queryText, queryName, minimumShouldMatch, settings);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj == null || getClass() != obj.getClass()) {
+            return false;
+        }
+        SimpleQueryStringBuilder other = (SimpleQueryStringBuilder) obj;
+        return Objects.equals(fieldsAndWeights, other.fieldsAndWeights) && Objects.equals(analyzer, other.analyzer)
+                && Objects.equals(defaultOperator, other.defaultOperator) && Objects.equals(queryText, other.queryText)
+                && Objects.equals(queryName, other.queryName) && Objects.equals(minimumShouldMatch, other.minimumShouldMatch)
+                && Objects.equals(settings, other.settings);
+    }
 }
+

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

@@ -19,22 +19,15 @@
 
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.analysis.Analyzer;
-import org.apache.lucene.search.BooleanClause;
-import org.apache.lucene.search.BooleanQuery;
-import org.apache.lucene.search.Query;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.util.LocaleUtils;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
+import org.elasticsearch.index.query.SimpleQueryStringBuilder.Operator;
 
 import java.io.IOException;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
@@ -70,7 +63,7 @@ import java.util.Map;
  * {@code fields} - fields to search, defaults to _all if not set, allows
  * boosting a field with ^n
  */
-public class SimpleQueryStringParser extends BaseQueryParserTemp {
+public class SimpleQueryStringParser extends BaseQueryParser {
 
     @Inject
     public SimpleQueryStringParser(Settings settings) {
@@ -83,7 +76,7 @@ public class SimpleQueryStringParser extends BaseQueryParserTemp {
     }
 
     @Override
-    public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
+    public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
         XContentParser parser = parseContext.parser();
 
         String currentFieldName = null;
@@ -92,11 +85,14 @@ public class SimpleQueryStringParser extends BaseQueryParserTemp {
         String queryName = null;
         String field = null;
         String minimumShouldMatch = null;
-        Map<String, Float> fieldsAndWeights = null;
+        Map<String, Float> fieldsAndWeights = new HashMap<>();
-        BooleanClause.Occur defaultOperator = null;
+        Operator defaultOperator = null;
-        Analyzer analyzer = null;
+        String analyzerName = null;
         int flags = -1;
-        SimpleQueryParser.Settings sqsSettings = new SimpleQueryParser.Settings();
+        boolean lenient = SimpleQueryStringBuilder.DEFAULT_LENIENT;
+        boolean lowercaseExpandedTerms = SimpleQueryStringBuilder.DEFAULT_LOWERCASE_EXPANDED_TERMS;
+        boolean analyzeWildcard = SimpleQueryStringBuilder.DEFAULT_ANALYZE_WILDCARD;
+        Locale locale = null;
 
         XContentParser.Token token;
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
@@ -121,10 +117,6 @@ public class SimpleQueryStringParser extends BaseQueryParserTemp {
                             fField = parser.text();
                         }
 
-                        if (fieldsAndWeights == null) {
-                            fieldsAndWeights = new HashMap<>();
-                        }
-
                         if (Regex.isSimpleMatchPattern(fField)) {
                             for (String fieldName : parseContext.mapperService().simpleMatchToIndexNames(fField)) {
                                 fieldsAndWeights.put(fieldName, fBoost);
@@ -149,18 +141,15 @@ public class SimpleQueryStringParser extends BaseQueryParserTemp {
                 } else if ("boost".equals(currentFieldName)) {
                     boost = parser.floatValue();
                 } else if ("analyzer".equals(currentFieldName)) {
-                    analyzer = parseContext.analysisService().analyzer(parser.text());
+                    analyzerName = parser.text();
-                    if (analyzer == null) {
-                        throw new QueryParsingException(parseContext, "[" + SimpleQueryStringBuilder.NAME + "] analyzer [" + parser.text() + "] not found");
-                    }
                 } else if ("field".equals(currentFieldName)) {
                     field = parser.text();
                 } else if ("default_operator".equals(currentFieldName) || "defaultOperator".equals(currentFieldName)) {
                     String op = parser.text();
                     if ("or".equalsIgnoreCase(op)) {
-                        defaultOperator = BooleanClause.Occur.SHOULD;
+                        defaultOperator = Operator.OR;
                     } else if ("and".equalsIgnoreCase(op)) {
-                        defaultOperator = BooleanClause.Occur.MUST;
+                        defaultOperator = Operator.AND;
                     } else {
                         throw new QueryParsingException(parseContext, "[" + SimpleQueryStringBuilder.NAME + "] default operator [" + op + "] is not allowed");
                     }
@@ -177,14 +166,13 @@ public class SimpleQueryStringParser extends BaseQueryParserTemp {
                     }
                 } else if ("locale".equals(currentFieldName)) {
                     String localeStr = parser.text();
-                    Locale locale = LocaleUtils.parse(localeStr);
+                    locale = Locale.forLanguageTag(localeStr);
-                    sqsSettings.locale(locale);
                 } else if ("lowercase_expanded_terms".equals(currentFieldName)) {
-                    sqsSettings.lowercaseExpandedTerms(parser.booleanValue());
+                    lowercaseExpandedTerms = parser.booleanValue();
                 } else if ("lenient".equals(currentFieldName)) {
-                    sqsSettings.lenient(parser.booleanValue());
+                    lenient = parser.booleanValue();
                 } else if ("analyze_wildcard".equals(currentFieldName)) {
-                    sqsSettings.analyzeWildcard(parser.booleanValue());
+                    analyzeWildcard = parser.booleanValue();
                 } else if ("_name".equals(currentFieldName)) {
                     queryName = parser.text();
                 } else if ("minimum_should_match".equals(currentFieldName)) {
@@ -199,45 +187,18 @@ public class SimpleQueryStringParser extends BaseQueryParserTemp {
         if (queryBody == null) {
             throw new QueryParsingException(parseContext, "[" + SimpleQueryStringBuilder.NAME + "] query text missing");
         }
-        
+
         // Support specifying only a field instead of a map
         if (field == null) {
             field = currentFieldName;
         }
 
-        // Use the default field (_all) if no fields specified
+        SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder(queryBody);
-        if (fieldsAndWeights == null) {
+        qb.boost(boost).fields(fieldsAndWeights).analyzer(analyzerName).queryName(queryName).minimumShouldMatch(minimumShouldMatch);
-            field = parseContext.defaultField();
+        qb.flags(flags).defaultOperator(defaultOperator).locale(locale).lowercaseExpandedTerms(lowercaseExpandedTerms);
-        }
+        qb.lenient(lenient).analyzeWildcard(analyzeWildcard).boost(boost);
 
-        // Use standard analyzer by default
+        return qb;
-        if (analyzer == null) {
-            analyzer = parseContext.mapperService().searchAnalyzer();
-        }
-
-        if (fieldsAndWeights == null) {
-            fieldsAndWeights = Collections.singletonMap(field, 1.0F);
-        }
-        SimpleQueryParser sqp = new SimpleQueryParser(analyzer, fieldsAndWeights, flags, sqsSettings);
-
-        if (defaultOperator != null) {
-            sqp.setDefaultOperator(defaultOperator);
-        }
-
-        Query query = sqp.parse(queryBody);
-        if (queryName != null) {
-            parseContext.addNamedQuery(queryName, query);
-        }
-
-        if (minimumShouldMatch != null && query instanceof BooleanQuery) {
-            Queries.applyMinimumShouldMatch((BooleanQuery) query, minimumShouldMatch);
-        }
-
-        if (query != null) {
-            query.setBoost(boost);
-        }
-
-        return query;
     }
 
     @Override

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

@@ -207,8 +207,8 @@ public abstract class BaseQueryTestCase<QB extends QueryBuilder<QB>> extends Ela
 
         QueryBuilder newQuery = queryParserService.queryParser(testQuery.getName()).fromXContent(context);
         assertNotSame(newQuery, testQuery);
-        assertEquals(newQuery, testQuery);
+        assertEquals("Queries should be equal: " + newQuery + " vs. " + testQuery, newQuery, testQuery);
-        assertEquals(newQuery.hashCode(), testQuery.hashCode());
+        assertEquals("Queries should have equal hashcodes: " + newQuery + " vs. " + testQuery, newQuery.hashCode(), testQuery.hashCode());
     }
 
     /**

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

@@ -0,0 +1,301 @@
+/*
+ * 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.analysis.Analyzer;
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.search.BooleanQuery;
+import org.apache.lucene.search.MatchNoDocsQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.lucene.search.Queries;
+import org.elasticsearch.index.query.SimpleQueryParser.Settings;
+import org.elasticsearch.index.query.SimpleQueryStringBuilder.Operator;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+
+import static org.hamcrest.Matchers.*;
+
+public class SimpleQueryStringBuilderTest extends BaseQueryTestCase<SimpleQueryStringBuilder> {
+
+    private static final String[] MINIMUM_SHOULD_MATCH = new String[] { "1", "-1", "75%", "-25%", "2<75%", "2<-25%" };
+
+    @Override
+    protected SimpleQueryStringBuilder createTestQueryBuilder() {
+        SimpleQueryStringBuilder result = new SimpleQueryStringBuilder(randomAsciiOfLengthBetween(1, 10));
+
+        if (randomBoolean()) {
+            result.queryName(randomAsciiOfLengthBetween(1, 10));
+        }
+        if (randomBoolean()) {
+            result.analyzeWildcard(randomBoolean());
+        }
+        if (randomBoolean()) {
+            result.lenient(randomBoolean());
+        }
+        if (randomBoolean()) {
+            result.lowercaseExpandedTerms(randomBoolean());
+        }
+        if (randomBoolean()) {
+            result.locale(randomLocale(getRandom()));
+        }
+        if (randomBoolean()) {
+            result.minimumShouldMatch(randomFrom(MINIMUM_SHOULD_MATCH));
+        }
+        if (randomBoolean()) {
+            result.analyzer("simple");
+        }
+        if (randomBoolean()) {
+            result.defaultOperator(randomFrom(Operator.AND, Operator.OR));
+        }
+        if (randomBoolean()) {
+            result.boost(2.0f / randomIntBetween(1, 20));
+        }
+
+        if (randomBoolean()) {
+            Set<SimpleQueryStringFlag> flagSet = new HashSet<>();
+            int size = randomIntBetween(0, SimpleQueryStringFlag.values().length);
+            for (int i = 0; i < size; i++) {
+                randomFrom(SimpleQueryStringFlag.values());
+            }
+            if (flagSet.size() > 0) {
+                result.flags(flagSet.toArray(new SimpleQueryStringFlag[flagSet.size()]));
+            }
+        }
+
+        int fieldCount = randomIntBetween(0, 10);
+        Map<String, Float> fields = new TreeMap<>();
+        for (int i = 0; i < fieldCount; i++) {
+            if (randomBoolean()) {
+                fields.put(randomAsciiOfLengthBetween(1, 10), 1.0f);
+            } else {
+                fields.put(randomAsciiOfLengthBetween(1, 10), 2.0f / randomIntBetween(1, 20));
+            }
+        }
+        result.fields(fields);
+
+        return result;
+    }
+
+    @Test
+    public void testDefaults() {
+        SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder("The quick brown fox.");
+
+        assertEquals("Wrong default default boost.", 1.0f, qb.boost(), 0.001);
+        assertEquals("Wrong default default boost field.", 1.0f, SimpleQueryStringBuilder.DEFAULT_BOOST, 0.001);
+
+        assertEquals("Wrong default flags.", SimpleQueryStringFlag.ALL.value, qb.flags());
+        assertEquals("Wrong default flags field.", SimpleQueryStringFlag.ALL.value(), SimpleQueryStringBuilder.DEFAULT_FLAGS);
+
+        assertEquals("Wrong default default operator.", Operator.OR, qb.defaultOperator());
+        assertEquals("Wrong default default operator field.", Operator.OR, SimpleQueryStringBuilder.DEFAULT_OPERATOR);
+
+        assertEquals("Wrong default default locale.", Locale.ROOT, qb.locale());
+        assertEquals("Wrong default default locale field.", Locale.ROOT, SimpleQueryStringBuilder.DEFAULT_LOCALE);
+
+        assertEquals("Wrong default default analyze_wildcard.", false, qb.analyzeWildcard());
+        assertEquals("Wrong default default analyze_wildcard field.", false, SimpleQueryStringBuilder.DEFAULT_ANALYZE_WILDCARD);
+
+        assertEquals("Wrong default default lowercase_expanded_terms.", true, qb.lowercaseExpandedTerms());
+        assertEquals("Wrong default default lowercase_expanded_terms field.", true, SimpleQueryStringBuilder.DEFAULT_LOWERCASE_EXPANDED_TERMS);
+
+        assertEquals("Wrong default default lenient.", false, qb.lenient());
+        assertEquals("Wrong default default lenient field.", false, SimpleQueryStringBuilder.DEFAULT_LENIENT);
+
+        assertEquals("Wrong default default locale.", Locale.ROOT, qb.locale());
+        assertEquals("Wrong default default locale field.", Locale.ROOT, SimpleQueryStringBuilder.DEFAULT_LOCALE);
+    }
+
+    @Test
+    public void testDefaultNullLocale() {
+        SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder("The quick brown fox.");
+        qb.locale(null);
+        assertEquals("Setting locale to null should result in returning to default value.", 
+                SimpleQueryStringBuilder.DEFAULT_LOCALE, qb.locale());
+    }
+
+    @Test
+    public void testDefaultNullComplainFlags() {
+        SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder("The quick brown fox.");
+        qb.flags((SimpleQueryStringFlag[]) null);
+        assertEquals("Setting flags to null should result in returning to default value.", 
+                SimpleQueryStringBuilder.DEFAULT_FLAGS, qb.flags());
+    }
+
+    @Test
+    public void testDefaultEmptyComplainFlags() {
+        SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder("The quick brown fox.");
+        qb.flags(new SimpleQueryStringFlag[]{});
+        assertEquals("Setting flags to empty should result in returning to default value.",
+                SimpleQueryStringBuilder.DEFAULT_FLAGS, qb.flags());
+    }
+
+    @Test
+    public void testDefaultNullComplainOp() {
+        SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder("The quick brown fox.");
+        qb.defaultOperator(null);
+        assertEquals("Setting operator to null should result in returning to default value.", 
+                SimpleQueryStringBuilder.DEFAULT_OPERATOR, qb.defaultOperator());
+    }
+
+    // Check operator handling, and default field handling.
+    @Test
+    public void testDefaultOperatorHandling() {
+        SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder("The quick brown fox.");
+        BooleanQuery boolQuery = (BooleanQuery) qb.toQuery(createContext());
+        assertThat(shouldClauses(boolQuery), is(4));
+
+        qb.defaultOperator(Operator.AND);
+        boolQuery = (BooleanQuery) qb.toQuery(createContext());
+        assertThat(shouldClauses(boolQuery), is(0));
+
+        qb.defaultOperator(Operator.OR);
+        boolQuery = (BooleanQuery) qb.toQuery(createContext());
+        assertThat(shouldClauses(boolQuery), is(4));
+    }
+
+    @Test
+    public void testValidation() {
+        SimpleQueryStringBuilder qb = createTestQueryBuilder();
+        assertNull(qb.validate());
+    }
+
+    @Test
+    public void testNullQueryTextGeneratesException() {
+        SimpleQueryStringBuilder builder = new SimpleQueryStringBuilder(null);
+        QueryValidationException exception = builder.validate();
+        assertThat(exception, notNullValue());
+    }
+
+    @Test
+    public void testHandlingDefaults() throws IOException {
+        SimpleQueryStringBuilder qb = createTestQueryBuilder();
+        qb.analyzer(null);
+        qb.minimumShouldMatch(null);
+        qb.queryName(null);
+        assertEquals(qb.toQuery(createContext()), createExpectedQuery(qb, createContext()));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testFieldCannotBeNull() {
+        SimpleQueryStringBuilder qb = createTestQueryBuilder();
+        qb.field(null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testFieldCannotBeNullAndWeighted() {
+        SimpleQueryStringBuilder qb = createTestQueryBuilder();
+        qb.field(null, 1.0f);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testFieldCannotBeEmpty() {
+        SimpleQueryStringBuilder qb = createTestQueryBuilder();
+        qb.field("");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testFieldCannotBeEmptyAndWeighted() {
+        SimpleQueryStringBuilder qb = createTestQueryBuilder();
+        qb.field("", 1.0f);
+    }
+
+    /**
+     * The following should fail fast - never silently set the map containing
+     * fields and weights to null but refuse to accept null instead.
+     * */
+    @Test(expected = NullPointerException.class)
+    public void testFieldsCannotBeSetToNull() {
+        SimpleQueryStringBuilder qb = createTestQueryBuilder();
+        qb.fields(null);
+    }
+
+    @Override
+    protected void assertLuceneQuery(SimpleQueryStringBuilder queryBuilder, Query query, QueryParseContext context) {
+        if (queryBuilder.queryName() != null) {
+            Query namedQuery = context.copyNamedFilters().get(queryBuilder.queryName());
+            assertThat(namedQuery, equalTo(query));
+        }
+    }
+
+    private int shouldClauses(BooleanQuery query) {
+        int result = 0;
+        for (BooleanClause c : query.clauses()) {
+            if (c.getOccur() == BooleanClause.Occur.SHOULD) {
+                result++;
+            }
+        }
+        return result;
+    }
+
+    @Override
+    protected Query createExpectedQuery(SimpleQueryStringBuilder queryBuilder, QueryParseContext context) throws IOException {
+        Map<String, Float> fields = new TreeMap<>();
+        // Use the default field (_all) if no fields specified
+        if (queryBuilder.fields().isEmpty()) {
+            String field = context.defaultField();
+            fields.put(field, 1.0F);
+        } else {
+            fields.putAll(queryBuilder.fields());
+        }
+
+        // Use standard analyzer by default if none specified
+        Analyzer luceneAnalyzer;
+        if (queryBuilder.analyzer() == null) {
+            luceneAnalyzer = context.mapperService().searchAnalyzer();
+        } else {
+            luceneAnalyzer = context.analysisService().analyzer(queryBuilder.analyzer());
+        }
+        SimpleQueryParser sqp = new SimpleQueryParser(luceneAnalyzer, fields, queryBuilder.flags(), new Settings(queryBuilder.locale(),
+                queryBuilder.lowercaseExpandedTerms(), queryBuilder.lenient(), queryBuilder.analyzeWildcard()));
+
+        if (queryBuilder.defaultOperator() != null) {
+            switch (queryBuilder.defaultOperator()) {
+            case OR:
+                sqp.setDefaultOperator(Occur.SHOULD);
+                break;
+            case AND:
+                sqp.setDefaultOperator(Occur.MUST);
+                break;
+            }
+        }
+
+        Query query = sqp.parse(queryBuilder.text());
+        if (queryBuilder.queryName() != null) {
+            context.addNamedQuery(queryBuilder.queryName(), query);
+        }
+
+        if (queryBuilder.minimumShouldMatch() != null && query instanceof BooleanQuery) {
+            Queries.applyMinimumShouldMatch((BooleanQuery) query, queryBuilder.minimumShouldMatch());
+        }
+        query.setBoost(queryBuilder.boost());
+        return query;
+    }
+
+}
+