From 370fa1b62907fd5fbbe598a266c256f6545d1fe7 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Wed, 10 Jul 2019 13:49:27 -0500 Subject: [PATCH 1/2] Adding null check for missing input step property name when loading definition from storage to give better error message. --- .../DefinitionStorage/DefinitionLoader.cs | 13 +++++++++---- test/WorkflowCore.TestAssets/Utils.cs | 7 ++++++- .../WorkflowCore.TestAssets.csproj | 3 +++ .../stored-definition-input-exc.json | 19 +++++++++++++++++++ .../DefinitionLoaderTests.cs | 7 ++++++- 5 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 test/WorkflowCore.TestAssets/stored-definition-input-exc.json diff --git a/src/WorkflowCore/Services/DefinitionStorage/DefinitionLoader.cs b/src/WorkflowCore/Services/DefinitionStorage/DefinitionLoader.cs index 9d37dd9da..5483e0851 100644 --- a/src/WorkflowCore/Services/DefinitionStorage/DefinitionLoader.cs +++ b/src/WorkflowCore/Services/DefinitionStorage/DefinitionLoader.cs @@ -25,7 +25,7 @@ public DefinitionLoader(IWorkflowRegistry registry) { _registry = registry; } - + public WorkflowDefinition LoadDefinition(string source, Func deserializer) { var sourceObj = deserializer(source); @@ -119,7 +119,7 @@ private WorkflowStepCollection ConvertSteps(ICollection 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 acn = (pStep, pData) => { object resolvedValue = sourceExpr.Compile().DynamicInvoke(pStep); ; diff --git a/test/WorkflowCore.TestAssets/Utils.cs b/test/WorkflowCore.TestAssets/Utils.cs index 62875e7c1..fdf7541f2 100644 --- a/test/WorkflowCore.TestAssets/Utils.cs +++ b/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 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 GetTestDefinitionJsonInputExc() + { + return File.ReadAllText("stored-definition-input-exc.json"); + } } } diff --git a/test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj b/test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj index 541f2afa9..d01b8f985 100644 --- a/test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj +++ b/test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj @@ -25,6 +25,9 @@ Always + + Always + diff --git a/test/WorkflowCore.TestAssets/stored-definition-input-exc.json b/test/WorkflowCore.TestAssets/stored-definition-input-exc.json new file mode 100644 index 000000000..dbf43105a --- /dev/null +++ b/test/WorkflowCore.TestAssets/stored-definition-input-exc.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" + } + } + ] +} \ No newline at end of file diff --git a/test/WorkflowCore.UnitTests/Services/DefinitionStorage/DefinitionLoaderTests.cs b/test/WorkflowCore.UnitTests/Services/DefinitionStorage/DefinitionLoaderTests.cs index 03494c5de..3e093f0f9 100644 --- a/test/WorkflowCore.UnitTests/Services/DefinitionStorage/DefinitionLoaderTests.cs +++ b/test/WorkflowCore.UnitTests/Services/DefinitionStorage/DefinitionLoaderTests.cs @@ -59,13 +59,18 @@ public void ParseDefinitionDynamic() A.CallTo(() => _registry.RegisterWorkflow(A.That.Matches(MatchTestDefinition, ""))).MustHaveHappened(); } + [Fact(DisplayName = "Should throw error for bad input property name on step")] + public void ParseDefinitionInputException() + { + Assert.Throws(() => _subject.LoadDefinition(TestAssets.Utils.GetTestDefinitionJsonInputExc(), 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); From dca94dfa7a1920ac61eb1e49c67560bbc7dfadd5 Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 23 Jul 2019 11:11:14 -0500 Subject: [PATCH 2/2] Renamed stored definition test file to be more descriptive. --- test/WorkflowCore.TestAssets/Utils.cs | 4 ++-- test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj | 2 +- ...-input-exc.json => stored-def-missing-input-property.json} | 0 .../Services/DefinitionStorage/DefinitionLoaderTests.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename test/WorkflowCore.TestAssets/{stored-definition-input-exc.json => stored-def-missing-input-property.json} (100%) diff --git a/test/WorkflowCore.TestAssets/Utils.cs b/test/WorkflowCore.TestAssets/Utils.cs index fdf7541f2..ea0f85f54 100644 --- a/test/WorkflowCore.TestAssets/Utils.cs +++ b/test/WorkflowCore.TestAssets/Utils.cs @@ -33,9 +33,9 @@ public static string GetTestDefinitionDynamicJson() return File.ReadAllText("stored-dynamic-definition.json"); } - public static string GetTestDefinitionJsonInputExc() + public static string GetTestDefinitionJsonMissingInputProperty() { - return File.ReadAllText("stored-definition-input-exc.json"); + return File.ReadAllText("stored-def-missing-input-property.json"); } } } diff --git a/test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj b/test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj index d01b8f985..0105a6e9d 100644 --- a/test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj +++ b/test/WorkflowCore.TestAssets/WorkflowCore.TestAssets.csproj @@ -25,7 +25,7 @@ Always - + Always diff --git a/test/WorkflowCore.TestAssets/stored-definition-input-exc.json b/test/WorkflowCore.TestAssets/stored-def-missing-input-property.json similarity index 100% rename from test/WorkflowCore.TestAssets/stored-definition-input-exc.json rename to test/WorkflowCore.TestAssets/stored-def-missing-input-property.json diff --git a/test/WorkflowCore.UnitTests/Services/DefinitionStorage/DefinitionLoaderTests.cs b/test/WorkflowCore.UnitTests/Services/DefinitionStorage/DefinitionLoaderTests.cs index 3e093f0f9..0a9293523 100644 --- a/test/WorkflowCore.UnitTests/Services/DefinitionStorage/DefinitionLoaderTests.cs +++ b/test/WorkflowCore.UnitTests/Services/DefinitionStorage/DefinitionLoaderTests.cs @@ -62,7 +62,7 @@ public void ParseDefinitionDynamic() [Fact(DisplayName = "Should throw error for bad input property name on step")] public void ParseDefinitionInputException() { - Assert.Throws(() => _subject.LoadDefinition(TestAssets.Utils.GetTestDefinitionJsonInputExc(), Deserializers.Json)); + Assert.Throws(() => _subject.LoadDefinition(TestAssets.Utils.GetTestDefinitionJsonMissingInputProperty(), Deserializers.Json)); } private bool MatchTestDefinition(WorkflowDefinition def)