Skip to content

Commit 1fb1b57

Browse files
nik9000SivagurunathanV
authored andcommitted
Add "did you mean" to ObjectParser (elastic#50938)
Check it out: ``` $ curl -u elastic:password -HContent-Type:application/json -XPOST localhost:9200/test/_update/foo?pretty -d'{ "dac": {} }' { "error" : { "root_cause" : [ { "type" : "x_content_parse_exception", "reason" : "[2:3] [UpdateRequest] unknown field [dac] did you mean [doc]?" } ], "type" : "x_content_parse_exception", "reason" : "[2:3] [UpdateRequest] unknown field [dac] did you mean [doc]?" }, "status" : 400 } ``` The tricky thing about implementing this is that x-content doesn't depend on Lucene. So this works by creating an extension point for the error message using SPI. Elasticsearch's server module provides the "spell checking" implementation.
1 parent a1df655 commit 1fb1b57

File tree

25 files changed

+240
-43
lines changed

25 files changed

+240
-43
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common.xcontent;
21+
22+
import java.util.ServiceLoader;
23+
24+
/**
25+
* Extension point to customize the error message for unknown fields. We expect
26+
* Elasticsearch to plug a fancy implementation that uses Lucene's spelling
27+
* correction infrastructure to suggest corrections.
28+
*/
29+
public interface ErrorOnUnknown {
30+
/**
31+
* The implementation of this interface that was loaded from SPI.
32+
*/
33+
ErrorOnUnknown IMPLEMENTATION = findImplementation();
34+
35+
/**
36+
* Build the error message to use when {@link ObjectParser} encounters an unknown field.
37+
* @param parserName the name of the thing we're parsing
38+
* @param unknownField the field that we couldn't recognize
39+
* @param candidates the possible fields
40+
*/
41+
String errorMessage(String parserName, String unknownField, Iterable<String> candidates);
42+
43+
/**
44+
* Priority that this error message handler should be used.
45+
*/
46+
int priority();
47+
48+
private static ErrorOnUnknown findImplementation() {
49+
ErrorOnUnknown best = new ErrorOnUnknown() {
50+
@Override
51+
public String errorMessage(String parserName, String unknownField, Iterable<String> candidates) {
52+
return "[" + parserName + "] unknown field [" + unknownField + "]";
53+
}
54+
55+
@Override
56+
public int priority() {
57+
return Integer.MIN_VALUE;
58+
}
59+
};
60+
for (ErrorOnUnknown c : ServiceLoader.load(ErrorOnUnknown.class)) {
61+
if (best.priority() < c.priority()) {
62+
best = c;
63+
}
64+
}
65+
return best;
66+
}
67+
}

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

+11-11
Original file line numberDiff line numberDiff line change
@@ -81,18 +81,17 @@ public static <Value, ElementValue> BiConsumer<Value, List<ElementValue>> fromLi
8181
}
8282

8383
private interface UnknownFieldParser<Value, Context> {
84-
85-
void acceptUnknownField(String parserName, String field, XContentLocation location, XContentParser parser,
86-
Value value, Context context) throws IOException;
84+
void acceptUnknownField(ObjectParser<Value, Context> objectParser, String field, XContentLocation location, XContentParser parser,
85+
Value value, Context context) throws IOException;
8786
}
8887

8988
private static <Value, Context> UnknownFieldParser<Value, Context> ignoreUnknown() {
90-
return (n, f, l, p, v, c) -> p.skipChildren();
89+
return (op, f, l, p, v, c) -> p.skipChildren();
9190
}
9291

9392
private static <Value, Context> UnknownFieldParser<Value, Context> errorOnUnknown() {
94-
return (n, f, l, p, v, c) -> {
95-
throw new XContentParseException(l, "[" + n + "] unknown field [" + f + "], parser not found");
93+
return (op, f, l, p, v, c) -> {
94+
throw new XContentParseException(l, ErrorOnUnknown.IMPLEMENTATION.errorMessage(op.name, f, op.fieldParserMap.keySet()));
9695
};
9796
}
9897

@@ -104,7 +103,7 @@ public interface UnknownFieldConsumer<Value> {
104103
}
105104

106105
private static <Value, Context> UnknownFieldParser<Value, Context> consumeUnknownField(UnknownFieldConsumer<Value> consumer) {
107-
return (parserName, field, location, parser, value, context) -> {
106+
return (objectParser, field, location, parser, value, context) -> {
108107
XContentParser.Token t = parser.currentToken();
109108
switch (t) {
110109
case VALUE_STRING:
@@ -127,7 +126,7 @@ private static <Value, Context> UnknownFieldParser<Value, Context> consumeUnknow
127126
break;
128127
default:
129128
throw new XContentParseException(parser.getTokenLocation(),
130-
"[" + parserName + "] cannot parse field [" + field + "] with value type [" + t + "]");
129+
"[" + objectParser.name + "] cannot parse field [" + field + "] with value type [" + t + "]");
131130
}
132131
};
133132
}
@@ -136,12 +135,13 @@ private static <Value, Category, Context> UnknownFieldParser<Value, Context> unk
136135
Class<Category> categoryClass,
137136
BiConsumer<Value, ? super Category> consumer
138137
) {
139-
return (parserName, field, location, parser, value, context) -> {
138+
return (objectParser, field, location, parser, value, context) -> {
140139
Category o;
141140
try {
142141
o = parser.namedObject(categoryClass, field, context);
143142
} catch (NamedObjectNotFoundException e) {
144-
throw new XContentParseException(location, "[" + parserName + "] " + e.getBareMessage(), e);
143+
// 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!
144+
throw new XContentParseException(location, "[" + objectParser.name + "] " + e.getBareMessage(), e);
145145
}
146146
consumer.accept(value, o);
147147
};
@@ -278,7 +278,7 @@ public Value parse(XContentParser parser, Value value, Context context) throws I
278278
throw new XContentParseException(parser.getTokenLocation(), "[" + name + "] no field found");
279279
}
280280
if (fieldParser == null) {
281-
unknownFieldParser.acceptUnknownField(name, currentFieldName, currentPosition, parser, value, context);
281+
unknownFieldParser.acceptUnknownField(this, currentFieldName, currentPosition, parser, value, context);
282282
} else {
283283
fieldParser.assertSupports(name, parser, currentFieldName);
284284
parseSub(parser, fieldParser, currentFieldName, value, context);

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

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

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

-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,6 @@ public void testXContentParsingIsNotLenient() throws IOException {
143143
exception = exception.getCause();
144144
}
145145
assertThat(exception.getMessage(), containsString("unknown field"));
146-
assertThat(exception.getMessage(), containsString("parser not found"));
147146
}
148147
}
149148

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'Misspelled fields get "did you mean"':
3+
- skip:
4+
version: " - 7.99.99"
5+
reason: Implemented in 8.0
6+
- do:
7+
catch: /\[UpdateRequest\] unknown field \[dac\] did you mean \[doc\]\?/
8+
update:
9+
index: test
10+
id: 1
11+
body:
12+
dac: { foo: baz }
13+
upsert: { foo: bar }
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common.xcontent;
21+
22+
import org.apache.lucene.search.spell.LevenshteinDistance;
23+
import org.apache.lucene.util.CollectionUtil;
24+
import org.elasticsearch.common.collect.Tuple;
25+
26+
import java.util.ArrayList;
27+
import java.util.List;
28+
import java.util.Locale;
29+
30+
import static java.util.stream.Collectors.toList;
31+
32+
public class SuggestingErrorOnUnknown implements ErrorOnUnknown {
33+
@Override
34+
public String errorMessage(String parserName, String unknownField, Iterable<String> candidates) {
35+
String message = String.format(Locale.ROOT, "[%s] unknown field [%s]", parserName, unknownField);
36+
// TODO it'd be nice to combine this with BaseRestHandler's implementation.
37+
LevenshteinDistance ld = new LevenshteinDistance();
38+
final List<Tuple<Float, String>> scored = new ArrayList<>();
39+
for (String candidate : candidates) {
40+
float distance = ld.getDistance(unknownField, candidate);
41+
if (distance > 0.5f) {
42+
scored.add(new Tuple<>(distance, candidate));
43+
}
44+
}
45+
if (scored.isEmpty()) {
46+
return message;
47+
}
48+
CollectionUtil.timSort(scored, (a, b) -> {
49+
// sort by distance in reverse order, then parameter name for equal distances
50+
int compare = a.v1().compareTo(b.v1());
51+
if (compare != 0) {
52+
return -compare;
53+
}
54+
return a.v2().compareTo(b.v2());
55+
});
56+
List<String> keys = scored.stream().map(Tuple::v2).collect(toList());
57+
StringBuilder builder = new StringBuilder(message).append(" did you mean ");
58+
if (keys.size() == 1) {
59+
builder.append("[").append(keys.get(0)).append("]");
60+
} else {
61+
builder.append("any of ").append(keys.toString());
62+
}
63+
builder.append("?");
64+
return builder.toString();
65+
}
66+
67+
@Override
68+
public int priority() {
69+
return 0;
70+
}
71+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
org.elasticsearch.common.xcontent.SuggestingErrorOnUnknown

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws
5656
XContentParseException iae = expectThrows(XContentParseException.class,
5757
() -> ClusterUpdateSettingsRequest.fromXContent(createParser(xContentType.xContent(), mutated)));
5858
assertThat(iae.getMessage(),
59-
containsString("[cluster_update_settings_request] unknown field [" + unsupportedField + "], parser not found"));
59+
containsString("[cluster_update_settings_request] unknown field [" + unsupportedField + "]"));
6060
} else {
6161
try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
6262
ClusterUpdateSettingsRequest parsedRequest = ClusterUpdateSettingsRequest.fromXContent(parser);

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ public void testUnknownFieldParsing() throws Exception {
288288
.endObject());
289289

290290
XContentParseException ex = expectThrows(XContentParseException.class, () -> request.fromXContent(contentParser));
291-
assertEquals("[1:2] [UpdateRequest] unknown field [unknown_field], parser not found", ex.getMessage());
291+
assertEquals("[1:2] [UpdateRequest] unknown field [unknown_field]", ex.getMessage());
292292

293293
UpdateRequest request2 = new UpdateRequest("test", "1");
294294
XContentParser unknownObject = createParser(XContentFactory.jsonBuilder()
@@ -299,7 +299,7 @@ public void testUnknownFieldParsing() throws Exception {
299299
.endObject()
300300
.endObject());
301301
ex = expectThrows(XContentParseException.class, () -> request2.fromXContent(unknownObject));
302-
assertEquals("[1:76] [UpdateRequest] unknown field [params], parser not found", ex.getMessage());
302+
assertEquals("[1:76] [UpdateRequest] unknown field [params]", ex.getMessage());
303303
}
304304

305305
public void testFetchSourceParsing() throws Exception {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.common.xcontent;
21+
22+
import org.elasticsearch.test.ESTestCase;
23+
24+
import java.util.Arrays;
25+
26+
import static org.hamcrest.Matchers.equalTo;
27+
28+
public class SuggestingErrorOnUnknownTests extends ESTestCase {
29+
private String errorMessage(String unknownField, String... candidates) {
30+
return new SuggestingErrorOnUnknown().errorMessage("test", unknownField, Arrays.asList(candidates));
31+
}
32+
33+
public void testNoCandidates() {
34+
assertThat(errorMessage("foo"), equalTo("[test] unknown field [foo]"));
35+
}
36+
public void testBadCandidates() {
37+
assertThat(errorMessage("foo", "bar", "baz"), equalTo("[test] unknown field [foo]"));
38+
}
39+
public void testOneCandidate() {
40+
assertThat(errorMessage("foo", "bar", "fop"), equalTo("[test] unknown field [foo] did you mean [fop]?"));
41+
}
42+
public void testManyCandidate() {
43+
assertThat(errorMessage("foo", "bar", "fop", "fou", "baz"),
44+
equalTo("[test] unknown field [foo] did you mean any of [fop, fou]?"));
45+
}
46+
}

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public void testUnknownArrayNameExpection() throws IOException {
163163
XContentParseException e = expectParseThrows(XContentParseException.class, "{\n" +
164164
" \"bad_fieldname\" : [ \"field1\" 1 \"field2\" ]\n" +
165165
"}\n");
166-
assertEquals("[2:5] [highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
166+
assertEquals("[2:5] [highlight] unknown field [bad_fieldname]", e.getMessage());
167167
}
168168

169169
{
@@ -176,7 +176,7 @@ public void testUnknownArrayNameExpection() throws IOException {
176176
"}\n");
177177
assertThat(e.getMessage(), containsString("[highlight] failed to parse field [fields]"));
178178
assertThat(e.getCause().getMessage(), containsString("[fields] failed to parse field [body]"));
179-
assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
179+
assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname]", e.getCause().getCause().getMessage());
180180
}
181181
}
182182

@@ -194,7 +194,7 @@ public void testUnknownFieldnameExpection() throws IOException {
194194
XContentParseException e = expectParseThrows(XContentParseException.class, "{\n" +
195195
" \"bad_fieldname\" : \"value\"\n" +
196196
"}\n");
197-
assertEquals("[2:5] [highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
197+
assertEquals("[2:5] [highlight] unknown field [bad_fieldname]", e.getMessage());
198198
}
199199

200200
{
@@ -207,7 +207,7 @@ public void testUnknownFieldnameExpection() throws IOException {
207207
"}\n");
208208
assertThat(e.getMessage(), containsString("[highlight] failed to parse field [fields]"));
209209
assertThat(e.getCause().getMessage(), containsString("[fields] failed to parse field [body]"));
210-
assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
210+
assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname]", e.getCause().getCause().getMessage());
211211
}
212212
}
213213

@@ -219,7 +219,7 @@ public void testUnknownObjectFieldnameExpection() throws IOException {
219219
XContentParseException e = expectParseThrows(XContentParseException.class, "{\n" +
220220
" \"bad_fieldname\" : { \"field\" : \"value\" }\n \n" +
221221
"}\n");
222-
assertEquals("[2:5] [highlight] unknown field [bad_fieldname], parser not found", e.getMessage());
222+
assertEquals("[2:5] [highlight] unknown field [bad_fieldname]", e.getMessage());
223223
}
224224

225225
{
@@ -232,7 +232,7 @@ public void testUnknownObjectFieldnameExpection() throws IOException {
232232
"}\n");
233233
assertThat(e.getMessage(), containsString("[highlight] failed to parse field [fields]"));
234234
assertThat(e.getCause().getMessage(), containsString("[fields] failed to parse field [body]"));
235-
assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname], parser not found", e.getCause().getCause().getMessage());
235+
assertEquals("[4:9] [highlight_field] unknown field [bad_fieldname]", e.getCause().getCause().getMessage());
236236
}
237237
}
238238

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ public void testUnknownFieldsExpection() throws IOException {
254254
"}\n";
255255
try (XContentParser parser = createParser(rescoreElement)) {
256256
XContentParseException e = expectThrows(XContentParseException.class, () -> RescorerBuilder.parseFromXContent(parser));
257-
assertEquals("[3:17] [query] unknown field [bad_fieldname], parser not found", e.getMessage());
257+
assertEquals("[3:17] [query] unknown field [bad_fieldname]", e.getMessage());
258258
}
259259

260260
rescoreElement = "{\n" +

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ public void testUnknownOptionFails() throws IOException {
316316
parser.nextToken();
317317

318318
XContentParseException e = expectThrows(XContentParseException.class, () -> FieldSortBuilder.fromXContent(parser, ""));
319-
assertEquals("[1:18] [field_sort] unknown field [reverse], parser not found", e.getMessage());
319+
assertEquals("[1:18] [field_sort] unknown field [reverse]", e.getMessage());
320320
}
321321
}
322322

0 commit comments

Comments
 (0)