Reserve __temporal prefix (#410)

.editorconfig

@@ -47,6 +47,9 @@ dotnet_diagnostic.CA2237.severity = none
 # Warn on unused imports
 dotnet_diagnostic.IDE0005.severity = warning
 
+# Don't warn on using var
+dotnet_diagnostic.IDE0008.severity = none
+
 # Cannot use range operator on older versions
 dotnet_diagnostic.IDE0057.severity = none
 
@@ -186,4 +189,4 @@ dotnet_style_require_accessibility_modifiers = for_non_interface_members:suggest
 #Style - Pattern matching
 
 #prefer pattern matching instead of is expression with type casts
-csharp_style_pattern_matching_over_as_with_null_check = true:suggestion
+csharp_style_pattern_matching_over_as_with_null_check = true:suggestion

omnisharp.json

@@ -1,5 +1,8 @@
 {
   "FormattingOptions": {
     "EnableEditorConfigSupport": true
+  },
+  "RoslynExtensionsOptions": {
+    "enableAnalyzersSupport": true
   }
 }

src/Temporalio/Activities/ActivityDefinition.cs

@@ -4,6 +4,7 @@
 using System.Reflection;
 using System.Runtime.ExceptionServices;
 using System.Threading.Tasks;
+using Temporalio.Runtime;
 
 namespace Temporalio.Activities
 {
@@ -293,6 +294,11 @@ private static ActivityDefinition Create(
             Func<object?[], object?> invoker,
             MethodInfo? methodInfo)
         {
+            if (name != null && name.StartsWith(TemporalRuntime.ReservedNamePrefix))
+            {
+                throw new ArgumentException(
+                    $"Activity name {name} cannot start with {TemporalRuntime.ReservedNamePrefix}");
+            }
             // If there is a null name, which means dynamic, there must only be one parameter type
             // and it must be varargs IRawValue
             if (name == null && (
@@ -320,4 +326,4 @@ private static ActivityDefinition Create(
             return new(name, returnType, parameterTypes, requiredParameterCount, invoker, methodInfo);
         }
     }
-}
+}

src/Temporalio/Runtime/TemporalRuntime.cs

@@ -13,6 +13,11 @@ namespace Temporalio.Runtime
     /// </remarks>
     public sealed class TemporalRuntime
     {
+        /// <summary>
+        /// Prefix for reserved handler and definition names.
+        /// </summary>
+        internal const string ReservedNamePrefix = "__temporal";
+
         private static readonly Lazy<TemporalRuntime> LazyDefault =
             new(() => new TemporalRuntime(new TemporalRuntimeOptions()));
 

src/Temporalio/Worker/ActivityWorker.cs

@@ -722,4 +722,4 @@ public override void Heartbeat(HeartbeatInput input)
             }
         }
     }
-}
+}

src/Temporalio/Worker/TemporalWorkerOptions.cs

@@ -424,4 +424,4 @@ internal void OnTaskCompleted(WorkflowInstance instance, Exception? failureExcep
             }
         }
     }
-}
+}

src/Temporalio/Worker/WorkflowInstance.cs

@@ -20,6 +20,7 @@
 using Temporalio.Common;
 using Temporalio.Converters;
 using Temporalio.Exceptions;
+using Temporalio.Runtime;
 using Temporalio.Worker.Interceptors;
 using Temporalio.Workflows;
 
@@ -939,7 +940,11 @@ private Task ApplyDoUpdateAsync(DoUpdate update)
             var updates = mutableUpdates.IsValueCreated ? mutableUpdates.Value : Definition.Updates;
             if (!updates.TryGetValue(update.Name, out var updateDefn))
             {
-                updateDefn = DynamicUpdate;
+                // Do not fall back onto dynamic update if using the reserved prefix
+                if (!update.Name.StartsWith(TemporalRuntime.ReservedNamePrefix))
+                {
+                    updateDefn = DynamicUpdate;
+                }
                 if (updateDefn == null)
                 {
                     var knownUpdates = updates.Keys.OrderBy(k => k);
@@ -1136,26 +1141,27 @@ private void ApplyQueryWorkflow(QueryWorkflow query)
                 try
                 {
                     WorkflowQueryDefinition? queryDefn;
-                    // If it's a stack trace query, create definition
+                    object? resultObj;
+
                     if (query.QueryType == "__stack_trace")
                     {
-                        Func<string> getter = GetStackTrace;
-                        queryDefn = WorkflowQueryDefinition.CreateWithoutAttribute(
-                            "__stack_trace", getter);
+                        resultObj = GetStackTrace();
                     }
                     else if (query.QueryType == "__temporal_workflow_metadata")
                     {
-                        Func<Api.Sdk.V1.WorkflowMetadata> getter = GetWorkflowMetadata;
-                        queryDefn = WorkflowQueryDefinition.CreateWithoutAttribute(
-                            "__temporal_workflow_metadata", getter);
+                        resultObj = GetWorkflowMetadata();
                     }
                     else
                     {
                         // Find definition or fail
                         var queries = mutableQueries.IsValueCreated ? mutableQueries.Value : Definition.Queries;
                         if (!queries.TryGetValue(query.QueryType, out queryDefn))
                         {
-                            queryDefn = DynamicQuery;
+                            // Do not fall back onto dynamic query if using the reserved prefix
+                            if (!query.QueryType.StartsWith(TemporalRuntime.ReservedNamePrefix))
+                            {
+                                queryDefn = DynamicQuery;
+                            }
                             if (queryDefn == null)
                             {
                                 var knownQueries = queries.Keys.OrderBy(k => k);
@@ -1164,8 +1170,7 @@ private void ApplyQueryWorkflow(QueryWorkflow query)
                                     $"known queries: [{string.Join(" ", knownQueries)}]");
                             }
                         }
-                    }
-                    var resultObj = inbound.Value.HandleQuery(new(
+                        resultObj = inbound.Value.HandleQuery(new(
                             Id: query.QueryId,
                             Query: query.QueryType,
                             Definition: queryDefn,
@@ -1176,6 +1181,7 @@ private void ApplyQueryWorkflow(QueryWorkflow query)
                                 dynamic: queryDefn.Dynamic,
                                 dynamicArgPrepend: query.QueryType),
                             Headers: query.Headers));
+                    }
                     AddCommand(new()
                     {
                         RespondToQuery = new()
@@ -1267,7 +1273,11 @@ private void ApplySignalWorkflow(SignalWorkflow signal)
             var signals = mutableSignals.IsValueCreated ? mutableSignals.Value : Definition.Signals;
             if (!signals.TryGetValue(signal.SignalName, out var signalDefn))
             {
-                signalDefn = DynamicSignal;
+                // Do not fall back onto dynamic signal if using the reserved prefix
+                if (!signal.SignalName.StartsWith(TemporalRuntime.ReservedNamePrefix))
+                {
+                    signalDefn = DynamicSignal;
+                }
                 if (signalDefn == null)
                 {
                     // No definition found, buffer

src/Temporalio/Workflows/Workflow.cs

@@ -1339,4 +1339,4 @@ public static class Unsafe
             public static bool IsReplaying => Context.IsReplaying;
         }
     }
-}
+}

src/Temporalio/Workflows/WorkflowDefinition.cs

@@ -4,6 +4,7 @@
 using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
+using Temporalio.Runtime;
 
 namespace Temporalio.Workflows
 {
@@ -427,6 +428,12 @@ public static WorkflowDefinition Create(
                 }
             }
 
+            // Verify that registered names to not use our reserved prefix
+            if (name != null && name.StartsWith(TemporalRuntime.ReservedNamePrefix))
+            {
+                errs.Add($"Workflow name {name} cannot start with {TemporalRuntime.ReservedNamePrefix}");
+            }
+
             // If there are any errors, throw
             if (errs.Count > 0)
             {
@@ -525,4 +532,4 @@ private static bool IsDefinedOnBase<T>(MethodInfo method)
             }
         }
     }
-}
+}

src/Temporalio/Workflows/WorkflowQueryDefinition.cs

@@ -1,7 +1,9 @@
 using System;
 using System.Collections.Concurrent;
+using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
+using Temporalio.Runtime;
 
 namespace Temporalio.Workflows
 {
@@ -10,11 +12,26 @@ namespace Temporalio.Workflows
     /// </summary>
     public class WorkflowQueryDefinition
     {
+        private static readonly string[] ReservedQueryHandlerPrefixes =
+        {
+            TemporalRuntime.ReservedNamePrefix,
+            "__stack_trace",
+            "__enhanced_stack_trace",
+        };
+
         private static readonly ConcurrentDictionary<MethodInfo, WorkflowQueryDefinition> MethodDefinitions = new();
         private static readonly ConcurrentDictionary<PropertyInfo, WorkflowQueryDefinition> PropertyDefinitions = new();
 
         private WorkflowQueryDefinition(string? name, string? description, MethodInfo? method, Delegate? del)
         {
+            if (name != null)
+            {
+                var reservedQ = ReservedQueryHandlerPrefixes.FirstOrDefault(p => name.StartsWith(p));
+                if (!string.IsNullOrEmpty(reservedQ))
+                {
+                    throw new ArgumentException($"Query handler name {name} cannot start with {reservedQ}");
+                }
+            }
             Name = name;
             Description = description;
             Method = method;
@@ -168,4 +185,4 @@ private static void AssertValid(MethodInfo method, bool dynamic)
             }
         }
     }
-}
+}

src/Temporalio/Workflows/WorkflowSignalDefinition.cs

@@ -2,6 +2,7 @@
 using System.Collections.Concurrent;
 using System.Reflection;
 using System.Threading.Tasks;
+using Temporalio.Runtime;
 
 namespace Temporalio.Workflows
 {
@@ -19,6 +20,10 @@ private WorkflowSignalDefinition(
             Delegate? del,
             HandlerUnfinishedPolicy unfinishedPolicy)
         {
+            if (name != null && name.StartsWith(TemporalRuntime.ReservedNamePrefix))
+            {
+                throw new ArgumentException($"Signal handler name {name} cannot start with {TemporalRuntime.ReservedNamePrefix}");
+            }
             Name = name;
             Description = description;
             Method = method;
@@ -146,4 +151,4 @@ private static void AssertValid(MethodInfo method, bool dynamic)
             }
         }
     }
-}
+}

src/Temporalio/Workflows/WorkflowUpdateDefinition.cs

@@ -3,6 +3,7 @@
 using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
+using Temporalio.Runtime;
 
 namespace Temporalio.Workflows
 {
@@ -22,6 +23,10 @@ private WorkflowUpdateDefinition(
             Delegate? validatorDel,
             HandlerUnfinishedPolicy unfinishedPolicy)
         {
+            if (name != null && name.StartsWith(TemporalRuntime.ReservedNamePrefix))
+            {
+                throw new ArgumentException($"Update handler name {name} cannot start with {TemporalRuntime.ReservedNamePrefix}");
+            }
             Name = name;
             Description = description;
             Method = method;
@@ -190,4 +195,4 @@ private static void AssertValid(
             }
         }
     }
-}
+}

tests/Temporalio.Tests/Activities/ActivityDefinitionTests.cs

@@ -208,6 +208,13 @@ public async Task Create_WithLambda_DoesNotHaveValidMethodInfo()
         Assert.Null(defn.MethodInfo);
     }
 
+    [Fact]
+    public void Create_With_Reserved_Name_Throws()
+    {
+        var exc = Assert.ThrowsAny<Exception>(() => ActivityDefinition.Create(IHaveABadName));
+        Assert.Contains("Activity name __temporal_badname cannot start with __temporal", exc.Message);
+    }
+
     protected static void BadAct1()
     {
     }
@@ -220,6 +227,9 @@ protected static void BadAct2(ref string foo)
     [Activity]
     protected static Task GoodAct1Async() => Task.CompletedTask;
 
+    [Activity("__temporal_badname")]
+    protected static Task IHaveABadName() => Task.CompletedTask;
+
     public static class GoodActivityClassStatic
     {
         [Activity]
@@ -252,4 +262,4 @@ public class GoodActivityClassInstance
         [Activity]
         public int MyActivity(int param) => param + 5;
     }
-}
+}

tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs

@@ -2997,6 +2997,17 @@ await ExecuteWorkerAsync<DynamicWorkflow>(
                 await handle.SignalAsync(wf => wf.SomeSignalAsync("signal arg"));
                 Assert.Equal("done", await handle.QueryAsync(wf => wf.SomeQuery("query arg")));
                 Assert.Equal("done", await handle.ExecuteUpdateAsync(wf => wf.SomeUpdateAsync("update arg")));
+
+                // Verify that invocations using the reserved prefix are not forwarded to dynamic
+                // handlers
+                await handle.SignalAsync($"{TemporalRuntime.ReservedNamePrefix}_whatever", new[] { "signal arg 1" });
+                var queryExc2 = await Assert.ThrowsAsync<WorkflowQueryFailedException>(
+                    () => handle.QueryAsync<string>($"{TemporalRuntime.ReservedNamePrefix}_whatever", new[] { "query arg 1" }));
+                Assert.Contains("not found", queryExc2.Message);
+                var updateExc = await Assert.ThrowsAsync<WorkflowUpdateFailedException>(
+                    () => handle.ExecuteUpdateAsync<string>($"{TemporalRuntime.ReservedNamePrefix}_whatever", new[] { "update arg 1" }));
+                Assert.Contains("not found", updateExc.InnerException?.Message);
+
                 // Event list must be collected before the WF finishes, since when it finishes it
                 // will be evicted from the cache and the first SomeQuery event will not exist upon
                 // replay.