From a2a92b96297a8ac0012b4d8921b3bf4623c4043d Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 24 Aug 2016 14:21:24 +0200 Subject: [PATCH 01/19] Add roundtrip xcontent test to DiscountedCumulativeGainAt This factors the roundtripping out of RatedDocumentTests. Makes RankedListQualityMetric and RatedDocument implement FromXContenBuilder to be able to do the aforementioned refactoring in a generic way. Adds a roundtrip test to DiscountedCumulativeGainAt. Open questions: DiscountedCumulativeGain didn't have a constructor that accepted all possible parameters as arguments. Added one. I guess we still want to keep the one that only requires the position argument? To make roundtripping work I had to change the NAME parameter when generating XContent for DiscountedCumulativeGainAt - all remaining unit tests seem to be passing (haven't checked the REST tests yet) - need to figure out why that was there to begin with. --- .../rankeval/DiscountedCumulativeGainAt.java | 47 ++++++++++++++++- .../index/rankeval/PrecisionAtN.java | 14 ++++- .../rankeval/RankedListQualityMetric.java | 5 +- .../index/rankeval/RatedDocument.java | 22 ++++++-- .../index/rankeval/RatedDocumentKey.java | 5 +- .../index/rankeval/ReciprocalRank.java | 14 ++++- .../DiscountedCumulativeGainAtTests.java | 12 ++++- .../index/rankeval/RatedDocumentTests.java | 30 ++--------- .../rankeval/XContentRoundtripTestCase.java | 51 +++++++++++++++++++ 9 files changed, 160 insertions(+), 40 deletions(-) create mode 100644 modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTestCase.java diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java index e620fa63d53..18e79bec3b9 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -35,8 +36,9 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; -public class DiscountedCumulativeGainAt extends RankedListQualityMetric { +public class DiscountedCumulativeGainAt extends RankedListQualityMetric { /** rank position up to which to check results. */ private int position; @@ -83,6 +85,17 @@ public class DiscountedCumulativeGainAt extends RankedListQualityMetric { this.position = position; } + /** + * @param position number of top results to check against a given set of relevant results. Must be positive. // TODO is there a way to enforce this? + * @param normalize If set to true, dcg will be normalized (ndcg) See https://en.wikipedia.org/wiki/Discounted_cumulative_gain + * @param unknownDocRating the rating for docs the user hasn't supplied an explicit rating for + * */ + public DiscountedCumulativeGainAt(int position, boolean normalize, Integer unknownDocRating) { + this.position = position; + this.normalize = normalize; + this.unknownDocRating = unknownDocRating; + } + /** * Return number of search results to check for quality metric. */ @@ -178,13 +191,24 @@ public class DiscountedCumulativeGainAt extends RankedListQualityMetric { PARSER.declareInt(DiscountedCumulativeGainAt::setUnknownDocRating, UNKNOWN_DOC_RATING_FIELD); } + @Override + public DiscountedCumulativeGainAt fromXContent(XContentParser parser, ParseFieldMatcher matcher) { + return DiscountedCumulativeGainAt.fromXContent(parser, new ParseFieldMatcherSupplier() { + @Override + public ParseFieldMatcher getParseFieldMatcher() { + return matcher; + } + }); + } + public static DiscountedCumulativeGainAt fromXContent(XContentParser parser, ParseFieldMatcherSupplier matcher) { return PARSER.apply(parser, matcher); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(NAME); + //builder.startObject(NAME); // TODO roundtrip xcontent only works w/o this, wtf? + builder.startObject(); builder.field(SIZE_FIELD.getPreferredName(), this.position); builder.field(NORMALIZE_FIELD.getPreferredName(), this.normalize); if (unknownDocRating != null) { @@ -193,4 +217,23 @@ public class DiscountedCumulativeGainAt extends RankedListQualityMetric { builder.endObject(); return builder; } + + @Override + public final boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + DiscountedCumulativeGainAt other = (DiscountedCumulativeGainAt) obj; + return Objects.equals(position, other.position) && + Objects.equals(normalize, other.normalize) && + Objects.equals(unknownDocRating, other.unknownDocRating); + } + + @Override + public final int hashCode() { + return Objects.hash(getClass(), position, normalize, unknownDocRating); + } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java index ba571c576ef..c9ad3deb759 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -40,7 +41,7 @@ import javax.naming.directory.SearchResult; * * Documents of unkonwn quality are ignored in the precision at n computation and returned by document id. * */ -public class PrecisionAtN extends RankedListQualityMetric { +public class PrecisionAtN extends RankedListQualityMetric { /** Number of results to check against a given set of relevant results. */ private int n; @@ -90,6 +91,17 @@ public class PrecisionAtN extends RankedListQualityMetric { PARSER.declareInt(ConstructingObjectParser.constructorArg(), SIZE_FIELD); } + @Override + public PrecisionAtN fromXContent(XContentParser parser, ParseFieldMatcher matcher) { + return PrecisionAtN.fromXContent(parser, new ParseFieldMatcherSupplier() { + + @Override + public ParseFieldMatcher getParseFieldMatcher() { + return matcher; + } + }); + } + public static PrecisionAtN fromXContent(XContentParser parser, ParseFieldMatcherSupplier matcher) { return PARSER.apply(parser, matcher); } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java index e423ab3533c..30793d564e3 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java @@ -23,6 +23,7 @@ import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.NamedWriteable; +import org.elasticsearch.common.xcontent.FromXContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; @@ -38,7 +39,9 @@ import java.util.List; * * RelevancyLevel specifies the type of object determining the relevancy level of some known docid. * */ -public abstract class RankedListQualityMetric extends ToXContentToBytes implements NamedWriteable { +public abstract class RankedListQualityMetric + extends ToXContentToBytes + implements NamedWriteable, FromXContentBuilder { /** * Returns a single metric representing the ranking quality of a set of returned documents diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java index 8e7c1aac888..c5faecb986e 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java @@ -21,11 +21,14 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.common.ParseFieldMatcherSupplier; 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.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.FromXContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -35,12 +38,12 @@ import java.util.Objects; /** * A document ID and its rating for the query QA use case. * */ -public class RatedDocument extends ToXContentToBytes implements Writeable { +public class RatedDocument extends ToXContentToBytes implements Writeable, FromXContentBuilder { public static final ParseField RATING_FIELD = new ParseField("rating"); public static final ParseField KEY_FIELD = new ParseField("key"); - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("rated_document", + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("rated_document", a -> new RatedDocument((RatedDocumentKey) a[0], (Integer) a[1])); static { @@ -93,8 +96,19 @@ public class RatedDocument extends ToXContentToBytes implements Writeable { out.writeVInt(rating); } - public static RatedDocument fromXContent(XContentParser parser, RankEvalContext context) throws IOException { - return PARSER.apply(parser, context); + @Override + public RatedDocument fromXContent(XContentParser parser, ParseFieldMatcher parseFieldMatcher) throws IOException { + return RatedDocument.fromXContent(parser, new ParseFieldMatcherSupplier() { + + @Override + public ParseFieldMatcher getParseFieldMatcher() { + return parseFieldMatcher; + } + }); + } + + public static RatedDocument fromXContent(XContentParser parser, ParseFieldMatcherSupplier supplier) throws IOException { + return PARSER.apply(parser, supplier); } @Override diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java index d68149a2ca0..c10bc3f0ce0 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java @@ -21,6 +21,7 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -36,7 +37,7 @@ public class RatedDocumentKey extends ToXContentToBytes implements Writeable { public static final ParseField TYPE_FIELD = new ParseField("type"); public static final ParseField INDEX_FIELD = new ParseField("index"); - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("ratings", + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("ratings", a -> new RatedDocumentKey((String) a[0], (String) a[1], (String) a[2])); static { @@ -103,7 +104,7 @@ public class RatedDocumentKey extends ToXContentToBytes implements Writeable { out.writeString(docId); } - public static RatedDocumentKey fromXContent(XContentParser parser, RankEvalContext context) throws IOException { + public static RatedDocumentKey fromXContent(XContentParser parser, ParseFieldMatcherSupplier context) throws IOException { return PARSER.apply(parser, context); } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java index dd4e710859b..791e857eca8 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -42,7 +43,7 @@ import javax.naming.directory.SearchResult; /** * Evaluate reciprocal rank. * */ -public class ReciprocalRank extends RankedListQualityMetric { +public class ReciprocalRank extends RankedListQualityMetric { public static final String NAME = "reciprocal_rank"; public static final int DEFAULT_MAX_ACCEPTABLE_RANK = 10; @@ -140,6 +141,17 @@ public class ReciprocalRank extends RankedListQualityMetric { PARSER.declareInt(ReciprocalRank::setMaxAcceptableRank, MAX_RANK_FIELD); } + @Override + public ReciprocalRank fromXContent(XContentParser parser, ParseFieldMatcher matcher) { + return ReciprocalRank.fromXContent(parser, new ParseFieldMatcherSupplier() { + + @Override + public ParseFieldMatcher getParseFieldMatcher() { + return matcher; + } + }); + } + public static ReciprocalRank fromXContent(XContentParser parser, ParseFieldMatcherSupplier matcher) { return PARSER.apply(parser, matcher); } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java index 2221ee2a1e3..68207470382 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java @@ -26,7 +26,6 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.internal.InternalSearchHit; -import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; @@ -34,7 +33,7 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.ExecutionException; -public class DiscountedCumulativeGainAtTests extends ESTestCase { +public class DiscountedCumulativeGainAtTests extends XContentRoundtripTestCase { /** * Assuming the docs are ranked in the following order: @@ -121,4 +120,13 @@ public class DiscountedCumulativeGainAtTests extends ESTestCase { assertEquals(8, dcgAt.getPosition()); assertEquals(true, dcgAt.getNormalize()); } + + public void testXContentRoundtrip() throws IOException { + int position = randomIntBetween(0, 1000); + boolean normalize = randomBoolean(); + Integer unknownDocRating = new Integer(randomIntBetween(0, 1000)); + + DiscountedCumulativeGainAt testItem = new DiscountedCumulativeGainAt(position, normalize, unknownDocRating); + roundtrip(testItem); + } } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java index 6e48b72d38e..8cb4a194026 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java @@ -19,41 +19,17 @@ package org.elasticsearch.index.rankeval; -import org.elasticsearch.common.ParseFieldMatcher; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.test.ESTestCase; - import java.io.IOException; -public class RatedDocumentTests extends ESTestCase { +public class RatedDocumentTests extends XContentRoundtripTestCase { public void testXContentParsing() throws IOException { String index = randomAsciiOfLength(10); String type = randomAsciiOfLength(10); String docId = randomAsciiOfLength(10); int rating = randomInt(); + RatedDocument testItem = new RatedDocument(new RatedDocumentKey(index, type, docId), rating); - - XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); - if (randomBoolean()) { - builder.prettyPrint(); - } - testItem.toXContent(builder, ToXContent.EMPTY_PARAMS); - XContentBuilder shuffled = shuffleXContent(builder); - XContentParser itemParser = XContentHelper.createParser(shuffled.bytes()); - itemParser.nextToken(); - - RankEvalContext context = new RankEvalContext(ParseFieldMatcher.STRICT, null, null); - RatedDocument parsedItem = RatedDocument.fromXContent(itemParser, context); - assertNotSame(testItem, parsedItem); - assertEquals(testItem, parsedItem); - assertEquals(testItem.hashCode(), parsedItem.hashCode()); - + roundtrip(testItem); } - } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTestCase.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTestCase.java new file mode 100644 index 00000000000..689a42726ff --- /dev/null +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTestCase.java @@ -0,0 +1,51 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.rankeval; + +import org.elasticsearch.action.support.ToXContentToBytes; +import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.common.xcontent.FromXContentBuilder; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; + +public class XContentRoundtripTestCase> extends ESTestCase { + + public void roundtrip(T testItem) throws IOException { + XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); + if (randomBoolean()) { + builder.prettyPrint(); + } + testItem.toXContent(builder, ToXContent.EMPTY_PARAMS); + XContentBuilder shuffled = shuffleXContent(builder); + XContentParser itemParser = XContentHelper.createParser(shuffled.bytes()); + itemParser.nextToken(); + T parsedItem = testItem.fromXContent(itemParser, ParseFieldMatcher.STRICT); + assertNotSame(testItem, parsedItem); + assertEquals(testItem, parsedItem); + assertEquals(testItem.hashCode(), parsedItem.hashCode()); + } +} From 5c9cc1d453297ad8e706f7800398e7610c70ef1f Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 24 Aug 2016 14:39:12 +0200 Subject: [PATCH 02/19] Add roundtripping to PrecisionAtN --- .../index/rankeval/PrecisionAtN.java | 21 ++++++++++++++++++- .../index/rankeval/PrecisionAtNTests.java | 10 +++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java index c9ad3deb759..145270c15b1 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java @@ -33,6 +33,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Objects; import javax.naming.directory.SearchResult; @@ -165,10 +166,28 @@ public class PrecisionAtN extends RankedListQualityMetric { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(NAME); + //builder.startObject(NAME); TODO Why does roundtripping fail with the name? + builder.startObject(); builder.field(SIZE_FIELD.getPreferredName(), this.n); builder.endObject(); return builder; } + + @Override + public final boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + PrecisionAtN other = (PrecisionAtN) obj; + return Objects.equals(n, other.n); + } + + @Override + public final int hashCode() { + return Objects.hash(getClass(), n); + } } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java index d668b21630e..32b5c1cabc3 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java @@ -27,7 +27,6 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.rankeval.PrecisionAtN.Rating; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.internal.InternalSearchHit; -import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; @@ -38,7 +37,7 @@ import java.util.concurrent.ExecutionException; import static java.util.Collections.emptyList; -public class PrecisionAtNTests extends ESTestCase { +public class PrecisionAtNTests extends XContentRoundtripTestCase { public void testPrecisionAtFiveCalculation() throws IOException, InterruptedException, ExecutionException { List rated = new ArrayList<>(); @@ -111,4 +110,11 @@ public class PrecisionAtNTests extends ESTestCase { partialResults.add(new EvalQueryQuality(0.6, emptyList())); assertEquals(0.3, metric.combine(partialResults), Double.MIN_VALUE); } + + public void testXContentRoundtrip() throws IOException { + int position = randomIntBetween(0, 1000); + + PrecisionAtN testItem = new PrecisionAtN(position); + roundtrip(testItem); + } } From cebb0ba0d8eb3003041c1082363ae10165b8726b Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 24 Aug 2016 14:41:58 +0200 Subject: [PATCH 03/19] Add roundtripping to ReciprocalRank --- .../index/rankeval/PrecisionAtN.java | 3 +-- .../index/rankeval/ReciprocalRank.java | 22 +++++++++++++++++-- .../index/rankeval/ReciprocalRankTests.java | 12 ++++++++-- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java index 145270c15b1..e9fa325a03f 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java @@ -172,7 +172,7 @@ public class PrecisionAtN extends RankedListQualityMetric { builder.endObject(); return builder; } - + @Override public final boolean equals(Object obj) { if (this == obj) { @@ -189,5 +189,4 @@ public class PrecisionAtN extends RankedListQualityMetric { public final int hashCode() { return Objects.hash(getClass(), n); } - } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java index 791e857eca8..7b0576cd098 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java @@ -36,6 +36,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import javax.naming.directory.SearchResult; @@ -159,10 +160,27 @@ public class ReciprocalRank extends RankedListQualityMetric { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - builder.startObject(NAME); + //builder.startObject(NAME); builder.field(MAX_RANK_FIELD.getPreferredName(), this.maxAcceptableRank); - builder.endObject(); + //builder.endObject(); builder.endObject(); return builder; } + + @Override + public final boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + ReciprocalRank other = (ReciprocalRank) obj; + return Objects.equals(maxAcceptableRank, other.maxAcceptableRank); + } + + @Override + public final int hashCode() { + return Objects.hash(getClass(), maxAcceptableRank); + } } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java index b524e763dc7..b25faba52d4 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java @@ -24,8 +24,8 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.rankeval.PrecisionAtN.Rating; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.internal.InternalSearchHit; -import org.elasticsearch.test.ESTestCase; +import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -33,7 +33,7 @@ import java.util.Vector; import static java.util.Collections.emptyList; -public class ReciprocalRankTests extends ESTestCase { +public class ReciprocalRankTests extends XContentRoundtripTestCase { public void testMaxAcceptableRank() { ReciprocalRank reciprocalRank = new ReciprocalRank(); @@ -123,4 +123,12 @@ public class ReciprocalRankTests extends ESTestCase { EvalQueryQuality evaluation = reciprocalRank.evaluate(hits, ratedDocs); assertEquals(0.0, evaluation.getQualityLevel(), Double.MIN_VALUE); } + + public void testXContentRoundtrip() throws IOException { + int position = randomIntBetween(0, 1000); + + ReciprocalRank testItem = new ReciprocalRank(position); + roundtrip(testItem); + } + } From 5979802415424d159902a23c6e72ef39726b9e3b Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 24 Aug 2016 15:17:42 +0200 Subject: [PATCH 04/19] Add comment wrt to changed xcontent generation --- .../java/org/elasticsearch/index/rankeval/ReciprocalRank.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java index 7b0576cd098..a40ecc1012e 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java @@ -160,7 +160,7 @@ public class ReciprocalRank extends RankedListQualityMetric { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - //builder.startObject(NAME); + //builder.startObject(NAME); // TODO for roundtripping to work builder.field(MAX_RANK_FIELD.getPreferredName(), this.maxAcceptableRank); //builder.endObject(); builder.endObject(); From 94497871b5465b552b6bf91632887efdd51dc5bb Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 24 Aug 2016 15:17:58 +0200 Subject: [PATCH 05/19] Add roundtrip testing to QuerySpec --- .../index/rankeval/QuerySpec.java | 22 +++++++ .../index/rankeval/QuerySpecTests.java | 59 ++++++++++++++++++- .../index/rankeval/RatedDocumentTests.java | 9 ++- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/QuerySpec.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/QuerySpec.java index 92445bd4697..329839750b1 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/QuerySpec.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/QuerySpec.java @@ -33,6 +33,7 @@ import org.elasticsearch.search.builder.SearchSourceBuilder; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Objects; /** * Defines a QA specification: All end user supplied query intents will be mapped to the search request specified in this search request @@ -205,4 +206,25 @@ public class QuerySpec extends ToXContentToBytes implements Writeable { builder.endObject(); return builder; } + + @Override + public final boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + QuerySpec other = (QuerySpec) obj; + return Objects.equals(specId, other.specId) && + Objects.equals(testRequest, other.testRequest) && + Objects.equals(indices, other.indices) && + Objects.equals(types, other.types) && + Objects.equals(ratedDocs, other.ratedDocs); + } + + @Override + public final int hashCode() { + return Objects.hash(getClass(), specId, testRequest, indices.hashCode(), types.hashCode(), ratedDocs.hashCode()); + } } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java index 8d77e002a12..deec3f73b1a 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java @@ -22,19 +22,26 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ParseFieldRegistry; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.indices.query.IndicesQueriesRegistry; 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; import org.junit.BeforeClass; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import static java.util.Collections.emptyList; @@ -63,7 +70,55 @@ public class QuerySpecTests extends ESTestCase { searchRequestParsers = null; } - // TODO add some sort of roundtrip testing like we have now for queries? + public void testXContentRoundtrip() throws IOException { + String specId = randomAsciiOfLength(50); + + SearchSourceBuilder testRequest = new SearchSourceBuilder(); + testRequest.size(23); + testRequest.query(new MatchAllQueryBuilder()); + + List indices = new ArrayList<>(); + int size = randomIntBetween(0, 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 ratedDocs = new ArrayList<>(); + size = randomIntBetween(0, 20); + for (int i = 0; i < size; i++) { + ratedDocs.add(RatedDocumentTests.createTestItem()); + } + + + QuerySpec testItem = new QuerySpec(specId, testRequest, indices, types, ratedDocs); + + XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); + if (randomBoolean()) { + builder.prettyPrint(); + } + testItem.toXContent(builder, ToXContent.EMPTY_PARAMS); + XContentBuilder shuffled = shuffleXContent(builder); + XContentParser itemParser = XContentHelper.createParser(shuffled.bytes()); + itemParser.nextToken(); // TODO this could be the reason why the metric roundtrip tests failed + + QueryParseContext queryContext = new QueryParseContext(searchRequestParsers.queryParsers, itemParser, ParseFieldMatcher.STRICT); + RankEvalContext rankContext = new RankEvalContext(ParseFieldMatcher.STRICT, queryContext, + searchRequestParsers); + + QuerySpec parsedItem = QuerySpec.fromXContent(itemParser, rankContext); + parsedItem.setIndices(indices); // IRL these come from URL parameters - see RestRankEvalAction + parsedItem.setTypes(types); // IRL these come from URL parameters - see RestRankEvalAction + assertNotSame(testItem, parsedItem); + assertEquals(testItem, parsedItem); + assertEquals(testItem.hashCode(), parsedItem.hashCode()); + } + public void testParseFromXContent() throws IOException { String querySpecString = " {\n" + " \"id\": \"my_qa_query\",\n" @@ -99,4 +154,6 @@ public class QuerySpecTests extends ESTestCase { assertEquals("3", ratedDocs.get(2).getKey().getDocID()); assertEquals(1, ratedDocs.get(2).getRating()); } + + } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java index 8cb4a194026..5188093defb 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java @@ -23,13 +23,16 @@ import java.io.IOException; public class RatedDocumentTests extends XContentRoundtripTestCase { - public void testXContentParsing() throws IOException { + public static RatedDocument createTestItem() { String index = randomAsciiOfLength(10); String type = randomAsciiOfLength(10); String docId = randomAsciiOfLength(10); int rating = randomInt(); - RatedDocument testItem = new RatedDocument(new RatedDocumentKey(index, type, docId), rating); - roundtrip(testItem); + return new RatedDocument(new RatedDocumentKey(index, type, docId), rating); + } + + public void testXContentParsing() throws IOException { + roundtrip(createTestItem()); } } From 87367be4e787aca6796392b2d5575bcd23980fd0 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 24 Aug 2016 15:32:44 +0200 Subject: [PATCH 06/19] Add roundtrip testing to RatedDocumentKey --- .../index/rankeval/RatedDocumentKey.java | 17 ++++++++-- .../index/rankeval/RatedDocumentKeyTests.java | 34 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java index c10bc3f0ce0..3efcc7e16a4 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java @@ -21,18 +21,20 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; 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.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.FromXContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; import java.util.Objects; -public class RatedDocumentKey extends ToXContentToBytes implements Writeable { +public class RatedDocumentKey extends ToXContentToBytes implements Writeable, FromXContentBuilder { public static final ParseField DOC_ID_FIELD = new ParseField("doc_id"); public static final ParseField TYPE_FIELD = new ParseField("type"); public static final ParseField INDEX_FIELD = new ParseField("index"); @@ -103,7 +105,18 @@ public class RatedDocumentKey extends ToXContentToBytes implements Writeable { out.writeString(type); out.writeString(docId); } - + + @Override + public RatedDocumentKey fromXContent(XContentParser parser, ParseFieldMatcher matcher) throws IOException { + return RatedDocumentKey.fromXContent(parser, new ParseFieldMatcherSupplier() { + + @Override + public ParseFieldMatcher getParseFieldMatcher() { + return matcher; + } + }); + } + public static RatedDocumentKey fromXContent(XContentParser parser, ParseFieldMatcherSupplier context) throws IOException { return PARSER.apply(parser, context); } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java new file mode 100644 index 00000000000..1ec1bbbcc06 --- /dev/null +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java @@ -0,0 +1,34 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.rankeval; + +import java.io.IOException; + +public class RatedDocumentKeyTests extends XContentRoundtripTestCase { + + public void testXContentRoundtrip() throws IOException { + String index = randomAsciiOfLengthBetween(0, 10); + String type = randomAsciiOfLengthBetween(0, 10); + String docId = randomAsciiOfLengthBetween(0, 10); + + RatedDocumentKey testItem = new RatedDocumentKey(index, type, docId); + roundtrip(testItem); + } +} From f18e23eb51b89de16a2901f393a504835ad70984 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Thu, 25 Aug 2016 12:09:55 +0200 Subject: [PATCH 07/19] Fix test errors, add roundtrip testing to RankEvalSpec This adds roundtrip testing to RankEvalSpec, fixes issues introduced with the previous roundtrip tests, splits xcontent generation/parsing from actually checking the resulting objects to deal with e.g. all evaluation metrics needing some extra treatment. Renames QuerySpec to RatedRequest, renames newly introduced xcontent generation helper to conform with naming conventions. Fixes several lines that were too long, adds missing types where needed. --- .../rankeval/DiscountedCumulativeGainAt.java | 9 +- .../index/rankeval/PrecisionAtN.java | 3 +- .../index/rankeval/RankEvalSpec.java | 67 ++++++---- .../rankeval/RankedListQualityMetric.java | 6 +- .../index/rankeval/RatedDocument.java | 3 +- .../index/rankeval/RatedDocumentKey.java | 6 +- .../{QuerySpec.java => RatedRequest.java} | 22 ++-- .../index/rankeval/ReciprocalRank.java | 4 +- .../index/rankeval/RestRankEvalAction.java | 2 +- .../rankeval/TransportRankEvalAction.java | 8 +- .../DiscountedCumulativeGainAtTests.java | 17 ++- .../index/rankeval/PrecisionAtNTests.java | 17 ++- .../index/rankeval/QuerySpecTests.java | 25 ++-- .../index/rankeval/RankEvalRequestTests.java | 12 +- .../index/rankeval/RankEvalSpecTests.java | 119 ++++++++++++++++++ .../index/rankeval/RatedDocumentKeyTests.java | 11 +- .../index/rankeval/RatedDocumentTests.java | 12 +- .../index/rankeval/ReciprocalRankTests.java | 12 +- ...tCase.java => XContentRoundtripTests.java} | 11 +- 19 files changed, 275 insertions(+), 91 deletions(-) rename modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/{QuerySpec.java => RatedRequest.java} (90%) create mode 100644 modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalSpecTests.java rename modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/{XContentRoundtripTestCase.java => XContentRoundtripTests.java} (77%) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java index 18e79bec3b9..22d98663738 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java @@ -86,8 +86,10 @@ public class DiscountedCumulativeGainAt extends RankedListQualityMetric { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - //builder.startObject(NAME); TODO Why does roundtripping fail with the name? builder.startObject(); + builder.startObject(NAME); builder.field(SIZE_FIELD.getPreferredName(), this.n); builder.endObject(); + builder.endObject(); return builder; } 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 7f88104b4eb..0aece1d0aa1 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 @@ -32,6 +32,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Objects; /** * This class defines a ranking evaluation task including an id, a collection of queries to evaluate and the evaluation metric. @@ -43,9 +44,9 @@ import java.util.Collection; public class RankEvalSpec extends ToXContentToBytes implements Writeable { /** Collection of query specifications, that is e.g. search request templates to use for query translation. */ - private Collection specifications = new ArrayList<>(); + private Collection ratedRequests = new ArrayList<>(); /** Definition of the quality metric, e.g. precision at N */ - private RankedListQualityMetric eval; + private RankedListQualityMetric metric; /** a unique id for the whole QA task */ private String specId; @@ -53,34 +54,34 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { // TODO think if no args ctor is okay } - public RankEvalSpec(String specId, Collection specs, RankedListQualityMetric metric) { + public RankEvalSpec(String specId, Collection specs, RankedListQualityMetric metric) { this.specId = specId; - this.specifications = specs; - this.eval = metric; + this.ratedRequests = specs; + this.metric = metric; } public RankEvalSpec(StreamInput in) throws IOException { int specSize = in.readInt(); - specifications = new ArrayList<>(specSize); + ratedRequests = new ArrayList<>(specSize); for (int i = 0; i < specSize; i++) { - specifications.add(new QuerySpec(in)); + ratedRequests.add(new RatedRequest(in)); } - eval = in.readNamedWriteable(RankedListQualityMetric.class); + metric = in.readNamedWriteable(RankedListQualityMetric.class); specId = in.readString(); } @Override public void writeTo(StreamOutput out) throws IOException { - out.writeInt(specifications.size()); - for (QuerySpec spec : specifications) { + out.writeInt(ratedRequests.size()); + for (RatedRequest spec : ratedRequests) { spec.writeTo(out); } - out.writeNamedWriteable(eval); + out.writeNamedWriteable(metric); out.writeString(specId); } - public void setEval(RankedListQualityMetric eval) { - this.eval = eval; + public void setEval(RankedListQualityMetric eval) { + this.metric = eval; } public void setTaskId(String taskId) { @@ -92,23 +93,23 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { } /** Returns the precision at n configuration (containing level of n to consider).*/ - public RankedListQualityMetric getEvaluator() { - return eval; + public RankedListQualityMetric getEvaluator() { + return metric; } /** Sets the precision at n configuration (containing level of n to consider).*/ - public void setEvaluator(RankedListQualityMetric config) { - this.eval = config; + public void setEvaluator(RankedListQualityMetric config) { + this.metric = config; } /** Returns a list of intent to query translation specifications to evaluate. */ - public Collection getSpecifications() { - return specifications; + public Collection getSpecifications() { + return ratedRequests; } /** Set the list of intent to query translation specifications to evaluate. */ - public void setSpecifications(Collection specifications) { - this.specifications = specifications; + public void setSpecifications(Collection specifications) { + this.ratedRequests = specifications; } private static final ParseField SPECID_FIELD = new ParseField("spec_id"); @@ -127,7 +128,7 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { } , METRIC_FIELD); PARSER.declareObjectArray(RankEvalSpec::setSpecifications, (p, c) -> { try { - return QuerySpec.fromXContent(p, c); + return RatedRequest.fromXContent(p, c); } catch (IOException ex) { throw new ParsingException(p.getTokenLocation(), "error parsing rank request", ex); } @@ -139,11 +140,11 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { builder.startObject(); builder.field(SPECID_FIELD.getPreferredName(), this.specId); builder.startArray(REQUESTS_FIELD.getPreferredName()); - for (QuerySpec spec : this.specifications) { + for (RatedRequest spec : this.ratedRequests) { spec.toXContent(builder, params); } builder.endArray(); - builder.field(METRIC_FIELD.getPreferredName(), this.eval); + builder.field(METRIC_FIELD.getPreferredName(), this.metric); builder.endObject(); return builder; } @@ -152,4 +153,22 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { return PARSER.parse(parser, context); } + @Override + public final boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + RankEvalSpec other = (RankEvalSpec) obj; + return Objects.equals(specId, other.specId) && + Objects.equals(ratedRequests, other.ratedRequests) && + Objects.equals(metric, other.metric); + } + + @Override + public final int hashCode() { + return Objects.hash(getClass(), specId, ratedRequests, metric); + } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java index 30793d564e3..b5ce6121ffb 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java @@ -39,7 +39,7 @@ import java.util.List; * * RelevancyLevel specifies the type of object determining the relevancy level of some known docid. * */ -public abstract class RankedListQualityMetric +public abstract class RankedListQualityMetric> extends ToXContentToBytes implements NamedWriteable, FromXContentBuilder { @@ -52,8 +52,8 @@ public abstract class RankedListQualityMetric * */ public abstract EvalQueryQuality evaluate(SearchHit[] hits, List ratedDocs); - public static RankedListQualityMetric fromXContent(XContentParser parser, ParseFieldMatcherSupplier context) throws IOException { - RankedListQualityMetric rc; + public static RankedListQualityMetric fromXContent(XContentParser parser, ParseFieldMatcherSupplier context) throws IOException { + RankedListQualityMetric rc; Token token = parser.nextToken(); if (token != XContentParser.Token.FIELD_NAME) { throw new ParsingException(parser.getTokenLocation(), "[_na] missing required metric name"); diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java index c5faecb986e..9b2dbf3fc09 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java @@ -43,7 +43,8 @@ public class RatedDocument extends ToXContentToBytes implements Writeable, FromX public static final ParseField RATING_FIELD = new ParseField("rating"); public static final ParseField KEY_FIELD = new ParseField("key"); - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("rated_document", + private static final ConstructingObjectParser PARSER = + new ConstructingObjectParser<>("rated_document", a -> new RatedDocument((RatedDocumentKey) a[0], (Integer) a[1])); static { diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java index 3efcc7e16a4..cdffcf2ac57 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java @@ -39,7 +39,8 @@ public class RatedDocumentKey extends ToXContentToBytes implements Writeable, Fr public static final ParseField TYPE_FIELD = new ParseField("type"); public static final ParseField INDEX_FIELD = new ParseField("index"); - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("ratings", + private static final ConstructingObjectParser PARSER = + new ConstructingObjectParser<>("ratings", a -> new RatedDocumentKey((String) a[0], (String) a[1], (String) a[2])); static { @@ -117,7 +118,8 @@ public class RatedDocumentKey extends ToXContentToBytes implements Writeable, Fr }); } - public static RatedDocumentKey fromXContent(XContentParser parser, ParseFieldMatcherSupplier context) throws IOException { + public static RatedDocumentKey fromXContent( + XContentParser parser, ParseFieldMatcherSupplier context) throws IOException { return PARSER.apply(parser, context); } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/QuerySpec.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java similarity index 90% rename from modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/QuerySpec.java rename to modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java index 329839750b1..2eb88a7f0e8 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/QuerySpec.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedRequest.java @@ -41,7 +41,7 @@ import java.util.Objects; * * The resulting document lists can then be compared against what was specified in the set of rated documents as part of a QAQuery. * */ -public class QuerySpec extends ToXContentToBytes implements Writeable { +public class RatedRequest extends ToXContentToBytes implements Writeable { private String specId; private SearchSourceBuilder testRequest; @@ -50,12 +50,12 @@ public class QuerySpec extends ToXContentToBytes implements Writeable { /** Collection of rated queries for this query QA specification.*/ private List ratedDocs = new ArrayList<>(); - public QuerySpec() { + 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 QuerySpec(String specId, SearchSourceBuilder testRequest, List indices, List types, + public RatedRequest(String specId, SearchSourceBuilder testRequest, List indices, List types, List ratedDocs) { this.specId = specId; this.testRequest = testRequest; @@ -64,7 +64,7 @@ public class QuerySpec extends ToXContentToBytes implements Writeable { this.ratedDocs = ratedDocs; } - public QuerySpec(StreamInput in) throws IOException { + public RatedRequest(StreamInput in) throws IOException { this.specId = in.readString(); testRequest = new SearchSourceBuilder(in); int indicesSize = in.readInt(); @@ -149,18 +149,18 @@ public class QuerySpec extends ToXContentToBytes implements Writeable { 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 ObjectParser PARSER = new ObjectParser<>("requests", QuerySpec::new); + private static final ObjectParser PARSER = new ObjectParser<>("requests", RatedRequest::new); static { - PARSER.declareString(QuerySpec::setSpecId, ID_FIELD); - PARSER.declareObject(QuerySpec::setTestRequest, (p, c) -> { + PARSER.declareString(RatedRequest::setSpecId, ID_FIELD); + PARSER.declareObject(RatedRequest::setTestRequest, (p, c) -> { try { return SearchSourceBuilder.fromXContent(c.getParseContext(), c.getAggs(), c.getSuggesters()); } catch (IOException ex) { throw new ParsingException(p.getTokenLocation(), "error parsing request", ex); } } , REQUEST_FIELD); - PARSER.declareObjectArray(QuerySpec::setRatedDocs, (p, c) -> { + PARSER.declareObjectArray(RatedRequest::setRatedDocs, (p, c) -> { try { return RatedDocument.fromXContent(p, c); } catch (IOException ex) { @@ -170,7 +170,7 @@ public class QuerySpec extends ToXContentToBytes implements Writeable { } /** - * Parses {@link QuerySpec} from rest representation: + * Parses {@link RatedRequest} from rest representation: * * Example: * { @@ -189,7 +189,7 @@ public class QuerySpec extends ToXContentToBytes implements Writeable { * "ratings": [{ "1": 1 }, { "2": 0 }, { "3": 1 } ] * } */ - public static QuerySpec fromXContent(XContentParser parser, RankEvalContext context) throws IOException { + public static RatedRequest fromXContent(XContentParser parser, RankEvalContext context) throws IOException { return PARSER.parse(parser, context); } @@ -215,7 +215,7 @@ public class QuerySpec extends ToXContentToBytes implements Writeable { if (obj == null || getClass() != obj.getClass()) { return false; } - QuerySpec other = (QuerySpec) obj; + RatedRequest other = (RatedRequest) obj; return Objects.equals(specId, other.specId) && Objects.equals(testRequest, other.testRequest) && Objects.equals(indices, other.indices) && diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java index a40ecc1012e..5e9d4874e4d 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java @@ -160,9 +160,9 @@ public class ReciprocalRank extends RankedListQualityMetric { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - //builder.startObject(NAME); // TODO for roundtripping to work + builder.startObject(NAME); builder.field(MAX_RANK_FIELD.getPreferredName(), this.maxAcceptableRank); - //builder.endObject(); + builder.endObject(); builder.endObject(); return builder; } 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 8acc7b34a51..e3e5941c969 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 @@ -196,7 +196,7 @@ 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 = RankEvalSpec.parse(context.parser(), context); - for (QuerySpec specification : spec.getSpecifications()) { + for (RatedRequest specification : spec.getSpecifications()) { 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 3e4a8d0ea19..79332a22dc7 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 @@ -64,11 +64,11 @@ public class TransportRankEvalAction extends HandledTransportAction> unknownDocs = new ConcurrentHashMap<>(); - Collection specifications = qualityTask.getSpecifications(); + Collection specifications = qualityTask.getSpecifications(); AtomicInteger responseCounter = new AtomicInteger(specifications.size()); Map partialResults = new ConcurrentHashMap<>(specifications.size()); - for (QuerySpec querySpecification : specifications) { + for (RatedRequest querySpecification : specifications) { final RankEvalActionListener searchListener = new RankEvalActionListener(listener, qualityTask, querySpecification, partialResults, unknownDocs, responseCounter); SearchSourceBuilder specRequest = querySpecification.getTestRequest(); @@ -85,13 +85,13 @@ public class TransportRankEvalAction extends HandledTransportAction { private ActionListener listener; - private QuerySpec specification; + private RatedRequest specification; private Map partialResults; private RankEvalSpec task; private Map> unknownDocs; private AtomicInteger responseCounter; - public RankEvalActionListener(ActionListener listener, RankEvalSpec task, QuerySpec specification, + public RankEvalActionListener(ActionListener listener, RankEvalSpec task, RatedRequest specification, Map partialResults, Map> unknownDocs, AtomicInteger responseCounter) { this.listener = listener; diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java index 68207470382..e92f147acea 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java @@ -33,7 +33,7 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.ExecutionException; -public class DiscountedCumulativeGainAtTests extends XContentRoundtripTestCase { +public class DiscountedCumulativeGainAtTests extends XContentRoundtripTests { /** * Assuming the docs are ranked in the following order: @@ -121,12 +121,21 @@ public class DiscountedCumulativeGainAtTests extends XContentRoundtripTestCase { +public class PrecisionAtNTests extends XContentRoundtripTests { public void testPrecisionAtFiveCalculation() throws IOException, InterruptedException, ExecutionException { List rated = new ArrayList<>(); @@ -111,10 +111,19 @@ public class PrecisionAtNTests extends XContentRoundtripTestCase { assertEquals(0.3, metric.combine(partialResults), Double.MIN_VALUE); } - public void testXContentRoundtrip() throws IOException { + public static PrecisionAtN createTestItem() { int position = randomIntBetween(0, 1000); + return new PrecisionAtN(position); + } - PrecisionAtN testItem = new PrecisionAtN(position); - roundtrip(testItem); + public void testXContentRoundtrip() throws IOException { + PrecisionAtN testItem = createTestItem(); + XContentParser itemParser = roundtrip(testItem); + itemParser.nextToken(); + itemParser.nextToken(); + PrecisionAtN parsedItem = testItem.fromXContent(itemParser, ParseFieldMatcher.STRICT); + assertNotSame(testItem, parsedItem); + assertEquals(testItem, parsedItem); + assertEquals(testItem.hashCode(), parsedItem.hashCode()); } } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java index deec3f73b1a..887abf1a652 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java @@ -70,13 +70,23 @@ public class QuerySpecTests extends ESTestCase { searchRequestParsers = null; } - public void testXContentRoundtrip() throws IOException { + public static RatedRequest createTestItem(List indices, List types) { String specId = randomAsciiOfLength(50); SearchSourceBuilder testRequest = new SearchSourceBuilder(); testRequest.size(23); testRequest.query(new MatchAllQueryBuilder()); + List ratedDocs = new ArrayList<>(); + int size = randomIntBetween(0, 2); + for (int i = 0; i < size; i++) { + ratedDocs.add(RatedDocumentTests.createTestItem()); + } + + return new RatedRequest(specId, testRequest, indices, types, ratedDocs); + } + + public void testXContentRoundtrip() throws IOException { List indices = new ArrayList<>(); int size = randomIntBetween(0, 20); for (int i = 0; i < size; i++) { @@ -89,15 +99,8 @@ public class QuerySpecTests extends ESTestCase { types.add(randomAsciiOfLengthBetween(0, 50)); } - List ratedDocs = new ArrayList<>(); - size = randomIntBetween(0, 20); - for (int i = 0; i < size; i++) { - ratedDocs.add(RatedDocumentTests.createTestItem()); - } - + RatedRequest testItem = createTestItem(indices, types); - QuerySpec testItem = new QuerySpec(specId, testRequest, indices, types, ratedDocs); - XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); if (randomBoolean()) { builder.prettyPrint(); @@ -111,7 +114,7 @@ public class QuerySpecTests extends ESTestCase { RankEvalContext rankContext = new RankEvalContext(ParseFieldMatcher.STRICT, queryContext, searchRequestParsers); - QuerySpec parsedItem = QuerySpec.fromXContent(itemParser, rankContext); + RatedRequest parsedItem = RatedRequest.fromXContent(itemParser, rankContext); parsedItem.setIndices(indices); // IRL these come from URL parameters - see RestRankEvalAction parsedItem.setTypes(types); // IRL these come from URL parameters - see RestRankEvalAction assertNotSame(testItem, parsedItem); @@ -142,7 +145,7 @@ public class QuerySpecTests extends ESTestCase { QueryParseContext queryContext = new QueryParseContext(searchRequestParsers.queryParsers, parser, ParseFieldMatcher.STRICT); RankEvalContext rankContext = new RankEvalContext(ParseFieldMatcher.STRICT, queryContext, searchRequestParsers); - QuerySpec specification = QuerySpec.fromXContent(parser, rankContext); + RatedRequest specification = RatedRequest.fromXContent(parser, rankContext); assertEquals("my_qa_query", specification.getSpecId()); assertNotNull(specification.getTestRequest()); List ratedDocs = specification.getRatedDocs(); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java index c03010c73b1..bae486ef0e3 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java @@ -72,11 +72,11 @@ public class RankEvalRequestTests extends ESIntegTestCase { List types = Arrays.asList(new String[] { "testtype" }); String specId = randomAsciiOfLength(10); - List specifications = new ArrayList<>(); + List specifications = new ArrayList<>(); SearchSourceBuilder testQuery = new SearchSourceBuilder(); testQuery.query(new MatchAllQueryBuilder()); - specifications.add(new QuerySpec("amsterdam_query", testQuery, indices, types, createRelevant("2", "3", "4", "5"))); - specifications.add(new QuerySpec("berlin_query", testQuery, indices, types, createRelevant("1"))); + specifications.add(new RatedRequest("amsterdam_query", testQuery, indices, types, createRelevant("2", "3", "4", "5"))); + specifications.add(new RatedRequest("berlin_query", testQuery, indices, types, createRelevant("1"))); RankEvalSpec task = new RankEvalSpec(specId, specifications, new PrecisionAtN(10)); @@ -106,14 +106,14 @@ public class RankEvalRequestTests extends ESIntegTestCase { List types = Arrays.asList(new String[] { "testtype" }); String specId = randomAsciiOfLength(10); - List specifications = new ArrayList<>(); + List specifications = new ArrayList<>(); SearchSourceBuilder amsterdamQuery = new SearchSourceBuilder(); amsterdamQuery.query(new MatchAllQueryBuilder()); - specifications.add(new QuerySpec("amsterdam_query", amsterdamQuery, indices, types, createRelevant("2", "3", "4", "5"))); + specifications.add(new RatedRequest("amsterdam_query", amsterdamQuery, indices, types, createRelevant("2", "3", "4", "5"))); SearchSourceBuilder brokenQuery = new SearchSourceBuilder(); RangeQueryBuilder brokenRangeQuery = new RangeQueryBuilder("text").timeZone("CET"); brokenQuery.query(brokenRangeQuery); - specifications.add(new QuerySpec("broken_query", brokenQuery, indices, types, createRelevant("1"))); + specifications.add(new RatedRequest("broken_query", brokenQuery, indices, types, createRelevant("1"))); RankEvalSpec task = new RankEvalSpec(specId, specifications, new PrecisionAtN(10)); 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 new file mode 100644 index 00000000000..3fac5cb3868 --- /dev/null +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalSpecTests.java @@ -0,0 +1,119 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.rankeval; + +import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.ParseFieldRegistry; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.query.QueryParseContext; +import org.elasticsearch.indices.query.IndicesQueriesRegistry; +import org.elasticsearch.search.SearchModule; +import org.elasticsearch.search.SearchRequestParsers; +import org.elasticsearch.search.aggregations.AggregatorParsers; +import org.elasticsearch.search.suggest.Suggesters; +import org.elasticsearch.test.ESTestCase; +import org.junit.AfterClass; +import org.junit.BeforeClass; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import static java.util.Collections.emptyList; + +public class RankEvalSpecTests extends ESTestCase { + private static SearchModule searchModule; + private static SearchRequestParsers searchRequestParsers; + + /** + * setup for the whole base test class + */ + @BeforeClass + public static void init() throws IOException { + AggregatorParsers aggsParsers = new AggregatorParsers(new ParseFieldRegistry<>("aggregation"), + new ParseFieldRegistry<>("aggregation_pipes")); + searchModule = new SearchModule(Settings.EMPTY, false, emptyList()); + IndicesQueriesRegistry queriesRegistry = searchModule.getQueryParserRegistry(); + Suggesters suggesters = searchModule.getSuggesters(); + searchRequestParsers = new SearchRequestParsers(queriesRegistry, aggsParsers, suggesters); + } + + @AfterClass + public static void afterClass() throws Exception { + searchModule = null; + searchRequestParsers = null; + } + + public void testRoundtripping() throws IOException { + List indices = new ArrayList<>(); + int size = randomIntBetween(0, 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(QuerySpecTests.createTestItem(indices, types)); + } + + String specId = randomAsciiOfLengthBetween(1, 10); // TODO we should reject zero length ids ... + @SuppressWarnings("rawtypes") + RankedListQualityMetric metric; + if (randomBoolean()) { + metric = PrecisionAtNTests.createTestItem(); + } else { + metric = DiscountedCumulativeGainAtTests.createTestItem(); + } + + RankEvalSpec testItem = new RankEvalSpec(specId, specs, metric); + + XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); + if (randomBoolean()) { + builder.prettyPrint(); + } + testItem.toXContent(builder, ToXContent.EMPTY_PARAMS); + XContentBuilder shuffled = shuffleXContent(builder); + XContentParser itemParser = XContentHelper.createParser(shuffled.bytes()); + + QueryParseContext queryContext = new QueryParseContext(searchRequestParsers.queryParsers, itemParser, ParseFieldMatcher.STRICT); + RankEvalContext rankContext = new RankEvalContext(ParseFieldMatcher.STRICT, queryContext, + searchRequestParsers); + + 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);}); + assertNotSame(testItem, parsedItem); + assertEquals(testItem, parsedItem); + assertEquals(testItem.hashCode(), parsedItem.hashCode()); + } + +} diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java index 1ec1bbbcc06..7111a0064a4 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java @@ -19,9 +19,12 @@ package org.elasticsearch.index.rankeval; +import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.common.xcontent.XContentParser; + import java.io.IOException; -public class RatedDocumentKeyTests extends XContentRoundtripTestCase { +public class RatedDocumentKeyTests extends XContentRoundtripTests { public void testXContentRoundtrip() throws IOException { String index = randomAsciiOfLengthBetween(0, 10); @@ -29,6 +32,10 @@ public class RatedDocumentKeyTests extends XContentRoundtripTestCase { +public class RatedDocumentTests extends XContentRoundtripTests { public static RatedDocument createTestItem() { String index = randomAsciiOfLength(10); @@ -33,6 +36,11 @@ public class RatedDocumentTests extends XContentRoundtripTestCase } public void testXContentParsing() throws IOException { - roundtrip(createTestItem()); + RatedDocument testItem = createTestItem(); + XContentParser itemParser = roundtrip(testItem); + RatedDocument parsedItem = testItem.fromXContent(itemParser, ParseFieldMatcher.STRICT); + assertNotSame(testItem, parsedItem); + assertEquals(testItem, parsedItem); + assertEquals(testItem.hashCode(), parsedItem.hashCode()); } } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java index b25faba52d4..56443b85c11 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java @@ -19,7 +19,9 @@ package org.elasticsearch.index.rankeval; +import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.text.Text; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.Index; import org.elasticsearch.index.rankeval.PrecisionAtN.Rating; import org.elasticsearch.search.SearchShardTarget; @@ -33,7 +35,7 @@ import java.util.Vector; import static java.util.Collections.emptyList; -public class ReciprocalRankTests extends XContentRoundtripTestCase { +public class ReciprocalRankTests extends XContentRoundtripTests { public void testMaxAcceptableRank() { ReciprocalRank reciprocalRank = new ReciprocalRank(); @@ -128,7 +130,13 @@ public class ReciprocalRankTests extends XContentRoundtripTestCase> extends ESTestCase { +public class XContentRoundtripTests> extends ESTestCase { - public void roundtrip(T testItem) throws IOException { + public XContentParser roundtrip(T testItem) throws IOException { XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); if (randomBoolean()) { builder.prettyPrint(); @@ -42,10 +41,6 @@ public class XContentRoundtripTestCase Date: Wed, 7 Sep 2016 11:21:29 +0200 Subject: [PATCH 08/19] Test wouldn't compile w/o these annotations. --- .../index/rankeval/RankEvalRequestTests.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java index bae486ef0e3..35bc96146f3 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java @@ -35,7 +35,15 @@ import java.util.List; import java.util.Map.Entry; import java.util.Set; -@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.SUITE) +@ESIntegTestCase.ClusterScope( + scope = ESIntegTestCase.Scope.SUITE, + minNumDataNodes = 1, + numDataNodes = 2, + maxNumDataNodes = 3, + numClientNodes = 1, + transportClientRatio = 0.5, + supportsDedicatedMasters = true, + randomDynamicTemplates = true) public class RankEvalRequestTests extends ESIntegTestCase { @Override protected Collection> transportClientPlugins() { From 6bc646ac84bc6e5d881da6106ac515abdb67025d Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 11:21:50 +0200 Subject: [PATCH 09/19] Make constructor enforce non-optional position argument. --- .../rankeval/DiscountedCumulativeGainAt.java | 46 ++++++++----------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java index 22d98663738..9836def6d3b 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java @@ -50,24 +50,6 @@ public class DiscountedCumulativeGainAt extends RankedListQualityMetric matcher); } public static DiscountedCumulativeGainAt fromXContent(XContentParser parser, ParseFieldMatcherSupplier matcher) { From 333b7698719d4d3fc88c3321f0525db9b2730de0 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 11:48:50 +0200 Subject: [PATCH 10/19] Remove usage of FromXContentBuilder --- .../index/rankeval/DiscountedCumulativeGainAt.java | 8 +------- .../elasticsearch/index/rankeval/PrecisionAtN.java | 14 +------------- .../elasticsearch/index/rankeval/RankEvalSpec.java | 10 +++++----- .../index/rankeval/RankedListQualityMetric.java | 9 +++------ .../index/rankeval/ReciprocalRank.java | 14 +------------- .../rankeval/DiscountedCumulativeGainAtTests.java | 2 +- .../index/rankeval/PrecisionAtNTests.java | 2 +- .../index/rankeval/RankEvalSpecTests.java | 1 - .../index/rankeval/ReciprocalRankTests.java | 2 +- .../index/rankeval/XContentRoundtripTests.java | 3 +-- 10 files changed, 15 insertions(+), 50 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java index 9836def6d3b..07a3ac1a467 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java @@ -20,7 +20,6 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -38,7 +37,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; -public class DiscountedCumulativeGainAt extends RankedListQualityMetric { +public class DiscountedCumulativeGainAt extends RankedListQualityMetric { /** rank position up to which to check results. */ private int position; @@ -192,11 +191,6 @@ public class DiscountedCumulativeGainAt extends RankedListQualityMetric matcher); - } - public static DiscountedCumulativeGainAt fromXContent(XContentParser parser, ParseFieldMatcherSupplier matcher) { return PARSER.apply(parser, matcher); } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java index 50c2d54fa34..7d64dc31c8c 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java @@ -20,7 +20,6 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -43,7 +42,7 @@ import javax.naming.directory.SearchResult; * By default documents with a rating equal or bigger than 1 are considered to be "relevant" for the precision * calculation. This value can be changes using the "relevant_rating_threshold" parameter. * */ -public class PrecisionAtN extends RankedListQualityMetric { +public class PrecisionAtN extends RankedListQualityMetric { /** Number of results to check against a given set of relevant results. */ private int n; @@ -113,17 +112,6 @@ public class PrecisionAtN extends RankedListQualityMetric { return relevantRatingThreshhold ; } - @Override - public PrecisionAtN fromXContent(XContentParser parser, ParseFieldMatcher matcher) { - return PrecisionAtN.fromXContent(parser, new ParseFieldMatcherSupplier() { - - @Override - public ParseFieldMatcher getParseFieldMatcher() { - return matcher; - } - }); - } - public static PrecisionAtN fromXContent(XContentParser parser, ParseFieldMatcherSupplier matcher) { return PARSER.apply(parser, matcher); } 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 0aece1d0aa1..f329ee17f9e 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 @@ -46,7 +46,7 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { /** Collection of query specifications, that is e.g. search request templates to use for query translation. */ private Collection ratedRequests = new ArrayList<>(); /** Definition of the quality metric, e.g. precision at N */ - private RankedListQualityMetric metric; + private RankedListQualityMetric metric; /** a unique id for the whole QA task */ private String specId; @@ -54,7 +54,7 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { // TODO think if no args ctor is okay } - public RankEvalSpec(String specId, Collection specs, RankedListQualityMetric metric) { + public RankEvalSpec(String specId, Collection specs, RankedListQualityMetric metric) { this.specId = specId; this.ratedRequests = specs; this.metric = metric; @@ -80,7 +80,7 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { out.writeString(specId); } - public void setEval(RankedListQualityMetric eval) { + public void setEval(RankedListQualityMetric eval) { this.metric = eval; } @@ -93,12 +93,12 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { } /** Returns the precision at n configuration (containing level of n to consider).*/ - public RankedListQualityMetric getEvaluator() { + public RankedListQualityMetric getEvaluator() { return metric; } /** Sets the precision at n configuration (containing level of n to consider).*/ - public void setEvaluator(RankedListQualityMetric config) { + public void setEvaluator(RankedListQualityMetric config) { this.metric = config; } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java index b5ce6121ffb..e423ab3533c 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankedListQualityMetric.java @@ -23,7 +23,6 @@ import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.io.stream.NamedWriteable; -import org.elasticsearch.common.xcontent.FromXContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser.Token; @@ -39,9 +38,7 @@ import java.util.List; * * RelevancyLevel specifies the type of object determining the relevancy level of some known docid. * */ -public abstract class RankedListQualityMetric> - extends ToXContentToBytes - implements NamedWriteable, FromXContentBuilder { +public abstract class RankedListQualityMetric extends ToXContentToBytes implements NamedWriteable { /** * Returns a single metric representing the ranking quality of a set of returned documents @@ -52,8 +49,8 @@ public abstract class RankedListQualityMetric ratedDocs); - public static RankedListQualityMetric fromXContent(XContentParser parser, ParseFieldMatcherSupplier context) throws IOException { - RankedListQualityMetric rc; + public static RankedListQualityMetric fromXContent(XContentParser parser, ParseFieldMatcherSupplier context) throws IOException { + RankedListQualityMetric rc; Token token = parser.nextToken(); if (token != XContentParser.Token.FIELD_NAME) { throw new ParsingException(parser.getTokenLocation(), "[_na] missing required metric name"); diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java index 5907dba7a44..6d6626f9c4e 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java @@ -20,7 +20,6 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -44,7 +43,7 @@ import javax.naming.directory.SearchResult; * By default documents with a rating equal or bigger than 1 are considered to be "relevant" for the reciprocal rank * calculation. This value can be changes using the "relevant_rating_threshold" parameter. * */ -public class ReciprocalRank extends RankedListQualityMetric { +public class ReciprocalRank extends RankedListQualityMetric { public static final String NAME = "reciprocal_rank"; public static final int DEFAULT_MAX_ACCEPTABLE_RANK = 10; @@ -160,17 +159,6 @@ public class ReciprocalRank extends RankedListQualityMetric { PARSER.declareInt(ReciprocalRank::setRelevantRatingThreshhold, RELEVANT_RATING_FIELD); } - @Override - public ReciprocalRank fromXContent(XContentParser parser, ParseFieldMatcher matcher) { - return ReciprocalRank.fromXContent(parser, new ParseFieldMatcherSupplier() { - - @Override - public ParseFieldMatcher getParseFieldMatcher() { - return matcher; - } - }); - } - public static ReciprocalRank fromXContent(XContentParser parser, ParseFieldMatcherSupplier matcher) { return PARSER.apply(parser, matcher); } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java index e92f147acea..9bd93d1a9ad 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java @@ -133,7 +133,7 @@ public class DiscountedCumulativeGainAtTests extends XContentRoundtripTests ParseFieldMatcher.STRICT); assertNotSame(testItem, parsedItem); assertEquals(testItem, parsedItem); assertEquals(testItem.hashCode(), parsedItem.hashCode()); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java index 21aa9f6f957..4522d698bc7 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java @@ -144,7 +144,7 @@ public class PrecisionAtNTests extends XContentRoundtripTests { XContentParser itemParser = roundtrip(testItem); itemParser.nextToken(); itemParser.nextToken(); - PrecisionAtN parsedItem = testItem.fromXContent(itemParser, ParseFieldMatcher.STRICT); + PrecisionAtN parsedItem = PrecisionAtN.fromXContent(itemParser, () -> ParseFieldMatcher.STRICT); assertNotSame(testItem, parsedItem); assertEquals(testItem, parsedItem); assertEquals(testItem.hashCode(), parsedItem.hashCode()); 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 3fac5cb3868..7bc2c7649a3 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 @@ -86,7 +86,6 @@ public class RankEvalSpecTests extends ESTestCase { } String specId = randomAsciiOfLengthBetween(1, 10); // TODO we should reject zero length ids ... - @SuppressWarnings("rawtypes") RankedListQualityMetric metric; if (randomBoolean()) { metric = PrecisionAtNTests.createTestItem(); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java index f0d9cacdc38..db232662ea8 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java @@ -157,7 +157,7 @@ public class ReciprocalRankTests extends XContentRoundtripTests XContentParser itemParser = roundtrip(testItem); itemParser.nextToken(); itemParser.nextToken(); - ReciprocalRank parsedItem = testItem.fromXContent(itemParser, ParseFieldMatcher.STRICT); + ReciprocalRank parsedItem = ReciprocalRank.fromXContent(itemParser, () -> ParseFieldMatcher.STRICT); assertNotSame(testItem, parsedItem); assertEquals(testItem, parsedItem); assertEquals(testItem.hashCode(), parsedItem.hashCode()); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTests.java index 14bd3d20f79..a37055d897a 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTests.java @@ -20,7 +20,6 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.action.support.ToXContentToBytes; -import org.elasticsearch.common.xcontent.FromXContentBuilder; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -31,7 +30,7 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; -public class XContentRoundtripTests> extends ESTestCase { +public class XContentRoundtripTests extends ESTestCase { public XContentParser roundtrip(T testItem) throws IOException { XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); From efbef20361e4ca04fe8493ecb3a4a49441a524db Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 11:53:39 +0200 Subject: [PATCH 11/19] Remove call to getClass from hashCode implementations. --- .../index/rankeval/DiscountedCumulativeGainAt.java | 2 +- .../java/org/elasticsearch/index/rankeval/PrecisionAtN.java | 2 +- .../java/org/elasticsearch/index/rankeval/RankEvalSpec.java | 2 +- .../java/org/elasticsearch/index/rankeval/RatedDocument.java | 2 +- .../java/org/elasticsearch/index/rankeval/RatedDocumentKey.java | 2 +- .../java/org/elasticsearch/index/rankeval/RatedRequest.java | 2 +- .../java/org/elasticsearch/index/rankeval/ReciprocalRank.java | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java index 07a3ac1a467..36d2a208353 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAt.java @@ -225,6 +225,6 @@ public class DiscountedCumulativeGainAt extends RankedListQualityMetric { @Override public final int hashCode() { - return Objects.hash(getClass(), position, normalize, unknownDocRating); + return Objects.hash(position, normalize, unknownDocRating); } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java index 7d64dc31c8c..ac8675a2a37 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/PrecisionAtN.java @@ -197,6 +197,6 @@ public class PrecisionAtN extends RankedListQualityMetric { @Override public final int hashCode() { - return Objects.hash(getClass(), n); + return Objects.hash(n); } } 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 f329ee17f9e..39e937aaf6c 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 @@ -169,6 +169,6 @@ public class RankEvalSpec extends ToXContentToBytes implements Writeable { @Override public final int hashCode() { - return Objects.hash(getClass(), specId, ratedRequests, metric); + return Objects.hash(specId, ratedRequests, metric); } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java index 9b2dbf3fc09..8a9ad2071e2 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java @@ -136,6 +136,6 @@ public class RatedDocument extends ToXContentToBytes implements Writeable, FromX @Override public final int hashCode() { - return Objects.hash(getClass(), key, rating); + return Objects.hash(key, rating); } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java index cdffcf2ac57..feef20b890c 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java @@ -139,6 +139,6 @@ public class RatedDocumentKey extends ToXContentToBytes implements Writeable, Fr @Override public final int hashCode() { - return Objects.hash(getClass(), index, type, docId); + return Objects.hash(index, type, docId); } } 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 2eb88a7f0e8..86861a2c95e 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 @@ -225,6 +225,6 @@ public class RatedRequest extends ToXContentToBytes implements Writeable { @Override public final int hashCode() { - return Objects.hash(getClass(), specId, testRequest, indices.hashCode(), types.hashCode(), ratedDocs.hashCode()); + return Objects.hash(specId, testRequest, indices.hashCode(), types.hashCode(), ratedDocs.hashCode()); } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java index 6d6626f9c4e..803900a3521 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/ReciprocalRank.java @@ -187,6 +187,6 @@ public class ReciprocalRank extends RankedListQualityMetric { @Override public final int hashCode() { - return Objects.hash(getClass(), maxAcceptableRank); + return Objects.hash(maxAcceptableRank); } } From c8a3b3c32fd15b8f972aa530951f1a0d6930a14e Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 12:02:00 +0200 Subject: [PATCH 12/19] Move xcontent roundtrip method to helper class. --- .../rankeval/DiscountedCumulativeGainAtTests.java | 5 +++-- .../index/rankeval/PrecisionAtNTests.java | 5 +++-- .../index/rankeval/RatedDocumentKeyTests.java | 5 +++-- .../index/rankeval/RatedDocumentTests.java | 5 +++-- .../index/rankeval/ReciprocalRankTests.java | 5 +++-- ...tentRoundtripTests.java => XContentTestHelper.java} | 10 +++++----- 6 files changed, 20 insertions(+), 15 deletions(-) rename modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/{XContentRoundtripTests.java => XContentTestHelper.java} (84%) diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java index 9bd93d1a9ad..1ef63e16405 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/DiscountedCumulativeGainAtTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.internal.InternalSearchHit; +import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; @@ -33,7 +34,7 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.ExecutionException; -public class DiscountedCumulativeGainAtTests extends XContentRoundtripTests { +public class DiscountedCumulativeGainAtTests extends ESTestCase { /** * Assuming the docs are ranked in the following order: @@ -130,7 +131,7 @@ public class DiscountedCumulativeGainAtTests extends XContentRoundtripTests ParseFieldMatcher.STRICT); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java index 4522d698bc7..14a4be5ec38 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/PrecisionAtNTests.java @@ -27,6 +27,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.rankeval.PrecisionAtN.Rating; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.internal.InternalSearchHit; +import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; @@ -37,7 +38,7 @@ import java.util.concurrent.ExecutionException; import static java.util.Collections.emptyList; -public class PrecisionAtNTests extends XContentRoundtripTests { +public class PrecisionAtNTests extends ESTestCase { public void testPrecisionAtFiveCalculation() throws IOException, InterruptedException, ExecutionException { List rated = new ArrayList<>(); @@ -141,7 +142,7 @@ public class PrecisionAtNTests extends XContentRoundtripTests { public void testXContentRoundtrip() throws IOException { PrecisionAtN testItem = createTestItem(); - XContentParser itemParser = roundtrip(testItem); + XContentParser itemParser = XContentTestHelper.roundtrip(testItem); itemParser.nextToken(); itemParser.nextToken(); PrecisionAtN parsedItem = PrecisionAtN.fromXContent(itemParser, () -> ParseFieldMatcher.STRICT); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java index 7111a0064a4..8ac79b67b2c 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java @@ -21,10 +21,11 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.ESTestCase; import java.io.IOException; -public class RatedDocumentKeyTests extends XContentRoundtripTests { +public class RatedDocumentKeyTests extends ESTestCase { public void testXContentRoundtrip() throws IOException { String index = randomAsciiOfLengthBetween(0, 10); @@ -32,7 +33,7 @@ public class RatedDocumentKeyTests extends XContentRoundtripTests { +public class RatedDocumentTests extends ESTestCase { public static RatedDocument createTestItem() { String index = randomAsciiOfLength(10); @@ -37,7 +38,7 @@ public class RatedDocumentTests extends XContentRoundtripTests { public void testXContentParsing() throws IOException { RatedDocument testItem = createTestItem(); - XContentParser itemParser = roundtrip(testItem); + XContentParser itemParser = XContentTestHelper.roundtrip(testItem); RatedDocument parsedItem = testItem.fromXContent(itemParser, ParseFieldMatcher.STRICT); assertNotSame(testItem, parsedItem); assertEquals(testItem, parsedItem); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java index db232662ea8..40fb7a05eea 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/ReciprocalRankTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.index.Index; import org.elasticsearch.index.rankeval.PrecisionAtN.Rating; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.search.internal.InternalSearchHit; +import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; @@ -36,7 +37,7 @@ import java.util.concurrent.ExecutionException; import static java.util.Collections.emptyList; -public class ReciprocalRankTests extends XContentRoundtripTests { +public class ReciprocalRankTests extends ESTestCase { public void testMaxAcceptableRank() { ReciprocalRank reciprocalRank = new ReciprocalRank(); @@ -154,7 +155,7 @@ public class ReciprocalRankTests extends XContentRoundtripTests int position = randomIntBetween(0, 1000); ReciprocalRank testItem = new ReciprocalRank(position); - XContentParser itemParser = roundtrip(testItem); + XContentParser itemParser = XContentTestHelper.roundtrip(testItem); itemParser.nextToken(); itemParser.nextToken(); ReciprocalRank parsedItem = ReciprocalRank.fromXContent(itemParser, () -> ParseFieldMatcher.STRICT); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentTestHelper.java similarity index 84% rename from modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTests.java rename to modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentTestHelper.java index a37055d897a..e02772ad6f6 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentRoundtripTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/XContentTestHelper.java @@ -30,15 +30,15 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; -public class XContentRoundtripTests extends ESTestCase { +public class XContentTestHelper { - public XContentParser roundtrip(T testItem) throws IOException { - XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); - if (randomBoolean()) { + public static XContentParser roundtrip(ToXContentToBytes testItem) throws IOException { + XContentBuilder builder = XContentFactory.contentBuilder(ESTestCase.randomFrom(XContentType.values())); + if (ESTestCase.randomBoolean()) { builder.prettyPrint(); } testItem.toXContent(builder, ToXContent.EMPTY_PARAMS); - XContentBuilder shuffled = shuffleXContent(builder); + XContentBuilder shuffled = ESTestCase.shuffleXContent(builder); XContentParser itemParser = XContentHelper.createParser(shuffled.bytes()); return itemParser; } From b3a4a89151175f23ff4f09202c28e35a87d0da42 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 12:06:39 +0200 Subject: [PATCH 13/19] Replace magic number in test with random number. --- .../java/org/elasticsearch/index/rankeval/QuerySpecTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java index 887abf1a652..86b421c7ac3 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java @@ -74,7 +74,7 @@ public class QuerySpecTests extends ESTestCase { String specId = randomAsciiOfLength(50); SearchSourceBuilder testRequest = new SearchSourceBuilder(); - testRequest.size(23); + testRequest.size(randomInt()); testRequest.query(new MatchAllQueryBuilder()); List ratedDocs = new ArrayList<>(); @@ -108,7 +108,7 @@ public class QuerySpecTests extends ESTestCase { testItem.toXContent(builder, ToXContent.EMPTY_PARAMS); XContentBuilder shuffled = shuffleXContent(builder); XContentParser itemParser = XContentHelper.createParser(shuffled.bytes()); - itemParser.nextToken(); // TODO this could be the reason why the metric roundtrip tests failed + itemParser.nextToken(); QueryParseContext queryContext = new QueryParseContext(searchRequestParsers.queryParsers, itemParser, ParseFieldMatcher.STRICT); RankEvalContext rankContext = new RankEvalContext(ParseFieldMatcher.STRICT, queryContext, From 0d25fc4925d8b29296e774b9772d5d4f1c70f305 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 12:11:07 +0200 Subject: [PATCH 14/19] Re-use roundtrip helper for QuerySpecTests --- .../index/rankeval/QuerySpecTests.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java index 86b421c7ac3..48232854c9b 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java @@ -22,12 +22,8 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ParseFieldRegistry; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.MatchAllQueryBuilder; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.indices.query.IndicesQueriesRegistry; @@ -100,14 +96,7 @@ public class QuerySpecTests extends ESTestCase { } RatedRequest testItem = createTestItem(indices, types); - - XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); - if (randomBoolean()) { - builder.prettyPrint(); - } - testItem.toXContent(builder, ToXContent.EMPTY_PARAMS); - XContentBuilder shuffled = shuffleXContent(builder); - XContentParser itemParser = XContentHelper.createParser(shuffled.bytes()); + XContentParser itemParser = XContentTestHelper.roundtrip(testItem); itemParser.nextToken(); QueryParseContext queryContext = new QueryParseContext(searchRequestParsers.queryParsers, itemParser, ParseFieldMatcher.STRICT); From 2af0218bdd467ceeba3af80909109f1619a765eb Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 12:12:06 +0200 Subject: [PATCH 15/19] Rename test class to match renamed implementation Used to be QuerySpec, is now RatedRequest; changing name of test accordingly. --- .../org/elasticsearch/index/rankeval/RankEvalSpecTests.java | 2 +- .../rankeval/{QuerySpecTests.java => RatedRequestsTests.java} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/{QuerySpecTests.java => RatedRequestsTests.java} (99%) 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 7bc2c7649a3..fc1cf0abafd 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 @@ -82,7 +82,7 @@ public class RankEvalSpecTests extends ESTestCase { 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(QuerySpecTests.createTestItem(indices, types)); + specs.add(RatedRequestsTests.createTestItem(indices, types)); } String specId = randomAsciiOfLengthBetween(1, 10); // TODO we should reject zero length ids ... diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java similarity index 99% rename from modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java rename to modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java index 48232854c9b..7104b6aa4f4 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/QuerySpecTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedRequestsTests.java @@ -42,7 +42,7 @@ import java.util.List; import static java.util.Collections.emptyList; -public class QuerySpecTests extends ESTestCase { +public class RatedRequestsTests extends ESTestCase { private static SearchModule searchModule; private static SearchRequestParsers searchRequestParsers; From 9d8bb720dce1c455e983d5b5e1c469004c8babcf Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 12:50:04 +0200 Subject: [PATCH 16/19] Remove leftover FromXContentBuilder reference --- .../index/rankeval/RatedDocument.java | 15 +-------------- .../index/rankeval/RatedDocumentTests.java | 2 +- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java index 8a9ad2071e2..1cde3828507 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocument.java @@ -21,14 +21,12 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; 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.ConstructingObjectParser; -import org.elasticsearch.common.xcontent.FromXContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -38,7 +36,7 @@ import java.util.Objects; /** * A document ID and its rating for the query QA use case. * */ -public class RatedDocument extends ToXContentToBytes implements Writeable, FromXContentBuilder { +public class RatedDocument extends ToXContentToBytes implements Writeable { public static final ParseField RATING_FIELD = new ParseField("rating"); public static final ParseField KEY_FIELD = new ParseField("key"); @@ -97,17 +95,6 @@ public class RatedDocument extends ToXContentToBytes implements Writeable, FromX out.writeVInt(rating); } - @Override - public RatedDocument fromXContent(XContentParser parser, ParseFieldMatcher parseFieldMatcher) throws IOException { - return RatedDocument.fromXContent(parser, new ParseFieldMatcherSupplier() { - - @Override - public ParseFieldMatcher getParseFieldMatcher() { - return parseFieldMatcher; - } - }); - } - public static RatedDocument fromXContent(XContentParser parser, ParseFieldMatcherSupplier supplier) throws IOException { return PARSER.apply(parser, supplier); } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java index 37dfb28b160..3084682ad75 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentTests.java @@ -39,7 +39,7 @@ public class RatedDocumentTests extends ESTestCase { public void testXContentParsing() throws IOException { RatedDocument testItem = createTestItem(); XContentParser itemParser = XContentTestHelper.roundtrip(testItem); - RatedDocument parsedItem = testItem.fromXContent(itemParser, ParseFieldMatcher.STRICT); + RatedDocument parsedItem = RatedDocument.fromXContent(itemParser, () -> ParseFieldMatcher.STRICT); assertNotSame(testItem, parsedItem); assertEquals(testItem, parsedItem); assertEquals(testItem.hashCode(), parsedItem.hashCode()); From ba9956a46894f9770f4121d44a41fefa8b5b7abd Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 12:52:30 +0200 Subject: [PATCH 17/19] Remove FromXContentBuilder from RatedDocumentKey --- .../index/rankeval/RatedDocumentKey.java | 15 +-------------- .../index/rankeval/RatedDocumentKeyTests.java | 2 +- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java index feef20b890c..5e69f902dea 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RatedDocumentKey.java @@ -21,20 +21,18 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.action.support.ToXContentToBytes; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; 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.ConstructingObjectParser; -import org.elasticsearch.common.xcontent.FromXContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import java.io.IOException; import java.util.Objects; -public class RatedDocumentKey extends ToXContentToBytes implements Writeable, FromXContentBuilder { +public class RatedDocumentKey extends ToXContentToBytes implements Writeable { public static final ParseField DOC_ID_FIELD = new ParseField("doc_id"); public static final ParseField TYPE_FIELD = new ParseField("type"); public static final ParseField INDEX_FIELD = new ParseField("index"); @@ -107,17 +105,6 @@ public class RatedDocumentKey extends ToXContentToBytes implements Writeable, Fr out.writeString(docId); } - @Override - public RatedDocumentKey fromXContent(XContentParser parser, ParseFieldMatcher matcher) throws IOException { - return RatedDocumentKey.fromXContent(parser, new ParseFieldMatcherSupplier() { - - @Override - public ParseFieldMatcher getParseFieldMatcher() { - return matcher; - } - }); - } - public static RatedDocumentKey fromXContent( XContentParser parser, ParseFieldMatcherSupplier context) throws IOException { return PARSER.apply(parser, context); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java index 8ac79b67b2c..6c0bd766b35 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedDocumentKeyTests.java @@ -34,7 +34,7 @@ public class RatedDocumentKeyTests extends ESTestCase { RatedDocumentKey testItem = new RatedDocumentKey(index, type, docId); XContentParser itemParser = XContentTestHelper.roundtrip(testItem); - RatedDocumentKey parsedItem = testItem.fromXContent(itemParser, ParseFieldMatcher.STRICT); + RatedDocumentKey parsedItem = RatedDocumentKey.fromXContent(itemParser, () -> ParseFieldMatcher.STRICT); assertNotSame(testItem, parsedItem); assertEquals(testItem, parsedItem); assertEquals(testItem.hashCode(), parsedItem.hashCode()); From d34a117422f76057c0cf5dd1f7e825b8a71f7a02 Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 13:03:18 +0200 Subject: [PATCH 18/19] Use roundtrip helper for rank eval spec tests. --- .../index/rankeval/RankEvalSpecTests.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) 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 fc1cf0abafd..b6a6025a0ef 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 @@ -22,12 +22,7 @@ package org.elasticsearch.index.rankeval; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ParseFieldRegistry; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.QueryParseContext; import org.elasticsearch.indices.query.IndicesQueriesRegistry; import org.elasticsearch.search.SearchModule; @@ -95,13 +90,7 @@ public class RankEvalSpecTests extends ESTestCase { RankEvalSpec testItem = new RankEvalSpec(specId, specs, metric); - XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values())); - if (randomBoolean()) { - builder.prettyPrint(); - } - testItem.toXContent(builder, ToXContent.EMPTY_PARAMS); - XContentBuilder shuffled = shuffleXContent(builder); - XContentParser itemParser = XContentHelper.createParser(shuffled.bytes()); + XContentParser itemParser = XContentTestHelper.roundtrip(testItem); QueryParseContext queryContext = new QueryParseContext(searchRequestParsers.queryParsers, itemParser, ParseFieldMatcher.STRICT); RankEvalContext rankContext = new RankEvalContext(ParseFieldMatcher.STRICT, queryContext, From 450b756152ba212737647a6d7291ac9f1c00536a Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Wed, 7 Sep 2016 13:54:27 +0200 Subject: [PATCH 19/19] We don't actually need this annotation anymore. --- .../index/rankeval/RankEvalRequestTests.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java index 35bc96146f3..3a61b3c98ec 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java @@ -35,15 +35,6 @@ import java.util.List; import java.util.Map.Entry; import java.util.Set; -@ESIntegTestCase.ClusterScope( - scope = ESIntegTestCase.Scope.SUITE, - minNumDataNodes = 1, - numDataNodes = 2, - maxNumDataNodes = 3, - numClientNodes = 1, - transportClientRatio = 0.5, - supportsDedicatedMasters = true, - randomDynamicTemplates = true) public class RankEvalRequestTests extends ESIntegTestCase { @Override protected Collection> transportClientPlugins() {