Query DSL: deprecate _name and boost in short variants of queries

As discussed in #11744 this is the last step to unify parsing of boost and _name. Those fields are supported only in long version of queries, while we sometimes parse them when wwe shouldn't, inconsistently. Closes #11744 Closes #12966
elastic · Aug 27, 2015 · dc807f2 · dc807f2
1 parent 793fcb6
commit dc807f2
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 11 deletions.
diff --git a/core/src/main/java/org/elasticsearch/common/Strings.java b/core/src/main/java/org/elasticsearch/common/Strings.java
@@ -943,7 +943,8 @@ public static String toCamelCase(String value, StringBuilder sb) {
         boolean changed = false;
         for (int i = 0; i < value.length(); i++) {
             char c = value.charAt(i);
-            if (c == '_') {
+            //e.g. _name stays as-is, _first_name becomes _firstName
+            if (c == '_' && i > 0) {
                 if (!changed) {
                     if (sb != null) {
                         sb.setLength(0);

diff --git a/core/src/main/java/org/elasticsearch/index/query/PrefixQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/PrefixQueryParser.java
@@ -23,6 +23,7 @@
 import org.apache.lucene.search.MultiTermQuery;
 import org.apache.lucene.search.PrefixQuery;
 import org.apache.lucene.search.Query;
+import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.lucene.BytesRefs;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -38,6 +39,8 @@ public class PrefixQueryParser implements QueryParser {
 
     public static final String NAME = "prefix";
 
+    private static final ParseField NAME_FIELD = new ParseField("_name").withAllDeprecated("query name is not supported in short version of prefix query");
+
     @Inject
     public PrefixQueryParser() {
     }
@@ -84,7 +87,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                     }
                 }
             } else {
-                if ("_name".equals(currentFieldName)) {
+                if (parseContext.parseFieldMatcher().match(currentFieldName, NAME_FIELD)) {
                     queryName = parser.text();
                 } else {
                     fieldName = currentFieldName;

diff --git a/core/src/main/java/org/elasticsearch/index/query/RangeQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/RangeQueryParser.java
@@ -40,6 +40,7 @@ public class RangeQueryParser implements QueryParser {
 
     public static final String NAME = "range";
     private static final ParseField FIELDDATA_FIELD = new ParseField("fielddata").withAllDeprecated("[no replacement]");
+    private static final ParseField NAME_FIELD = new ParseField("_name").withAllDeprecated("query name is not supported in short version of range query");
 
     @Inject
     public RangeQueryParser() {
@@ -109,7 +110,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                     }
                 }
             } else if (token.isValue()) {
-                if ("_name".equals(currentFieldName)) {
+                if (parseContext.parseFieldMatcher().match(currentFieldName, NAME_FIELD)) {
                     queryName = parser.text();
                 } else if (parseContext.parseFieldMatcher().match(currentFieldName, FIELDDATA_FIELD)) {
                     // ignore

diff --git a/core/src/main/java/org/elasticsearch/index/query/RegexpQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/RegexpQueryParser.java
@@ -24,6 +24,7 @@
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.RegexpQuery;
 import org.apache.lucene.util.automaton.Operations;
+import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.lucene.BytesRefs;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -41,6 +42,8 @@ public class RegexpQueryParser implements QueryParser {
 
     public static final int DEFAULT_FLAGS_VALUE = RegexpFlag.ALL.value();
 
+    private static final ParseField NAME_FIELD = new ParseField("_name").withAllDeprecated("query name is not supported in short version of regexp query");
+
     @Inject
     public RegexpQueryParser() {
     }
@@ -96,7 +99,7 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                     }
                 }
             } else {
-                if ("_name".equals(currentFieldName)) {
+                if (parseContext.parseFieldMatcher().match(currentFieldName, NAME_FIELD)) {
                     queryName = parser.text();
                 } else {
                     fieldName = currentFieldName;

diff --git a/core/src/main/java/org/elasticsearch/index/query/TermQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/TermQueryParser.java
@@ -22,10 +22,10 @@
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
+import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.lucene.BytesRefs;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 
 import java.io.IOException;
@@ -37,6 +37,9 @@ public class TermQueryParser implements QueryParser {
 
     public static final String NAME = "term";
 
+    private static final ParseField NAME_FIELD = new ParseField("_name").withAllDeprecated("query name is not supported in short version of term query");
+    private static final ParseField BOOST_FIELD = new ParseField("boost").withAllDeprecated("boost is not supported in short version of term query");
+
     @Inject
     public TermQueryParser() {
     }
@@ -85,9 +88,9 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
                     }
                 }
             } else if (token.isValue()) {
-                if ("_name".equals(currentFieldName)) {
+                if (parseContext.parseFieldMatcher().match(currentFieldName, NAME_FIELD)) {
                     queryName = parser.text();
-                } else if ("boost".equals(currentFieldName)) {
+                } else if (parseContext.parseFieldMatcher().match(currentFieldName, BOOST_FIELD)) {
                     boost = parser.floatValue();
                 } else {
                     if (fieldName != null) {

diff --git a/core/src/test/java/org/elasticsearch/common/StringsTests.java b/core/src/test/java/org/elasticsearch/common/StringsTests.java
@@ -29,6 +29,8 @@ public void testToCamelCase() {
         assertEquals("fooBar", Strings.toCamelCase("foo_bar"));
         assertEquals("fooBarFooBar", Strings.toCamelCase("foo_bar_foo_bar"));
         assertEquals("fooBar", Strings.toCamelCase("foo_bar_"));
+        assertEquals("_foo", Strings.toCamelCase("_foo"));
+        assertEquals("_fooBar", Strings.toCamelCase("_foo_bar_"));
     }
 
     public void testSubstring() {

diff --git a/core/src/test/java/org/elasticsearch/index/analysis/AnalysisModuleTests.java b/core/src/test/java/org/elasticsearch/index/analysis/AnalysisModuleTests.java
@@ -54,9 +54,7 @@
 import java.util.Set;
 
 import static org.elasticsearch.common.settings.Settings.settingsBuilder;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.*;
 
 /**
  *
@@ -249,7 +247,7 @@ public void testUnderscoreInAnalyzerName() {
             fail("This should fail with IllegalArgumentException because the analyzers name starts with _");
         } catch (ProvisionException e) {
             assertTrue(e.getCause() instanceof IllegalArgumentException);
-            assertThat(e.getCause().getMessage(), equalTo("analyzer name must not start with '_'. got \"_invalid_name\""));
+            assertThat(e.getCause().getMessage(), either(equalTo("analyzer name must not start with '_'. got \"_invalid_name\"")).or(equalTo("analyzer name must not start with '_'. got \"_invalidName\"")));
         }
     }