Add "did you mean" to ObjectParser

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ErrorOnUnknown.java

@@ -0,0 +1,67 @@
+/*
+ * 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.common.xcontent;
+
+import java.util.ServiceLoader;
+
+/**
+ * Extension point to customize the error message for unknown fields. We expect
+ * Elasticsearch to plug a fancy implementation that uses Lucene's spelling
+ * correction infrastructure to suggest corrections.
+ */
+public interface ErrorOnUnknown {
+    /**
+     * The implementation of this interface that was loaded from SPI.
+     */
+    ErrorOnUnknown IMPLEMENTATION = findImplementation();
+
+    /**
+     * Build the error message to use when {@link ObjectParser} encounters an unknown field.
+     * @param parserName the name of the thing we're parsing
+     * @param unknownField the field that we couldn't recognize
+     * @param candidates the possible fields
+     */
+    String errorMessage(String parserName, String unknownField, Iterable<String> candidates);
+
+    /**
+     * Priority that this error message handler should be used.
+     */
+    int priority();
+
+    private static ErrorOnUnknown findImplementation() {
+        ErrorOnUnknown best = new ErrorOnUnknown() {
+            @Override
+            public String errorMessage(String parserName, String unknownField, Iterable<String> candidates) {
+                return "[" + parserName + "] unknown field [" + unknownField + "]";
+            }
+
+            @Override
+            public int priority() {
+                return Integer.MIN_VALUE;
+            }
+        };
+        for (ErrorOnUnknown c : ServiceLoader.load(ErrorOnUnknown.class)) {
+            if (best.priority() < c.priority()) {
+                best = c;
+            }
+        }
+        return best;
+    }
+}

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java

@@ -81,18 +81,17 @@ public static <Value, ElementValue> BiConsumer<Value, List<ElementValue>> fromLi
     }
 
     private interface UnknownFieldParser<Value, Context> {
-
-        void acceptUnknownField(String parserName, String field, XContentLocation location, XContentParser parser,
-                                Value value, Context context) throws IOException;
+        void acceptUnknownField(ObjectParser<Value, Context> objectParser, String field, XContentLocation location, XContentParser parser,
+                Value value, Context context) throws IOException;
     }
 
     private static <Value, Context> UnknownFieldParser<Value, Context> ignoreUnknown() {
-      return (n, f, l, p, v, c) -> p.skipChildren();
+      return (op, f, l, p, v, c) -> p.skipChildren();
     }
 
     private static <Value, Context> UnknownFieldParser<Value, Context> errorOnUnknown() {
-        return (n, f, l, p, v, c) -> {
-            throw new XContentParseException(l, "[" + n + "] unknown field [" + f + "], parser not found");
+        return (op, f, l, p, v, c) -> {
+            throw new XContentParseException(l, ErrorOnUnknown.IMPLEMENTATION.errorMessage(op.name, f, op.fieldParserMap.keySet()));
         };
     }
 
@@ -104,7 +103,7 @@ public interface UnknownFieldConsumer<Value> {
     }
 
     private static <Value, Context> UnknownFieldParser<Value, Context> consumeUnknownField(UnknownFieldConsumer<Value> consumer) {
-        return (parserName, field, location, parser, value, context) -> {
+        return (objectParser, field, location, parser, value, context) -> {
             XContentParser.Token t = parser.currentToken();
             switch (t) {
                 case VALUE_STRING:
@@ -127,7 +126,7 @@ private static <Value, Context> UnknownFieldParser<Value, Context> consumeUnknow
                     break;
                 default:
                     throw new XContentParseException(parser.getTokenLocation(),
-                        "[" + parserName + "] cannot parse field [" + field + "] with value type [" + t + "]");
+                        "[" + objectParser.name + "] cannot parse field [" + field + "] with value type [" + t + "]");
             }
         };
     }
@@ -136,12 +135,13 @@ private static <Value, Category, Context> UnknownFieldParser<Value, Context> unk
         Class<Category> categoryClass,
         BiConsumer<Value, ? super Category> consumer
     ) {
-        return (parserName, field, location, parser, value, context) -> {
+        return (objectParser, field, location, parser, value, context) -> {
             Category o;
             try {
                 o = parser.namedObject(categoryClass, field, context);
             } catch (NamedObjectNotFoundException e) {
-                throw new XContentParseException(location, "[" + parserName  + "] " + e.getBareMessage(), e);
+                // TODO It'd be lovely if we could the options here but we don't have the right stuff plumbed through. We'll get to it!
+                throw new XContentParseException(location, "[" + objectParser.name  + "] " + e.getBareMessage(), e);
             }
             consumer.accept(value, o);
         };
@@ -278,7 +278,7 @@ public Value parse(XContentParser parser, Value value, Context context) throws I
                     throw new XContentParseException(parser.getTokenLocation(), "[" + name  + "] no field found");
                 }
                 if (fieldParser == null) {
-                    unknownFieldParser.acceptUnknownField(name, currentFieldName, currentPosition, parser, value, context);
+                    unknownFieldParser.acceptUnknownField(this, currentFieldName, currentPosition, parser, value, context);
                 } else {
                     fieldParser.assertSupports(name, parser, currentFieldName);
                     parseSub(parser, fieldParser, currentFieldName, value, context);

libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java

@@ -206,7 +206,7 @@ public void setTest(int test) {
         {
             XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"not_supported_field\" : \"foo\"}");
             XContentParseException ex = expectThrows(XContentParseException.class, () -> objectParser.parse(parser, s, null));
-            assertEquals(ex.getMessage(), "[1:2] [the_parser] unknown field [not_supported_field], parser not found");
+            assertEquals(ex.getMessage(), "[1:2] [the_parser] unknown field [not_supported_field]");
         }
     }
 

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java

@@ -143,7 +143,6 @@ public void testXContentParsingIsNotLenient() throws IOException {
                 exception = exception.getCause();
             }
             assertThat(exception.getMessage(), containsString("unknown field"));
-            assertThat(exception.getMessage(), containsString("parser not found"));
         }
     }
 

rest-api-spec/src/main/resources/rest-api-spec/test/update/90_error.yml

@@ -0,0 +1,13 @@
+---
+'Misspelled fields get "did you mean"':
+  - skip:
+      version: " - 7.99.99"
+      reason: Implemented in 8.0
+  - do:
+      catch: /\[UpdateRequest\] unknown field \[dac\] did you mean \[doc\]\?/
+      update:
+          index: test
+          id:    1
+          body:
+            dac:    { foo: baz }
+            upsert: { foo: bar }

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

@@ -0,0 +1,71 @@
+/*
+ * 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.common.xcontent;
+
+import org.apache.lucene.search.spell.LevenshteinDistance;
+import org.apache.lucene.util.CollectionUtil;
+import org.elasticsearch.common.collect.Tuple;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Locale;
+
+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);
+        // TODO it'd be nice to combine this with BaseRestHandler's implementation.
+        LevenshteinDistance ld = new LevenshteinDistance();
+        final List<Tuple<Float, String>> scored = new ArrayList<>();
+        for (String candidate : candidates) {
+            float distance = ld.getDistance(unknownField, candidate);
+            if (distance > 0.5f) {
+                scored.add(new Tuple<>(distance, candidate));
+            }
+        }
+        if (scored.isEmpty()) {
+            return message;
+        }
+        CollectionUtil.timSort(scored, (a, b) -> {
+            // sort by distance in reverse order, then parameter name for equal distances
+            int compare = a.v1().compareTo(b.v1());
+            if (compare != 0) {
+                return -compare;
+            }
+            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 ");
+        if (keys.size() == 1) {
+            builder.append("[").append(keys.get(0)).append("]");
+        } else {
+            builder.append("any of ").append(keys.toString());
+        }
+        builder.append("?");
+        return builder.toString();
+    }
+
+    @Override
+    public int priority() {
+        return 0;
+    }
+}

server/src/main/resources/META-INF/services/org.elasticsearch.common.xcontent.ErrorOnUnknown

@@ -0,0 +1 @@
+org.elasticsearch.common.xcontent.SuggestingErrorOnUnknown

server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequestTests.java

@@ -56,7 +56,7 @@ private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws
             XContentParseException iae = expectThrows(XContentParseException.class,
                     () -> ClusterUpdateSettingsRequest.fromXContent(createParser(xContentType.xContent(), mutated)));
             assertThat(iae.getMessage(),
-                    containsString("[cluster_update_settings_request] unknown field [" + unsupportedField + "], parser not found"));
+                    containsString("[cluster_update_settings_request] unknown field [" + unsupportedField + "]"));
         } else {
             try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
                 ClusterUpdateSettingsRequest parsedRequest = ClusterUpdateSettingsRequest.fromXContent(parser);

server/src/test/java/org/elasticsearch/action/update/UpdateRequestTests.java

@@ -288,7 +288,7 @@ public void testUnknownFieldParsing() throws Exception {
                 .endObject());
 
         XContentParseException ex = expectThrows(XContentParseException.class, () -> request.fromXContent(contentParser));
-        assertEquals("[1:2] [UpdateRequest] unknown field [unknown_field], parser not found", ex.getMessage());
+        assertEquals("[1:2] [UpdateRequest] unknown field [unknown_field]", ex.getMessage());
 
         UpdateRequest request2 = new UpdateRequest("test", "1");
         XContentParser unknownObject = createParser(XContentFactory.jsonBuilder()
@@ -299,7 +299,7 @@ public void testUnknownFieldParsing() throws Exception {
                     .endObject()
                 .endObject());
         ex = expectThrows(XContentParseException.class, () -> request2.fromXContent(unknownObject));
-        assertEquals("[1:76] [UpdateRequest] unknown field [params], parser not found", ex.getMessage());
+        assertEquals("[1:76] [UpdateRequest] unknown field [params]", ex.getMessage());
     }
 
     public void testFetchSourceParsing() throws Exception {

server/src/test/java/org/elasticsearch/common/xcontent/SuggestingErrorOnUnknownTests.java

@@ -0,0 +1,46 @@
+/*
+ * 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.common.xcontent;
+
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.Arrays;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class SuggestingErrorOnUnknownTests extends ESTestCase {
+    private String errorMessage(String unknownField, String... candidates) {
+        return new SuggestingErrorOnUnknown().errorMessage("test", unknownField, Arrays.asList(candidates));
+    }
+
+    public void testNoCandidates() {
+        assertThat(errorMessage("foo"), equalTo("[test] unknown field [foo]"));
+    }
+    public void testBadCandidates() {
+        assertThat(errorMessage("foo", "bar", "baz"), equalTo("[test] unknown field [foo]"));
+    }
+    public void testOneCandidate() {
+        assertThat(errorMessage("foo", "bar", "fop"), equalTo("[test] unknown field [foo] did you mean [fop]?"));
+    }
+    public void testManyCandidate() {
+        assertThat(errorMessage("foo", "bar", "fop", "fou", "baz"),
+                equalTo("[test] unknown field [foo] did you mean any of [fop, fou]?"));
+    }
+}

server/src/test/java/org/elasticsearch/search/fetch/subphase/highlight/HighlightBuilderTests.java

@@ -163,7 +163,7 @@ public void testUnknownArrayNameExpection() throws IOException {
             XContentParseException e = expectParseThrows(XContentParseException.class, "{\n" +
                     "    \"bad_fieldname\" : [ \"field1\" 1 \"field2\" ]\n" +
                     "}\n");
-            assertEquals("[2:5] [highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
+            assertEquals("[2:5] [highlight] unknown field [bad_fieldname]", e.getMessage());
         }
 
         {
@@ -176,7 +176,7 @@ public void testUnknownArrayNameExpection() throws IOException {
                     "}\n");
             assertThat(e.getMessage(), containsString("[highlight] failed to parse field [fields]"));
             assertThat(e.getCause().getMessage(), containsString("[fields] failed to parse field [body]"));
-            assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
+            assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname]", e.getCause().getCause().getMessage());
         }
     }
 
@@ -194,7 +194,7 @@ public void testUnknownFieldnameExpection() throws IOException {
             XContentParseException e = expectParseThrows(XContentParseException.class, "{\n" +
                     "    \"bad_fieldname\" : \"value\"\n" +
                     "}\n");
-            assertEquals("[2:5] [highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
+            assertEquals("[2:5] [highlight] unknown field [bad_fieldname]", e.getMessage());
         }
 
         {
@@ -207,7 +207,7 @@ public void testUnknownFieldnameExpection() throws IOException {
                     "}\n");
             assertThat(e.getMessage(), containsString("[highlight] failed to parse field [fields]"));
             assertThat(e.getCause().getMessage(), containsString("[fields] failed to parse field [body]"));
-            assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
+            assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname]", e.getCause().getCause().getMessage());
         }
     }
 
@@ -219,7 +219,7 @@ public void testUnknownObjectFieldnameExpection() throws IOException {
             XContentParseException e = expectParseThrows(XContentParseException.class, "{\n" +
                     "    \"bad_fieldname\" :  { \"field\" : \"value\" }\n \n" +
                     "}\n");
-            assertEquals("[2:5] [highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
+            assertEquals("[2:5] [highlight] unknown field [bad_fieldname]", e.getMessage());
         }
 
         {
@@ -232,7 +232,7 @@ public void testUnknownObjectFieldnameExpection() throws IOException {
                     "}\n");
             assertThat(e.getMessage(), containsString("[highlight] failed to parse field [fields]"));
             assertThat(e.getCause().getMessage(), containsString("[fields] failed to parse field [body]"));
-            assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
+            assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname]", e.getCause().getCause().getMessage());
         }
     }
 

server/src/test/java/org/elasticsearch/search/rescore/QueryRescorerBuilderTests.java

@@ -254,7 +254,7 @@ public void testUnknownFieldsExpection() throws IOException {
             "}\n";
         try (XContentParser parser = createParser(rescoreElement)) {
             XContentParseException e = expectThrows(XContentParseException.class, () -> RescorerBuilder.parseFromXContent(parser));
-            assertEquals("[3:17] [query] unknown field [bad_fieldname], parser not found", e.getMessage());
+            assertEquals("[3:17] [query] unknown field [bad_fieldname]", e.getMessage());
         }
 
         rescoreElement = "{\n" +

server/src/test/java/org/elasticsearch/search/sort/FieldSortBuilderTests.java

@@ -316,7 +316,7 @@ public void testUnknownOptionFails() throws IOException {
             parser.nextToken();
 
             XContentParseException e = expectThrows(XContentParseException.class, () -> FieldSortBuilder.fromXContent(parser, ""));
-            assertEquals("[1:18] [field_sort] unknown field [reverse], parser not found", e.getMessage());
+            assertEquals("[1:18] [field_sort] unknown field [reverse]", e.getMessage());
         }
     }