This is an automated email from the ASF dual-hosted git repository.
bamaer 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 7434abb818 fix simple mapping import, fixes #4119 (#8582)
7434abb818 is described below
commit 7434abb8181b91033158c72f99721a7b7a9e0adc
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Sep 25 11:24:39 2026 +0200
fix simple mapping import, fixes #4119 (#8582)
* fix simple mapping import, fixes #4119
* fix simple mapping parameters import from kettle, fixes #4501
---
.../org/apache/hop/imports/kettle/KettleConst.java | 4 +-
.../apache/hop/imports/kettle/KettleImport.java | 104 ++++++---
.../imports/kettle/KettleImportMappingTest.java | 255 +++++++++++++++++++++
3 files changed, 333 insertions(+), 30 deletions(-)
diff --git
a/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleConst.java
b/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleConst.java
index 03f5a93d16..234ca9357e 100644
---
a/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleConst.java
+++
b/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleConst.java
@@ -45,7 +45,6 @@ public class KettleConst {
{"step_performance_capturing_delay",
"transform_performance_capturing_delay"},
{"transformationPath", "pipelinePath"},
{"SUB_STEP", "subTransform"},
- {"variablemapping", "variable_mapping"},
// jobs
{"job", "workflow"},
{"job_version", "workflow_version"},
@@ -249,6 +248,9 @@ public class KettleConst {
public static final List<String> transTypes = Arrays.asList(CONST_TRANS);
+ /** Kettle's "Mapping" and "Simple Mapping" steps both become a Hop Simple
Mapping transform. */
+ public static final List<String> mappingTypes = Arrays.asList("Mapping",
"SimpleMapping");
+
public KettleConst() {
// Do nothing
}
diff --git
a/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleImport.java
b/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleImport.java
index 142848bc91..09e71a80dc 100644
---
a/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleImport.java
+++
b/plugins/misc/import/src/main/java/org/apache/hop/imports/kettle/KettleImport.java
@@ -84,6 +84,7 @@ public class KettleImport extends HopImportBase implements
IHopImport {
public static final String CONST_SERVERNAME = "servername";
public static final String CONST_PASSWORD = "password";
private static final String SFTP_PUT_TYPE = "SFTPPut";
+ private static final String TRANS_EXECUTOR_TYPE = "TransExecutor";
private static final String SFTP_CONNECTION_METADATA_KEY = "sftp-connection";
/** The elements of a Kettle SFTPPut step which describe the server, not the
upload itself. */
@@ -628,6 +629,22 @@ public class KettleImport extends HopImportBase implements
IHopImport {
setChildElement(doc, stepNode, "connection", connectionName);
}
+ /**
+ * Kettle writes the parameters of a mapping, a job executor and a
transformation executor step as
+ * {@code <parameters><variablemapping>}. Hop reads them the same way,
except for the pipeline
+ * executor, which reads {@code <variable_mapping>}.
+ */
+ private void migrateTransExecutorParameters(Document doc, Node stepNode) {
+ Element parametersElement = getChildElement(stepNode, "parameters");
+ if (parametersElement == null) {
+ return;
+ }
+ Element variableMapping;
+ while ((variableMapping = getChildElement(parametersElement,
"variablemapping")) != null) {
+ renameNode(doc, variableMapping, "variable_mapping");
+ }
+ }
+
/**
* Create an SFTP connection in the metadata for the given step settings, or
return the name of
* the one created earlier for the very same settings: a transformation with
five steps talking to
@@ -838,6 +855,7 @@ public class KettleImport extends HopImportBase implements
IHopImport {
if (currentNode.getNodeName().equals("step")) {
entryType = EntryType.OTHER;
boolean sftpPutStep = false;
+ boolean transExecutorStep = false;
NodeList currentNodeChildNodes = currentNode.getChildNodes();
for (int i1 = 0; i1 < currentNodeChildNodes.getLength(); i1++) {
Node childNode = currentNodeChildNodes.item(i1);
@@ -846,6 +864,10 @@ public class KettleImport extends HopImportBase implements
IHopImport {
&&
childNode.getChildNodes().item(0).getNodeValue().equals(SFTP_PUT_TYPE)) {
sftpPutStep = true;
}
+ if (childNode.getNodeName().equals("type")
+ &&
childNode.getChildNodes().item(0).getNodeValue().equals(TRANS_EXECUTOR_TYPE)) {
+ transExecutorStep = true;
+ }
if (childNode.getNodeName().equals("type")
&&
childNode.getChildNodes().item(0).getNodeValue().equals("Formula")) {
entryType = EntryType.FORMULA;
@@ -859,7 +881,8 @@ public class KettleImport extends HopImportBase implements
IHopImport {
entryType = EntryType.GOOGLE_SHEETS_INPUT;
}
if (childNode.getNodeName().equals("type")
- &&
childNode.getChildNodes().item(0).getNodeValue().equals("Mapping")) {
+ && KettleConst.mappingTypes.contains(
+ childNode.getChildNodes().item(0).getNodeValue())) {
entryType = EntryType.SIMPLE_MAPPING;
}
if (childNode.getNodeName().equals("type")
@@ -871,6 +894,9 @@ public class KettleImport extends HopImportBase implements
IHopImport {
if (sftpPutStep) {
migrateSftpPutStep(doc, currentNode);
}
+ if (transExecutorStep) {
+ migrateTransExecutorParameters(doc, currentNode);
+ }
}
// remove superfluous elements
@@ -981,36 +1007,16 @@ public class KettleImport extends HopImportBase
implements IHopImport {
if ((entryType == EntryType.SIMPLE_MAPPING || entryType ==
EntryType.METAINJECT)
&& currentNode.getNodeName().equals("transform")) {
- Node filenameNode = null;
- String transName = "";
- String directoryPath = "";
- // get trans name, file name, path, set correct filename when needed.
- for (int j = 0; j < currentNode.getChildNodes().getLength(); j++) {
- if
(currentNode.getChildNodes().item(j).getNodeName().equals("directory_path")) {
- directoryPath =
currentNode.getChildNodes().item(j).getTextContent();
- currentNode.removeChild(currentNode.getChildNodes().item(j));
- }
- if
(currentNode.getChildNodes().item(j).getNodeName().equals("trans_name")) {
- transName = currentNode.getChildNodes().item(j).getTextContent();
- currentNode.removeChild(currentNode.getChildNodes().item(j));
- }
- if
(currentNode.getChildNodes().item(j).getNodeName().equals("filename")) {
- filenameNode = currentNode.getChildNodes().item(j);
- }
- }
-
- // if we have a trans name and directory path, use it to update the
mapping or injectable
- // pipeline
- // filename.
- if (!StringUtils.isEmpty(transName) &&
!StringUtils.isEmpty(directoryPath)) {
- filenameNode.setTextContent(
- Const.VAR_PROJECT_HOME + directoryPath + '/' + transName +
".hpl");
- }
+ migrateTransformationReference(doc, currentNode);
// add the default pipeline run configuration.
- Element runConfigElement = doc.createElement("runConfiguration");
-
runConfigElement.appendChild(doc.createTextNode(defaultPipelineRunConfiguration));
- currentNode.appendChild(runConfigElement);
+ String runConfigElementName =
+ entryType == EntryType.METAINJECT ? "run_configuration" :
"runConfiguration";
+ if (getChildElement(currentNode, runConfigElementName) == null) {
+ Element runConfigElement = doc.createElement(runConfigElementName);
+
runConfigElement.appendChild(doc.createTextNode(defaultPipelineRunConfiguration));
+ currentNode.appendChild(runConfigElement);
+ }
}
if (entryType == EntryType.GOOGLE_SHEETS_INPUT
@@ -1091,6 +1097,46 @@ public class KettleImport extends HopImportBase
implements IHopImport {
}
}
+ /**
+ * A Kettle mapping or metadata injection step refers to its
sub-transformation either by filename
+ * or, when it was saved in a repository, by name and repository folder. Hop
only knows filenames,
+ * so a repository reference becomes a path below the project home. The
repository elements are
+ * removed either way.
+ */
+ private void migrateTransformationReference(Document doc, Node
transformNode) {
+ String specificationMethod = getChildText(transformNode,
"specification_method");
+ String transName = getChildText(transformNode, "trans_name");
+ String directoryPath = getChildText(transformNode, "directory_path");
+ for (String name :
+ new String[] {"specification_method", "trans_object_id", "trans_name",
"directory_path"}) {
+ removeChildElement(transformNode, name);
+ }
+
+ Element filenameElement = getChildElement(transformNode, "filename");
+ String filename = filenameElement == null ? "" :
filenameElement.getTextContent();
+
+ // Kettle keeps the repository name when you switch a step to a filename,
so a filename wins
+ // unless the step explicitly refers to the repository.
+ if (StringUtils.isEmpty(transName)
+ || (StringUtils.isNotEmpty(filename)
+ && !StringUtils.startsWith(specificationMethod, "REPOSITORY"))) {
+ return;
+ }
+
+ String folder = directoryPath == null ? "" :
StringUtils.strip(directoryPath, "/");
+ if (filenameElement == null) {
+ filenameElement = doc.createElement("filename");
+ transformNode.appendChild(filenameElement);
+ }
+ filenameElement.setTextContent(
+ Const.VAR_PROJECT_HOME + (folder.isEmpty() ? "" : "/" + folder) + "/"
+ transName + ".hpl");
+ }
+
+ private String getChildText(Node parent, String name) {
+ Element child = getChildElement(parent, name);
+ return child == null ? null : child.getTextContent();
+ }
+
private Node processRepositoryNode(Node repositoryNode) {
String filename = "";
diff --git
a/plugins/misc/import/src/test/java/org/apache/hop/imports/kettle/KettleImportMappingTest.java
b/plugins/misc/import/src/test/java/org/apache/hop/imports/kettle/KettleImportMappingTest.java
new file mode 100644
index 0000000000..dc5e6b8b90
--- /dev/null
+++
b/plugins/misc/import/src/test/java/org/apache/hop/imports/kettle/KettleImportMappingTest.java
@@ -0,0 +1,255 @@
+/*
+ * 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.imports.kettle;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import java.io.ByteArrayInputStream;
+import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
+import org.apache.hop.core.HopClientEnvironment;
+import org.apache.hop.core.xml.XmlHandler;
+import org.apache.hop.core.xml.XmlParserFactoryProducer;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.w3c.dom.Document;
+import org.w3c.dom.Node;
+
+/**
+ * Kettle mapping and metadata injection steps saved in a repository refer to
their
+ * sub-transformation by name and folder instead of by filename (issue #4119).
Their parameters must
+ * survive the import (issue #4501).
+ */
+class KettleImportMappingTest {
+
+ private static final String ENTRY_TYPE =
"org.apache.hop.imports.kettle.KettleImport$EntryType";
+
+ private KettleImport kettleImport;
+
+ @BeforeAll
+ static void setUpBeforeClass() throws Exception {
+ HopClientEnvironment.init();
+ }
+
+ @BeforeEach
+ void setUp() {
+ kettleImport = new KettleImport();
+ kettleImport.setDefaultPipelineRunConfiguration("local");
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"Mapping", "SimpleMapping"})
+ void testRepositoryReferenceBecomesFilename(String type) throws Exception {
+ Node transform =
+ importStep(type, "REPOSITORY_BY_NAME", "child", "", "/public/sub",
"runConfiguration");
+
+ assertEquals("SimpleMapping", XmlHandler.getTagValue(transform, "type"));
+ assertEquals(
+ "${PROJECT_HOME}/public/sub/child.hpl",
XmlHandler.getTagValue(transform, "filename"));
+ assertEquals("local", XmlHandler.getTagValue(transform,
"runConfiguration"));
+ assertRepositoryElementsRemoved(transform);
+ }
+
+ @Test
+ void testMetaInjectGetsItsOwnRunConfigurationElement() throws Exception {
+ Node transform =
+ importStep("MetaInject", "REPOSITORY_BY_NAME", "child", "",
"/public/sub", null);
+
+ assertEquals(
+ "${PROJECT_HOME}/public/sub/child.hpl",
XmlHandler.getTagValue(transform, "filename"));
+ assertEquals("local", XmlHandler.getTagValue(transform,
"run_configuration"));
+ assertNull(XmlHandler.getSubNode(transform, "runConfiguration"));
+ assertRepositoryElementsRemoved(transform);
+ }
+
+ @Test
+ void testRootFolderHasNoDoubleSlash() throws Exception {
+ Node transform = importStep("SimpleMapping", "REPOSITORY_BY_NAME",
"child", "", "/", null);
+
+ assertEquals("${PROJECT_HOME}/child.hpl",
XmlHandler.getTagValue(transform, "filename"));
+ }
+
+ /** Kettle keeps the old repository name after the step is switched to a
filename. */
+ @Test
+ void testFilenameWinsOverStaleRepositoryName() throws Exception {
+ Node transform =
+ importStep(
+ "SimpleMapping",
+ "FILENAME",
+ "old-child",
+ "${Internal.Entry.Current.Directory}/child.ktr",
+ "/public/old",
+ null);
+
+ assertEquals(
+ "${Internal.Entry.Current.Folder}/child.hpl",
+ XmlHandler.getTagValue(transform, "filename"));
+ assertRepositoryElementsRemoved(transform);
+ }
+
+ @Test
+ void testMissingFilenameElementIsAdded() throws Exception {
+ Document doc =
+ parse(
+ "<transformation><step><name>map</name><type>SimpleMapping</type>"
+ +
"<trans_name>child</trans_name><directory_path>/sub</directory_path>"
+ + "</step></transformation>");
+ processNode(doc);
+
+ Node transform = XmlHandler.getSubNode(XmlHandler.getSubNode(doc,
"pipeline"), "transform");
+ assertEquals("${PROJECT_HOME}/sub/child.hpl",
XmlHandler.getTagValue(transform, "filename"));
+ }
+
+ /** The parameters of a mapping step keep their Kettle layout in Hop (issue
#4501). */
+ @ParameterizedTest
+ @ValueSource(strings = {"Mapping", "SimpleMapping"})
+ void testMappingParametersAreKept(String type) throws Exception {
+ Document doc =
+ parse(
+ "<transformation><step><name>map</name><type>"
+ + type
+ + "</type><filename>child.ktr</filename><mappings>"
+ + PARAMETERS
+ + "</mappings></step></transformation>");
+ processNode(doc);
+
+ Node transform = XmlHandler.getSubNode(XmlHandler.getSubNode(doc,
"pipeline"), "transform");
+ assertParameters(XmlHandler.getSubNode(transform, "mappings",
"parameters"), "variablemapping");
+ }
+
+ /** Only the pipeline executor reads its parameters from a differently named
element. */
+ @ParameterizedTest
+ @CsvSource({
+ "TransExecutor, PipelineExecutor, variable_mapping",
+ "JobExecutor, WorkflowExecutor, variablemapping"
+ })
+ void testExecutorParameters(String kettleType, String hopType, String
variableTag)
+ throws Exception {
+ Document doc =
+ parse(
+ "<transformation><step><name>exec</name><type>"
+ + kettleType
+ + "</type>"
+ + PARAMETERS
+ + "</step></transformation>");
+ processNode(doc);
+
+ Node transform = XmlHandler.getSubNode(XmlHandler.getSubNode(doc,
"pipeline"), "transform");
+ assertEquals(hopType, XmlHandler.getTagValue(transform, "type"));
+ assertParameters(XmlHandler.getSubNode(transform, "parameters"),
variableTag);
+ }
+
+ private static final String PARAMETERS =
+ "<parameters>"
+ +
"<variablemapping><variable>A</variable><input>${PARENT_A}</input></variablemapping>"
+ +
"<variablemapping><variable>B</variable><input>${PARENT_B}</input></variablemapping>"
+ + "<inherit_all_vars>N</inherit_all_vars>"
+ + "</parameters>";
+
+ private static void assertParameters(Node parameters, String variableTag) {
+ assertEquals(2, XmlHandler.countNodes(parameters, variableTag));
+ assertEquals(
+ 0,
+ XmlHandler.countNodes(
+ parameters,
+ "variable_mapping".equals(variableTag) ? "variablemapping" :
"variable_mapping"));
+ assertEquals(
+ "A",
+ XmlHandler.getTagValue(XmlHandler.getSubNodeByNr(parameters,
variableTag, 0), "variable"));
+ assertEquals(
+ "${PARENT_B}",
+ XmlHandler.getTagValue(XmlHandler.getSubNodeByNr(parameters,
variableTag, 1), "input"));
+ assertEquals("N", XmlHandler.getTagValue(parameters, "inherit_all_vars"));
+ }
+
+ private static void assertRepositoryElementsRemoved(Node transform) {
+ for (String tag :
+ new String[] {"specification_method", "trans_object_id", "trans_name",
"directory_path"}) {
+ assertNull(XmlHandler.getSubNode(transform, tag), tag + " should have
been removed");
+ }
+ }
+
+ /** A step laid out the way Kettle writes it, whitespace included. */
+ private Node importStep(
+ String type,
+ String specificationMethod,
+ String transName,
+ String filename,
+ String directoryPath,
+ String existingRunConfigurationTag)
+ throws Exception {
+ Document doc =
+ parse(
+ "<transformation>\n"
+ + " <step>\n"
+ + " <name>map</name>\n"
+ + " <type>"
+ + type
+ + "</type>\n"
+ + " <specification_method>"
+ + specificationMethod
+ + "</specification_method>\n"
+ + " <trans_object_id/>\n"
+ + " <trans_name>"
+ + transName
+ + "</trans_name>\n"
+ + " <filename>"
+ + filename
+ + "</filename>\n"
+ + " <directory_path>"
+ + directoryPath
+ + "</directory_path>\n"
+ + " <mappings>\n"
+ + " <input><mapping><input_step/></mapping></input>\n"
+ + " </mappings>\n"
+ + " </step>\n"
+ + "</transformation>");
+ processNode(doc);
+ Node transform = XmlHandler.getSubNode(XmlHandler.getSubNode(doc,
"pipeline"), "transform");
+ if (existingRunConfigurationTag != null) {
+ // The run configuration is only added once.
+ assertEquals(1, XmlHandler.countNodes(transform,
existingRunConfigurationTag));
+ }
+ return transform;
+ }
+
+ private void processNode(Document doc) throws Exception {
+ Class<?> entryTypeClass = Class.forName(ENTRY_TYPE);
+ Object other = null;
+ for (Object constant : entryTypeClass.getEnumConstants()) {
+ if ("OTHER".equals(constant.toString())) {
+ other = constant;
+ }
+ }
+ Method method =
+ KettleImport.class.getDeclaredMethod(
+ "processNode", Document.class, Node.class, entryTypeClass,
int.class);
+ method.setAccessible(true);
+ method.invoke(kettleImport, doc, doc, other, 0);
+ }
+
+ private static Document parse(String xml) throws Exception {
+ return XmlParserFactoryProducer.createSecureDocBuilderFactory()
+ .newDocumentBuilder()
+ .parse(new ByteArrayInputStream(xml.getBytes(StandardCharsets.UTF_8)));
+ }
+}
