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 67b7994987 Fix Wait for SQL verification reading the missing property
WaitForSQL (#9013)
67b7994987 is described below
commit 67b799498732dcf35792cf245ee20a416c1b258d
Author: Lance <[email protected]>
AuthorDate: Mon Oct 19 16:50:19 2026 +0800
Fix Wait for SQL verification reading the missing property WaitForSQL
(#9013)
Signed-off-by: leehaut <[email protected]>
---
.../actions/waitforsql/ActionWaitForSql.java | 249 ++-------------------
.../waitforsql/ActionWaitForSqlCheckTest.java | 170 ++++++++++++++
.../actions/waitforsql/ActionWaitForSqlTest.java | 78 +++++++
.../resources/wf-wait-for-sql-ghost-property.hwf | 73 ++++++
4 files changed, 342 insertions(+), 228 deletions(-)
diff --git
a/plugins/actions/waitforsql/src/main/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSql.java
b/plugins/actions/waitforsql/src/main/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSql.java
index 147c22f935..ce3ce30192 100644
---
a/plugins/actions/waitforsql/src/main/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSql.java
+++
b/plugins/actions/waitforsql/src/main/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSql.java
@@ -19,6 +19,8 @@ package org.apache.hop.workflow.actions.waitforsql;
import java.util.ArrayList;
import java.util.List;
+import lombok.Getter;
+import lombok.Setter;
import org.apache.hop.core.Const;
import org.apache.hop.core.ICheckResult;
import org.apache.hop.core.Result;
@@ -47,6 +49,8 @@ import
org.apache.hop.workflow.action.validator.ActionValidatorUtils;
import org.apache.hop.workflow.action.validator.AndValidator;
/** This defines a Wait for SQL data action */
+@Getter
+@Setter
@Action(
id = "WAIT_FOR_SQL",
name = "i18n::ActionWaitForSQL.Name",
@@ -114,6 +118,7 @@ public class ActionWaitForSql extends ActionBase implements
Cloneable, IAction {
public static final int SUCCESS_CONDITION_ROWS_COUNT_GREATER = 4;
public static final int SUCCESS_CONDITION_ROWS_COUNT_GREATER_EQUAL = 5;
+ @Getter
public enum SuccessCondition implements IEnumHasCodeAndDescription {
ROWS_COUNT_EQUAL(
"rows_count_equal",
@@ -155,26 +160,6 @@ public class ActionWaitForSql extends ActionBase
implements Cloneable, IAction {
public static SuccessCondition lookupCode(String code) {
return IEnumHasCode.lookupCode(SuccessCondition.class, code,
ROWS_COUNT_EQUAL);
}
-
- /**
- * Gets code
- *
- * @return value of code
- */
- @Override
- public String getCode() {
- return code;
- }
-
- /**
- * Gets description
- *
- * @return value of description
- */
- @Override
- public String getDescription() {
- return description;
- }
}
private static final String DEFAULT_MAXIMUM_TIMEOUT = "0"; // infinite
timeout
@@ -201,10 +186,6 @@ public class ActionWaitForSql extends ActionBase
implements Cloneable, IAction {
this("");
}
- public SuccessCondition getSuccessCondition() {
- return successCondition;
- }
-
@Override
public boolean isEvaluation() {
return true;
@@ -524,216 +505,28 @@ public class ActionWaitForSql extends ActionBase
implements Cloneable, IAction {
ActionValidatorUtils.andValidator()
.validate(
this,
- "WaitForSQL",
+ "connection",
remarks,
AndValidator.putValidators(ActionValidatorUtils.notBlankValidator()));
- }
-
- /**
- * Gets isClearResultList
- *
- * @return value of isClearResultList
- */
- public boolean isClearResultList() {
- return clearResultList;
- }
-
- /**
- * @param isClearResultList The isClearResultList to set
- */
- public void setClearResultList(boolean isClearResultList) {
- this.clearResultList = isClearResultList;
- }
-
- /**
- * Gets isAddRowsResult
- *
- * @return value of isAddRowsResult
- */
- public boolean isAddRowsResult() {
- return addRowsResult;
- }
-
- /**
- * @param isAddRowsResult The isAddRowsResult to set
- */
- public void setAddRowsResult(boolean isAddRowsResult) {
- this.addRowsResult = isAddRowsResult;
- }
-
- /**
- * Gets isUseVars
- *
- * @return value of isUseVars
- */
- public boolean isUseVars() {
- return useVars;
- }
-
- /**
- * @param isUseVars The isUseVars to set
- */
- public void setUseVars(boolean isUseVars) {
- this.useVars = isUseVars;
- }
-
- /**
- * Gets isCustomSql
- *
- * @return value of isCustomSql
- */
- public boolean isCustomSqlEnabled() {
- return customSqlEnabled;
- }
-
- /**
- * @param isCustomSql The isCustomSql to set
- */
- public void setCustomSqlEnabled(boolean isCustomSql) {
- this.customSqlEnabled = isCustomSql;
- }
-
- /**
- * Gets customSql
- *
- * @return value of customSql
- */
- public String getCustomSql() {
- return customSql;
- }
-
- /**
- * @param customSql The customSql to set
- */
- public void setCustomSql(String customSql) {
- this.customSql = customSql;
- }
-
- /**
- * Gets tableName
- *
- * @return value of tableName
- */
- public String getTableName() {
- return tableName;
- }
- /**
- * @param tableName The tableName to set
- */
- public void setTableName(String tableName) {
- this.tableName = tableName;
- }
-
- /**
- * Gets schemaName
- *
- * @return value of schemaName
- */
- public String getSchemaName() {
- return schemaName;
- }
-
- /**
- * @param schemaName The schemaName to set
- */
- public void setSchemaName(String schemaName) {
- this.schemaName = schemaName;
- }
-
- /**
- * Gets maximumTimeout
- *
- * @return value of maximumTimeout
- */
- public String getMaximumTimeout() {
- return maximumTimeout;
- }
-
- /**
- * @param maximumTimeout The maximumTimeout to set
- */
- public void setMaximumTimeout(String maximumTimeout) {
- this.maximumTimeout = maximumTimeout;
- }
-
- /**
- * Gets checkCycleTime
- *
- * @return value of checkCycleTime
- */
- public String getCheckCycleTime() {
- return checkCycleTime;
- }
-
- /**
- * @param checkCycleTime The checkCycleTime to set
- */
- public void setCheckCycleTime(String checkCycleTime) {
- this.checkCycleTime = checkCycleTime;
- }
-
- /**
- * Gets successOnTimeout
- *
- * @return value of successOnTimeout
- */
- public boolean isSuccessOnTimeout() {
- return successOnTimeout;
- }
-
- /**
- * @param successOnTimeout The successOnTimeout to set
- */
- public void setSuccessOnTimeout(boolean successOnTimeout) {
- this.successOnTimeout = successOnTimeout;
+ if (customSqlEnabled) {
+ ActionValidatorUtils.andValidator()
+ .validate(
+ this,
+ "customSql",
+ remarks,
+
AndValidator.putValidators(ActionValidatorUtils.notBlankValidator()));
+ } else {
+ ActionValidatorUtils.andValidator()
+ .validate(
+ this,
+ "tableName",
+ remarks,
+
AndValidator.putValidators(ActionValidatorUtils.notBlankValidator()));
+ }
}
- /**
- * Gets selectCount
- *
- * @return value of selectCount
- */
public static String getSelectCount() {
return SELECT_COUNT;
}
-
- /**
- * Gets rowsCountValue
- *
- * @return value of rowsCountValue
- */
- public String getRowsCountValue() {
- return rowsCountValue;
- }
-
- /**
- * @param rowsCountValue The rowsCountValue to set
- */
- public void setRowsCountValue(String rowsCountValue) {
- this.rowsCountValue = rowsCountValue;
- }
-
- /**
- * @param successCondition The successCondition to set
- */
- public void setSuccessCondition(SuccessCondition successCondition) {
- this.successCondition = successCondition;
- }
-
- /**
- * Gets connection
- *
- * @return value of connection
- */
- public String getConnection() {
- return connection;
- }
-
- /**
- * @param connection The connection to set
- */
- public void setConnection(String connection) {
- this.connection = connection;
- }
}
diff --git
a/plugins/actions/waitforsql/src/test/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSqlCheckTest.java
b/plugins/actions/waitforsql/src/test/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSqlCheckTest.java
new file mode 100644
index 0000000000..83d29235f5
--- /dev/null
+++
b/plugins/actions/waitforsql/src/test/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSqlCheckTest.java
@@ -0,0 +1,170 @@
+/*
+ * 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.waitforsql;
+
+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.io.InputStream;
+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.annotations.Action;
+import org.apache.hop.core.plugins.ActionPluginType;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
+import org.apache.hop.workflow.WorkflowMeta;
+import org.apache.hop.workflow.actions.start.ActionStart;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/** Check validates connection plus tableName or customSql, never the ghost
property WaitForSQL. */
+class ActionWaitForSqlCheckTest {
+
+ @BeforeAll
+ static void initPlugins() throws Exception {
+ HopClientEnvironment.init();
+ PluginRegistry registry = PluginRegistry.getInstance();
+ registry.registerPluginType(ActionPluginType.class);
+ registry.registerPluginClass(ActionStart.class.getName(),
ActionPluginType.class, Action.class);
+ registry.registerPluginClass(
+ ActionWaitForSql.class.getName(), ActionPluginType.class,
Action.class);
+ }
+
+ @Test
+ void blankConnectionAndTableAreReported() {
+ ActionWaitForSql action = new ActionWaitForSql("Wait for SQL");
+
+ assertEquals(
+ List.of("connection cannot be null or blank.", "tableName cannot be
null or blank."),
+ errors(action));
+ }
+
+ @Test
+ void whitespaceConnectionAndTableAreReported() {
+ ActionWaitForSql action = new ActionWaitForSql("Wait for SQL");
+ action.setConnection(" ");
+ action.setTableName("\t");
+ action.setCustomSql("select 1");
+
+ assertEquals(
+ List.of("connection cannot be null or blank.", "tableName cannot be
null or blank."),
+ errors(action));
+ }
+
+ @Test
+ void connectionAndTablePassWithoutReadingCustomSql() {
+ ActionWaitForSql action = readyTableAction();
+ action.setCustomSql(" ");
+
+ assertEquals(List.of(), errors(action));
+ assertTrue(texts(action).contains("connection is OK. Value: dummy-db"));
+ assertTrue(texts(action).contains("tableName is OK. Value: dummy_table"));
+ assertFalse(texts(action).stream().anyMatch(text ->
text.contains("customSql")));
+ }
+
+ @Test
+ void customSqlBlankIsReportedAndTableNameIsIgnored() {
+ ActionWaitForSql action = new ActionWaitForSql("Wait for SQL");
+ action.setConnection("dummy-db");
+ action.setCustomSqlEnabled(true);
+ action.setCustomSql(" ");
+
+ assertEquals(List.of("customSql cannot be null or blank."),
errors(action));
+ }
+
+ @Test
+ void customSqlAndConnectionPassWithoutATable() {
+ ActionWaitForSql action = new ActionWaitForSql("Wait for SQL");
+ action.setConnection("dummy-db");
+ action.setCustomSqlEnabled(true);
+ action.setCustomSql("SELECT count(*) FROM dummy_table");
+
+ assertEquals(List.of(), errors(action));
+ assertTrue(texts(action).contains("customSql is OK. Value: SELECT count(*)
FROM..."));
+ }
+
+ @Test
+ void customSqlStillRequiresAConnection() {
+ ActionWaitForSql action = new ActionWaitForSql("Wait for SQL");
+ action.setCustomSqlEnabled(true);
+ action.setCustomSql("SELECT count(*) FROM dummy_table");
+ action.setTableName("dummy_table");
+
+ assertEquals(List.of("connection cannot be null or blank."),
errors(action));
+ }
+
+ @Test
+ void loadedWorkflowDoesNotReportGhostProperty() throws Exception {
+ MemoryMetadataProvider provider = new MemoryMetadataProvider();
+ Variables variables = new Variables();
+ WorkflowMeta workflowMeta;
+ try (InputStream in =
+ ActionWaitForSqlCheckTest.class.getResourceAsStream(
+ "/wf-wait-for-sql-ghost-property.hwf")) {
+ workflowMeta = new WorkflowMeta(in, provider, variables);
+ }
+
+ ActionWaitForSql action =
+ (ActionWaitForSql) workflowMeta.findAction("Wait for SQL").getAction();
+ assertEquals("dummy-db", action.getConnection());
+ assertEquals("dummy_table", action.getTableName());
+ assertFalse(action.isCustomSqlEnabled());
+
+ List<ICheckResult> remarks = new ArrayList<>();
+ workflowMeta.checkActions(remarks, false, null, variables, provider);
+ List<String> texts = remarks.stream().map(ICheckResult::getText).toList();
+
+ assertEquals(List.of(), errorTexts(remarks));
+ assertFalse(texts.stream().anyMatch(text -> text.contains("WaitForSQL")));
+ assertTrue(texts.contains("connection is OK. Value: dummy-db"));
+ assertTrue(texts.contains("tableName is OK. Value: dummy_table"));
+ }
+
+ private static ActionWaitForSql readyTableAction() {
+ ActionWaitForSql action = new ActionWaitForSql("Wait for SQL");
+ action.setConnection("dummy-db");
+ action.setTableName("dummy_table");
+ action.setCustomSqlEnabled(false);
+ return action;
+ }
+
+ private static List<String> errors(ActionWaitForSql action) {
+ return errorTexts(remarks(action));
+ }
+
+ private static List<String> texts(ActionWaitForSql action) {
+ return remarks(action).stream().map(ICheckResult::getText).toList();
+ }
+
+ private static List<ICheckResult> remarks(ActionWaitForSql action) {
+ List<ICheckResult> remarks = new ArrayList<>();
+ action.check(remarks, new WorkflowMeta(), new Variables(), null);
+ return remarks;
+ }
+
+ private static List<String> errorTexts(List<ICheckResult> remarks) {
+ return remarks.stream()
+ .filter(remark -> remark.getType() == ICheckResult.TYPE_RESULT_ERROR)
+ .map(ICheckResult::getText)
+ .toList();
+ }
+}
diff --git
a/plugins/actions/waitforsql/src/test/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSqlTest.java
b/plugins/actions/waitforsql/src/test/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSqlTest.java
index 73f8d6611a..7042de812b 100644
---
a/plugins/actions/waitforsql/src/test/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSqlTest.java
+++
b/plugins/actions/waitforsql/src/test/java/org/apache/hop/workflow/actions/waitforsql/ActionWaitForSqlTest.java
@@ -19,13 +19,17 @@ package org.apache.hop.workflow.actions.waitforsql;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.List;
import org.apache.hop.core.HopClientEnvironment;
import org.apache.hop.core.database.DatabaseMeta;
+import org.apache.hop.i18n.BaseMessages;
import org.apache.hop.metadata.serializer.memory.MemoryMetadataProvider;
import org.apache.hop.workflow.action.ActionSerializationTestUtil;
import
org.apache.hop.workflow.actions.waitforsql.ActionWaitForSql.SuccessCondition;
+import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;
/** Unit tests for wait for sql action. */
@@ -58,4 +62,78 @@ class ActionWaitForSqlTest {
assertFalse(action.isSuccessOnTimeout());
assertTrue(action.isClearResultList());
}
+
+ @Test
+ void constructorUsesDocumentedDefaults() {
+ ActionWaitForSql action = new ActionWaitForSql("Wait for SQL");
+
+ assertEquals(
+ List.of(true, false, false, false, false),
+ List.of(
+ action.isClearResultList(),
+ action.isAddRowsResult(),
+ action.isUseVars(),
+ action.isCustomSqlEnabled(),
+ action.isSuccessOnTimeout()));
+ assertNull(action.getConnection());
+ assertNull(action.getSchemaName());
+ assertNull(action.getTableName());
+ assertNull(action.getCustomSql());
+ assertEquals(
+ List.of("0", "0", "60"),
+ List.of(
+ action.getRowsCountValue(), action.getMaximumTimeout(),
action.getCheckCycleTime()));
+ assertEquals(SuccessCondition.ROWS_COUNT_GREATER,
action.getSuccessCondition());
+ assertEquals("SELECT count(*) FROM ", ActionWaitForSql.getSelectCount());
+ }
+
+ @Test
+ void accessorsRoundTrip() {
+ ActionWaitForSql action = getActionWaitForSql();
+
+ assertEquals(
+ List.of(false, true, true, true, true),
+ List.of(
+ action.isClearResultList(),
+ action.isAddRowsResult(),
+ action.isUseVars(),
+ action.isCustomSqlEnabled(),
+ action.isSuccessOnTimeout()));
+ assertEquals(
+ List.of("dummy-db", "public", "dummy_table", "select 1", "3", "30",
"5"),
+ List.of(
+ action.getConnection(),
+ action.getSchemaName(),
+ action.getTableName(),
+ action.getCustomSql(),
+ action.getRowsCountValue(),
+ action.getMaximumTimeout(),
+ action.getCheckCycleTime()));
+ assertEquals(SuccessCondition.ROWS_COUNT_EQUAL,
action.getSuccessCondition());
+ assertEquals("rows_count_equal", action.getSuccessCondition().getCode());
+ assertEquals(
+ BaseMessages.getString(
+ ActionWaitForSql.class,
"ActionWaitForSQL.SuccessWhenRowCountEqual.Label"),
+ action.getSuccessCondition().getDescription());
+ assertEquals(
+ SuccessCondition.ROWS_COUNT_EQUAL,
SuccessCondition.lookupCode("rows_count_equal"));
+ }
+
+ private static @NotNull ActionWaitForSql getActionWaitForSql() {
+ ActionWaitForSql action = new ActionWaitForSql("Action wait for SQL");
+ action.setClearResultList(false);
+ action.setAddRowsResult(true);
+ action.setUseVars(true);
+ action.setCustomSqlEnabled(true);
+ action.setSuccessOnTimeout(true);
+ action.setConnection("dummy-db");
+ action.setSchemaName("public");
+ action.setTableName("dummy_table");
+ action.setCustomSql("select 1");
+ action.setRowsCountValue("3");
+ action.setMaximumTimeout("30");
+ action.setCheckCycleTime("5");
+ action.setSuccessCondition(SuccessCondition.ROWS_COUNT_EQUAL);
+ return action;
+ }
}
diff --git
a/plugins/actions/waitforsql/src/test/resources/wf-wait-for-sql-ghost-property.hwf
b/plugins/actions/waitforsql/src/test/resources/wf-wait-for-sql-ghost-property.hwf
new file mode 100644
index 0000000000..55195cdbf6
--- /dev/null
+++
b/plugins/actions/waitforsql/src/test/resources/wf-wait-for-sql-ghost-property.hwf
@@ -0,0 +1,73 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+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.
+
+-->
+<workflow>
+ <name>wf-wait-for-sql-ghost-property</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description/>
+ <extended_description/>
+ <created_user>-</created_user>
+ <modified_user>-</modified_user>
+ <created_date>2026/10/19 09:00:00.000</created_date>
+ <modified_date>2026/10/19 09:00:00.000</modified_date>
+ <parameters/>
+ <actions>
+ <action>
+ <name>Start</name>
+ <description/>
+ <type>SPECIAL</type>
+ <attributes/>
+ <xloc>80</xloc>
+ <yloc>80</yloc>
+ <parallel>N</parallel>
+ </action>
+ <action>
+ <clear_result_rows>N</clear_result_rows>
+ <add_rows_result>N</add_rows_result>
+ <is_usevars>N</is_usevars>
+ <is_custom_sql>N</is_custom_sql>
+ <custom_sql/>
+ <connection>dummy-db</connection>
+ <tablename>dummy_table</tablename>
+ <schemaname/>
+ <maximum_timeout>0</maximum_timeout>
+ <check_cycle_time>5</check_cycle_time>
+ <success_on_timeout>N</success_on_timeout>
+ <rows_count_value>0</rows_count_value>
+ <success_condition>rows_count_greater</success_condition>
+ <name>Wait for SQL</name>
+ <description/>
+ <type>WAIT_FOR_SQL</type>
+ <attributes/>
+ <xloc>320</xloc>
+ <yloc>80</yloc>
+ <parallel>N</parallel>
+ </action>
+ </actions>
+ <hops>
+ <hop>
+ <from>Start</from>
+ <to>Wait for SQL</to>
+ <evaluation>Y</evaluation>
+ <unconditional>Y</unconditional>
+ <enabled>Y</enabled>
+ </hop>
+ </hops>
+ <notepads/>
+</workflow>
