diff --git a/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java b/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java index fd5ff846529..3bc91dc48e2 100644 --- a/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java +++ b/core/src/main/java/org/elasticsearch/rest/BaseRestHandler.java @@ -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 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 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> 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 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); } diff --git a/core/src/main/java/org/elasticsearch/rest/RestRequest.java b/core/src/main/java/org/elasticsearch/rest/RestRequest.java index 5960fc8979e..3f0f32fff37 100644 --- a/core/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/core/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -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 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. diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestAnalyzeAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestAnalyzeAction.java index 4b3035e08b5..247df1a380e 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestAnalyzeAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestAnalyzeAction.java @@ -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); diff --git a/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java b/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java index fc5af192b74..8f9dd445369 100644 --- a/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java +++ b/core/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java @@ -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 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 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()); }