mattcasters commented on code in PR #8867:
URL: https://github.com/apache/hop/pull/8867#discussion_r4246686904
##########
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:
**[bug]** A blank `sourceFileFolder` (or a variable that resolves to blank)
passes Check. `notNullValidator` accepts `""`, then `FileExistsValidator`
resolves it and calls `HopVfs.getFileObject("")`. That treats an empty name as
a relative path, so `new File("").getAbsolutePath()` is the process working
directory, which exists, and the row is reported OK (`failIfDoesNotExist` never
fires). `processFileFolder()` does the opposite:
`Utils.isEmpty(realSourceFilefoldername)` logs
`ActionEvalFilesMetrics.log.FileFolderEmpty` and increments errors. The dialog
keeps a row when the folder cell is empty but the wildcard or
include-subfolders cell is not, and XML can store an empty `source_filefolder`,
so this is a reachable configuration. `notBlankValidator` alone is not enough
for `${VAR}` when `VAR` is set to empty, because the raw value is non-blank and
the empty string appears only after `variables.resolve`.
**Suggestion:** Before calling the file-exists validator, resolve
`sourceFiles.get(i).getSourceFileFolder()` with the `variables` argument passed
to `check()`. If the resolved value is empty, add a `TYPE_RESULT_ERROR` remark
for that row and do not call `FileExistsValidator`. Use `notBlankValidator` in
place of `notNullValidator` so a literal blank fails even if resolution is
skipped. Add a test whose only row has `sourceFileFolder` equal to `""` (and a
non-empty wildcard, so it is a real configured row) and assert an error that
mentions `sourceFiles[0].sourceFileFolder`.
--
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]
