Skip to content

[Improvement-17942][API&UI]Add startParams validation logic in both the frontend and backend.#17956

Merged
SbloodyS merged 23 commits intoapache:devfrom
njnu-seafish:Improvement-17942
Mar 12, 2026
Merged

[Improvement-17942][API&UI]Add startParams validation logic in both the frontend and backend.#17956
SbloodyS merged 23 commits intoapache:devfrom
njnu-seafish:Improvement-17942

Conversation

@njnu-seafish
Copy link
Copy Markdown
Contributor

Purpose of the pull request

close #17942

Brief change log

Add startParams validation logic in both the frontend and backend.

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

@SbloodyS SbloodyS added the improvement make more easy to user or prompt friendly label Feb 10, 2026
@SbloodyS SbloodyS added this to the 3.4.1 milestone Feb 10, 2026
Comment on lines +294 to +297
// UI configuration state from form management hook
...startState.startForm,
// Business data injected from modal operations hook
startParamsList: variables.startParamsList
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// UI configuration state from form management hook
...startState.startForm,
// Business data injected from modal operations hook
startParamsList: variables.startParamsList
...startState.startForm,
startParamsList: variables.startParamsList

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// UI configuration state from form management hook

ok

Comment on lines +87 to +102
// null key
if (StringUtils.isEmpty(param.getProp())) {
throw new ServiceException("Parameter key cannot be empty");
}

String key = param.getProp().trim();
// duplicate keys
if (keys.contains(key)) {
throw new ServiceException("Duplicate parameter key: " + key);
}
keys.add(key);

// IN-type params require a non-empty value
if (Direct.IN.equals(param.getDirect()) && StringUtils.isEmpty(param.getValue())) {
throw new ServiceException("IN parameter value cannot be empty for key: " + key);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// null key
if (StringUtils.isEmpty(param.getProp())) {
throw new ServiceException("Parameter key cannot be empty");
}
String key = param.getProp().trim();
// duplicate keys
if (keys.contains(key)) {
throw new ServiceException("Duplicate parameter key: " + key);
}
keys.add(key);
// IN-type params require a non-empty value
if (Direct.IN.equals(param.getDirect()) && StringUtils.isEmpty(param.getValue())) {
throw new ServiceException("IN parameter value cannot be empty for key: " + key);
}
if (StringUtils.isEmpty(param.getProp())) {
throw new ServiceException("Parameter key cannot be empty");
}
String key = param.getProp().trim();
if (keys.contains(key)) {
throw new ServiceException("Duplicate parameter key: " + key);
}
if (Direct.IN.equals(param.getDirect()) && StringUtils.isEmpty(param.getValue())) {
throw new ServiceException("IN parameter value cannot be empty for key: " + key);
}
keys.add(key);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Comment on lines +81 to +83
if (startParamList == null || startParamList.isEmpty()) {
return;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (startParamList == null || startParamList.isEmpty()) {
return;
}
if (CollectionUtils.isEmpty(startParamList)) {
return;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (CollectionUtils.isEmpty(startParamList)) {

good

@njnu-seafish
Copy link
Copy Markdown
Contributor Author

Hey @ruanwenjun @SbloodyS, whenever you have time, would you mind reviewing this fix to see if it looks reasonable? Really thanks a lot!

@SbloodyS SbloodyS modified the milestones: 3.4.1, 3.4.2 Feb 28, 2026
SbloodyS
SbloodyS previously approved these changes Mar 5, 2026
Copy link
Copy Markdown
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SbloodyS SbloodyS requested a review from ruanwenjun March 5, 2026 03:02
return triggerWorkflowDTO;
}

private void validateStartParamList(List<Property> startParamList) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should do the validation work at TriggerWorkflowDTOValidator.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should do the validation work at TriggerWorkflowDTOValidator.

good idea

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

import com.alibaba.druid.util.StringUtils;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use common utils.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I made a mistake.

@njnu-seafish
Copy link
Copy Markdown
Contributor Author

Hey @ruanwenjun @SbloodyS, whenever you have time, would you mind reviewing this fix? Really thanks a lot!

ruanwenjun
ruanwenjun previously approved these changes Mar 6, 2026
Copy link
Copy Markdown
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@njnu-seafish njnu-seafish requested a review from ruanwenjun March 9, 2026 06:45
Copy link
Copy Markdown
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@sonarqubecloud
Copy link
Copy Markdown

@SbloodyS SbloodyS merged commit 0472599 into apache:dev Mar 12, 2026
106 of 135 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend improvement make more easy to user or prompt friendly test UI ui and front end related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement][API] Add null/empty/duplicate validation logic for startParams when manually triggering a workflow

3 participants