Special behavior for Temporal built-in prefixes (#2409)

Author: Quinn-With-Two-Ns

temporal-sdk/src/main/java/io/temporal/activity/ActivityMethod.java

@@ -40,6 +40,8 @@
    *
    * <p>Be careful with names that contain special characters, as these names can be used as metric
    * tags. Systems like Prometheus ignore metrics which have tags with unsupported characters.
+   *
+   * <p>Name cannot start with __temporal_ as it is reserved for internal use.
    */
   String name() default "";
 }

temporal-sdk/src/main/java/io/temporal/common/metadata/POJOActivityImplMetadata.java

@@ -23,6 +23,7 @@
 import com.google.common.collect.ImmutableList;
 import io.temporal.activity.ActivityMethod;
 import io.temporal.common.MethodRetry;
+import io.temporal.internal.common.InternalUtils;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -90,6 +91,7 @@ private POJOActivityImplMetadata(Class<?> implClass) {
       activityInterfaces.add(interfaceMetadata);
       List<POJOActivityMethodMetadata> methods = interfaceMetadata.getMethodsMetadata();
       for (POJOActivityMethodMetadata methodMetadata : methods) {
+        InternalUtils.checkMethodName(methodMetadata);
         POJOActivityMethodMetadata registeredMM =
             byName.put(methodMetadata.getActivityTypeName(), methodMetadata);
         if (registeredMM != null && !registeredMM.equals(methodMetadata)) {

temporal-sdk/src/main/java/io/temporal/common/metadata/POJOWorkflowImplMetadata.java

@@ -22,6 +22,7 @@
 
 import com.google.common.collect.ImmutableList;
 import io.temporal.common.Experimental;
+import io.temporal.internal.common.InternalUtils;
 import io.temporal.internal.common.env.ReflectionUtils;
 import java.lang.reflect.Constructor;
 import java.util.*;
@@ -145,6 +146,8 @@ private POJOWorkflowImplMetadata(
                   + methodMetadata.getWorkflowMethod()
                   + "\"");
         }
+        // Validate the method name is not using any reserved prefixes or names.
+        InternalUtils.checkMethodName(methodMetadata);
         switch (methodMetadata.getType()) {
           case WORKFLOW:
             workflowMethods.put(methodMetadata.getName(), methodMetadata);

temporal-sdk/src/main/java/io/temporal/internal/common/InternalUtils.java

@@ -22,11 +22,15 @@
 
 import com.google.common.base.Defaults;
 import io.nexusrpc.Header;
+import io.nexusrpc.handler.ServiceImplInstance;
 import io.temporal.api.common.v1.Callback;
 import io.temporal.api.enums.v1.TaskQueueKind;
 import io.temporal.api.taskqueue.v1.TaskQueue;
 import io.temporal.client.WorkflowOptions;
 import io.temporal.client.WorkflowStub;
+import io.temporal.common.metadata.POJOActivityMethodMetadata;
+import io.temporal.common.metadata.POJOWorkflowMethodMetadata;
+import io.temporal.common.metadata.WorkflowMethodType;
 import io.temporal.internal.client.NexusStartWorkflowRequest;
 import java.util.Arrays;
 import java.util.Map;
@@ -37,7 +41,11 @@
 
 /** Utility functions shared by the implementation code. */
 public final class InternalUtils {
+  public static String TEMPORAL_RESERVED_PREFIX = "__temporal_";
+
   private static final Logger log = LoggerFactory.getLogger(InternalUtils.class);
+  private static String QUERY_TYPE_STACK_TRACE = "__stack_trace";
+  private static String ENHANCED_QUERY_TYPE_STACK_TRACE = "__enhanced_stack_trace";
 
   public static TaskQueue createStickyTaskQueue(
       String stickyTaskQueueName, String normalTaskQueueName) {
@@ -135,6 +143,57 @@ public static WorkflowStub createNexusBoundStub(
     return stub.newInstance(nexusWorkflowOptions.build());
   }
 
+  /** Check the method name for reserved prefixes or names. */
+  public static void checkMethodName(POJOWorkflowMethodMetadata methodMetadata) {
+    if (methodMetadata.getName().startsWith(TEMPORAL_RESERVED_PREFIX)) {
+      throw new IllegalArgumentException(
+          methodMetadata.getType().toString().toLowerCase()
+              + " name \""
+              + methodMetadata.getName()
+              + "\" must not start with \""
+              + TEMPORAL_RESERVED_PREFIX
+              + "\"");
+    }
+    if (methodMetadata.getType().equals(WorkflowMethodType.QUERY)
+        && (methodMetadata.getName().equals(QUERY_TYPE_STACK_TRACE)
+            || methodMetadata.getName().equals(ENHANCED_QUERY_TYPE_STACK_TRACE))) {
+      throw new IllegalArgumentException(
+          "Query method name \"" + methodMetadata.getName() + "\" is reserved for internal use");
+    }
+  }
+
+  public static void checkMethodName(POJOActivityMethodMetadata methodMetadata) {
+    if (methodMetadata.getActivityTypeName().startsWith(TEMPORAL_RESERVED_PREFIX)) {
+      throw new IllegalArgumentException(
+          "Activity name \""
+              + methodMetadata.getActivityTypeName()
+              + "\" must not start with \""
+              + TEMPORAL_RESERVED_PREFIX
+              + "\"");
+    }
+  }
+
   /** Prohibit instantiation */
   private InternalUtils() {}
+
+  public static void checkMethodName(ServiceImplInstance instance) {
+    if (instance.getDefinition().getName().startsWith(TEMPORAL_RESERVED_PREFIX)) {
+      throw new IllegalArgumentException(
+          "Service name \""
+              + instance.getDefinition().getName()
+              + "\" must not start with \""
+              + TEMPORAL_RESERVED_PREFIX
+              + "\"");
+    }
+    for (String operationName : instance.getDefinition().getOperations().keySet()) {
+      if (operationName.startsWith(TEMPORAL_RESERVED_PREFIX)) {
+        throw new IllegalArgumentException(
+            "Operation name \""
+                + operationName
+                + "\" must not start with \""
+                + TEMPORAL_RESERVED_PREFIX
+                + "\"");
+      }
+    }
+  }
 }

temporal-sdk/src/main/java/io/temporal/internal/nexus/NexusTaskHandlerImpl.java

@@ -35,6 +35,7 @@
 import io.temporal.common.converter.DataConverter;
 import io.temporal.common.interceptors.WorkerInterceptor;
 import io.temporal.failure.ApplicationFailure;
+import io.temporal.internal.common.InternalUtils;
 import io.temporal.internal.common.NexusUtil;
 import io.temporal.internal.worker.NexusTask;
 import io.temporal.internal.worker.NexusTaskHandler;
@@ -331,6 +332,7 @@ private void registerNexusService(Object nexusService) {
       throw new IllegalArgumentException("Nexus service object instance expected, not the class");
     }
     ServiceImplInstance instance = ServiceImplInstance.fromInstance(nexusService);
+    InternalUtils.checkMethodName(instance);
     if (serviceImplInstances.put(instance.getDefinition().getName(), instance) != null) {
       throw new TypeAlreadyRegisteredException(
           instance.getDefinition().getName(),

temporal-sdk/src/main/java/io/temporal/internal/sync/QueryDispatcher.java

@@ -20,6 +20,8 @@
 
 package io.temporal.internal.sync;
 
+import static io.temporal.internal.common.InternalUtils.TEMPORAL_RESERVED_PREFIX;
+
 import io.temporal.api.common.v1.Payloads;
 import io.temporal.api.sdk.v1.WorkflowInteractionDefinition;
 import io.temporal.common.converter.DataConverter;
@@ -72,6 +74,10 @@ public WorkflowInboundCallsInterceptor.QueryOutput handleInterceptedQuery(
   public Optional<Payloads> handleQuery(String queryName, Header header, Optional<Payloads> input) {
     WorkflowOutboundCallsInterceptor.RegisterQueryInput handler = queryCallbacks.get(queryName);
     Object[] args;
+    if (queryName.startsWith(TEMPORAL_RESERVED_PREFIX)) {
+      throw new IllegalArgumentException(
+          "Unknown query type: " + queryName + ", knownTypes=" + queryCallbacks.keySet());
+    }
     if (handler == null) {
       if (dynamicQueryHandler == null) {
         throw new IllegalArgumentException(

temporal-sdk/src/main/java/io/temporal/internal/sync/SignalDispatcher.java

@@ -20,6 +20,8 @@
 
 package io.temporal.internal.sync;
 
+import static io.temporal.internal.common.InternalUtils.TEMPORAL_RESERVED_PREFIX;
+
 import io.temporal.api.common.v1.Payloads;
 import io.temporal.api.sdk.v1.WorkflowInteractionDefinition;
 import io.temporal.common.converter.DataConverter;
@@ -86,6 +88,10 @@ public void handleSignal(
         signalCallbacks.get(signalName);
     Object[] args;
     HandlerUnfinishedPolicy policy;
+    if (signalName.startsWith(TEMPORAL_RESERVED_PREFIX)) {
+      // Ignore internal signals
+      return;
+    }
     if (handler == null) {
       if (dynamicSignalHandler == null) {
         signalBuffer.add(new SignalData(signalName, input, eventId, header));

temporal-sdk/src/main/java/io/temporal/internal/sync/UpdateDispatcher.java

@@ -20,6 +20,8 @@
 
 package io.temporal.internal.sync;
 
+import static io.temporal.internal.common.InternalUtils.TEMPORAL_RESERVED_PREFIX;
+
 import io.temporal.api.common.v1.Payloads;
 import io.temporal.api.sdk.v1.WorkflowInteractionDefinition;
 import io.temporal.common.converter.DataConverter;
@@ -61,6 +63,10 @@ public void handleValidateUpdate(
         updateCallbacks.get(updateName);
     Object[] args;
     HandlerUnfinishedPolicy policy;
+    if (updateName.startsWith(TEMPORAL_RESERVED_PREFIX)) {
+      throw new IllegalArgumentException(
+          "Unknown update name: " + updateName + ", knownTypes=" + updateCallbacks.keySet());
+    }
     if (handler == null) {
       if (dynamicUpdateHandler == null) {
         throw new IllegalArgumentException(

temporal-sdk/src/main/java/io/temporal/workflow/QueryMethod.java

@@ -41,6 +41,9 @@
    *
    * <p>Be careful about names that contain special characters. These names can be used as metric
    * tags. And systems like prometheus ignore metrics which have tags with unsupported characters.
+   *
+   * <p>Name cannot start with __temporal as it is reserved for internal use. The name also cannot
+   * be __stack_trace or __enhanced_stack_trace as they are reserved for internal use.
    */
   String name() default "";
 

temporal-sdk/src/main/java/io/temporal/workflow/SignalMethod.java

@@ -57,6 +57,8 @@
    *
    * <p>Be careful about names that contain special characters. These names can be used as metric
    * tags. And systems like prometheus ignore metrics which have tags with unsupported characters.
+   *
+   * <p>Name cannot start with __temporal_ as it is reserved for internal use.
    */
   String name() default "";
 

temporal-sdk/src/main/java/io/temporal/workflow/UpdateMethod.java

@@ -39,6 +39,8 @@
    *
    * <p>Be careful about names that contain special characters. These names can be used as metric
    * tags. And systems like prometheus ignore metrics which have tags with unsupported characters.
+   *
+   * <p>Name cannot start with __temporal as it is reserved for internal use.
    */
   String name() default "";
 

temporal-sdk/src/main/java/io/temporal/workflow/WorkflowMethod.java

@@ -33,6 +33,13 @@
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.METHOD)
 public @interface WorkflowMethod {
-  /** Name of the workflow type. Default is {short class name} */
+  /**
+   * Name of the workflow type. Default is {short class name}.
+   *
+   * <p>Be careful with names that contain special characters, as these names can be used as metric
+   * tags. Systems like Prometheus ignore metrics which have tags with unsupported characters.
+   *
+   * <p>Name cannot start with __temporal_ as it is reserved for internal use.
+   */
   String name() default "";
 }

temporal-sdk/src/test/java/io/temporal/workflow/WorkflowRestrictedNameTest.java

@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2022 Temporal Technologies, Inc. All Rights Reserved.
+ *
+ * Copyright (C) 2012-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Modifications copyright (C) 2017 Uber Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this material 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 io.temporal.workflow;
+
+import io.temporal.testing.internal.SDKTestWorkflowRule;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+
+public class WorkflowRestrictedNameTest {
+
+  @Rule
+  public SDKTestWorkflowRule testWorkflowRule =
+      SDKTestWorkflowRule.newBuilder().setDoNotStart(true).build();
+
+  @Test
+  public void testRegisteringRestrictedWorkflowMethod() {
+    IllegalArgumentException e =
+        Assert.assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                testWorkflowRule
+                    .getWorker()
+                    .registerWorkflowImplementationTypes(WorkflowMethodWithOverrideNameImpl.class));
+    Assert.assertEquals(
+        "workflow name \"__temporal_workflow\" must not start with \"__temporal_\"",
+        e.getMessage());
+
+    e =
+        Assert.assertThrows(
+            IllegalArgumentException.class,
+            () ->
+                testWorkflowRule
+                    .getWorker()
+                    .registerWorkflowImplementationTypes(WorkflowMethodRestrictedImpl.class));
+    Assert.assertEquals(
+        "workflow name \"__temporal_workflow\" must not start with \"__temporal_\"",
+        e.getMessage());
+  }
+
+  @WorkflowInterface
+  public interface WorkflowMethodWithOverrideNameRestricted {
+    @WorkflowMethod(name = "__temporal_workflow")
+    void workflowMethod();
+  }
+
+  public static class WorkflowMethodWithOverrideNameImpl
+      implements WorkflowMethodWithOverrideNameRestricted {
+
+    @Override
+    public void workflowMethod() {}
+  }
+
+  @WorkflowInterface
+  public interface __temporal_workflow {
+    @WorkflowMethod
+    void workflowMethod();
+  }
+
+  public static class WorkflowMethodRestrictedImpl implements __temporal_workflow {
+    @Override
+    public void workflowMethod() {}
+  }
+}