Skip to content

Commit 41b5b26

Browse files
cbueschercbuescher
committed
Query refactoring: refactored IdsQueryBuilder and Parser and added test
Split the parse(QueryParseContext ctx) method into a parsing and a query building part, adding Streamable for serialization and hashCode(), equals() for better testing. Add basic unit test for Builder and Parser. Closes elastic#10670
1 parent 304dddf commit 41b5b26

File tree

4 files changed

+224
-48
lines changed

4 files changed

+224
-48
lines changed

‎src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java‎

Lines changed: 115 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,45 +19,73 @@
1919

2020
package org.elasticsearch.index.query;
2121

22+
import com.google.common.collect.Iterables;
23+
24+
import org.apache.lucene.queries.TermsQuery;
25+
import org.apache.lucene.search.Query;
26+
import org.apache.lucene.util.BytesRef;
27+
import org.elasticsearch.common.io.stream.StreamInput;
28+
import org.elasticsearch.common.io.stream.StreamOutput;
29+
import org.elasticsearch.common.io.stream.Streamable;
30+
import org.elasticsearch.common.lucene.search.Queries;
2231
import org.elasticsearch.common.xcontent.XContentBuilder;
32+
import org.elasticsearch.index.mapper.Uid;
33+
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
2334

2435
import java.io.IOException;
2536
import java.util.ArrayList;
2637
import java.util.Arrays;
38+
import java.util.Collection;
2739
import java.util.List;
40+
import java.util.Objects;
2841

2942
/**
3043
* A query that will return only documents matching specific ids (and a type).
3144
*/
32-
public class IdsQueryBuilder extends BaseQueryBuilder implements BoostableQueryBuilder<IdsQueryBuilder> {
45+
public class IdsQueryBuilder extends BaseQueryBuilder implements Streamable, BoostableQueryBuilder<IdsQueryBuilder> {
3346

34-
private final List<String> types;
47+
private List<String> types = new ArrayList<>();
3548

36-
private List<String> values = new ArrayList<>();
49+
private List<String> ids = new ArrayList<>();
3750

38-
private float boost = -1;
51+
private float boost = 1.0f;
3952

4053
private String queryName;
4154

4255
public IdsQueryBuilder(String... types) {
43-
this.types = types == null ? null : Arrays.asList(types);
56+
this.types = (types == null || types.length == 0) ? new ArrayList<String>() : Arrays.asList(types);
57+
}
58+
59+
/**
60+
* Get the types used in this query
61+
* @return the types
62+
*/
63+
public Collection<String> types() {
64+
return this.types;
4465
}
4566

4667
/**
47-
* Adds ids to the filter.
68+
* Adds ids to the query.
4869
*/
4970
public IdsQueryBuilder addIds(String... ids) {
50-
values.addAll(Arrays.asList(ids));
71+
this.ids.addAll(Arrays.asList(ids));
5172
return this;
5273
}
5374

5475
/**
55-
* Adds ids to the filter.
76+
* Adds ids to the query.
5677
*/
5778
public IdsQueryBuilder ids(String... ids) {
5879
return addIds(ids);
5980
}
6081

82+
/**
83+
* Gets the ids for the query.
84+
*/
85+
public Collection<String> ids() {
86+
return this.ids;
87+
}
88+
6189
/**
6290
* Sets the boost for this query. Documents matching this query will (in addition to the normal
6391
* weightings) have their score multiplied by the boost provided.
@@ -69,13 +97,27 @@ public IdsQueryBuilder boost(float boost) {
6997
}
7098

7199
/**
72-
* Sets the query name for the filter that can be used when searching for matched_filters per hit.
100+
* Gets the boost for this query.
101+
*/
102+
public float boost() {
103+
return this.boost;
104+
}
105+
106+
/**
107+
* Sets the query name for the query that can be used when searching for matched_filters per hit.
73108
*/
74109
public IdsQueryBuilder queryName(String queryName) {
75110
this.queryName = queryName;
76111
return this;
77112
}
78113

114+
/**
115+
* Gets the query name for the query.
116+
*/
117+
public String queryName() {
118+
return this.queryName;
119+
}
120+
79121
@Override
80122
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
81123
builder.startObject(IdsQueryParser.NAME);
@@ -84,14 +126,14 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
84126
builder.field("type", types.get(0));
85127
} else {
86128
builder.startArray("types");
87-
for (Object type : types) {
129+
for (String type : types) {
88130
builder.value(type);
89131
}
90132
builder.endArray();
91133
}
92134
}
93135
builder.startArray("values");
94-
for (Object value : values) {
136+
for (String value : ids) {
95137
builder.value(value);
96138
}
97139
builder.endArray();
@@ -108,4 +150,66 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
108150
protected String parserName() {
109151
return IdsQueryParser.NAME;
110152
}
153+
154+
public Query toQuery(QueryParseContext parseContext) throws IOException, QueryParsingException {
155+
if (this.ids.isEmpty()) {
156+
return Queries.newMatchNoDocsQuery();
157+
}
158+
159+
Collection<String> typesForQuery = this.types;
160+
if (typesForQuery == null || typesForQuery.isEmpty()) {
161+
typesForQuery = parseContext.queryTypes();
162+
} else if (typesForQuery.size() == 1 && Iterables.getFirst(typesForQuery, null).equals("_all")) {
163+
typesForQuery = parseContext.mapperService().types();
164+
}
165+
166+
TermsQuery query = new TermsQuery(UidFieldMapper.NAME, Uid.createTypeUids(typesForQuery, ids));
167+
query.setBoost(boost);
168+
if (queryName != null) {
169+
parseContext.addNamedQuery(queryName, query);
170+
}
171+
return query;
172+
}
173+
174+
@Override
175+
public QueryValidationException validate() {
176+
// all fields can be empty or null
177+
return null;
178+
}
179+
180+
@Override
181+
public void readFrom(StreamInput in) throws IOException {
182+
this.types = in.readStringList();
183+
this.ids = in.readStringList();
184+
queryName = in.readOptionalString();
185+
boost = in.readFloat();
186+
}
187+
188+
@Override
189+
public void writeTo(StreamOutput out) throws IOException {
190+
out.writeStringList(this.types);
191+
out.writeStringList(this.ids);
192+
out.writeOptionalString(queryName);
193+
out.writeFloat(boost);
194+
}
195+
196+
@Override
197+
public int hashCode() {
198+
return Objects.hash(ids, types, boost, queryName);
199+
}
200+
201+
@Override
202+
public boolean equals(Object obj) {
203+
if (this == obj) {
204+
return true;
205+
}
206+
if (obj == null || getClass() != obj.getClass()) {
207+
return false;
208+
}
209+
IdsQueryBuilder other = (IdsQueryBuilder) obj;
210+
return Objects.equals(ids, other.ids) &&
211+
Objects.equals(types, other.types) &&
212+
Objects.equals(boost, other.boost) &&
213+
Objects.equals(queryName, other.queryName);
214+
}
111215
}

‎src/main/java/org/elasticsearch/index/query/IdsQueryParser.java‎

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,18 @@
2020
package org.elasticsearch.index.query;
2121

2222
import com.google.common.collect.ImmutableList;
23-
import com.google.common.collect.Iterables;
2423

25-
26-
import org.apache.lucene.queries.TermsQuery;
27-
import org.apache.lucene.search.Query;
28-
import org.apache.lucene.util.BytesRef;
2924
import org.elasticsearch.common.inject.Inject;
30-
import org.elasticsearch.common.lucene.search.Queries;
3125
import org.elasticsearch.common.xcontent.XContentParser;
32-
import org.elasticsearch.index.mapper.Uid;
33-
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
3426

3527
import java.io.IOException;
3628
import java.util.ArrayList;
37-
import java.util.Collection;
3829
import java.util.List;
3930

4031
/**
41-
*
32+
* Parser for the IdsQuery.
4233
*/
43-
public class IdsQueryParser extends BaseQueryParserTemp {
34+
public class IdsQueryParser extends BaseQueryParser {
4435

4536
public static final String NAME = "ids";
4637

@@ -53,15 +44,18 @@ public String[] names() {
5344
return new String[]{NAME};
5445
}
5546

47+
/**
48+
* @return a QueryBuilder representation of the query passed in as XContent in the parse context
49+
*/
5650
@Override
57-
public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException {
51+
public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException {
5852
XContentParser parser = parseContext.parser();
59-
60-
List<BytesRef> ids = new ArrayList<>();
61-
Collection<String> types = null;
62-
String currentFieldName = null;
53+
List<String> ids = new ArrayList<>();
54+
List<String> types = new ArrayList<>();
6355
float boost = 1.0f;
6456
String queryName = null;
57+
58+
String currentFieldName = null;
6559
XContentParser.Token token;
6660
boolean idsProvided = false;
6761
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
@@ -73,18 +67,17 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
7367
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
7468
if ((token == XContentParser.Token.VALUE_STRING) ||
7569
(token == XContentParser.Token.VALUE_NUMBER)) {
76-
BytesRef value = parser.utf8BytesOrNull();
77-
if (value == null) {
70+
String id = parser.textOrNull();
71+
if (id == null) {
7872
throw new QueryParsingException(parseContext, "No value specified for term filter");
7973
}
80-
ids.add(value);
74+
ids.add(id);
8175
} else {
8276
throw new QueryParsingException(parseContext, "Illegal value for id, expecting a string or number, got: "
8377
+ token);
8478
}
8579
}
8680
} else if ("types".equals(currentFieldName) || "type".equals(currentFieldName)) {
87-
types = new ArrayList<>();
8881
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
8982
String value = parser.textOrNull();
9083
if (value == null) {
@@ -107,26 +100,14 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars
107100
}
108101
}
109102
}
110-
111103
if (!idsProvided) {
112104
throw new QueryParsingException(parseContext, "[ids] query, no ids values provided");
113105
}
114106

115-
if (ids.isEmpty()) {
116-
return Queries.newMatchNoDocsQuery();
117-
}
118-
119-
if (types == null || types.isEmpty()) {
120-
types = parseContext.queryTypes();
121-
} else if (types.size() == 1 && Iterables.getFirst(types, null).equals("_all")) {
122-
types = parseContext.mapperService().types();
123-
}
124-
125-
TermsQuery query = new TermsQuery(UidFieldMapper.NAME, Uid.createTypeUids(types, ids));
126-
query.setBoost(boost);
127-
if (queryName != null) {
128-
parseContext.addNamedQuery(queryName, query);
129-
}
107+
IdsQueryBuilder query = new IdsQueryBuilder(types.toArray(new String[types.size()]));
108+
query.addIds(ids.toArray(new String[ids.size()]));
109+
query.boost(boost).queryName(queryName);
110+
query.validate();
130111
return query;
131112
}
132113
}

‎src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ protected static QueryParseContext createContext() {
193193
return new QueryParseContext(index, queryParserService);
194194
}
195195

196-
private static void assertQueryHeader(XContentParser parser, String expectedParserName) throws IOException {
196+
protected static void assertQueryHeader(XContentParser parser, String expectedParserName) throws IOException {
197197
assertThat(parser.nextToken(), is(XContentParser.Token.START_OBJECT));
198198
assertThat(parser.nextToken(), is(XContentParser.Token.FIELD_NAME));
199199
assertThat(parser.currentName(), is(expectedParserName));

0 commit comments

Comments
 (0)