Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

REST high-level client: add validate query API #31077

Merged
merged 11 commits into from
Jun 18, 2018
Merged

REST high-level client: add validate query API #31077

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6a6b61f
added validate query API
sohaibiftikhar Jun 4, 2018
aad5341
Fixes for review
sohaibiftikhar Jun 5, 2018
7bcc283
removed changes to AbstractStreamableXContentTestCase.java
sohaibiftikhar Jun 5, 2018
53b0747
switched to RequestOptions from Headers
sohaibiftikhar Jun 7, 2018
2d8f6b3
Fixes for review
sohaibiftikhar Jun 12, 2018
ed05533
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
sohaibiftikhar Jun 13, 2018
8c9cb3c
Fixed client documentation
sohaibiftikhar Jun 13, 2018
e4da255
Fixed formatting
sohaibiftikhar Jun 13, 2018
1677ed6
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
sohaibiftikhar Jun 14, 2018
00e7169
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
sohaibiftikhar Jun 14, 2018
c5095db
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
sohaibiftikhar Jun 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
import org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse;
import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest;
import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateResponse;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryRequest;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryResponse;
import org.elasticsearch.rest.RestStatus;

import java.io.IOException;
Expand Down Expand Up @@ -661,6 +663,36 @@ public void putTemplateAsync(PutIndexTemplateRequest putIndexTemplateRequest, Re
PutIndexTemplateResponse::fromXContent, listener, emptySet());
}

/**
* Validate a potentially expensive query without executing it.
* <p>
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-validate.html"> Validate Query API
* on elastic.co</a>
* @param validateQueryRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @return the response
* @throws IOException in case there is a problem sending the request or parsing back the response
*/
public ValidateQueryResponse validateQuery(ValidateQueryRequest validateQueryRequest, RequestOptions options) throws IOException {
return restHighLevelClient.performRequestAndParseEntity(validateQueryRequest, RequestConverters::validateQuery, options,
ValidateQueryResponse::fromXContent, emptySet());
}

/**
* Asynchronously validate a potentially expensive query without executing it.
* <p>
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-validate.html"> Validate Query API
* on elastic.co</a>
* @param validateQueryRequest the request
* @param options the request options (e.g. headers), use {@link RequestOptions#DEFAULT} if nothing needs to be customized
* @param listener the listener to be notified upon request completion
*/
public void validateQueryAsync(ValidateQueryRequest validateQueryRequest, RequestOptions options,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please add params tag like we have now in all of the other methods?

ActionListener<ValidateQueryResponse> listener) {
restHighLevelClient.performRequestAsyncAndParseEntity(validateQueryRequest, RequestConverters::validateQuery, options,
ValidateQueryResponse::fromXContent, listener, emptySet());
}

/**
* Gets index templates using the Index Templates API
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-templates.html"> Index Templates API
Expand Down
34 changes: 25 additions & 9 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.elasticsearch.action.admin.indices.shrink.ResizeType;
import org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesRequest;
import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryRequest;
import org.elasticsearch.action.bulk.BulkRequest;
import org.elasticsearch.action.delete.DeleteRequest;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest;
Expand Down Expand Up @@ -490,7 +491,7 @@ static Request update(UpdateRequest updateRequest) throws IOException {
XContentType upsertContentType = updateRequest.upsertRequest().getContentType();
if ((xContentType != null) && (xContentType != upsertContentType)) {
throw new IllegalStateException("Update request cannot have different content types for doc [" + xContentType + "]" +
" and upsert [" + upsertContentType + "] documents");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you revert these formatting changes that aren't part the change? It make git blame harder to work with and I kind of like the old formatting better anyway.

" and upsert [" + upsertContentType + "] documents");
} else {
xContentType = upsertContentType;
}
Expand Down Expand Up @@ -579,7 +580,7 @@ static Request searchTemplate(SearchTemplateRequest searchTemplateRequest) throw

static Request existsAlias(GetAliasesRequest getAliasesRequest) {
if ((getAliasesRequest.indices() == null || getAliasesRequest.indices().length == 0) &&
(getAliasesRequest.aliases() == null || getAliasesRequest.aliases().length == 0)) {
(getAliasesRequest.aliases() == null || getAliasesRequest.aliases().length == 0)) {
throw new IllegalArgumentException("existsAlias requires at least an alias or an index");
}
String[] indices = getAliasesRequest.indices() == null ? Strings.EMPTY_ARRAY : getAliasesRequest.indices();
Expand Down Expand Up @@ -628,8 +629,8 @@ static Request shrink(ResizeRequest resizeRequest) throws IOException {

private static Request resize(ResizeRequest resizeRequest) throws IOException {
String endpoint = new EndpointBuilder().addPathPart(resizeRequest.getSourceIndex())
.addPathPartAsIs("_" + resizeRequest.getResizeType().name().toLowerCase(Locale.ROOT))
.addPathPart(resizeRequest.getTargetIndexRequest().index()).build();
.addPathPartAsIs("_" + resizeRequest.getResizeType().name().toLowerCase(Locale.ROOT))
.addPathPart(resizeRequest.getTargetIndexRequest().index()).build();
Request request = new Request(HttpPut.METHOD_NAME, endpoint);

Params params = new Params(request);
Expand Down Expand Up @@ -733,7 +734,7 @@ static Request clusterHealth(ClusterHealthRequest healthRequest) {

static Request rollover(RolloverRequest rolloverRequest) throws IOException {
String endpoint = new EndpointBuilder().addPathPart(rolloverRequest.getAlias()).addPathPartAsIs("_rollover")
.addPathPart(rolloverRequest.getNewIndexName()).build();
.addPathPart(rolloverRequest.getNewIndexName()).build();
Request request = new Request(HttpPost.METHOD_NAME, endpoint);

Params params = new Params(request);
Expand Down Expand Up @@ -856,6 +857,21 @@ static Request putTemplate(PutIndexTemplateRequest putIndexTemplateRequest) thro
return request;
}

static Request validateQuery(ValidateQueryRequest validateQueryRequest) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test to https://github.com/elastic/elasticsearch/blob/master/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks for catching this.

String[] indices = validateQueryRequest.indices() == null ? Strings.EMPTY_ARRAY : validateQueryRequest.indices();
String[] types = validateQueryRequest.types() == null || indices.length <= 0 ? Strings.EMPTY_ARRAY : validateQueryRequest.types();
String endpoint = endpoint(indices, types, "_validate/query");
Request request = new Request(HttpGet.METHOD_NAME, endpoint);
Params params = new Params(request);
params.withIndicesOptions(validateQueryRequest.indicesOptions());
params.putParam("explain", Boolean.toString(validateQueryRequest.explain()));
params.putParam("all_shards", Boolean.toString(validateQueryRequest.allShards()));
params.putParam("rewrite", Boolean.toString(validateQueryRequest.rewrite()));
request.setEntity(createEntity(validateQueryRequest, REQUEST_BODY_CONTENT_TYPE));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like we might be missing a few more options such as types and indicesOptions, as specified in https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryAction.java#L62-L67

Copy link
Contributor Author

@sohaibiftikhar sohaibiftikhar Jun 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

types are being set in the URL I believe. I will add the indicesOptions.

return request;
}


static Request getAlias(GetAliasesRequest getAliasesRequest) {
String[] indices = getAliasesRequest.indices() == null ? Strings.EMPTY_ARRAY : getAliasesRequest.indices();
String[] aliases = getAliasesRequest.aliases() == null ? Strings.EMPTY_ARRAY : getAliasesRequest.aliases();
Expand Down Expand Up @@ -900,12 +916,12 @@ static String endpoint(String[] indices, String endpoint) {

static String endpoint(String[] indices, String[] types, String endpoint) {
return new EndpointBuilder().addCommaSeparatedPathParts(indices).addCommaSeparatedPathParts(types)
.addPathPartAsIs(endpoint).build();
.addPathPartAsIs(endpoint).build();
}

static String endpoint(String[] indices, String endpoint, String[] suffixes) {
return new EndpointBuilder().addCommaSeparatedPathParts(indices).addPathPartAsIs(endpoint)
.addCommaSeparatedPathParts(suffixes).build();
.addCommaSeparatedPathParts(suffixes).build();
}

static String endpoint(String[] indices, String endpoint, String type) {
Expand Down Expand Up @@ -1201,14 +1217,14 @@ static XContentType enforceSameContentType(IndexRequest indexRequest, @Nullable
XContentType requestContentType = indexRequest.getContentType();
if (requestContentType != XContentType.JSON && requestContentType != XContentType.SMILE) {
throw new IllegalArgumentException("Unsupported content-type found for request with content-type [" + requestContentType
+ "], only JSON and SMILE are supported");
+ "], only JSON and SMILE are supported");
}
if (xContentType == null) {
return requestContentType;
}
if (requestContentType != xContentType) {
throw new IllegalArgumentException("Mismatching content-type found for request with content-type [" + requestContentType
+ "], previous requests have content-type [" + xContentType + "]");
+ "], previous requests have content-type [" + xContentType + "]");
}
return xContentType;
}
Expand Down
39 changes: 39 additions & 0 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.client;

import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ElasticsearchStatusException;
Expand Down Expand Up @@ -63,6 +64,8 @@
import org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse;
import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest;
import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateResponse;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryRequest;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryResponse;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.WriteRequest;
Expand All @@ -80,6 +83,8 @@
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.rest.RestStatus;

import java.io.IOException;
Expand Down Expand Up @@ -1155,6 +1160,40 @@ public void testPutTemplateBadRequests() throws Exception {
assertThat(unknownSettingError.getDetailedMessage(), containsString("unknown setting [index.this-setting-does-not-exist]"));
}

public void testValidateQuery() throws IOException{
String index = "some_index";
createIndex(index, Settings.EMPTY);
QueryBuilder builder = QueryBuilders
.boolQuery()
.must(QueryBuilders.queryStringQuery("*:*"))
.filter(QueryBuilders.termQuery("user", "kimchy"));
ValidateQueryRequest request = new ValidateQueryRequest(index).query(builder);
request.explain(randomBoolean());
ValidateQueryResponse response = execute(request, highLevelClient().indices()::validateQuery,
highLevelClient().indices()::validateQueryAsync);
assertTrue(response.isValid());
}

public void testInvalidValidateQuery() throws IOException{
String index = "shakespeare";

createIndex(index, Settings.EMPTY);
Request postDoc = new Request(HttpPost.METHOD_NAME, "/" + index + "/1");
postDoc.setJsonEntity(
"{\"type\":\"act\",\"line_id\":1,\"play_name\":\"Henry IV\", \"speech_number\":\"\"," +
"\"line_number\":\"\",\"speaker\":\"\",\"text_entry\":\"ACT I\"}");
assertOK(client().performRequest(postDoc));

QueryBuilder builder = QueryBuilders
.queryStringQuery("line_id:foo")
.lenient(false);
ValidateQueryRequest request = new ValidateQueryRequest(index).query(builder);
request.explain(true);
ValidateQueryResponse response = execute(request, highLevelClient().indices()::validateQuery,
highLevelClient().indices()::validateQueryAsync);
assertFalse(response.isValid());
}

public void testGetIndexTemplate() throws Exception {
RestHighLevelClient client = highLevelClient();

Expand Down
39 changes: 37 additions & 2 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.elasticsearch.action.admin.indices.shrink.ResizeType;
import org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesRequest;
import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryRequest;
import org.elasticsearch.action.bulk.BulkRequest;
import org.elasticsearch.action.bulk.BulkShardRequest;
import org.elasticsearch.action.delete.DeleteRequest;
Expand Down Expand Up @@ -712,8 +713,8 @@ public void testSyncedFlush() {
Request request = RequestConverters.flushSynced(syncedFlushRequest);
StringJoiner endpoint = new StringJoiner("/", "/", "");
if (indices != null && indices.length > 0) {
endpoint.add(String.join(",", indices));
}
endpoint.add(String.join(",", indices));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This formatting change is fine though, because the old code was just wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware that this is somewhat contradictory.

}
endpoint.add("_flush/synced");
assertThat(request.getEndpoint(), equalTo(endpoint.toString()));
assertThat(request.getParameters(), equalTo(expectedParams));
Expand Down Expand Up @@ -1895,6 +1896,40 @@ public void testPutTemplateRequest() throws Exception {
assertToXContentBody(putTemplateRequest, request.getEntity());
}

public void testValidateQuery() throws Exception {
String[] indices = randomBoolean() ? null : randomIndicesNames(0, 5);
String[] types = randomBoolean() ? generateRandomStringArray(5, 5, false, false) : null;
ValidateQueryRequest validateQueryRequest;
if (randomBoolean()) {
validateQueryRequest = new ValidateQueryRequest(indices);
} else {
validateQueryRequest = new ValidateQueryRequest();
validateQueryRequest.indices(indices);
}
validateQueryRequest.types(types);
Map<String, String> expectedParams = new HashMap<>();
setRandomIndicesOptions(validateQueryRequest::indicesOptions, validateQueryRequest::indicesOptions, expectedParams);
validateQueryRequest.explain(randomBoolean());
validateQueryRequest.rewrite(randomBoolean());
validateQueryRequest.allShards(randomBoolean());
expectedParams.put("explain", Boolean.toString(validateQueryRequest.explain()));
expectedParams.put("rewrite", Boolean.toString(validateQueryRequest.rewrite()));
expectedParams.put("all_shards", Boolean.toString(validateQueryRequest.allShards()));
Request request = RequestConverters.validateQuery(validateQueryRequest);
StringJoiner endpoint = new StringJoiner("/", "/", "");
if (indices != null && indices.length > 0) {
endpoint.add(String.join(",", indices));
if (types != null && types.length > 0) {
endpoint.add(String.join(",", types));
}
}
endpoint.add("_validate/query");
assertThat(request.getEndpoint(), equalTo(endpoint.toString()));
assertThat(request.getParameters(), equalTo(expectedParams));
assertToXContentBody(validateQueryRequest, request.getEntity());
assertThat(request.getMethod(), equalTo(HttpGet.METHOD_NAME));
}

public void testGetTemplateRequest() throws Exception {
Map<String, String> encodes = new HashMap<>();
encodes.put("log", "log");
Expand Down
Loading