Query refactoring: better naming consistency for getters
In the query refactoring branch we've been introducing getter methods for every bit that you can set to each query. The naming is not every consistent at the moment. The applied naming convention are the following: - `innerQuery()` for any inner query, when there's only one of them - when there's more than one inner query, use a prefix that identifies which query it is, and the `query` suffix (e.g. `positiveQuery` or `littleQuery`) - `fieldName()` for the name of the field to be queried - `value()` for the actual query These changes don't break bw comp given that these getters were all introduced with the query refactoring which hasn't been released yet. Also we are modifying getters that don't have a corresponding setter, as the fields are final, hence we are not breaking consistency between getter and setter. Closes #12800
This commit is contained in:
parent
ecc5516d32
commit
a1be2d63f9
|
@ -30,6 +30,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Objects;
|
||||
|
||||
|
@ -50,9 +51,7 @@ public class AndQueryBuilder extends AbstractQueryBuilder<AndQueryBuilder> {
|
|||
* @param filters nested filters, no <tt>null</tt> values are allowed
|
||||
*/
|
||||
public AndQueryBuilder(QueryBuilder... filters) {
|
||||
for (QueryBuilder filter : filters) {
|
||||
this.filters.add(filter);
|
||||
}
|
||||
Collections.addAll(this.filters, filters);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -65,9 +64,9 @@ public class AndQueryBuilder extends AbstractQueryBuilder<AndQueryBuilder> {
|
|||
}
|
||||
|
||||
/**
|
||||
* @return the list of filters added to "and".
|
||||
* @return the list of queries added to "and".
|
||||
*/
|
||||
public List<QueryBuilder> filters() {
|
||||
public List<QueryBuilder> innerQueries() {
|
||||
return this.filters;
|
||||
}
|
||||
|
||||
|
|
|
@ -66,14 +66,14 @@ public class BoostingQueryBuilder extends AbstractQueryBuilder<BoostingQueryBuil
|
|||
/**
|
||||
* Get the positive query for this boosting query.
|
||||
*/
|
||||
public QueryBuilder positive() {
|
||||
public QueryBuilder positiveQuery() {
|
||||
return this.positiveQuery;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the negative query for this boosting query.
|
||||
*/
|
||||
public QueryBuilder negative() {
|
||||
public QueryBuilder negativeQuery() {
|
||||
return this.negativeQuery;
|
||||
}
|
||||
|
||||
|
|
|
@ -99,7 +99,7 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder<CommonTermsQue
|
|||
return this.fieldName;
|
||||
}
|
||||
|
||||
public Object text() {
|
||||
public Object value() {
|
||||
return this.text;
|
||||
}
|
||||
|
||||
|
|
|
@ -53,7 +53,7 @@ public class ConstantScoreQueryBuilder extends AbstractQueryBuilder<ConstantScor
|
|||
/**
|
||||
* @return the query that was wrapped in this constant score query
|
||||
*/
|
||||
public QueryBuilder query() {
|
||||
public QueryBuilder innerQuery() {
|
||||
return this.filterBuilder;
|
||||
}
|
||||
|
||||
|
|
|
@ -59,7 +59,7 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder<DisMaxQueryBuilder>
|
|||
/**
|
||||
* @return an immutable list copy of the current sub-queries of this disjunction
|
||||
*/
|
||||
public List<QueryBuilder> queries() {
|
||||
public List<QueryBuilder> innerQueries() {
|
||||
return this.queries;
|
||||
}
|
||||
|
||||
|
|
|
@ -39,32 +39,32 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder<ExistsQueryBuilder>
|
|||
|
||||
public static final String NAME = "exists";
|
||||
|
||||
private final String name;
|
||||
private final String fieldName;
|
||||
|
||||
static final ExistsQueryBuilder PROTOTYPE = new ExistsQueryBuilder(null);
|
||||
|
||||
public ExistsQueryBuilder(String name) {
|
||||
this.name = name;
|
||||
public ExistsQueryBuilder(String fieldName) {
|
||||
this.fieldName = fieldName;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the field name that has to exist for this query to match
|
||||
*/
|
||||
public String name() {
|
||||
return this.name;
|
||||
public String fieldName() {
|
||||
return this.fieldName;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
|
||||
builder.startObject(NAME);
|
||||
builder.field("field", name);
|
||||
builder.field("field", fieldName);
|
||||
printBoostAndQueryName(builder);
|
||||
builder.endObject();
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Query doToQuery(QueryShardContext context) throws IOException {
|
||||
return newFilter(context, name);
|
||||
return newFilter(context, fieldName);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -119,12 +119,12 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder<ExistsQueryBuilder>
|
|||
|
||||
@Override
|
||||
protected int doHashCode() {
|
||||
return Objects.hash(name);
|
||||
return Objects.hash(fieldName);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected boolean doEquals(ExistsQueryBuilder other) {
|
||||
return Objects.equals(name, other.name);
|
||||
return Objects.equals(fieldName, other.fieldName);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -134,7 +134,7 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder<ExistsQueryBuilder>
|
|||
|
||||
@Override
|
||||
protected void doWriteTo(StreamOutput out) throws IOException {
|
||||
out.writeString(name);
|
||||
out.writeString(fieldName);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -74,12 +74,12 @@ public class FilteredQueryBuilder extends AbstractQueryBuilder<FilteredQueryBuil
|
|||
}
|
||||
|
||||
/** Returns the query to apply the filter to. */
|
||||
public QueryBuilder query() {
|
||||
public QueryBuilder innerQuery() {
|
||||
return queryBuilder;
|
||||
}
|
||||
|
||||
/** Returns the filter to apply to the query results. */
|
||||
public QueryBuilder filter() {
|
||||
public QueryBuilder innerFilter() {
|
||||
return filterBuilder;
|
||||
}
|
||||
|
||||
|
|
|
@ -44,9 +44,9 @@ public class NotQueryBuilder extends AbstractQueryBuilder<NotQueryBuilder> {
|
|||
}
|
||||
|
||||
/**
|
||||
* @return the filter added to "not".
|
||||
* @return the query added to "not".
|
||||
*/
|
||||
public QueryBuilder filter() {
|
||||
public QueryBuilder innerQuery() {
|
||||
return this.filter;
|
||||
}
|
||||
|
||||
|
|
|
@ -30,6 +30,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Objects;
|
||||
|
||||
|
@ -47,9 +48,7 @@ public class OrQueryBuilder extends AbstractQueryBuilder<OrQueryBuilder> {
|
|||
static final OrQueryBuilder PROTOTYPE = new OrQueryBuilder();
|
||||
|
||||
public OrQueryBuilder(QueryBuilder... filters) {
|
||||
for (QueryBuilder filter : filters) {
|
||||
this.filters.add(filter);
|
||||
}
|
||||
Collections.addAll(this.filters, filters);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -62,9 +61,9 @@ public class OrQueryBuilder extends AbstractQueryBuilder<OrQueryBuilder> {
|
|||
}
|
||||
|
||||
/**
|
||||
* @return the list of filters added to "or".
|
||||
* @return the list of queries added to "or".
|
||||
*/
|
||||
public List<QueryBuilder> filters() {
|
||||
public List<QueryBuilder> innerQueries() {
|
||||
return this.filters;
|
||||
}
|
||||
|
||||
|
|
|
@ -94,7 +94,7 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
|
|||
}
|
||||
|
||||
/** Returns the text to parse the query from. */
|
||||
public String text() {
|
||||
public String value() {
|
||||
return this.queryText;
|
||||
}
|
||||
|
||||
|
|
|
@ -51,14 +51,14 @@ public class SpanContainingQueryBuilder extends AbstractQueryBuilder<SpanContain
|
|||
/**
|
||||
* @return the big clause, it must enclose {@code little} for a match.
|
||||
*/
|
||||
public SpanQueryBuilder big() {
|
||||
public SpanQueryBuilder bigQuery() {
|
||||
return this.big;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the little clause, it must be contained within {@code big} for a match.
|
||||
*/
|
||||
public SpanQueryBuilder little() {
|
||||
public SpanQueryBuilder littleQuery() {
|
||||
return this.little;
|
||||
}
|
||||
|
||||
|
|
|
@ -54,7 +54,7 @@ public class SpanFirstQueryBuilder extends AbstractQueryBuilder<SpanFirstQueryBu
|
|||
/**
|
||||
* @return the inner {@link SpanQueryBuilder} defined in this query
|
||||
*/
|
||||
public SpanQueryBuilder matchBuilder() {
|
||||
public SpanQueryBuilder innerQuery() {
|
||||
return this.matchBuilder;
|
||||
}
|
||||
|
||||
|
|
|
@ -42,7 +42,7 @@ public class SpanMultiTermQueryBuilder extends AbstractQueryBuilder<SpanMultiTer
|
|||
this.multiTermQueryBuilder = multiTermQueryBuilder;
|
||||
}
|
||||
|
||||
public MultiTermQueryBuilder multiTermQueryBuilder() {
|
||||
public MultiTermQueryBuilder innerQuery() {
|
||||
return this.multiTermQueryBuilder;
|
||||
}
|
||||
|
||||
|
|
|
@ -62,14 +62,14 @@ public class SpanNotQueryBuilder extends AbstractQueryBuilder<SpanNotQueryBuilde
|
|||
/**
|
||||
* @return the span query whose matches are filtered
|
||||
*/
|
||||
public SpanQueryBuilder include() {
|
||||
public SpanQueryBuilder includeQuery() {
|
||||
return this.include;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the span query whose matches must not overlap
|
||||
*/
|
||||
public SpanQueryBuilder exclude() {
|
||||
public SpanQueryBuilder excludeQuery() {
|
||||
return this.exclude;
|
||||
}
|
||||
|
||||
|
@ -137,8 +137,7 @@ public class SpanNotQueryBuilder extends AbstractQueryBuilder<SpanNotQueryBuilde
|
|||
Query excludeQuery = this.exclude.toQuery(context);
|
||||
assert excludeQuery instanceof SpanQuery;
|
||||
|
||||
SpanNotQuery query = new SpanNotQuery((SpanQuery) includeQuery, (SpanQuery) excludeQuery, pre, post);
|
||||
return query;
|
||||
return new SpanNotQuery((SpanQuery) includeQuery, (SpanQuery) excludeQuery, pre, post);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -52,14 +52,14 @@ public class SpanWithinQueryBuilder extends AbstractQueryBuilder<SpanWithinQuery
|
|||
/**
|
||||
* @return the little clause, contained within {@code big} for a match.
|
||||
*/
|
||||
public SpanQueryBuilder little() {
|
||||
public SpanQueryBuilder littleQuery() {
|
||||
return this.little;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the big clause that must enclose {@code little} for a match.
|
||||
*/
|
||||
public SpanQueryBuilder big() {
|
||||
public SpanQueryBuilder bigQuery() {
|
||||
return this.big;
|
||||
}
|
||||
|
||||
|
|
|
@ -49,8 +49,8 @@ public class TypeQueryBuilder extends AbstractQueryBuilder<TypeQueryBuilder> {
|
|||
this.type = type;
|
||||
}
|
||||
|
||||
public BytesRef type() {
|
||||
return this.type;
|
||||
public String type() {
|
||||
return BytesRefs.toString(this.type);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -53,11 +53,11 @@ public class AndQueryBuilderTest extends BaseQueryTestCase<AndQueryBuilder> {
|
|||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(AndQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
if (queryBuilder.filters().isEmpty()) {
|
||||
if (queryBuilder.innerQueries().isEmpty()) {
|
||||
assertThat(query, nullValue());
|
||||
} else {
|
||||
List<Query> clauses = new ArrayList<>();
|
||||
for (QueryBuilder innerFilter : queryBuilder.filters()) {
|
||||
for (QueryBuilder innerFilter : queryBuilder.innerQueries()) {
|
||||
Query clause = innerFilter.toQuery(context);
|
||||
if (clause != null) {
|
||||
clauses.add(clause);
|
||||
|
|
|
@ -39,8 +39,8 @@ public class BoostingQueryBuilderTest extends BaseQueryTestCase<BoostingQueryBui
|
|||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(BoostingQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
Query positive = queryBuilder.positive().toQuery(context);
|
||||
Query negative = queryBuilder.negative().toQuery(context);
|
||||
Query positive = queryBuilder.positiveQuery().toQuery(context);
|
||||
Query negative = queryBuilder.negativeQuery().toQuery(context);
|
||||
if (positive == null || negative == null) {
|
||||
assertThat(query, nullValue());
|
||||
} else {
|
||||
|
|
|
@ -43,7 +43,7 @@ public class ConstantScoreQueryBuilderTest extends BaseQueryTestCase<ConstantSco
|
|||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(ConstantScoreQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
Query innerQuery = queryBuilder.query().toQuery(context);
|
||||
Query innerQuery = queryBuilder.innerQuery().toQuery(context);
|
||||
if (innerQuery == null) {
|
||||
assertThat(query, nullValue());
|
||||
} else {
|
||||
|
|
|
@ -53,7 +53,7 @@ public class DisMaxQueryBuilderTest extends BaseQueryTestCase<DisMaxQueryBuilder
|
|||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(DisMaxQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
Collection<Query> queries = AbstractQueryBuilder.toQueries(queryBuilder.queries(), context);
|
||||
Collection<Query> queries = AbstractQueryBuilder.toQueries(queryBuilder.innerQueries(), context);
|
||||
if (queries.isEmpty()) {
|
||||
assertThat(query, nullValue());
|
||||
} else {
|
||||
|
|
|
@ -55,7 +55,7 @@ public class ExistsQueryBuilderTest extends BaseQueryTestCase<ExistsQueryBuilder
|
|||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(ExistsQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
String fieldPattern = queryBuilder.name();
|
||||
String fieldPattern = queryBuilder.fieldName();
|
||||
ObjectMapper objectMapper = context.getObjectMapper(fieldPattern);
|
||||
if (objectMapper != null) {
|
||||
// automatic make the object mapper pattern
|
||||
|
|
|
@ -44,11 +44,11 @@ public class FilteredQueryBuilderTest extends BaseQueryTestCase<FilteredQueryBui
|
|||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(FilteredQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
Query innerQuery = queryBuilder.query().toQuery(context);
|
||||
Query innerQuery = queryBuilder.innerQuery().toQuery(context);
|
||||
if (innerQuery == null) {
|
||||
assertThat(query, nullValue());
|
||||
} else {
|
||||
Query innerFilter = queryBuilder.filter().toQuery(context);
|
||||
Query innerFilter = queryBuilder.innerFilter().toQuery(context);
|
||||
if (innerFilter == null || Queries.isConstantMatchAllQuery(innerFilter)) {
|
||||
innerQuery.setBoost(queryBuilder.boost());
|
||||
assertThat(query, equalTo(innerQuery));
|
||||
|
|
|
@ -45,7 +45,7 @@ public class NotQueryBuilderTest extends BaseQueryTestCase<NotQueryBuilder> {
|
|||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(NotQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
Query filter = queryBuilder.filter().toQuery(context);
|
||||
Query filter = queryBuilder.innerQuery().toQuery(context);
|
||||
if (filter == null) {
|
||||
assertThat(query, nullValue());
|
||||
} else {
|
||||
|
|
|
@ -74,11 +74,11 @@ public class OrQueryBuilderTest extends BaseQueryTestCase<OrQueryBuilder> {
|
|||
|
||||
@Override
|
||||
protected void doAssertLuceneQuery(OrQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
if (queryBuilder.filters().isEmpty()) {
|
||||
if (queryBuilder.innerQueries().isEmpty()) {
|
||||
assertThat(query, nullValue());
|
||||
} else {
|
||||
List<Query> innerQueries = new ArrayList<>();
|
||||
for (QueryBuilder subQuery : queryBuilder.filters()) {
|
||||
for (QueryBuilder subQuery : queryBuilder.innerQueries()) {
|
||||
Query innerQuery = subQuery.toQuery(context);
|
||||
// ignore queries that are null
|
||||
if (innerQuery != null) {
|
||||
|
|
|
@ -98,7 +98,7 @@ public class SimpleQueryStringBuilderTest extends BaseQueryTestCase<SimpleQueryS
|
|||
for (BooleanClause booleanClause : booleanQuery) {
|
||||
assertThat(booleanClause.getQuery(), instanceOf(TermQuery.class));
|
||||
TermQuery termQuery = (TermQuery) booleanClause.getQuery();
|
||||
assertThat(termQuery.getTerm(), equalTo(new Term(fields.next(), queryBuilder.text().toLowerCase(Locale.ROOT))));
|
||||
assertThat(termQuery.getTerm(), equalTo(new Term(fields.next(), queryBuilder.value().toLowerCase(Locale.ROOT))));
|
||||
}
|
||||
} else {
|
||||
assertThat(query, instanceOf(TermQuery.class));
|
||||
|
@ -109,7 +109,7 @@ public class SimpleQueryStringBuilderTest extends BaseQueryTestCase<SimpleQueryS
|
|||
field = queryBuilder.fields().keySet().iterator().next();
|
||||
}
|
||||
TermQuery termQuery = (TermQuery) query;
|
||||
assertThat(termQuery.getTerm(), equalTo(new Term(field, queryBuilder.text().toLowerCase(Locale.ROOT))));
|
||||
assertThat(termQuery.getTerm(), equalTo(new Term(field, queryBuilder.value().toLowerCase(Locale.ROOT))));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -251,7 +251,7 @@ public class SimpleQueryStringBuilderTest extends BaseQueryTestCase<SimpleQueryS
|
|||
XContentParser parser = XContentFactory.xContent(contentString).createParser(contentString);
|
||||
context.reset(parser);
|
||||
SimpleQueryStringBuilder queryBuilder = new SimpleQueryStringParser().fromXContent(context);
|
||||
assertThat(queryBuilder.text(), equalTo(query));
|
||||
assertThat(queryBuilder.value(), equalTo(query));
|
||||
assertThat(queryBuilder.fields(), notNullValue());
|
||||
assertThat(queryBuilder.fields().size(), equalTo(0));
|
||||
Query luceneQuery = queryBuilder.toQuery(createShardContext());
|
||||
|
|
|
@ -41,7 +41,7 @@ public class SpanMultiTermQueryBuilderTest extends BaseQueryTestCase<SpanMultiTe
|
|||
protected void doAssertLuceneQuery(SpanMultiTermQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
assertThat(query, instanceOf(SpanMultiTermQueryWrapper.class));
|
||||
SpanMultiTermQueryWrapper spanMultiTermQueryWrapper = (SpanMultiTermQueryWrapper) query;
|
||||
Query multiTermQuery = queryBuilder.multiTermQueryBuilder().toQuery(context);
|
||||
Query multiTermQuery = queryBuilder.innerQuery().toQuery(context);
|
||||
assertThat(multiTermQuery, instanceOf(MultiTermQuery.class));
|
||||
assertThat(spanMultiTermQueryWrapper.getWrappedQuery(), equalTo(new SpanMultiTermQueryWrapper<>((MultiTermQuery)multiTermQuery).getWrappedQuery()));
|
||||
}
|
||||
|
|
|
@ -58,8 +58,8 @@ public class SpanNotQueryBuilderTest extends BaseQueryTestCase<SpanNotQueryBuild
|
|||
protected void doAssertLuceneQuery(SpanNotQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
|
||||
assertThat(query, instanceOf(SpanNotQuery.class));
|
||||
SpanNotQuery spanNotQuery = (SpanNotQuery) query;
|
||||
assertThat(spanNotQuery.getExclude(), equalTo(queryBuilder.exclude().toQuery(context)));
|
||||
assertThat(spanNotQuery.getInclude(), equalTo(queryBuilder.include().toQuery(context)));
|
||||
assertThat(spanNotQuery.getExclude(), equalTo(queryBuilder.excludeQuery().toQuery(context)));
|
||||
assertThat(spanNotQuery.getInclude(), equalTo(queryBuilder.includeQuery().toQuery(context)));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -141,8 +141,8 @@ public class SpanNotQueryBuilderTest extends BaseQueryTestCase<SpanNotQueryBuild
|
|||
SpanNotQueryBuilder query = (SpanNotQueryBuilder) new SpanNotQueryParser().fromXContent(context);
|
||||
assertThat(query.pre(), equalTo(3));
|
||||
assertThat(query.post(), equalTo(3));
|
||||
assertNotNull(query.include());
|
||||
assertNotNull(query.exclude());
|
||||
assertNotNull(query.includeQuery());
|
||||
assertNotNull(query.excludeQuery());
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -45,7 +45,7 @@ public class TypeQueryBuilderTest extends BaseQueryTestCase<TypeQueryBuilder> {
|
|||
}
|
||||
TermQuery termQuery = (TermQuery) query;
|
||||
assertThat(termQuery.getTerm().field(), equalTo(TypeFieldMapper.NAME));
|
||||
assertThat(termQuery.getTerm().bytes(), equalTo(queryBuilder.type()));
|
||||
assertThat(termQuery.getTerm().text(), equalTo(queryBuilder.type()));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
Loading…
Reference in New Issue