Run Options variables in pipeline do not appear anymore, fixes #2481

core/src/main/java/org/apache/hop/core/util/StringUtil.java

@@ -369,9 +369,31 @@ public static boolean isSystemVariable(String aString) {
 
   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(

core/src/test/java/org/apache/hop/core/util/StringUtilTest.java

@@ -23,7 +23,9 @@
 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 @@ void testTrimEnd_None() {
     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"));
+  }
 }

engine/src/main/java/org/apache/hop/core/reflection/StringSearcher.java

@@ -176,6 +176,12 @@ private static void stringSearchInObject(
       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);
     }

engine/src/main/java/org/apache/hop/pipeline/PipelineMeta.java

@@ -2915,8 +2915,10 @@ public List<String> getUsedVariables() {
     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;

engine/src/main/java/org/apache/hop/workflow/WorkflowMeta.java

@@ -1366,8 +1366,10 @@ public List<String> getUsedVariables() {
     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;

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");
+  }
 }