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 c5414eced3 Run Options variables in pipeline do not appear anymore,
fixes #2481 (#7627)
c5414eced3 is described below
commit c5414eced361e796c4a673391714542514d314e2
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Jul 24 17:43:57 2026 +0200
Run Options variables in pipeline do not appear anymore, fixes #2481
(#7627)
---
.../java/org/apache/hop/core/util/StringUtil.java | 24 +++-
.../org/apache/hop/core/util/StringUtilTest.java | 35 +++++
.../apache/hop/core/reflection/StringSearcher.java | 6 +
.../java/org/apache/hop/pipeline/PipelineMeta.java | 4 +-
.../java/org/apache/hop/workflow/WorkflowMeta.java | 4 +-
.../hop/core/reflection/StringSearcherTest.java | 145 +++++++++++++--------
6 files changed, 159 insertions(+), 59 deletions(-)
diff --git a/core/src/main/java/org/apache/hop/core/util/StringUtil.java
b/core/src/main/java/org/apache/hop/core/util/StringUtil.java
index 3688528bc4..0668ec52c5 100644
--- a/core/src/main/java/org/apache/hop/core/util/StringUtil.java
+++ b/core/src/main/java/org/apache/hop/core/util/StringUtil.java
@@ -369,9 +369,31 @@ public class StringUtil {
public static void getUsedVariables(
String aString, List<String> list, boolean includeSystemVariables) {
+ // Include variable resolvers by default. This is what the encryption code
relies on: a value
+ // like #{vault:hop/data/some-db:password} must be recognized as "contains
variables" so it is
+ // not encrypted (see #7293).
+ getUsedVariables(aString, list, includeSystemVariables, true);
+ }
+
+ /**
+ * Collect the variables used in the given string.
+ *
+ * @param aString the string to scan
+ * @param list the list to add the used variable names to
+ * @param includeSystemVariables whether to include already-set system
variables
+ * @param includeResolvers whether to also collect variable resolver
references (the {@code
+ * #{name:arguments}} syntax). These are not plain, user-settable
variables, so callers that
+ * build a list of variables to present to the user (e.g. the run
options dialog) should pass
+ * {@code false} to avoid polluting the list with resolver names and
secret paths. Real {@code
+ * ${...}} variables nested inside resolver arguments are still
collected by the UNIX scan.
+ */
+ public static void getUsedVariables(
+ String aString, List<String> list, boolean includeSystemVariables,
boolean includeResolvers) {
getUsedVariables(aString, UNIX_OPEN, UNIX_CLOSE, list,
includeSystemVariables);
getUsedVariables(aString, WINDOWS_OPEN, WINDOWS_CLOSE, list,
includeSystemVariables);
- getUsedVariables(aString, RESOLVER_OPEN, RESOLVER_CLOSE, list,
includeSystemVariables);
+ if (includeResolvers) {
+ getUsedVariables(aString, RESOLVER_OPEN, RESOLVER_CLOSE, list,
includeSystemVariables);
+ }
}
public static String generateRandomString(
diff --git a/core/src/test/java/org/apache/hop/core/util/StringUtilTest.java
b/core/src/test/java/org/apache/hop/core/util/StringUtilTest.java
index fa4222d476..41013a8a23 100644
--- a/core/src/test/java/org/apache/hop/core/util/StringUtilTest.java
+++ b/core/src/test/java/org/apache/hop/core/util/StringUtilTest.java
@@ -23,7 +23,9 @@ import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import java.util.ArrayList;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import org.apache.hop.core.exception.HopRuntimeException;
import org.junit.jupiter.api.Test;
@@ -220,4 +222,37 @@ class StringUtilTest {
assertEquals("", StringUtil.trimEnd("/", '/'));
assertEquals("", StringUtil.trimEnd("///", '/'));
}
+
+ /** By default (and for the encryption path) resolvers count as "used
variables". */
+ @Test
+ void testGetUsedVariablesIncludesResolversByDefault() {
+ List<String> list = new ArrayList<>();
+ StringUtil.getUsedVariables("#{vault:hop/data/some-db:password}", list,
true);
+ assertTrue(
+ list.contains("vault:hop/data/some-db:password"),
+ "resolver reference should be collected by default");
+ }
+
+ /**
+ * The run options variable list excludes resolvers: a {@code #{...}}
reference must not show up
+ * as a settable variable, but a real {@code ${...}} variable nested in the
resolver arguments
+ * still must.
+ */
+ @Test
+ void testGetUsedVariablesExcludesResolvers() {
+ List<String> list = new ArrayList<>();
+ StringUtil.getUsedVariables("#{vault:${env}/data/some-db:password}", list,
false, false);
+ assertFalse(
+ list.stream().anyMatch(v -> v.startsWith("vault:")),
+ "resolver reference should not be collected when
includeResolvers=false");
+ assertTrue(list.contains("env"), "nested ${...} variable should still be
collected");
+ }
+
+ @Test
+ void testGetUsedVariablesStillCollectsPlainVariables() {
+ List<String> list = new ArrayList<>();
+ StringUtil.getUsedVariables("${VARIABLE_1} and %%VARIABLE_2%%", list,
false, false);
+ assertTrue(list.contains("VARIABLE_1"));
+ assertTrue(list.contains("VARIABLE_2"));
+ }
}
diff --git
a/engine/src/main/java/org/apache/hop/core/reflection/StringSearcher.java
b/engine/src/main/java/org/apache/hop/core/reflection/StringSearcher.java
index f359229824..2d6c011841 100644
--- a/engine/src/main/java/org/apache/hop/core/reflection/StringSearcher.java
+++ b/engine/src/main/java/org/apache/hop/core/reflection/StringSearcher.java
@@ -176,6 +176,12 @@ public class StringSearcher {
for (int j = 0; j < objectArray.length; j++) {
findMetaData(((Object[]) obj)[j], level + 1, stringList, parentObject,
grandParentObject);
}
+ } else if (obj instanceof Iterable<?> iterable) {
+ for (Object element : iterable) {
+ if (element != null) {
+ stringSearchInObject(element, level, stringList, parentObject,
grandParentObject, field);
+ }
+ }
} else {
findMetaData(obj, level + 1, stringList, parentObject,
grandParentObject);
}
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 b45e0aace9..b53a7c7db2 100644
--- a/engine/src/main/java/org/apache/hop/pipeline/PipelineMeta.java
+++ b/engine/src/main/java/org/apache/hop/pipeline/PipelineMeta.java
@@ -2915,8 +2915,10 @@ public class PipelineMeta extends AbstractMeta
List<String> varList = new ArrayList<>();
// Look around in the strings, see what we find...
+ // Exclude variable resolvers (#{...}): their content is a resolver name
plus a secret path, not
+ // a user-settable variable, so it shouldn't end up in the run options
variable list.
for (StringSearchResult result : stringList) {
- StringUtil.getUsedVariables(result.getString(), varList, false);
+ StringUtil.getUsedVariables(result.getString(), varList, false, false);
}
return varList;
diff --git a/engine/src/main/java/org/apache/hop/workflow/WorkflowMeta.java
b/engine/src/main/java/org/apache/hop/workflow/WorkflowMeta.java
index be0d4c550f..05f2fc36a8 100644
--- a/engine/src/main/java/org/apache/hop/workflow/WorkflowMeta.java
+++ b/engine/src/main/java/org/apache/hop/workflow/WorkflowMeta.java
@@ -1366,8 +1366,10 @@ public class WorkflowMeta extends AbstractMeta
List<String> varList = new ArrayList<>();
// Look around in the strings, see what we find...
+ // Exclude variable resolvers (#{...}): their content is a resolver name
plus a secret path, not
+ // a user-settable variable, so it shouldn't end up in the run options
variable list.
for (StringSearchResult result : stringList) {
- StringUtil.getUsedVariables(result.getString(), varList, false);
+ StringUtil.getUsedVariables(result.getString(), varList, false, false);
}
return varList;
diff --git
a/engine/src/test/java/org/apache/hop/core/reflection/StringSearcherTest.java
b/engine/src/test/java/org/apache/hop/core/reflection/StringSearcherTest.java
index 387e904c62..af3889c3ee 100644
---
a/engine/src/test/java/org/apache/hop/core/reflection/StringSearcherTest.java
+++
b/engine/src/test/java/org/apache/hop/core/reflection/StringSearcherTest.java
@@ -16,68 +16,101 @@
*/
package org.apache.hop.core.reflection;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.ArrayList;
+import java.util.List;
import org.apache.hop.junit.rules.RestoreHopEngineEnvironmentExtension;
+import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
-// import org.apache.hop.pipeline.transforms.filterrows.FilterRowsMeta;
-
@ExtendWith(RestoreHopEngineEnvironmentExtension.class)
public class StringSearcherTest {
- // TODO: Move Test
- /* @Test
- public void testSearchConditionCase() {
- String dummyTransformName = "Output";
- DummyMeta dummyMeta = new DummyMeta();
- String dummyTransformPid = PluginRegistry.getInstance().getPluginId(
TransformPluginType.class, dummyMeta );
- TransformMeta dummyTransform = new TransformMeta( dummyTransformPid,
dummyTransformname, dummyMeta );
-
- List<StringSearchResult> stringList = new ArrayList<StringSearchResult>();
- StringSearcher.findMetaData( dummyTransform, 0, stringList, dummyMeta, 0 );
-
- int checkCount = 0;
- String aResult = null;
- // Check that it found a couple of fields and emits the values properly
- for ( int i = 0; i < stringList.size(); i++ ) {
- aResult = stringList.get( i ).toString();
- if ( aResult.endsWith( "Dummy (transformId)" ) ) {
- checkCount++;
- } else if ( aResult.endsWith( "Output (name)" ) ) {
- checkCount++;
- }
- if ( checkCount == 2 ) {
- break;
- }
+ /** A leaf metadata object holding a variable reference, e.g. a GetVariables
field definition. */
+ public static class Field {
+ private String variableString;
+
+ public Field(String variableString) {
+ this.variableString = variableString;
+ }
+
+ public String getVariableString() {
+ return variableString;
+ }
+ }
+
+ /** Metadata shape used before the @HopMetadataProperty migration: an array
of objects. */
+ public static class ArrayHolder {
+ private Field[] fields;
+
+ public Field[] getFields() {
+ return fields;
}
- assertEquals( 2, checkCount );
-
- FilterRowsMeta filterRowsMeta = new FilterRowsMeta();
- Condition condition = new Condition();
- condition.setNegated( false );
- condition.setLeftValuename( "wibble_t" );
- condition.setRightValuename( "wobble_s" );
- condition.setFunction( org.apache.hop.core.Condition.FUNC_EQUAL );
- filterRowsMeta.setDefault();
- filterRowsMeta.setCondition( condition );
-
- String filterRowsPluginPid = PluginRegistry.getInstance().getPluginId(
TransformPluginType.class, filterRowsMeta );
- TransformMeta filterRowsTransform = new TransformMeta(
filterRowsPluginPid, "Filter Rows", filterRowsMeta );
-
- stringList.clear();
- StringSearcher.findMetaData( filterRowsTransform, 0, stringList,
filterRowsMeta, 0 );
-
- checkCount = 0;
- for ( int i = 0; i < stringList.size(); i++ ) {
- aResult = stringList.get( i ).toString();
- if ( aResult.endsWith( "FilterRows (transformId)" ) ) {
- checkCount++;
- } else if ( aResult.endsWith( "Filter Rows (name)" ) ) {
- checkCount++;
- }
- if ( checkCount == 2 ) {
- break;
- }
+ }
+
+ /** Metadata shape used after the migration: a List of objects (issue
#2481). */
+ public static class ListHolder {
+ private List<Field> fields;
+
+ public List<Field> getFields() {
+ return fields;
}
- assertEquals( 2, checkCount );
- }*/
+ }
+
+ /** A List of plain strings must also have its elements discovered. */
+ public static class StringListHolder {
+ private List<String> values;
+
+ public List<String> getValues() {
+ return values;
+ }
+ }
+
+ private static boolean containsString(List<StringSearchResult> results,
String expected) {
+ return results.stream().anyMatch(r -> expected.equals(r.getString()));
+ }
+
+ @Test
+ public void testFindMetaDataScansObjectArray() {
+ ArrayHolder holder = new ArrayHolder();
+ holder.fields = new Field[] {new Field("${VARIABLE_1}"), new
Field("${VARIABLE_2}")};
+
+ List<StringSearchResult> results = new ArrayList<>();
+ StringSearcher.findMetaData(holder, 1, results, holder, holder);
+
+ assertTrue(containsString(results, "${VARIABLE_1}"), "array element 1 not
found");
+ assertTrue(containsString(results, "${VARIABLE_2}"), "array element 2 not
found");
+ }
+
+ /**
+ * Regression test for <a
href="https://github.com/apache/hop/issues/2481";>#2481</a>: variables
+ * nested in List-based metadata (the @HopMetadataProperty model most
transforms migrated to) were
+ * no longer discovered, so Run Options no longer pre-populated them.
+ */
+ @Test
+ public void testFindMetaDataScansObjectList() {
+ ListHolder holder = new ListHolder();
+ holder.fields = new ArrayList<>();
+ holder.fields.add(new Field("${VARIABLE_1}"));
+ holder.fields.add(new Field("${VARIABLE_2}"));
+
+ List<StringSearchResult> results = new ArrayList<>();
+ StringSearcher.findMetaData(holder, 1, results, holder, holder);
+
+ assertTrue(containsString(results, "${VARIABLE_1}"), "list element 1 not
found");
+ assertTrue(containsString(results, "${VARIABLE_2}"), "list element 2 not
found");
+ }
+
+ @Test
+ public void testFindMetaDataScansStringList() {
+ StringListHolder holder = new StringListHolder();
+ holder.values = new ArrayList<>();
+ holder.values.add("${VARIABLE_1}");
+
+ List<StringSearchResult> results = new ArrayList<>();
+ StringSearcher.findMetaData(holder, 1, results, holder, holder);
+
+ assertTrue(containsString(results, "${VARIABLE_1}"), "string list element
not found");
+ }
}
