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

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.

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 @@ import static java.util.stream.Collectors.toList;
 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 class SuggestingErrorOnUnknown implements ErrorOnUnknown {
             }
         }
         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 class SuggestingErrorOnUnknown implements ErrorOnUnknown {
             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 class SuggestingErrorOnUnknown implements ErrorOnUnknown {
         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.io.stream.StreamOutput;
 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.nio.CharBuffer;
 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 abstract class AbstractQueryBuilder<QB extends AbstractQueryBuilder<QB>>
         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 class AbstractQueryBuilderTests extends ESTestCase {
             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

@@ -284,7 +284,7 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase<BoolQueryBuilde
         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

@@ -533,7 +533,7 @@ public class SimpleIndexTemplateIT extends ESIntegTestCase {
             () -> 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