Simplified Linear & RRF Retrievers - Return error on empty fields param (#129962) (#129970)

Author: Mikep86

docs/changelog/129962.yaml

@@ -0,0 +1,5 @@
+pr: 129962
+summary: Simplified Linear & RRF Retrievers - Return error on empty fields param
+area: Search
+type: bug
+issues: []

x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/MultiFieldsInnerRetrieverUtils.java

@@ -56,15 +56,15 @@ public record WeightedRetrieverSource(CompoundRetrieverBuilder.RetrieverSource r
      */
     public static ActionRequestValidationException validateParams(
         List<CompoundRetrieverBuilder.RetrieverSource> innerRetrievers,
-        List<String> fields,
+        @Nullable List<String> fields,
         @Nullable String query,
         String retrieverName,
         String retrieversParamName,
         String fieldsParamName,
         String queryParamName,
         ActionRequestValidationException validationException
     ) {
-        if (fields.isEmpty() == false || query != null) {
+        if (fields != null || query != null) {
             // Using the multi-fields query format
             if (query == null) {
                 // Return early here because the following validation checks assume a query param value is provided
@@ -87,6 +87,13 @@ public static ActionRequestValidationException validateParams(
                 );
             }
 
+            if (fields != null && fields.isEmpty()) {
+                validationException = addValidationError(
+                    String.format(Locale.ROOT, "[%s] [%s] cannot be empty", retrieverName, fieldsParamName),
+                    validationException
+                );
+            }
+
             if (innerRetrievers.isEmpty() == false) {
                 validationException = addValidationError(
                     String.format(Locale.ROOT, "[%s] cannot combine [%s] and [%s]", retrieverName, retrieversParamName, queryParamName),
@@ -131,13 +138,15 @@ public static ActionRequestValidationException validateParams(
      * @return The inner retriever tree as a {@code RetrieverBuilder} list
      */
     public static List<RetrieverBuilder> generateInnerRetrievers(
-        List<String> fieldsAndWeights,
+        @Nullable List<String> fieldsAndWeights,
         String query,
         Collection<IndexMetadata> indicesMetadata,
         Function<List<WeightedRetrieverSource>, CompoundRetrieverBuilder<?>> innerNormalizerGenerator,
         @Nullable Consumer<Float> weightValidator
     ) {
-        Map<String, Float> parsedFieldsAndWeights = QueryParserHelper.parseFieldsAndWeights(fieldsAndWeights);
+        Map<String, Float> parsedFieldsAndWeights = fieldsAndWeights != null
+            ? QueryParserHelper.parseFieldsAndWeights(fieldsAndWeights)
+            : Map.of();
         if (weightValidator != null) {
             parsedFieldsAndWeights.values().forEach(weightValidator);
         }

x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilder.java

@@ -163,7 +163,7 @@ public LinearRetrieverBuilder(
             throw new IllegalArgumentException("The number of normalizers must match the number of inner retrievers");
         }
 
-        this.fields = fields == null ? List.of() : List.copyOf(fields);
+        this.fields = fields == null ? null : List.copyOf(fields);
         this.query = query;
         this.normalizer = normalizer;
         this.weights = weights;
@@ -400,7 +400,7 @@ public void doToXContent(XContentBuilder builder, Params params) throws IOExcept
             builder.endArray();
         }
 
-        if (fields.isEmpty() == false) {
+        if (fields != null) {
             builder.startArray(FIELDS_FIELD.getPreferredName());
             for (String field : fields) {
                 builder.value(field);

x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java

@@ -124,7 +124,7 @@ public RRFRetrieverBuilder(
     ) {
         // Use a mutable list for childRetrievers so that we can use addChild
         super(childRetrievers == null ? new ArrayList<>() : new ArrayList<>(childRetrievers), rankWindowSize);
-        this.fields = fields == null ? List.of() : List.copyOf(fields);
+        this.fields = fields == null ? null : List.copyOf(fields);
         this.query = query;
         this.rankConstant = rankConstant;
     }
@@ -302,7 +302,7 @@ public void doToXContent(XContentBuilder builder, Params params) throws IOExcept
             builder.endArray();
         }
 
-        if (fields.isEmpty() == false) {
+        if (fields != null) {
             builder.startArray(FIELDS_FIELD.getPreferredName());
             for (String field : fields) {
                 builder.value(field);

x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/20_linear_retriever_simplified.yml

@@ -465,6 +465,18 @@ setup:
 
   - contains: { error.root_cause.0.reason: "[linear] [query] cannot be empty" }
 
+  - do:
+      catch: bad_request
+      search:
+        index: test-index
+        body:
+          retriever:
+            linear:
+              fields: [ ]
+              query: "foo"
+
+  - contains: { error.root_cause.0.reason: "[linear] [fields] cannot be empty" }
+
   - do:
       catch: bad_request
       search:

x-pack/plugin/rank-rrf/src/yamlRestTest/resources/rest-api-spec/test/rrf/310_rrf_retriever_simplified.yml

@@ -359,6 +359,18 @@ setup:
 
   - contains: { error.root_cause.0.reason: "[rrf] [query] cannot be empty" }
 
+  - do:
+      catch: bad_request
+      search:
+        index: test-index
+        body:
+          retriever:
+            rrf:
+              fields: [ ]
+              query: "foo"
+
+  - contains: { error.root_cause.0.reason: "[rrf] [fields] cannot be empty" }
+
   - do:
       catch: bad_request
       search: