Pārlūkot izejas kodu

[Rest Api Compatibility] Nested path and filter sort options (#76022)

previously removed in #42809. nested_path and nested_filter sort options will now throw exceptions suggesting to use nested option
relates #51816
Przemyslaw Gomulka 4 gadi atpakaļ
vecāks
revīzija
d85acf48ae

+ 2 - 2
rest-api-spec/build.gradle

@@ -82,7 +82,8 @@ def v7compatibilityNotSupportedTests = {
 
           'indices.forcemerge/10_basic/Check deprecation warning when incompatible only_expunge_deletes and max_num_segments values are both set', //#44761 bug fix,
 
-          'search/340_type_query/type query' //#47207 type query throws exception in compatible mode
+          'search/340_type_query/type query', //#47207 type query throws exception in compatible mode
+          'search.aggregation/200_top_hits_metric/top_hits aggregation with sequence numbers', // #42809 the use nested path and filter sort throws an exception
 
   ]
 }
@@ -92,7 +93,6 @@ tasks.named("yamlRestCompatTest").configure {
     OS.current() != OS.WINDOWS
   }
   systemProperty 'tests.rest.blacklist', ([
-    'search.aggregation/200_top_hits_metric/top_hits aggregation with sequence numbers',
     'search/310_match_bool_prefix/multi_match multiple fields with cutoff_frequency throws exception', //cutoff_frequency
   ] + v7compatibilityNotSupportedTests())
     .join(',')

+ 149 - 0
rest-api-spec/src/yamlRestCompatTest/resources/rest-api-spec/test/v7compat/search/sort/10_nested_path_filter.yml

@@ -0,0 +1,149 @@
+---
+setup:
+- skip:
+    features:
+      - "headers"
+      - "allowed_warnings_regex"
+- do:
+    indices.create:
+      index: "my-index"
+      body:
+        settings:
+          number_of_shards: 1
+          number_of_replicas: 0
+        mappings:
+          properties:
+            offer:
+              type: "nested"
+- do:
+    index:
+      index: "my-index"
+      id: 1
+      refresh: true
+      body:
+        offer:
+          price: 10
+          color: blue
+
+    headers:
+      Content-Type: "application/vnd.elasticsearch+json;compatible-with=7"
+      Accept: "application/vnd.elasticsearch+json;compatible-with=7"
+
+- do:
+    indices.create:
+      index: "my-locations"
+      body:
+        settings:
+          number_of_shards: 1
+          number_of_replicas: 0
+        mappings:
+          properties:
+            pin:
+              properties:
+                location:
+                  type: geo_point
+            offer:
+                type: "nested"
+    headers:
+      Content-Type: "application/vnd.elasticsearch+json;compatible-with=7"
+      Accept: "application/vnd.elasticsearch+json;compatible-with=7"
+
+- do:
+    index:
+      index: "my-locations"
+      id: 1
+      refresh: true
+      body:
+        offer:
+          price: 10
+          color: blue
+        pin:
+          location:
+            lat: 40.12
+            lon: -71.34
+    headers:
+      Content-Type: "application/vnd.elasticsearch+json;compatible-with=7"
+      Accept: "application/vnd.elasticsearch+json;compatible-with=7"
+
+
+
+
+
+---
+"Sort with nested_path throws exception":
+- do:
+    catch: /\[nested_path\] has been removed in favour of the \[nested\] parameter/
+    search:
+      rest_total_hits_as_int: true
+      index: "my-index"
+      body:
+        sort:
+          - offer.price:
+              mode: avg
+              order: asc
+              nested_path: offer
+    headers:
+      Content-Type: "application/vnd.elasticsearch+json;compatible-with=7"
+      Accept: "application/vnd.elasticsearch+json;compatible-with=7"
+
+---
+"Sort with nested_filter throws exception":
+  - do:
+      catch: /\[nested_filter\] has been removed in favour of the \[nested\] parameter/
+      search:
+        rest_total_hits_as_int: true
+        index: "my-index"
+        body:
+          sort:
+            - offer.price:
+                mode: avg
+                order: asc
+                nested_filter:
+                  term:
+                    offer.color: blue
+      headers:
+        Content-Type: "application/vnd.elasticsearch+json;compatible-with=7"
+        Accept: "application/vnd.elasticsearch+json;compatible-with=7"
+
+
+---
+"Geo search with nested_filter throws exception":
+  - do:
+      catch: /\[nested_filter\] has been removed in favour of the \[nested\] parameter/
+      search:
+        rest_total_hits_as_int: true
+        index: "my-locations"
+        body:
+          query:
+            match_all: {}
+          sort:
+            _geo_distance:
+              pin.location:
+                - -70
+                - 40
+              nested_filter:
+                term:
+                  offer.color: blue
+      headers:
+        Content-Type: "application/vnd.elasticsearch+json;compatible-with=7"
+        Accept: "application/vnd.elasticsearch+json;compatible-with=7"
+
+---
+"Geo search with nested_path throws exception":
+  - do:
+      catch: /\[nested_path\] has been removed in favour of the \[nested\] parameter/
+      search:
+        rest_total_hits_as_int: true
+        index: "my-locations"
+        body:
+          query:
+            match_all: {}
+          sort:
+            _geo_distance:
+              pin.location:
+                - -70
+                - 40
+              nested_path: "offer"
+      headers:
+        Content-Type: "application/vnd.elasticsearch+json;compatible-with=7"
+        Accept: "application/vnd.elasticsearch+json;compatible-with=7"

+ 14 - 0
server/src/main/java/org/elasticsearch/search/sort/FieldSortBuilder.java

@@ -16,6 +16,7 @@ import org.apache.lucene.index.Terms;
 import org.apache.lucene.search.SortField;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.Version;
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.xcontent.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -692,6 +693,19 @@ public class FieldSortBuilder extends SortBuilder<FieldSortBuilder> {
         PARSER.declareObject(FieldSortBuilder::setNestedSort, (p, c) -> NestedSortBuilder.fromXContent(p), NESTED_FIELD);
         PARSER.declareString(FieldSortBuilder::setNumericType, NUMERIC_TYPE);
         PARSER.declareString(FieldSortBuilder::setFormat, FORMAT);
+        PARSER.declareField((b,v)->{}, (p, c) -> {
+            throw new ParsingException(
+                p.getTokenLocation(),
+                "[nested_path] has been removed in favour of the [nested] parameter",
+                c);
+        }, NESTED_PATH_FIELD, ValueType.STRING);
+
+        PARSER.declareObject((b,v)->{}, (p, c) -> {
+            throw new ParsingException(
+                p.getTokenLocation(),
+                "[nested_filter] has been removed in favour of the [nested] parameter",
+                c);
+        }, NESTED_FILTER_FIELD);
     }
 
     @Override

+ 30 - 11
server/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java

@@ -19,6 +19,7 @@ import org.apache.lucene.search.comparators.DoubleComparator;
 import org.apache.lucene.util.BitSet;
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.Version;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.xcontent.ParseField;
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.geo.GeoDistance;
@@ -31,6 +32,7 @@ import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentParser.Token;
+import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.index.fielddata.FieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested;
@@ -63,6 +65,7 @@ import static org.elasticsearch.search.sort.NestedSortBuilder.NESTED_FIELD;
  * A geo distance based sorting on a geo point like field.
  */
 public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder> {
+    private static final DeprecationLogger deprecationLogger =  DeprecationLogger.getLogger(GeoDistanceSortBuilder.class);
 
     public static final String NAME = "_geo_distance";
     public static final String ALTERNATIVE_NAME = "_geoDistance";
@@ -406,7 +409,15 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
 
                 fieldName = currentName;
             } else if (token == XContentParser.Token.START_OBJECT) {
-                if (NESTED_FIELD.match(currentName, parser.getDeprecationHandler())) {
+                if (parser.getRestApiVersion() == RestApiVersion.V_7 &&
+                    NESTED_FILTER_FIELD.match(currentName, parser.getDeprecationHandler())) {
+                    deprecationLogger.compatibleApiWarning("nested_filter",
+                        "[nested_filter] has been removed in favour of the [nested] parameter");
+                    throw new ParsingException(
+                        parser.getTokenLocation(),
+                        "[nested_filter] has been removed in favour of the [nested] parameter",
+                        currentName);
+                } else if (NESTED_FIELD.match(currentName, parser.getDeprecationHandler())) {
                     nestedSort = NestedSortBuilder.fromXContent(parser);
                 } else {
                     // the json in the format of -> field : { lat : 30, lon : 12 }
@@ -423,7 +434,15 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
                     geoPoints.add(point);
                 }
             } else if (token.isValue()) {
-                if (ORDER_FIELD.match(currentName, parser.getDeprecationHandler())) {
+                if (parser.getRestApiVersion() == RestApiVersion.V_7 &&
+                    NESTED_PATH_FIELD.match(currentName, parser.getDeprecationHandler())) {
+                    deprecationLogger.compatibleApiWarning("nested_path",
+                        "[nested_path] has been removed in favour of the [nested] parameter");
+                    throw new ParsingException(
+                        parser.getTokenLocation(),
+                        "[nested_path] has been removed in favour of the [nested] parameter",
+                        currentName);
+                } else if (ORDER_FIELD.match(currentName, parser.getDeprecationHandler())) {
                     order = SortOrder.fromString(parser.text());
                 } else if (UNIT_FIELD.match(currentName, parser.getDeprecationHandler())) {
                     unit = DistanceUnit.fromString(parser.text());
@@ -435,24 +454,24 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
                     sortMode = SortMode.fromString(parser.text());
                 } else if (IGNORE_UNMAPPED.match(currentName, parser.getDeprecationHandler())) {
                     ignoreUnmapped = parser.booleanValue();
-                } else if (token == Token.VALUE_STRING){
+                } else if (token == Token.VALUE_STRING) {
                     if (fieldName != null && fieldName.equals(currentName) == false) {
                         throw new ParsingException(
-                                parser.getTokenLocation(),
-                                "Trying to reset fieldName to [{}], already set to [{}].",
-                                currentName,
-                                fieldName);
+                            parser.getTokenLocation(),
+                            "Trying to reset fieldName to [{}], already set to [{}].",
+                            currentName,
+                            fieldName);
                     }
 
                     GeoPoint point = new GeoPoint();
                     point.resetFromString(parser.text());
                     geoPoints.add(point);
                     fieldName = currentName;
-                } else if (fieldName.equals(currentName)){
+                } else if (fieldName.equals(currentName)) {
                     throw new ParsingException(
-                            parser.getTokenLocation(),
-                            "Only geohashes of type string supported for field [{}]",
-                            currentName);
+                        parser.getTokenLocation(),
+                        "Only geohashes of type string supported for field [{}]",
+                        currentName);
                 } else {
                     throw new ParsingException(
                         parser.getTokenLocation(),

+ 15 - 0
server/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java

@@ -15,6 +15,7 @@ import org.apache.lucene.search.SortField;
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.BytesRefBuilder;
 import org.elasticsearch.Version;
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.xcontent.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -206,6 +207,20 @@ public class ScriptSortBuilder extends SortBuilder<ScriptSortBuilder> {
         PARSER.declareString((b, v) -> b.order(SortOrder.fromString(v)), ORDER_FIELD);
         PARSER.declareString((b, v) -> b.sortMode(SortMode.fromString(v)), SORTMODE_FIELD);
         PARSER.declareObject(ScriptSortBuilder::setNestedSort, (p, c) -> NestedSortBuilder.fromXContent(p), NESTED_FIELD);
+
+        PARSER.declareObject((b,v)->{}, (p, c) -> {
+            throw new ParsingException(
+                p.getTokenLocation(),
+                "[nested_path] has been removed in favour of the [nested] parameter",
+                c);
+        }, NESTED_PATH_FIELD);
+
+        PARSER.declareObject((b,v)->{}, (p, c) -> {
+            throw new ParsingException(
+                p.getTokenLocation(),
+                "[nested_filter] has been removed in favour of the [nested] parameter",
+                c);
+        }, NESTED_FILTER_FIELD);
     }
 
     /**

+ 7 - 0
server/src/main/java/org/elasticsearch/search/sort/SortBuilder.java

@@ -20,6 +20,7 @@ import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.index.fielddata.IndexFieldData.XFieldComparatorSource.Nested;
 import org.elasticsearch.index.mapper.NestedObjectMapper;
 import org.elasticsearch.index.mapper.ObjectMapper;
@@ -45,6 +46,12 @@ public abstract class SortBuilder<T extends SortBuilder<T>> implements NamedWrit
 
     // parse fields common to more than one SortBuilder
     public static final ParseField ORDER_FIELD = new ParseField("order");
+    public static final ParseField NESTED_FILTER_FIELD = new ParseField("nested_filter")
+        .withAllDeprecated()
+        .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.V_7));
+    public static final ParseField NESTED_PATH_FIELD = new ParseField("nested_path")
+        .withAllDeprecated()
+        .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.V_7));
 
     private static final Map<String, Parser<?>> PARSERS = Map.of(
             ScriptSortBuilder.NAME, ScriptSortBuilder::fromXContent,