Skip to content

Commit 0472599

Browse files
authored
[Improvement-17942][API&UI]Add startParams validation logic in both the frontend and backend. (#17956)
1 parent 059ed17 commit 0472599

9 files changed

Lines changed: 248 additions & 21 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.dolphinscheduler.api.validator;
19+
20+
import org.apache.dolphinscheduler.plugin.task.api.model.Property;
21+
22+
import org.apache.commons.collections.CollectionUtils;
23+
import org.apache.commons.lang3.StringUtils;
24+
25+
import java.util.HashSet;
26+
import java.util.List;
27+
import java.util.Set;
28+
29+
import lombok.extern.slf4j.Slf4j;
30+
31+
import org.springframework.stereotype.Component;
32+
33+
/**
34+
* Validator for the list of start parameters (Property list).
35+
* <p> If startParamList is not valid, an {@link IllegalArgumentException} will be thrown. </p>
36+
*/
37+
@Slf4j
38+
@Component
39+
public class StartParamListValidator implements IValidator<List<Property>> {
40+
41+
@Override
42+
public void validate(List<Property> startParamList) {
43+
if (CollectionUtils.isEmpty(startParamList)) {
44+
return;
45+
}
46+
47+
Set<String> keys = new HashSet<>();
48+
for (Property param : startParamList) {
49+
if (StringUtils.isBlank(param.getProp())) {
50+
throw new IllegalArgumentException("Parameter key cannot be empty");
51+
}
52+
53+
String key = param.getProp().trim();
54+
if (keys.contains(key)) {
55+
throw new IllegalArgumentException("Duplicate parameter key: " + key);
56+
}
57+
keys.add(key);
58+
}
59+
}
60+
}

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/validator/workflow/BackfillWorkflowDTOValidator.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.dolphinscheduler.api.validator.workflow;
1919

2020
import org.apache.dolphinscheduler.api.validator.IValidator;
21+
import org.apache.dolphinscheduler.api.validator.StartParamListValidator;
2122
import org.apache.dolphinscheduler.api.validator.TenantExistValidator;
2223
import org.apache.dolphinscheduler.common.enums.CommandType;
2324
import org.apache.dolphinscheduler.common.enums.ReleaseState;
@@ -34,8 +35,12 @@ public class BackfillWorkflowDTOValidator implements IValidator<BackfillWorkflow
3435

3536
private final TenantExistValidator tenantExistValidator;
3637

37-
public BackfillWorkflowDTOValidator(TenantExistValidator tenantExistValidator) {
38+
private final StartParamListValidator startParamListValidator;
39+
40+
public BackfillWorkflowDTOValidator(TenantExistValidator tenantExistValidator,
41+
StartParamListValidator startParamListValidator) {
3842
this.tenantExistValidator = tenantExistValidator;
43+
this.startParamListValidator = startParamListValidator;
3944
}
4045

4146
@Override
@@ -59,6 +64,9 @@ public void validate(final BackfillWorkflowDTO backfillWorkflowDTO) {
5964
if (backfillWorkflowDTO.getWorkflowDefinition().getReleaseState() != ReleaseState.ONLINE) {
6065
throw new IllegalStateException("The workflowDefinition should be online");
6166
}
67+
6268
tenantExistValidator.validate(backfillWorkflowDTO.getTenantCode());
69+
70+
startParamListValidator.validate(backfillWorkflowDTO.getStartParamList());
6371
}
6472
}

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/validator/workflow/TriggerWorkflowDTOValidator.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.dolphinscheduler.api.validator.workflow;
1919

2020
import org.apache.dolphinscheduler.api.validator.IValidator;
21+
import org.apache.dolphinscheduler.api.validator.StartParamListValidator;
2122
import org.apache.dolphinscheduler.api.validator.TenantExistValidator;
2223
import org.apache.dolphinscheduler.common.enums.CommandType;
2324
import org.apache.dolphinscheduler.common.enums.ReleaseState;
@@ -32,8 +33,12 @@ public class TriggerWorkflowDTOValidator implements IValidator<TriggerWorkflowDT
3233

3334
private final TenantExistValidator tenantExistValidator;
3435

35-
public TriggerWorkflowDTOValidator(TenantExistValidator tenantExistValidator) {
36+
private final StartParamListValidator startParamListValidator;
37+
38+
public TriggerWorkflowDTOValidator(TenantExistValidator tenantExistValidator,
39+
StartParamListValidator startParamListValidator) {
3640
this.tenantExistValidator = tenantExistValidator;
41+
this.startParamListValidator = startParamListValidator;
3742
}
3843

3944
@Override
@@ -47,6 +52,9 @@ public void validate(final TriggerWorkflowDTO triggerWorkflowDTO) {
4752
if (triggerWorkflowDTO.getWorkflowDefinition().getReleaseState() != ReleaseState.ONLINE) {
4853
throw new IllegalStateException("The workflowDefinition should be online");
4954
}
55+
5056
tenantExistValidator.validate(triggerWorkflowDTO.getTenantCode());
57+
58+
startParamListValidator.validate(triggerWorkflowDTO.getStartParamList());
5159
}
5260
}
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.dolphinscheduler.api.validator;
19+
20+
import static org.assertj.core.api.Assertions.assertThatCode;
21+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
22+
23+
import org.apache.dolphinscheduler.plugin.task.api.enums.DataType;
24+
import org.apache.dolphinscheduler.plugin.task.api.enums.Direct;
25+
import org.apache.dolphinscheduler.plugin.task.api.model.Property;
26+
27+
import java.util.ArrayList;
28+
import java.util.Collections;
29+
import java.util.List;
30+
31+
import org.junit.jupiter.api.Test;
32+
import org.junit.jupiter.api.extension.ExtendWith;
33+
import org.mockito.InjectMocks;
34+
import org.mockito.junit.jupiter.MockitoExtension;
35+
36+
@ExtendWith(MockitoExtension.class)
37+
class StartParamListValidatorTest {
38+
39+
@InjectMocks
40+
private StartParamListValidator startParamListValidator;
41+
42+
@Test
43+
void testValidate_nullList() {
44+
assertThatCode(() -> startParamListValidator.validate(null))
45+
.doesNotThrowAnyException();
46+
}
47+
48+
@Test
49+
void testValidate_emptyList() {
50+
assertThatCode(() -> startParamListValidator.validate(Collections.emptyList()))
51+
.doesNotThrowAnyException();
52+
}
53+
54+
@Test
55+
void testValidate_validParameters() {
56+
List<Property> params = Collections.singletonList(
57+
new Property("workflow_id", Direct.IN, DataType.VARCHAR, "1001"));
58+
assertThatCode(() -> startParamListValidator.validate(params))
59+
.doesNotThrowAnyException();
60+
}
61+
62+
@Test
63+
void testValidate_rejectsBlankOrEmptyKeys() {
64+
assertThrowsIllegalArgument("");
65+
66+
assertThrowsIllegalArgument(" ");
67+
68+
assertThrowsIllegalArgument("\t");
69+
70+
assertThrowsIllegalArgument("\n");
71+
72+
assertThrowsIllegalArgument(" \t\n ");
73+
}
74+
75+
private void assertThrowsIllegalArgument(String propValue) {
76+
List<Property> params = Collections.singletonList(
77+
new Property(propValue, Direct.IN, DataType.VARCHAR, "dummyValue"));
78+
79+
assertThatThrownBy(() -> startParamListValidator.validate(params))
80+
.isInstanceOf(IllegalArgumentException.class)
81+
.hasMessage("Parameter key cannot be empty");
82+
}
83+
84+
@Test
85+
void testValidate_duplicateKeys() {
86+
List<Property> params = new ArrayList<>();
87+
params.add(new Property("app_name", Direct.IN, DataType.VARCHAR, "A"));
88+
params.add(new Property("app_name", Direct.IN, DataType.VARCHAR, "B"));
89+
90+
assertThatThrownBy(() -> startParamListValidator.validate(params))
91+
.isInstanceOf(IllegalArgumentException.class)
92+
.hasMessageContaining("Duplicate parameter key: app_name");
93+
}
94+
95+
@Test
96+
void testValidate_duplicateKeysAfterTrim() {
97+
List<Property> params = new ArrayList<>();
98+
params.add(new Property(" app_name ", Direct.IN, DataType.VARCHAR, "A"));
99+
params.add(new Property("app_name", Direct.IN, DataType.VARCHAR, "B"));
100+
101+
assertThatThrownBy(() -> startParamListValidator.validate(params))
102+
.isInstanceOf(IllegalArgumentException.class)
103+
.hasMessageContaining("Duplicate parameter key: app_name");
104+
}
105+
106+
@Test
107+
void testValidate_inTypeEmptyValueAllowed() {
108+
List<Property> params = Collections.singletonList(
109+
new Property("input_var", Direct.IN, DataType.VARCHAR, ""));
110+
111+
assertThatCode(() -> startParamListValidator.validate(params))
112+
.doesNotThrowAnyException();
113+
}
114+
115+
@Test
116+
void testValidate_outTypeEmptyValueAllowed() {
117+
List<Property> params = Collections.singletonList(
118+
new Property("output_var", Direct.OUT, DataType.VARCHAR, ""));
119+
120+
assertThatCode(() -> startParamListValidator.validate(params))
121+
.doesNotThrowAnyException();
122+
}
123+
}

dolphinscheduler-ui/src/locales/en_US/project.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,8 @@ export default {
353353
update_directly: 'Whether to update the workflow definition',
354354
dag_name_empty: 'DAG graph name cannot be empty',
355355
positive_integer: 'Please enter a positive integer greater than 0',
356-
prop_empty: 'prop is empty',
357-
prop_repeat: 'prop is repeat',
356+
prop_key_repeat: 'prop key is repeat',
357+
prop_key_empty: 'prop key is empty',
358358
node_not_created: 'Failed to save node not created',
359359
copy_name: 'Copy Name',
360360
view_variables: 'View Variables',

dolphinscheduler-ui/src/locales/zh_CN/project.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,8 +347,8 @@ export default {
347347
update_directly: '是否更新工作流定义',
348348
dag_name_empty: 'DAG图名称不能为空',
349349
positive_integer: '请输入大于 0 的正整数',
350-
prop_empty: '自定义参数prop不能为空',
351-
prop_repeat: 'prop中有重复',
350+
prop_key_repeat: '自定义参数prop中key有重复',
351+
prop_key_empty: '自定义参数prop中key不能为空',
352352
node_not_created: '未创建节点保存失败',
353353
copy_name: '复制名称',
354354
view_variables: '查看变量',

dolphinscheduler-ui/src/views/projects/workflow/components/dag/dag-save-modal.tsx

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -98,23 +98,23 @@ export default defineComponent({
9898
},
9999
globalParams: {
100100
validator() {
101-
const props = new Set()
101+
const globalParams = formValue.value.globalParams || []
102102

103-
const keys = formValue.value.globalParams.map((item) => item.key)
104-
const keysSet = new Set(keys)
105-
if (keysSet.size !== keys.length) {
106-
return new Error(t('project.dag.prop_repeat'))
107-
}
103+
if (!globalParams || globalParams.length === 0) return true
108104

109-
for (const param of formValue.value.globalParams) {
110-
const prop = param.value
111-
const direct = param.direct
112-
if (direct === 'IN' && !prop) {
113-
return new Error(t('project.dag.prop_empty'))
105+
for (const param of globalParams) {
106+
if (!param.key || param.key.trim() === '') {
107+
return new Error(t('project.dag.prop_key_empty'))
114108
}
109+
}
115110

116-
props.add(prop)
111+
const keys = globalParams.map((item) => (item.key || '').trim())
112+
const uniqueKeys = new Set(keys)
113+
if (uniqueKeys.size !== keys.length) {
114+
return new Error(t('project.dag.prop_key_repeat'))
117115
}
116+
117+
return true
118118
}
119119
}
120120
}

dolphinscheduler-ui/src/views/projects/workflow/definition/components/start-modal.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,11 @@ export default defineComponent({
290290
}
291291
)
292292

293+
const formModel = computed(() => ({
294+
...startState.startForm,
295+
startParamsList: variables.startParamsList
296+
}))
297+
293298
return {
294299
t,
295300
showTaskDependType,
@@ -302,6 +307,7 @@ export default defineComponent({
302307
removeStartParams,
303308
addStartParams,
304309
updateParamsList,
310+
formModel,
305311
...toRefs(variables),
306312
...toRefs(startState),
307313
...toRefs(props),
@@ -319,7 +325,7 @@ export default defineComponent({
319325
onConfirm={this.handleStart}
320326
confirmLoading={this.saving}
321327
>
322-
<NForm ref='startFormRef' model={this.startForm} rules={this.rules}>
328+
<NForm ref='startFormRef' model={this.formModel} rules={this.rules}>
323329
<NFormItem
324330
label={t('project.workflow.workflow_name')}
325331
path='workflow_name'
@@ -577,13 +583,13 @@ export default defineComponent({
577583
)}
578584
<NFormItem
579585
label={t('project.workflow.startup_parameter')}
580-
path='startup_parameter'
586+
path='startParamsList'
581587
>
582588
<NDynamicInput
583589
v-model:value={this.startParamsList}
584590
onCreate={() => {
585591
return {
586-
key: '',
592+
prop: '',
587593
direct: 'IN',
588594
type: 'VARCHAR',
589595
value: ''

dolphinscheduler-ui/src/views/projects/workflow/definition/components/use-form.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,28 @@ export const useForm = () => {
101101
return new Error(t('project.workflow.warning_group_tip'))
102102
}
103103
}
104+
},
105+
startParamsList: {
106+
trigger: ['input', 'blur'],
107+
validator: (rule: any, value: Array<any>) => {
108+
const params = value || []
109+
110+
if (!params || params.length === 0) return true
111+
112+
for (const param of params) {
113+
if (!param.prop || param.prop.trim() === '') {
114+
return new Error(t('project.dag.prop_key_empty'))
115+
}
116+
}
117+
118+
const keys = params.map((item) => (item.prop || '').trim())
119+
const uniqueKeys = new Set(keys)
120+
if (uniqueKeys.size !== keys.length) {
121+
return new Error(t('project.dag.prop_key_repeat'))
122+
}
123+
124+
return true
125+
}
104126
}
105127
}
106128
})

0 commit comments

Comments
 (0)