Add checks to RankEvalSpec to safe guard against missing parameters. (#22026)

Add checks to RankEvalSpec to safe guard against missing parameters.

Fail early in case no metric is supplied, no rated requests are supplied or the search source builder is missing but no template is supplied neither.

Add stricter checks around rank eval request parsing: Fail if in a rated request we see both, a verbatim request as well as request
template parameters.


Relates to #21260

modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalSpec.java

@@ -26,7 +26,7 @@ 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.ObjectParser;
+import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.script.Script;
@@ -52,13 +52,31 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable {
     @Nullable
     private Script template;
 
-    public RankEvalSpec() {
-        // TODO think if no args ctor is okay
+    public RankEvalSpec(Collection<RatedRequest> ratedRequests, RankedListQualityMetric metric, Script template) {
+        if (ratedRequests == null || ratedRequests.size() < 1) {
+            throw new IllegalStateException(
+                    "Cannot evaluate ranking if no search requests with rated results are provided. Seen: " + ratedRequests);
+        }
+        if (metric == null) {
+            throw new IllegalStateException(
+                    "Cannot evaluate ranking if no evaluation metric is provided.");
+        }
+        if (template == null) {
+            for (RatedRequest request : ratedRequests) {
+                if (request.getTestRequest() == null) {
+                    throw new IllegalStateException(
+                            "Cannot evaluate ranking if neither template nor test request is provided. Seen for request id: "
+                    + request.getId());
+                }
+            }
+        }
+        this.ratedRequests = ratedRequests;
+        this.metric = metric;
+        this.template = template;
     }
 
-    public RankEvalSpec(Collection<RatedRequest> specs, RankedListQualityMetric metric) {
-        this.ratedRequests = specs;
-        this.metric = metric;
+    public RankEvalSpec(Collection<RatedRequest> ratedRequests, RankedListQualityMetric metric) {
+        this(ratedRequests, metric, null);
     }
 
     public RankEvalSpec(StreamInput in) throws IOException {
@@ -88,31 +106,16 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable {
         }
     }
 
-    /** Set the metric to use for quality evaluation. */
-    public void setMetric(RankedListQualityMetric metric) {
-        this.metric = metric;
-    }
-
     /** Returns the metric to use for quality evaluation.*/
     public RankedListQualityMetric getMetric() {
         return metric;
     }
 
     /** Returns a list of intent to query translation specifications to evaluate. */
-    public Collection<RatedRequest> getSpecifications() {
+    public Collection<RatedRequest> getRatedRequests() {
         return ratedRequests;
     }
 
-    /** Set the list of intent to query translation specifications to evaluate. */
-    public void setSpecifications(Collection<RatedRequest> specifications) {
-        this.ratedRequests = specifications;
-    }
-
-    /** Set the template to base test requests on. */
-    public void setTemplate(Script script) {
-        this.template = script;
-    }
-
     /** Returns the template to base test requests on. */
     public Script getTemplate() {
         return this.template;
@@ -121,34 +124,37 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable {
     private static final ParseField TEMPLATE_FIELD = new ParseField("template");
     private static final ParseField METRIC_FIELD = new ParseField("metric");
     private static final ParseField REQUESTS_FIELD = new ParseField("requests");
-    private static final ObjectParser<RankEvalSpec, RankEvalContext> PARSER = new ObjectParser<>("rank_eval", RankEvalSpec::new);
+    @SuppressWarnings("unchecked")
+    private static final ConstructingObjectParser<RankEvalSpec, RankEvalContext> PARSER =
+            new ConstructingObjectParser<>("rank_eval",
+            a -> new RankEvalSpec((Collection<RatedRequest>) a[0], (RankedListQualityMetric) a[1], (Script) a[2]));
 
     static {
-        PARSER.declareObject(RankEvalSpec::setMetric, (p, c) -> {
-            try {
-                return RankedListQualityMetric.fromXContent(p, c);
-            } catch (IOException ex) {
-                throw new ParsingException(p.getTokenLocation(), "error parsing rank request", ex);
-            }
-        } , METRIC_FIELD);
-        PARSER.declareObject(RankEvalSpec::setTemplate, (p, c) -> {
-            try {
-                return Script.parse(p, c.getParseFieldMatcher(), "mustache");
-            } catch (IOException ex) {
-                throw new ParsingException(p.getTokenLocation(), "error parsing rank request", ex);
-            }
-        }, TEMPLATE_FIELD);
-        PARSER.declareObjectArray(RankEvalSpec::setSpecifications, (p, c) -> {
+        PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), (p, c) -> {
             try {
                 return RatedRequest.fromXContent(p, c);
             } catch (IOException ex) {
                 throw new ParsingException(p.getTokenLocation(), "error parsing rank request", ex);
             }
         } , REQUESTS_FIELD);
+        PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> {
+            try {
+                return RankedListQualityMetric.fromXContent(p, c);
+            } catch (IOException ex) {
+                throw new ParsingException(p.getTokenLocation(), "error parsing rank request", ex);
+            }
+        } , METRIC_FIELD);
+        PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> {
+            try {
+                return Script.parse(p, c.getParseFieldMatcher(), "mustache");
+            } catch (IOException ex) {
+                throw new ParsingException(p.getTokenLocation(), "error parsing rank request", ex);
+            }
+        }, TEMPLATE_FIELD);
     }
 
-    public static RankEvalSpec parse(XContentParser parser, RankEvalContext context, boolean templated) throws IOException {
-        return PARSER.parse(parser, context);
+    public static RankEvalSpec parse(XContentParser parser, RankEvalContext context) throws IOException {
+        return PARSER.apply(parser, context);
     }
 
     @Override

modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java

@@ -20,12 +20,13 @@
 package org.elasticsearch.index.rankeval;
 
 import org.elasticsearch.action.support.ToXContentToBytes;
+import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseField;
 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.ObjectParser;
+import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
@@ -48,32 +49,55 @@ import java.util.Set;
  * */
 @SuppressWarnings("unchecked")
 public class RatedRequest extends ToXContentToBytes implements Writeable {
-    private String specId;
-    private SearchSourceBuilder testRequest;
+    private String id;
     private List<String> indices = new ArrayList<>();
     private List<String> types = new ArrayList<>();
     private List<String> summaryFields = new ArrayList<>();
     /** Collection of rated queries for this query QA specification.*/
     private List<RatedDocument> ratedDocs = new ArrayList<>();
+    /** Search request to execute for this rated request, can be null, if template and corresponding params are supplied. */
+    @Nullable
+    private SearchSourceBuilder testRequest;
     /** Map of parameters to use for filling a query template, can be used instead of providing testRequest. */
     private Map<String, Object> params = new HashMap<>();
 
-    public RatedRequest() {
-        // ctor that doesn't require all args to be present immediatly is easier to use with ObjectParser
-        // TODO decide if we can require only id as mandatory, set default values for the rest?
+    public RatedRequest(String id, List<RatedDocument> ratedDocs, SearchSourceBuilder testRequest, Map<String, Object> params) {
+        if (params != null && (params.size() > 0 && testRequest != null)) {
+            throw new IllegalArgumentException(
+                    "Ambiguous rated request: Set both, verbatim test request and test request template parameters.");
+        }
+        if ((params == null || params.size() < 1) && testRequest == null) {
+            throw new IllegalArgumentException(
+                    "Need to set at least test request or test request template parameters.");
+        }
+        // No documents with same _index/_type/id allowed.
+        Set<DocumentKey> docKeys = new HashSet<>();
+        for (RatedDocument doc : ratedDocs) {
+            if (docKeys.add(doc.getKey()) == false) {
+                String docKeyToString = doc.getKey().toString().replaceAll("\n", "").replaceAll("  ", " ");
+                throw new IllegalArgumentException(
+                        "Found duplicate rated document key [" + docKeyToString + "]");
+            }
+        }
+
+        this.id = id;
+        this.testRequest = testRequest;
+        this.ratedDocs = ratedDocs;
+        if (params != null) {
+            this.params = params;
+        }
+    }
+    
+    public RatedRequest(String id, List<RatedDocument> ratedDocs, Map<String, Object> params) {
+        this(id, ratedDocs, null, params);
     }
 
-    public RatedRequest(String specId, SearchSourceBuilder testRequest, List<String> indices, List<String> types,
-            List<RatedDocument> ratedDocs) {
-        this.specId = specId;
-        this.testRequest = testRequest;
-        this.indices = indices;
-        this.types = types;
-        setRatedDocs(ratedDocs);
+    public RatedRequest(String id, List<RatedDocument> ratedDocs, SearchSourceBuilder testRequest) {
+        this(id, ratedDocs, testRequest, new HashMap<>());
     }
 
     public RatedRequest(StreamInput in) throws IOException {
-        this.specId = in.readString();
+        this.id = in.readString();
         testRequest = in.readOptionalWriteable(SearchSourceBuilder::new);
 
         int indicesSize = in.readInt();
@@ -101,7 +125,7 @@ public class RatedRequest extends ToXContentToBytes implements Writeable {
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
-        out.writeString(specId);
+        out.writeString(id);
         out.writeOptionalWriteable(testRequest);
 
         out.writeInt(indices.size());
@@ -127,34 +151,25 @@ public class RatedRequest extends ToXContentToBytes implements Writeable {
         return testRequest;
     }
 
-    public void setTestRequest(SearchSourceBuilder testRequest) {
-        this.testRequest = testRequest;
+    public void setIndices(List<String> indices) {
+        this.indices = indices;
     }
 
     public List<String> getIndices() {
         return indices;
     }
 
-    public void setIndices(List<String> indices) {
-        this.indices = indices;
+    public void setTypes(List<String> types) {
+        this.types = types;
     }
 
     public List<String> getTypes() {
         return types;
     }
 
-    public void setTypes(List<String> types) {
-        this.types = types;
-    }
-
     /** Returns a user supplied spec id for easier referencing. */
-    public String getSpecId() {
-        return specId;
-    }
-
-    /** Sets a user supplied spec id for easier referencing. */
-    public void setSpecId(String specId) {
-        this.specId = specId;
+    public String getId() {
+        return id;
     }
 
     /** Returns a list of rated documents to evaluate. */
@@ -162,70 +177,56 @@ public class RatedRequest extends ToXContentToBytes implements Writeable {
         return ratedDocs;
     }
 
-    /**
-     * Set a list of rated documents for this query.
-     * No documents with same _index/_type/id allowed.
-     **/
-    public void setRatedDocs(List<RatedDocument> ratedDocs) {
-        Set<DocumentKey> docKeys = new HashSet<>();
-        for (RatedDocument doc : ratedDocs) {
-            if (docKeys.add(doc.getKey()) == false) {
-                String docKeyToString = doc.getKey().toString().replaceAll("\n", "").replaceAll("  ", " ");
-                throw new IllegalArgumentException(
-                        "Found duplicate rated document key [" + docKeyToString + "]");
-            }
-        }
-        this.ratedDocs = ratedDocs;
-    }
-
-    public void setParams(Map<String, Object> params) {
-        this.params = params;
-    }
-
     public Map<String, Object> getParams() {
         return this.params;
     }
 
-    public void setSummaryFields(List<String> fields) {
-        this.summaryFields = fields;
-    }
-
     /** Returns a list of fields that are included in the docs summary of matched documents. */
     public List<String> getSummaryFields() {
         return summaryFields;
     }
+    
+    public void setSummaryFields(List<String> summaryFields) {
+        if (summaryFields == null) {
+            throw new IllegalArgumentException("Setting summaryFields to null not allowed.");
+        }
+        this.summaryFields = summaryFields;
+    }
 
     private static final ParseField ID_FIELD = new ParseField("id");
     private static final ParseField REQUEST_FIELD = new ParseField("request");
     private static final ParseField RATINGS_FIELD = new ParseField("ratings");
     private static final ParseField PARAMS_FIELD = new ParseField("params");
     private static final ParseField FIELDS_FIELD = new ParseField("summary_fields");
-    private static final ObjectParser<RatedRequest, RankEvalContext> PARSER = new ObjectParser<>("requests", RatedRequest::new);
+
+    private static final ConstructingObjectParser<RatedRequest, RankEvalContext> PARSER = 
+            new ConstructingObjectParser<>("requests", a -> new RatedRequest(
+                    (String) a[0], (List<RatedDocument>) a[1], (SearchSourceBuilder) a[2], (Map<String, Object>) a[3]));
 
     static {
-        PARSER.declareString(RatedRequest::setSpecId, ID_FIELD);
-        PARSER.declareStringArray(RatedRequest::setSummaryFields, FIELDS_FIELD);
-        PARSER.declareObject(RatedRequest::setTestRequest, (p, c) -> {
-            try {
-                return SearchSourceBuilder.fromXContent(c.getParseContext(), c.getAggs(),  c.getSuggesters(), c.getSearchExtParsers());
-            } catch (IOException ex) {
-                throw new ParsingException(p.getTokenLocation(), "error parsing request", ex);
-            }
-        } , REQUEST_FIELD);
-        PARSER.declareObjectArray(RatedRequest::setRatedDocs, (p, c) -> {
+        PARSER.declareString(ConstructingObjectParser.constructorArg(), ID_FIELD);
+        PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), (p, c) -> {
             try {
                 return RatedDocument.fromXContent(p, c);
             } catch (IOException ex) {
                 throw new ParsingException(p.getTokenLocation(), "error parsing ratings", ex);
             }
         }, RATINGS_FIELD);
-        PARSER.declareObject(RatedRequest::setParams, (p, c) -> {
+        PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> {
+            try {
+                return SearchSourceBuilder.fromXContent(c.getParseContext(), c.getAggs(),  c.getSuggesters(), c.getSearchExtParsers());
+            } catch (IOException ex) {
+                throw new ParsingException(p.getTokenLocation(), "error parsing request", ex);
+            }
+        } , REQUEST_FIELD);
+        PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> {
             try {
                 return (Map) p.map();
             } catch (IOException ex) {
                 throw new ParsingException(p.getTokenLocation(), "error parsing ratings", ex);
             }
         }, PARAMS_FIELD);
+        PARSER.declareStringArray(RatedRequest::setSummaryFields, FIELDS_FIELD);
     }
 
     /**
@@ -250,13 +251,13 @@ public class RatedRequest extends ToXContentToBytes implements Writeable {
      *  }
      */
     public static RatedRequest fromXContent(XContentParser parser, RankEvalContext context) throws IOException {
-        return PARSER.parse(parser, context);
+        return PARSER.apply(parser, context);
     }
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
-        builder.field(ID_FIELD.getPreferredName(), this.specId);
+        builder.field(ID_FIELD.getPreferredName(), this.id);
         if (testRequest != null) {
             builder.field(REQUEST_FIELD.getPreferredName(), this.testRequest);
         }
@@ -292,7 +293,7 @@ public class RatedRequest extends ToXContentToBytes implements Writeable {
 
         RatedRequest other = (RatedRequest) obj;
         
-        return Objects.equals(specId, other.specId) &&
+        return Objects.equals(id, other.id) &&
                 Objects.equals(testRequest, other.testRequest) &&
                 Objects.equals(indices, other.indices) &&
                 Objects.equals(types, other.types) &&
@@ -303,6 +304,6 @@ public class RatedRequest extends ToXContentToBytes implements Writeable {
 
     @Override
     public final int hashCode() {
-        return Objects.hash(specId, testRequest, indices, types, summaryFields, ratedDocs, params);
+        return Objects.hash(id, testRequest, indices, types, summaryFields, ratedDocs, params);
     }
 }

modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RestRankEvalAction.java

@@ -175,13 +175,6 @@ public class RestRankEvalAction extends BaseRestHandler {
         controller.registerHandler(POST, "/{index}/_rank_eval", this);
         controller.registerHandler(GET, "/{index}/{type}/_rank_eval", this);
         controller.registerHandler(POST, "/{index}/{type}/_rank_eval", this);
-
-        controller.registerHandler(GET, "/_rank_eval/template", this);
-        controller.registerHandler(POST, "/_rank_eval/template", this);
-        controller.registerHandler(GET, "/{index}/_rank_eval/template", this);
-        controller.registerHandler(POST, "/{index}/_rank_eval/template", this);
-        controller.registerHandler(GET, "/{index}/{type}/_rank_eval/template", this);
-        controller.registerHandler(POST, "/{index}/{type}/_rank_eval/template", this);
     }
 
     @Override
@@ -202,9 +195,8 @@ public class RestRankEvalAction extends BaseRestHandler {
         List<String> indices = Arrays.asList(Strings.splitStringByCommaToArray(request.param("index")));
         List<String> types = Arrays.asList(Strings.splitStringByCommaToArray(request.param("type")));
         RankEvalSpec spec = null;
-        boolean containsTemplate = request.path().contains("template");
-        spec = RankEvalSpec.parse(context.parser(), context, containsTemplate);
-        for (RatedRequest specification : spec.getSpecifications()) {
+        spec = RankEvalSpec.parse(context.parser(), context);
+        for (RatedRequest specification : spec.getRatedRequests()) {
             specification.setIndices(indices);
             specification.setTypes(types);
         };

modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java

@@ -79,7 +79,7 @@ public class TransportRankEvalAction extends HandledTransportAction<RankEvalRequ
     protected void doExecute(RankEvalRequest request, ActionListener<RankEvalResponse> listener) {
         RankEvalSpec qualityTask = request.getRankEvalSpec();
 
-        Collection<RatedRequest> specifications = qualityTask.getSpecifications();
+        Collection<RatedRequest> specifications = qualityTask.getRatedRequests();
         AtomicInteger responseCounter = new AtomicInteger(specifications.size());
         Map<String, EvalQueryQuality> partialResults = new ConcurrentHashMap<>(specifications.size());
         Map<String, Exception> errors = new ConcurrentHashMap<>(specifications.size());
@@ -88,34 +88,36 @@ public class TransportRankEvalAction extends HandledTransportAction<RankEvalRequ
         if (qualityTask.getTemplate() != null) {
              scriptWithoutParams = scriptService.compile(qualityTask.getTemplate(), ScriptContext.Standard.SEARCH, new HashMap<>());
         }
-        for (RatedRequest querySpecification : specifications) {
-            final RankEvalActionListener searchListener = new RankEvalActionListener(listener, qualityTask.getMetric(), querySpecification,
+        for (RatedRequest ratedRequest : specifications) {
+            final RankEvalActionListener searchListener = new RankEvalActionListener(listener, qualityTask.getMetric(), ratedRequest,
                     partialResults, errors, responseCounter);
-            SearchSourceBuilder specRequest = querySpecification.getTestRequest();
-            if (specRequest == null) {
-                Map<String, Object> params = querySpecification.getParams();
+            SearchSourceBuilder ratedSearchSource = ratedRequest.getTestRequest();
+            if (ratedSearchSource == null) {
+                Map<String, Object> params = ratedRequest.getParams();
                 String resolvedRequest = ((BytesReference) (scriptService.executable(scriptWithoutParams, params).run())).utf8ToString();
                 try (XContentParser subParser = XContentFactory.xContent(resolvedRequest).createParser(resolvedRequest)) {
                     QueryParseContext parseContext = new QueryParseContext(searchRequestParsers.queryParsers, subParser, parseFieldMatcher);
-                    specRequest = SearchSourceBuilder.fromXContent(parseContext, searchRequestParsers.aggParsers,
+                    ratedSearchSource = SearchSourceBuilder.fromXContent(parseContext, searchRequestParsers.aggParsers,
                             searchRequestParsers.suggesters, searchRequestParsers.searchExtParsers);
                 } catch (IOException e) {
                     listener.onFailure(e);
                 }
             }
-            List<String> summaryFields = querySpecification.getSummaryFields();
+            List<String> summaryFields = ratedRequest.getSummaryFields();
             if (summaryFields.isEmpty()) {
-                specRequest.fetchSource(false);
+                ratedSearchSource.fetchSource(false);
             } else {
-                specRequest.fetchSource(summaryFields.toArray(new String[summaryFields.size()]), new String[0]);
+                ratedSearchSource.fetchSource(summaryFields.toArray(new String[summaryFields.size()]), new String[0]);
             }
 
-            String[] indices = new String[querySpecification.getIndices().size()];
-            querySpecification.getIndices().toArray(indices);
-            SearchRequest templatedRequest = new SearchRequest(indices, specRequest);
-            String[] types = new String[querySpecification.getTypes().size()];
-            querySpecification.getTypes().toArray(types);
+            String[] indices = new String[ratedRequest.getIndices().size()];
+            indices = ratedRequest.getIndices().toArray(indices);
+            SearchRequest templatedRequest = new SearchRequest(indices, ratedSearchSource);
+
+            String[] types = new String[ratedRequest.getTypes().size()];
+            types = ratedRequest.getTypes().toArray(types);
             templatedRequest.types(types);
+
             client.search(templatedRequest, searchListener);
         }
     }
@@ -142,14 +144,14 @@ public class TransportRankEvalAction extends HandledTransportAction<RankEvalRequ
         @Override
         public void onResponse(SearchResponse searchResponse) {
             SearchHit[] hits = searchResponse.getHits().getHits();
-            EvalQueryQuality queryQuality = metric.evaluate(specification.getSpecId(), hits, specification.getRatedDocs());
-            requestDetails.put(specification.getSpecId(), queryQuality);
+            EvalQueryQuality queryQuality = metric.evaluate(specification.getId(), hits, specification.getRatedDocs());
+            requestDetails.put(specification.getId(), queryQuality);
             handleResponse();
         }
 
         @Override
         public void onFailure(Exception exception) {
-            errors.put(specification.getSpecId(), exception);
+            errors.put(specification.getId(), exception);
             handleResponse();
         }
 

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java

@@ -76,11 +76,19 @@ public class RankEvalRequestIT  extends ESIntegTestCase {
         List<RatedRequest> specifications = new ArrayList<>();
         SearchSourceBuilder testQuery = new SearchSourceBuilder();
         testQuery.query(new MatchAllQueryBuilder());
-        RatedRequest amsterdamRequest = new RatedRequest("amsterdam_query", testQuery, indices, types, createRelevant("2", "3", "4", "5"));
+        RatedRequest amsterdamRequest = new RatedRequest(
+                "amsterdam_query", createRelevant("2", "3", "4", "5"), testQuery);
+        amsterdamRequest.setIndices(indices);
+        amsterdamRequest.setTypes(types);
         amsterdamRequest.setSummaryFields(Arrays.asList(new String[]{ "text", "title" }));
+
         specifications.add(amsterdamRequest);
-        RatedRequest berlinRequest = new RatedRequest("berlin_query", testQuery, indices, types, createRelevant("1"));
+        RatedRequest berlinRequest = new RatedRequest(
+                "berlin_query", createRelevant("1"), testQuery);
+        berlinRequest.setIndices(indices);
+        berlinRequest.setTypes(types);
         berlinRequest.setSummaryFields(Arrays.asList(new String[]{ "text", "title" }));
+        
         specifications.add(berlinRequest);
 
         Precision metric = new Precision();
@@ -135,11 +143,18 @@ public class RankEvalRequestIT  extends ESIntegTestCase {
         List<RatedRequest> specifications = new ArrayList<>();
         SearchSourceBuilder amsterdamQuery = new SearchSourceBuilder();
         amsterdamQuery.query(new MatchAllQueryBuilder());
-        specifications.add(new RatedRequest("amsterdam_query", amsterdamQuery, indices, types, createRelevant("2", "3", "4", "5")));
+        RatedRequest amsterdamRequest = new RatedRequest("amsterdam_query", createRelevant("2", "3", "4", "5"), amsterdamQuery);
+        amsterdamRequest.setIndices(indices);
+        amsterdamRequest.setTypes(types);
+        specifications.add(amsterdamRequest);
+
         SearchSourceBuilder brokenQuery = new SearchSourceBuilder();
         RangeQueryBuilder brokenRangeQuery = new RangeQueryBuilder("text").timeZone("CET");
         brokenQuery.query(brokenRangeQuery);
-        specifications.add(new RatedRequest("broken_query", brokenQuery, indices, types, createRelevant("1")));
+        RatedRequest brokenRequest = new RatedRequest("broken_query", createRelevant("1"), brokenQuery);
+        brokenRequest.setIndices(indices);
+        brokenRequest.setTypes(types);
+        specifications.add(brokenRequest);
 
         RankEvalSpec task = new RankEvalSpec(specifications, new Precision());
 

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalSpecTests.java

@@ -34,6 +34,7 @@ import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.search.SearchModule;
 import org.elasticsearch.search.SearchRequestParsers;
 import org.elasticsearch.search.aggregations.AggregatorParsers;
+import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.suggest.Suggesters;
 import org.elasticsearch.test.ESTestCase;
 import org.junit.AfterClass;
@@ -41,9 +42,12 @@ import org.junit.BeforeClass;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Supplier;
 
 import static java.util.Collections.emptyList;
 
@@ -70,24 +74,16 @@ public class RankEvalSpecTests extends ESTestCase {
         searchRequestParsers = null;
     }
 
-    public void testRoundtripping() throws IOException {
-        List<String> indices = new ArrayList<>();
-        int size = randomIntBetween(0, 20);
+    private static <T> List<T> randomList(Supplier<T> randomSupplier) {
+        List<T> result = new ArrayList<>();
+        int size = randomIntBetween(1, 20);
         for (int i = 0; i < size; i++) {
-            indices.add(randomAsciiOfLengthBetween(0, 50));
-        }
-
-        List<String> types = new ArrayList<>();
-        size = randomIntBetween(0, 20);
-        for (int i = 0; i < size; i++) {
-            types.add(randomAsciiOfLengthBetween(0, 50));
-        }
-        List<RatedRequest> specs = new ArrayList<>();
-        size = randomIntBetween(1, 2); // TODO I guess requests with no query spec should be rejected...
-        for (int i = 0; i < size; i++) {
-            specs.add(RatedRequestsTests.createTestItem(indices, types));
+            result.add(randomSupplier.get());
         }
+        return result;
+    }
 
+    private RankEvalSpec createTestItem() throws IOException {
         RankedListQualityMetric metric;
         if (randomBoolean()) {
             metric = PrecisionTests.createTestItem();
@@ -95,8 +91,8 @@ public class RankEvalSpecTests extends ESTestCase {
             metric = DiscountedCumulativeGainTests.createTestItem();
         }
 
-        RankEvalSpec testItem = new RankEvalSpec(specs, metric);
-
+        Script template = null;
+        List<RatedRequest> ratedRequests = null;
         if (randomBoolean()) {
             final Map<String, Object> params = randomBoolean() ? Collections.emptyMap() : Collections.singletonMap("key", "value");
             ScriptType scriptType = randomFrom(ScriptType.values());
@@ -112,9 +108,25 @@ public class RankEvalSpecTests extends ESTestCase {
                 script = randomAsciiOfLengthBetween(1, 5);
             }
 
-            testItem.setTemplate(new Script(scriptType, randomFrom("_lang1", "_lang2"), script, params));
+            template = new Script(scriptType, randomFrom("_lang1", "_lang2"), script, params);
+
+            Map<String, Object> templateParams = new HashMap<>();
+            templateParams.put("key", "value");
+            RatedRequest ratedRequest = new RatedRequest(
+                    "id", Arrays.asList(RatedDocumentTests.createRatedDocument()), templateParams);
+            ratedRequests = Arrays.asList(ratedRequest);
+        } else {
+            RatedRequest ratedRequest = new RatedRequest(
+                    "id", Arrays.asList(RatedDocumentTests.createRatedDocument()), new SearchSourceBuilder());
+            ratedRequests = Arrays.asList(ratedRequest);
         }
 
+        return new RankEvalSpec(ratedRequests, metric, template); 
+    }
+
+    public void testRoundtripping() throws IOException {
+        RankEvalSpec testItem = createTestItem();
+
         XContentBuilder shuffled = ESTestCase.shuffleXContent(testItem.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS));
         XContentParser itemParser = XContentHelper.createParser(shuffled.bytes());
 
@@ -122,12 +134,34 @@ public class RankEvalSpecTests extends ESTestCase {
         RankEvalContext rankContext = new RankEvalContext(ParseFieldMatcher.STRICT, queryContext,
                 searchRequestParsers, null);
 
-        RankEvalSpec parsedItem = RankEvalSpec.parse(itemParser, rankContext, false);
+        RankEvalSpec parsedItem = RankEvalSpec.parse(itemParser, rankContext);
         // IRL these come from URL parameters - see RestRankEvalAction
-        parsedItem.getSpecifications().stream().forEach(e -> {e.setIndices(indices); e.setTypes(types);});
+        // TODO Do we still need this? parsedItem.getRatedRequests().stream().forEach(e -> {e.setIndices(indices); e.setTypes(types);});
         assertNotSame(testItem, parsedItem);
         assertEquals(testItem, parsedItem);
         assertEquals(testItem.hashCode(), parsedItem.hashCode());
     }
 
+    public void testMissingRatedRequestsFailsParsing() {
+        RankedListQualityMetric metric = new Precision();
+        expectThrows(IllegalStateException.class, () -> new RankEvalSpec(new ArrayList<>(), metric));
+        expectThrows(IllegalStateException.class, () -> new RankEvalSpec(null, metric));
+    }
+    
+    public void testMissingMetricFailsParsing() {
+        List<String> strings = Arrays.asList("value");
+        List<RatedRequest> ratedRequests = randomList(() -> RatedRequestsTests.createTestItem(strings, strings));
+        expectThrows(IllegalStateException.class, () -> new RankEvalSpec(ratedRequests, null));
+    }
+
+    public void testMissingTemplateAndSearchRequestFailsParsing() {
+        List<RatedDocument> ratedDocs = Arrays.asList(new RatedDocument(new DocumentKey("index1", "type1", "id1"), 1));
+        Map<String, Object> params = new HashMap<>();
+        params.put("key", "value");
+
+        RatedRequest request = new RatedRequest("id", ratedDocs, params);
+        List<RatedRequest> ratedRequests = Arrays.asList(request);
+        
+        expectThrows(IllegalStateException.class, () -> new RankEvalSpec(ratedRequests, new Precision()));
+    }
 }

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java

@@ -72,11 +72,8 @@ public class RatedRequestsTests extends ESTestCase {
     }
 
     public static RatedRequest createTestItem(List<String> indices, List<String> types) {
-        String specId = randomAsciiOfLength(50);
+        String requestId = randomAsciiOfLength(50);
 
-        SearchSourceBuilder testRequest = new SearchSourceBuilder();
-        testRequest.size(randomInt());
-        testRequest.query(new MatchAllQueryBuilder());
 
         List<RatedDocument> ratedDocs = new ArrayList<>();
         int size = randomIntBetween(0, 2);
@@ -84,16 +81,17 @@ public class RatedRequestsTests extends ESTestCase {
             ratedDocs.add(RatedDocumentTests.createRatedDocument());
         }
 
-        RatedRequest ratedRequest = new RatedRequest(specId, testRequest, indices, types, ratedDocs);
-
-
+        Map<String, Object> params = new HashMap<>();
+        SearchSourceBuilder testRequest = null;
         if (randomBoolean()) {
-            Map<String, Object> params = new HashMap<>();
             int randomSize = randomIntBetween(1, 10);
             for (int i = 0; i < randomSize; i++) {
                 params.put(randomAsciiOfLengthBetween(1, 10), randomAsciiOfLengthBetween(1, 10));
             }
-            ratedRequest.setParams(params);
+        } else {
+            testRequest = new SearchSourceBuilder();
+            testRequest.size(randomInt());
+            testRequest.query(new MatchAllQueryBuilder());
         }
 
         List<String> summaryFields = new ArrayList<>();
@@ -101,7 +99,21 @@ public class RatedRequestsTests extends ESTestCase {
         for (int i = 0; i < numSummaryFields; i++) {
             summaryFields.add(randomAsciiOfLength(5));
         }
-        ratedRequest.setSummaryFields(summaryFields);
+
+        RatedRequest ratedRequest = null; 
+        if (params.size() == 0) {
+            ratedRequest = new RatedRequest(requestId, ratedDocs, testRequest);
+            ratedRequest.setIndices(indices);
+            ratedRequest.setTypes(types);
+            ratedRequest.setSummaryFields(summaryFields);
+        } else {
+            ratedRequest = new RatedRequest(requestId, ratedDocs, params);
+            ratedRequest.setIndices(indices);
+            ratedRequest.setTypes(types);
+            ratedRequest.setSummaryFields(summaryFields);
+        }
+
+
         return ratedRequest;
     }
 
@@ -181,53 +193,101 @@ public class RatedRequestsTests extends ESTestCase {
     }
 
     private RatedRequest mutateTestItem(RatedRequest original) {
-        String specId = original.getSpecId();
-        int size = original.getTestRequest().size();
+        String id = original.getId();
+        SearchSourceBuilder testRequest = original.getTestRequest();
         List<RatedDocument> ratedDocs = original.getRatedDocs();
         List<String> indices = original.getIndices();
         List<String> types = original.getTypes();
         Map<String, Object> params = original.getParams();
         List<String> summaryFields = original.getSummaryFields();
 
-        SearchSourceBuilder testRequest = new SearchSourceBuilder();
-        testRequest.size(size);
-        testRequest.query(new MatchAllQueryBuilder());
+        int mutate = randomIntBetween(0, 6);
+        switch (mutate) {
+            case 0:
+                id = randomValueOtherThan(id, () -> randomAsciiOfLength(10));
+                break;
+            case 1:
+                int size = randomValueOtherThan(testRequest.size(), () -> randomInt());
+                testRequest = new SearchSourceBuilder();
+                testRequest.size(size);
+                testRequest.query(new MatchAllQueryBuilder());
+                break;
+            case 2:
+                ratedDocs = Arrays.asList(
+                        randomValueOtherThanMany(ratedDocs::contains, () -> RatedDocumentTests.createRatedDocument()));
+                break;
+            case 3:
+                indices = Arrays.asList(randomValueOtherThanMany(indices::contains, () -> randomAsciiOfLength(10)));
+                break;
+            case 4:
+                types =  Arrays.asList(randomValueOtherThanMany(types::contains, () -> randomAsciiOfLength(10)));
+                break;
+            case 5:
+                params = new HashMap<>();
+                params.putAll(params);
+                params.put("one_more_key", "one_more_value");
+                break;
+            case 6:
+                summaryFields = Arrays.asList(randomValueOtherThanMany(summaryFields::contains, () -> randomAsciiOfLength(10)));
+                break;
+            default:
+                throw new IllegalStateException("Requested to modify more than available parameters.");
+        }
 
-        RatedRequest ratedRequest = new RatedRequest(specId, testRequest, indices, types, ratedDocs);
+        RatedRequest ratedRequest = new RatedRequest(id, ratedDocs, testRequest, params);
         ratedRequest.setIndices(indices);
         ratedRequest.setTypes(types);
-        ratedRequest.setParams(params);
         ratedRequest.setSummaryFields(summaryFields);
 
-        List<Runnable> mutators = new ArrayList<>();
-        mutators.add(() -> ratedRequest.setSpecId(randomValueOtherThan(specId, () -> randomAsciiOfLength(10))));
-        mutators.add(() -> ratedRequest.getTestRequest().size(randomValueOtherThan(size, () -> randomInt())));
-        mutators.add(() -> ratedRequest.setRatedDocs(
-                Arrays.asList(randomValueOtherThanMany(ratedDocs::contains, () -> RatedDocumentTests.createRatedDocument()))));
-        mutators.add(() -> ratedRequest.setIndices(
-                Arrays.asList(randomValueOtherThanMany(indices::contains, () -> randomAsciiOfLength(10)))));
-
-        HashMap<String, Object> modified = new HashMap<>();
-        modified.putAll(params);
-        modified.put("one_more_key", "one_more_value");
-        mutators.add(() -> ratedRequest.setParams(modified));
-
-        mutators.add(() -> ratedRequest.setSummaryFields(
-                Arrays.asList(randomValueOtherThanMany(summaryFields::contains, () -> randomAsciiOfLength(10)))));
-
-        
-        randomFrom(mutators).run();
         return ratedRequest;
     }
 
     public void testDuplicateRatedDocThrowsException() {
-        RatedRequest request = createTestItem(Arrays.asList("index"), Arrays.asList("type"));
         List<RatedDocument> ratedDocs = Arrays.asList(new RatedDocument(new DocumentKey("index1", "type1", "id1"), 1),
                 new RatedDocument(new DocumentKey("index1", "type1", "id1"), 5));
-        IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> request.setRatedDocs(ratedDocs));
+
+        // search request set, no summary fields
+        IllegalArgumentException ex = expectThrows(
+                IllegalArgumentException.class,
+                () -> new RatedRequest("id", ratedDocs, new SearchSourceBuilder()));
         assertEquals(
                 "Found duplicate rated document key [{ \"_index\" : \"index1\", \"_type\" : \"type1\", \"_id\" : \"id1\"}]",
                 ex.getMessage());
+        // templated path, no summary fields
+        Map<String, Object> params = new HashMap<>();
+        params.put("key", "value");
+        ex = expectThrows(
+                IllegalArgumentException.class,
+                () -> new RatedRequest("id", ratedDocs, params));
+        assertEquals(
+                "Found duplicate rated document key [{ \"_index\" : \"index1\", \"_type\" : \"type1\", \"_id\" : \"id1\"}]",
+                ex.getMessage());
+    }
+    
+    public void testNullSummaryFieldsTreatment() {
+        List<RatedDocument> ratedDocs = Arrays.asList(new RatedDocument(new DocumentKey("index1", "type1", "id1"), 1));
+        RatedRequest request = new RatedRequest("id", ratedDocs, new SearchSourceBuilder());
+        expectThrows(IllegalArgumentException.class, () -> request.setSummaryFields(null));
+    }
+    
+    public void testNullParamsTreatment() {
+        List<RatedDocument> ratedDocs = Arrays.asList(new RatedDocument(new DocumentKey("index1", "type1", "id1"), 1));
+        RatedRequest request = new RatedRequest("id", ratedDocs, new SearchSourceBuilder(), null);
+        assertNotNull(request.getParams());
+    }
+    
+    public void testSettingParamsAndRequestThrows() {
+        List<RatedDocument> ratedDocs = Arrays.asList(new RatedDocument(new DocumentKey("index1", "type1", "id1"), 1));
+        Map<String, Object> params = new HashMap<>();
+        params.put("key", "value");
+        expectThrows(IllegalArgumentException.class,
+                () -> new RatedRequest("id", ratedDocs, new SearchSourceBuilder(), params));
+    }
+    
+    public void testSettingNeitherParamsNorRequestThrows() {
+        List<RatedDocument> ratedDocs = Arrays.asList(new RatedDocument(new DocumentKey("index1", "type1", "id1"), 1));
+        expectThrows(IllegalArgumentException.class, () -> new RatedRequest("id", ratedDocs, null, null));
+        expectThrows(IllegalArgumentException.class, () -> new RatedRequest("id", ratedDocs, null, new HashMap<>()));
     }
 
     public void testParseFromXContent() throws IOException {
@@ -256,7 +316,7 @@ public class RatedRequestsTests extends ESTestCase {
         RankEvalContext rankContext = new RankEvalContext(ParseFieldMatcher.STRICT, queryContext,
                 searchRequestParsers, null);
         RatedRequest specification = RatedRequest.fromXContent(parser, rankContext);
-        assertEquals("my_qa_query", specification.getSpecId());
+        assertEquals("my_qa_query", specification.getId());
         assertNotNull(specification.getTestRequest());
         List<RatedDocument> ratedDocs = specification.getRatedDocs();
         assertEquals(3, ratedDocs.size());

qa/smoke-test-rank-eval-with-mustache/src/test/java/org/elasticsearch/smoketest/SmokeMultipleTemplatesIT.java

@@ -80,26 +80,29 @@ public class SmokeMultipleTemplatesIT  extends ESIntegTestCase {
         List<String> types = Arrays.asList(new String[] { "testtype" });
 
         List<RatedRequest> specifications = new ArrayList<>();
-        RatedRequest amsterdamRequest = new RatedRequest("amsterdam_query", null, indices, types, createRelevant("2", "3", "4", "5"));
         Map<String, Object> ams_params = new HashMap<>();
         ams_params.put("querystring", "amsterdam");
-        amsterdamRequest.setParams(ams_params);
+        RatedRequest amsterdamRequest = new RatedRequest("amsterdam_query", createRelevant("2", "3", "4", "5"), ams_params);
+        amsterdamRequest.setIndices(indices);
+        amsterdamRequest.setTypes(types);
+
         specifications.add(amsterdamRequest);
 
-        RatedRequest berlinRequest = new RatedRequest("berlin_query", null, indices, types, createRelevant("1"));
         Map<String, Object> berlin_params = new HashMap<>();
         berlin_params.put("querystring", "berlin");
-        berlinRequest.setParams(berlin_params);
+        RatedRequest berlinRequest = new RatedRequest("berlin_query", createRelevant("1"), berlin_params);
+        berlinRequest.setIndices(indices);
+        berlinRequest.setTypes(types);
         specifications.add(berlinRequest);
 
         Precision metric = new Precision();
-        RankEvalSpec task = new RankEvalSpec(specifications, metric);
-        task.setTemplate(
+
+        Script template = 
                 new Script(
                         ScriptType.INLINE,
                         "mustache", "{\"query\": {\"match\": {\"text\": \"{{querystring}}\"}}}",
-                        new HashMap<>()));
-
+                        new HashMap<>());
+        RankEvalSpec task = new RankEvalSpec(specifications, metric, template);
         RankEvalRequestBuilder builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, new RankEvalRequest());
         builder.setRankEvalSpec(task);
 

qa/smoke-test-rank-eval-with-mustache/src/test/resources/rest-api-spec/test/rank-eval/30_template.yaml

@@ -39,7 +39,7 @@
       indices.refresh: {}
 
   - do:
-      rank_eval_template:
+      rank_eval:
         body: {
           "template": { 
             "inline": "{\"query\": { \"match\" : {\"text\" : \"{{query_string}}\" }}}"

rest-api-spec/src/main/resources/rest-api-spec/api/rank_eval_template.json

@@ -1,25 +0,0 @@
-{ 
-  "rank_eval_template": { 
-    "documentation": "https://www.elastic.co/guide/en/elasticsearch/reference/master/docs-rank-eval.html", 
-    "methods": ["POST"], 
-    "url": { 
-      "path": "/_rank_eval/template", 
-      "paths": ["/_rank_eval/template", "/{index}/_rank_eval/template", "/{index}/{type}/_rank_eval/template"], 
-      "parts": {
-        "index": {
-         "type": "list",
-         "description" : "A comma-separated list of index names to search; use `_all` or empty string to perform the operation on all indices"
-        }, 
-        "type": {
-          "type" : "list",
-          "description" : "A comma-separated list of document types to search; leave empty to perform the operation on all types"
-        }
-     },
-     "params": {}
-    }, 
-    "body": { 
-      "description": "The search definition using the Query DSL and the prototype for the eval request.", 
-      "required": true 
-    } 
-  } 
-}