yzeng1618 commented on code in PR #10592:
URL: https://github.com/apache/seatunnel/pull/10592#discussion_r2922504082
##########
seatunnel-api/src/test/java/org/apache/seatunnel/api/configuration/util/ConfigValidatorTest.java:
##########
@@ -293,4 +293,59 @@ public void testSingleChoiceOptionValueValidator() {
"ErrorCode:[API-02], ErrorDescription:[Option item validate
failed] - These options('single_choice_test') are SingleChoiceOption, the
value(N) must be one of the optionValues([A, B, C]).",
assertThrows(OptionValidationException.class,
executable).getMessage());
}
+
+ @Test
+ public void testNestedOptionRule() {
Review Comment:
It seems that some boundary scenarios are currently missing from the unit
tests. For example passing an empty List for expectValuesăconditionalOptionRule
being an empty OptionRule with no contentăwhether the OR logic can correctly
trigger validation when multiple expectValues are passed.
Please confirm whether these need to be added based on your judgment.
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/OptionRule.java:
##########
Review Comment:
ditto
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/OptionRule.java:
##########
@@ -218,8 +239,63 @@ public Builder bundled(@NonNull Option<?>...
requiredOptions) {
return this;
}
+ public <T> Builder conditionalRule(
+ @NonNull Option<T> conditionalOption,
+ @NonNull List<T> expectValues,
+ @NonNull OptionRule conditionalOptionRule) {
+ verifyConditionalExists(conditionalOption);
+
+ if (expectValues.isEmpty()) {
+ throw new OptionValidationException(
+ String.format(
+ "conditional option '%s' must have expect
values .",
+ conditionalOption.key()));
+ }
+
+ if (!conditionalOptionRule.hasOptions()) {
+ throw new OptionValidationException(
+ String.format(
+ "conditional option rule for '%s' must have
options.",
+ conditionalOption.key()));
+ }
+
+ Expression expression =
+ Expression.of(Condition.of(conditionalOption,
expectValues.get(0)));
+ for (int i = 0; i < expectValues.size(); i++) {
+ if (i != 0) {
Review Comment:
the loop starts at i=0 but skips i=0, which is effectively equivalent to
starting at i=1. Could this logical redundancy be revised?
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/OptionRule.java:
##########
Review Comment:
conditionRules is not included in equals() or hashCode(). Please update both
methods to include it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
