Improve adding clauses to span_near and span_or query

Currently the query builders expose the clauses of the span
query as a modifiable list. Instead we should make the that
getter return an unmodifiable list. Also renaming the method
used to add a clause from `clause(spanQuery)` to
`addClause(spanQuery)`.
This commit is contained in:
Christoph Büscher 2016-05-20 13:36:55 +02:00
parent 34ef5306d2
commit d3fe22c990
9 changed files with 66 additions and 55 deletions

View File

@ -31,6 +31,7 @@ import org.elasticsearch.common.xcontent.XContentParser;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
@ -64,7 +65,7 @@ public class SpanNearQueryBuilder extends AbstractQueryBuilder<SpanNearQueryBuil
*/
public SpanNearQueryBuilder(SpanQueryBuilder initialClause, int slop) {
if (initialClause == null) {
throw new IllegalArgumentException("query must include at least one clause");
throw new IllegalArgumentException("[" + NAME + "] must include at least one clause");
}
this.clauses.add(initialClause);
this.slop = slop;
@ -96,9 +97,12 @@ public class SpanNearQueryBuilder extends AbstractQueryBuilder<SpanNearQueryBuil
return this.slop;
}
public SpanNearQueryBuilder clause(SpanQueryBuilder clause) {
/**
* Add a span clause to the current list of clauses
*/
public SpanNearQueryBuilder addClause(SpanQueryBuilder clause) {
if (clause == null) {
throw new IllegalArgumentException("query clauses cannot be null");
throw new IllegalArgumentException("[" + NAME + "] clauses cannot be null");
}
clauses.add(clause);
return this;
@ -108,7 +112,7 @@ public class SpanNearQueryBuilder extends AbstractQueryBuilder<SpanNearQueryBuil
* @return the {@link SpanQueryBuilder} clauses that were set for this query
*/
public List<SpanQueryBuilder> clauses() {
return this.clauses;
return Collections.unmodifiableList(this.clauses);
}
/**
@ -198,7 +202,7 @@ public class SpanNearQueryBuilder extends AbstractQueryBuilder<SpanNearQueryBuil
SpanNearQueryBuilder queryBuilder = new SpanNearQueryBuilder(clauses.get(0), slop);
for (int i = 1; i < clauses.size(); i++) {
queryBuilder.clause(clauses.get(i));
queryBuilder.addClause(clauses.get(i));
}
queryBuilder.inOrder(inOrder);
queryBuilder.boost(boost);

View File

@ -31,6 +31,7 @@ import org.elasticsearch.common.xcontent.XContentParser;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
@ -48,7 +49,7 @@ public class SpanOrQueryBuilder extends AbstractQueryBuilder<SpanOrQueryBuilder>
public SpanOrQueryBuilder(SpanQueryBuilder initialClause) {
if (initialClause == null) {
throw new IllegalArgumentException("query must include at least one clause");
throw new IllegalArgumentException("[" + NAME + "] must include at least one clause");
}
clauses.add(initialClause);
}
@ -68,9 +69,12 @@ public class SpanOrQueryBuilder extends AbstractQueryBuilder<SpanOrQueryBuilder>
writeQueries(out, clauses);
}
public SpanOrQueryBuilder clause(SpanQueryBuilder clause) {
/**
* Add a span clause to the current list of clauses
*/
public SpanOrQueryBuilder addClause(SpanQueryBuilder clause) {
if (clause == null) {
throw new IllegalArgumentException("inner bool query clause cannot be null");
throw new IllegalArgumentException("[" + NAME + "] inner clause cannot be null");
}
clauses.add(clause);
return this;
@ -80,7 +84,7 @@ public class SpanOrQueryBuilder extends AbstractQueryBuilder<SpanOrQueryBuilder>
* @return the {@link SpanQueryBuilder} clauses that were set for this query
*/
public List<SpanQueryBuilder> clauses() {
return this.clauses;
return Collections.unmodifiableList(this.clauses);
}
@Override
@ -137,7 +141,7 @@ public class SpanOrQueryBuilder extends AbstractQueryBuilder<SpanOrQueryBuilder>
SpanOrQueryBuilder queryBuilder = new SpanOrQueryBuilder(clauses.get(0));
for (int i = 1; i < clauses.size(); i++) {
queryBuilder.clause(clauses.get(i));
queryBuilder.addClause(clauses.get(i));
}
queryBuilder.boost(boost);
queryBuilder.queryName(queryName);

View File

@ -314,7 +314,7 @@ public class QueryDSLDocumentationTests extends ESTestCase {
public void testSpanContaining() {
spanContainingQuery(
spanNearQuery(spanTermQuery("field1","bar"), 5)
.clause(spanTermQuery("field1","baz"))
.addClause(spanTermQuery("field1","baz"))
.inOrder(true),
spanTermQuery("field1","foo"));
}
@ -332,8 +332,8 @@ public class QueryDSLDocumentationTests extends ESTestCase {
public void testSpanNear() {
spanNearQuery(spanTermQuery("field","value1"), 12)
.clause(spanTermQuery("field","value2"))
.clause(spanTermQuery("field","value3"))
.addClause(spanTermQuery("field","value2"))
.addClause(spanTermQuery("field","value3"))
.inOrder(false);
}
@ -344,8 +344,8 @@ public class QueryDSLDocumentationTests extends ESTestCase {
public void testSpanOr() {
spanOrQuery(spanTermQuery("field","value1"))
.clause(spanTermQuery("field","value2"))
.clause(spanTermQuery("field","value3"));
.addClause(spanTermQuery("field","value2"))
.addClause(spanTermQuery("field","value3"));
}
public void testSpanTerm() {
@ -355,7 +355,7 @@ public class QueryDSLDocumentationTests extends ESTestCase {
public void testSpanWithin() {
spanWithinQuery(
spanNearQuery(spanTermQuery("field1", "bar"), 5)
.clause(spanTermQuery("field1", "baz"))
.addClause(spanTermQuery("field1", "baz"))
.inOrder(true),
spanTermQuery("field1", "foo"));
}

View File

@ -37,7 +37,7 @@ public class SpanNearQueryBuilderTests extends AbstractQueryTestCase<SpanNearQue
SpanTermQueryBuilder[] spanTermQueries = new SpanTermQueryBuilderTests().createSpanTermQueryBuilders(randomIntBetween(1, 6));
SpanNearQueryBuilder queryBuilder = new SpanNearQueryBuilder(spanTermQueries[0], randomIntBetween(-10, 10));
for (int i = 1; i < spanTermQueries.length; i++) {
queryBuilder.clause(spanTermQueries[i]);
queryBuilder.addClause(spanTermQueries[i]);
}
queryBuilder.inOrder(randomBoolean());
return queryBuilder;
@ -57,20 +57,18 @@ public class SpanNearQueryBuilderTests extends AbstractQueryTestCase<SpanNearQue
}
public void testIllegalArguments() {
try {
new SpanNearQueryBuilder(null, 1);
fail("cannot be null");
} catch (IllegalArgumentException e) {
// ecpected
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new SpanNearQueryBuilder(null, 1));
assertEquals("[span_near] must include at least one clause", e.getMessage());
SpanNearQueryBuilder spanNearQueryBuilder = new SpanNearQueryBuilder(new SpanTermQueryBuilder("field", "value"), 1);
e = expectThrows(IllegalArgumentException.class, () -> spanNearQueryBuilder.addClause(null));
assertEquals("[span_near] clauses cannot be null", e.getMessage());
}
try {
public void testClausesUnmodifiable() {
SpanNearQueryBuilder spanNearQueryBuilder = new SpanNearQueryBuilder(new SpanTermQueryBuilder("field", "value"), 1);
spanNearQueryBuilder.clause(null);
fail("cannot be null");
} catch (IllegalArgumentException e) {
// ecpected
}
expectThrows(UnsupportedOperationException.class,
() -> spanNearQueryBuilder.clauses().add(new SpanTermQueryBuilder("field", "value2")));
}
public void testFromJson() throws IOException {

View File

@ -110,7 +110,7 @@ public class SpanNotQueryBuilderTests extends AbstractQueryTestCase<SpanNotQuery
spanTermQuery("description", "jumped").toXContent(builder, null);
builder.field("include");
spanNearQuery(QueryBuilders.spanTermQuery("description", "quick"), 1)
.clause(QueryBuilders.spanTermQuery("description", "fox")).toXContent(builder, null);
.addClause(QueryBuilders.spanTermQuery("description", "fox")).toXContent(builder, null);
builder.field("dist", 3);
builder.endObject();
builder.endObject();
@ -148,7 +148,7 @@ public class SpanNotQueryBuilderTests extends AbstractQueryTestCase<SpanNotQuery
builder.startObject(SpanNotQueryBuilder.NAME);
builder.field("include");
spanNearQuery(QueryBuilders.spanTermQuery("description", "quick"), 1)
.clause(QueryBuilders.spanTermQuery("description", "fox")).toXContent(builder, null);
.addClause(QueryBuilders.spanTermQuery("description", "fox")).toXContent(builder, null);
builder.field("dist", 2);
builder.endObject();
builder.endObject();
@ -166,7 +166,7 @@ public class SpanNotQueryBuilderTests extends AbstractQueryTestCase<SpanNotQuery
builder.startObject(SpanNotQueryBuilder.NAME);
builder.field("include");
spanNearQuery(QueryBuilders.spanTermQuery("description", "quick"), 1)
.clause(QueryBuilders.spanTermQuery("description", "fox")).toXContent(builder, null);
.addClause(QueryBuilders.spanTermQuery("description", "fox")).toXContent(builder, null);
builder.field("exclude");
spanTermQuery("description", "jumped").toXContent(builder, null);
builder.field("dist", 2);

View File

@ -35,7 +35,7 @@ public class SpanOrQueryBuilderTests extends AbstractQueryTestCase<SpanOrQueryBu
SpanTermQueryBuilder[] spanTermQueries = new SpanTermQueryBuilderTests().createSpanTermQueryBuilders(randomIntBetween(1, 6));
SpanOrQueryBuilder queryBuilder = new SpanOrQueryBuilder(spanTermQueries[0]);
for (int i = 1; i < spanTermQueries.length; i++) {
queryBuilder.clause(spanTermQueries[i]);
queryBuilder.addClause(spanTermQueries[i]);
}
return queryBuilder;
}
@ -52,15 +52,18 @@ public class SpanOrQueryBuilderTests extends AbstractQueryTestCase<SpanOrQueryBu
}
public void testIllegalArguments() {
expectThrows(IllegalArgumentException.class, () -> new SpanOrQueryBuilder((SpanQueryBuilder) null));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new SpanOrQueryBuilder((SpanQueryBuilder) null));
assertEquals("[span_or] must include at least one clause", e.getMessage());
try {
SpanOrQueryBuilder spanOrBuilder = new SpanOrQueryBuilder(new SpanTermQueryBuilder("field", "value"));
spanOrBuilder.clause(null);
fail("cannot be null");
} catch (IllegalArgumentException e) {
// expected
e = expectThrows(IllegalArgumentException.class, () -> spanOrBuilder.addClause(null));
assertEquals("[span_or] inner clause cannot be null", e.getMessage());
}
public void testClausesUnmodifiable() {
SpanNearQueryBuilder spanNearQueryBuilder = new SpanNearQueryBuilder(new SpanTermQueryBuilder("field", "value"), 1);
expectThrows(UnsupportedOperationException.class,
() -> spanNearQueryBuilder.clauses().add(new SpanTermQueryBuilder("field", "value2")));
}
public void testFromJson() throws IOException {

View File

@ -161,8 +161,8 @@ public class PercolatorQuerySearchIT extends ESSingleNodeTestCase {
client().prepareIndex("test", "queries", "3")
.setSource(jsonBuilder().startObject().field("query",
spanNearQuery(spanTermQuery("field1", "quick"), 0)
.clause(spanTermQuery("field1", "brown"))
.clause(spanTermQuery("field1", "fox"))
.addClause(spanTermQuery("field1", "brown"))
.addClause(spanTermQuery("field1", "fox"))
.inOrder(true)
).endObject())
.get();
@ -172,12 +172,12 @@ public class PercolatorQuerySearchIT extends ESSingleNodeTestCase {
.setSource(jsonBuilder().startObject().field("query",
spanNotQuery(
spanNearQuery(spanTermQuery("field1", "quick"), 0)
.clause(spanTermQuery("field1", "brown"))
.clause(spanTermQuery("field1", "fox"))
.addClause(spanTermQuery("field1", "brown"))
.addClause(spanTermQuery("field1", "fox"))
.inOrder(true),
spanNearQuery(spanTermQuery("field1", "the"), 0)
.clause(spanTermQuery("field1", "lazy"))
.clause(spanTermQuery("field1", "dog"))
.addClause(spanTermQuery("field1", "lazy"))
.addClause(spanTermQuery("field1", "dog"))
.inOrder(true)).dist(2)
).endObject())
.get();
@ -187,12 +187,12 @@ public class PercolatorQuerySearchIT extends ESSingleNodeTestCase {
.setSource(jsonBuilder().startObject().field("query",
spanNotQuery(
spanNearQuery(spanTermQuery("field1", "quick"), 0)
.clause(spanTermQuery("field1", "brown"))
.clause(spanTermQuery("field1", "fox"))
.addClause(spanTermQuery("field1", "brown"))
.addClause(spanTermQuery("field1", "fox"))
.inOrder(true),
spanNearQuery(spanTermQuery("field1", "the"), 0)
.clause(spanTermQuery("field1", "lazy"))
.clause(spanTermQuery("field1", "dog"))
.addClause(spanTermQuery("field1", "lazy"))
.addClause(spanTermQuery("field1", "dog"))
.inOrder(true)).dist(3)
).endObject())
.get();

View File

@ -1436,7 +1436,7 @@ public class SearchQueryIT extends ESIntegTestCase {
searchResponse = client().prepareSearch("test").setQuery(
spanNearQuery(spanTermQuery("description", "foo"), 3)
.clause(spanTermQuery("description", "other"))).get();
.addClause(spanTermQuery("description", "other"))).get();
assertHitCount(searchResponse, 3L);
}
@ -1481,17 +1481,17 @@ public class SearchQueryIT extends ESIntegTestCase {
SearchResponse searchResponse = client().prepareSearch("test")
.setQuery(spanNotQuery(spanNearQuery(QueryBuilders.spanTermQuery("description", "quick"), 1)
.clause(QueryBuilders.spanTermQuery("description", "fox")), spanTermQuery("description", "brown"))).get();
.addClause(QueryBuilders.spanTermQuery("description", "fox")), spanTermQuery("description", "brown"))).get();
assertHitCount(searchResponse, 1L);
searchResponse = client().prepareSearch("test")
.setQuery(spanNotQuery(spanNearQuery(QueryBuilders.spanTermQuery("description", "quick"), 1)
.clause(QueryBuilders.spanTermQuery("description", "fox")), spanTermQuery("description", "sleeping")).dist(5)).get();
.addClause(QueryBuilders.spanTermQuery("description", "fox")), spanTermQuery("description", "sleeping")).dist(5)).get();
assertHitCount(searchResponse, 1L);
searchResponse = client().prepareSearch("test")
.setQuery(spanNotQuery(spanNearQuery(QueryBuilders.spanTermQuery("description", "quick"), 1)
.clause(QueryBuilders.spanTermQuery("description", "fox")), spanTermQuery("description", "jumped")).pre(1).post(1)).get();
.addClause(QueryBuilders.spanTermQuery("description", "fox")), spanTermQuery("description", "jumped")).pre(1).post(1)).get();
assertHitCount(searchResponse, 1L);
}

View File

@ -84,12 +84,14 @@ static factory methods in QueryBuilders accordingly.
Making sure that query contains at least one clause by making initial clause mandatory
in constructor.
Renaming method to add clauses from `clause(SpanQueryBuilder)` to `addClause(SpanQueryBuilder)`.
===== SpanNearQueryBuilder
Removed setter for mandatory slop parameter, needs to be set in constructor now. Also
making sure that query contains at least one clause by making initial clause mandatory
in constructor. Updated the static factory methods in QueryBuilders accordingly.
Renaming method to add clauses from `clause(SpanQueryBuilder)` to `addClause(SpanQueryBuilder)`.
===== SpanNotQueryBuilder