Skip to content

Commit

Permalink
Remove QueryParseContext (#25486)
Browse files Browse the repository at this point in the history
QueryParseContext is currently only used as a wrapper for an XContentParser, so
this change removes it entirely and changes the appropriate APIs that use it so
far to only accept a parser instead.
  • Loading branch information
cbuescher authored Jul 3, 2017
1 parent 0e2cfc6 commit f576c98
Show file tree
Hide file tree
Showing 118 changed files with 484 additions and 701 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentLocation;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.rest.RestStatus;

import java.io.IOException;

/**
* Exception that can be used when parsing queries with a given {@link QueryParseContext}.
* Exception that can be used when parsing queries with a given {@link
* XContentParser}.
* Can contain information about location of the error.
*/
public class ParsingException extends ElasticsearchException {
Expand All @@ -57,7 +58,7 @@ public ParsingException(XContentLocation contentLocation, String msg, Throwable

/**
* This constructor is provided for use in unit tests where a
* {@link QueryParseContext} may not be available
* {@link XContentParser} may not be available
*/
public ParsingException(int line, int col, String msg, Throwable cause) {
super(msg, cause);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentLocation;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentParser.Token;
import org.elasticsearch.common.xcontent.XContentType;

import java.io.IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
builder.endObject();
}

private static ObjectParser<IdsQueryBuilder, QueryParseContext> PARSER = new ObjectParser<>(NAME,
private static ObjectParser<IdsQueryBuilder, Void> PARSER = new ObjectParser<>(NAME,
() -> new IdsQueryBuilder());

static {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl
public static final ParseField IGNORE_UNMAPPED = new ParseField("ignore_unmapped");
public static final QueryBuilder DEFAULT_INNER_HIT_QUERY = new MatchAllQueryBuilder();

private static final ObjectParser<InnerHitBuilder, QueryParseContext> PARSER = new ObjectParser<>("inner_hits", InnerHitBuilder::new);
private static final ObjectParser<InnerHitBuilder, Void> PARSER = new ObjectParser<>("inner_hits", InnerHitBuilder::new);

static {
PARSER.declareString(InnerHitBuilder::setName, NAME_FIELD);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
builder.endObject();
}

private static final ObjectParser<MatchAllQueryBuilder, QueryParseContext> PARSER = new ObjectParser<>(NAME, MatchAllQueryBuilder::new);
private static final ObjectParser<MatchAllQueryBuilder, Void> PARSER = new ObjectParser<>(NAME, MatchAllQueryBuilder::new);

static {
declareStandardFields(PARSER);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,7 @@ protected boolean doEquals(MoreLikeThisQueryBuilder other) {
}

@Override
protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException {
protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) {
// TODO this needs heavy cleanups before we can rewrite it
return this;
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,6 @@ public NamedXContentRegistry getXContentRegistry() {
return xContentRegistry;
}

/**
* Returns a new {@link QueryParseContext} that wraps the provided parser.
*/
public QueryParseContext newParseContext(XContentParser parser) {
return new QueryParseContext(parser);
}

public long nowInMillis() {
return nowInMillis.getAsLong();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ protected boolean doEquals(TermsQueryBuilder other) {
}

@Override
protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException {
protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) {
if (this.termsLookup != null) {
TermsLookup termsLookup = new TermsLookup(this.termsLookup);
if (termsLookup.index() == null) { // TODO this should go away?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
Expand Down Expand Up @@ -91,8 +90,7 @@ public static MultiSearchRequest parseRequest(RestRequest restRequest, boolean a

parseMultiLineRequest(restRequest, multiRequest.indicesOptions(), allowExplicitIndex, (searchRequest, parser) -> {
try {
final QueryParseContext queryParseContext = new QueryParseContext(parser);
searchRequest.source(SearchSourceBuilder.fromXContent(queryParseContext));
searchRequest.source(SearchSourceBuilder.fromXContent(parser));
multiRequest.add(searchRequest);
} catch (IOException e) {
throw new ElasticsearchParseException("Exception when parsing search request", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
Expand Down Expand Up @@ -94,8 +93,7 @@ public static void parseSearchRequest(SearchRequest searchRequest, RestRequest r
}
searchRequest.indices(Strings.splitStringByCommaToArray(request.param("index")));
if (requestContentParser != null) {
QueryParseContext context = new QueryParseContext(requestContentParser);
searchRequest.source().parseXContent(context);
searchRequest.source().parseXContent(requestContentParser);
}

final int batchedReduceSize = request.paramAsInt("batched_reduce_size", searchRequest.getBatchedReduceSize());
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/elasticsearch/search/SearchModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ private void registerAggregations(List<SearchPlugin> plugins) {
registerAggregation(new AggregationSpec(FiltersAggregationBuilder.NAME, FiltersAggregationBuilder::new,
FiltersAggregationBuilder::parse).addResultReader(InternalFilters::new));
registerAggregation(new AggregationSpec(AdjacencyMatrixAggregationBuilder.NAME, AdjacencyMatrixAggregationBuilder::new,
AdjacencyMatrixAggregationBuilder.getParser()).addResultReader(InternalAdjacencyMatrix::new));
AdjacencyMatrixAggregationBuilder::parse).addResultReader(InternalAdjacencyMatrix::new));
registerAggregation(new AggregationSpec(SamplerAggregationBuilder.NAME, SamplerAggregationBuilder::new,
SamplerAggregationBuilder::parse)
.addResultReader(InternalSampler.NAME, InternalSampler::new)
Expand Down Expand Up @@ -410,7 +410,7 @@ private void registerAggregation(AggregationSpec spec) {
if (false == transportClient) {
namedXContents.add(new NamedXContentRegistry.Entry(BaseAggregationBuilder.class, spec.getName(), (p, c) -> {
AggregatorFactories.AggParseContext context = (AggregatorFactories.AggParseContext) c;
return spec.getParser().parse(context.name, context.queryParseContext);
return spec.getParser().parse(context.name, p);
}));
}
namedWriteables.add(
Expand Down Expand Up @@ -507,7 +507,7 @@ private void registerPipelineAggregation(PipelineAggregationSpec spec) {
if (false == transportClient) {
namedXContents.add(new NamedXContentRegistry.Entry(BaseAggregationBuilder.class, spec.getName(), (p, c) -> {
AggregatorFactories.AggParseContext context = (AggregatorFactories.AggParseContext) c;
return spec.getParser().parse(context.name, context.queryParseContext);
return spec.getParser().parse(context.name, p);
}));
}
namedWriteables.add(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.NamedWriteable;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.search.internal.SearchContext;

import java.io.IOException;
Expand Down Expand Up @@ -84,9 +84,10 @@ public List<PipelineAggregationBuilder> getPipelineAggregations() {
}

/**
* Internal: Registers sub-factories with this factory. The sub-factory will be
* responsible for the creation of sub-aggregators under the aggregator
* created by this factory. This is only for use by {@link AggregatorFactories#parseAggregators(QueryParseContext)}.
* Internal: Registers sub-factories with this factory. The sub-factory will
* be responsible for the creation of sub-aggregators under the aggregator
* created by this factory. This is only for use by
* {@link AggregatorFactories#parseAggregators(XContentParser)}.
*
* @param subFactories
* The sub-factories
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.lease.Releasable;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.search.aggregations.bucket.BucketsAggregator;
import org.elasticsearch.search.internal.SearchContext;

Expand All @@ -50,11 +50,11 @@ public interface Parser {
* aggregation should be skipped (e.g. when trying to aggregate on unmapped fields).
*
* @param aggregationName The name of the aggregation
* @param context The parse context
* @param parser The parser
* @return The resolved aggregator factory or {@code null} in case the aggregation should be skipped
* @throws java.io.IOException When parsing fails
*/
AggregationBuilder parse(String aggregationName, QueryParseContext context) throws IOException;
AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
import org.elasticsearch.search.aggregations.support.AggregationPath;
import org.elasticsearch.search.aggregations.support.AggregationPath.PathElement;
Expand All @@ -50,24 +49,18 @@ public class AggregatorFactories {
public static final Pattern VALID_AGG_NAME = Pattern.compile("[^\\[\\]>]+");

/**
* Parses the aggregation request recursively generating aggregator factories in turn.
*
* @param parseContext The parse context.
*
* @return The parsed aggregator factories.
*
* @throws IOException When parsing fails for unknown reasons.
* Parses the aggregation request recursively generating aggregator
* factories in turn.
*/
public static AggregatorFactories.Builder parseAggregators(QueryParseContext parseContext) throws IOException {
return parseAggregators(parseContext, 0);
public static AggregatorFactories.Builder parseAggregators(XContentParser parser) throws IOException {
return parseAggregators(parser, 0);
}

private static AggregatorFactories.Builder parseAggregators(QueryParseContext parseContext, int level) throws IOException {
private static AggregatorFactories.Builder parseAggregators(XContentParser parser, int level) throws IOException {
Matcher validAggMatcher = VALID_AGG_NAME.matcher("");
AggregatorFactories.Builder factories = new AggregatorFactories.Builder();

XContentParser.Token token = null;
XContentParser parser = parseContext.parser();
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token != XContentParser.Token.FIELD_NAME) {
throw new ParsingException(parser.getTokenLocation(),
Expand Down Expand Up @@ -111,7 +104,7 @@ private static AggregatorFactories.Builder parseAggregators(QueryParseContext pa
throw new ParsingException(parser.getTokenLocation(),
"Found two sub aggregation definitions under [" + aggregationName + "]");
}
subFactories = parseAggregators(parseContext, level + 1);
subFactories = parseAggregators(parser, level + 1);
break;
default:
if (aggBuilder != null) {
Expand All @@ -120,7 +113,7 @@ private static AggregatorFactories.Builder parseAggregators(QueryParseContext pa
}

aggBuilder = parser.namedObject(BaseAggregationBuilder.class, fieldName,
new AggParseContext(aggregationName, parseContext));
new AggParseContext(aggregationName));
}
} else {
throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.START_OBJECT + "] under ["
Expand Down Expand Up @@ -156,11 +149,9 @@ private static AggregatorFactories.Builder parseAggregators(QueryParseContext pa
*/
public static final class AggParseContext {
public final String name;
public final QueryParseContext queryParseContext;

public AggParseContext(String name, QueryParseContext queryParseContext) {
public AggParseContext(String name) {
this.name = name;
this.queryParseContext = queryParseContext;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.elasticsearch.common.xcontent.XContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.query.QueryParseContext;
import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation.Bucket;
import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregator;
import org.elasticsearch.search.aggregations.support.AggregationPath;
Expand Down Expand Up @@ -527,11 +526,10 @@ public static class Parser {
* Parse a {@link BucketOrder} from {@link XContent}.
*
* @param parser for parsing {@link XContent} that contains the order.
* @param context parsing context.
* @return bucket ordering strategy
* @throws IOException on error a {@link XContent} parsing error.
*/
public static BucketOrder parseOrderParam(XContentParser parser, QueryParseContext context) throws IOException {
public static BucketOrder parseOrderParam(XContentParser parser) throws IOException {
XContentParser.Token token;
String orderKey = null;
boolean orderAsc = false;
Expand Down
Loading

0 comments on commit f576c98

Please sign in to comment.