This is an automated email from the ASF dual-hosted git repository.
hansva 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 1b555e7ae5 Fix#8985 Delete folders workflow check results (#8984)
1b555e7ae5 is described below
commit 1b555e7ae5f4fa70ade7c6849cbf9af91670be20
Author: Lance <[email protected]>
AuthorDate: Tue Oct 20 20:40:47 2026 +0800
Fix#8985 Delete folders workflow check results (#8984)
* Fix Delete folders workflow check results
Signed-off-by: leehaut <[email protected]>
* Fix Delete folders workflow check results
Signed-off-by: leehaut <[email protected]>
* Fix Delete folders workflow check results
Signed-off-by: leehaut <[email protected]>
* Align Delete folders checks with runtime deletion
Signed-off-by: leehaut <[email protected]>
---------
Signed-off-by: leehaut <[email protected]>
---
.../actions/deletefolders/ActionDeleteFolders.java | 119 ++++++++--
.../messages/messages_en_US.properties | 4 +
.../messages/messages_zh_CN.properties | 4 +
.../ActionDeleteFoldersCheckTest.java | 260 +++++++++++++++++++++
4 files changed, 365 insertions(+), 22 deletions(-)
diff --git
a/plugins/actions/deletefolders/src/main/java/org/apache/hop/workflow/actions/deletefolders/ActionDeleteFolders.java
b/plugins/actions/deletefolders/src/main/java/org/apache/hop/workflow/actions/deletefolders/ActionDeleteFolders.java
index 4f23b7682a..91545f1ffd 100644
---
a/plugins/actions/deletefolders/src/main/java/org/apache/hop/workflow/actions/deletefolders/ActionDeleteFolders.java
+++
b/plugins/actions/deletefolders/src/main/java/org/apache/hop/workflow/actions/deletefolders/ActionDeleteFolders.java
@@ -20,6 +20,7 @@ package org.apache.hop.workflow.actions.deletefolders;
import java.util.List;
import lombok.Getter;
import lombok.Setter;
+import org.apache.commons.lang3.StringUtils;
import org.apache.commons.vfs2.FileObject;
import org.apache.commons.vfs2.FileSelectInfo;
import org.apache.commons.vfs2.FileSelector;
@@ -43,6 +44,11 @@ import org.apache.hop.resource.ResourceEntry.ResourceType;
import org.apache.hop.resource.ResourceReference;
import org.apache.hop.workflow.WorkflowMeta;
import org.apache.hop.workflow.action.ActionBase;
+import org.apache.hop.workflow.action.validator.AbstractFileValidator;
+import org.apache.hop.workflow.action.validator.ActionValidatorUtils;
+import org.apache.hop.workflow.action.validator.AndValidator;
+import org.apache.hop.workflow.action.validator.FileExistsValidator;
+import org.apache.hop.workflow.action.validator.ValidatorContext;
/** This defines a 'delete folders' action. */
@Action(
@@ -155,8 +161,9 @@ public class ActionDeleteFolders extends ActionBase {
return result;
}
RowMetaAndData resultRow = rows.get(iteration);
+ // Result rows are data. Leave ${...} unresolved, as the other file
actions do.
String argsPrevious = resultRow.getString(0, null);
- if (!Utils.isEmpty(argsPrevious)) {
+ if (!StringUtils.isBlank(argsPrevious)) {
if (deleteFolder(argsPrevious)) {
updateSuccess();
} else {
@@ -179,7 +186,7 @@ public class ActionDeleteFolders extends ActionBase {
return result;
}
String realFileName = resolve(item.getFileName());
- if (!Utils.isEmpty(realFileName)) {
+ if (!StringUtils.isBlank(realFileName)) {
if (deleteFolder(realFileName)) {
updateSuccess();
} else {
@@ -292,32 +299,99 @@ public class ActionDeleteFolders extends ActionBase {
WorkflowMeta workflowMeta,
IVariables variables,
IHopMetadataProvider metadataProvider) {
- // boolean res =
- // ActionValidatorUtils.andValidator()
- // .validate(
- // this,
- // "fileItems",
- // remarks,
- //
AndValidator.putValidators(ActionValidatorUtils.notNullValidator()));
- //
- // if (!res) {
- // return;
- // }
+ // Folder names come from the previous action result at runtime.
+ if (argFromPrevious) {
+ return;
+ }
if (Utils.isEmpty(fileItems)) {
- String message = BaseMessages.getString(PKG,
"CloneRowMeta.CheckResult.NrClonesdMissing");
remarks.add(
- new CheckResult(ICheckResult.TYPE_RESULT_WARNING, "Any folders to
deletes", this));
+ new CheckResult(
+ ICheckResult.TYPE_RESULT_WARNING,
+ BaseMessages.getString(PKG,
"ActionDeleteFolders.CheckResult.NoFolders"),
+ this));
+ return;
}
- /* TODO: If we enable action check
+ IVariables space = variables != null ? variables : getVariables();
ValidatorContext ctx = new ValidatorContext();
- AbstractFileValidator.putVariableSpace(ctx, getVariables());
+ AbstractFileValidator.putVariableSpace(ctx, space);
+ // A missing folder is a warning. Runtime treats it as already deleted,
and hop lint fails
+ // on ERROR by default.
+ FileExistsValidator.putFailIfDoesNotExist(ctx, true);
+ ActionValidatorUtils.putLevelOnFail(
+ ctx.getMap(), FileExistsValidator.INSTANCE.getName(),
ICheckResult.TYPE_RESULT_WARNING);
AndValidator.putValidators(
- ctx, ActionValidatorUtils.notNullValidator(),
ActionValidatorUtils.fileExistsValidator());
- for (FileItem item : fileItems) {
- ActionValidatorUtils.andValidator().validate(this, "fileName", remarks,
ctx);
- }*/
+ ctx, ActionValidatorUtils.notBlankValidator(),
ActionValidatorUtils.fileExistsValidator());
+
+ for (int i = 0; i < fileItems.size(); i++) {
+ String propertyName = "fileItems[" + i + "].fileName";
+ String folderName =
Const.NVL(space.resolve(fileItems.get(i).getFileName()), "");
+ if (skipsExistenceProbe(folderName)) {
+ ActionValidatorUtils.notBlankValidator().validate(this, propertyName,
remarks, ctx);
+ continue;
+ }
+ int sizeBefore = remarks.size();
+ if (!ActionValidatorUtils.andValidator().validate(this, propertyName,
remarks, ctx)) {
+ continue;
+ }
+ ICheckResult passed = remarks.size() > sizeBefore ? remarks.removeLast()
: null;
+ ICheckResult folderType = folderTypeRemark(space, folderName);
+ if (folderType != null) {
+ remarks.add(folderType);
+ } else if (passed != null) {
+ remarks.add(passed);
+ }
+ }
+ }
+
+ /**
+ * Background lint calls this method while the workflow is edited. Opening
sftp, s3, and similar
+ * URLs would block that pass on the network, so only local paths are probed.
+ */
+ private static boolean skipsExistenceProbe(String folderName) {
+ int separator = folderName.indexOf("://");
+ if (separator <= 0) {
+ return false;
+ }
+ return !"file".equalsIgnoreCase(folderName.substring(0, separator));
+ }
+
+ /** One remark for this row. {@code null} means the path is a folder and the
OK remark stands. */
+ private ICheckResult folderTypeRemark(IVariables space, String folderName) {
+ FileObject folder = null;
+ try {
+ folder = HopVfs.getFileObject(folderName, space);
+ if (!folder.exists()) {
+ return new CheckResult(
+ ICheckResult.TYPE_RESULT_WARNING,
+ BaseMessages.getString(
+ PKG, "ActionDeleteFolders.CheckResult.FolderDoesNotExist",
folderName),
+ this);
+ }
+ if (folder.getType() != FileType.FOLDER) {
+ return new CheckResult(
+ ICheckResult.TYPE_RESULT_ERROR,
+ BaseMessages.getString(PKG,
"ActionDeleteFolders.CheckResult.NotAFolder", folderName),
+ this);
+ }
+ return null;
+ } catch (Exception e) {
+ String detail = e.getMessage() == null ? e.getClass().getSimpleName() :
e.getMessage();
+ return new CheckResult(
+ ICheckResult.TYPE_RESULT_ERROR,
+ BaseMessages.getString(
+ PKG, "ActionDeleteFolders.CheckResult.CouldNotValidate",
folderName, detail),
+ this);
+ } finally {
+ if (folder != null) {
+ try {
+ folder.close();
+ } catch (Exception ignored) {
+ // close() must not add a second remark for the same row.
+ }
+ }
+ }
}
@Override
@@ -326,8 +400,9 @@ public class ActionDeleteFolders extends ActionBase {
List<ResourceReference> references =
super.getResourceDependencies(variables, workflowMeta);
ResourceReference reference = null;
+ IVariables space = variables != null ? variables : getVariables();
for (FileItem item : fileItems) {
- String filename = resolve(item.getFileName());
+ String filename = Const.NVL(space.resolve(item.getFileName()), "");
if (reference == null) {
reference = new ResourceReference(this);
references.add(reference);
diff --git
a/plugins/actions/deletefolders/src/main/resources/org/apache/hop/workflow/actions/deletefolders/messages/messages_en_US.properties
b/plugins/actions/deletefolders/src/main/resources/org/apache/hop/workflow/actions/deletefolders/messages/messages_en_US.properties
index 76776e1c08..4e55e7dd9c 100644
---
a/plugins/actions/deletefolders/src/main/resources/org/apache/hop/workflow/actions/deletefolders/messages/messages_en_US.properties
+++
b/plugins/actions/deletefolders/src/main/resources/org/apache/hop/workflow/actions/deletefolders/messages/messages_en_US.properties
@@ -15,6 +15,10 @@
# limitations under the License.
#
+ActionDeleteFolders.CheckResult.CouldNotValidate=Could not validate folder
[{0}], exception\: {1}
+ActionDeleteFolders.CheckResult.FolderDoesNotExist=Folder [{0}] does not exist.
+ActionDeleteFolders.CheckResult.NoFolders=No folders to delete
+ActionDeleteFolders.CheckResult.NotAFolder=[{0}] is not a folder.
ActionDeleteFolders.CouldNotDelete=Could not delete folder [{0}], exception\:
{1}
ActionDeleteFolders.Description=Delete specified folders.\nAttention : if a
the folder contains files, Apache Hop will delete them all!
ActionDeleteFolders.Error.EmptyLine=Empty line..ignoring
diff --git
a/plugins/actions/deletefolders/src/main/resources/org/apache/hop/workflow/actions/deletefolders/messages/messages_zh_CN.properties
b/plugins/actions/deletefolders/src/main/resources/org/apache/hop/workflow/actions/deletefolders/messages/messages_zh_CN.properties
index 2be2f3d757..41dcfbf3ac 100644
---
a/plugins/actions/deletefolders/src/main/resources/org/apache/hop/workflow/actions/deletefolders/messages/messages_zh_CN.properties
+++
b/plugins/actions/deletefolders/src/main/resources/org/apache/hop/workflow/actions/deletefolders/messages/messages_zh_CN.properties
@@ -17,6 +17,10 @@
#
#
+ActionDeleteFolders.CheckResult.CouldNotValidate=\u65E0\u6CD5\u6821\u9A8C\u6587\u4EF6\u5939
[{0}]\uFF0C\u5F02\u5E38\uFF1A{1}
+ActionDeleteFolders.CheckResult.FolderDoesNotExist=\u6587\u4EF6\u5939 [{0}]
\u4E0D\u5B58\u5728\u3002
+ActionDeleteFolders.CheckResult.NoFolders=\u6CA1\u6709\u8981\u5220\u9664\u7684\u6587\u4EF6\u5939
+ActionDeleteFolders.CheckResult.NotAFolder=[{0}]
\u4E0D\u662F\u6587\u4EF6\u5939\u3002
ActionDeleteFolders.CouldNotDelete=\u65E0\u6CD5\u5220\u9664\u76EE\u5F55 [{0},
\u5F02\u5E38\: {1}
ActionDeleteFolders.Description=\u5220\u9664\u6307\u5B9A\u76EE\u5F55.\n\u63D0\u9192\:
Apache Hop
\u5C06\u5220\u9664\u8BE5\u76EE\u5F55\u53CA\u5176\u4E0B\u9762\u6240\u6709\u6587\u4EF6
ActionDeleteFolders.Error.EmptyLine=\u5FFD\u7565\u7A7A\u884C
diff --git
a/plugins/actions/deletefolders/src/test/java/org/apache/hop/workflow/actions/deletefolders/ActionDeleteFoldersCheckTest.java
b/plugins/actions/deletefolders/src/test/java/org/apache/hop/workflow/actions/deletefolders/ActionDeleteFoldersCheckTest.java
new file mode 100644
index 0000000000..65229464e4
--- /dev/null
+++
b/plugins/actions/deletefolders/src/test/java/org/apache/hop/workflow/actions/deletefolders/ActionDeleteFoldersCheckTest.java
@@ -0,0 +1,260 @@
+/*
+ * 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.deletefolders;
+
+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.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hop.core.HopClientEnvironment;
+import org.apache.hop.core.ICheckResult;
+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.RowMeta;
+import org.apache.hop.core.row.value.ValueMetaString;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.i18n.BaseMessages;
+import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
+import org.apache.hop.resource.ResourceEntry;
+import org.apache.hop.resource.ResourceReference;
+import org.apache.hop.workflow.WorkflowMeta;
+import org.apache.hop.workflow.action.validator.ValidatorMessages;
+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;
+
+class ActionDeleteFoldersCheckTest {
+
+ @TempDir Path tempDir;
+
+ @BeforeAll
+ static void setUpClass() throws Exception {
+ HopClientEnvironment.init();
+ HopLogStore.init();
+ }
+
+ @Test
+ void emptyFolderListWarns() {
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+
+ List<ICheckResult> remarks = check(action, new Variables());
+
+ assertEquals(1, remarks.size());
+ assertEquals(ICheckResult.TYPE_RESULT_WARNING,
remarks.getFirst().getType());
+ assertEquals(
+ message("ActionDeleteFolders.CheckResult.NoFolders"),
remarks.getFirst().getText());
+ }
+
+ @Test
+ void previousResultsSkipTheFolderTable() {
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ action.setArgFromPrevious(true);
+
+ assertTrue(check(action, new Variables()).isEmpty());
+ }
+
+ @Test
+ void missingFolderIsAWarning() {
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ action.setFileItems(List.of(new
FileItem(tempDir.resolve("missing").toString())));
+
+ List<ICheckResult> remarks = check(action, new Variables());
+
+ assertEquals(1, remarks.size());
+ assertEquals(ICheckResult.TYPE_RESULT_WARNING,
remarks.getFirst().getType());
+ assertEquals(
+ ValidatorMessages.getString("messages.failed.fileExists",
"fileItems[0].fileName"),
+ remarks.getFirst().getText());
+ }
+
+ @Test
+ void existingFolderIsOk() throws Exception {
+ Path folder = tempDir.resolve("to-delete");
+ Files.createDirectories(folder);
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ action.setFileItems(List.of(new FileItem(folder.toString())));
+
+ List<ICheckResult> remarks = check(action, new Variables());
+
+ assertEquals(1, remarks.size());
+ assertEquals(ICheckResult.TYPE_RESULT_OK, remarks.getFirst().getType());
+ assertEquals(
+ passedMessage("fileItems[0].fileName", folder.toString()),
remarks.getFirst().getText());
+ }
+
+ @Test
+ void fileIsNotAFolder() throws Exception {
+ Path file = tempDir.resolve("a.txt");
+ Files.writeString(file, "x", StandardCharsets.UTF_8);
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ action.setFileItems(List.of(new FileItem(file.toString())));
+
+ List<ICheckResult> remarks = check(action, new Variables());
+
+ assertEquals(1, remarks.size());
+ assertEquals(ICheckResult.TYPE_RESULT_ERROR, remarks.getFirst().getType());
+ assertEquals(
+ message("ActionDeleteFolders.CheckResult.NotAFolder", file.toString()),
+ remarks.getFirst().getText());
+ }
+
+ @Test
+ void blankFolderNameIsReported() {
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ action.setFileItems(List.of(new FileItem(" ")));
+
+ List<ICheckResult> remarks = check(action, new Variables());
+
+ assertEquals(1, remarks.size());
+ assertEquals(ICheckResult.TYPE_RESULT_ERROR, remarks.getFirst().getType());
+ assertEquals(
+ ValidatorMessages.getString("messages.failed.notBlank",
"fileItems[0].fileName"),
+ remarks.getFirst().getText());
+ }
+
+ @Test
+ void variablePathResolvesBeforeTheCheck() throws Exception {
+ Path folder = tempDir.resolve("from-variable");
+ Files.createDirectories(folder);
+ Variables variables = new Variables();
+ variables.setVariable("FOLDER", folder.toString());
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ action.setFileItems(List.of(new FileItem("${FOLDER}")));
+
+ List<ICheckResult> remarks = check(action, variables);
+
+ assertEquals(1, remarks.size());
+ assertEquals(ICheckResult.TYPE_RESULT_OK, remarks.getFirst().getType());
+ assertEquals(passedMessage("fileItems[0].fileName", "${FOLDER}"),
remarks.getFirst().getText());
+ }
+
+ @Test
+ void remoteFolderIsNotProbed() {
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ action.setFileItems(List.of(new FileItem("sftp://example.com/data";)));
+
+ assertTrue(check(action, new Variables()).isEmpty());
+ }
+
+ @Test
+ void unknownSchemeIsNotProbed() {
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ action.setFileItems(List.of(new FileItem("nosuchscheme://x")));
+
+ assertTrue(check(action, new Variables()).isEmpty());
+ }
+
+ @Test
+ void previousResultNameIsNotResolved() throws Exception {
+ Path kept = tempDir.resolve("kept");
+ Files.createDirectories(kept);
+ Files.writeString(kept.resolve("marker.txt"), "x", StandardCharsets.UTF_8);
+ ActionDeleteFolders action = attachedAction();
+ action.setArgFromPrevious(true);
+ action.setVariable("FOLDER", kept.toString());
+
+ Result result = action.execute(resultWithFolder("${FOLDER}"), 0);
+
+ assertEquals(0, result.getNrErrors());
+ assertTrue(Files.exists(kept.resolve("marker.txt")));
+ }
+
+ @Test
+ void previousResultDeletesTheLiteralFolder() throws Exception {
+ Path folder = tempDir.resolve("from-row");
+ Files.createDirectories(folder);
+ ActionDeleteFolders action = attachedAction();
+ action.setArgFromPrevious(true);
+
+ Result result = action.execute(resultWithFolder(folder.toString()), 0);
+
+ assertEquals(0, result.getNrErrors());
+ assertEquals(1, result.getNrLinesDeleted());
+ assertFalse(Files.exists(folder));
+ }
+
+ @Test
+ void blankNamesAreIgnoredAtRuntime() throws Exception {
+ ActionDeleteFolders action = attachedAction();
+ action.setFileItems(List.of(new FileItem(" ")));
+
+ Result result = action.execute(new Result(), 0);
+
+ assertEquals(0, result.getNrErrors());
+ assertEquals(0, result.getNrLinesDeleted());
+ assertTrue(result.getResult());
+ }
+
+ @Test
+ void resourceDependenciesUseTheGivenVariables() {
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ action.setVariable("FOLDER", "from-action");
+ action.setFileItems(List.of(new FileItem("${FOLDER}/data")));
+ Variables caller = new Variables();
+ caller.setVariable("FOLDER", "from-caller");
+
+ List<ResourceReference> references =
action.getResourceDependencies(caller, new WorkflowMeta());
+
+ assertEquals(1, references.size());
+ ResourceEntry entry = references.getFirst().getEntries().getFirst();
+ assertEquals(ResourceEntry.ResourceType.FILE, entry.getResourcetype());
+ assertEquals("from-caller/data", entry.getResource());
+ }
+
+ private static String passedMessage(String propertyName, String value) {
+ String shown = value;
+ if (value.length() > 20) {
+ shown = value.substring(0, 20) + "...";
+ }
+ return ValidatorMessages.getString("messages.passed", propertyName, shown);
+ }
+
+ private static ActionDeleteFolders attachedAction() {
+ ActionDeleteFolders action = new ActionDeleteFolders("delete folders");
+ IWorkflowEngine<WorkflowMeta> workflow = new LocalWorkflowEngine(new
WorkflowMeta());
+ workflow.setStopped(false);
+ action.setParentWorkflow(workflow);
+ return action;
+ }
+
+ private static Result resultWithFolder(String folderName) {
+ RowMeta rowMeta = new RowMeta();
+ rowMeta.addValueMeta(new ValueMetaString("folder"));
+ Result result = new Result();
+ result.setRows(List.of(new RowMetaAndData(rowMeta, folderName)));
+ return result;
+ }
+
+ private static String message(String key, String... arguments) {
+ return BaseMessages.getString(ActionDeleteFolders.class, key, (Object[])
arguments);
+ }
+
+ private static List<ICheckResult> check(ActionDeleteFolders action,
Variables variables) {
+ List<ICheckResult> remarks = new ArrayList<>();
+ action.check(remarks, new WorkflowMeta(), variables, new
MemoryMetadataProvider());
+ return remarks;
+ }
+}
