Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove types from TermsLookup #46943

Merged
merged 4 commits into from
Sep 23, 2019
Merged

Remove types from TermsLookup #46943

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c1fe58
Remove types from TermsLookup
romseygeek Sep 20, 2019
56caebf
yaml tests
romseygeek Sep 20, 2019
fb3c925
yaml tests
romseygeek Sep 20, 2019
c360bf3
yaml security test
romseygeek Sep 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 0 additions & 57 deletions ...pi-spec/src/main/resources/rest-api-spec/test/search.aggregation/51_filter_with_types.yml
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions rest-api-spec/src/main/resources/rest-api-spec/test/search/150_rewrite_on_coordinator.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
search:
rest_total_hits_as_int: true
index: "search_index"
body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "type": "_doc", "id": "1", "path": "followers"} } } }
body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "id": "1", "path": "followers"} } } }
- do:
indices.create:
index: lookup_index
Expand All @@ -58,7 +58,7 @@
search:
rest_total_hits_as_int: true
index: "search_index"
body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "type": "_doc", "id": "1", "path": "followers"} } } }
body: { "size" : 0, "query" : { "terms" : { "user" : { "index": "lookup_index", "id": "1", "path": "followers"} } } }

- match: { _shards.total: 5 }
- match: { _shards.successful: 5 }
Expand Down
58 changes: 0 additions & 58 deletions rest-api-spec/src/main/resources/rest-api-spec/test/search/171_terms_query_with_types.yml
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.elasticsearch.index.query;

import org.apache.logging.log4j.LogManager;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermInSetQuery;
import org.apache.lucene.util.BytesRef;
Expand All @@ -34,7 +33,6 @@
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.lucene.BytesRefs;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand Down Expand Up @@ -64,11 +62,6 @@
public class TermsQueryBuilder extends AbstractQueryBuilder<TermsQueryBuilder> {
public static final String NAME = "terms";

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
LogManager.getLogger(TermsQueryBuilder.class));
static final String TYPES_DEPRECATION_MESSAGE = "[types removal] Types are deprecated " +
"in [terms] lookup queries.";

private final String fieldName;
private final List<?> values;
private final TermsLookup termsLookup;
Expand Down Expand Up @@ -217,10 +210,6 @@ public TermsLookup termsLookup() {
return this.termsLookup;
}

public boolean isTypeless() {
return termsLookup == null || termsLookup.type() == null;
}

private static final Set<Class<? extends Number>> INTEGER_TYPES = new HashSet<>(
Arrays.asList(Byte.class, Short.class, Integer.class, Long.class));
private static final Set<Class<?>> STRING_TYPES = new HashSet<>(
Expand Down Expand Up @@ -406,10 +395,6 @@ public static TermsQueryBuilder fromXContent(XContentParser parser) throws IOExc
.boost(boost)
.queryName(queryName);

if (builder.isTypeless() == false) {
deprecationLogger.deprecatedAndMaybeLog("terms_lookup_with_types", TYPES_DEPRECATION_MESSAGE);
}

return builder;
}

Expand Down
52 changes: 11 additions & 41 deletions server/src/main/java/org/elasticsearch/indices/TermsLookup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@

package org.elasticsearch.indices;

import org.elasticsearch.common.Nullable;
import org.elasticsearch.Version;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.query.TermsQueryBuilder;

import java.io.IOException;
Expand All @@ -37,21 +38,11 @@
*/
public class TermsLookup implements Writeable, ToXContentFragment {
private final String index;
private @Nullable String type;
private final String id;
private final String path;
private String routing;


public TermsLookup(String index, String id, String path) {
this(index, null, id, path);
}

/**
* @deprecated Types are in the process of being removed, use {@link TermsLookup(String, String, String)} instead.
*/
@Deprecated
public TermsLookup(String index, String type, String id, String path) {
if (id == null) {
throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the id.");
}
Expand All @@ -62,7 +53,6 @@ public TermsLookup(String index, String type, String id, String path) {
throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element requires specifying the index.");
}
this.index = index;
this.type = type;
this.id = id;
this.path = path;
}
Expand All @@ -71,7 +61,9 @@ public TermsLookup(String index, String type, String id, String path) {
* Read from a stream.
*/
public TermsLookup(StreamInput in) throws IOException {
type = in.readOptionalString();
if (in.getVersion().before(Version.V_8_0_0)) {
in.readOptionalString();
}
id = in.readString();
path = in.readString();
index = in.readString();
Expand All @@ -80,7 +72,9 @@ public TermsLookup(StreamInput in) throws IOException {

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalString(type);
if (out.getVersion().before(Version.V_8_0_0)) {
out.writeOptionalString(MapperService.SINGLE_MAPPING_NAME);
}
out.writeString(id);
out.writeString(path);
out.writeString(index);
Expand All @@ -91,14 +85,6 @@ public String index() {
return index;
}

/**
* @deprecated Types are in the process of being removed.
*/
@Deprecated
public String type() {
return type;
}

public String id() {
return id;
}
Expand All @@ -118,7 +104,6 @@ public TermsLookup routing(String routing) {

public static TermsLookup parseTermsLookup(XContentParser parser) throws IOException {
String index = null;
String type = null;
String id = null;
String path = null;
String routing = null;
Expand All @@ -132,9 +117,6 @@ public static TermsLookup parseTermsLookup(XContentParser parser) throws IOExcep
case "index":
index = parser.text();
break;
case "type":
type = parser.text();
break;
case "id":
id = parser.text();
break;
Expand All @@ -153,28 +135,17 @@ public static TermsLookup parseTermsLookup(XContentParser parser) throws IOExcep
+ token + "] after [" + currentFieldName + "]");
}
}
if (type == null) {
return new TermsLookup(index, id, path).routing(routing);
} else {
return new TermsLookup(index, type, id, path).routing(routing);
}
return new TermsLookup(index, id, path).routing(routing);
}

@Override
public String toString() {
if (type == null) {
return index + "/" + id + "/" + path;
} else {
return index + "/" + type + "/" + id + "/" + path;
}
return index + "/" + id + "/" + path;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.field("index", index);
if (type != null) {
builder.field("type", type);
}
builder.field("id", id);
builder.field("path", path);
if (routing != null) {
Expand All @@ -185,7 +156,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws

@Override
public int hashCode() {
return Objects.hash(index, type, id, path, routing);
return Objects.hash(index, id, path, routing);
}

@Override
Expand All @@ -198,7 +169,6 @@ public boolean equals(Object obj) {
}
TermsLookup other = (TermsLookup) obj;
return Objects.equals(index, other.index) &&
Objects.equals(type, other.type) &&
Objects.equals(id, other.id) &&
Objects.equals(path, other.path) &&
Objects.equals(routing, other.routing);
Expand Down
11 changes: 1 addition & 10 deletions server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,7 @@ protected TermsQueryBuilder doCreateTestQueryBuilder() {
}

private TermsLookup randomTermsLookup() {
// Randomly choose between a typeless terms lookup and one with an explicit type to make sure we are
// testing both cases.
TermsLookup lookup = randomBoolean()
? new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath)
: new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath);
TermsLookup lookup = new TermsLookup(randomAlphaOfLength(10), randomAlphaOfLength(10), termsPath);
lookup.routing(randomBoolean() ? randomAlphaOfLength(10) : null);
return lookup;
}
Expand Down Expand Up @@ -322,11 +318,6 @@ public void testTypeField() throws IOException {
protected QueryBuilder parseQuery(XContentParser parser) throws IOException {
QueryBuilder query = super.parseQuery(parser);
assertThat(query, CoreMatchers.instanceOf(TermsQueryBuilder.class));

TermsQueryBuilder termsQuery = (TermsQueryBuilder) query;
if (termsQuery.isTypeless() == false) {
assertWarnings(TermsQueryBuilder.TYPES_DEPRECATION_MESSAGE);
}
return query;
}
}
Loading