From 1cfe1b7332733c24b600cd35b8053f791091d3c5 Mon Sep 17 00:00:00 2001 From: Kathleen DeRusso Date: Thu, 24 Oct 2024 14:34:20 -0400 Subject: [PATCH] HUZZAH! Local tests now pass. --- .../retriever/RetrieverBuilderWrapper.java | 7 +- .../retriever/TestRetrieverBuilder.java | 2 + .../rules/80_query_rules_retriever.yml | 124 +++++++++--------- .../retriever/QueryRuleRetrieverBuilder.java | 2 + 4 files changed, 72 insertions(+), 63 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilderWrapper.java b/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilderWrapper.java index fa38996d3c4f0..153203c0527d2 100644 --- a/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilderWrapper.java +++ b/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilderWrapper.java @@ -127,7 +127,12 @@ protected void doToXContent(XContentBuilder builder, Params params) throws IOExc @Override protected boolean doEquals(Object o) { - return in.equals(o); + // Handle the edge case where we need to unwrap the incoming retriever + if (o instanceof RetrieverBuilderWrapper wrapper) { + return in.doEquals(wrapper.in); + } else { + return in.doEquals(o); + } } @Override diff --git a/test/framework/src/main/java/org/elasticsearch/search/retriever/TestRetrieverBuilder.java b/test/framework/src/main/java/org/elasticsearch/search/retriever/TestRetrieverBuilder.java index 01380aa4d86b0..5ce775c508976 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/retriever/TestRetrieverBuilder.java +++ b/test/framework/src/main/java/org/elasticsearch/search/retriever/TestRetrieverBuilder.java @@ -89,6 +89,8 @@ public void doToXContent(XContentBuilder builder, Params params) throws IOExcept @Override public boolean doEquals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; TestRetrieverBuilder that = (TestRetrieverBuilder) o; return Objects.equals(value, that.value); } diff --git a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/rules/80_query_rules_retriever.yml b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/rules/80_query_rules_retriever.yml index c359b5c406dbe..7967516c6ad5a 100644 --- a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/rules/80_query_rules_retriever.yml +++ b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/rules/80_query_rules_retriever.yml @@ -293,67 +293,67 @@ setup: - match: { hits.total.value: 3 } - match: { hits.hits.0._id: foo } - - match: { hits.hits.0._score: 1.7014124430769804E38 - - match: { hits.hits.1._score: 0 } - - match: { hits.hits.2._score: 0 } - - - do: - headers: - # Force JSON content type so that we use a parser that interprets the floating-point score as a double - Content-Type: application/json - search: - index: test-index1 - body: - retriever: - rule: - match_criteria: - foo: foo - bar: bar - ruleset_ids: - test-ruleset - retriever: - standard: - query: - query_string: - query: bar - rank_window_size: 2 - - - match: { hits.total.value: 3 } - - match: { hits.hits.0._id: foo } - - match: { hits.hits.0._score: 1.7014124E38 } - - match: { hits.hits.1._id: foo2 } - - match: { hits.hits.1._score: 1.7014122E38 } - - match: { hits.hits.2._id: bar_no_rule } - - match: { hits.hits.2._score: 0 } - - - do: - headers: - # Force JSON content type so that we use a parser that interprets the floating-point score as a double - Content-Type: application/json - search: - index: test-index1 - body: - retriever: - rule: - match_criteria: - foo: foo - bar: bar - ruleset_ids: - test-ruleset - retriever: - standard: - query: - query_string: - query: bar - rank_window_size: 10 - - - match: { hits.total.value: 3 } - - match: { hits.hits.0._id: foo } - - match: { hits.hits.0._score: 1.7014124E38 } - - match: { hits.hits.1._id: foo2 } - - match: { hits.hits.1._score: 1.7014122E38 } - - match: { hits.hits.2._id: bar_no_rule } - - match: { hits.hits.2._score: 0.87832844 } + - match: { hits.hits.0._score: 1.7014124E38 } + - match: { hits.hits.1._score: 0 } + - match: { hits.hits.2._score: 0 } + + - do: + headers: + # Force JSON content type so that we use a parser that interprets the floating-point score as a double + Content-Type: application/json + search: + index: test-index1 + body: + retriever: + rule: + match_criteria: + foo: foo + bar: bar + ruleset_ids: + test-ruleset + retriever: + standard: + query: + query_string: + query: bar + rank_window_size: 2 + + - match: { hits.total.value: 3 } + - match: { hits.hits.0._id: foo } + - match: { hits.hits.0._score: 1.7014124E38 } + - match: { hits.hits.1._id: foo2 } + - match: { hits.hits.1._score: 1.7014122E38 } + - match: { hits.hits.2._id: bar_no_rule } + - match: { hits.hits.2._score: 0 } + + - do: + headers: + # Force JSON content type so that we use a parser that interprets the floating-point score as a double + Content-Type: application/json + search: + index: test-index1 + body: + retriever: + rule: + match_criteria: + foo: foo + bar: bar + ruleset_ids: + test-ruleset + retriever: + standard: + query: + query_string: + query: bar + rank_window_size: 10 + + - match: { hits.total.value: 3 } + - match: { hits.hits.0._id: foo } + - match: { hits.hits.0._score: 1.7014124E38 } + - match: { hits.hits.1._id: foo2 } + - match: { hits.hits.1._score: 1.7014122E38 } + - match: { hits.hits.2._id: bar_no_rule } + - match: { hits.hits.2._score: 0.87832844 } --- "query rules will error if sorting by anything other than score": @@ -404,7 +404,7 @@ setup: explain: true - match: { hits.hits.0._id: foo } - - match: { hits.hits.0._explanation.value: 1.7014124430769804E38 } + - match: { hits.hits.0._explanation.value: 1.7014124E38 } - match: { hits.hits.0._explanation.description: "query rules evaluated rules from rulesets [test-ruleset] and match criteria {bar=bar, foo=foo}" } - match: { hits.hits.0._explanation.details.0.value: 1 } - match: { hits.hits.0._explanation.details.0.description: "doc [0] with an original score of [1.7014124E38] is at rank [1] from the following source queries." } diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java index a993d4ad1b2e7..9ef2f630b50bd 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java @@ -153,6 +153,8 @@ public void doToXContent(XContentBuilder builder, Params params) throws IOExcept builder.mapContents(matchCriteria); builder.endObject(); builder.field(RETRIEVER_FIELD.getPreferredName(), innerRetrievers.getFirst().retriever()); + // We need to explicitly include this here as it's not propagated by the wrapper + builder.field(RANK_WINDOW_SIZE_FIELD.getPreferredName(), rankWindowSize); } @Override