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 92c2a50fbe Keep the action name and plugin id when cloning, fixes
#5644 (#7542)
92c2a50fbe is described below
commit 92c2a50fbe4a255f006b7a4253b97260ae7f2f01
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Thu Jul 16 15:43:02 2026 +0200
Keep the action name and plugin id when cloning, fixes #5644 (#7542)
Resource export clones every action before serializing it, so a copy
constructor that drops the name or plugin id corrupts the export:
* ActionCheckFilesLocked delegated to this(), which reset the name to an
empty string. The exported workflow then held an empty <name>, and the
server failed with a NullPointerException from WorkflowMeta.findAction()
while resolving the hops.
* ActionBase's copy constructor and ActionFtpPut used the two argument
super(), which dropped the plugin id. The exported action was written
without its <type>, and the server failed to load the workflow with
"Unable to load workflow info from XML node".
The ActionBase fix covers the eleven other actions that delegate to it.
Adds unit tests guarding the name/plugin id on clone, and integration test
0010 which runs a workflow containing a Check Files Locked action on a
remote server through the resource export. The test fails on both bugs
before this change.
Co-authored-by: Claude Opus 4.8 <[email protected]>
---
.../org/apache/hop/workflow/action/ActionBase.java | 6 +-
.../hop_server/0010-check-files-locked.hwf | 127 +++++++++++++++++++++
.../main-0010-test-check-files-locked-export.hwf | 95 +++++++++++++++
.../checkfilelocked/ActionCheckFilesLocked.java | 11 +-
.../ActionCheckFilesLockedTest.java | 23 ++++
.../hop/workflow/actions/ftpput/ActionFtpPut.java | 2 +-
.../workflow/actions/ftpput/ActionFtpPutTest.java | 21 ++++
7 files changed, 278 insertions(+), 7 deletions(-)
diff --git
a/engine/src/main/java/org/apache/hop/workflow/action/ActionBase.java
b/engine/src/main/java/org/apache/hop/workflow/action/ActionBase.java
index 456c0a8400..ffc66fa4dd 100644
--- a/engine/src/main/java/org/apache/hop/workflow/action/ActionBase.java
+++ b/engine/src/main/java/org/apache/hop/workflow/action/ActionBase.java
@@ -170,10 +170,14 @@ public abstract class ActionBase
/**
* Copy constructor variant
*
+ * <p>The plugin id is copied as well: resource export clones every action,
and a clone without a
+ * plugin id is written to the export without its {@code <type>} element,
which then fails to load
+ * ("Unable to load workflow info from XML node"). See issue #5644.
+ *
* @param b The action base to copy
*/
protected ActionBase(ActionBase b) {
- this(b.name, b.description);
+ this(b.name, b.description, b.pluginId);
}
/**
diff --git a/integration-tests/hop_server/0010-check-files-locked.hwf
b/integration-tests/hop_server/0010-check-files-locked.hwf
new file mode 100644
index 0000000000..ad99ec518e
--- /dev/null
+++ b/integration-tests/hop_server/0010-check-files-locked.hwf
@@ -0,0 +1,127 @@
+<?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>0010-check-files-locked</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description>Contains a Check Files Locked action and is executed remotely
by the parent workflow,
+ so it is serialized through the resource export. Resource export clones
every action; if an
+ action's copy constructor does not carry over its name and plugin id, the
exported XML holds an
+ empty name and no type element, and loading it fails on the server (see
issue #5644).
+
+ The file to check is created by this workflow rather than taken from the
project, so that it
+ exists on the file system of whichever host runs the export.</description>
+ <extended_description/>
+ <workflow_version/>
+ <created_user>-</created_user>
+ <created_date>2024/01/01 00:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2024/01/01 00:00:00.000</modified_date>
+ <parameters>
+ </parameters>
+ <actions>
+ <action>
+ <name>Start</name>
+ <description/>
+ <type>SPECIAL</type>
+ <attributes/>
+ <DayOfMonth>1</DayOfMonth>
+ <hour>12</hour>
+ <intervalMinutes>60</intervalMinutes>
+ <intervalSeconds>0</intervalSeconds>
+ <minutes>0</minutes>
+ <repeat>N</repeat>
+ <schedulerType>0</schedulerType>
+ <weekDay>1</weekDay>
+ <parallel>N</parallel>
+ <xloc>50</xloc>
+ <yloc>50</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Create file to check</name>
+ <description/>
+ <type>CREATE_FILE</type>
+ <attributes/>
+ <filename>${java.io.tmpdir}/0010-not-locked.txt</filename>
+ <fail_if_file_exists>N</fail_if_file_exists>
+ <add_filename_result>N</add_filename_result>
+ <parallel>N</parallel>
+ <xloc>250</xloc>
+ <yloc>50</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Check files locked</name>
+ <description/>
+ <type>CHECK_FILES_LOCKED</type>
+ <attributes/>
+ <arg_from_previous>N</arg_from_previous>
+ <fields>
+ <field>
+ <name>${java.io.tmpdir}/0010-not-locked.txt</name>
+ <filemask/>
+ </field>
+ </fields>
+ <include_subfolders>N</include_subfolders>
+ <parallel>N</parallel>
+ <xloc>450</xloc>
+ <yloc>50</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Delete file to check</name>
+ <description/>
+ <type>DELETE_FILE</type>
+ <attributes/>
+ <filename>${java.io.tmpdir}/0010-not-locked.txt</filename>
+ <fail_if_file_not_exists>Y</fail_if_file_not_exists>
+ <parallel>N</parallel>
+ <xloc>650</xloc>
+ <yloc>50</yloc>
+ <attributes_hac/>
+ </action>
+ </actions>
+ <hops>
+ <hop>
+ <from>Start</from>
+ <to>Create file to check</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>Y</unconditional>
+ </hop>
+ <hop>
+ <from>Create file to check</from>
+ <to>Check files locked</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ <hop>
+ <from>Check files locked</from>
+ <to>Delete file to check</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>N</unconditional>
+ </hop>
+ </hops>
+ <notepads>
+ </notepads>
+ <attributes/>
+</workflow>
diff --git
a/integration-tests/hop_server/main-0010-test-check-files-locked-export.hwf
b/integration-tests/hop_server/main-0010-test-check-files-locked-export.hwf
new file mode 100644
index 0000000000..9b0a9956d8
--- /dev/null
+++ b/integration-tests/hop_server/main-0010-test-check-files-locked-export.hwf
@@ -0,0 +1,95 @@
+<?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>main-0010-test-check-files-locked-export</name>
+ <name_sync_with_filename>Y</name_sync_with_filename>
+ <description>Regression test for issue #5644: a workflow containing a Check
Files Locked action must
+ survive the resource export to a remote Hop Server. The inner workflow is
executed remotely with
+ the "remote" run configuration (export_resources=true). Resource export
clones every action, so
+ an action whose copy constructor drops its name is exported with an empty
name; loading that XML
+ then fails with "Cannot invoke String.equalsIgnoreCase(String) because the
return value of
+ ActionMeta.getName() is null" and this workflow fails.</description>
+ <extended_description/>
+ <workflow_version/>
+ <created_user>-</created_user>
+ <created_date>2024/01/01 00:00:00.000</created_date>
+ <modified_user>-</modified_user>
+ <modified_date>2024/01/01 00:00:00.000</modified_date>
+ <parameters>
+ </parameters>
+ <actions>
+ <action>
+ <name>Start</name>
+ <description/>
+ <type>SPECIAL</type>
+ <attributes/>
+ <DayOfMonth>1</DayOfMonth>
+ <hour>12</hour>
+ <intervalMinutes>60</intervalMinutes>
+ <intervalSeconds>0</intervalSeconds>
+ <minutes>0</minutes>
+ <repeat>N</repeat>
+ <schedulerType>0</schedulerType>
+ <weekDay>1</weekDay>
+ <parallel>N</parallel>
+ <xloc>50</xloc>
+ <yloc>50</yloc>
+ <attributes_hac/>
+ </action>
+ <action>
+ <name>Run check files locked on server</name>
+ <description/>
+ <type>WORKFLOW</type>
+ <attributes/>
+ <run_configuration>remote</run_configuration>
+ <filename>${PROJECT_HOME}/0010-check-files-locked.hwf</filename>
+ <params_from_previous>N</params_from_previous>
+ <exec_per_row>N</exec_per_row>
+ <set_logfile>N</set_logfile>
+ <logfile/>
+ <logext/>
+ <add_date>N</add_date>
+ <add_time>N</add_time>
+ <loglevel>Basic</loglevel>
+ <wait_until_finished>Y</wait_until_finished>
+ <create_parent_folder>N</create_parent_folder>
+ <parameters>
+ <pass_all_parameters>Y</pass_all_parameters>
+ </parameters>
+ <set_append_logfile>N</set_append_logfile>
+ <parallel>N</parallel>
+ <xloc>250</xloc>
+ <yloc>50</yloc>
+ <attributes_hac/>
+ </action>
+ </actions>
+ <hops>
+ <hop>
+ <from>Start</from>
+ <to>Run check files locked on server</to>
+ <enabled>Y</enabled>
+ <evaluation>Y</evaluation>
+ <unconditional>Y</unconditional>
+ </hop>
+ </hops>
+ <notepads>
+ </notepads>
+ <attributes/>
+</workflow>
diff --git
a/plugins/actions/checkfilelocked/src/main/java/org/apache/hop/workflow/actions/checkfilelocked/ActionCheckFilesLocked.java
b/plugins/actions/checkfilelocked/src/main/java/org/apache/hop/workflow/actions/checkfilelocked/ActionCheckFilesLocked.java
index 30345bdec3..e4ed8838e9 100644
---
a/plugins/actions/checkfilelocked/src/main/java/org/apache/hop/workflow/actions/checkfilelocked/ActionCheckFilesLocked.java
+++
b/plugins/actions/checkfilelocked/src/main/java/org/apache/hop/workflow/actions/checkfilelocked/ActionCheckFilesLocked.java
@@ -75,11 +75,12 @@ public class ActionCheckFilesLocked extends ActionBase
implements Cloneable, IAc
return new ActionCheckFilesLocked(this);
}
- public ActionCheckFilesLocked(ActionCheckFilesLocked a) {
- this();
- this.argFromPrevious = a.argFromPrevious;
- this.includeSubfolders = a.includeSubfolders;
- for (CheckedFile checkedFile : a.checkedFiles) {
+ public ActionCheckFilesLocked(ActionCheckFilesLocked other) {
+ super(other.getName(), other.getDescription(), other.getPluginId());
+ this.checkedFiles = new ArrayList<>();
+ this.argFromPrevious = other.argFromPrevious;
+ this.includeSubfolders = other.includeSubfolders;
+ for (CheckedFile checkedFile : other.checkedFiles) {
this.checkedFiles.add(new CheckedFile(checkedFile));
}
}
diff --git
a/plugins/actions/checkfilelocked/src/test/java/org/apache/hop/workflow/actions/checkfilelocked/ActionCheckFilesLockedTest.java
b/plugins/actions/checkfilelocked/src/test/java/org/apache/hop/workflow/actions/checkfilelocked/ActionCheckFilesLockedTest.java
index 160e87d24a..de6c545e7a 100644
---
a/plugins/actions/checkfilelocked/src/test/java/org/apache/hop/workflow/actions/checkfilelocked/ActionCheckFilesLockedTest.java
+++
b/plugins/actions/checkfilelocked/src/test/java/org/apache/hop/workflow/actions/checkfilelocked/ActionCheckFilesLockedTest.java
@@ -43,4 +43,27 @@ class ActionCheckFilesLockedTest {
Assertions.assertEquals("/tmp/folder2",
action.getCheckedFiles().get(1).getName());
Assertions.assertEquals(".*\\.zip$",
action.getCheckedFiles().get(1).getWildcard());
}
+
+ /**
+ * Resource export clones every action before serializing it. A clone that
loses its name or
+ * plugin id ends up in the export with an empty name and without its type,
and the exported
+ * workflow then fails to load on a remote server. See issue #5644.
+ */
+ @Test
+ void cloneKeepsNamePluginIdAndFields() {
+ ActionCheckFilesLocked action = new ActionCheckFilesLocked("Check files
locked");
+ action.setDescription("a description");
+ action.setPluginId("CHECK_FILES_LOCKED");
+ action.setIncludeSubfolders(true);
+ action.getCheckedFiles().add(new
ActionCheckFilesLocked.CheckedFile("/tmp/file", ".*\\.txt$"));
+
+ ActionCheckFilesLocked copy = (ActionCheckFilesLocked) action.clone();
+
+ Assertions.assertEquals("Check files locked", copy.getName());
+ Assertions.assertEquals("a description", copy.getDescription());
+ Assertions.assertEquals("CHECK_FILES_LOCKED", copy.getPluginId());
+ Assertions.assertTrue(copy.isIncludeSubfolders());
+ Assertions.assertEquals(1, copy.getCheckedFiles().size());
+ Assertions.assertEquals("/tmp/file",
copy.getCheckedFiles().get(0).getName());
+ }
}
diff --git
a/plugins/actions/ftp/src/main/java/org/apache/hop/workflow/actions/ftpput/ActionFtpPut.java
b/plugins/actions/ftp/src/main/java/org/apache/hop/workflow/actions/ftpput/ActionFtpPut.java
index 79c302d79b..20af2116c0 100644
---
a/plugins/actions/ftp/src/main/java/org/apache/hop/workflow/actions/ftpput/ActionFtpPut.java
+++
b/plugins/actions/ftp/src/main/java/org/apache/hop/workflow/actions/ftpput/ActionFtpPut.java
@@ -163,7 +163,7 @@ public class ActionFtpPut extends ActionBase implements
Cloneable, IAction, IFtp
}
public ActionFtpPut(ActionFtpPut a) {
- super(a.getName(), a.getDescription());
+ super(a.getName(), a.getDescription(), a.getPluginId());
this.serverName = a.serverName;
this.serverPort = a.serverPort;
this.userName = a.userName;
diff --git
a/plugins/actions/ftp/src/test/java/org/apache/hop/workflow/actions/ftpput/ActionFtpPutTest.java
b/plugins/actions/ftp/src/test/java/org/apache/hop/workflow/actions/ftpput/ActionFtpPutTest.java
index cf83a6f527..1a538e7236 100644
---
a/plugins/actions/ftp/src/test/java/org/apache/hop/workflow/actions/ftpput/ActionFtpPutTest.java
+++
b/plugins/actions/ftp/src/test/java/org/apache/hop/workflow/actions/ftpput/ActionFtpPutTest.java
@@ -69,4 +69,25 @@ class ActionFtpPutTest {
assertEquals("proxy-username", action.getSocksProxyUsername());
assertEquals("proxy-password", action.getSocksProxyPassword());
}
+
+ /**
+ * Resource export clones every action. A clone that loses its plugin id is
written to the export
+ * without its type element, and the exported workflow then fails to load on
a remote server. See
+ * issue #5644.
+ */
+ @Test
+ void cloneKeepsNameDescriptionAndPluginId() {
+ ActionFtpPut action = new ActionFtpPut();
+ action.setName("Put files");
+ action.setDescription("a description");
+ action.setPluginId("FTP_PUT");
+ action.setServerName("ftp-server");
+
+ ActionFtpPut copy = (ActionFtpPut) action.clone();
+
+ assertEquals("Put files", copy.getName());
+ assertEquals("a description", copy.getDescription());
+ assertEquals("FTP_PUT", copy.getPluginId());
+ assertEquals("ftp-server", copy.getServerName());
+ }
}
