From e4b30071bb8243be96baa744e8ebbe61057b1474 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 22 Mar 2018 11:58:55 +0100 Subject: [PATCH] RankEvalRequest should implement IndicesRequest (#29188) Change RankEvalRequest to implement IndicesRequest, so it gets treated in a similar fashion to regular search requests e.g. by security. --- .../org/elasticsearch/client/Request.java | 2 +- .../index/rankeval/RankEvalRequest.java | 78 +++++++++++++---- .../index/rankeval/RestRankEvalAction.java | 2 +- .../rankeval/TransportRankEvalAction.java | 6 +- .../index/rankeval/RankEvalRequestIT.java | 19 ++--- .../index/rankeval/RankEvalRequestTests.java | 83 +++++++++++++++++++ .../index/rankeval/RankEvalSpecTests.java | 7 +- .../rankeval/SmokeMultipleTemplatesIT.java | 2 +- 8 files changed, 162 insertions(+), 37 deletions(-) create mode 100644 modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java index 66b34da777b6a..0d8057060efeb 100755 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java @@ -531,7 +531,7 @@ static Request existsAlias(GetAliasesRequest getAliasesRequest) { } static Request rankEval(RankEvalRequest rankEvalRequest) throws IOException { - String endpoint = endpoint(rankEvalRequest.getIndices(), Strings.EMPTY_ARRAY, "_rank_eval"); + String endpoint = endpoint(rankEvalRequest.indices(), Strings.EMPTY_ARRAY, "_rank_eval"); HttpEntity entity = createEntity(rankEvalRequest.getRankEvalSpec(), REQUEST_BODY_CONTENT_TYPE); return new Request(HttpGet.METHOD_NAME, endpoint, Collections.emptyMap(), entity); } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalRequest.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalRequest.java index 58fd3b0a694ae..7d3ec94811c5a 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalRequest.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalRequest.java @@ -22,24 +22,47 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; +import org.elasticsearch.action.IndicesRequest; +import org.elasticsearch.action.search.SearchRequest; +import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import java.io.IOException; +import java.util.Arrays; import java.util.Objects; /** * Request to perform a search ranking evaluation. */ -public class RankEvalRequest extends ActionRequest { +public class RankEvalRequest extends ActionRequest implements IndicesRequest.Replaceable { private RankEvalSpec rankingEvaluationSpec; + + private IndicesOptions indicesOptions = SearchRequest.DEFAULT_INDICES_OPTIONS; private String[] indices = Strings.EMPTY_ARRAY; public RankEvalRequest(RankEvalSpec rankingEvaluationSpec, String[] indices) { - this.rankingEvaluationSpec = rankingEvaluationSpec; - setIndices(indices); + this.rankingEvaluationSpec = Objects.requireNonNull(rankingEvaluationSpec, "ranking evaluation specification must not be null"); + indices(indices); + } + + RankEvalRequest(StreamInput in) throws IOException { + super.readFrom(in); + rankingEvaluationSpec = new RankEvalSpec(in); + if (in.getVersion().onOrAfter(Version.V_6_3_0)) { + indices = in.readStringArray(); + indicesOptions = IndicesOptions.readIndicesOptions(in); + } else { + // readStringArray uses readVInt for size, we used readInt in 6.2 + int indicesSize = in.readInt(); + String[] indices = new String[indicesSize]; + for (int i = 0; i < indicesSize; i++) { + indices[i] = in.readString(); + } + // no indices options yet + } } RankEvalRequest() { @@ -72,7 +95,8 @@ public void setRankEvalSpec(RankEvalSpec task) { /** * Sets the indices the search will be executed on. */ - public RankEvalRequest setIndices(String... indices) { + @Override + public RankEvalRequest indices(String... indices) { Objects.requireNonNull(indices, "indices must not be null"); for (String index : indices) { Objects.requireNonNull(index, "index must not be null"); @@ -84,24 +108,23 @@ public RankEvalRequest setIndices(String... indices) { /** * @return the indices for this request */ - public String[] getIndices() { + @Override + public String[] indices() { return indices; } + @Override + public IndicesOptions indicesOptions() { + return indicesOptions; + } + + public void indicesOptions(IndicesOptions indicesOptions) { + this.indicesOptions = Objects.requireNonNull(indicesOptions, "indicesOptions must not be null"); + } + @Override public void readFrom(StreamInput in) throws IOException { - super.readFrom(in); - rankingEvaluationSpec = new RankEvalSpec(in); - if (in.getVersion().onOrAfter(Version.V_6_3_0)) { - indices = in.readStringArray(); - } else { - // readStringArray uses readVInt for size, we used readInt in 6.2 - int indicesSize = in.readInt(); - String[] indices = new String[indicesSize]; - for (int i = 0; i < indicesSize; i++) { - indices[i] = in.readString(); - } - } + throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable"); } @Override @@ -110,12 +133,33 @@ public void writeTo(StreamOutput out) throws IOException { rankingEvaluationSpec.writeTo(out); if (out.getVersion().onOrAfter(Version.V_6_3_0)) { out.writeStringArray(indices); + indicesOptions.writeIndicesOptions(out); } else { // writeStringArray uses writeVInt for size, we used writeInt in 6.2 out.writeInt(indices.length); for (String index : indices) { out.writeString(index); } + // no indices options yet + } + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; } + RankEvalRequest that = (RankEvalRequest) o; + return Objects.equals(indicesOptions, that.indicesOptions) && + Arrays.equals(indices, that.indices) && + Objects.equals(rankingEvaluationSpec, that.rankingEvaluationSpec); + } + + @Override + public int hashCode() { + return Objects.hash(indicesOptions, Arrays.hashCode(indices), rankingEvaluationSpec); } } 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 a596caf4f5c7b..34cf953ea50b7 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 @@ -108,7 +108,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli } private static void parseRankEvalRequest(RankEvalRequest rankEvalRequest, RestRequest request, XContentParser parser) { - rankEvalRequest.setIndices(Strings.splitStringByCommaToArray(request.param("index"))); + rankEvalRequest.indices(Strings.splitStringByCommaToArray(request.param("index"))); RankEvalSpec spec = RankEvalSpec.parse(parser); rankEvalRequest.setRankEvalSpec(spec); } 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 a4ce4c7ee92e7..d24a779fd61ce 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 @@ -75,8 +75,8 @@ public class TransportRankEvalAction extends HandledTransportAction { + + private static RankEvalPlugin rankEvalPlugin = new RankEvalPlugin(); + + @AfterClass + public static void releasePluginResources() throws IOException { + rankEvalPlugin.close(); + } + + @Override + protected NamedXContentRegistry xContentRegistry() { + return new NamedXContentRegistry(rankEvalPlugin.getNamedXContent()); + } + + @Override + protected NamedWriteableRegistry getNamedWriteableRegistry() { + return new NamedWriteableRegistry(rankEvalPlugin.getNamedWriteables()); + } + + @Override + protected RankEvalRequest createTestInstance() { + int numberOfIndices = randomInt(3); + String[] indices = new String[numberOfIndices]; + for (int i=0; i < numberOfIndices; i++) { + indices[i] = randomAlphaOfLengthBetween(5, 10); + } + RankEvalRequest rankEvalRequest = new RankEvalRequest(RankEvalSpecTests.createTestItem(), indices); + IndicesOptions indicesOptions = IndicesOptions.fromOptions( + randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean()); + rankEvalRequest.indicesOptions(indicesOptions); + return rankEvalRequest; + } + + @Override + protected Reader instanceReader() { + return RankEvalRequest::new; + } + + @Override + protected RankEvalRequest mutateInstance(RankEvalRequest instance) throws IOException { + RankEvalRequest mutation = copyInstance(instance); + List mutators = new ArrayList<>(); + mutators.add(() -> mutation.indices(ArrayUtils.concat(instance.indices(), new String[] { randomAlphaOfLength(10) }))); + mutators.add(() -> mutation.indicesOptions(randomValueOtherThan(instance.indicesOptions(), + () -> IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean())))); + mutators.add(() -> mutation.setRankEvalSpec(RankEvalSpecTests.mutateTestItem(instance.getRankEvalSpec()))); + randomFrom(mutators).run(); + return mutation; + } +} 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 26611679f3494..94338e570a5d2 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 @@ -70,7 +70,7 @@ private static List randomList(Supplier randomSupplier) { return result; } - private static RankEvalSpec createTestItem() throws IOException { + static RankEvalSpec createTestItem() { Supplier metric = randomFrom(Arrays.asList( () -> PrecisionAtKTests.createTestItem(), () -> MeanReciprocalRankTests.createTestItem(), @@ -87,6 +87,9 @@ private static RankEvalSpec createTestItem() throws IOException { builder.field("field", randomAlphaOfLengthBetween(1, 5)); builder.endObject(); script = Strings.toString(builder); + } catch (IOException e) { + // this shouldn't happen in tests, re-throw just not to swallow it + throw new RuntimeException(e); } templates = new HashSet<>(); @@ -156,7 +159,7 @@ public void testEqualsAndHash() throws IOException { checkEqualsAndHashCode(createTestItem(), RankEvalSpecTests::copy, RankEvalSpecTests::mutateTestItem); } - private static RankEvalSpec mutateTestItem(RankEvalSpec original) { + static RankEvalSpec mutateTestItem(RankEvalSpec original) { List ratedRequests = new ArrayList<>(original.getRatedRequests()); EvaluationMetric metric = original.getMetric(); Map templates = new HashMap<>(original.getTemplates()); diff --git a/qa/smoke-test-rank-eval-with-mustache/src/test/java/org/elasticsearch/index/rankeval/SmokeMultipleTemplatesIT.java b/qa/smoke-test-rank-eval-with-mustache/src/test/java/org/elasticsearch/index/rankeval/SmokeMultipleTemplatesIT.java index 178d429ca9ffd..50860ddd87b21 100644 --- a/qa/smoke-test-rank-eval-with-mustache/src/test/java/org/elasticsearch/index/rankeval/SmokeMultipleTemplatesIT.java +++ b/qa/smoke-test-rank-eval-with-mustache/src/test/java/org/elasticsearch/index/rankeval/SmokeMultipleTemplatesIT.java @@ -102,7 +102,7 @@ public void testPrecisionAtRequest() throws IOException { RankEvalRequestBuilder builder = new RankEvalRequestBuilder(client(), RankEvalAction.INSTANCE, new RankEvalRequest()); builder.setRankEvalSpec(task); - RankEvalResponse response = client().execute(RankEvalAction.INSTANCE, builder.request().setIndices("test")).actionGet(); + RankEvalResponse response = client().execute(RankEvalAction.INSTANCE, builder.request().indices("test")).actionGet(); assertEquals(0.9, response.getEvaluationResult(), Double.MIN_VALUE); }