Add "did you mean" to unknown queries (#51177)

This replaces the message we return for unknown queries with the standard
one that we use for unknown fields from `ObjectParser`. This is nice
because it includes "did you mean". One day we might convert parsing
queries to using object parser, but that looks complex. This change is
much smaller and seems useful.

Author: nik9000

modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/50_multi_search_template.yml

@@ -100,7 +100,7 @@ setup:
   - match:  { responses.1.error.root_cause.0.reason: "/Unexpected.end.of.input/" }
   - match:  { responses.2.hits.total:     1  }
   - match:  { responses.3.error.root_cause.0.type: parsing_exception }
-  - match:  { responses.3.error.root_cause.0.reason: "/no.\\[query\\].registered.for.\\[unknown\\]/" }
+  - match:  { responses.3.error.root_cause.0.reason: "/unknown.query.\\[unknown\\]/" }
 
 ---
 "Multi-search template with invalid request":

rest-api-spec/src/main/resources/rest-api-spec/test/indices.validate_query/10_basic.yml

@@ -12,6 +12,10 @@ setup:
 
 ---
 "Validate query api":
+  - skip:
+      version: ' - 7.99.99'
+      reason: message changed in 8.0.0
+
   - do:
       indices.validate_query:
         q: query string
@@ -43,7 +47,17 @@ setup:
             invalid_query: {}
 
   - is_false: valid
-  - match: {error: 'org.elasticsearch.common.ParsingException: no [query] registered for [invalid_query]'}
+  - match: {error: '/.+unknown\squery\s\[invalid_query\].+/' }
+
+  - do:
+      indices.validate_query:
+        explain: true
+        body:
+          query:
+            boool: {}
+
+  - is_false: valid
+  - match: {error: '/.+unknown\squery\s\[boool\]\sdid\syou\smean\s\[bool\]\?.+/' }
 
   - do:
       indices.validate_query:

server/src/main/java/org/elasticsearch/common/xcontent/SuggestingErrorOnUnknown.java

@@ -32,7 +32,19 @@
 public class SuggestingErrorOnUnknown implements ErrorOnUnknown {
     @Override
     public String errorMessage(String parserName, String unknownField, Iterable<String> candidates) {
-        String message = String.format(Locale.ROOT, "[%s] unknown field [%s]", parserName, unknownField);
+        return String.format(Locale.ROOT, "[%s] unknown field [%s]%s", parserName, unknownField, suggest(unknownField, candidates));
+    }
+
+    @Override
+    public int priority() {
+        return 0;
+    }
+
+    /**
+     * Builds suggestions for an unknown field, returning an empty string if there
+     * aren't any suggestions or " did you mean " and then the list of suggestions. 
+     */
+    public static String suggest(String unknownField, Iterable<String> candidates) {
         // TODO it'd be nice to combine this with BaseRestHandler's implementation.
         LevenshteinDistance ld = new LevenshteinDistance();
         final List<Tuple<Float, String>> scored = new ArrayList<>();
@@ -43,7 +55,7 @@ public String errorMessage(String parserName, String unknownField, Iterable<Stri
             }
         }
         if (scored.isEmpty()) {
-            return message;
+            return "";
         }
         CollectionUtil.timSort(scored, (a, b) -> {
             // sort by distance in reverse order, then parameter name for equal distances
@@ -54,7 +66,7 @@ public String errorMessage(String parserName, String unknownField, Iterable<Stri
             return a.v2().compareTo(b.v2());
         });
         List<String> keys = scored.stream().map(Tuple::v2).collect(toList());
-        StringBuilder builder = new StringBuilder(message).append(" did you mean ");
+        StringBuilder builder = new StringBuilder(" did you mean ");
         if (keys.size() == 1) {
             builder.append("[").append(keys.get(0)).append("]");
         } else {
@@ -63,9 +75,4 @@ public String errorMessage(String parserName, String unknownField, Iterable<Stri
         builder.append("?");
         return builder.toString();
     }
-
-    @Override
-    public int priority() {
-        return 0;
-    }
 }

server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java

@@ -32,6 +32,7 @@
 import org.elasticsearch.common.lucene.BytesRefs;
 import org.elasticsearch.common.xcontent.AbstractObjectParser;
 import org.elasticsearch.common.xcontent.NamedObjectNotFoundException;
+import org.elasticsearch.common.xcontent.SuggestingErrorOnUnknown;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentLocation;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -41,6 +42,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 
@@ -313,10 +315,9 @@ public static QueryBuilder parseInnerQueryBuilder(XContentParser parser) throws
         try {
             result = parser.namedObject(QueryBuilder.class, queryName, null);
         } catch (NamedObjectNotFoundException e) {
-            // Preserve the error message from 5.0 until we have a compellingly better message so we don't break BWC.
-            // This intentionally doesn't include the causing exception because that'd change the "root_cause" of any unknown query errors
-            throw new ParsingException(new XContentLocation(e.getLineNumber(), e.getColumnNumber()),
-                    "no [query] registered for [" + queryName + "]");
+            String message = String.format(Locale.ROOT, "unknown query [%s]%s", queryName,
+                    SuggestingErrorOnUnknown.suggest(queryName, e.getCandidates()));
+            throw new ParsingException(new XContentLocation(e.getLineNumber(), e.getColumnNumber()), message, e);
         }
         //end_object of the specific query (e.g. match, multi_match etc.) element
         if (parser.currentToken() != XContentParser.Token.END_OBJECT) {

server/src/test/java/org/elasticsearch/index/query/AbstractQueryBuilderTests.java

@@ -78,10 +78,15 @@ public void testParseInnerQueryBuilderExceptions() throws IOException {
             assertEquals("[foo] query malformed, no start_object after query name", exception.getMessage());
         }
 
-        source = "{ \"foo\" : {} }";
+        source = "{ \"boool\" : {} }";
         try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {
             ParsingException exception = expectThrows(ParsingException.class, () ->  parseInnerQueryBuilder(parser));
-            assertEquals("no [query] registered for [foo]", exception.getMessage());
+            assertEquals("unknown query [boool] did you mean [bool]?", exception.getMessage());
+        }
+        source = "{ \"match_\" : {} }";
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {
+            ParsingException exception = expectThrows(ParsingException.class, () ->  parseInnerQueryBuilder(parser));
+            assertEquals("unknown query [match_] did you mean any of [match, match_all, match_none]?", exception.getMessage());
         }
     }
 

server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java

@@ -283,7 +283,7 @@ public void testUnknownQueryName() throws IOException {
         String query = "{\"bool\" : {\"must\" : { \"unknown_query\" : { } } } }";
 
         ParsingException ex = expectThrows(ParsingException.class, () -> parseQuery(query));
-        assertEquals("no [query] registered for [unknown_query]", ex.getMessage());
+        assertEquals("unknown query [unknown_query]", ex.getMessage());
     }
 
     public void testRewrite() throws IOException {

server/src/test/java/org/elasticsearch/indices/template/SimpleIndexTemplateIT.java

@@ -531,7 +531,7 @@ public void testAliasInvalidFilterValidJson() throws Exception {
             () -> createIndex("test"));
         assertThat(e.getMessage(), equalTo("failed to parse filter for alias [invalid_alias]"));
         assertThat(e.getCause(), instanceOf(ParsingException.class));
-        assertThat(e.getCause().getMessage(), equalTo("no [query] registered for [invalid]"));
+        assertThat(e.getCause().getMessage(), equalTo("unknown query [invalid]"));
     }
 
     public void testAliasInvalidFilterInvalidJson() throws Exception {

x-pack/plugin/src/test/resources/rest-api-spec/test/ml/datafeeds_crud.yml

@@ -160,7 +160,7 @@ setup:
 ---
 "Test put datafeed with invalid query":
   - do:
-      catch: /parsing_exception/
+      catch: /failed\sto\sparse\sfield\s\[query\]/
       ml.put_datafeed:
         datafeed_id: test-datafeed-1
         body:  >

x-pack/plugin/src/test/resources/rest-api-spec/test/watcher/execute_watch/20_transform.yml

@@ -188,5 +188,5 @@ setup:
   - match: { watch_record.state: "executed" }
   - match: { watch_record.status.execution_state: "executed" }
   - match: { watch_record.result.transform.status: "failure" }
-  - match: { watch_record.result.transform.reason: "no [query] registered for [does_not_exist]" }
+  - match: { watch_record.result.transform.reason: "unknown query [does_not_exist]" }
   - is_true: watch_record.result.transform.error