yzeng1618 commented on code in PR #11048:
URL: https://github.com/apache/seatunnel/pull/11048#discussion_r3401332279
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/Condition.java:
##########
@@ -219,6 +234,9 @@ private static String conditionToString(Condition<?> cond) {
ConditionOperator op = cond.operator;
String key = "'" + cond.option.key() + "'";
+ if (op == ConditionOperator.EXTENSION) {
+ return key + " " + cond.extension.description();
Review Comment:
If description() returns null, the concatenated string will be "option_key
null", and NPE may occur in certain serialization scenarios.
##########
seatunnel-core/seatunnel-starter/src/main/java/org/apache/seatunnel/core/starter/seatunnel/command/MetadataExportCommand.java:
##########
@@ -420,10 +420,14 @@ private ObjectNode exportCondition(Condition<?>
condition) {
}
ObjectNode node = mapper.createObjectNode();
node.put("key", condition.getOption().key());
- if (condition.getExpectValue() != null) {
- node.put("expectValue",
String.valueOf(condition.getExpectValue()));
- }
ConditionOperator op = condition.getOperator();
+ Object expectValue = condition.getExpectValue();
+ if (op == ConditionOperator.EXTENSION && condition.getExtension() !=
null) {
+ expectValue = condition.getExtension().description();
+ }
Review Comment:
Code Duplication (OptionRulesService.java 286-288)
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConditionEvaluators.java:
##########
@@ -169,6 +169,14 @@ private static Map<ConditionOperator, Evaluator>
createRegistry() {
return compareNumbers(v, other) >= 0;
});
+ // Extension (custom logic delegated to ConditionExtension)
+ m.put(
+ ConditionOperator.EXTENSION,
+ (v, c, cfg) -> {
+ ConditionExtension<Object> ext =
(ConditionExtension<Object>) c.getExtension();
Review Comment:
The return value of getExtension() is forcibly cast to
ConditionExtension<Object>. Creating mismatched Condition via unconventional
methods like reflection will trigger a runtime ClassCastException and fail job
submission. Type validation logic is recommended.
##########
seatunnel-engine/seatunnel-engine-server/src/main/java/org/apache/seatunnel/engine/server/rest/service/OptionRulesService.java:
##########
@@ -282,9 +282,13 @@ private OptionRuleResponse.ConditionNode
toConditionNode(Condition<?> condition)
: null;
String conditionOperator = (op != null) ? op.name() : null;
String conditionOperatorCategory = (op != null) ?
op.getCategory().name() : null;
+ Object expectValue = condition.getExpectValue();
+ if (op == ConditionOperator.EXTENSION && condition.getExtension() !=
null) {
+ expectValue = condition.getExtension().description();
+ }
Review Comment:
ditto
##########
seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/ConditionEvaluators.java:
##########
@@ -169,6 +169,14 @@ private static Map<ConditionOperator, Evaluator>
createRegistry() {
return compareNumbers(v, other) >= 0;
});
+ // Extension (custom logic delegated to ConditionExtension)
+ m.put(
+ ConditionOperator.EXTENSION,
+ (v, c, cfg) -> {
+ ConditionExtension<Object> ext =
(ConditionExtension<Object>) c.getExtension();
+ return ext.evaluate(cfg, v);
Review Comment:
Add a try-catch block at the call site to catch all Throwables, wrap them
into an OptionValidationException, and ensure the error message contains the
context "Custom validation plugin execution failed".
--
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]
