honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add did you mean to strict REST params 

This commit adds a did you mean feature to the strict REST params error
message. This works by comparing any unconsumed parameters to all of the
consumer parameters, comparing the Levenstein distance between those
parameters, and taking any consumed parameters that are close to an
unconsumed parameter as candiates for the did you mean.

* Fix pluralization in strict REST params message

This commit fixes the pluralization in the strict REST parameters error
message so that the word "parameter" is not unconditionally written as
"parameters" even when there is only one unrecognized parameter.

* Strength strict REST params did you mean test

This commit adds an unconsumed parameter that is too far from every
consumed parameter to have any candidate suggestions.

Relates #20747
This commit is contained in:
Jason Tedor 2016-10-04 20:45:07 -04:00 committed by Simon Willnauer
parent 4b82703bf7
commit d7dca15970
4 changed files with 108 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java
View File

@ -19,8 +19,11 @@
package org.elasticsearch.rest;
import org.apache.lucene.search.spell.LevensteinDistance;
import org.apache.lucene.util.CollectionUtil;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
@ -28,10 +31,13 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugins.ActionPlugin;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.stream.Collectors;
/**
@ -59,16 +65,44 @@ public abstract class BaseRestHandler extends AbstractComponent implements RestH
final RestChannelConsumer action = prepareRequest(request, client);
// validate unconsumed params, but we must exclude params used to format the response
final List<String> unconsumedParams =
request.unconsumedParams().stream().filter(p -> !responseParams().contains(p)).collect(Collectors.toList());
// use a sorted set so the unconsumed parameters appear in a reliable sorted order
final SortedSet<String> unconsumedParams =
request.unconsumedParams().stream().filter(p -> !responseParams().contains(p)).collect(Collectors.toCollection(TreeSet::new));
// validate the non-response params
if (!unconsumedParams.isEmpty()) {
final String message = String.format(
String message = String.format(
Locale.ROOT,
"request [%s] contains unrecognized parameters: %s",
"request [%s] contains unrecognized parameter%s: ",
request.path(),
unconsumedParams.toString());
unconsumedParams.size() > 1 ? "s" : "");
boolean first = true;
for (final String unconsumedParam : unconsumedParams) {
final LevensteinDistance ld = new LevensteinDistance();
final List<Tuple<Float, String>> scoredParams = new ArrayList<>();
for (String consumedParam : request.consumedParams()) {
final float distance = ld.getDistance(unconsumedParam, consumedParam);
if (distance > 0.5f) {
scoredParams.add(new Tuple<>(distance, consumedParam));
}
}
CollectionUtil.timSort(scoredParams, (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;
else return a.v2().compareTo(b.v2());
});
if (first == false) {
message += ", ";
}
message += "[" + unconsumedParam + "]";
final List<String> keys = scoredParams.stream().map(Tuple::v2).collect(Collectors.toList());
if (keys.isEmpty() == false) {
message += " -> did you mean " + (keys.size() == 1 ? "[" + keys.get(0) + "]": "any of " + keys.toString()) + "?";
}
first = false;
}
throw new IllegalArgumentException(message);
}

11
core/src/main/java/org/elasticsearch/rest/RestRequest.java
View File

@ -28,7 +28,6 @@ import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.ToXContent;
import java.net.SocketAddress;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@ -129,6 +128,16 @@ public abstract class RestRequest implements ToXContent.Params {
return params;
}
/**
* Returns a list of parameters that have been consumed. This method returns a copy, callers
* are free to modify the returned list.
*
* @return the list of currently consumed parameters.
*/
List<String> consumedParams() {
return consumedParams.stream().collect(Collectors.toList());
}
/**
* Returns a list of parameters that have not yet been consumed. This method returns a copy,
* callers are free to modify the returned list.

5
core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestAnalyzeAction.java
View File

@ -73,8 +73,9 @@ public class RestAnalyzeAction extends BaseRestHandler {
analyzeRequest.text(texts);
analyzeRequest.analyzer(request.param("analyzer"));
analyzeRequest.field(request.param("field"));
if (request.hasParam("tokenizer")) {
analyzeRequest.tokenizer(request.param("tokenizer"));
final String tokenizer = request.param("tokenizer");
if (tokenizer != null) {
analyzeRequest.tokenizer(tokenizer);
}
for (String filter : request.paramAsStringArray("filter", Strings.EMPTY_ARRAY)) {
analyzeRequest.addTokenFilter(filter);

62
core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java
View File

@ -33,14 +33,34 @@ import java.util.HashMap;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import static org.hamcrest.core.AnyOf.anyOf;
import static org.hamcrest.core.StringContains.containsString;
import static org.hamcrest.object.HasToString.hasToString;
import static org.mockito.Mockito.mock;
public class BaseRestHandlerTests extends ESTestCase {
public void testUnconsumedParameters() throws Exception {
public void testOneUnconsumedParameters() throws Exception {
final AtomicBoolean executed = new AtomicBoolean();
BaseRestHandler handler = new BaseRestHandler(Settings.EMPTY) {
@Override
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
request.param("consumed");
return channel -> executed.set(true);
}
};
final HashMap<String, String> params = new HashMap<>();
params.put("consumed", randomAsciiOfLength(8));
params.put("unconsumed", randomAsciiOfLength(8));
RestRequest request = new FakeRestRequest.Builder().withParams(params).build();
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> handler.handleRequest(request, channel, mock(NodeClient.class)));
assertThat(e, hasToString(containsString("request [/] contains unrecognized parameter: [unconsumed]")));
assertFalse(executed.get());
}
public void testMultipleUnconsumedParameters() throws Exception {
final AtomicBoolean executed = new AtomicBoolean();
BaseRestHandler handler = new BaseRestHandler(Settings.EMPTY) {
@Override
@ -56,14 +76,44 @@ public class BaseRestHandlerTests extends ESTestCase {
params.put("unconsumed-second", randomAsciiOfLength(8));
RestRequest request = new FakeRestRequest.Builder().withParams(params).build();
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> handler.handleRequest(request, channel, mock(NodeClient.class)));
assertThat(e, hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-first], [unconsumed-second]")));
assertFalse(executed.get());
}
public void testUnconsumedParametersDidYouMean() throws Exception {
final AtomicBoolean executed = new AtomicBoolean();
BaseRestHandler handler = new BaseRestHandler(Settings.EMPTY) {
@Override
protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
request.param("consumed");
request.param("field");
request.param("tokenizer");
request.param("very_close_to_parameter_1");
request.param("very_close_to_parameter_2");
return channel -> executed.set(true);
}
};
final HashMap<String, String> params = new HashMap<>();
params.put("consumed", randomAsciiOfLength(8));
params.put("flied", randomAsciiOfLength(8));
params.put("tokenzier", randomAsciiOfLength(8));
params.put("very_close_to_parametre", randomAsciiOfLength(8));
params.put("very_far_from_every_consumed_parameter", randomAsciiOfLength(8));
RestRequest request = new FakeRestRequest.Builder().withParams(params).build();
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> handler.handleRequest(request, channel, mock(NodeClient.class)));
assertThat(
e,
// we can not rely on ordering of the unconsumed parameters here
anyOf(
hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-first, unconsumed-second]")),
hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-second, unconsumed-first]"))));
hasToString(containsString(
"request [/] contains unrecognized parameters: " +
"[flied] -> did you mean [field]?, " +
"[tokenzier] -> did you mean [tokenizer]?, " +
"[very_close_to_parametre] -> did you mean any of [very_close_to_parameter_1, very_close_to_parameter_2]?, " +
"[very_far_from_every_consumed_parameter]")));
assertFalse(executed.get());
}