Skip to content

Commit 4f43d90

Browse files
committed
Switch query parsing to namedObject
1 parent 7141f6b commit 4f43d90

File tree

15 files changed

+140
-30
lines changed

15 files changed

+140
-30
lines changed

core/src/main/java/org/elasticsearch/common/xcontent/NamedXContentRegistry.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,16 @@ public <T> Entry(Class<T> categoryClass, ParseField name, FromXContentWithContex
9797
}
9898
}
9999

100+
private static class Category {
101+
final String name;
102+
final Map<String, Entry> entries;
103+
104+
public Category(String name, Map<String, Entry> entries) {
105+
this.name = name;
106+
this.entries = entries;
107+
}
108+
}
109+
100110
private final Map<Class<?>, Map<String, Entry>> registry;
101111

102112
public NamedXContentRegistry(List<Entry> entries) {

core/src/main/java/org/elasticsearch/index/query/MatchAllQueryBuilder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
5858
builder.endObject();
5959
}
6060

61-
private static ObjectParser<MatchAllQueryBuilder, QueryParseContext> PARSER = new ObjectParser<>(NAME, MatchAllQueryBuilder::new);
61+
private static final ObjectParser<MatchAllQueryBuilder, QueryParseContext> PARSER = new ObjectParser<>(NAME, MatchAllQueryBuilder::new);
6262

6363
static {
6464
declareStandardFields(PARSER);

core/src/main/java/org/elasticsearch/index/query/QueryParseContext.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import org.elasticsearch.common.ParseFieldMatcher;
2424
import org.elasticsearch.common.ParseFieldMatcherSupplier;
2525
import org.elasticsearch.common.ParsingException;
26+
import org.elasticsearch.common.xcontent.NamedXContentRegistry.UnknownNamedObjectException;
27+
import org.elasticsearch.common.xcontent.XContentLocation;
2628
import org.elasticsearch.common.xcontent.XContentParser;
2729
import org.elasticsearch.indices.query.IndicesQueriesRegistry;
2830
import org.elasticsearch.script.Script;
@@ -105,7 +107,14 @@ public QueryBuilder parseInnerQueryBuilder() throws IOException {
105107
if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
106108
throw new ParsingException(parser.getTokenLocation(), "[" + queryName + "] query malformed, no start_object after query name");
107109
}
108-
QueryBuilder result = indicesQueriesRegistry.lookup(queryName, parseFieldMatcher, parser.getTokenLocation()).fromXContent(this);
110+
QueryBuilder result;
111+
try {
112+
result = parser.namedObject(QueryBuilder.class, queryName, this);
113+
} catch (UnknownNamedObjectException e) {
114+
// Preserve the error message from 5.0 until we have a compellingly better message so we don't break BWC.
115+
throw new ParsingException(new XContentLocation(e.getLineNumber(), e.getColumnNumber()),
116+
"no [query] registered for [" + e.getName() + "]", e);
117+
}
109118
//end_object of the specific query (e.g. match, multi_match etc.) element
110119
if (parser.currentToken() != XContentParser.Token.END_OBJECT) {
111120
throw new ParsingException(parser.getTokenLocation(),

core/src/main/java/org/elasticsearch/search/SearchModule.java

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -442,11 +442,12 @@ private void registerAggregation(AggregationSpec spec) {
442442
if (false == transportClient) {
443443
aggregationParserRegistry.register(spec.getParser(), spec.getName());
444444
}
445-
namedWriteables.add(new Entry(AggregationBuilder.class, spec.getName().getPreferredName(), spec.getReader()));
445+
namedWriteables.add(
446+
new NamedWriteableRegistry.Entry(AggregationBuilder.class, spec.getName().getPreferredName(), spec.getReader()));
446447
for (Map.Entry<String, Writeable.Reader<? extends InternalAggregation>> t : spec.getResultReaders().entrySet()) {
447448
String writeableName = t.getKey();
448449
Writeable.Reader<? extends InternalAggregation> internalReader = t.getValue();
449-
namedWriteables.add(new Entry(InternalAggregation.class, writeableName, internalReader));
450+
namedWriteables.add(new NamedWriteableRegistry.Entry(InternalAggregation.class, writeableName, internalReader));
450451
}
451452
}
452453

@@ -535,10 +536,13 @@ private void registerPipelineAggregation(PipelineAggregationSpec spec) {
535536
if (false == transportClient) {
536537
pipelineAggregationParserRegistry.register(spec.getParser(), spec.getName());
537538
}
538-
namedWriteables.add(new Entry(PipelineAggregationBuilder.class, spec.getName().getPreferredName(), spec.getReader()));
539-
namedWriteables.add(new Entry(PipelineAggregator.class, spec.getName().getPreferredName(), spec.getAggregatorReader()));
539+
namedWriteables.add(
540+
new NamedWriteableRegistry.Entry(PipelineAggregationBuilder.class, spec.getName().getPreferredName(), spec.getReader()));
541+
namedWriteables.add(
542+
new NamedWriteableRegistry.Entry(PipelineAggregator.class, spec.getName().getPreferredName(), spec.getAggregatorReader()));
540543
for (Map.Entry<String, Writeable.Reader<? extends InternalAggregation>> resultReader : spec.getResultReaders().entrySet()) {
541-
namedWriteables.add(new Entry(InternalAggregation.class, resultReader.getKey(), resultReader.getValue()));
544+
namedWriteables
545+
.add(new NamedWriteableRegistry.Entry(InternalAggregation.class, resultReader.getKey(), resultReader.getValue()));
542546
}
543547
}
544548

@@ -549,14 +553,14 @@ private void registerShapes() {
549553
}
550554

551555
private void registerRescorers() {
552-
namedWriteables.add(new Entry(RescoreBuilder.class, QueryRescorerBuilder.NAME, QueryRescorerBuilder::new));
556+
namedWriteables.add(new NamedWriteableRegistry.Entry(RescoreBuilder.class, QueryRescorerBuilder.NAME, QueryRescorerBuilder::new));
553557
}
554558

555559
private void registerSorts() {
556-
namedWriteables.add(new Entry(SortBuilder.class, GeoDistanceSortBuilder.NAME, GeoDistanceSortBuilder::new));
557-
namedWriteables.add(new Entry(SortBuilder.class, ScoreSortBuilder.NAME, ScoreSortBuilder::new));
558-
namedWriteables.add(new Entry(SortBuilder.class, ScriptSortBuilder.NAME, ScriptSortBuilder::new));
559-
namedWriteables.add(new Entry(SortBuilder.class, FieldSortBuilder.NAME, FieldSortBuilder::new));
560+
namedWriteables.add(new NamedWriteableRegistry.Entry(SortBuilder.class, GeoDistanceSortBuilder.NAME, GeoDistanceSortBuilder::new));
561+
namedWriteables.add(new NamedWriteableRegistry.Entry(SortBuilder.class, ScoreSortBuilder.NAME, ScoreSortBuilder::new));
562+
namedWriteables.add(new NamedWriteableRegistry.Entry(SortBuilder.class, ScriptSortBuilder.NAME, ScriptSortBuilder::new));
563+
namedWriteables.add(new NamedWriteableRegistry.Entry(SortBuilder.class, FieldSortBuilder.NAME, FieldSortBuilder::new));
560564
}
561565

562566
private <T> void registerFromPlugin(List<SearchPlugin> plugins, Function<SearchPlugin, List<T>> producer, Consumer<T> consumer) {
@@ -568,9 +572,9 @@ private <T> void registerFromPlugin(List<SearchPlugin> plugins, Function<SearchP
568572
}
569573

570574
public static void registerSmoothingModels(List<Entry> namedWriteables) {
571-
namedWriteables.add(new Entry(SmoothingModel.class, Laplace.NAME, Laplace::new));
572-
namedWriteables.add(new Entry(SmoothingModel.class, LinearInterpolation.NAME, LinearInterpolation::new));
573-
namedWriteables.add(new Entry(SmoothingModel.class, StupidBackoff.NAME, StupidBackoff::new));
575+
namedWriteables.add(new NamedWriteableRegistry.Entry(SmoothingModel.class, Laplace.NAME, Laplace::new));
576+
namedWriteables.add(new NamedWriteableRegistry.Entry(SmoothingModel.class, LinearInterpolation.NAME, LinearInterpolation::new));
577+
namedWriteables.add(new NamedWriteableRegistry.Entry(SmoothingModel.class, StupidBackoff.NAME, StupidBackoff::new));
574578
}
575579

576580
private Map<String, Suggester<?>> setupSuggesters(List<SearchPlugin> plugins) {
@@ -581,7 +585,7 @@ private Map<String, Suggester<?>> setupSuggesters(List<SearchPlugin> plugins) {
581585
@Override
582586
public void register(String name, Suggester<?> t) {
583587
super.register(name, t);
584-
namedWriteables.add(new Entry(SuggestionBuilder.class, name, t));
588+
namedWriteables.add(new NamedWriteableRegistry.Entry(SuggestionBuilder.class, name, t));
585589
}
586590
};
587591
suggesters.register("phrase", PhraseSuggester.INSTANCE);
@@ -619,7 +623,7 @@ private void registerScoreFunctions(List<SearchPlugin> plugins) {
619623

620624
//weight doesn't have its own parser, so every function supports it out of the box.
621625
//Can be a single function too when not associated to any other function, which is why it needs to be registered manually here.
622-
namedWriteables.add(new Entry(ScoreFunctionBuilder.class, WeightBuilder.NAME, WeightBuilder::new));
626+
namedWriteables.add(new NamedWriteableRegistry.Entry(ScoreFunctionBuilder.class, WeightBuilder.NAME, WeightBuilder::new));
623627

624628
registerFromPlugin(plugins, SearchPlugin::getScoreFunctions, this::registerScoreFunction);
625629
}
@@ -646,7 +650,7 @@ private void registerValueFormats() {
646650
* Register a new ValueFormat.
647651
*/
648652
private void registerValueFormat(String name, Writeable.Reader<? extends DocValueFormat> reader) {
649-
namedWriteables.add(new Entry(DocValueFormat.class, name, reader));
653+
namedWriteables.add(new NamedWriteableRegistry.Entry(DocValueFormat.class, name, reader));
650654
}
651655

652656
private void registerSignificanceHeuristics(List<SearchPlugin> plugins) {
@@ -662,7 +666,8 @@ private void registerSignificanceHeuristics(List<SearchPlugin> plugins) {
662666

663667
private void registerSignificanceHeuristic(SearchExtensionSpec<SignificanceHeuristic, SignificanceHeuristicParser> heuristic) {
664668
significanceHeuristicParserRegistry.register(heuristic.getParser(), heuristic.getName());
665-
namedWriteables.add(new Entry(SignificanceHeuristic.class, heuristic.getName().getPreferredName(), heuristic.getReader()));
669+
namedWriteables.add(new NamedWriteableRegistry.Entry(SignificanceHeuristic.class, heuristic.getName().getPreferredName(),
670+
heuristic.getReader()));
666671
}
667672

668673
private void registerMovingAverageModels(List<SearchPlugin> plugins) {
@@ -677,7 +682,8 @@ private void registerMovingAverageModels(List<SearchPlugin> plugins) {
677682

678683
private void registerMovingAverageModel(SearchExtensionSpec<MovAvgModel, MovAvgModel.AbstractModelParser> movAvgModel) {
679684
movingAverageModelParserRegistry.register(movAvgModel.getParser(), movAvgModel.getName());
680-
namedWriteables.add(new Entry(MovAvgModel.class, movAvgModel.getName().getPreferredName(), movAvgModel.getReader()));
685+
namedWriteables.add(
686+
new NamedWriteableRegistry.Entry(MovAvgModel.class, movAvgModel.getName().getPreferredName(), movAvgModel.getReader()));
681687
}
682688

683689
private void registerFetchSubPhases(List<SearchPlugin> plugins) {
@@ -700,7 +706,7 @@ private void registerSearchExts(List<SearchPlugin> plugins) {
700706

701707
private void registerSearchExt(SearchExtSpec<?> spec) {
702708
searchExtParserRegistry.register(spec.getParser(), spec.getName());
703-
namedWriteables.add(new Entry(SearchExtBuilder.class, spec.getName().getPreferredName(), spec.getReader()));
709+
namedWriteables.add(new NamedWriteableRegistry.Entry(SearchExtBuilder.class, spec.getName().getPreferredName(), spec.getReader()));
704710
}
705711

706712
private void registerFetchSubPhase(FetchSubPhase subPhase) {
@@ -775,7 +781,9 @@ private void registerQueryParsers(List<SearchPlugin> plugins) {
775781

776782
private void registerQuery(QuerySpec<?> spec) {
777783
queryParserRegistry.register(spec.getParser(), spec.getName());
778-
namedWriteables.add(new Entry(QueryBuilder.class, spec.getName().getPreferredName(), spec.getReader()));
784+
namedWriteables.add(new NamedWriteableRegistry.Entry(QueryBuilder.class, spec.getName().getPreferredName(), spec.getReader()));
785+
namedXContents.add(new NamedXContentRegistry.Entry(QueryBuilder.class, spec.getName(),
786+
(p, c) -> spec.getParser().fromXContent((QueryParseContext) c)));
779787
}
780788

781789
public FetchPhase getFetchPhase() {

core/src/test/java/org/elasticsearch/action/search/MultiSearchRequestTests.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,18 @@
2020
package org.elasticsearch.action.search;
2121

2222
import org.elasticsearch.action.support.IndicesOptions;
23+
import org.elasticsearch.common.ParseField;
2324
import org.elasticsearch.common.ParseFieldMatcher;
2425
import org.elasticsearch.common.Strings;
2526
import org.elasticsearch.common.bytes.BytesArray;
27+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2628
import org.elasticsearch.common.xcontent.ToXContent;
2729
import org.elasticsearch.common.xcontent.XContentBuilder;
2830
import org.elasticsearch.common.xcontent.XContentFactory;
31+
import org.elasticsearch.common.xcontent.XContentParser;
2932
import org.elasticsearch.index.query.MatchAllQueryBuilder;
33+
import org.elasticsearch.index.query.QueryBuilder;
34+
import org.elasticsearch.index.query.QueryParseContext;
3035
import org.elasticsearch.index.query.QueryParser;
3136
import org.elasticsearch.indices.query.IndicesQueriesRegistry;
3237
import org.elasticsearch.rest.RestRequest;
@@ -38,6 +43,7 @@
3843

3944
import java.io.IOException;
4045

46+
import static java.util.Collections.singletonList;
4147
import static org.hamcrest.Matchers.equalTo;
4248
import static org.hamcrest.Matchers.is;
4349
import static org.hamcrest.Matchers.nullValue;
@@ -171,6 +177,12 @@ private MultiSearchRequest parseMultiSearchRequest(String sample) throws IOExcep
171177
return RestMultiSearchAction.parseRequest(restRequest, true, parsers(), ParseFieldMatcher.EMPTY);
172178
}
173179

180+
@Override
181+
protected NamedXContentRegistry xContentRegistry() {
182+
return new NamedXContentRegistry(singletonList(new NamedXContentRegistry.Entry(QueryBuilder.class,
183+
new ParseField(MatchAllQueryBuilder.NAME), (p, c) -> MatchAllQueryBuilder.fromXContent((QueryParseContext) c))));
184+
}
185+
174186
private SearchRequestParsers parsers() {
175187
IndicesQueriesRegistry registry = new IndicesQueriesRegistry();
176188
QueryParser<MatchAllQueryBuilder> parser = MatchAllQueryBuilder::fromXContent;

core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import org.elasticsearch.common.xcontent.XContentBuilder;
3131
import org.elasticsearch.common.xcontent.XContentFactory;
3232
import org.elasticsearch.common.xcontent.XContentType;
33-
import org.elasticsearch.common.xcontent.json.JsonXContent;
3433
import org.elasticsearch.search.internal.SearchContext;
3534
import org.elasticsearch.test.AbstractQueryTestCase;
3635
import org.hamcrest.Matchers;

core/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.elasticsearch.common.ParseFieldMatcher;
2323
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
2424
import org.elasticsearch.common.settings.Settings;
25+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2526
import org.elasticsearch.common.xcontent.ToXContent;
2627
import org.elasticsearch.common.xcontent.XContentBuilder;
2728
import org.elasticsearch.common.xcontent.XContentFactory;
@@ -63,18 +64,26 @@ public class InnerHitBuilderTests extends ESTestCase {
6364
private static final int NUMBER_OF_TESTBUILDERS = 20;
6465
private static NamedWriteableRegistry namedWriteableRegistry;
6566
private static IndicesQueriesRegistry indicesQueriesRegistry;
67+
private static NamedXContentRegistry xContentRegistry;
6668

6769
@BeforeClass
6870
public static void init() {
6971
SearchModule searchModule = new SearchModule(Settings.EMPTY, false, emptyList());
7072
namedWriteableRegistry = new NamedWriteableRegistry(searchModule.getNamedWriteables());
7173
indicesQueriesRegistry = searchModule.getQueryParserRegistry();
74+
xContentRegistry = new NamedXContentRegistry(searchModule.getNamedXContents());
7275
}
7376

7477
@AfterClass
7578
public static void afterClass() throws Exception {
7679
namedWriteableRegistry = null;
7780
indicesQueriesRegistry = null;
81+
xContentRegistry = null;
82+
}
83+
84+
@Override
85+
protected NamedXContentRegistry xContentRegistry() {
86+
return xContentRegistry;
7887
}
7988

8089
public void testSerialization() throws Exception {

core/src/test/java/org/elasticsearch/index/query/QueryParseContextTests.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.common.logging.DeprecationLogger;
2525
import org.elasticsearch.common.settings.Settings;
2626
import org.elasticsearch.common.util.concurrent.ThreadContext;
27+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2728
import org.elasticsearch.common.xcontent.XContentParser;
2829
import org.elasticsearch.common.xcontent.json.JsonXContent;
2930
import org.elasticsearch.indices.query.IndicesQueriesRegistry;
@@ -40,10 +41,12 @@
4041
public class QueryParseContextTests extends ESTestCase {
4142

4243
private static IndicesQueriesRegistry indicesQueriesRegistry;
44+
private static NamedXContentRegistry xContentRegistry;
4345

4446
@BeforeClass
4547
public static void init() {
4648
indicesQueriesRegistry = new SearchModule(Settings.EMPTY, false, emptyList()).getQueryParserRegistry();
49+
xContentRegistry = new NamedXContentRegistry(new SearchModule(Settings.EMPTY, false, emptyList()).getNamedXContents());
4750
}
4851

4952
private ThreadContext threadContext;
@@ -60,6 +63,11 @@ public void teardown() throws IOException {
6063
this.threadContext.close();
6164
}
6265

66+
@Override
67+
protected NamedXContentRegistry xContentRegistry() {
68+
return xContentRegistry;
69+
}
70+
6371
public void testParseTopLevelBuilder() throws IOException {
6472
QueryBuilder query = new MatchQueryBuilder("foo", "bar");
6573
String requestBody = "{ \"query\" : " + query.toString() + "}";

core/src/test/java/org/elasticsearch/search/AbstractSearchTestCase.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.elasticsearch.common.io.stream.StreamInput;
2626
import org.elasticsearch.common.io.stream.StreamOutput;
2727
import org.elasticsearch.common.settings.Settings;
28+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2829
import org.elasticsearch.common.xcontent.XContentBuilder;
2930
import org.elasticsearch.common.xcontent.XContentParser;
3031
import org.elasticsearch.indices.IndicesModule;
@@ -55,6 +56,7 @@ public abstract class AbstractSearchTestCase extends ESTestCase {
5556
protected SearchRequestParsers searchRequestParsers;
5657
private TestSearchExtPlugin searchExtPlugin;
5758
protected IndicesQueriesRegistry queriesRegistry;
59+
private NamedXContentRegistry xContentRegistry;
5860

5961
public void setUp() throws Exception {
6062
super.setUp();
@@ -65,10 +67,16 @@ public void setUp() throws Exception {
6567
entries.addAll(indicesModule.getNamedWriteables());
6668
entries.addAll(searchModule.getNamedWriteables());
6769
namedWriteableRegistry = new NamedWriteableRegistry(entries);
70+
xContentRegistry = new NamedXContentRegistry(searchModule.getNamedXContents());
6871
searchRequestParsers = searchModule.getSearchRequestParsers();
6972
queriesRegistry = searchModule.getQueryParserRegistry();
7073
}
7174

75+
@Override
76+
protected NamedXContentRegistry xContentRegistry() {
77+
return xContentRegistry;
78+
}
79+
7280
protected SearchSourceBuilder createSearchSourceBuilder() {
7381
Supplier<List<SearchExtBuilder>> randomExtBuilders = () -> {
7482
Set<String> elementNames = new HashSet<>(searchExtPlugin.getSupportedElements().keySet());

core/src/test/java/org/elasticsearch/search/aggregations/AggregatorParsingTests.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,17 @@
2222
import org.elasticsearch.common.ParseFieldMatcher;
2323
import org.elasticsearch.common.ParsingException;
2424
import org.elasticsearch.common.settings.Settings;
25+
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2526
import org.elasticsearch.common.xcontent.XContent;
2627
import org.elasticsearch.common.xcontent.XContentBuilder;
2728
import org.elasticsearch.common.xcontent.XContentParser;
2829
import org.elasticsearch.common.xcontent.json.JsonXContent;
2930
import org.elasticsearch.env.Environment;
30-
import org.elasticsearch.test.AbstractQueryTestCase;
3131
import org.elasticsearch.index.query.QueryParseContext;
3232
import org.elasticsearch.indices.query.IndicesQueriesRegistry;
3333
import org.elasticsearch.script.ScriptService;
3434
import org.elasticsearch.search.SearchModule;
35+
import org.elasticsearch.test.AbstractQueryTestCase;
3536
import org.elasticsearch.test.ESTestCase;
3637

3738
import java.util.Random;
@@ -51,6 +52,7 @@ protected String[] getCurrentTypes() {
5152

5253
protected AggregatorParsers aggParsers;
5354
protected IndicesQueriesRegistry queriesRegistry;
55+
private NamedXContentRegistry xContentRegistry;
5456
protected ParseFieldMatcher parseFieldMatcher;
5557

5658
/**
@@ -74,6 +76,7 @@ public void setUp() throws Exception {
7476
currentTypes[i] = type;
7577
}
7678
queriesRegistry = searchModule.getQueryParserRegistry();
79+
xContentRegistry = new NamedXContentRegistry(searchModule.getNamedXContents());
7780
parseFieldMatcher = ParseFieldMatcher.STRICT;
7881
}
7982

@@ -265,4 +268,9 @@ public void testMissingType() throws Exception {
265268
// All Good
266269
}
267270
}
271+
272+
@Override
273+
protected NamedXContentRegistry xContentRegistry() {
274+
return xContentRegistry;
275+
}
268276
}

0 commit comments

Comments
 (0)