Make step param separator configurable to allow __ in step IDs and param names

maestro-common/src/main/java/com/netflix/maestro/models/Constants.java

@@ -213,6 +213,12 @@ public static WorkflowVersion of(String version) {
   /** Metadata key for internal parameter mode. */
   public static final String METADATA_INTERNAL_PARAM_MODE = "internal_mode";
 
+  /**
+   * Default step parameter separator used in cross-step parameter references (e.g. {@code
+   * step1__param}). Configurable via {@code maestro.param-evaluator.step-param-separator}.
+   */
+  public static final String DEFAULT_STEP_PARAM_SEPARATOR = "__";
+
   /** Step ID Param Key. */
   public static final String STEP_ID_PARAM = "step_id";
 

maestro-common/src/main/java/com/netflix/maestro/utils/StepParamSeparator.java

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2024 Netflix, Inc.
+ *
+ * Licensed 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 com.netflix.maestro.utils;
+
+/** Interface for providing the configured step parameter separator string. */
+@FunctionalInterface
+public interface StepParamSeparator {
+
+  /** Returns the separator used in cross-step parameter references (e.g. {@code step1__param}). */
+  String getStepParamSeparator();
+}

maestro-common/src/main/java/com/netflix/maestro/validations/MaestroReferenceIdConstraint.java

@@ -12,7 +12,10 @@
  */
 package com.netflix.maestro.validations;
 
+import com.netflix.maestro.annotations.VisibleForTesting;
 import com.netflix.maestro.models.Constants;
+import com.netflix.maestro.utils.StepParamSeparator;
+import jakarta.inject.Inject;
 import jakarta.validation.Constraint;
 import jakarta.validation.ConstraintValidator;
 import jakarta.validation.ConstraintValidatorContext;
@@ -45,9 +48,22 @@ class MaestroIdValidator implements ConstraintValidator<MaestroReferenceIdConstr
     private static final Pattern ID_PATTERN = Pattern.compile("[_a-zA-Z][.\\-_a-zA-Z0-9]*+");
     private static final String REJECTED_VALUE = "- rejected value is [%s]";
 
+    @Inject private StepParamSeparator stepParamSeparator;
+
+    @VisibleForTesting
+    void setStepParamSeparator(StepParamSeparator stepParamSeparator) {
+      this.stepParamSeparator = stepParamSeparator;
+    }
+
     @Override
     public void initialize(MaestroReferenceIdConstraint constraint) {}
 
+    private String getSeparator() {
+      return stepParamSeparator != null
+          ? stepParamSeparator.getStepParamSeparator()
+          : Constants.DEFAULT_STEP_PARAM_SEPARATOR;
+    }
+
     @Override
     public boolean isValid(String id, ConstraintValidatorContext context) {
       if (id == null || id.isEmpty()) {
@@ -81,12 +97,13 @@ public boolean isValid(String id, ConstraintValidatorContext context) {
         return false;
       }
 
-      if (id.contains("__")) {
+      if (id.contains(getSeparator())) {
         context
             .buildConstraintViolationWithTemplate(
                 String.format(
-                    "[maestro id or name reference] cannot contain double underscores '__' "
+                    "[maestro id or name reference] cannot contain the step param separator '%s' "
                         + REJECTED_VALUE,
+                    getSeparator(),
                     id))
             .addConstraintViolation();
         return false;

maestro-common/src/test/java/com/netflix/maestro/validations/MaestroReferenceIdConstraintTest.java

@@ -16,9 +16,14 @@
 import static org.junit.Assert.assertNull;
 
 import com.netflix.maestro.models.Constants;
+import jakarta.validation.ConstraintValidator;
+import jakarta.validation.ConstraintValidatorFactory;
 import jakarta.validation.ConstraintViolation;
+import jakarta.validation.Validation;
+import jakarta.validation.Validator;
 import java.util.LinkedHashSet;
 import java.util.Set;
+import org.apache.bval.jsr.ApacheValidationProvider;
 import org.junit.Test;
 
 public class MaestroReferenceIdConstraintTest extends BaseConstraintTest {
@@ -99,10 +104,55 @@ public void isIdUsingDoubleUnderscores() {
     ConstraintViolation<TestId> violation = violations.iterator().next();
     assertEquals("foo__bar", violation.getInvalidValue());
     assertEquals(
-        "[maestro id or name reference] cannot contain double underscores '__' - rejected value is [foo__bar]",
+        "[maestro id or name reference] cannot contain the step param separator '__' - rejected value is [foo__bar]",
         violation.getMessage());
   }
 
+  @Test
+  public void isIdValidWithCustomSeparator() {
+    Set<ConstraintViolation<TestId>> violations =
+        validatorWithSeparator("___").validate(new TestId("foo__bar"));
+    assertEquals(0, violations.size());
+  }
+
+  @Test
+  public void isIdUsingCustomSeparator() {
+    Set<ConstraintViolation<TestId>> violations =
+        validatorWithSeparator("___").validate(new TestId("foo___bar"));
+    assertEquals(1, violations.size());
+    ConstraintViolation<TestId> violation = violations.iterator().next();
+    assertEquals("foo___bar", violation.getInvalidValue());
+    assertEquals(
+        "[maestro id or name reference] cannot contain the step param separator '___' - rejected value is [foo___bar]",
+        violation.getMessage());
+  }
+
+  private Validator validatorWithSeparator(String separator) {
+    return Validation.byProvider(ApacheValidationProvider.class)
+        .configure()
+        .constraintValidatorFactory(
+            new ConstraintValidatorFactory() {
+              @Override
+              public <T extends ConstraintValidator<?, ?>> T getInstance(Class<T> key) {
+                try {
+                  T instance = key.getDeclaredConstructor().newInstance();
+                  if (instance instanceof MaestroReferenceIdConstraint.MaestroIdValidator) {
+                    ((MaestroReferenceIdConstraint.MaestroIdValidator) instance)
+                        .setStepParamSeparator(() -> separator);
+                  }
+                  return instance;
+                } catch (Exception e) {
+                  throw new RuntimeException(e);
+                }
+              }
+
+              @Override
+              public void releaseInstance(ConstraintValidator<?, ?> instance) {}
+            })
+        .buildValidatorFactory()
+        .getValidator();
+  }
+
   @Test
   public void isIdUsingReservedPrefix() {
     Set<ConstraintViolation<TestId>> violations = validator.validate(new TestId("maestro_foo"));

maestro-database/src/main/java/com/netflix/maestro/database/AbstractDatabaseDao.java

@@ -406,19 +406,6 @@ private <R> R withTransaction(final ConnectionFunction<R> function) throws SQLEx
     }
   }
 
-  /**
-   * Mark the current transaction as SERIALIZABLE isolation level.
-   *
-   * @param conn the connection
-   * @throws SQLException sql exception
-   */
-  protected void markTransactionSerializable(Connection conn) throws SQLException {
-    try (PreparedStatement stmt =
-        conn.prepareStatement("SET TRANSACTION ISOLATION LEVEL SERIALIZABLE")) {
-      stmt.execute();
-    }
-  }
-
   /**
    * Mark the current transaction as SERIALIZABLE READ ONLY isolation level.
    *

maestro-engine/src/main/java/com/netflix/maestro/engine/eval/ParamEvaluator.java

@@ -59,13 +59,18 @@
 @Slf4j
 @AllArgsConstructor
 public class ParamEvaluator {
-  private static final String STEP_PARAM_SEPARATOR = "__";
   private static final String SIGNAL_EXPRESSION_TEMPLATE =
       "return params.getFromSignal('%s', '%s');";
   private static final String PARAM_NAME_FOR_ALL = "params";
 
   private final ExprEvaluator exprEvaluator;
   private final ObjectMapper objectMapper;
+  private final String stepParamSeparator;
+
+  /** Convenience constructor using the default step param separator {@code __}. */
+  public ParamEvaluator(ExprEvaluator exprEvaluator, ObjectMapper objectMapper) {
+    this(exprEvaluator, objectMapper, Constants.DEFAULT_STEP_PARAM_SEPARATOR);
+  }
 
   /**
    * Evaluate workflow parameters.
@@ -154,7 +159,7 @@ private Map<String, Parameter> getReferencedParams(
         parseWorkflowParameter(
             workflowParams, workflowParams.get(refParamName), workflowId, visited);
         refParams.put(refParamName, workflowParams.get(refParamName));
-      } else if (refParamName.contains(STEP_PARAM_SEPARATOR)) {
+      } else if (refParamName.contains(stepParamSeparator)) {
         // here it might be from signal triggers
         Map.Entry<String, String> pair = parseReferenceName(refParamName, Collections.emptyMap());
         refParams.put(
@@ -313,7 +318,7 @@ private Map<String, Parameter> getReferencedParams(
       Set<String> visited) {
     Map<String, Parameter> usedParams = new HashMap<>();
     for (String refParam : refParamNames) {
-      if (refParam.contains(STEP_PARAM_SEPARATOR)) {
+      if (refParam.contains(stepParamSeparator)) {
         usedParams.put(
             refParam,
             getReferenceParam(
@@ -350,11 +355,15 @@ private Map<String, Parameter> getReferencedParams(
     return usedParams;
   }
 
-  /** Extract step id and param name from the reference. It handles `__`, `___`, and `____`. */
+  /**
+   * Extract step id and param name from the reference. It handles separator, separator with one
+   * extra underscore, and separator with two extra underscores (e.g. {@code __}, {@code ___}, and
+   * {@code ____} for the default separator).
+   */
   private Map.Entry<String, String> parseReferenceName(
       String refParam, Map<String, Map<String, Object>> allStepOutputData) {
-    int idx1 = refParam.indexOf(STEP_PARAM_SEPARATOR);
-    int idx2 = refParam.lastIndexOf(STEP_PARAM_SEPARATOR);
+    int idx1 = refParam.indexOf(stepParamSeparator);
+    int idx2 = refParam.lastIndexOf(stepParamSeparator);
     //  ___ or ____ case
     if (idx1 != idx2) {
       String id1 = refParam.substring(0, idx1);
@@ -374,7 +383,7 @@ private Map.Entry<String, String> parseReferenceName(
       }
     }
     String refStepId = refParam.substring(0, idx1);
-    String refParamName = refParam.substring(idx1 + 2);
+    String refParamName = refParam.substring(idx1 + stepParamSeparator.length());
     return new AbstractMap.SimpleEntry<>(refStepId, refParamName);
   }
 

maestro-engine/src/test/java/com/netflix/maestro/engine/eval/ParamEvaluatorTest.java

@@ -311,6 +311,21 @@ public void testParseStepParameterWith4Underscore() {
     assertEquals("123-1", bar.getEvaluatedResult());
   }
 
+  @Test
+  public void testParseStepParameterWithCustomSeparator() {
+    ParamEvaluator customEvaluator = new ParamEvaluator(evaluator, MAPPER, "___");
+    StringParameter bar =
+        StringParameter.builder().name("bar").expression("step1___foo + '-1';").build();
+    customEvaluator.parseStepParameter(
+        Collections.emptyMap(),
+        Collections.emptyMap(),
+        Collections.singletonMap(
+            "foo", StringParameter.builder().evaluatedResult("123").evaluatedTime(123L).build()),
+        bar,
+        "step1");
+    assertEquals("123-1", bar.getEvaluatedResult());
+  }
+
   @Test
   public void testParseStepParameterUsingParamsToGetWorkflowParams() {
     StringParameter bar =

maestro-server/src/main/java/com/netflix/maestro/server/config/MaestroEngineConfiguration.java

@@ -34,6 +34,7 @@
 import com.netflix.maestro.server.properties.MaestroProperties;
 import com.netflix.maestro.server.properties.StepRuntimeProperties;
 import com.netflix.maestro.utils.JsonHelper;
+import com.netflix.maestro.utils.StepParamSeparator;
 import com.netflix.spectator.api.DefaultRegistry;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -142,11 +143,19 @@ public ExprEvaluator exprEvaluator(
     return new ExprEvaluator(properties.getSel(), extensionRepo);
   }
 
+  @Bean
+  public StepParamSeparator stepParamSeparator(MaestroProperties properties) {
+    LOG.info("Creating maestro stepParamSeparator within Spring boot...");
+    return properties.getParamEvaluator();
+  }
+
   @Bean
   public ParamEvaluator paramEvaluatorHelper(
       ExprEvaluator exprEvaluator,
-      @Qualifier(Constants.MAESTRO_QUALIFIER) ObjectMapper objectMapper) {
+      @Qualifier(Constants.MAESTRO_QUALIFIER) ObjectMapper objectMapper,
+      MaestroProperties properties) {
     LOG.info("Creating maestro parameterHelper within Spring boot...");
-    return new ParamEvaluator(exprEvaluator, objectMapper);
+    return new ParamEvaluator(
+        exprEvaluator, objectMapper, properties.getParamEvaluator().getStepParamSeparator());
   }
 }

maestro-server/src/main/java/com/netflix/maestro/server/properties/MaestroProperties.java

@@ -27,5 +27,11 @@
 public class MaestroProperties {
   private final QueueProperties queue;
   private final SelProperties sel;
+  private final ParamEvaluatorProperties paramEvaluator;
   private final StepActionProperties stepAction;
+
+  /** Returns the param evaluator properties, defaulting to {@code __} separator if not set. */
+  public ParamEvaluatorProperties getParamEvaluator() {
+    return paramEvaluator != null ? paramEvaluator : new ParamEvaluatorProperties();
+  }
 }

maestro-server/src/main/java/com/netflix/maestro/server/properties/ParamEvaluatorProperties.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2024 Netflix, Inc.
+ *
+ * Licensed 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 com.netflix.maestro.server.properties;
+
+import com.netflix.maestro.models.Constants;
+import com.netflix.maestro.utils.StepParamSeparator;
+import lombok.Getter;
+import lombok.Setter;
+
+/**
+ * Parameter evaluator properties. Please check {@link
+ * com.netflix.maestro.engine.eval.ParamEvaluator} about how they are used.
+ */
+@Getter
+@Setter
+public class ParamEvaluatorProperties implements StepParamSeparator {
+  private String stepParamSeparator = Constants.DEFAULT_STEP_PARAM_SEPARATOR;
+}