Skip to content

Commit 3f023c4

Browse files
committed
Refactored code to clean it up based on PR comments
1 parent f5dc56e commit 3f023c4

File tree

2 files changed

+49
-22
lines changed

2 files changed

+49
-22
lines changed

x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/EnterpriseSearchModuleTestUtils.java

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -99,27 +99,28 @@ public static QueryRule randomQueryRule() {
9999
return new QueryRule(ruleId, type, criteria, randomQueryRuleActions(), randomQueryRulePriority());
100100
}
101101

102-
public static QueryRuleCriteria randomQueryRuleCriteria() {
103-
QueryRuleCriteriaType criteriaType = randomFrom(QueryRuleCriteriaType.values());
104-
String metadata = randomAlphaOfLengthBetween(3, 10);
102+
private static List<Object> generateRandomValues(QueryRuleCriteriaType criteriaType, int numValues) {
103+
if (criteriaType == QueryRuleCriteriaType.ALWAYS) {
104+
return null;
105+
}
106+
105107
List<Object> values = new ArrayList<>();
106-
int numValues = randomIntBetween(1, 3);
107-
108-
if (List.of(QueryRuleCriteriaType.GT, QueryRuleCriteriaType.GTE, QueryRuleCriteriaType.LT, QueryRuleCriteriaType.LTE)
109-
.contains(criteriaType)) {
110-
for (int i = 0; i < numValues; i++) {
111-
BigDecimal value = BigDecimal.valueOf(randomDoubleBetween(0, 1000, true)).setScale(2, RoundingMode.HALF_UP);
112-
values.add(value.toString());
113-
}
114-
} else if (criteriaType == QueryRuleCriteriaType.ALWAYS) {
115-
metadata = null;
116-
values = null;
117-
} else {
118-
for (int i = 0; i < numValues; i++) {
119-
values.add(randomAlphaOfLengthBetween(3, 10));
120-
}
108+
for (int i = 0; i < numValues; i++) {
109+
values.add(
110+
List.of(QueryRuleCriteriaType.GT, QueryRuleCriteriaType.GTE, QueryRuleCriteriaType.LT, QueryRuleCriteriaType.LTE)
111+
.contains(criteriaType)
112+
? BigDecimal.valueOf(randomDoubleBetween(0, 1000, true)).setScale(2, RoundingMode.HALF_UP).toString()
113+
: randomAlphaOfLengthBetween(3, 10)
114+
);
121115
}
116+
return values;
117+
}
122118

119+
public static QueryRuleCriteria randomQueryRuleCriteria() {
120+
QueryRuleCriteriaType criteriaType = randomFrom(QueryRuleCriteriaType.values());
121+
String metadata = criteriaType == QueryRuleCriteriaType.ALWAYS ? null : randomAlphaOfLengthBetween(3, 10);
122+
int numValues = randomIntBetween(1, 3);
123+
List<Object> values = generateRandomValues(criteriaType, numValues);
123124
return new QueryRuleCriteria(criteriaType, metadata, values);
124125
}
125126

x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/rules/QueryRuleTests.java

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import org.elasticsearch.xpack.application.EnterpriseSearchModuleTestUtils;
2323
import org.elasticsearch.xpack.searchbusinessrules.SpecifiedDocument;
2424
import org.junit.Before;
25+
import org.slf4j.Logger;
26+
import org.slf4j.LoggerFactory;
2527

2628
import java.io.IOException;
2729
import java.util.Collections;
@@ -39,6 +41,7 @@
3941

4042
public class QueryRuleTests extends ESTestCase {
4143
private NamedWriteableRegistry namedWriteableRegistry;
44+
private static final Logger logger = LoggerFactory.getLogger(QueryRuleTests.class);
4245

4346
@Before
4447
public void registerNamedObjects() {
@@ -83,7 +86,13 @@ public void testNumericValidationWithInvalidValues() throws IOException {
8386
"ids": ["id1"]
8487
}
8588
}""");
86-
expectThrows(IllegalArgumentException.class, () -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON));
89+
IllegalArgumentException e = expectThrows(
90+
IllegalArgumentException.class,
91+
() -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON)
92+
);
93+
logger.info("Actual error message for invalid values: " + e.getMessage());
94+
assertTrue("Error message [" + e.getMessage() + "] should contain 'Failed to build [query_rule]'",
95+
e.getMessage().contains("Failed to build [query_rule]"));
8796
}
8897

8998
public void testNumericValidationWithMixedValues() throws IOException {
@@ -98,7 +107,13 @@ public void testNumericValidationWithMixedValues() throws IOException {
98107
"ids": ["id1"]
99108
}
100109
}""");
101-
expectThrows(IllegalArgumentException.class, () -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON));
110+
IllegalArgumentException e = expectThrows(
111+
IllegalArgumentException.class,
112+
() -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON)
113+
);
114+
logger.info("Actual error message for mixed values: " + e.getMessage());
115+
assertTrue("Error message [" + e.getMessage() + "] should contain 'Failed to build [query_rule]'",
116+
e.getMessage().contains("Failed to build [query_rule]"));
102117
}
103118

104119
public void testNumericValidationWithEmptyValues() throws IOException {
@@ -113,7 +128,12 @@ public void testNumericValidationWithEmptyValues() throws IOException {
113128
"ids": ["id1"]
114129
}
115130
}""");
116-
expectThrows(IllegalArgumentException.class, () -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON));
131+
IllegalArgumentException e = expectThrows(
132+
IllegalArgumentException.class,
133+
() -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON)
134+
);
135+
logger.info("Actual error message: " + e.getMessage());
136+
assertTrue(e.getMessage().contains("failed to parse field [criteria]"));
117137
}
118138

119139
public void testToXContent() throws IOException {
@@ -140,7 +160,13 @@ public void testToXContentEmptyCriteria() throws IOException {
140160
"criteria": [],
141161
"actions": {}
142162
}""");
143-
expectThrows(IllegalArgumentException.class, () -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON));
163+
IllegalArgumentException e = expectThrows(
164+
IllegalArgumentException.class,
165+
() -> QueryRule.fromXContentBytes(new BytesArray(content), XContentType.JSON)
166+
);
167+
logger.info("Actual error message for empty criteria: " + e.getMessage());
168+
assertTrue("Error message [" + e.getMessage() + "] should contain 'Failed to build [query_rule]'",
169+
e.getMessage().contains("Failed to build [query_rule]"));
144170
}
145171

146172
public void testToXContentValidPinnedRulesWithIds() throws IOException {

0 commit comments

Comments
 (0)