diff --git a/docs/changelog/122823.yaml b/docs/changelog/122823.yaml new file mode 100644 index 0000000000000..e0d44cc261f2c --- /dev/null +++ b/docs/changelog/122823.yaml @@ -0,0 +1,5 @@ +pr: 122823 +summary: Prevent Query Rule Creation with Invalid Numeric Match Criteria +area: Relevance +type: bug +issues: [] diff --git a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/rules/10_query_ruleset_put.yml b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/rules/10_query_ruleset_put.yml index ee94cff46d1fb..24887b760d445 100644 --- a/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/rules/10_query_ruleset_put.yml +++ b/x-pack/plugin/ent-search/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/entsearch/rules/10_query_ruleset_put.yml @@ -15,6 +15,16 @@ teardown: ruleset_id: test-query-ruleset-recreating ignore: 404 + - do: + query_rules.delete_ruleset: + ruleset_id: invalid_numeric_test + ignore: 404 + + - do: + query_rules.delete_ruleset: + ruleset_id: multiple_values_test + ignore: 404 + --- 'Create Query Ruleset': - do: @@ -160,3 +170,51 @@ teardown: - 'id2' - match: { error.type: 'security_exception' } + +--- +"Test numeric comparison rule validation": + - skip: + features: headers + + - requires: + cluster_features: "query_rules.numeric_validation" + reason: "Fixed bug to enforce numeric rule validation starting with 8.19" + + - do: + catch: /Input \[not_a_number\] is not valid for CriteriaType \[lte\]/ + query_rules.put_ruleset: + ruleset_id: invalid_numeric_test + body: + rules: + - rule_id: invalid_numeric_rule + type: pinned + criteria: + - type: lte + metadata: price + values: ["not_a_number"] + actions: + ids: ["1"] + +--- +"Test numeric comparison with multiple values": + - skip: + features: headers + + - requires: + cluster_features: "query_rules.numeric_validation" + reason: "Fixed bug to enforce numeric rule validation starting with 8.19" + + - do: + catch: /Input \[not_a_number\] is not valid for CriteriaType \[lte\]/ + query_rules.put_ruleset: + ruleset_id: multiple_values_test + body: + rules: + - rule_id: multiple_values_rule + type: pinned + criteria: + - type: lte + metadata: price + values: ["100", "not_a_number"] + actions: + ids: ["1"] diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/EnterpriseSearchFeatures.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/EnterpriseSearchFeatures.java index 86882a28ec39f..79985a5772393 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/EnterpriseSearchFeatures.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/EnterpriseSearchFeatures.java @@ -12,6 +12,7 @@ import org.elasticsearch.features.NodeFeature; import org.elasticsearch.xpack.application.analytics.AnalyticsTemplateRegistry; import org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry; +import org.elasticsearch.xpack.application.rules.QueryRule; import org.elasticsearch.xpack.application.rules.action.ListQueryRulesetsAction; import org.elasticsearch.xpack.application.rules.retriever.QueryRuleRetrieverBuilder; @@ -40,4 +41,9 @@ public Map getHistoricalFeatures() { Version.V_8_12_0 ); } + + @Override + public Set getTestFeatures() { + return Set.of(QueryRule.NUMERIC_VALIDATION); + } } diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRule.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRule.java index e1734268fbf46..22bb1673cb638 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRule.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRule.java @@ -16,6 +16,7 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.Nullable; +import org.elasticsearch.features.NodeFeature; import org.elasticsearch.xcontent.ConstructingObjectParser; import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xcontent.ToXContentObject; @@ -81,6 +82,8 @@ public String toString() { } } + public static final NodeFeature NUMERIC_VALIDATION = new NodeFeature("query_rules.numeric_validation", true); + /** * Public constructor. * @@ -140,7 +143,6 @@ public QueryRule(StreamInput in) throws IOException { } private void validate() { - if (priority != null && (priority < MIN_PRIORITY || priority > MAX_PRIORITY)) { throw new IllegalArgumentException("Priority was " + priority + ", must be between " + MIN_PRIORITY + " and " + MAX_PRIORITY); } @@ -155,6 +157,13 @@ private void validate() { throw new ElasticsearchParseException(type.toString() + " query rule actions must contain only one of either ids or docs"); } } + + criteria.forEach(criterion -> { + List values = criterion.criteriaValues(); + if (values != null) { + values.forEach(criterion.criteriaType()::validateInput); + } + }); } private void validateIdOrDocAction(Object action) { diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRuleCriteria.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRuleCriteria.java index 5b07d81d90df0..2e638ecf64ffe 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRuleCriteria.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/rules/QueryRuleCriteria.java @@ -55,6 +55,7 @@ public class QueryRuleCriteria implements Writeable, ToXContentObject { public QueryRuleCriteria(QueryRuleCriteriaType criteriaType, @Nullable String criteriaMetadata, @Nullable List criteriaValues) { Objects.requireNonNull(criteriaType); + this.criteriaType = criteriaType; if (criteriaType != ALWAYS) { if (Strings.isNullOrEmpty(criteriaMetadata)) { @@ -63,22 +64,35 @@ public QueryRuleCriteria(QueryRuleCriteriaType criteriaType, @Nullable String cr if (criteriaValues == null || criteriaValues.isEmpty()) { throw new IllegalArgumentException("criteriaValues cannot be null or empty"); } + this.criteriaMetadata = criteriaMetadata; + this.criteriaValues = criteriaValues; + } else { + this.criteriaMetadata = ""; + this.criteriaValues = List.of(); } - - this.criteriaMetadata = criteriaMetadata; - this.criteriaValues = criteriaValues; - this.criteriaType = criteriaType; - } public QueryRuleCriteria(StreamInput in) throws IOException { this.criteriaType = in.readEnum(QueryRuleCriteriaType.class); - if (in.getTransportVersion().onOrAfter(CRITERIA_METADATA_VALUES_TRANSPORT_VERSION)) { - this.criteriaMetadata = in.readOptionalString(); - this.criteriaValues = in.readOptionalCollectionAsList(StreamInput::readGenericValue); + if (this.criteriaType == QueryRuleCriteriaType.ALWAYS) { + this.criteriaMetadata = ""; + if (in.getTransportVersion().onOrAfter(CRITERIA_METADATA_VALUES_TRANSPORT_VERSION)) { + in.readOptionalString(); + in.readOptionalCollectionAsList(StreamInput::readGenericValue); + } else { + in.readString(); + in.readGenericValue(); + } + this.criteriaValues = List.of(); } else { - this.criteriaMetadata = in.readString(); - this.criteriaValues = List.of(in.readGenericValue()); + if (in.getTransportVersion().onOrAfter(CRITERIA_METADATA_VALUES_TRANSPORT_VERSION)) { + this.criteriaMetadata = in.readOptionalString(); + this.criteriaValues = in.readOptionalCollectionAsList(StreamInput::readGenericValue); + } else { + this.criteriaMetadata = in.readString(); + Object value = in.readGenericValue(); + this.criteriaValues = value != null ? List.of(value) : List.of(); + } } } @@ -86,11 +100,22 @@ public QueryRuleCriteria(StreamInput in) throws IOException { public void writeTo(StreamOutput out) throws IOException { out.writeEnum(criteriaType); if (out.getTransportVersion().onOrAfter(CRITERIA_METADATA_VALUES_TRANSPORT_VERSION)) { - out.writeOptionalString(criteriaMetadata); - out.writeOptionalCollection(criteriaValues, StreamOutput::writeGenericValue); + if (criteriaType == QueryRuleCriteriaType.ALWAYS) { + out.writeOptionalString(""); + out.writeOptionalCollection(List.of(), StreamOutput::writeGenericValue); + } else { + out.writeOptionalString(criteriaMetadata); + out.writeOptionalCollection(criteriaValues, StreamOutput::writeGenericValue); + } } else { - out.writeString(criteriaMetadata); - out.writeGenericValue(criteriaValues().get(0)); + if (criteriaType == QueryRuleCriteriaType.ALWAYS) { + out.writeString(""); + out.writeGenericValue(""); + } else { + out.writeString(criteriaMetadata != null ? criteriaMetadata : ""); + Object valueToWrite = criteriaValues != null && criteriaValues.isEmpty() == false ? criteriaValues.get(0) : ""; + out.writeGenericValue(valueToWrite); + } } } @@ -147,11 +172,16 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject(); { builder.field(TYPE_FIELD.getPreferredName(), criteriaType); - if (criteriaMetadata != null) { - builder.field(METADATA_FIELD.getPreferredName(), criteriaMetadata); - } - if (criteriaValues != null) { - builder.array(VALUES_FIELD.getPreferredName(), criteriaValues.toArray()); + if (criteriaType == ALWAYS) { + builder.field(METADATA_FIELD.getPreferredName(), ""); + builder.startArray(VALUES_FIELD.getPreferredName()).endArray(); + } else { + if (criteriaMetadata != null) { + builder.field(METADATA_FIELD.getPreferredName(), criteriaMetadata); + } + if (criteriaValues != null) { + builder.array(VALUES_FIELD.getPreferredName(), criteriaValues.toArray()); + } } } builder.endObject(); diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/EnterpriseSearchModuleTestUtils.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/EnterpriseSearchModuleTestUtils.java index 190b3c3e53169..4aa0e337a7568 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/EnterpriseSearchModuleTestUtils.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/EnterpriseSearchModuleTestUtils.java @@ -20,25 +20,26 @@ import org.elasticsearch.xpack.application.search.TemplateParamValidator; import org.elasticsearch.xpack.core.action.util.PageParams; +import java.math.BigDecimal; +import java.math.RoundingMode; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import static org.elasticsearch.test.ESTestCase.generateRandomStringArray; import static org.elasticsearch.test.ESTestCase.randomAlphaOfLengthBetween; import static org.elasticsearch.test.ESTestCase.randomBoolean; +import static org.elasticsearch.test.ESTestCase.randomDoubleBetween; import static org.elasticsearch.test.ESTestCase.randomFrom; import static org.elasticsearch.test.ESTestCase.randomIdentifier; import static org.elasticsearch.test.ESTestCase.randomIntBetween; -import static org.elasticsearch.test.ESTestCase.randomList; import static org.elasticsearch.test.ESTestCase.randomLongBetween; import static org.elasticsearch.test.ESTestCase.randomMap; import static org.elasticsearch.xpack.application.rules.QueryRule.MAX_PRIORITY; import static org.elasticsearch.xpack.application.rules.QueryRule.MIN_PRIORITY; -import static org.elasticsearch.xpack.application.rules.QueryRuleCriteriaType.ALWAYS; public final class EnterpriseSearchModuleTestUtils { @@ -86,19 +87,48 @@ public static Map randomSearchApplicationQueryParams() { return randomMap(0, 10, () -> Tuple.tuple(randomIdentifier(), randomAlphaOfLengthBetween(0, 10))); } - public static QueryRuleCriteria randomQueryRuleCriteria() { - // We intentionally don't allow ALWAYS criteria in this method, since we want to test parsing metadata and values - QueryRuleCriteriaType type = randomFrom(Arrays.stream(QueryRuleCriteriaType.values()).filter(t -> t != ALWAYS).toList()); - return new QueryRuleCriteria(type, randomAlphaOfLengthBetween(1, 10), randomList(1, 5, () -> randomAlphaOfLengthBetween(1, 10))); - } - public static QueryRule randomQueryRule() { - String id = randomIdentifier(); + String ruleId = randomIdentifier(); QueryRule.QueryRuleType type = randomFrom(QueryRule.QueryRuleType.values()); - List criteria = List.of(randomQueryRuleCriteria()); - Map actions = Map.of(randomFrom("ids", "docs"), List.of(randomAlphaOfLengthBetween(2, 10))); - Integer priority = randomQueryRulePriority(); - return new QueryRule(id, type, criteria, actions, priority); + List criteria = new ArrayList<>(); + int numCriteria = randomIntBetween(1, 3); + + for (int i = 0; i < numCriteria; i++) { + criteria.add(randomQueryRuleCriteria()); + } + + return new QueryRule(ruleId, type, criteria, randomQueryRuleActions(), randomQueryRulePriority()); + } + + public static QueryRuleCriteria randomQueryRuleCriteria() { + Set numericValueCriteriaType = Set.of( + QueryRuleCriteriaType.GT, + QueryRuleCriteriaType.GTE, + QueryRuleCriteriaType.LT, + QueryRuleCriteriaType.LTE + ); + QueryRuleCriteriaType criteriaType = randomFrom(QueryRuleCriteriaType.values()); + + String metadata; + List values; + if (criteriaType == QueryRuleCriteriaType.ALWAYS) { + metadata = null; + values = null; + } else { + metadata = randomAlphaOfLengthBetween(3, 10); + values = new ArrayList<>(); + + int numValues = randomIntBetween(1, 3); + for (int i = 0; i < numValues; i++) { + values.add( + numericValueCriteriaType.contains(criteriaType) + ? BigDecimal.valueOf(randomDoubleBetween(0, 1000, true)).setScale(2, RoundingMode.HALF_UP).toString() + : randomAlphaOfLengthBetween(3, 10) + ); + } + } + + return new QueryRuleCriteria(criteriaType, metadata, values); } public static Integer randomQueryRulePriority() { @@ -115,8 +145,24 @@ public static QueryRuleset randomQueryRuleset() { return new QueryRuleset(id, rules); } + public static Map randomQueryRuleActions() { + if (randomBoolean()) { + return Map.of(QueryRule.IDS_FIELD.getPreferredName(), List.of(randomAlphaOfLengthBetween(3, 10))); + } + return Map.of( + QueryRule.DOCS_FIELD.getPreferredName(), + List.of( + Map.of( + QueryRule.INDEX_FIELD.getPreferredName(), + randomAlphaOfLengthBetween(3, 10), + QueryRule.ID_FIELD.getPreferredName(), + randomAlphaOfLengthBetween(3, 10) + ) + ) + ); + } + public static Map randomMatchCriteria() { return randomMap(1, 3, () -> Tuple.tuple(randomIdentifier(), randomAlphaOfLengthBetween(0, 10))); } - } diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/QueryRuleTests.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/QueryRuleTests.java index 67e7f6ac7d9e9..a0bf53f1c88e4 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/QueryRuleTests.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/QueryRuleTests.java @@ -15,6 +15,8 @@ import org.elasticsearch.search.SearchModule; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.ToXContent; +import org.elasticsearch.xcontent.XContentBuilder; +import org.elasticsearch.xcontent.XContentFactory; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.application.EnterpriseSearchModuleTestUtils; @@ -30,8 +32,10 @@ import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; import static org.elasticsearch.xpack.application.rules.QueryRuleCriteriaType.EXACT; +import static org.elasticsearch.xpack.application.rules.QueryRuleCriteriaType.LTE; import static org.elasticsearch.xpack.application.rules.QueryRuleCriteriaType.PREFIX; import static org.elasticsearch.xpack.application.rules.QueryRuleCriteriaType.SUFFIX; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; public class QueryRuleTests extends ESTestCase { @@ -53,6 +57,78 @@ public final void testRandomSerialization() throws IOException { } } + public void testNumericValidationWithValidValues() throws IOException { + String content = XContentHelper.stripWhitespace(""" + { + "rule_id": "numeric_rule", + "type": "pinned", + "criteria": [ + { "type": "lte", "metadata": "price", "values": ["100.50", "200"] } + ], + "actions": { + "ids": ["id1"] + } + }"""); + testToXContentRules(content); + } + + public void testNumericValidationWithInvalidValues() throws IOException { + String content = XContentHelper.stripWhitespace(""" + { + "rule_id": "numeric_rule", + "type": "pinned", + "criteria": [ + { "type": "lte", "metadata": "price", "values": ["abc"] } + ], + "actions": { + "ids": ["id1"] + } + }"""); + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON) + ); + assertThat(e.getMessage(), containsString("Failed to build [query_rule]")); + } + + public void testNumericValidationWithMixedValues() throws IOException { + String content = XContentHelper.stripWhitespace(""" + { + "rule_id": "numeric_rule", + "type": "pinned", + "criteria": [ + { "type": "lte", "metadata": "price", "values": ["100", "abc", "200"] } + ], + "actions": { + "ids": ["id1"] + } + }"""); + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON) + ); + assertThat(e.getMessage(), containsString("Failed to build [query_rule]")); + } + + public void testNumericValidationWithEmptyValues() throws IOException { + String content = XContentHelper.stripWhitespace(""" + { + "rule_id": "numeric_rule", + "type": "pinned", + "criteria": [ + { "type": "lte", "metadata": "price", "values": [] } + ], + "actions": { + "ids": ["id1"] + } + }"""); + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON) + ); + assertThat(e.getMessage(), containsString("failed to parse field [criteria]")); + } + public void testToXContent() throws IOException { String content = XContentHelper.stripWhitespace(""" { @@ -66,15 +142,7 @@ public void testToXContent() throws IOException { }, "priority": 5 }"""); - - QueryRule queryRule = QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON); - boolean humanReadable = true; - BytesReference originalBytes = toShuffledXContent(queryRule, XContentType.JSON, ToXContent.EMPTY_PARAMS, humanReadable); - QueryRule parsed; - try (XContentParser parser = createParser(XContentType.JSON.xContent(), originalBytes)) { - parsed = QueryRule.fromXContent(parser); - } - assertToXContentEquivalent(originalBytes, toXContent(parsed, XContentType.JSON, humanReadable), XContentType.JSON); + testToXContentRules(content); } public void testToXContentEmptyCriteria() throws IOException { @@ -85,7 +153,11 @@ public void testToXContentEmptyCriteria() throws IOException { "criteria": [], "actions": {} }"""); - expectThrows(IllegalArgumentException.class, () -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON)); + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON) + ); + assertThat(e.getMessage(), containsString("Failed to build [query_rule]")); } public void testToXContentValidPinnedRulesWithIds() throws IOException { @@ -333,6 +405,62 @@ public void testApplyRuleWithMultipleCriteria() throws IOException { assertEquals(Collections.emptyList(), appliedQueryRules.excludedDocs()); } + public void testValidateNumericComparisonRule() { + IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> new QueryRule( + "test_rule", + QueryRule.QueryRuleType.PINNED, + List.of(new QueryRuleCriteria(QueryRuleCriteriaType.LTE, "price", List.of("not_a_number"))), + Map.of("ids", List.of("1")), + null + ) + ); + assertEquals("Input [not_a_number] is not valid for CriteriaType [lte]", e.getMessage()); + } + + public void testParseNumericComparisonRule() throws IOException { + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + { + builder.field("rule_id", "test_rule"); + builder.field("type", "pinned"); + builder.startArray("criteria"); + { + builder.startObject(); + builder.field("type", "lte"); + builder.field("metadata", "price"); + builder.startArray("values").value("100").endArray(); + builder.endObject(); + } + builder.endArray(); + builder.startObject("actions"); + { + builder.startArray("ids").value("1").endArray(); + } + builder.endObject(); + } + builder.endObject(); + + BytesReference bytesRef = BytesReference.bytes(builder); + QueryRule rule = QueryRule.fromXContentBytes(bytesRef, builder.contentType()); + assertNotNull(rule); + assertEquals("test_rule", rule.id()); + assertEquals(QueryRule.QueryRuleType.PINNED, rule.type()); + assertEquals(1, rule.criteria().size()); + assertEquals(LTE, rule.criteria().get(0).criteriaType()); + assertEquals("100", rule.criteria().get(0).criteriaValues().get(0)); + } + + public void testIsRuleMatchWithNumericComparison() { + QueryRuleCriteria criteria = new QueryRuleCriteria(LTE, "price", List.of("100")); + QueryRule rule = new QueryRule("test_rule", QueryRule.QueryRuleType.PINNED, List.of(criteria), Map.of("ids", List.of("1")), null); + + assertTrue(rule.isRuleMatch(Map.of("price", "50"))); + assertFalse(rule.isRuleMatch(Map.of("price", "150"))); + assertFalse(rule.isRuleMatch(Map.of("price", "not_a_number"))); + } + private void assertXContent(QueryRule queryRule, boolean humanReadable) throws IOException { BytesReference originalBytes = toShuffledXContent(queryRule, XContentType.JSON, ToXContent.EMPTY_PARAMS, humanReadable); QueryRule parsed; diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/action/GetQueryRulesetActionResponseBWCSerializingTests.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/action/GetQueryRulesetActionResponseBWCSerializingTests.java index 8bfbcd5ec471b..0813317edfb16 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/action/GetQueryRulesetActionResponseBWCSerializingTests.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/action/GetQueryRulesetActionResponseBWCSerializingTests.java @@ -54,9 +54,12 @@ protected GetQueryRulesetAction.Response mutateInstanceForVersion(GetQueryRulese for (QueryRule rule : instance.queryRuleset().rules()) { List newCriteria = new ArrayList<>(); for (QueryRuleCriteria criteria : rule.criteria()) { - newCriteria.add( - new QueryRuleCriteria(criteria.criteriaType(), criteria.criteriaMetadata(), criteria.criteriaValues().subList(0, 1)) - ); + List values = criteria.criteriaValues(); + if (values != null && values.isEmpty() == false) { + newCriteria.add(new QueryRuleCriteria(criteria.criteriaType(), criteria.criteriaMetadata(), values.subList(0, 1))); + } else { + newCriteria.add(new QueryRuleCriteria(criteria.criteriaType(), criteria.criteriaMetadata(), null)); + } } rules.add(new QueryRule(rule.id(), rule.type(), newCriteria, rule.actions(), null)); } diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/action/PutQueryRulesetActionRequestBWCSerializingTests.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/action/PutQueryRulesetActionRequestBWCSerializingTests.java index 3b6195cc8f495..77a2ed3b1b8c2 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/action/PutQueryRulesetActionRequestBWCSerializingTests.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/action/PutQueryRulesetActionRequestBWCSerializingTests.java @@ -56,9 +56,12 @@ protected PutQueryRulesetAction.Request mutateInstanceForVersion(PutQueryRuleset for (QueryRule rule : instance.queryRuleset().rules()) { List newCriteria = new ArrayList<>(); for (QueryRuleCriteria criteria : rule.criteria()) { - newCriteria.add( - new QueryRuleCriteria(criteria.criteriaType(), criteria.criteriaMetadata(), criteria.criteriaValues().subList(0, 1)) - ); + List values = criteria.criteriaValues(); + if (values != null && values.isEmpty() == false) { + newCriteria.add(new QueryRuleCriteria(criteria.criteriaType(), criteria.criteriaMetadata(), values.subList(0, 1))); + } else { + newCriteria.add(new QueryRuleCriteria(criteria.criteriaType(), criteria.criteriaMetadata(), null)); + } } rules.add(new QueryRule(rule.id(), rule.type(), newCriteria, rule.actions(), null)); }