honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-17 02:14:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Don't require types parameter in IdsQueryBuilder constructor

Browse Source 
According to the docs and our own tests we accept an ids query without specified
types and default to all types in the index mapping in this case. This changes
the builder to reflect this by making the types no longer a required constructor
argument and changes the parser to reflect that.
This commit is contained in:
Christoph Büscher 2016-11-16 11:08:47 +01:00
parent b8cae39b7c
commit 4a7b70cc08
4 changed files with 54 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java
View File

@ -304,7 +304,10 @@ public abstract class AbstractQueryBuilder<QB extends AbstractQueryBuilder<QB>>
}
/**
* Adds 'boost' and 'query_name' parsing to all query builder parsers passed in
* Adds {@code boost} and {@code query_name} parsing to the
* {@link AbstractObjectParser} passed in. All query builders except
* {@link MatchAllQueryBuilder} and {@link MatchNoneQueryBuilder} support these fields so they
* should use this method.
*/
protected static void declareStandardFields(AbstractObjectParser<? extends QueryBuilder, ? extends ParseFieldMatcherSupplier> parser) {
parser.declareFloat((builder, value) -> builder.boost(value), AbstractQueryBuilder.BOOST_FIELD);

45
core/src/main/java/org/elasticsearch/index/query/IdsQueryBuilder.java
View File

@ -24,10 +24,11 @@ import org.apache.lucene.search.Query;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.Uid;
import org.elasticsearch.index.mapper.UidFieldMapper;
@ -37,12 +38,11 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.common.xcontent.ObjectParser.fromList;
/**
* A query that will return only documents matching specific ids (and a type).
@ -55,16 +55,22 @@ public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
private final Set<String> ids = new HashSet<>();
private final String[] types;
private String[] types = Strings.EMPTY_ARRAY;
/**
* Creates a new IdsQueryBuilder with no types specified upfront
*/
public IdsQueryBuilder() {
// nothing to do
}
/**
* Creates a new IdsQueryBuilder by providing the types of the documents to look for
* @deprecated Replaced by {@link #types(String...)}
*/
@Deprecated
public IdsQueryBuilder(String... types) {
if (types == null) {
throw new IllegalArgumentException("[ids] types cannot be null");
}
this.types = types;
types(types);
}
/**
@ -82,6 +88,17 @@ public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
out.writeStringArray(ids.toArray(new String[ids.size()]));
}
/**
* Add types to query
*/
public IdsQueryBuilder types(String... types) {
if (types == null) {
throw new IllegalArgumentException("[" + NAME + "] types cannot be null");
}
this.types = types;
return this;
}
/**
* Returns the types used in this query
*/
@ -94,7 +111,7 @@ public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
*/
public IdsQueryBuilder addIds(String... ids) {
if (ids == null) {
throw new IllegalArgumentException("[ids] ids cannot be null");
throw new IllegalArgumentException("[" + NAME + "] ids cannot be null");
}
Collections.addAll(this.ids, ids);
return this;
@ -120,14 +137,12 @@ public class IdsQueryBuilder extends AbstractQueryBuilder<IdsQueryBuilder> {
builder.endObject();
}
@SuppressWarnings("unchecked")
private static ConstructingObjectParser<IdsQueryBuilder, QueryParseContext> PARSER = new ConstructingObjectParser<>(NAME,
a -> new IdsQueryBuilder(((List<String>) a[0]).toArray(new String[0])));
private static ObjectParser<IdsQueryBuilder, QueryParseContext> PARSER = new ObjectParser<>(NAME,
() -> new IdsQueryBuilder());
static {
PARSER.declareStringArray(constructorArg(), IdsQueryBuilder.TYPE_FIELD);
PARSER.declareStringArray((builder, values) -> builder.addIds(values.toArray(new String[values.size()])),
IdsQueryBuilder.VALUES_FIELD);
PARSER.declareStringArray(fromList(String.class, IdsQueryBuilder::types), IdsQueryBuilder.TYPE_FIELD);
PARSER.declareStringArray(fromList(String.class, IdsQueryBuilder::addIds), IdsQueryBuilder.VALUES_FIELD);
declareStandardFields(PARSER);
}

4
core/src/main/java/org/elasticsearch/index/query/QueryBuilders.java
View File

@ -29,12 +29,10 @@ import org.elasticsearch.index.query.functionscore.FunctionScoreQueryBuilder;
import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilder;
import org.elasticsearch.indices.TermsLookup;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptService;
import java.io.IOException;
import java.util.Collection;
import java.util.List;
import java.util.Map;
/**
* A static factory for simple "import static" usage.
@ -120,7 +118,7 @@ public abstract class QueryBuilders {
* @param types The mapping/doc type
*/
public static IdsQueryBuilder idsQuery(String... types) {
return new IdsQueryBuilder(types);
return new IdsQueryBuilder().types(types);
}
/**

26
core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java
View File

@ -63,7 +63,7 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase<IdsQueryBuilder>
}
IdsQueryBuilder query;
if (types.length > 0 || randomBoolean()) {
query = new IdsQueryBuilder(types);
query = new IdsQueryBuilder().types(types);
query.addIds(ids);
} else {
query = new IdsQueryBuilder();
@ -82,11 +82,11 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase<IdsQueryBuilder>
}
public void testIllegalArguments() {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IdsQueryBuilder((String[]) null));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> new IdsQueryBuilder().types((String[]) null));
assertEquals("[ids] types cannot be null", e.getMessage());
IdsQueryBuilder idsQueryBuilder = new IdsQueryBuilder();
e = expectThrows(IllegalArgumentException.class, () -> idsQueryBuilder.addIds((String[])null));
e = expectThrows(IllegalArgumentException.class, () -> idsQueryBuilder.addIds((String[]) null));
assertEquals("[ids] ids cannot be null", e.getMessage());
}
@ -136,10 +136,22 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase<IdsQueryBuilder>
parsed = (IdsQueryBuilder) parseQuery(json);
assertThat(parsed.ids(), contains("1","100","4"));
assertEquals(json, 0, parsed.types().length);
// check without type
json =
"{\n" +
" \"ids\" : {\n" +
" \"values\" : [ \"1\", \"100\", \"4\" ],\n" +
" \"boost\" : 1.0\n" +
" }\n" +
"}";
parsed = (IdsQueryBuilder) parseQuery(json);
assertThat(parsed.ids(), contains("1","100","4"));
assertEquals(json, 0, parsed.types().length);
}
public void testFromJsonDeprecatedSyntax() throws IOException {
IdsQueryBuilder testQuery = new IdsQueryBuilder("my_type");
IdsQueryBuilder testQuery = new IdsQueryBuilder().types("my_type");
//single value type can also be called _type
final String contentString = "{\n" +
@ -158,18 +170,18 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase<IdsQueryBuilder>
assertEquals(3, e.getLineNumber());
assertEquals(19, e.getColumnNumber());
//array of types can also be called type rather than types
//array of types can also be called types rather than type
final String contentString2 = "{\n" +
" \"ids\" : {\n" +
" \"types\" : [\"my_type\"],\n" +
" \"values\" : [ ]\n" +
" }\n" +
"}";
parsed = (IdsQueryBuilder) parseQuery(contentString, ParseFieldMatcher.EMPTY);
parsed = (IdsQueryBuilder) parseQuery(contentString2, ParseFieldMatcher.EMPTY);
assertEquals(testQuery, parsed);
e = expectThrows(ParsingException.class, () -> parseQuery(contentString2));
checkWarningHeaders("Deprecated field [_type] used, expected [type] instead");
checkWarningHeaders("Deprecated field [types] used, expected [type] instead");
assertEquals("Deprecated field [types] used, expected [type] instead", e.getMessage());
assertEquals(3, e.getLineNumber());
assertEquals(19, e.getColumnNumber());