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 6bd9e191ef #8726: Fix incorrect check validation logic in
ClosureGenerator and ActionEvalTableContent (#8727)
6bd9e191ef is described below
commit 6bd9e191ef1c7d0219c695620ba744255aa7ca59
Author: Samuel Willyanto <[email protected]>
AuthorDate: Tue Oct 6 04:40:28 2026 +0700
#8726: Fix incorrect check validation logic in ClosureGenerator and
ActionEvalTableContent (#8727)
---
.../ActionEvalTableContent.java | 18 ++++++++-
.../WorkflowActionEvalTableContentTest.java | 47 ++++++++++++++++++++++
.../transforms/closure/ClosureGeneratorMeta.java | 4 +-
.../closure/ClosureGeneratorMetaTest.java | 32 +++++++++++++++
4 files changed, 98 insertions(+), 3 deletions(-)
diff --git
a/plugins/actions/evaluatetablecontent/src/main/java/org/apache/hop/workflow/actions/evaluatetablecontent/ActionEvalTableContent.java
b/plugins/actions/evaluatetablecontent/src/main/java/org/apache/hop/workflow/actions/evaluatetablecontent/ActionEvalTableContent.java
index 28885298e8..e040551e66 100644
---
a/plugins/actions/evaluatetablecontent/src/main/java/org/apache/hop/workflow/actions/evaluatetablecontent/ActionEvalTableContent.java
+++
b/plugins/actions/evaluatetablecontent/src/main/java/org/apache/hop/workflow/actions/evaluatetablecontent/ActionEvalTableContent.java
@@ -392,8 +392,24 @@ public class ActionEvalTableContent extends ActionBase {
ActionValidatorUtils.andValidator()
.validate(
this,
- "WaitForSQL",
+ "connection",
remarks,
AndValidator.putValidators(ActionValidatorUtils.notBlankValidator()));
+
+ if (useCustomSql) {
+ ActionValidatorUtils.andValidator()
+ .validate(
+ this,
+ "customSql",
+ remarks,
+
AndValidator.putValidators(ActionValidatorUtils.notBlankValidator()));
+ } else {
+ ActionValidatorUtils.andValidator()
+ .validate(
+ this,
+ "tableName",
+ remarks,
+
AndValidator.putValidators(ActionValidatorUtils.notBlankValidator()));
+ }
}
}
diff --git
a/plugins/actions/evaluatetablecontent/src/test/java/org/apache/hop/workflow/actions/evaluatetablecontent/WorkflowActionEvalTableContentTest.java
b/plugins/actions/evaluatetablecontent/src/test/java/org/apache/hop/workflow/actions/evaluatetablecontent/WorkflowActionEvalTableContentTest.java
index 2a8a321a50..a7800bd37a 100644
---
a/plugins/actions/evaluatetablecontent/src/test/java/org/apache/hop/workflow/actions/evaluatetablecontent/WorkflowActionEvalTableContentTest.java
+++
b/plugins/actions/evaluatetablecontent/src/test/java/org/apache/hop/workflow/actions/evaluatetablecontent/WorkflowActionEvalTableContentTest.java
@@ -26,9 +26,12 @@ import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
+import java.util.ArrayList;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import org.apache.hop.core.HopClientEnvironment;
+import org.apache.hop.core.ICheckResult;
import org.apache.hop.core.Result;
import org.apache.hop.core.database.BaseDatabaseMeta;
import org.apache.hop.core.database.DatabaseMeta;
@@ -39,6 +42,7 @@ import org.apache.hop.core.plugins.IPlugin;
import org.apache.hop.core.plugins.IPluginType;
import org.apache.hop.core.plugins.PluginRegistry;
import org.apache.hop.core.row.IValueMeta;
+import org.apache.hop.core.variables.Variables;
import org.apache.hop.junit.rules.RestoreHopEngineEnvironmentExtension;
import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
import org.apache.hop.workflow.WorkflowMeta;
@@ -264,4 +268,47 @@ class WorkflowActionEvalTableContentTest {
assertNull(action.getDatabase(), "The previously resolved connection has
to be discarded");
}
+
+ @Test
+ void testCheck() {
+ List<ICheckResult> remarks = new ArrayList<>();
+ WorkflowMeta workflowMeta = new WorkflowMeta();
+ Variables variables = new Variables();
+
+ // 1. When connection and tableName are blank, expect error remarks
+ action.setConnection("");
+ action.setTableName("");
+ action.setUseCustomSql(false);
+ action.check(remarks, workflowMeta, variables, null);
+ assertTrue(
+ remarks.stream().anyMatch(r -> r.getType() ==
ICheckResult.TYPE_RESULT_ERROR),
+ "Expected errors when connection and table name are blank");
+
+ // 2. When connection and tableName are provided, expect only OK remarks
(no WaitForSQL error)
+ remarks.clear();
+ action.setConnection("my_connection");
+ action.setTableName("my_table");
+ action.setUseCustomSql(false);
+ action.check(remarks, workflowMeta, variables, null);
+ assertTrue(
+ remarks.stream().noneMatch(r -> r.getType() ==
ICheckResult.TYPE_RESULT_ERROR),
+ "Expected no errors when connection and table name are set");
+
+ // 3. When custom SQL is used and customSql is blank, expect error
+ remarks.clear();
+ action.setUseCustomSql(true);
+ action.setCustomSql("");
+ action.check(remarks, workflowMeta, variables, null);
+ assertTrue(
+ remarks.stream().anyMatch(r -> r.getType() ==
ICheckResult.TYPE_RESULT_ERROR),
+ "Expected error when custom SQL is enabled but customSql is blank");
+
+ // 4. When custom SQL is used and customSql is provided, expect no error
+ remarks.clear();
+ action.setCustomSql("SELECT count(*) FROM my_table");
+ action.check(remarks, workflowMeta, variables, null);
+ assertTrue(
+ remarks.stream().noneMatch(r -> r.getType() ==
ICheckResult.TYPE_RESULT_ERROR),
+ "Expected no errors when connection and customSql are set");
+ }
}
diff --git
a/plugins/transforms/closure/src/main/java/org/apache/hop/pipeline/transforms/closure/ClosureGeneratorMeta.java
b/plugins/transforms/closure/src/main/java/org/apache/hop/pipeline/transforms/closure/ClosureGeneratorMeta.java
index 48fa2de064..67350b79a6 100644
---
a/plugins/transforms/closure/src/main/java/org/apache/hop/pipeline/transforms/closure/ClosureGeneratorMeta.java
+++
b/plugins/transforms/closure/src/main/java/org/apache/hop/pipeline/transforms/closure/ClosureGeneratorMeta.java
@@ -115,7 +115,7 @@ public class ClosureGeneratorMeta
CheckResult cr;
IValueMeta parentValueMeta = prev.searchValueMeta(parentIdFieldName);
- if (parentValueMeta != null) {
+ if (parentValueMeta == null) {
cr =
new CheckResult(
ICheckResult.TYPE_RESULT_ERROR,
@@ -132,7 +132,7 @@ public class ClosureGeneratorMeta
}
IValueMeta childValueMeta = prev.searchValueMeta(childIdFieldName);
- if (childValueMeta != null) {
+ if (childValueMeta == null) {
cr =
new CheckResult(
ICheckResult.TYPE_RESULT_ERROR,
diff --git
a/plugins/transforms/closure/src/test/java/org/apache/hop/pipeline/transforms/closure/ClosureGeneratorMetaTest.java
b/plugins/transforms/closure/src/test/java/org/apache/hop/pipeline/transforms/closure/ClosureGeneratorMetaTest.java
index 27d62d3160..4cd6dfaf1f 100644
---
a/plugins/transforms/closure/src/test/java/org/apache/hop/pipeline/transforms/closure/ClosureGeneratorMetaTest.java
+++
b/plugins/transforms/closure/src/test/java/org/apache/hop/pipeline/transforms/closure/ClosureGeneratorMetaTest.java
@@ -46,4 +46,36 @@ class ClosureGeneratorMetaTest {
void testSerialization() throws HopException {
loadSaveTester.testSerialization();
}
+
+ @Test
+ void testCheckReportsOkWhenFieldsExistAndErrorWhenMissing() {
+ ClosureGeneratorMeta meta = new ClosureGeneratorMeta();
+ meta.setParentIdFieldName("parent_id");
+ meta.setChildIdFieldName("child_id");
+
+ org.apache.hop.core.row.IRowMeta prev = new
org.apache.hop.core.row.RowMeta();
+ prev.addValueMeta(new
org.apache.hop.core.row.value.ValueMetaInteger("parent_id"));
+ prev.addValueMeta(new
org.apache.hop.core.row.value.ValueMetaInteger("child_id"));
+
+ java.util.List<org.apache.hop.core.ICheckResult> remarks = new
java.util.ArrayList<>();
+ meta.check(remarks, null, null, prev, new String[0], new String[0], null,
null, null);
+
+ org.junit.jupiter.api.Assertions.assertEquals(2, remarks.size());
+ org.junit.jupiter.api.Assertions.assertEquals(
+ org.apache.hop.core.ICheckResult.TYPE_RESULT_OK,
remarks.get(0).getType());
+ org.junit.jupiter.api.Assertions.assertEquals(
+ org.apache.hop.core.ICheckResult.TYPE_RESULT_OK,
remarks.get(1).getType());
+
+ // When fields are missing
+ remarks.clear();
+ meta.setParentIdFieldName("missing_parent");
+ meta.setChildIdFieldName("missing_child");
+ meta.check(remarks, null, null, prev, new String[0], new String[0], null,
null, null);
+
+ org.junit.jupiter.api.Assertions.assertEquals(2, remarks.size());
+ org.junit.jupiter.api.Assertions.assertEquals(
+ org.apache.hop.core.ICheckResult.TYPE_RESULT_ERROR,
remarks.get(0).getType());
+ org.junit.jupiter.api.Assertions.assertEquals(
+ org.apache.hop.core.ICheckResult.TYPE_RESULT_ERROR,
remarks.get(1).getType());
+ }
}
