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 3e53c72232 fixes #7743: stop overwriting workflow parentId with Spark 
variable name (#7746)
3e53c72232 is described below

commit 3e53c7223266ca3f247c5c37956f6c7a2a396f2d
Author: Matt Casters <[email protected]>
AuthorDate: Sun Aug 2 13:31:51 2026 +0200

    fixes #7743: stop overwriting workflow parentId with Spark variable name 
(#7746)
    
    LocalWorkflowEngine rebind used resolve("Internal.Spark.TransformOwnerId"),
    which returns the bare name unchanged when no ${} tokens are present, so 
every
    local workflow got that literal as PARENT_ID. Use getVariable() so rebind 
only
    applies when Spark actually set the owner id.
---
 .../engines/local/LocalWorkflowEngine.java         |  32 +++++-
 .../LocalWorkflowEngineSparkParentRebindTest.java  | 120 +++++++++++++++++++++
 2 files changed, 148 insertions(+), 4 deletions(-)

diff --git 
a/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java
 
b/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java
index 5d673aa4d7..916ebd609b 100644
--- 
a/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java
+++ 
b/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java
@@ -61,6 +61,12 @@ import org.apache.hop.workflow.engine.WorkflowEnginePlugin;
     description = "Executes your workflow locally")
 public class LocalWorkflowEngine extends Workflow implements 
IWorkflowEngine<WorkflowMeta> {
 
+  /**
+   * Must stay in sync with {@code 
org.apache.hop.spark.util.SparkConst#VAR_TRANSFORM_OWNER_ID}.
+   * Engine cannot depend on the spark plugin.
+   */
+  static final String VAR_SPARK_TRANSFORM_OWNER_ID = 
"Internal.Spark.TransformOwnerId";
+
   private ExecutionInfoLocation executionInfoLocation;
   private Timer executionInfoTimer;
 
@@ -325,27 +331,45 @@ public class LocalWorkflowEngine extends Workflow 
implements IWorkflowEngine<Wor
    * When this workflow is nested under a Native Spark mapPartitions transform 
(Workflow Executor),
    * the parent transform is registered under a synthetic id {@code 
pipelineId|name|copy}. Rebind so
    * the execution perspective can drill down from that transform node.
+   *
+   * <p>Uses {@link IVariables#getVariable(String)} (not {@link 
IVariables#resolve(String)}):
+   * resolve only substitutes {@code ${...}} tokens and returns a bare name 
unchanged, which would
+   * always overwrite parentId with the literal variable name (issue #7743).
    */
-  private void rebindSparkTransformOwnerParent(Execution execution) {
+  /** Package-private for unit tests. */
+  void rebindSparkTransformOwnerParent(Execution execution) {
     if (execution == null) {
       return;
     }
-    String sparkOwner = resolve("Internal.Spark.TransformOwnerId");
+    String sparkOwner = sparkTransformOwnerId(this);
     if (StringUtils.isNotEmpty(sparkOwner)) {
       execution.setParentId(sparkOwner);
     }
   }
 
-  private void rebindSparkTransformOwnerParent(ExecutionState state) {
+  /** Package-private for unit tests. */
+  void rebindSparkTransformOwnerParent(ExecutionState state) {
     if (state == null) {
       return;
     }
-    String sparkOwner = resolve("Internal.Spark.TransformOwnerId");
+    String sparkOwner = sparkTransformOwnerId(this);
     if (StringUtils.isNotEmpty(sparkOwner)) {
       state.setParentId(sparkOwner);
     }
   }
 
+  /**
+   * Returns the Spark transform owner id when set on the variable space; 
otherwise null.
+   *
+   * <p>Package-private for unit tests.
+   */
+  static String sparkTransformOwnerId(IVariables variables) {
+    if (variables == null) {
+      return null;
+    }
+    return variables.getVariable(VAR_SPARK_TRANSFORM_OWNER_ID);
+  }
+
   public void startExecutionInfoTimer() {
     if (executionInfoLocation == null) {
       return;
diff --git 
a/engine/src/test/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngineSparkParentRebindTest.java
 
b/engine/src/test/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngineSparkParentRebindTest.java
new file mode 100644
index 0000000000..d25001e12e
--- /dev/null
+++ 
b/engine/src/test/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngineSparkParentRebindTest.java
@@ -0,0 +1,120 @@
+/*
+ * 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.engines.local;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.execution.Execution;
+import org.apache.hop.execution.ExecutionState;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression for issue #7743: parentId must not be overwritten with the 
literal variable name
+ * {@code Internal.Spark.TransformOwnerId} when the Spark owner variable is 
unset.
+ */
+class LocalWorkflowEngineSparkParentRebindTest {
+
+  @Test
+  void sparkTransformOwnerId_nullWhenUnset() {
+    Variables vars = new Variables();
+    vars.initializeFrom(null);
+    assertNull(LocalWorkflowEngine.sparkTransformOwnerId(vars));
+  }
+
+  @Test
+  void sparkTransformOwnerId_returnsValueWhenSet() {
+    Variables vars = new Variables();
+    vars.initializeFrom(null);
+    vars.setVariable(LocalWorkflowEngine.VAR_SPARK_TRANSFORM_OWNER_ID, 
"pipe|Workflow Executor|0");
+    assertEquals("pipe|Workflow Executor|0", 
LocalWorkflowEngine.sparkTransformOwnerId(vars));
+  }
+
+  @Test
+  void sparkTransformOwnerId_nullVariables() {
+    assertNull(LocalWorkflowEngine.sparkTransformOwnerId(null));
+  }
+
+  @Test
+  void rebind_doesNotOverwriteParentWhenSparkVariableUnset() {
+    LocalWorkflowEngine engine = new LocalWorkflowEngine();
+    engine.initializeFrom(null);
+
+    Execution execution = new Execution();
+    execution.setParentId("parent-workflow-uuid");
+    engine.rebindSparkTransformOwnerParent(execution);
+    assertEquals("parent-workflow-uuid", execution.getParentId());
+
+    Execution root = new Execution();
+    root.setParentId(null);
+    engine.rebindSparkTransformOwnerParent(root);
+    assertNull(root.getParentId());
+  }
+
+  @Test
+  void rebind_setsSparkOwnerWhenVariablePresent() {
+    LocalWorkflowEngine engine = new LocalWorkflowEngine();
+    engine.initializeFrom(null);
+    engine.setVariable(
+        LocalWorkflowEngine.VAR_SPARK_TRANSFORM_OWNER_ID, "spark-pipe|Workflow 
Executor|0");
+
+    Execution execution = new Execution();
+    execution.setParentId("would-be-overwritten");
+    engine.rebindSparkTransformOwnerParent(execution);
+    assertEquals("spark-pipe|Workflow Executor|0", execution.getParentId());
+
+    ExecutionState state = new ExecutionState();
+    state.setParentId("state-parent");
+    engine.rebindSparkTransformOwnerParent(state);
+    assertEquals("spark-pipe|Workflow Executor|0", state.getParentId());
+  }
+
+  @Test
+  void rebind_ignoresEmptySparkOwner() {
+    LocalWorkflowEngine engine = new LocalWorkflowEngine();
+    engine.initializeFrom(null);
+    engine.setVariable(LocalWorkflowEngine.VAR_SPARK_TRANSFORM_OWNER_ID, "");
+
+    Execution execution = new Execution();
+    execution.setParentId("keep-me");
+    engine.rebindSparkTransformOwnerParent(execution);
+    assertEquals("keep-me", execution.getParentId());
+  }
+
+  /**
+   * Documents the bug: {@code resolve(bareName)} returns the name unchanged 
and must not be used
+   * for variable lookup.
+   */
+  @Test
+  void resolveBareName_returnsLiteral_notVariableValue() {
+    Variables vars = new Variables();
+    vars.initializeFrom(null);
+    // Unset: resolve leaves the bare token as-is (no ${} markers)
+    assertEquals(
+        LocalWorkflowEngine.VAR_SPARK_TRANSFORM_OWNER_ID,
+        vars.resolve(LocalWorkflowEngine.VAR_SPARK_TRANSFORM_OWNER_ID));
+    // Set: resolve still returns the bare name (it is not a ${var} expression)
+    vars.setVariable(LocalWorkflowEngine.VAR_SPARK_TRANSFORM_OWNER_ID, 
"actual-owner");
+    assertEquals(
+        LocalWorkflowEngine.VAR_SPARK_TRANSFORM_OWNER_ID,
+        vars.resolve(LocalWorkflowEngine.VAR_SPARK_TRANSFORM_OWNER_ID));
+    assertEquals(
+        "actual-owner", 
vars.getVariable(LocalWorkflowEngine.VAR_SPARK_TRANSFORM_OWNER_ID));
+  }
+}

Reply via email to