Skip to content

Commit

Permalink
Made APIs consistently accept a query in the request body's query f…
Browse files Browse the repository at this point in the history
…ield.

The following APIs now accept the query in a top level `query` field like:
* delete_by_query
* validate_query
* count

These APIs used to accept the query directly in the request body which was inconsistent with the search and explain APIs. For this reason t

Closes #4074
  • Loading branch information
martijnvg committed Jan 2, 2014
1 parent dee325d commit bb01995
Show file tree
Hide file tree
Showing 36 changed files with 351 additions and 303 deletions.
2 changes: 1 addition & 1 deletion rest-api-spec/api/count.json
Expand Up @@ -41,7 +41,7 @@
}
},
"body": {
"description" : "A query to restrict the results (optional)"
"description" : "A query to restrict the results specified with the Query DSL (optional)"
}
}
}
2 changes: 1 addition & 1 deletion rest-api-spec/api/delete_by_query.json
Expand Up @@ -67,7 +67,7 @@
}
},
"body": {
"description" : "A query to restrict the operation"
"description" : "A query to restrict the operation specified with the Query DSL"
}
}
}
2 changes: 1 addition & 1 deletion rest-api-spec/api/indices.validate_query.json
Expand Up @@ -40,7 +40,7 @@
}
},
"body": {
"description" : "The query definition"
"description" : "The query definition specified with the Query DSL"
}
}
}
5 changes: 3 additions & 2 deletions rest-api-spec/test/delete_by_query/10_basic.yaml
Expand Up @@ -28,8 +28,9 @@
delete_by_query:
index: test_1
body:
match:
foo: bar
query:
match:
foo: bar

- is_true: ok

Expand Down
Expand Up @@ -27,6 +27,7 @@
import org.elasticsearch.action.admin.indices.refresh.TransportRefreshAction;
import org.elasticsearch.action.deletebyquery.DeleteByQueryResponse;
import org.elasticsearch.action.deletebyquery.TransportDeleteByQueryAction;
import org.elasticsearch.action.support.QuerySourceBuilder;
import org.elasticsearch.action.support.master.TransportMasterNodeOperationAction;
import org.elasticsearch.client.Requests;
import org.elasticsearch.cluster.ClusterService;
Expand Down Expand Up @@ -102,7 +103,9 @@ protected void masterOperation(final DeleteMappingRequest request, final Cluster
flushAction.execute(Requests.flushRequest(request.indices()), new ActionListener<FlushResponse>() {
@Override
public void onResponse(FlushResponse flushResponse) {
deleteByQueryAction.execute(Requests.deleteByQueryRequest(request.indices()).query(QueryBuilders.filteredQuery(QueryBuilders.matchAllQuery(), FilterBuilders.typeFilter(request.type()))), new ActionListener<DeleteByQueryResponse>() {
QuerySourceBuilder querySourceBuilder = new QuerySourceBuilder()
.setQuery(QueryBuilders.filteredQuery(QueryBuilders.matchAllQuery(), FilterBuilders.typeFilter(request.type())));
deleteByQueryAction.execute(Requests.deleteByQueryRequest(request.indices()).source(querySourceBuilder), new ActionListener<DeleteByQueryResponse>() {
@Override
public void onResponse(DeleteByQueryResponse deleteByQueryResponse) {
refreshAction.execute(Requests.refreshRequest(request.indices()), new ActionListener<RefreshResponse>() {
Expand Down
Expand Up @@ -34,7 +34,7 @@
*/
class ShardValidateQueryRequest extends BroadcastShardOperationRequest {

private BytesReference querySource;
private BytesReference source;
private String[] types = Strings.EMPTY_ARRAY;
private boolean explain;
private long nowInMillis;
Expand All @@ -48,15 +48,15 @@ class ShardValidateQueryRequest extends BroadcastShardOperationRequest {

public ShardValidateQueryRequest(String index, int shardId, @Nullable String[] filteringAliases, ValidateQueryRequest request) {
super(index, shardId, request);
this.querySource = request.querySource();
this.source = request.source();
this.types = request.types();
this.explain = request.explain();
this.filteringAliases = filteringAliases;
this.nowInMillis = request.nowInMillis;
}

public BytesReference querySource() {
return querySource;
public BytesReference source() {
return source;
}

public String[] types() {
Expand All @@ -78,7 +78,7 @@ public long nowInMillis() {
@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
querySource = in.readBytesReference();
source = in.readBytesReference();

int typesSize = in.readVInt();
if (typesSize > 0) {
Expand Down Expand Up @@ -107,7 +107,7 @@ public void readFrom(StreamInput in) throws IOException {
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeBytesReference(querySource);
out.writeBytesReference(source);

out.writeVInt(types.length);
for (String type : types) {
Expand Down
Expand Up @@ -174,15 +174,15 @@ protected ShardValidateQueryResponse shardOperation(ShardValidateQueryRequest re
boolean valid;
String explanation = null;
String error = null;
if (request.querySource().length() == 0) {
if (request.source().length() == 0) {
valid = true;
} else {
SearchContext.setCurrent(new DefaultSearchContext(0,
new ShardSearchRequest().types(request.types()).nowInMillis(request.nowInMillis()),
null, indexShard.acquireSearcher("validate_query"), indexService, indexShard,
scriptService, cacheRecycler));
try {
ParsedQuery parsedQuery = queryParserService.parse(request.querySource());
ParsedQuery parsedQuery = queryParserService.parseQuery(request.source());
valid = true;
if (request.explain()) {
explanation = parsedQuery.query().toString();
Expand Down
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.ElasticSearchGenerationException;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.QuerySourceBuilder;
import org.elasticsearch.action.support.broadcast.BroadcastOperationRequest;
import org.elasticsearch.client.Requests;
import org.elasticsearch.common.Strings;
Expand All @@ -32,7 +33,6 @@
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.query.QueryBuilder;

import java.io.IOException;
import java.util.Arrays;
Expand All @@ -41,15 +41,15 @@
/**
* A request to validate a specific query.
* <p/>
* <p>The request requires the query source to be set either using {@link #query(org.elasticsearch.index.query.QueryBuilder)},
* or {@link #query(byte[])}.
* <p>The request requires the query source to be set either using {@link #source(QuerySourceBuilder)},
* or {@link #source(byte[])}.
*/
public class ValidateQueryRequest extends BroadcastOperationRequest<ValidateQueryRequest> {

private static final XContentType contentType = Requests.CONTENT_TYPE;

private BytesReference querySource;
private boolean querySourceUnsafe;
private BytesReference source;
private boolean sourceUnsafe;

private boolean explain;

Expand All @@ -76,79 +76,74 @@ public ActionRequestValidationException validate() {

@Override
protected void beforeStart() {
if (querySourceUnsafe) {
querySource = querySource.copyBytesArray();
querySourceUnsafe = false;
if (sourceUnsafe) {
source = source.copyBytesArray();
sourceUnsafe = false;
}
}

/**
* The query source to execute.
* The source to execute.
*/
BytesReference querySource() {
return querySource;
BytesReference source() {
return source;
}

/**
* The query source to execute.
*
* @see org.elasticsearch.index.query.QueryBuilders
*/
public ValidateQueryRequest query(QueryBuilder queryBuilder) {
this.querySource = queryBuilder.buildAsBytes();
this.querySourceUnsafe = false;
public ValidateQueryRequest source(QuerySourceBuilder sourceBuilder) {
this.source = sourceBuilder.buildAsBytes(contentType);
this.sourceUnsafe = false;
return this;
}

/**
* The query source to execute in the form of a map.
* The source to execute in the form of a map.
*/
public ValidateQueryRequest query(Map querySource) {
public ValidateQueryRequest source(Map source) {
try {
XContentBuilder builder = XContentFactory.contentBuilder(contentType);
builder.map(querySource);
return query(builder);
builder.map(source);
return source(builder);
} catch (IOException e) {
throw new ElasticSearchGenerationException("Failed to generate [" + querySource + "]", e);
throw new ElasticSearchGenerationException("Failed to generate [" + source + "]", e);
}
}

public ValidateQueryRequest query(XContentBuilder builder) {
this.querySource = builder.bytes();
this.querySourceUnsafe = false;
public ValidateQueryRequest source(XContentBuilder builder) {
this.source = builder.bytes();
this.sourceUnsafe = false;
return this;
}

/**
* The query source to validate. It is preferable to use either {@link #query(byte[])}
* or {@link #query(org.elasticsearch.index.query.QueryBuilder)}.
* The query source to validate. It is preferable to use either {@link #source(byte[])}
* or {@link #source(QuerySourceBuilder)}.
*/
public ValidateQueryRequest query(String querySource) {
this.querySource = new BytesArray(querySource);
this.querySourceUnsafe = false;
public ValidateQueryRequest source(String source) {
this.source = new BytesArray(source);
this.sourceUnsafe = false;
return this;
}

/**
* The query source to validate.
* The source to validate.
*/
public ValidateQueryRequest query(byte[] querySource) {
return query(querySource, 0, querySource.length, false);
public ValidateQueryRequest source(byte[] source) {
return source(source, 0, source.length, false);
}

/**
* The query source to validate.
* The source to validate.
*/
public ValidateQueryRequest query(byte[] querySource, int offset, int length, boolean unsafe) {
return query(new BytesArray(querySource, offset, length), unsafe);
public ValidateQueryRequest source(byte[] source, int offset, int length, boolean unsafe) {
return source(new BytesArray(source, offset, length), unsafe);
}

/**
* The query source to validate.
* The source to validate.
*/
public ValidateQueryRequest query(BytesReference querySource, boolean unsafe) {
this.querySource = querySource;
this.querySourceUnsafe = unsafe;
public ValidateQueryRequest source(BytesReference source, boolean unsafe) {
this.source = source;
this.sourceUnsafe = unsafe;
return this;
}

Expand Down Expand Up @@ -185,8 +180,8 @@ public boolean explain() {
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);

querySourceUnsafe = false;
querySource = in.readBytesReference();
sourceUnsafe = false;
source = in.readBytesReference();

int typesSize = in.readVInt();
if (typesSize > 0) {
Expand All @@ -204,7 +199,7 @@ public void readFrom(StreamInput in) throws IOException {
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);

out.writeBytesReference(querySource);
out.writeBytesReference(source);

out.writeVInt(types.length);
for (String type : types) {
Expand All @@ -218,10 +213,10 @@ public void writeTo(StreamOutput out) throws IOException {
public String toString() {
String sSource = "_na_";
try {
sSource = XContentHelper.convertToJson(querySource, false);
sSource = XContentHelper.convertToJson(source, false);
} catch (Exception e) {
// ignore
}
return "[" + Arrays.toString(indices) + "]" + Arrays.toString(types) + ", querySource[" + sSource + "], explain:" + explain;
return "[" + Arrays.toString(indices) + "]" + Arrays.toString(types) + ", source[" + sSource + "], explain:" + explain;
}
}
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.action.admin.indices.validate.query;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.QuerySourceBuilder;
import org.elasticsearch.action.support.broadcast.BroadcastOperationRequestBuilder;
import org.elasticsearch.client.IndicesAdminClient;
import org.elasticsearch.client.internal.InternalIndicesAdminClient;
Expand All @@ -31,6 +32,8 @@
*/
public class ValidateQueryRequestBuilder extends BroadcastOperationRequestBuilder<ValidateQueryRequest, ValidateQueryResponse, ValidateQueryRequestBuilder> {

private QuerySourceBuilder sourceBuilder;

public ValidateQueryRequestBuilder(IndicesAdminClient client) {
super((InternalIndicesAdminClient) client, new ValidateQueryRequest());
}
Expand All @@ -49,37 +52,37 @@ public ValidateQueryRequestBuilder setTypes(String... types) {
* @see org.elasticsearch.index.query.QueryBuilders
*/
public ValidateQueryRequestBuilder setQuery(QueryBuilder queryBuilder) {
request.query(queryBuilder);
sourceBuilder().setQuery(queryBuilder);
return this;
}

/**
* The query source to validate.
* The source to validate.
*
* @see org.elasticsearch.index.query.QueryBuilders
*/
public ValidateQueryRequestBuilder setQuery(BytesReference querySource) {
request.query(querySource, false);
public ValidateQueryRequestBuilder setSource(BytesReference source) {
request().source(source, false);
return this;
}

/**
* The query source to validate.
* The source to validate.
*
* @see org.elasticsearch.index.query.QueryBuilders
*/
public ValidateQueryRequestBuilder setQuery(BytesReference querySource, boolean unsafe) {
request.query(querySource, unsafe);
public ValidateQueryRequestBuilder setSource(BytesReference source, boolean unsafe) {
request().source(source, unsafe);
return this;
}

/**
* The query source to validate.
* The source to validate.
*
* @see org.elasticsearch.index.query.QueryBuilders
*/
public ValidateQueryRequestBuilder setQuery(byte[] querySource) {
request.query(querySource);
public ValidateQueryRequestBuilder setSource(byte[] source) {
request.source(source);
return this;
}

Expand All @@ -95,6 +98,17 @@ public ValidateQueryRequestBuilder setExplain(boolean explain) {

@Override
protected void doExecute(ActionListener<ValidateQueryResponse> listener) {
if (sourceBuilder != null) {
request.source(sourceBuilder);
}

((IndicesAdminClient) client).validateQuery(request, listener);
}

private QuerySourceBuilder sourceBuilder() {
if (sourceBuilder == null) {
sourceBuilder = new QuerySourceBuilder();
}
return sourceBuilder;
}
}

2 comments on commit bb01995

@roytmana
Copy link

Choose a reason for hiding this comment

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

Could we also make all APIs take it as form encoded post parameter. GET parameter may have size issue when a large query needs to be passed.

The reason is that some client side frameworks (ex. ExtJs) do not work with post body wery well relying on form encoded parameters instead

@martijnvg
Copy link
Member Author

Choose a reason for hiding this comment

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

Hey @roytmana this is unrelated to this commit, which is about consistently supporting query json field in request body. If you think that supporting form encoded post parameters is needed then I suggest you open a dedicated issue for it.

Please sign in to comment.