This is an automated email from the ASF dual-hosted git repository.
mattcasters pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 0bc064d5d9 Issue #9049 : Fix the result of Add filenames to result,
Unzip, Delete file, Create folder and Delete filenames from result (#9101)
0bc064d5d9 is described below
commit 0bc064d5d9cbad845593f4a6cc4973922fee32a2
Author: vbhanuchander-lang <[email protected]>
AuthorDate: Sat Oct 24 17:32:44 2026 -0400
Issue #9049 : Fix the result of Add filenames to result, Unzip, Delete
file, Create folder and Delete filenames from result (#9101)
- Add filenames to result: a result row with only the file or folder no
longer throws, it takes all files as with an empty wildcard.
- Unzip: a zip file that can't be read or deleted counts as one error, not
two, and "If file exists = Fail" only applies to entries that pass the
include/exclude wildcards.
- Delete file: false when the file could not be deleted.
- Create folder: false when a file exists with the folder name.
- Delete filenames from result: matching result files are removed even when
they no longer exist on disk.
The NOTEs that documented the old behaviour are removed.
---
.../pages/workflow/actions/addresultfilenames.adoc | 2 +-
.../ROOT/pages/workflow/actions/createfolder.adoc | 4 +-
.../ROOT/pages/workflow/actions/deletefile.adoc | 4 +-
.../workflow/actions/deleteresultfilenames.adoc | 4 +-
.../modules/ROOT/pages/workflow/actions/unzip.adoc | 2 +-
.../ActionAddResultFilenames.java | 3 +-
.../ActionAddResultFilenamesSubfolderTest.java | 19 ++++++
.../actions/createfolder/ActionCreateFolder.java | 13 +++--
.../createfolder/ActionCreateFolderTest.java | 13 +++++
.../actions/deletefile/ActionDeleteFile.java | 9 +--
.../actions/deletefile/ActionDeleteFileTest.java | 67 ++++++++++++++++++++++
.../ActionDeleteResultFilenames.java | 1 -
.../ActionDeleteResultFilenamesTest.java | 48 ++++++++++++++++
.../hop/workflow/actions/unzip/ActionUnZip.java | 14 ++---
.../actions/unzip/WorkflowActionUnZipTest.java | 56 ++++++++++++++++++
15 files changed, 228 insertions(+), 31 deletions(-)
diff --git
a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/addresultfilenames.adoc
b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/addresultfilenames.adoc
index c1647617db..5ffe9f251a 100644
---
a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/addresultfilenames.adoc
+++
b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/addresultfilenames.adoc
@@ -52,7 +52,7 @@ What this action passes on to the next action. See
xref:workflow/action-results.
|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.
|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 stops the workflow
with an error, and no hop is followed.
+|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.
diff --git
a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/createfolder.adoc
b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/createfolder.adoc
index dc2b51b248..bb79eaa165 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/createfolder.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/createfolder.adoc
@@ -42,9 +42,7 @@ What this action passes on to the next action. See
xref:workflow/action-results.
|===
|Result|Value
|True|The folder was created, including any missing parent folders, or it
already exists and *Fail if folder exists* is not selected.
-|False|The folder already exists and *Fail if folder exists* is selected, or
no folder name is set: the number of errors is 0. The folder could not be
created, for example because of missing permissions: the number of errors is 1.
+|False|The folder already exists and *Fail if folder exists* is selected, a
file exists with the folder name, or no folder name is set: the number of
errors is 0. The folder could not be created, for example because of missing
permissions: the number of errors is 1.
|Result files|Passed on unchanged.
|Result rows|Passed on unchanged.
|===
-
-NOTE: When a file exists with the folder name, no folder is created. The
action is true, or false when *Fail if folder exists* is selected.
diff --git
a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/deletefile.adoc
b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/deletefile.adoc
index 06a054c3e9..76ab7d597f 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/deletefile.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/deletefile.adoc
@@ -43,9 +43,7 @@ What this action passes on to the next action. See
xref:workflow/action-results.
|===
|Result|Value
|True|The file was deleted, or it doesn't exist and *Fail if file doesn't
exist* is not selected. An empty folder is deleted too.
-|False|The file doesn't exist and *Fail if file doesn't exist* is selected, or
no file name is set: the number of errors is 0. The file could not be deleted,
for example because of missing permissions: the number of errors is 1.
+|False|The file doesn't exist and *Fail if file doesn't exist* is selected, or
no file name is set: the number of errors is 0. The file could not be deleted,
for example because of missing permissions or because it is a folder that is
not empty: the number of errors is 1.
|Result files|Passed on unchanged.
|Result rows|Passed on unchanged.
|===
-
-NOTE: A folder that is not empty is not deleted, but the action is still true,
with 1 error.
diff --git
a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/deleteresultfilenames.adoc
b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/deleteresultfilenames.adoc
index ef89c02abc..e75be2f33d 100644
---
a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/deleteresultfilenames.adoc
+++
b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/deleteresultfilenames.adoc
@@ -42,10 +42,8 @@ What this action passes on to the next action. See
xref:workflow/action-results.
[options="header", width="90%", cols="1,3"]
|===
|Result|Value
-|True|The result files were removed: all of them, or with *Limit action to*,
those whose whole file name, without the folder, matches *Wildcard (RegExp)*
and not *Exclude wildcard (RegExp)*. With *Limit action to* and an empty
*Wildcard (RegExp)*, all of them are removed.
+|True|The result files were removed: all of them, or with *Limit action to*,
those whose whole file name, without the folder, matches *Wildcard (RegExp)*
and not *Exclude wildcard (RegExp)*, also when the file no longer exists on
disk. With *Limit action to* and an empty *Wildcard (RegExp)*, all of them are
removed.
|False|A wildcard is not a valid regular expression. No result file is
removed, and the number of errors is 0.
|Result files|The result files of the previous action, without the removed
ones. The files themselves are not deleted.
|Result rows|Passed on unchanged.
|===
-
-NOTE: With *Limit action to*, a result file that no longer exists on disk is
not removed from the result files, even when its name matches.
diff --git
a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/unzip.adoc
b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/unzip.adoc
index 6fbafb0f63..23df655c70 100644
--- a/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/unzip.adoc
+++ b/docs/hop-user-manual/modules/ROOT/pages/workflow/actions/unzip.adoc
@@ -84,4 +84,4 @@ What this action passes on to the next action. See
xref:workflow/action-results.
|Lines written|The number of zip files unzipped. When the action can't start,
or stops on the success condition before the last result row, the value of the
previous action is passed on.
|===
-NOTE: With *Nr errors less than*, the action stops and is false as soon as the
number of errors reaches *Limit files* while result rows are left to process,
but when the last row brings it to that number, the action is still true. For
the files of a single zip file or folder, it is still true when the number of
errors equals *Limit files*, and false only when it is higher. A zip file that
can't be read, or that can't be deleted after extraction with *Delete files*,
counts as two errors a [...]
+NOTE: With *Nr errors less than*, the action stops and is false as soon as the
number of errors reaches *Limit files* while result rows are left to process,
but when the last row brings it to that number, the action is still true. For
the files of a single zip file or folder, it is still true when the number of
errors equals *Limit files*, and false only when it is higher. A zip file that
can't be read, or that can't be deleted after extraction with *Delete files*,
counts as one error an [...]
diff --git
a/plugins/actions/addresultfilenames/src/main/java/org/apache/hop/workflow/actions/addresultfilenames/ActionAddResultFilenames.java
b/plugins/actions/addresultfilenames/src/main/java/org/apache/hop/workflow/actions/addresultfilenames/ActionAddResultFilenames.java
index 93aa843920..21df95d807 100644
---
a/plugins/actions/addresultfilenames/src/main/java/org/apache/hop/workflow/actions/addresultfilenames/ActionAddResultFilenames.java
+++
b/plugins/actions/addresultfilenames/src/main/java/org/apache/hop/workflow/actions/addresultfilenames/ActionAddResultFilenames.java
@@ -120,7 +120,8 @@ public class ActionAddResultFilenames extends ActionBase
implements Cloneable, I
// Get values from previous result
String fileFolderPrevious = resultRow.getString(0, null);
- String fileMasksPrevious = resultRow.getString(1, null);
+ // The wildcard is optional: a row with only the file/folder takes all
its files
+ String fileMasksPrevious = resultRow.size() > 1 ?
resultRow.getString(1, null) : null;
// ok we can process this file/folder
if (isDetailed()) {
diff --git
a/plugins/actions/addresultfilenames/src/test/java/org/apache/hop/workflow/actions/addresultfilenames/ActionAddResultFilenamesSubfolderTest.java
b/plugins/actions/addresultfilenames/src/test/java/org/apache/hop/workflow/actions/addresultfilenames/ActionAddResultFilenamesSubfolderTest.java
index fc288a383e..b42d99d17c 100644
---
a/plugins/actions/addresultfilenames/src/test/java/org/apache/hop/workflow/actions/addresultfilenames/ActionAddResultFilenamesSubfolderTest.java
+++
b/plugins/actions/addresultfilenames/src/test/java/org/apache/hop/workflow/actions/addresultfilenames/ActionAddResultFilenamesSubfolderTest.java
@@ -28,7 +28,9 @@ import java.nio.file.Path;
import java.nio.file.attribute.PosixFilePermissions;
import java.util.List;
import org.apache.hop.core.Result;
+import org.apache.hop.core.RowMetaAndData;
import org.apache.hop.core.logging.HopLogStore;
+import org.apache.hop.core.row.value.ValueMetaString;
import org.apache.hop.workflow.WorkflowMeta;
import org.apache.hop.workflow.action.ActionMeta;
import org.apache.hop.workflow.engine.IWorkflowEngine;
@@ -109,6 +111,23 @@ class ActionAddResultFilenamesSubfolderTest {
assertEquals(2, result.getResultFiles().size(), "the nested file must be
picked up");
}
+ @Test
+ void previousResultRowWithOnlyTheFolderAddsAllItsFiles() throws Exception {
+ Files.createFile(folder.resolve("report.csv"));
+ Files.createFile(folder.resolve("summary.txt"));
+ RowMetaAndData row = new RowMetaAndData();
+ row.addValue(new ValueMetaString("path"), folder.toString());
+ Result previousResult = new Result();
+ previousResult.setRows(List.of(row));
+ action.setArgFromPrevious(true);
+
+ Result result = action.execute(previousResult, 0);
+
+ assertTrue(result.isResult());
+ assertEquals(0, result.getNrErrors());
+ assertEquals(2, result.getResultFiles().size());
+ }
+
private Path createUnreadableFolder() throws IOException {
Path unreadable = Files.createDirectory(folder.resolve("no-access"));
Files.createFile(unreadable.resolve("hidden.csv"));
diff --git
a/plugins/actions/createfolder/src/main/java/org/apache/hop/workflow/actions/createfolder/ActionCreateFolder.java
b/plugins/actions/createfolder/src/main/java/org/apache/hop/workflow/actions/createfolder/ActionCreateFolder.java
index 66febf230b..42f4196f63 100644
---
a/plugins/actions/createfolder/src/main/java/org/apache/hop/workflow/actions/createfolder/ActionCreateFolder.java
+++
b/plugins/actions/createfolder/src/main/java/org/apache/hop/workflow/actions/createfolder/ActionCreateFolder.java
@@ -89,15 +89,16 @@ public class ActionCreateFolder extends ActionBase
implements Cloneable, IAction
// Check if it's a folder
//
- if (isFailIfFolderExists()) {
+ if (!isFolder) {
+ // A file has the folder name, so no folder can be created.
+ //
+ result.setResult(false);
+ logError("File [" + realFolderName + "] exists, failing.");
+ } else if (isFailIfFolderExists()) {
// Folder exists and fail flag is enabled.
//
result.setResult(false);
- if (isFolder) {
- logError(CONST_FOLDER + realFolderName + "] exists, failing.");
- } else {
- logError("File [" + realFolderName + "] exists, failing.");
- }
+ logError(CONST_FOLDER + realFolderName + "] exists, failing.");
} else {
// Folder already exists: there is no reason to try and create it.
//
diff --git
a/plugins/actions/createfolder/src/test/java/org/apache/hop/workflow/actions/createfolder/ActionCreateFolderTest.java
b/plugins/actions/createfolder/src/test/java/org/apache/hop/workflow/actions/createfolder/ActionCreateFolderTest.java
index 9813e4faac..36db325eb5 100644
---
a/plugins/actions/createfolder/src/test/java/org/apache/hop/workflow/actions/createfolder/ActionCreateFolderTest.java
+++
b/plugins/actions/createfolder/src/test/java/org/apache/hop/workflow/actions/createfolder/ActionCreateFolderTest.java
@@ -190,6 +190,19 @@ class ActionCreateFolderTest {
assertEquals(0, result.getNrErrors());
}
+ @Test
+ void
executeFailsWhenAFileOccupiesTheFolderNameAndAnExistingFolderIsAllowed() throws
Exception {
+ ActionCreateFolder action = new ActionCreateFolder();
+ Path file = Files.createFile(tempDir.resolve("not-a-folder"));
+ action.setFolderName(file.toString());
+ action.setFailIfFolderExists(false);
+
+ Result result = action.execute(new Result(), 0);
+
+ assertFalse(result.isResult());
+ assertTrue(Files.isRegularFile(file));
+ }
+
@Test
void executeReportsAnErrorWhenTheFolderCannotBeCreated() throws Exception {
ActionCreateFolder action = new ActionCreateFolder();
diff --git
a/plugins/actions/deletefile/src/main/java/org/apache/hop/workflow/actions/deletefile/ActionDeleteFile.java
b/plugins/actions/deletefile/src/main/java/org/apache/hop/workflow/actions/deletefile/ActionDeleteFile.java
index f78125bb06..95f35ff2e7 100644
---
a/plugins/actions/deletefile/src/main/java/org/apache/hop/workflow/actions/deletefile/ActionDeleteFile.java
+++
b/plugins/actions/deletefile/src/main/java/org/apache/hop/workflow/actions/deletefile/ActionDeleteFile.java
@@ -122,11 +122,12 @@ public class ActionDeleteFile extends ActionBase
implements Cloneable, IAction {
PKG, "ActionDeleteFile.ERROR_0005_Could_Not_Delete_File",
realFilename));
result.setResult(false);
result.setNrErrors(1);
+ } else {
+ if (isBasic()) {
+ logBasic(BaseMessages.getString(PKG,
"ActionDeleteFile.File_Deleted", realFilename));
+ }
+ result.setResult(true);
}
- if (isBasic()) {
- logBasic(BaseMessages.getString(PKG,
"ActionDeleteFile.File_Deleted", realFilename));
- }
- result.setResult(true);
}
} catch (Exception e) {
logError(
diff --git
a/plugins/actions/deletefile/src/test/java/org/apache/hop/workflow/actions/deletefile/ActionDeleteFileTest.java
b/plugins/actions/deletefile/src/test/java/org/apache/hop/workflow/actions/deletefile/ActionDeleteFileTest.java
new file mode 100644
index 0000000000..42e6ba47e7
--- /dev/null
+++
b/plugins/actions/deletefile/src/test/java/org/apache/hop/workflow/actions/deletefile/ActionDeleteFileTest.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hop.workflow.actions.deletefile;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.apache.hop.core.Result;
+import org.apache.hop.core.logging.HopLogStore;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+class ActionDeleteFileTest {
+
+ @TempDir Path tempDir;
+
+ @BeforeAll
+ static void setUpBeforeClass() {
+ HopLogStore.init();
+ }
+
+ @Test
+ void executeDeletesTheFile() throws Exception {
+ Path file = Files.createFile(tempDir.resolve("delete-me.txt"));
+ ActionDeleteFile action = new ActionDeleteFile();
+ action.setFilename(file.toString());
+
+ Result result = action.execute(new Result(), 0);
+
+ assertTrue(result.isResult());
+ assertEquals(0, result.getNrErrors());
+ assertFalse(Files.exists(file));
+ }
+
+ @Test
+ void executeFailsWhenTheFileCannotBeDeleted() throws Exception {
+ Path folder = Files.createDirectory(tempDir.resolve("not-empty"));
+ Files.createFile(folder.resolve("child.txt"));
+ ActionDeleteFile action = new ActionDeleteFile();
+ action.setFilename(folder.toString());
+
+ Result result = action.execute(new Result(), 0);
+
+ assertFalse(result.isResult());
+ assertEquals(1, result.getNrErrors());
+ assertTrue(Files.exists(folder));
+ }
+}
diff --git
a/plugins/actions/deleteresultfilenames/src/main/java/org/apache/hop/workflow/actions/deleteresultfilenames/ActionDeleteResultFilenames.java
b/plugins/actions/deleteresultfilenames/src/main/java/org/apache/hop/workflow/actions/deleteresultfilenames/ActionDeleteResultFilenames.java
index 23c15187ae..3132dc6a87 100644
---
a/plugins/actions/deleteresultfilenames/src/main/java/org/apache/hop/workflow/actions/deleteresultfilenames/ActionDeleteResultFilenames.java
+++
b/plugins/actions/deleteresultfilenames/src/main/java/org/apache/hop/workflow/actions/deleteresultfilenames/ActionDeleteResultFilenames.java
@@ -99,7 +99,6 @@ public class ActionDeleteResultFilenames extends ActionBase
implements Cloneable
ResultFile resultFile = it.next();
FileObject file = resultFile.getFile();
if (file != null
- && file.exists()
&& checkFileWildcard(file.getName().getBaseName(),
resolve(wildcard), true)
&& !checkFileWildcard(
file.getName().getBaseName(), resolve(wildcardExclude),
false)) {
diff --git
a/plugins/actions/deleteresultfilenames/src/test/java/org/apache/hop/workflow/actions/deleteresultfilenames/ActionDeleteResultFilenamesTest.java
b/plugins/actions/deleteresultfilenames/src/test/java/org/apache/hop/workflow/actions/deleteresultfilenames/ActionDeleteResultFilenamesTest.java
index 7876b91d5f..b92640c171 100644
---
a/plugins/actions/deleteresultfilenames/src/test/java/org/apache/hop/workflow/actions/deleteresultfilenames/ActionDeleteResultFilenamesTest.java
+++
b/plugins/actions/deleteresultfilenames/src/test/java/org/apache/hop/workflow/actions/deleteresultfilenames/ActionDeleteResultFilenamesTest.java
@@ -20,15 +20,33 @@ package
org.apache.hop.workflow.actions.deleteresultfilenames;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import org.apache.hop.core.ICheckResult;
+import org.apache.hop.core.Result;
+import org.apache.hop.core.ResultFile;
+import org.apache.hop.core.logging.HopLogStore;
+import org.apache.hop.core.vfs.HopVfs;
+import org.apache.hop.workflow.WorkflowMeta;
import org.apache.hop.workflow.action.ActionSerializationTestUtil;
+import org.apache.hop.workflow.engine.IWorkflowEngine;
+import org.apache.hop.workflow.engines.local.LocalWorkflowEngine;
+import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
/** Unit test for {@link ActionDeleteResultFilenames} */
class ActionDeleteResultFilenamesTest {
+ @TempDir Path tempDir;
+
+ @BeforeAll
+ static void setUpBeforeClass() {
+ HopLogStore.init();
+ }
+
@Test
void testSerializationRoundTrip() throws Exception {
ActionDeleteResultFilenames action =
@@ -49,4 +67,34 @@ class ActionDeleteResultFilenamesTest {
assertTrue(remarks.isEmpty());
}
+
+ @Test
+ void matchingResultFileIsRemovedEvenWhenItNoLongerExists() throws Exception {
+ IWorkflowEngine<WorkflowMeta> workflow = new LocalWorkflowEngine(new
WorkflowMeta());
+ workflow.setStopped(false);
+ ActionDeleteResultFilenames action = new ActionDeleteResultFilenames();
+ action.setParentWorkflow(workflow);
+ action.setSpecifyWildcard(true);
+ action.setWildcard(".*\\.csv");
+
+ Result result = new Result();
+ addResultFile(result, tempDir.resolve("deleted-earlier.csv"));
+ addResultFile(result, Files.createFile(tempDir.resolve("kept.txt")));
+
+ result = action.execute(result, 0);
+
+ assertTrue(result.isResult());
+ assertEquals(1, result.getResultFiles().size());
+
assertTrue(result.getResultFiles().keySet().iterator().next().endsWith("kept.txt"));
+ }
+
+ private static void addResultFile(Result result, Path path) throws Exception
{
+ ResultFile resultFile =
+ new ResultFile(
+ ResultFile.FILE_TYPE_GENERAL,
+ HopVfs.getFileObject(path.toString()),
+ "workflow",
+ "action");
+ result.getResultFiles().put(resultFile.getFile().toString(), resultFile);
+ }
}
diff --git
a/plugins/actions/unzip/src/main/java/org/apache/hop/workflow/actions/unzip/ActionUnZip.java
b/plugins/actions/unzip/src/main/java/org/apache/hop/workflow/actions/unzip/ActionUnZip.java
index 89088fb93e..5af455e938 100644
---
a/plugins/actions/unzip/src/main/java/org/apache/hop/workflow/actions/unzip/ActionUnZip.java
+++
b/plugins/actions/unzip/src/main/java/org/apache/hop/workflow/actions/unzip/ActionUnZip.java
@@ -445,7 +445,8 @@ public class ActionUnZip extends ActionBase implements
Cloneable, IAction {
try {
if (fileObject.getType().equals(FileType.FILE)) {
// We have to unzip one zip file
- if (!unzipFile(
+ // unzipFile counts its own errors
+ if (unzipFile(
fileObject,
realTargetdirectory,
realWildcard,
@@ -454,8 +455,6 @@ public class ActionUnZip extends ActionBase implements
Cloneable, IAction {
parentWorkflow,
movetodir,
realMovetodirectory)) {
- updateErrors();
- } else {
updateSuccess();
}
} else {
@@ -487,7 +486,7 @@ public class ActionUnZip extends ActionBase implements
Cloneable, IAction {
unzip = matchesWildcard(patternSource, children[i]);
}
if (unzip) {
- if (!unzipFile(
+ if (unzipFile(
children[i],
realTargetdirectory,
realWildcard,
@@ -496,8 +495,6 @@ public class ActionUnZip extends ActionBase implements
Cloneable, IAction {
parentWorkflow,
movetodir,
realMovetodirectory)) {
- updateErrors();
- } else {
updateSuccess();
}
}
@@ -676,9 +673,10 @@ public class ActionUnZip extends ActionBase implements
Cloneable, IAction {
getItexclude = matchesWildcard(patternexclude, item);
}
- boolean take = takeThisFile(item, newFileName);
+ // Only apply the "if file exists" rule to entries that pass
the wildcards
+ boolean take = getIt && !getItexclude && takeThisFile(item,
newFileName);
- if (getIt && !getItexclude && take) {
+ if (take) {
if (isDetailed()) {
logDetailed(
BaseMessages.getString(
diff --git
a/plugins/actions/unzip/src/test/java/org/apache/hop/workflow/actions/unzip/WorkflowActionUnZipTest.java
b/plugins/actions/unzip/src/test/java/org/apache/hop/workflow/actions/unzip/WorkflowActionUnZipTest.java
index 3567f2ccda..90416f6d2d 100644
---
a/plugins/actions/unzip/src/test/java/org/apache/hop/workflow/actions/unzip/WorkflowActionUnZipTest.java
+++
b/plugins/actions/unzip/src/test/java/org/apache/hop/workflow/actions/unzip/WorkflowActionUnZipTest.java
@@ -19,6 +19,8 @@ package org.apache.hop.workflow.actions.unzip;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assumptions.assumeFalse;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
import java.io.File;
import java.io.FileOutputStream;
@@ -382,6 +384,60 @@ class WorkflowActionUnZipTest extends
WorkflowActionLoadSaveTestSupport<ActionUn
"a.csv was refused by FAIL mode and must keep its modification date");
}
+ @Test
+ void invalidZipFileCountsAsOneError() throws Exception {
+ File workDir = Files.createTempDirectory("unzip-invalid").toFile();
+ workDir.deleteOnExit();
+ File zipFile = new File(workDir, "broken.zip");
+ Files.writeString(zipFile.toPath(), "not a zip file");
+
+ Result result = runUnzipSingleFile(zipFile, new File(workDir, "dst"),
FileExistsEnum.SKIP);
+
+ assertFalse(result.isResult());
+ assertEquals(1, result.getNrErrors());
+ }
+
+ @Test
+ void zipFileThatCannotBeDeletedAfterExtractionCountsAsOneError() throws
Exception {
+ File workDir = Files.createTempDirectory("unzip-delete").toFile();
+ workDir.deleteOnExit();
+ File sourceDir = new File(workDir, "src");
+ assertTrue(sourceDir.mkdir());
+ File zipFile = new File(sourceDir, "data.zip");
+ createZipWithEntries(zipFile, new String[] {"a.csv"}, new String[] {"a"});
+ assumeTrue(sourceDir.setWritable(false), "cannot make the source folder
read-only");
+ try {
+ assumeFalse(sourceDir.canWrite(), "source folder is still writable
(root?)");
+ ActionUnZip action = new ActionUnZip();
+ action.afterUnzip = 1; // delete the zip file
+
+ Result result =
+ runUnzipSingleFile(action, zipFile, new File(workDir, "dst"),
FileExistsEnum.SKIP);
+
+ assertEquals(1, result.getNrErrors());
+ } finally {
+ sourceDir.setWritable(true);
+ }
+ }
+
+ @Test
+ void failModeIgnoresExistingFilesForEntriesFilteredOutByTheWildcard() throws
Exception {
+ File workDir = Files.createTempDirectory("unzip-fail-wildcard").toFile();
+ workDir.deleteOnExit();
+ File zipFile = new File(workDir, "geoip.zip");
+ createZipWithEntries(
+ zipFile, new String[] {"country.txt", "city.txt"}, new String[]
{"country", "city"});
+ File targetDir = new File(workDir, "dst");
+ writeExistingFile(targetDir, "city.txt");
+
+ ActionUnZip action = new ActionUnZip();
+ action.setWildcard("country\\.txt"); // city.txt is filtered out
+ Result result = runUnzipSingleFile(action, zipFile, targetDir,
FileExistsEnum.FAIL);
+
+ assertEquals(0, result.getNrErrors());
+ assertTrue(new File(targetDir, "country.txt").exists());
+ }
+
/**
* Builds a temp folder with two {@code geoip_*.zip} archives, each holding
one distinct entry.
*/
