Skip to content

Replaced field_masking_span occurrences with respective ParseField #74718

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

Merged
merged 11 commits into from
Jul 9, 2021
Merged

Replaced field_masking_span occurrences with respective ParseField #74718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6d9935b
Replaced field_masking_span occurrences with respective ParseField
GrayFox1 Jun 29, 2021
c01be61
Replaced field_masking_span occurrences with respective
GrayFox1 Jun 29, 2021
1264af8
Updates error messages
GrayFox1 Jun 30, 2021
c9b9237
Updates list of expected deprecated queries
GrayFox1 Jun 30, 2021
68a5210
Adds the test testJsonWithDeprecatedName
GrayFox1 Jun 30, 2021
c8048ee
Updates list of expected queries
GrayFox1 Jul 1, 2021
08f92b6
Removes unused import and Reverts changes to eclipseCodeFormatter
GrayFox1 Jul 1, 2021
16c220b
Reverts changes to eclipseCodeFormatter
GrayFox1 Jul 1, 2021
e2a053f
Merge remote-tracking branch 'upstream/master'
GrayFox1 Jul 1, 2021
c6751ff
Fixes expected queries lists alphabetical order
GrayFox1 Jul 1, 2021
cc067db
Merge branch 'master' into master
elasticmachine Jul 8, 2021
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
4 changes: 2 additions & 2 deletions docs/reference/query-dsl/span-field-masking-query.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ GET /_search
}
},
{
"field_masking_span": {
"span_field_masking": {
"query": {
"span_term": {
"text.stems": "fox"
Expand All @@ -42,4 +42,4 @@ GET /_search
}
--------------------------------------------------

Note: as span field masking query returns the masked field, scoring will be done using the norms of the field name supplied. This may lead to unexpected scoring behaviour.
Note: as span field masking query returns the masked field, scoring will be done using the norms of the field name supplied. This may lead to unexpected scoring behaviour.
4 changes: 2 additions & 2 deletions docs/reference/query-dsl/span-queries.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ The queries in this group are:
<<query-dsl-span-containing-query,`span_containing` query>>::
Accepts a list of span queries, but only returns those spans which also match a second span query.

<<query-dsl-span-field-masking-query,`field_masking_span` query>>::
<<query-dsl-span-field-masking-query,`span_field_masking` query>>::
Allows queries like `span-near` or `span-or` across different fields.

<<query-dsl-span-first-query,`span_first` query>>::
Expand Down Expand Up @@ -66,4 +66,4 @@ include::span-or-query.asciidoc[]

include::span-term-query.asciidoc[]

include::span-within-query.asciidoc[]
include::span-within-query.asciidoc[]
19 changes: 10 additions & 9 deletions server/src/main/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import static org.elasticsearch.index.query.SpanQueryBuilder.SpanQueryBuilderUtil.checkNoBoost;

public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder<FieldMaskingSpanQueryBuilder> implements SpanQueryBuilder {
public static final String NAME = "field_masking_span";
public static final ParseField NAME = new ParseField("span_field_masking","field_masking_span");

private static final ParseField FIELD_FIELD = new ParseField("field");
private static final ParseField QUERY_FIELD = new ParseField("query");
Expand Down Expand Up @@ -83,7 +83,7 @@ public SpanQueryBuilder innerQuery() {

@Override
protected void doXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(NAME);
builder.startObject(NAME.getPreferredName());
builder.field(QUERY_FIELD.getPreferredName());
queryBuilder.toXContent(builder, params);
builder.field(FIELD_FIELD.getPreferredName(), fieldName);
Expand All @@ -107,12 +107,13 @@ public static FieldMaskingSpanQueryBuilder fromXContent(XContentParser parser) t
if (QUERY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
QueryBuilder query = parseInnerQueryBuilder(parser);
if (query instanceof SpanQueryBuilder == false) {
throw new ParsingException(parser.getTokenLocation(), "[field_masking_span] query must be of type span query");
throw new ParsingException(parser.getTokenLocation(), "[" + NAME.getPreferredName() + "] query must " +
"be of type span query");
}
inner = (SpanQueryBuilder) query;
checkNoBoost(NAME, currentFieldName, parser, inner);
checkNoBoost(NAME.getPreferredName(), currentFieldName, parser, inner);
} else {
throw new ParsingException(parser.getTokenLocation(), "[field_masking_span] query does not support ["
throw new ParsingException(parser.getTokenLocation(), "[" + NAME.getPreferredName() + "] query does not support ["
+ currentFieldName + "]");
}
} else {
Expand All @@ -124,15 +125,15 @@ public static FieldMaskingSpanQueryBuilder fromXContent(XContentParser parser) t
queryName = parser.text();
} else {
throw new ParsingException(parser.getTokenLocation(),
"[field_masking_span] query does not support [" + currentFieldName + "]");
"[" + NAME.getPreferredName() + "] query does not support [" + currentFieldName + "]");
}
}
}
if (inner == null) {
throw new ParsingException(parser.getTokenLocation(), "field_masking_span must have [query] span query clause");
throw new ParsingException(parser.getTokenLocation(), NAME.getPreferredName() + " must have [query] span query clause");
}
if (field == null) {
throw new ParsingException(parser.getTokenLocation(), "field_masking_span must have [field] set for it");
throw new ParsingException(parser.getTokenLocation(), NAME.getPreferredName() + " must have [field] set for it");
}
FieldMaskingSpanQueryBuilder queryBuilder = new FieldMaskingSpanQueryBuilder(inner, field);
queryBuilder.boost(boost);
Expand Down Expand Up @@ -165,6 +166,6 @@ protected boolean doEquals(FieldMaskingSpanQueryBuilder other) {

@Override
public String getWriteableName() {
return NAME;
return NAME.getPreferredName();
}
}
27 changes: 24 additions & 3 deletions server/src/test/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.io.IOException;

import static org.elasticsearch.index.query.FieldMaskingSpanQueryBuilder.NAME;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;

Expand Down Expand Up @@ -57,7 +58,7 @@ public void testIllegalArguments() {
public void testFromJson() throws IOException {
String json =
"{\n" +
" \"field_masking_span\" : {\n" +
" \"" + NAME.getPreferredName() + "\" : {\n" +
" \"query\" : {\n" +
" \"span_term\" : {\n" +
" \"value\" : {\n" +
Expand All @@ -73,13 +74,13 @@ public void testFromJson() throws IOException {
"}";
Exception exception = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(exception.getMessage(),
equalTo("field_masking_span [query] as a nested span clause can't have non-default boost value [0.23]"));
equalTo(NAME.getPreferredName() + " [query] as a nested span clause can't have non-default boost value [0.23]"));
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add a test that uses the old deprecated name, and checks for a deprecation warning? There's an assertWarnings() helper method that will check for specific deprecation strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, i'll add a new test in my next commit.

public void testJsonWithTopLevelBoost() throws IOException {
String json =
"{\n" +
" \"field_masking_span\" : {\n" +
" \"" + NAME.getPreferredName() + "\" : {\n" +
" \"query\" : {\n" +
" \"span_term\" : {\n" +
" \"value\" : {\n" +
Expand All @@ -100,4 +101,24 @@ public void testJsonWithTopLevelBoost() throws IOException {
q
);
}

public void testJsonWithDeprecatedName() throws IOException {
String json =
"{\n" +
" \"field_masking_span\" : {\n" +
" \"query\" : {\n" +
" \"span_term\" : {\n" +
" \"value\" : {\n" +
" \"value\" : \"foo\"\n" +
" }\n" +
" }\n" +
" },\n" +
" \"field\" : \"mapped_geo_shape\",\n" +
" \"boost\" : 42.0,\n" +
" \"_name\" : \"KPI\"\n" +
" }\n" +
"}";
Query q = parseQuery(json).toQuery(createSearchExecutionContext());
assertWarnings("Deprecated field [field_masking_span] used, expected [" + NAME.getPreferredName() + "] instead");
}
}
4 changes: 2 additions & 2 deletions server/src/test/java/org/elasticsearch/search/SearchModuleTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,6 @@ public CheckedBiConsumer<ShardSearchRequest, StreamOutput, IOException> getReque
"combined_fields",
"dis_max",
"exists",
"field_masking_span",
"function_score",
"fuzzy",
"geo_bounding_box",
Expand All @@ -367,6 +366,7 @@ public CheckedBiConsumer<ShardSearchRequest, StreamOutput, IOException> getReque
"script_score",
"simple_query_string",
"span_containing",
"span_field_masking",
"span_first",
"span_gap",
"span_multi",
Expand All @@ -384,7 +384,7 @@ public CheckedBiConsumer<ShardSearchRequest, StreamOutput, IOException> getReque
};

//add here deprecated queries to make sure we log a deprecation warnings when they are used
private static final String[] DEPRECATED_QUERIES = new String[] {"geo_polygon"};
private static final String[] DEPRECATED_QUERIES = new String[] {"field_masking_span", "geo_polygon"};

/**
* Dummy test {@link AggregationBuilder} used to test registering aggregation builders.
Expand Down