Fix#9264 Add filenames invalid regex and Delete folders error-limit boundary

docs/hop-user-manual/modules/ROOT/pages/workflow/actions/addresultfilenames.adoc

@@ -37,7 +37,7 @@ This list of filenames can then be used in the various workflow actions all arou
 |Clear result filenames|This option clears the list of result files (from previous workflow actions) before creating a new list.
 |File/Folder|Specify the list of files or folders with wildcards (regular expressions) in this grid. +
 You can add a different file or folder and wildcard on each line.
-|Wildcard (RegExp)|A regular expression wildcard to include file patterns in the list of file names.
+|Wildcard (RegExp)|A regular expression matched against the file name (basename) of each file in a folder. Ignored for file entries.
 |===
 
 After specifying a folder and wildcard (regular expression), use the `Add` button to add your file/folder selection to the table. Use the `Delete` or `Edit` buttons to remove or edit selected lines in the table.
@@ -49,10 +49,10 @@ What this action passes on to the next action. See xref:workflow/action-results.
 [options="header", width="90%", cols="1,3"]
 |===
 |Result|Value
-|True|Every file and folder in the list exists. A folder without matching files adds nothing and is still true.
-|False|A file or folder in the list doesn't exist. The number of errors is the number of entries that don't exist. The files of the other entries are still added.
+|True|Every file and folder in the list exists, and the *Wildcard (RegExp)* of every folder is a valid regular expression. A folder without matching files adds nothing and is still true.
+|False|A file or folder in the list doesn't exist, or the *Wildcard (RegExp)* of a folder is an invalid regular expression. The number of errors is the number of entries that failed. The files of the other entries are still added.
 |Result files|Each file in the list is added. For a folder, the files in it whose whole file name, without the folder, matches *Wildcard (RegExp)* are added, or all its files when the wildcard is empty, and with *Include Subfolders* also the matching files in its subfolders. With *Clear result filenames*, the result files of the previous action are removed first; otherwise they are kept.
 |Result rows|Passed on unchanged. With *Copy previous results to args?*, the file or folder comes from the first field and the wildcard from the second field of each result row, and the list in the dialog is not used, also when there are no result rows. A result row with only one field adds all the files of that folder, as with an empty wildcard.
 |===
 
-NOTE: An invalid regular expression in *Wildcard (RegExp)* adds nothing, and the action is still true.
+NOTE: An invalid regular expression in *Wildcard (RegExp)* of a folder makes that entry fail (the action is false), and no files from that folder are added. A file entry ignores the wildcard.

docs/hop-user-manual/modules/ROOT/pages/workflow/actions/deletefolders.adoc

@@ -54,4 +54,4 @@ What this action passes on to the next action. See xref:workflow/action-results.
 |Lines deleted|The number of folders processed: deleted, or already missing.
 |===
 
-NOTE: With *When number of errors less than*, the action stops and is false as soon as the number of errors reaches *Limit*, but when the last folder brings it to that number, the action is still true.
+NOTE: With *When number of errors less than*, the action stops and is false as soon as the number of errors reaches *Limit* (strictly fewer errors than the limit is required for success).

plugins/actions/addresultfilenames/src/main/java/org/apache/hop/workflow/actions/addresultfilenames/ActionAddResultFilenames.java

@@ -170,149 +170,142 @@ private boolean processFile(
       IWorkflowEngine<WorkflowMeta> parentWorkflow,
       Result result) {
 
-    boolean rcode = true;
+    String realFileFolderName = resolve(filename);
+    String realWildcard = resolve(wildcard);
     FileObject filefolder = null;
-    String realFilefoldername = resolve(filename);
-    String realwildcard = resolve(wildcard);
 
     try {
-      filefolder = HopVfs.getFileObject(realFilefoldername, getVariables());
-      if (filefolder.exists()) {
-        // the file or folder exists
-
-        if (filefolder.getType() == FileType.FILE) {
-          // Add filename to Resultfilenames ...
-          if (isDetailed()) {
-            logDetailed(
-                BaseMessages.getString(
-                    PKG, "ActionAddResultFilenames.AddingFileToResult", filefolder.toString()));
-          }
-          ResultFile resultFile =
-              new ResultFile(
-                  ResultFile.FILE_TYPE_GENERAL,
-                  HopVfs.getFileObject(filefolder.toString(), getVariables()),
-                  parentWorkflow.getWorkflowName(),
-                  toString());
-          result.getResultFiles().put(resultFile.getFile().toString(), resultFile);
-        } else {
-          FileObject[] list =
-              filefolder.findFiles(new TextFileSelector(filefolder.toString(), realwildcard));
-
-          for (int i = 0; i < list.length && !parentWorkflow.isStopped(); i++) {
-            // Add filename to Resultfilenames ...
-            if (isDetailed()) {
-              logDetailed(
-                  BaseMessages.getString(
-                      PKG, "ActionAddResultFilenames.AddingFileToResult", list[i].toString()));
-            }
-            ResultFile resultFile =
-                new ResultFile(
-                    ResultFile.FILE_TYPE_GENERAL,
-                    HopVfs.getFileObject(list[i].toString(), getVariables()),
-                    parentWorkflow.getWorkflowName(),
-                    toString());
-            result.getResultFiles().put(resultFile.getFile().toString(), resultFile);
-          }
-        }
-
-      } else {
-        // File can not be found
+      filefolder = HopVfs.getFileObject(realFileFolderName, getVariables());
+      if (!filefolder.exists()) {
         if (isBasic()) {
           logBasic(
               BaseMessages.getString(
-                  PKG, "ActionAddResultFilenames.FileCanNotbeFound", realFilefoldername));
+                  PKG, "ActionAddResultFilenames.FileCanNotbeFound", realFileFolderName));
         }
-        rcode = false;
+        return false;
+      }
+      if (filefolder.getType() == FileType.FILE) {
+        // Wildcard applies only to folders; a file entry is added as-is.
+        addToResultFiles(filefolder.toString(), parentWorkflow, result);
+      } else {
+        // Compile once so an invalid mask fails the folder entry (issue #9264),
+        // instead of being swallowed while selecting each file.
+        addMatchingFilesFromFolder(
+            filefolder, compileWildcard(realWildcard), parentWorkflow, result);
       }
+      return true;
     } catch (Exception e) {
-      rcode = false;
       logError(
           BaseMessages.getString(
-              PKG, "ActionAddResultFilenames.CouldNotProcess", realFilefoldername, e.getMessage()),
+              PKG, "ActionAddResultFilenames.CouldNotProcess", realFileFolderName, e.getMessage()),
           e);
+      return false;
     } finally {
-      if (filefolder != null) {
-        try {
-          filefolder.close();
-          filefolder = null;
-        } catch (IOException ex) {
-          // Ignore
-        }
-      }
+      closeQuietly(filefolder);
     }
+  }
 
-    return rcode;
+  private void addMatchingFilesFromFolder(
+      FileObject folder,
+      Pattern compiledWildcard,
+      IWorkflowEngine<WorkflowMeta> parentWorkflow,
+      Result result)
+      throws Exception {
+    FileObject[] list = folder.findFiles(new TextFileSelector(folder.toString(), compiledWildcard));
+    for (int i = 0; i < list.length && !parentWorkflow.isStopped(); i++) {
+      addToResultFiles(list[i].toString(), parentWorkflow, result);
+    }
   }
 
-  private class TextFileSelector implements FileSelector {
-    String fileWildcard = null;
-    String sourceFolder = null;
+  private void addToResultFiles(
+      String filePath, IWorkflowEngine<WorkflowMeta> parentWorkflow, Result result)
+      throws Exception {
+    if (isDetailed()) {
+      logDetailed(
+          BaseMessages.getString(PKG, "ActionAddResultFilenames.AddingFileToResult", filePath));
+    }
+    ResultFile resultFile =
+        new ResultFile(
+            ResultFile.FILE_TYPE_GENERAL,
+            HopVfs.getFileObject(filePath, getVariables()),
+            parentWorkflow.getWorkflowName(),
+            toString());
+    result.getResultFiles().put(resultFile.getFile().toString(), resultFile);
+  }
 
-    public TextFileSelector(String sourcefolderin, String filewildcard) {
-      if (!Utils.isEmpty(sourcefolderin)) {
-        sourceFolder = sourcefolderin;
-      }
+  private static void closeQuietly(FileObject fileFolder) {
+    if (fileFolder == null) {
+      return;
+    }
+    try {
+      fileFolder.close();
+    } catch (IOException ex) {
+      // Ignore
+    }
+  }
 
-      if (!Utils.isEmpty(filewildcard)) {
-        fileWildcard = filewildcard;
-      }
+  /**
+   * Compile the folder file-mask regex once. An invalid expression must fail that folder entry
+   * instead of being swallowed while selecting files. Not used for file entries.
+   */
+  private static Pattern compileWildcard(String wildcard) {
+    if (Utils.isEmpty(wildcard)) {
+      return null;
+    }
+    return Pattern.compile(wildcard);
+  }
+
+  private class TextFileSelector implements FileSelector {
+    private final Pattern fileWildcard;
+    private final String sourceFolder;
+
+    TextFileSelector(String sourceFolder, Pattern fileWildcard) {
+      this.sourceFolder = sourceFolder;
+      this.fileWildcard = fileWildcard;
     }
 
     @Override
     public boolean includeFile(FileSelectInfo info) {
-      boolean returncode = false;
       try {
-        if (!info.getFile().toString().equals(sourceFolder)) {
-          // Pass over the Base folder itself
-          String shortFilename = info.getFile().getName().getBaseName();
-
-          if (info.getFile().getParent().equals(info.getBaseFolder())
-              || (!info.getFile().getParent().equals(info.getBaseFolder()) && includeSubFolders)) {
-            if ((info.getFile().getType() == FileType.FILE && fileWildcard == null)
-                || (info.getFile().getType() == FileType.FILE
-                    && fileWildcard != null
-                    && GetFileWildcard(shortFilename, fileWildcard))) {
-              returncode = true;
-            }
-          }
+        if (info.getFile().toString().equals(sourceFolder)) {
+          return false;
+        }
+        if (info.getFile().getType() != FileType.FILE) {
+          return false;
+        }
+        if (!isInScope(info)) {
+          return false;
         }
+        return matchesWildcard(info.getFile().getName().getBaseName());
       } catch (Exception e) {
         logError(
             "Error while finding files ... in ["
                 + info.getFile().toString()
                 + "]. Exception :"
                 + e.getMessage());
-        returncode = false;
+        return false;
       }
-      return returncode;
     }
 
-    @Override
-    public boolean traverseDescendents(FileSelectInfo info) {
-      return info.getDepth() == 0 || includeSubFolders;
+    private boolean isInScope(FileSelectInfo info) throws Exception {
+      if (info.getFile().getParent().equals(info.getBaseFolder())) {
+        return true;
+      }
+      return includeSubFolders;
     }
-  }
 
-  /**********************************************************
-   *
-   * @param selectedfile
-   * @param wildcard
-   * @return True if the selectedfile matches the wildcard
-   **********************************************************/
-  private boolean GetFileWildcard(String selectedfile, String wildcard) {
-    Pattern pattern = null;
-    boolean getIt = true;
-
-    if (!Utils.isEmpty(wildcard)) {
-      pattern = Pattern.compile(wildcard);
-      // First see if the file matches the regular expression!
-      if (pattern != null) {
-        Matcher matcher = pattern.matcher(selectedfile);
-        getIt = matcher.matches();
+    private boolean matchesWildcard(String shortFilename) {
+      if (fileWildcard == null) {
+        return true;
       }
+      Matcher matcher = fileWildcard.matcher(shortFilename);
+      return matcher.matches();
     }
 
-    return getIt;
+    @Override
+    public boolean traverseDescendents(FileSelectInfo info) {
+      return info.getDepth() == 0 || includeSubFolders;
+    }
   }
 
   public void setIncludeSubFolders(boolean includeSubFolders) {
@@ -346,7 +339,7 @@ public void check(
                 remarks,
                 AndValidator.putValidators(ActionValidatorUtils.notNullValidator()));
 
-    if (res == false) {
+    if (!res) {
       return;
     }