Rename field_masking_span to span_field_masking (#74718)

GrayFox1 · web-flow · commit bb37e09d92e0 · 2021-07-09T08:56:38.000+01:00
`field_masking_span` is the only span query that does not begin with
`span_`.  This commit deprecates the existing name and adds a new
name `span_field_masking` to better fit with the other queries.
diff --git a/docs/reference/query-dsl/span-field-masking-query.asciidoc b/docs/reference/query-dsl/span-field-masking-query.asciidoc
@@ -25,7 +25,7 @@ GET /_search
           }
         },
         {
-          "field_masking_span": {
+          "span_field_masking": {
             "query": {
               "span_term": {
                 "text.stems": "fox"
@@ -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.
diff --git a/docs/reference/query-dsl/span-queries.asciidoc b/docs/reference/query-dsl/span-queries.asciidoc
@@ -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>>::
@@ -66,4 +66,4 @@ include::span-or-query.asciidoc[]
 
 include::span-term-query.asciidoc[]
 
-include::span-within-query.asciidoc[]
+include::span-within-query.asciidoc[]
diff --git a/server/src/main/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilder.java
@@ -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");
@@ -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);
@@ -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 {
@@ -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);
@@ -165,6 +166,6 @@ protected boolean doEquals(FieldMaskingSpanQueryBuilder other) {
 
     @Override
     public String getWriteableName() {
-        return NAME;
+        return NAME.getPreferredName();
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilderTests.java
@@ -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;
 
@@ -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" +
@@ -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]"));
     }
 
     public void testJsonWithTopLevelBoost() throws IOException {
         String json =
             "{\n" +
-                "  \"field_masking_span\" : {\n" +
+                "  \"" + NAME.getPreferredName() + "\" : {\n" +
                 "    \"query\" : {\n" +
                 "      \"span_term\" : {\n" +
                 "        \"value\" : {\n" +
@@ -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");
+    }
 }
diff --git a/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java b/server/src/test/java/org/elasticsearch/search/SearchModuleTests.java
@@ -342,7 +342,6 @@ public CheckedBiConsumer<ShardSearchRequest, StreamOutput, IOException> getReque
             "combined_fields",
             "dis_max",
             "exists",
-            "field_masking_span",
             "function_score",
             "fuzzy",
             "geo_bounding_box",
@@ -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",
@@ -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.

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ GET /_search`
`25`	`25`	`}`
`26`	`26`	`},`
`27`	`27`	`{`
`28`		`- "field_masking_span": {`
	`28`	`+ "span_field_masking": {`
`29`	`29`	`"query": {`
`30`	`30`	`"span_term": {`
`31`	`31`	`"text.stems": "fox"`
`@@ -42,4 +42,4 @@ GET /_search`
`42`	`42`	`}`
`43`	`43`	`--------------------------------------------------`
`44`	`44`
`45`		`-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.`
	`45`	`+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.`