Merge pull request #41 from microsoft/feature/MissingResolverWorkaround

NetStandard workaround for MissingResolver issue plus other cleanups

Forge.TreeWalker.UnitTests/test/ExecutorUnitTests.cs

@@ -119,8 +119,7 @@ public void TestExecutor_Fail_BadExpression()
         public void TestExecutor_Success_CompileWithExternalDependencies()
         {
             this.UserContext.Foo = ExternalTestType.TestEnum;
-            List<Type> dependencies = new List<Type>();
-            dependencies.Add(typeof(ExternalTestType));
+            List<Type> dependencies = new List<Type> { typeof(ExternalTestType) };
             ExpressionExecutor ex = new ExpressionExecutor(session: null, userContext: this.UserContext, dependencies: dependencies);
             string expression = "UserContext.Foo.ToString() == ExternalTestType.TestEnum.ToString()";
             Assert.IsTrue(ex.Execute<bool>(expression).GetAwaiter().GetResult(), "Expected ExpressionExecutor to successfully evaluate a true expression.");
@@ -148,8 +147,7 @@ public void TestExecutor_Fail_CompileExpressionWithExternalDependenciesAndMissin
         {
             this.UserContext.Foo = ExternalTestType.TestEnum;
             this.UserContext.Bar = DiffNamespaceType.TestOne;
-            List<Type> dependencies = new List<Type>();
-            dependencies.Add(typeof(ExternalTestType));
+            List<Type> dependencies = new List<Type> { typeof(ExternalTestType) };
 
             ExpressionExecutor ex = new ExpressionExecutor(session: null, userContext: this.UserContext, dependencies: dependencies);
             string expression = "UserContext.Foo == ExternalTestType.TestEnum && UserContext.Bar == DiffNamespaceType.TestOne";
@@ -169,8 +167,7 @@ public void TestExecutor_Success_CompileExpressionWithMissingDependenciesButOthe
         {
             this.UserContext.Foo = ExternalTestType.TestEnum;
             this.UserContext.Bar = TestType.Test;
-            List<Type> dependencies = new List<Type>();
-            dependencies.Add(typeof(ExternalTestType));
+            List<Type> dependencies = new List<Type> { typeof(ExternalTestType) };
 
             ExpressionExecutor ex = new ExpressionExecutor(session: null, userContext: this.UserContext, dependencies: dependencies);
             string expression = "UserContext.Foo.ToString() == ExternalTestType.TestEnum.ToString() && UserContext.Bar.ToString() == TestType.Test.ToString()";
@@ -181,16 +178,17 @@ public void TestExecutor_Success_CompileExpressionWithMissingDependenciesButOthe
         public void TestExecutor_Success_CompileExpressionWithForgeDefaultDependenciesBeingPassedInExternally()
         {
             this.UserContext.Foo = "Foo";
-            List<Type> dependencies = new List<Type>();
-
-            // Tasks dependency needed by Forge by default.
-            dependencies.Add(typeof(Task));
+            List<Type> dependencies = new List<Type>
+            {
+                // Tasks dependency needed by Forge by default.
+                typeof(Task),
 
-            // Reflection dependency needed by Forge for runtime compilation.
-            dependencies.Add(typeof(Type));
+                // Reflection dependency needed by Forge for runtime compilation.
+                typeof(Type)
+            };
 
             // Default dependencies are expected to be tossed away internally in ExpressionExecutor. 
-            ExpressionExecutor ex = new ExpressionExecutor(session: null, userContext: this.UserContext);
+            ExpressionExecutor ex = new ExpressionExecutor(session: null, userContext: this.UserContext, dependencies: dependencies);
             string expression = "UserContext.Foo == \"Foo\"";
             Assert.IsTrue(ex.Execute<bool>(expression).GetAwaiter().GetResult(), "Expected ExpressionExecutor to successfully evaluate a true expression.");
         }
@@ -328,9 +326,11 @@ public void TestExecutor_TreeInput_Success()
         [TestMethod]
         public void TestExecutor_TreeInputJObject_Success()
         {
-            JObject treeInput = new JObject();
-            treeInput["StringProperty"] = "TestValue";
-            treeInput["IntProperty"] = 10;
+            JObject treeInput = new JObject
+            {
+                ["StringProperty"] = "TestValue",
+                ["IntProperty"] = 10
+            };
 
             ExpressionExecutor ex = new ExpressionExecutor(session: null, userContext: this.UserContext, dependencies: null, scriptCache: null, treeInput: treeInput);
             string expression = "TreeInput.StringProperty == \"TestValue\" && TreeInput.IntProperty == 10";
@@ -355,16 +355,16 @@ public void TestExecutor_ParentScriptBehavior()
             ConcurrentDictionary<string, Script<object>> scriptCache = new ConcurrentDictionary<string, Script<object>>();
             ExpressionExecutor ex = new ExpressionExecutor(session: null, userContext: this.UserContext, dependencies: null, scriptCache: scriptCache);
 
-            if (ex.parentScriptTask.IsCompleted)
+            if (ex.ParentScriptTask.IsCompleted)
             {
                 Assert.Fail("Do not expect parentScriptTask to be completed immediately after ExpressionExecutor is initialized.");
             }
 
             // Test - After ScriptCache is initialized with parentScript, subsuquent ExpressionExecutor initialization should happen quickly.
-            ex.parentScriptTask.Wait();
+            ex.ParentScriptTask.Wait();
             ex = new ExpressionExecutor(session: null, userContext: this.UserContext, dependencies: null, scriptCache: scriptCache);
 
-            if (!ex.parentScriptTask.IsCompleted)
+            if (!ex.ParentScriptTask.IsCompleted)
             {
                 Assert.Fail("Expect parentScriptTask to be completed on ExpressionExecutor initialize when using an already initialized scriptCache.");
             }

Forge.TreeWalker.UnitTests/test/TreeWalkerUnitTests.cs

@@ -38,7 +38,7 @@ public class TreeWalkerUnitTests
         private TreeWalkerParameters parameters;
         private TreeWalkerSession session;
         private Dictionary<string, ForgeTree> forgeTrees = new Dictionary<string, ForgeTree>();
-        private ConcurrentDictionary<string, Script<object>> scriptCache = new ConcurrentDictionary<string, Script<object>>();
+        private readonly ConcurrentDictionary<string, Script<object>> scriptCache = new ConcurrentDictionary<string, Script<object>>();
 
 
         public void TestInitialize(string jsonSchema, string treeName = null, string currentNodeSkipActionContext = null)
@@ -728,8 +728,10 @@ public void Test_ExternalExecutors()
         {
             this.TestInitialize(jsonSchema: ForgeSchemaHelper.ExternalExecutors);
 
-            Dictionary<string, Func<string, CancellationToken, Task<object>>> externalExecutors = new Dictionary<string, Func<string, CancellationToken, Task<object>>>();
-	        externalExecutors.Add("External|", External);
+            Dictionary<string, Func<string, CancellationToken, Task<object>>> externalExecutors = new Dictionary<string, Func<string, CancellationToken, Task<object>>>
+            {
+                { "External|", External }
+            };
 
             this.parameters.ExternalExecutors = externalExecutors;
             this.session = new TreeWalkerSession(this.parameters);

Forge.TreeWalker/Forge.TreeWalker.csproj

@@ -14,6 +14,7 @@
     <PackageRequireLicenseAcceptance>true</PackageRequireLicenseAcceptance>
     <PackageOutputPath>bin\Release</PackageOutputPath>
     <IncludeSymbols>true</IncludeSymbols>
+    <PackageTags>Forge;TreeWalker;Roslyn;async;dynamic;generic;workflow engine;decision tree;config;stateful;low-code;tree visualization;workflow framework;JSON</PackageTags>
   </PropertyGroup>
   <ItemGroup>
     <PackageReference Include="GitVersionTask" Version="5.0.1">

Forge.TreeWalker/src/ActionContext.cs

@@ -144,7 +144,7 @@ public async Task<T> GetIntermediates<T>()
             }
             catch
             {
-                return default(T);
+                return default;
             }
         }
 

Forge.TreeWalker/src/ExpressionExecutor.cs

@@ -36,7 +36,7 @@ public class ExpressionExecutor
         /// The parentScriptTask kicks off initializing the Roslyn parentScript asynchronously, allowing ExpressionExecutor to construct very quickly.
         /// Initializing the Roslyn parentScript takes about 2 seconds. This time is saved if the application takes time to initialize before the first WalkTree call.
         /// </summary>
-        public Task parentScriptTask { get; private set; }
+        public Task ParentScriptTask { get; private set; }
 
         /// <summary>
         /// The parentScript that, upon initialization, gets Created, RunAsync, and added to the ScriptCache.
@@ -50,19 +50,19 @@ public class ExpressionExecutor
         /// <summary>
         /// List of external type dependencies needed to compile expressions.
         /// </summary>
-        private List<Type> dependencies;
+        private readonly List<Type> dependencies;
 
         /// <summary>
         /// The ScriptCache holds Roslyn Scripts that are created using parentScript.ContinueWith.
         /// This saves on memory and time since these continued Scripts use the already compiled parentScript as a base.
         /// The parentScript gets asynchronously compiled, ran, and cached on initialization.
         /// </summary>
-        private ConcurrentDictionary<string, Script<object>> scriptCache;
+        private readonly ConcurrentDictionary<string, Script<object>> scriptCache;
 
         /// <summary>
         /// Global parameters passed to Roslyn scripts that can be referenced inside expressions.
         /// </summary>
-        private CodeGenInputParams parameters;
+        private readonly CodeGenInputParams parameters;
 
         /// <summary>
         /// Instantiates the ExpressionExecutor class with objects that can be referenced in the schema.
@@ -126,7 +126,7 @@ public ExpressionExecutor(ITreeSession session, object userContext)
         /// <returns>The T value of the evaluated code.</returns>
         public async Task<T> Execute<T>(string expression)
         {
-            await this.parentScriptTask;
+            await this.ParentScriptTask;
 
             Script<object> expressionScript = this.scriptCache.GetOrAdd(
                 expression,
@@ -146,13 +146,18 @@ private void Initialize()
             if (this.scriptCache.TryGetValue(ParentScriptCode, out this.parentScript))
             {
                 // The parentScript is already initialized.
-                this.parentScriptTask = Task.CompletedTask;
+                this.ParentScriptTask = Task.CompletedTask;
                 return;
             }
 
-            this.parentScriptTask = Task.Run(async () =>
+            this.ParentScriptTask = Task.Run(async () =>
             {
-                ScriptOptions scriptOptions = ScriptOptions.Default.WithMetadataResolver(new MissingResolver());
+                ScriptOptions scriptOptions = ScriptOptions.Default;
+#if NET462
+
+                // MissingResolver improvement not available in NetStandard. Only add to Net 462.
+                scriptOptions = ScriptOptions.Default.WithMetadataResolver(new MissingResolver());
+#endif
 
                 // Add references to required assemblies.
                 Assembly mscorlib = typeof(object).Assembly;

Forge.TreeWalker/src/SubroutineAction.cs

@@ -30,7 +30,7 @@ public class SubroutineAction : BaseAction
         /// The tree walker parameters of the parent tree walker session.
         /// Used to call InitializeSubroutineTree to get the initialized TreeWalkerSession for this Subroutine.
         /// </summary>
-        private TreeWalkerParameters parameters { get; set; }
+        private readonly TreeWalkerParameters parameters;
 
         /// <summary>
         /// Instantiates a SubroutineAction with the required parameters.

Forge.TreeWalker/src/TreeWalkerSession.cs

@@ -17,7 +17,6 @@ namespace Microsoft.Forge.TreeWalker
     using System.Text.RegularExpressions;
     using System.Threading;
     using System.Threading.Tasks;
-
     using Microsoft.Forge.Attributes;
     using Microsoft.Forge.DataContracts;
     using Microsoft.Forge.TreeWalker.ForgeExceptions;
@@ -89,13 +88,13 @@ public class TreeWalkerSession : ITreeSession
         ///     Ex) C#|"expression"
         ///     Ex) C#<Boolean>|"expression"
         /// </summary>
-        private static Regex RoslynRegex = new Regex(@"^C#(\<(.+)\>)?\|");
+        private static readonly Regex RoslynRegex = new Regex(@"^C#(\<(.+)\>)?\|");
 
         /// <summary>
         /// The leading text to add to Schema strings to indicate the property value should be evaluated with Roslyn.
         /// The property value must match the RoslynRegex to be evaluated with Roslyn.
         /// </summary>
-        private static string RoslynLeadingText = "C#";
+        private static readonly string RoslynLeadingText = "C#";
 
         /// <summary>
         /// The TreeWalkerParameters contains the required and optional properties used by the TreeWalkerSession.
@@ -116,20 +115,20 @@ public class TreeWalkerSession : ITreeSession
         /// The WalkTree cancellation token source.
         /// Used to send cancellation signal to action tasks and to stop tree walker from visiting future nodes.
         /// </summary>
-        private CancellationTokenSource walkTreeCts;
+        private readonly CancellationTokenSource walkTreeCts;
 
         /// <summary>
         /// The ExpressionExecutor dynamically compiles code and executes it.
         /// </summary>
-        private ExpressionExecutor expressionExecutor;
+        private readonly ExpressionExecutor expressionExecutor;
 
         /// <summary>
         /// The map of string ActionNames to ActionDefinitions.
         /// This map is generated using reflection to find all the classes with the applied ForgeActionAttribute from the given Assembly.
         /// The string key is the Action class name.
         /// The ActionDefinition value contains the Action class type, and the InputType for the Action.
         /// </summary>
-        private Dictionary<string, ActionDefinition> actionsMap;
+        private readonly Dictionary<string, ActionDefinition> actionsMap;
 
         /// <summary>
         /// Volatile flag to determine if we are visiting a node normally, or upon rehydration.
@@ -1103,10 +1102,11 @@ public async Task<object> EvaluateDynamicProperty(dynamic schemaObj, Type knownT
         /// <param name="actionResponse">The action response object returned from the action.</param>
         private async Task CommitActionResponse(string treeActionKey, ActionResponse actionResponse)
         {
-            List<KeyValuePair<string, object>> itemsToPersist = new List<KeyValuePair<string, object>>();
-
-            itemsToPersist.Add(new KeyValuePair<string, object>(treeActionKey + ActionResponseSuffix, actionResponse));
-            itemsToPersist.Add(new KeyValuePair<string, object>(LastTreeActionSuffix, treeActionKey));
+            List<KeyValuePair<string, object>> itemsToPersist = new List<KeyValuePair<string, object>>
+            {
+                new KeyValuePair<string, object>(treeActionKey + ActionResponseSuffix, actionResponse),
+                new KeyValuePair<string, object>(LastTreeActionSuffix, treeActionKey)
+            };
 
             await this.Parameters.ForgeState.SetRange(itemsToPersist);
         }
@@ -1129,7 +1129,6 @@ private async Task CommitCurrentTreeNode(string treeNodeKey, TreeNode treeNode)
                 foreach (KeyValuePair<string, TreeAction> kvp in treeNode.Actions)
                 {
                     string treeActionKey = kvp.Key;
-                    TreeAction treeAction = kvp.Value;
                     ActionResponse actionResponse = await this.GetOutputAsync(treeActionKey).ConfigureAwait(false);
 
                     if (actionResponse == null)
@@ -1194,10 +1193,11 @@ private async Task<object> GetOrCommitTreeInput(object treeInput)
         /// <param name="actionsMap">The map of string ActionNames to ActionDefinitions.</param>
         public static void GetActionsMapFromAssembly(Assembly forgeActionsAssembly, out Dictionary<string, ActionDefinition> actionsMap)
         {
-            actionsMap = new Dictionary<string, ActionDefinition>();
-
-            // Add native ForgeActions: SubroutineAction.
-            actionsMap.Add(nameof(SubroutineAction), new ActionDefinition() { ActionType = typeof(SubroutineAction), InputType = typeof(SubroutineInput) });
+            actionsMap = new Dictionary<string, ActionDefinition>
+            {
+                // Add native ForgeActions: SubroutineAction.
+                { nameof(SubroutineAction), new ActionDefinition() { ActionType = typeof(SubroutineAction), InputType = typeof(SubroutineInput) } }
+            };
 
             if (forgeActionsAssembly == null)
             {