This is an automated email from the ASF dual-hosted git repository.
davidzollo pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/seatunnel.git
The following commit(s) were added to refs/heads/dev by this push:
new e8846b1011 [Fix][API] Fix validateSingleChoice and extension
exceptions bypassing error aggregation (#11121)
e8846b1011 is described below
commit e8846b10112849a5e350ddbcecf9654c4a3f7099
Author: zhiwei.niu <[email protected]>
AuthorDate: Mon Jun 29 22:49:33 2026 +0800
[Fix][API] Fix validateSingleChoice and extension exceptions bypassing
error aggregation (#11121)
---
.../api/configuration/util/ConditionExtension.java | 13 ++-
.../api/configuration/util/ConfigValidator.java | 99 ++++++++++------
.../api/configuration/util/OptionUtil.java | 13 ++-
.../configuration/util/ConfigValidatorTest.java | 127 ++++++++++++++++++++-
4 files changed, 207 insertions(+), 45 deletions(-)
diff --git
a/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConditionExtension.java
b/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConditionExtension.java
index 2048059678..df27000637 100644
---
a/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConditionExtension.java
+++
b/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConditionExtension.java
@@ -50,14 +50,19 @@ public interface ConditionExtension<T> {
* Evaluates whether {@code value} passes this validation rule.
*
* <p>Return {@code false} for simple failure — the framework composes the
error from {@link
- * #description()} automatically. Throw {@link OptionValidationException}
when a richer,
- * context-specific message is needed. Avoid other unchecked exceptions —
they propagate
- * unwrapped.
+ * #description()} automatically and continues collecting other validation
errors.
+ *
+ * <p>Throw {@link OptionValidationException} when a richer,
context-specific message is needed.
+ * The framework catches the exception, extracts its message, and adds it
to the aggregated
+ * error list — subsequent validations still run. Use this when you need a
more descriptive
+ * error message than {@link #description()} alone provides.
+ *
+ * <p>Avoid other unchecked exceptions — they propagate unwrapped.
*
* @param config full configuration context (read-only), available for
cross-field checks
* @param value the resolved option value; may be {@code null}
* @return {@code true} if valid
- * @throws OptionValidationException for detailed error reporting
+ * @throws OptionValidationException for detailed, context-specific error
reporting
*/
boolean evaluate(ReadonlyConfig config, T value) throws
OptionValidationException;
}
diff --git
a/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConfigValidator.java
b/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConfigValidator.java
index ae7e4450f9..42d1b1c7b9 100644
---
a/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConfigValidator.java
+++
b/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConfigValidator.java
@@ -30,18 +30,28 @@ import org.apache.commons.collections4.CollectionUtils;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
+import static
org.apache.seatunnel.api.configuration.util.OptionUtil.formatError;
+import static
org.apache.seatunnel.api.configuration.util.OptionUtil.formatOptionsError;
import static
org.apache.seatunnel.api.configuration.util.OptionUtil.getOptionKeys;
public class ConfigValidator {
private final ReadonlyConfig config;
+ /** Closed set of validation error categories used in formatted error
messages. */
+ private static final String TYPE_REQUIRED = "required";
+
+ private static final String TYPE_VALUE = "value";
+ private static final String TYPE_BUNDLED = "bundled";
+ private static final String TYPE_EXCLUSIVE = "exclusive";
+ private static final String TYPE_CONDITIONAL = "conditional";
+ private static final String TYPE_SINGLE_CHOICE = "singleChoice";
+
private static final Set<String> COMMON_KEYS = new HashSet<>();
static {
@@ -191,14 +201,14 @@ public class ConfigValidator {
(RequiredOption.ConditionalRequiredOptions) requiredOption)) {
continue;
}
- validateSingleChoice(option);
+ validateSingleChoice(option, errors);
}
}
}
for (Option option : rule.getOptionalOptions()) {
if (SingleChoiceOption.class.isAssignableFrom(option.getClass())) {
- validateSingleChoice(option);
+ validateSingleChoice(option, errors);
}
}
@@ -212,11 +222,20 @@ public class ConfigValidator {
if (structurallyAbsentKeys.contains(constraint.getOption().key()))
{
continue;
}
- if (isConstraintApplicable(constraint, rule) &&
!validate(constraint)) {
+ if (!isConstraintApplicable(constraint, rule)) {
+ continue;
+ }
+ try {
+ if (!validate(constraint)) {
+ errors.add(
+ formatError(
+ constraint.getOption().key(),
+ TYPE_VALUE,
+ constraint.toString()));
+ }
+ } catch (OptionValidationException e) {
errors.add(
- String.format(
- "option: %s\n type: value\n
constraint: %s",
- constraint.getOption().key(),
constraint.toString()));
+ formatError(constraint.getOption().key(), TYPE_VALUE,
e.getRawMessage()));
}
}
}
@@ -308,29 +327,32 @@ public class ConfigValidator {
return false;
}
- void validateSingleChoice(Option option) {
+ void validateSingleChoice(Option option, List<String> errors) {
SingleChoiceOption singleChoiceOption = (SingleChoiceOption) option;
List optionValues = singleChoiceOption.getOptionValues();
if (CollectionUtils.isEmpty(optionValues)) {
- throw new OptionValidationException(
- "These options(%s) are SingleChoiceOption, the
optionValues must not be null.",
-
getOptionKeys(Collections.singletonList(singleChoiceOption)));
+ errors.add(
+ formatError(
+ option.key(), TYPE_SINGLE_CHOICE, "optionValues
must not be empty"));
+ return;
}
Object o = singleChoiceOption.defaultValue();
if (o != null && !optionValues.contains(o)) {
- throw new OptionValidationException(
- "These options(%s) are SingleChoiceOption, the
defaultValue(%s) must be one of the optionValues(%s).",
-
getOptionKeys(Collections.singletonList(singleChoiceOption)), o, optionValues);
+ errors.add(
+ formatError(
+ option.key(),
+ TYPE_SINGLE_CHOICE,
+ String.format("defaultValue(%s) must be one of
%s", o, optionValues)));
}
Object value = config.get(option);
if (value != null && !optionValues.contains(value)) {
- throw new OptionValidationException(
- "These options(%s) are SingleChoiceOption, the value(%s)
must be one of the optionValues(%s).",
-
getOptionKeys(Collections.singletonList(singleChoiceOption)),
- value,
- optionValues);
+ errors.add(
+ formatError(
+ option.key(),
+ TYPE_SINGLE_CHOICE,
+ String.format("value(%s) must be one of %s",
value, optionValues)));
}
}
@@ -371,9 +393,10 @@ public class ConfigValidator {
return null;
}
String hint = expression == null ? "" : " when [" + expression + "]";
- return String.format(
- "option: %s\n type: required\n constraint: required
option is not configured%s",
- getOptionKeys(absentOptions), hint);
+ return formatError(
+ getOptionKeys(absentOptions),
+ TYPE_REQUIRED,
+ "required option is not configured" + hint);
}
boolean hasOption(Option<?> option) {
@@ -394,9 +417,12 @@ public class ConfigValidator {
if (present.size() == bundledOptions.size() || absent.size() ==
bundledOptions.size()) {
return null;
}
- return String.format(
- "options: %s\n type: bundled\n constraint: bundled
options must be present or absent together (present: [%s], absent: [%s])",
- getOptionKeys(bundledOptions), getOptionKeys(present),
getOptionKeys(absent));
+ return formatOptionsError(
+ getOptionKeys(bundledOptions),
+ TYPE_BUNDLED,
+ String.format(
+ "bundled options must be present or absent together
(present: [%s], absent: [%s])",
+ getOptionKeys(present), getOptionKeys(absent)));
}
String checkExclusive(RequiredOption.ExclusiveRequiredOptions
exclusiveRequiredOptions) {
@@ -411,14 +437,17 @@ public class ConfigValidator {
return null;
}
if (count == 0) {
- return String.format(
- "options: %s\n type: exclusive\n constraint:
exactly one option must be set, but none are configured",
-
getOptionKeys(exclusiveRequiredOptions.getExclusiveOptions()));
+ return formatOptionsError(
+
getOptionKeys(exclusiveRequiredOptions.getExclusiveOptions()),
+ TYPE_EXCLUSIVE,
+ "exactly one option must be set, but none are configured");
}
- return String.format(
- "options: %s\n type: exclusive\n constraint:
mutually exclusive, but multiple are set: [%s]",
+ return formatOptionsError(
getOptionKeys(exclusiveRequiredOptions.getExclusiveOptions()),
- getOptionKeys(presentOptions));
+ TYPE_EXCLUSIVE,
+ String.format(
+ "mutually exclusive, but multiple are set: [%s]",
+ getOptionKeys(presentOptions)));
}
String checkConditional(RequiredOption.ConditionalRequiredOptions
conditionalRequiredOptions) {
@@ -431,10 +460,12 @@ public class ConfigValidator {
if (absentOptions.isEmpty()) {
return null;
}
- return String.format(
- "option: %s\n type: conditional\n constraint:
required because [%s] is true",
+ return formatError(
getOptionKeys(absentOptions),
- conditionalRequiredOptions.getExpression().toString());
+ TYPE_CONDITIONAL,
+ String.format(
+ "required because [%s] is true",
+
conditionalRequiredOptions.getExpression().toString()));
}
private boolean validate(Expression expression) {
diff --git
a/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/OptionUtil.java
b/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/OptionUtil.java
index 7a4a838e00..128d59a091 100644
---
a/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/OptionUtil.java
+++
b/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/OptionUtil.java
@@ -22,15 +22,18 @@ import
org.apache.seatunnel.shade.org.apache.commons.lang3.StringUtils;
import org.apache.seatunnel.api.configuration.Option;
+import lombok.experimental.UtilityClass;
+
import java.lang.reflect.Field;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+@UtilityClass
public class OptionUtil {
- private OptionUtil() {}
+ private static final String ERROR_TEMPLATE = "%s: %s\n type: %s\n
constraint: %s";
public static String getOptionKeys(List<Option<?>> options) {
StringBuilder builder = new StringBuilder();
@@ -93,6 +96,14 @@ public class OptionUtil {
return options;
}
+ public static String formatError(String optionKey, String type, String
constraint) {
+ return String.format(ERROR_TEMPLATE, "option", optionKey, type,
constraint);
+ }
+
+ public static String formatOptionsError(String optionKeys, String type,
String constraint) {
+ return String.format(ERROR_TEMPLATE, "options", optionKeys, type,
constraint);
+ }
+
private static String formatUnderScoreCase(String camel) {
StringBuilder underScore =
new
StringBuilder(String.valueOf(Character.toLowerCase(camel.charAt(0))));
diff --git
a/seatunnel-api/src/test/java/org/apache/seatunnel/api/configuration/util/ConfigValidatorTest.java
b/seatunnel-api/src/test/java/org/apache/seatunnel/api/configuration/util/ConfigValidatorTest.java
index 2b859bb059..a0bcd46ff1 100644
---
a/seatunnel-api/src/test/java/org/apache/seatunnel/api/configuration/util/ConfigValidatorTest.java
+++
b/seatunnel-api/src/test/java/org/apache/seatunnel/api/configuration/util/ConfigValidatorTest.java
@@ -296,9 +296,13 @@ public class ConfigValidatorTest {
Map<String, Object> config = new HashMap<>();
config.put(SINGLE_CHOICE_TEST.key(), "A");
Executable executable = () -> validate(config, optionRule);
- assertEquals(
- "ErrorCode:[API-02], ErrorDescription:[Option item validate
failed] - These options('single_choice_test') are SingleChoiceOption, the
defaultValue(M) must be one of the optionValues([A, B, C]).",
- assertThrows(OptionValidationException.class,
executable).getMessage());
+ OptionValidationException ex =
assertThrows(OptionValidationException.class, executable);
+ String msg = ex.getMessage();
+ Assertions.assertTrue(
+ msg.contains("single_choice_test"), "Should mention option
key: " + msg);
+ Assertions.assertTrue(
+ msg.contains("defaultValue(M) must be one of"),
+ "Should mention invalid defaultValue: " + msg);
}
@Test
@@ -311,9 +315,12 @@ public class ConfigValidatorTest {
config.put(SINGLE_CHOICE_VALUE_TEST.key(), "N");
executable = () -> validate(config, optionRule);
- assertEquals(
- "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());
+ OptionValidationException ex =
assertThrows(OptionValidationException.class, executable);
+ String msg = ex.getMessage();
+ Assertions.assertTrue(
+ msg.contains("single_choice_test"), "Should mention option
key: " + msg);
+ Assertions.assertTrue(
+ msg.contains("value(N) must be one of"), "Should mention
invalid value: " + msg);
}
@Test
@@ -3483,4 +3490,112 @@ public class ConfigValidatorTest {
config5.put(TEST_TOPIC.key(), Collections.emptyList());
assertThrows(OptionValidationException.class, () -> validate(config5,
rule));
}
+
+ // ==================== collectErrors contract tests ====================
+
+ @Test
+ public void testMultipleSingleChoiceErrorsCollected() {
+ Option<String> choice1 =
+ Options.key("mode1")
+ .singleChoice(String.class, Arrays.asList("A", "B",
"C"))
+ .defaultValue("A")
+ .withDescription("mode1");
+ Option<String> choice2 =
+ Options.key("mode2")
+ .singleChoice(String.class, Arrays.asList("X", "Y",
"Z"))
+ .defaultValue("X")
+ .withDescription("mode2");
+
+ OptionRule rule = OptionRule.builder().required(choice1,
choice2).build();
+ Map<String, Object> config = new HashMap<>();
+ config.put("mode1", "INVALID1");
+ config.put("mode2", "INVALID2");
+
+ OptionValidationException ex =
+ assertThrows(OptionValidationException.class, () ->
validate(config, rule));
+ String msg = ex.getMessage();
+ Assertions.assertTrue(msg.contains("mode1"), "Should report mode1
error: " + msg);
+ Assertions.assertTrue(msg.contains("mode2"), "Should report mode2
error: " + msg);
+ Assertions.assertTrue(
+ msg.contains("INVALID1") && msg.contains("INVALID2"),
+ "Should report both invalid values: " + msg);
+ }
+
+ @Test
+ public void testMixedErrorTypesAllCollected() {
+ Option<String> choice =
+ Options.key("format")
+ .singleChoice(String.class, Arrays.asList("json",
"csv", "avro"))
+ .defaultValue("json")
+ .withDescription("format");
+ Option<String> host =
+
Options.key("host").stringType().noDefaultValue().withDescription("host");
+ Option<Integer> port =
+
Options.key("port").intType().noDefaultValue().withDescription("port");
+
+ OptionRule rule =
+ OptionRule.builder()
+ .required(choice, host)
+ .required(port, greaterOrEqual(port, 1))
+ .build();
+
+ Map<String, Object> config = new HashMap<>();
+ config.put("format", "xml");
+ config.put("port", 0);
+
+ OptionValidationException ex =
+ assertThrows(OptionValidationException.class, () ->
validate(config, rule));
+ String msg = ex.getMessage();
+ Assertions.assertTrue(msg.contains("host"), "Should report missing
host: " + msg);
+ Assertions.assertTrue(
+ msg.contains("format") && msg.contains("singleChoice"),
+ "Should report single_choice error: " + msg);
+ Assertions.assertTrue(
+ msg.contains("port") && msg.contains("value"),
+ "Should report value constraint error: " + msg);
+ }
+
+ @Test
+ public void testExtensionExceptionIsAggregatedNotFailFast() {
+ ConditionExtension<Integer> throwingExtension =
+ new ConditionExtension<Integer>() {
+ @Override
+ public String description() {
+ return "must be positive";
+ }
+
+ @Override
+ public boolean evaluate(ReadonlyConfig config, Integer
value)
+ throws OptionValidationException {
+ if (value != null && value <= 0) {
+ throw new OptionValidationException(
+ "port value %d is not positive", value);
+ }
+ return true;
+ }
+ };
+
+ OptionRule rule =
+ OptionRule.builder()
+ .required(HOST, notBlank(HOST))
+ .required(PORT, Conditions.extension(PORT,
throwingExtension))
+ .build();
+
+ Map<String, Object> config = new HashMap<>();
+ config.put(HOST.key(), "");
+ config.put(PORT.key(), -1);
+
+ OptionValidationException ex =
+ Assertions.assertThrows(
+ OptionValidationException.class,
+ () ->
ConfigValidator.of(ReadonlyConfig.fromMap(config)).validate(rule));
+ String msg = ex.getMessage();
+ Assertions.assertTrue(msg.contains("2 errors"), "both errors should be
aggregated: " + msg);
+ Assertions.assertTrue(
+ msg.contains("host") && msg.contains("is not blank"),
+ "host notBlank error should appear: " + msg);
+ Assertions.assertTrue(
+ msg.contains("port value -1 is not positive"),
+ "extension exception message should be preserved: " + msg);
+ }
}
