Merge pull request #340 from gthomps95/hotfix/better-err-msg-missing-property

Adding null check for missing input step property name

Author: danielgerlag

src/WorkflowCore/Services/DefinitionStorage/DefinitionLoader.cs

@@ -25,7 +25,7 @@ public DefinitionLoader(IWorkflowRegistry registry)
         {
             _registry = registry;
         }
-                        
+
         public WorkflowDefinition LoadDefinition(string source, Func<string, DefinitionSourceV1> deserializer)
         {
             var sourceObj = deserializer(source);
@@ -119,7 +119,7 @@ private WorkflowStepCollection ConvertSteps(ICollection<StepSourceV1> source, Ty
                     targetStep.Outcomes.Add(new StepOutcome() { ExternalNextStepId = $"{nextStep.NextStepId}" });
 
                 result.Add(targetStep);
-                
+
                 i++;
             }
 
@@ -176,6 +176,11 @@ private void AttachInputs(StepSourceV1 source, Type dataType, Type stepType, Wor
                 var environmentVarsParameter = Expression.Parameter(typeof(IDictionary), "environment");
                 var stepProperty = stepType.GetProperty(input.Key);
 
+                if (stepProperty == null)
+                {
+                    throw new ArgumentException($"Unknown property for input {input.Key} on {source.Id}");
+                }
+
                 if (input.Value is string)
                 {
                     var acn = BuildScalarInputAction(input, dataParameter, contextParameter, environmentVarsParameter, stepProperty);
@@ -203,7 +208,7 @@ private void AttachOutputs(StepSourceV1 source, Type dataType, Type stepType, Wo
 
                 var dataParameter = Expression.Parameter(dataType, "data");
                 Expression targetProperty;
-                
+
                 // Check if our datatype has a matching property
                 var propertyInfo = dataType.GetProperty(output.Key);
                 if (propertyInfo != null)
@@ -217,7 +222,7 @@ private void AttachOutputs(StepSourceV1 source, Type dataType, Type stepType, Wo
                     // If we did not find a matching property try to find a Indexer with string parameter
                     propertyInfo = dataType.GetProperty("Item");
                     targetProperty = Expression.Property(dataParameter, propertyInfo, Expression.Constant(output.Key));
-                    
+
                     Action<IStepBody, object> acn = (pStep, pData) =>
                     {
                         object resolvedValue = sourceExpr.Compile().DynamicInvoke(pStep); ;

test/WorkflowCore.TestAssets/Utils.cs

@@ -10,7 +10,7 @@ namespace WorkflowCore.TestAssets
     public static class Utils
     {
         private static JsonSerializerSettings SerializerSettings = new JsonSerializerSettings() { TypeNameHandling = TypeNameHandling.All, DateFormatHandling = DateFormatHandling.IsoDateFormat, DateTimeZoneHandling = DateTimeZoneHandling.Utc };
-              
+
         public static T DeepCopy<T>(T obj)
         {
             string str = JsonConvert.SerializeObject(obj, SerializerSettings);
@@ -32,6 +32,11 @@ public static string GetTestDefinitionDynamicJson()
         {
             return File.ReadAllText("stored-dynamic-definition.json");
         }
+
+        public static string GetTestDefinitionJsonMissingInputProperty()
+        {
+            return File.ReadAllText("stored-def-missing-input-property.json");
+        }
     }
 }
 

test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj

@@ -25,6 +25,9 @@
     <EmbeddedResource Include="stored-definition.json">
       <CopyToOutputDirectory>Always</CopyToOutputDirectory>
     </EmbeddedResource>
+    <EmbeddedResource Include="stored-def-missing-input-property.json">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </EmbeddedResource>
   </ItemGroup>
 
   <ItemGroup>

test/WorkflowCore.TestAssets/stored-def-missing-input-property.json

@@ -0,0 +1,19 @@
+{
+  "Id": "Test",
+  "Version": 1,
+  "Description": "",
+  "DataType": "WorkflowCore.TestAssets.DataTypes.CounterBoard, WorkflowCore.TestAssets",
+  "Steps": [
+    {
+      "Id": "Step1",
+      "StepType": "WorkflowCore.TestAssets.Steps.Counter, WorkflowCore.TestAssets",
+      "ErrorBehavior": "Retry",
+      "Inputs": {
+        "Value1": "data.Counter1"
+      },
+      "Outputs": {
+        "Counter1": "step.Value"
+      }
+    }
+  ]
+}

test/WorkflowCore.UnitTests/Services/DefinitionStorage/DefinitionLoaderTests.cs

@@ -59,13 +59,18 @@ public void ParseDefinitionDynamic()
             A.CallTo(() => _registry.RegisterWorkflow(A<WorkflowDefinition>.That.Matches(MatchTestDefinition, ""))).MustHaveHappened();
         }
 
+        [Fact(DisplayName = "Should throw error for bad input property name on step")]
+        public void ParseDefinitionInputException()
+        {
+            Assert.Throws<ArgumentException>(() => _subject.LoadDefinition(TestAssets.Utils.GetTestDefinitionJsonMissingInputProperty(), Deserializers.Json));
+        }
 
         private bool MatchTestDefinition(WorkflowDefinition def)
         {
             //TODO: make this better
             var step1 = def.Steps.Single(s => s.ExternalId == "Step1");
             var step2 = def.Steps.Single(s => s.ExternalId == "Step2");
-            
+
             step1.Outcomes.Count.Should().Be(1);
             step1.Inputs.Count.Should().Be(1);
             step1.Outputs.Count.Should().Be(1);