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 c8539891af Issue #4010 : Refuse target-stream hops and splits onto
multiple copies (#8685)
c8539891af is described below
commit c8539891af4b96c4489bdf7c78ab69dcdfc42fa7
Author: Matt Casters <[email protected]>
AuthorDate: Thu Oct 1 12:02:22 2026 +0200
Issue #4010 : Refuse target-stream hops and splits onto multiple copies
(#8685)
---
.../java/org/apache/hop/pipeline/PipelineMeta.java | 88 ++++++++
.../pipeline/PipelineMetaMultipleCopiesTest.java | 222 +++++++++++++++++++++
.../hopgui/file/pipeline/HopGuiPipelineGraph.java | 52 ++---
.../delegates/HopGuiPipelineHopDelegate.java | 5 +
.../delegates/HopGuiPipelineTransformDelegate.java | 6 +
5 files changed, 347 insertions(+), 26 deletions(-)
diff --git a/engine/src/main/java/org/apache/hop/pipeline/PipelineMeta.java
b/engine/src/main/java/org/apache/hop/pipeline/PipelineMeta.java
index 151b4fb515..647312f6f2 100644
--- a/engine/src/main/java/org/apache/hop/pipeline/PipelineMeta.java
+++ b/engine/src/main/java/org/apache/hop/pipeline/PipelineMeta.java
@@ -85,6 +85,7 @@ import org.apache.hop.partition.PartitionSchema;
import org.apache.hop.pipeline.analysis.BufferDeadlockRisk;
import org.apache.hop.pipeline.analysis.PipelineBufferDeadlockAnalyzer;
import org.apache.hop.pipeline.transform.BaseTransform;
+import org.apache.hop.pipeline.transform.ITransformIOMeta;
import org.apache.hop.pipeline.transform.ITransformMeta;
import org.apache.hop.pipeline.transform.ITransformMetaChangeListener;
import org.apache.hop.pipeline.transform.TransformErrorMeta;
@@ -781,6 +782,93 @@ public class PipelineMeta extends AbstractMeta
return !isTransformInformative(to, hop.getFromTransform());
}
+ /**
+ * Named target streams (Filter Rows true/false, Switch/Case, and similar)
deliver rows to copy 0
+ * of the target only. A transform that is such a target cannot run in
multiple copies.
+ *
+ * @param transformMeta transform that would be started in multiple copies
+ * @return {@code false} when an enabled previous hop comes from a transform
that names it as a
+ * target stream
+ */
+ public boolean allowsMultipleCopies(TransformMeta transformMeta) {
+ if (transformMeta == null) {
+ return true;
+ }
+ for (TransformMeta previous : findPreviousTransforms(transformMeta)) {
+ if (namesTarget(previous, transformMeta)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * @return {@code true} when the copies string resolves to an integer
greater than one. Unresolved
+ * variables and partitioning are ignored, matching the copies dialog.
+ */
+ public boolean hasMultipleCopies(TransformMeta transformMeta, IVariables
variables) {
+ if (transformMeta == null ||
Utils.isEmpty(transformMeta.getCopiesString())) {
+ return false;
+ }
+ IVariables space = variables != null ? variables :
Variables.getADefaultVariableSpace();
+ return Const.toInt(space.resolve(transformMeta.getCopiesString()), -1) > 1;
+ }
+
+ /**
+ * @return {@code true} when {@code hop} connects a named target stream to a
transform that
+ * already runs in multiple copies
+ */
+ public boolean isMultipleCopiesTargetHop(PipelineHopMeta hop, IVariables
variables) {
+ if (hop == null
+ || !hop.isEnabled()
+ || hop.getFromTransform() == null
+ || hop.getToTransform() == null) {
+ return false;
+ }
+ return hasMultipleCopies(hop.getToTransform(), variables)
+ && namesTarget(hop.getFromTransform(), hop.getToTransform());
+ }
+
+ /**
+ * Splitting {@code hop} redirects the source transform's target streams
from the current
+ * destination onto {@code inserted}.
+ *
+ * @return {@code true} when that redirect would land on a transform that
already runs in multiple
+ * copies
+ */
+ public boolean isMultipleCopiesTargetSplit(
+ PipelineHopMeta hop, TransformMeta inserted, IVariables variables) {
+ if (hop == null || hop.getFromTransform() == null || hop.getToTransform()
== null) {
+ return false;
+ }
+ return hasMultipleCopies(inserted, variables)
+ && namesTarget(hop.getFromTransform(), hop.getToTransform());
+ }
+
+ private boolean namesTarget(TransformMeta source, TransformMeta target) {
+ if (source == null || target == null || Utils.isEmpty(target.getName())) {
+ return false;
+ }
+ ITransformMeta meta = source.getTransform();
+ if (meta == null) {
+ return false;
+ }
+ ITransformIOMeta ioMeta = meta.getTransformIOMeta();
+ if (ioMeta == null) {
+ return false;
+ }
+ String[] targetNames = ioMeta.getTargetTransformNames();
+ if (targetNames == null) {
+ return false;
+ }
+ for (String targetName : targetNames) {
+ if (!Utils.isEmpty(targetName) &&
targetName.equalsIgnoreCase(target.getName())) {
+ return true;
+ }
+ }
+ return false;
+ }
+
/**
* Previous transforms on enabled main hops into {@code transformMeta}: not
info, not error.
* {@link #findPreviousTransforms(TransformMeta, boolean)} with {@code
info=false} still includes
diff --git
a/engine/src/test/java/org/apache/hop/pipeline/PipelineMetaMultipleCopiesTest.java
b/engine/src/test/java/org/apache/hop/pipeline/PipelineMetaMultipleCopiesTest.java
new file mode 100644
index 0000000000..51ce6ef1b6
--- /dev/null
+++
b/engine/src/test/java/org/apache/hop/pipeline/PipelineMetaMultipleCopiesTest.java
@@ -0,0 +1,222 @@
+/*
+ * 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.pipeline;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.hop.core.annotations.Transform;
+import org.apache.hop.core.plugins.PluginRegistry;
+import org.apache.hop.core.plugins.TransformPluginType;
+import org.apache.hop.core.variables.IVariables;
+import org.apache.hop.core.variables.Variables;
+import org.apache.hop.junit.rules.RestoreHopEngineEnvironmentExtension;
+import org.apache.hop.pipeline.transform.TransformIOMeta;
+import org.apache.hop.pipeline.transform.TransformMeta;
+import org.apache.hop.pipeline.transform.stream.IStream.StreamType;
+import org.apache.hop.pipeline.transform.stream.Stream;
+import org.apache.hop.pipeline.transform.stream.StreamIcon;
+import org.apache.hop.pipeline.transform.transforms.FakeMeta;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/** Copies are refused when a transform is the named target of a previous
transform. */
+@ExtendWith(RestoreHopEngineEnvironmentExtension.class)
+class PipelineMetaMultipleCopiesTest {
+
+ private PipelineMeta pipelineMeta;
+ private IVariables variables;
+
+ @BeforeEach
+ void setUp() throws Exception {
+ pipelineMeta = new PipelineMeta();
+ variables = new Variables();
+ PluginRegistry.getInstance()
+ .registerPluginClass(FakeMeta.class.getName(),
TransformPluginType.class, Transform.class);
+ }
+
+ @Test
+ void multipleCopiesAreDisallowedWhenAPreviousTransformTargetsIt() {
+ TransformMeta filter = transform("Filter");
+ TransformMeta selectValues = transform("Select values");
+ selectValues.setCopies(2);
+ nameTargets(filter, selectValues);
+ pipelineMeta.addPipelineHop(new PipelineHopMeta(filter, selectValues));
+
+ assertFalse(pipelineMeta.allowsMultipleCopies(selectValues));
+ assertTrue(pipelineMeta.isMultipleCopiesTargetHop(hop(filter,
selectValues), variables));
+ }
+
+ @Test
+ void multipleCopiesStayAllowedWhenThePreviousTransformDoesNotTargetIt() {
+ TransformMeta filter = transform("Filter");
+ TransformMeta selectValues = transform("Select values");
+ selectValues.setCopies(2);
+ pipelineMeta.addPipelineHop(new PipelineHopMeta(filter, selectValues));
+
+ assertTrue(pipelineMeta.allowsMultipleCopies(selectValues));
+ assertFalse(pipelineMeta.isMultipleCopiesTargetHop(hop(filter,
selectValues), variables));
+ }
+
+ @Test
+ void multipleCopiesStayAllowedWithoutAHopEvenIfTheTargetNameIsSet() {
+ TransformMeta filter = transform("Filter");
+ TransformMeta selectValues = transform("Select values");
+ selectValues.setCopies(2);
+ nameTargets(filter, selectValues);
+
+ assertTrue(pipelineMeta.allowsMultipleCopies(selectValues));
+ assertTrue(pipelineMeta.isMultipleCopiesTargetHop(hop(filter,
selectValues), variables));
+ }
+
+ @Test
+ void secondTargetStreamAlsoDisallowsMultipleCopies() {
+ TransformMeta filter = transform("Filter");
+ TransformMeta trueTarget = transform("True path");
+ TransformMeta falseTarget = transform("False path");
+ falseTarget.setCopies(3);
+ nameTargets(filter, trueTarget, falseTarget);
+ pipelineMeta.addPipelineHop(new PipelineHopMeta(filter, falseTarget));
+
+ assertTrue(pipelineMeta.allowsMultipleCopies(trueTarget));
+ assertFalse(pipelineMeta.allowsMultipleCopies(falseTarget));
+ assertFalse(pipelineMeta.isMultipleCopiesTargetHop(hop(filter,
trueTarget), variables));
+ assertTrue(pipelineMeta.isMultipleCopiesTargetHop(hop(filter,
falseTarget), variables));
+ }
+
+ @Test
+ void targetNameMatchIsCaseInsensitive() {
+ TransformMeta filter = transform("Filter");
+ TransformMeta selectValues = transform("Select values");
+ selectValues.setCopies(2);
+ nameTargets(filter, selectValues);
+
+ TransformMeta sameName = transform("SELECT VALUES");
+ sameName.setCopies(2);
+
+ assertTrue(pipelineMeta.isMultipleCopiesTargetHop(hop(filter, sameName),
variables));
+ }
+
+ @Test
+ void singleCopyTargetHopIsAllowed() {
+ TransformMeta filter = transform("Filter");
+ TransformMeta selectValues = transform("Select values");
+ nameTargets(filter, selectValues);
+
+ assertFalse(pipelineMeta.hasMultipleCopies(selectValues, variables));
+ assertFalse(pipelineMeta.isMultipleCopiesTargetHop(hop(filter,
selectValues), variables));
+ }
+
+ @Test
+ void disabledTargetHopIsNotRefused() {
+ TransformMeta filter = transform("Filter");
+ TransformMeta selectValues = transform("Select values");
+ selectValues.setCopies(2);
+ nameTargets(filter, selectValues);
+ PipelineHopMeta hop = hop(filter, selectValues);
+ hop.setEnabled(false);
+ pipelineMeta.addPipelineHop(hop);
+
+ assertFalse(pipelineMeta.isMultipleCopiesTargetHop(hop, variables));
+ assertTrue(pipelineMeta.allowsMultipleCopies(selectValues));
+ }
+
+ @Test
+ void splittingATargetHopOntoMultipleCopiesIsDisallowed() {
+ TransformMeta filter = transform("Filter");
+ TransformMeta dummy = transform("Dummy");
+ TransformMeta selectValues = transform("Select values");
+ selectValues.setCopies(2);
+ nameTargets(filter, dummy);
+ PipelineHopMeta hop = hop(filter, dummy);
+
+ assertTrue(pipelineMeta.isMultipleCopiesTargetSplit(hop, selectValues,
variables));
+ selectValues.setCopies(1);
+ assertFalse(pipelineMeta.isMultipleCopiesTargetSplit(hop, selectValues,
variables));
+ }
+
+ @Test
+ void splittingAPlainHopOntoMultipleCopiesIsAllowed() {
+ TransformMeta upstream = transform("Data grid");
+ TransformMeta downstream = transform("Dummy");
+ TransformMeta selectValues = transform("Select values");
+ selectValues.setCopies(2);
+
+ assertFalse(
+ pipelineMeta.isMultipleCopiesTargetSplit(
+ hop(upstream, downstream), selectValues, variables));
+ }
+
+ @Test
+ void splittingStillRefusesWhenTheHopIsDisabled() {
+ TransformMeta filter = transform("Filter");
+ TransformMeta dummy = transform("Dummy");
+ TransformMeta selectValues = transform("Select values");
+ selectValues.setCopies(2);
+ nameTargets(filter, dummy);
+ PipelineHopMeta hop = hop(filter, dummy);
+ hop.setEnabled(false);
+
+ assertTrue(pipelineMeta.isMultipleCopiesTargetSplit(hop, selectValues,
variables));
+ }
+
+ @Test
+ void copiesStringResolvesVariablesAndIgnoresUnresolvedOnes() {
+ TransformMeta selectValues = transform("Select values");
+ selectValues.setCopiesString("${COPIES}");
+
+ assertFalse(pipelineMeta.hasMultipleCopies(selectValues, variables));
+
+ variables.setVariable("COPIES", "4");
+ assertTrue(pipelineMeta.hasMultipleCopies(selectValues, variables));
+
+ variables.setVariable("COPIES", "1");
+ assertFalse(pipelineMeta.hasMultipleCopies(selectValues, variables));
+ }
+
+ @Test
+ void nullArgumentsAreSafe() {
+ assertTrue(pipelineMeta.allowsMultipleCopies(null));
+ assertFalse(pipelineMeta.hasMultipleCopies(null, null));
+ assertFalse(pipelineMeta.isMultipleCopiesTargetHop(null, variables));
+ assertFalse(pipelineMeta.isMultipleCopiesTargetSplit(null, null, null));
+ PipelineHopMeta emptyHop = new PipelineHopMeta((TransformMeta) null,
(TransformMeta) null);
+ emptyHop.setEnabled(true);
+ assertFalse(pipelineMeta.isMultipleCopiesTargetHop(emptyHop, variables));
+ assertFalse(pipelineMeta.isMultipleCopiesTargetSplit(emptyHop,
transform("Copies"), null));
+ }
+
+ private TransformMeta transform(String name) {
+ return new TransformMeta(name, new FakeMeta());
+ }
+
+ private static PipelineHopMeta hop(TransformMeta from, TransformMeta to) {
+ return new PipelineHopMeta(from, to);
+ }
+
+ private static void nameTargets(TransformMeta source, TransformMeta...
targets) {
+ TransformIOMeta ioMeta = new TransformIOMeta(true, true, false, false,
false, false);
+ for (TransformMeta target : targets) {
+ ioMeta.addStream(
+ new Stream(
+ StreamType.TARGET, target, "Result is true", StreamIcon.TRUE,
target.getName()));
+ }
+ ((FakeMeta) source.getTransform()).setTransformIOMeta(ioMeta);
+ }
+}
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
index 8df0a09c55..43be6f229d 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/HopGuiPipelineGraph.java
@@ -2591,6 +2591,18 @@ public class HopGuiPipelineGraph extends
HopGuiAbstractGraph
}
private void splitHop(PipelineHopMeta hop) {
+ if (pipelineMeta.isMultipleCopiesTargetSplit(hop, currentTransform,
getVariables())) {
+ if (hop != null) {
+ hop.setSplit(false);
+ }
+ if (lastHopSplit == hop) {
+ lastHopSplit = null;
+ }
+ showMultipleCopiesNotAllowedDialog();
+ splitHop = false;
+ return;
+ }
+
int id = 0;
if (!hopGui.getProps().getAutoSplit()) {
MessageDialogWithToggle md =
@@ -3170,6 +3182,11 @@ public class HopGuiPipelineGraph extends
HopGuiAbstractGraph
pipelineHopDelegate.newHop(pipelineMeta, candidate);
break;
case TARGET:
+ // Named targets receive rows on copy 0 only. Refuse before the target
is recorded.
+ if (pipelineMeta.hasMultipleCopies(candidate.getToTransform(),
getVariables())) {
+ showMultipleCopiesNotAllowedDialog();
+ break;
+ }
// We connect a target of the source transform to an output
transform...
//
stream.setTransformMeta(candidate.getToTransform());
@@ -4052,13 +4069,7 @@ public class HopGuiPipelineGraph extends
HopGuiAbstractGraph
int copies = Const.toInt(hopGui.getVariables().resolve(cop), -1);
if (copies > 1 && !multipleOK) {
cop = "1";
-
- modalMessageDialog(
- BaseMessages.getString(
- PKG,
"PipelineGraph.Dialog.MultipleCopiesAreNotAllowedHere.Title"),
- BaseMessages.getString(
- PKG,
"PipelineGraph.Dialog.MultipleCopiesAreNotAllowedHere.Message"),
- SWT.YES | SWT.ICON_WARNING);
+ showMultipleCopiesNotAllowedDialog();
}
String cps = transformMeta.getCopiesString();
if (cps == null || !cps.equals(cop)) {
@@ -4790,25 +4801,7 @@ public class HopGuiPipelineGraph extends
HopGuiAbstractGraph
}
private boolean checkNumberOfCopies(PipelineMeta pipelineMeta, TransformMeta
transformMeta) {
- boolean enabled = true;
- List<TransformMeta> prevTransforms =
pipelineMeta.findPreviousTransforms(transformMeta);
- for (TransformMeta prevTransform : prevTransforms) {
- // See what the target transforms are.
- // If one of the target transforms is our original transform, we can't
start multiple copies
- //
- String[] targetTransforms =
-
prevTransform.getTransform().getTransformIOMeta().getTargetTransformNames();
- if (targetTransforms != null) {
- for (int t = 0; t < targetTransforms.length && enabled; t++) {
- if (!Utils.isEmpty(targetTransforms[t])
- &&
targetTransforms[t].equalsIgnoreCase(transformMeta.getName())) {
- enabled = false;
- break;
- }
- }
- }
- }
- return enabled;
+ return pipelineMeta.allowsMultipleCopies(transformMeta);
}
/**
@@ -7252,6 +7245,13 @@ public class HopGuiPipelineGraph extends
HopGuiAbstractGraph
messageBox.open();
}
+ public void showMultipleCopiesNotAllowedDialog() {
+ modalMessageDialog(
+ BaseMessages.getString(PKG,
"PipelineGraph.Dialog.MultipleCopiesAreNotAllowedHere.Title"),
+ BaseMessages.getString(PKG,
"PipelineGraph.Dialog.MultipleCopiesAreNotAllowedHere.Message"),
+ SWT.YES | SWT.ICON_WARNING);
+ }
+
/**
* Gets fileType
*
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineHopDelegate.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineHopDelegate.java
index 38c606525a..c234f6bdef 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineHopDelegate.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineHopDelegate.java
@@ -143,6 +143,11 @@ public class HopGuiPipelineHopDelegate {
ok = false;
}
+ if (ok && pipelineMeta.isMultipleCopiesTargetHop(newHop,
pipelineGraph.getVariables())) {
+ pipelineGraph.showMultipleCopiesNotAllowedDialog();
+ ok = false;
+ }
+
if (ok) { // only do the following checks, e.g. checkRowMixingStatically
// when not looping, otherwise we get a loop with
// StackOverflow there ;-)
diff --git
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineTransformDelegate.java
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineTransformDelegate.java
index 8f23b0cc45..e29796e1a9 100644
---
a/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineTransformDelegate.java
+++
b/ui/src/main/java/org/apache/hop/ui/hopgui/file/pipeline/delegates/HopGuiPipelineTransformDelegate.java
@@ -529,6 +529,12 @@ public class HopGuiPipelineTransformDelegate {
*/
public TransformMeta insertTransform(
PipelineMeta pipelineMeta, PipelineHopMeta hop, TransformMeta
transformMeta) {
+ if (pipelineMeta.isMultipleCopiesTargetSplit(
+ hop, transformMeta, pipelineGraph.getVariables())) {
+ pipelineGraph.showMultipleCopiesNotAllowedDialog();
+ return null;
+ }
+
TransformMeta fromTransform = hop.getFromTransform();
TransformMeta toTransform = hop.getToTransform();
