diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalSpec.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalSpec.java index d8fd26c1152..4626b604f71 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalSpec.java +++ b/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 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 specs, RankedListQualityMetric metric) { - this.ratedRequests = specs; - this.metric = metric; + public RankEvalSpec(Collection 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 getSpecifications() { + public Collection getRatedRequests() { return ratedRequests; } - /** Set the list of intent to query translation specifications to evaluate. */ - public void setSpecifications(Collection 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 PARSER = new ObjectParser<>("rank_eval", RankEvalSpec::new); + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser PARSER = + new ConstructingObjectParser<>("rank_eval", + a -> new RankEvalSpec((Collection) 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 diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java index 3d334193ec8..6c61ade5c6a 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java +++ b/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 indices = new ArrayList<>(); private List types = new ArrayList<>(); private List summaryFields = new ArrayList<>(); /** Collection of rated queries for this query QA specification.*/ private List 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 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 ratedDocs, SearchSourceBuilder testRequest, Map 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 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 ratedDocs, Map params) { + this(id, ratedDocs, null, params); } - public RatedRequest(String specId, SearchSourceBuilder testRequest, List indices, List types, - List ratedDocs) { - this.specId = specId; - this.testRequest = testRequest; - this.indices = indices; - this.types = types; - setRatedDocs(ratedDocs); + public RatedRequest(String id, List 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 indices) { + this.indices = indices; } public List getIndices() { return indices; } - public void setIndices(List indices) { - this.indices = indices; + public void setTypes(List types) { + this.types = types; } public List getTypes() { return types; } - public void setTypes(List 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 ratedDocs) { - Set 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 params) { - this.params = params; - } - public Map getParams() { return this.params; } - public void setSummaryFields(List fields) { - this.summaryFields = fields; - } - /** Returns a list of fields that are included in the docs summary of matched documents. */ public List getSummaryFields() { return summaryFields; } + + public void setSummaryFields(List 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 PARSER = new ObjectParser<>("requests", RatedRequest::new); + + private static final ConstructingObjectParser PARSER = + new ConstructingObjectParser<>("requests", a -> new RatedRequest( + (String) a[0], (List) a[1], (SearchSourceBuilder) a[2], (Map) 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); } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RestRankEvalAction.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RestRankEvalAction.java index 36d260b2efa..fb042e1a8d6 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RestRankEvalAction.java +++ b/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 indices = Arrays.asList(Strings.splitStringByCommaToArray(request.param("index"))); List 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); }; diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java index 7d73cf5d9ca..9359e72c6e2 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java @@ -79,7 +79,7 @@ public class TransportRankEvalAction extends HandledTransportAction listener) { RankEvalSpec qualityTask = request.getRankEvalSpec(); - Collection specifications = qualityTask.getSpecifications(); + Collection specifications = qualityTask.getRatedRequests(); AtomicInteger responseCounter = new AtomicInteger(specifications.size()); Map partialResults = new ConcurrentHashMap<>(specifications.size()); Map errors = new ConcurrentHashMap<>(specifications.size()); @@ -88,34 +88,36 @@ public class TransportRankEvalAction extends HandledTransportAction()); } - 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 params = querySpecification.getParams(); + SearchSourceBuilder ratedSearchSource = ratedRequest.getTestRequest(); + if (ratedSearchSource == null) { + Map 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 summaryFields = querySpecification.getSummaryFields(); + List 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 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 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()); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalSpecTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalSpecTests.java index 6a344994a5a..b2a0a42bb90 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalSpecTests.java +++ b/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 indices = new ArrayList<>(); - int size = randomIntBetween(0, 20); + private static List randomList(Supplier randomSupplier) { + List result = new ArrayList<>(); + int size = randomIntBetween(1, 20); for (int i = 0; i < size; i++) { - indices.add(randomAsciiOfLengthBetween(0, 50)); - } - - List types = new ArrayList<>(); - size = randomIntBetween(0, 20); - for (int i = 0; i < size; i++) { - types.add(randomAsciiOfLengthBetween(0, 50)); - } - List 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 ratedRequests = null; if (randomBoolean()) { final Map 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 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 strings = Arrays.asList("value"); + List ratedRequests = randomList(() -> RatedRequestsTests.createTestItem(strings, strings)); + expectThrows(IllegalStateException.class, () -> new RankEvalSpec(ratedRequests, null)); + } + + public void testMissingTemplateAndSearchRequestFailsParsing() { + List ratedDocs = Arrays.asList(new RatedDocument(new DocumentKey("index1", "type1", "id1"), 1)); + Map params = new HashMap<>(); + params.put("key", "value"); + + RatedRequest request = new RatedRequest("id", ratedDocs, params); + List ratedRequests = Arrays.asList(request); + + expectThrows(IllegalStateException.class, () -> new RankEvalSpec(ratedRequests, new Precision())); + } } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java index e43b0e041d0..eddb8afc859 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java +++ b/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 indices, List types) { - String specId = randomAsciiOfLength(50); + String requestId = randomAsciiOfLength(50); - SearchSourceBuilder testRequest = new SearchSourceBuilder(); - testRequest.size(randomInt()); - testRequest.query(new MatchAllQueryBuilder()); List 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 params = new HashMap<>(); + SearchSourceBuilder testRequest = null; if (randomBoolean()) { - Map 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 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 ratedDocs = original.getRatedDocs(); List indices = original.getIndices(); List types = original.getTypes(); Map params = original.getParams(); List 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 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 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 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 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 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 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 ratedDocs = Arrays.asList(new RatedDocument(new DocumentKey("index1", "type1", "id1"), 1)); + Map params = new HashMap<>(); + params.put("key", "value"); + expectThrows(IllegalArgumentException.class, + () -> new RatedRequest("id", ratedDocs, new SearchSourceBuilder(), params)); + } + + public void testSettingNeitherParamsNorRequestThrows() { + List 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 ratedDocs = specification.getRatedDocs(); assertEquals(3, ratedDocs.size()); diff --git a/qa/smoke-test-rank-eval-with-mustache/src/test/java/org/elasticsearch/smoketest/SmokeMultipleTemplatesIT.java b/qa/smoke-test-rank-eval-with-mustache/src/test/java/org/elasticsearch/smoketest/SmokeMultipleTemplatesIT.java index 02213642d39..536249c0110 100644 --- a/qa/smoke-test-rank-eval-with-mustache/src/test/java/org/elasticsearch/smoketest/SmokeMultipleTemplatesIT.java +++ b/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 types = Arrays.asList(new String[] { "testtype" }); List specifications = new ArrayList<>(); - RatedRequest amsterdamRequest = new RatedRequest("amsterdam_query", null, indices, types, createRelevant("2", "3", "4", "5")); Map 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 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); diff --git a/qa/smoke-test-rank-eval-with-mustache/src/test/resources/rest-api-spec/test/rank-eval/30_template.yaml b/qa/smoke-test-rank-eval-with-mustache/src/test/resources/rest-api-spec/test/rank-eval/30_template.yaml index 8ab994878a1..00af4f9f41e 100644 --- a/qa/smoke-test-rank-eval-with-mustache/src/test/resources/rest-api-spec/test/rank-eval/30_template.yaml +++ b/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}}\" }}}" diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/rank_eval_template.json b/rest-api-spec/src/main/resources/rest-api-spec/api/rank_eval_template.json deleted file mode 100644 index abb21c273e4..00000000000 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/rank_eval_template.json +++ /dev/null @@ -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 - } - } -}