leehaut commented on code in PR #8867:
URL: https://github.com/apache/hop/pull/8867#discussion_r4248165316
##########
plugins/actions/evalfilesmetrics/src/main/java/org/apache/hop/workflow/actions/evalfilesmetrics/ActionEvalFilesMetrics.java:
##########
@@ -688,21 +689,29 @@ public void check(
ActionValidatorUtils.andValidator()
.validate(
this,
- "arguments",
+ "sourceFiles",
remarks,
AndValidator.putValidators(ActionValidatorUtils.notNullValidator()));
if (!res) {
return;
}
+ // Only static file/folder rows are checkable here; previous/result
filenames are runtime-only.
+ if (sourceFilesType != SourceFilesType.FILES || sourceFiles == null) {
+ return;
+ }
+
ValidatorContext ctx = new ValidatorContext();
- AbstractFileValidator.putVariableSpace(ctx, getVariables());
+ AbstractFileValidator.putVariableSpace(ctx, variables);
AndValidator.putValidators(
ctx, ActionValidatorUtils.notNullValidator(),
ActionValidatorUtils.fileExistsValidator());
Review Comment:
Thanks @mattcasters — good catch.
You're right: a blank `sourceFileFolder` (or a variable that resolves to
blank) was reaching `FileExistsValidator`, and `HopVfs.getFileObject("")`
effectively checked the CWD, so Check passed while `processFileFolder()` fails
at runtime.
I've updated `check()` to:
1. resolve each `sourceFileFolder` with the `variables` argument first;
2. if the resolved value is empty, add a `notBlank` ERROR remark for that
row and skip `FileExistsValidator`;
3. use `notBlankValidator` instead of `notNullValidator` for the per-row
validators.
Also added unit tests for a blank folder with a non-empty wildcard, and for
a variable that resolves to blank.
Please take another look when you have a moment.
--
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]
