From 759a5fccac9c6bdca3ddb772283569c7ce4dccb0 Mon Sep 17 00:00:00 2001 From: Vincent Spaa Date: Fri, 28 Feb 2025 09:21:08 +0100 Subject: [PATCH 1/4] Allows the evaluation of an expression with a method call on a static property Explicitly allows the use of `typeof(SomeType).FullName` Adds support for converting a nested workflow-input accessor to a Conductor-friendly `${}` string --- .../Util/ExpressionUtil.cs | 40 ++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/src/ConductorSharp.Engine/Util/ExpressionUtil.cs b/src/ConductorSharp.Engine/Util/ExpressionUtil.cs index b93b127..cebca54 100644 --- a/src/ConductorSharp.Engine/Util/ExpressionUtil.cs +++ b/src/ConductorSharp.Engine/Util/ExpressionUtil.cs @@ -71,30 +71,52 @@ private static bool IsStringInterpolation(Expression expression) => && methodExpression.Method.DeclaringType == typeof(string); private static object EvaluateExpression(Expression expr) => - IsEvaluatable(expr) ? Expression.Lambda(expr).Compile().DynamicInvoke() : throw new NonEvaluatableExpressionException(expr); + GetEvaluatableExpression(expr) is { } evaluatableExpression + ? Expression.Lambda(evaluatableExpression).Compile().DynamicInvoke() + : throw new NonEvaluatableExpressionException(expr); - private static bool IsEvaluatable(Expression expr) + private static Expression GetEvaluatableExpression(Expression expr) { switch (expr) { case MemberExpression memExpr: + if (ShouldCompileToJsonPathExpression(expr)) + return Expression.Constant(CreateExpressionString(expr), typeof(string)); + if ( typeof(ITaskModel).IsAssignableFrom(memExpr.Type) || typeof(WorkflowId).IsAssignableFrom(memExpr.Type) || typeof(IWorkflowInput).IsAssignableFrom(memExpr.Type) ) - return false; - return IsEvaluatable(memExpr.Expression); + return null; + + if (memExpr.Member is PropertyInfo { GetMethod.IsStatic: true }) + return memExpr; + + if (memExpr.Member is PropertyInfo propertyInfo && propertyInfo.DeclaringType == typeof(Type)) + return memExpr; + + return GetEvaluatableExpression(memExpr.Expression); case MethodCallExpression methodExpr: - return IsEvaluatable(methodExpr.Object) && methodExpr.Arguments.All(IsEvaluatable); + var arguments = methodExpr.Arguments.Select(GetEvaluatableExpression).ToArray(); + return GetEvaluatableExpression(methodExpr.Object) is { } objectExpression && arguments.Length == methodExpr.Arguments.Count + ? methodExpr.Update(objectExpression, arguments) + : null; case BinaryExpression binaryExpr: - return IsEvaluatable(binaryExpr.Left) && IsEvaluatable(binaryExpr.Right); + return GetEvaluatableExpression(binaryExpr.Left) is { } left && GetEvaluatableExpression(binaryExpr.Right) is { } right + ? binaryExpr.Update(left, binaryExpr.Conversion, right) + : null; case UnaryExpression unaryExpr: - return IsEvaluatable(unaryExpr.Operand); + return GetEvaluatableExpression(unaryExpr.Operand); case ConditionalExpression condExpr: - return IsEvaluatable(condExpr.Test) && IsEvaluatable(condExpr.IfTrue) && IsEvaluatable(condExpr.IfFalse); + return + GetEvaluatableExpression(condExpr.Test) is { } test + && GetEvaluatableExpression(condExpr.IfTrue) is { } ifTrue + && GetEvaluatableExpression(condExpr.IfFalse) is { } ifFalse + ? condExpr.Update(test, ifTrue, ifFalse) + : null; default: - return true; + return expr; } } From 5b83453e1397b43f6c38b5480ebf3c29b59d7aaf Mon Sep 17 00:00:00 2001 From: Vincent Spaa Date: Fri, 28 Feb 2025 11:08:10 +0100 Subject: [PATCH 2/4] Adds JetBrains' .idea folder to gitignore --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index fd2d83a..673f996 100644 --- a/.gitignore +++ b/.gitignore @@ -12,4 +12,5 @@ dist *.user .env *.xmldocs.xml -**/CONDUCTORSHARP_HEALTH.json \ No newline at end of file +**/CONDUCTORSHARP_HEALTH.json +.idea From 2bc0dc8d98a2764b6ac4dacced474cbda6eb3c69 Mon Sep 17 00:00:00 2001 From: Vincent Spaa Date: Fri, 28 Feb 2025 11:08:58 +0100 Subject: [PATCH 3/4] Marks the pre-commit hook as executable on Linux systems --- .husky/pre-commit | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 .husky/pre-commit diff --git a/.husky/pre-commit b/.husky/pre-commit old mode 100644 new mode 100755 From 9bb2dfc32eaa38110bc02c06b638f31abe82e75a Mon Sep 17 00:00:00 2001 From: Vincent Spaa Date: Fri, 28 Feb 2025 12:04:17 +0100 Subject: [PATCH 4/4] Adds a test case Fixes issue where some expressions in the tree are skipped Makes the selection of workflow-accessor-to-string-conversion more selective to prevent further method calls on top of it to work --- .../Util/ExpressionUtil.cs | 33 +++++++++----- .../ConductorSharp.Engine.Tests.csproj | 1 + .../Integration/WorkflowBuilderTests.cs | 9 ++++ .../Workflows/NonEvaluatableWorkflow.cs | 5 ++- .../Samples/Workflows/StaticFormatter.cs | 43 +++++++++++++++++++ .../Samples/Workflows/StaticFormatter.json | 26 +++++++++++ 6 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 test/ConductorSharp.Engine.Tests/Samples/Workflows/StaticFormatter.cs create mode 100644 test/ConductorSharp.Engine.Tests/Samples/Workflows/StaticFormatter.json diff --git a/src/ConductorSharp.Engine/Util/ExpressionUtil.cs b/src/ConductorSharp.Engine/Util/ExpressionUtil.cs index cebca54..3f8554a 100644 --- a/src/ConductorSharp.Engine/Util/ExpressionUtil.cs +++ b/src/ConductorSharp.Engine/Util/ExpressionUtil.cs @@ -75,12 +75,16 @@ private static object EvaluateExpression(Expression expr) => ? Expression.Lambda(evaluatableExpression).Compile().DynamicInvoke() : throw new NonEvaluatableExpressionException(expr); - private static Expression GetEvaluatableExpression(Expression expr) + private static Expression GetEvaluatableExpression(Expression expr, Expression parentExpr = null) { switch (expr) { case MemberExpression memExpr: - if (ShouldCompileToJsonPathExpression(expr)) + bool shouldCompileToJsonPath = ShouldCompileToJsonPathExpression(expr); + if (shouldCompileToJsonPath && parentExpr is MethodCallExpression methodCallExpression && methodCallExpression.Object == expr) + return null; + + if (shouldCompileToJsonPath) return Expression.Constant(CreateExpressionString(expr), typeof(string)); if ( @@ -96,23 +100,30 @@ private static Expression GetEvaluatableExpression(Expression expr) if (memExpr.Member is PropertyInfo propertyInfo && propertyInfo.DeclaringType == typeof(Type)) return memExpr; - return GetEvaluatableExpression(memExpr.Expression); + var subExpr = GetEvaluatableExpression(memExpr.Expression, parentExpr); + return memExpr.Expression is null || subExpr is not null ? memExpr.Update(subExpr) : null; case MethodCallExpression methodExpr: - var arguments = methodExpr.Arguments.Select(GetEvaluatableExpression).ToArray(); - return GetEvaluatableExpression(methodExpr.Object) is { } objectExpression && arguments.Length == methodExpr.Arguments.Count - ? methodExpr.Update(objectExpression, arguments) + var argumentExpressions = methodExpr + .Arguments.Select(arg => GetEvaluatableExpression(arg, methodExpr)) + .Where(argExpr => argExpr is not null) + .ToArray(); + var objectExpression = GetEvaluatableExpression(methodExpr.Object, methodExpr); + return (methodExpr.Object is null || objectExpression is not null) && argumentExpressions.Length == methodExpr.Arguments.Count + ? methodExpr.Update(objectExpression, argumentExpressions) : null; case BinaryExpression binaryExpr: - return GetEvaluatableExpression(binaryExpr.Left) is { } left && GetEvaluatableExpression(binaryExpr.Right) is { } right + return + GetEvaluatableExpression(binaryExpr.Left, binaryExpr) is { } left + && GetEvaluatableExpression(binaryExpr.Right, binaryExpr) is { } right ? binaryExpr.Update(left, binaryExpr.Conversion, right) : null; case UnaryExpression unaryExpr: - return GetEvaluatableExpression(unaryExpr.Operand); + return unaryExpr.Update(GetEvaluatableExpression(unaryExpr.Operand, unaryExpr)); case ConditionalExpression condExpr: return - GetEvaluatableExpression(condExpr.Test) is { } test - && GetEvaluatableExpression(condExpr.IfTrue) is { } ifTrue - && GetEvaluatableExpression(condExpr.IfFalse) is { } ifFalse + GetEvaluatableExpression(condExpr.Test, condExpr) is { } test + && GetEvaluatableExpression(condExpr.IfTrue, condExpr) is { } ifTrue + && GetEvaluatableExpression(condExpr.IfFalse, condExpr) is { } ifFalse ? condExpr.Update(test, ifTrue, ifFalse) : null; default: diff --git a/test/ConductorSharp.Engine.Tests/ConductorSharp.Engine.Tests.csproj b/test/ConductorSharp.Engine.Tests/ConductorSharp.Engine.Tests.csproj index 87ec8a0..f58b0ef 100644 --- a/test/ConductorSharp.Engine.Tests/ConductorSharp.Engine.Tests.csproj +++ b/test/ConductorSharp.Engine.Tests/ConductorSharp.Engine.Tests.csproj @@ -44,6 +44,7 @@ + diff --git a/test/ConductorSharp.Engine.Tests/Integration/WorkflowBuilderTests.cs b/test/ConductorSharp.Engine.Tests/Integration/WorkflowBuilderTests.cs index 727affc..a1e60a1 100644 --- a/test/ConductorSharp.Engine.Tests/Integration/WorkflowBuilderTests.cs +++ b/test/ConductorSharp.Engine.Tests/Integration/WorkflowBuilderTests.cs @@ -271,6 +271,15 @@ public void BuilderReturnsCorrectDefinitionDictionaryInitializationWorkflow() Assert.Equal(expectedDefinition, definition); } + [Fact] + public void BuilderReturnsCorrectDefinitionStaticFormatter() + { + var definition = GetDefinitionFromWorkflow(); + var expectedDefinition = EmbeddedFileHelper.GetLinesFromEmbeddedFile("~/Samples/Workflows/StaticFormatter.json"); + + Assert.Equal(expectedDefinition, definition); + } + private static string GetDefinitionFromWorkflow() where TWorkflow : IConfigurableWorkflow { diff --git a/test/ConductorSharp.Engine.Tests/Samples/Workflows/NonEvaluatableWorkflow.cs b/test/ConductorSharp.Engine.Tests/Samples/Workflows/NonEvaluatableWorkflow.cs index f8ba3ee..99ee0e6 100644 --- a/test/ConductorSharp.Engine.Tests/Samples/Workflows/NonEvaluatableWorkflow.cs +++ b/test/ConductorSharp.Engine.Tests/Samples/Workflows/NonEvaluatableWorkflow.cs @@ -20,13 +20,14 @@ public class NonEvaluatableWorkflow : Workflow builder - ) : base(builder) { } + ) + : base(builder) { } public override void BuildDefinition() { _builder.AddTask(wf => wf.GetCustomer, wf => new() { CustomerId = wf.WorkflowInput.Input }); - _builder.AddTask(wf => wf.PrepareEmail, wf => new() { Address = $"{wf.GetCustomer.Output.Address}".ToUpperInvariant(), }); + _builder.AddTask(wf => wf.PrepareEmail, wf => new() { Address = wf.GetCustomer.Output.Address.ToString().ToUpperInvariant(), }); } } } diff --git a/test/ConductorSharp.Engine.Tests/Samples/Workflows/StaticFormatter.cs b/test/ConductorSharp.Engine.Tests/Samples/Workflows/StaticFormatter.cs new file mode 100644 index 0000000..b171e22 --- /dev/null +++ b/test/ConductorSharp.Engine.Tests/Samples/Workflows/StaticFormatter.cs @@ -0,0 +1,43 @@ +namespace ConductorSharp.Engine.Tests.Samples.Workflows +{ + public class StaticFormatterInput : WorkflowInput + { + public string FirstName { get; set; } + public string LastName { get; set; } + } + + public class StaticFormatterOutput : WorkflowOutput + { + public object EmailBody { get; set; } + } + + public class StaticFormatter : Workflow + { + public StaticFormatter(WorkflowDefinitionBuilder builder) + : base(builder) { } + + public EmailPrepareV1 EmailPrepare { get; set; } + + public override void BuildDefinition() + { + _builder.AddTask( + wf => wf.EmailPrepare, + wf => + new() + { + Address = SmartEmailConverter.Format(wf.Input.FirstName, wf.Input.LastName), + Name = $"Workflow name: {NamingUtil.NameOf()}" + } + ); + + _builder.SetOutput(a => new() { EmailBody = a.EmailPrepare.Output.EmailBody }); + } + + public static EmailConverter SmartEmailConverter => new EmailConverter(); + + public class EmailConverter + { + public string Format(string firstName, string lastName) => $"{firstName}.{lastName}@example.com"; + } + } +} diff --git a/test/ConductorSharp.Engine.Tests/Samples/Workflows/StaticFormatter.json b/test/ConductorSharp.Engine.Tests/Samples/Workflows/StaticFormatter.json new file mode 100644 index 0000000..da20430 --- /dev/null +++ b/test/ConductorSharp.Engine.Tests/Samples/Workflows/StaticFormatter.json @@ -0,0 +1,26 @@ +{ + "name": "static_formatter", + "version": 1, + "tasks": [ + { + "name": "EMAIL_prepare", + "taskReferenceName": "email_prepare", + "inputParameters": { + "address": "${workflow.input.first_name}.${workflow.input.last_name}@example.com", + "name": "Workflow name: TEST_StringInterpolation" + }, + "type": "SIMPLE", + "optional": false, + "workflowTaskType": "SIMPLE" + } + ], + "inputParameters": [ + "first_name", + "last_name" + ], + "outputParameters": { + "email_body": "${email_prepare.output.email_body}" + }, + "schemaVersion": 2, + "timeoutSeconds": 0 +} \ No newline at end of file