From ec866ad381cb62d665ee66cd45942dfe0d84646b Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 23 Oct 2023 12:28:03 +0200 Subject: [PATCH] Prevent native property name collision This change ensures that the WorkflowDefinitionActivity supports custom input definitions even if they use the same name as properties already present on the activity, such as WorkflowDefinitionId. --- .../PropertyNameHelper.cs | 21 +++ .../WorkflowDefinitionActivity.cs | 160 +++++++++--------- .../WorkflowDefinitionActivityProvider.cs | 22 ++- 3 files changed, 112 insertions(+), 91 deletions(-) create mode 100644 src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/PropertyNameHelper.cs diff --git a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/PropertyNameHelper.cs b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/PropertyNameHelper.cs new file mode 100644 index 000000000..5c0ba53f4 --- /dev/null +++ b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/PropertyNameHelper.cs @@ -0,0 +1,21 @@ +namespace Elsa.Workflows.Management.Activities.WorkflowDefinitionActivity; + +internal static class PropertyNameHelper +{ + public static string GetSafePropertyName(Type type, string name) + { + var isReservedName = IsReservedName(type, name); + return isReservedName ? PrefixPropertyName(name) : name; + } + + public static string GetUnsafePropertyName(Type type, string name) + { + var unsafeName = RemovePropertyNamePrefix(name); + var isReservedName = IsReservedName(type, unsafeName); + return isReservedName ? unsafeName : name; + } + + private static string PrefixPropertyName(string name) => $"_{name}"; + private static string RemovePropertyNamePrefix(string name) => name.TrimStart('_'); + private static bool IsReservedName(Type type, string name) => type.GetProperty(name) != null; +} \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivity.cs b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivity.cs index 5039991d2..3ad9166b1 100644 --- a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivity.cs +++ b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivity.cs @@ -48,71 +48,6 @@ public class WorkflowDefinitionActivity : Composite, IInitializable await context.ScheduleActivityAsync(Root, OnChildCompletedAsync); } - private void CopyInputOutputToVariables(ActivityExecutionContext context) - { - foreach (var inputDescriptor in context.ActivityDescriptor.Inputs) - { - var input = SyntheticProperties.TryGetValue(inputDescriptor.Name, out var inputValue) ? (Input?)inputValue : default; - var evaluatedExpression = input != null ? context.Get(input.MemoryBlockReference()) : default; - - // Create a local scope variable for each input property. - var variable = new Variable - { - Id = inputDescriptor.Name, - Name = inputDescriptor.Name, - StorageDriverType = inputDescriptor.StorageDriverType - }; - - context.ExpressionExecutionContext.Memory.Declare(variable); - variable.Set(context, evaluatedExpression); - } - - foreach (var outputDescriptor in context.ActivityDescriptor.Outputs) - { - // Create a local scope variable for each output property. - var variable = new Variable - { - Id = outputDescriptor.Name, - Name = outputDescriptor.Name - }; - - context.ExpressionExecutionContext.Memory.Declare(variable); - } - } - - private void DeclareInputOutputAsVariables(InitializationContext context) - { - var activityRegistry = context.ServiceProvider.GetRequiredService(); - var activityDescriptor = activityRegistry.Find(Type, Version)!; - - // Declare input variables. - foreach (var inputDescriptor in activityDescriptor.Inputs) - { - // Create a local scope variable for each input property. - var variable = new Variable(inputDescriptor.Name) - { - Id = inputDescriptor.Name, - Name = inputDescriptor.Name, - StorageDriverType = inputDescriptor.StorageDriverType - }; - - Variables.Declare(variable); - } - - // Declare output variables. - foreach (var outputDescriptor in activityDescriptor.Outputs) - { - // Create a local scope variable for each output property. - var variable = new Variable(outputDescriptor.Name) - { - Id = outputDescriptor.Name, - Name = outputDescriptor.Name - }; - - Variables.Declare(variable); - } - } - private async ValueTask OnChildCompletedAsync(ActivityCompletedContext context) { var targetContext = context.TargetContext; @@ -150,10 +85,66 @@ public class WorkflowDefinitionActivity : Composite, IInitializable await targetContext.CompleteActivityAsync(completeCompositeSignal?.Value); } - async ValueTask IInitializable.InitializeAsync(InitializationContext context) + private void CopyInputOutputToVariables(ActivityExecutionContext context) + { + var serviceProvider = context.GetRequiredService(); + + DeclareInputAsVariables(serviceProvider, (descriptor, variable) => + { + var inputName = descriptor.Name; + var input = SyntheticProperties.TryGetValue(inputName, out var inputValue) ? (Input?)inputValue : default; + var evaluatedExpression = input != null ? context.Get(input.MemoryBlockReference()) : default; + + context.ExpressionExecutionContext.Memory.Declare(variable); + variable.Set(context, evaluatedExpression); + }); + + DeclareOutputAsVariables(serviceProvider, (descriptor, variable) => context.ExpressionExecutionContext.Memory.Declare(variable)); + } + + private void DeclareInputAsVariables(IServiceProvider serviceProvider, Action configureVariable) + { + var activityRegistry = serviceProvider.GetRequiredService(); + var activityDescriptor = activityRegistry.Find(Type, Version)!; + + foreach (var inputDescriptor in activityDescriptor.Inputs) + { + var inputName = inputDescriptor.Name; + var unsafeInputName = PropertyNameHelper.GetUnsafePropertyName(typeof(WorkflowDefinitionActivity), inputName); + + var variable = new Variable + { + Id = unsafeInputName, + Name = unsafeInputName, + StorageDriverType = inputDescriptor.StorageDriverType + }; + + configureVariable(inputDescriptor, variable); + } + } + + private void DeclareOutputAsVariables(IServiceProvider serviceProvider, Action configureVariable) + { + var activityRegistry = serviceProvider.GetRequiredService(); + var activityDescriptor = activityRegistry.Find(Type, Version)!; + + foreach (var outputDescriptor in activityDescriptor.Outputs) + { + var outputName = outputDescriptor.Name; + var unsafeOutputName = PropertyNameHelper.GetUnsafePropertyName(typeof(WorkflowDefinitionActivity), outputName); + + var variable = new Variable + { + Id = unsafeOutputName, + Name = unsafeOutputName + }; + + configureVariable(outputDescriptor, variable); + } + } + + private async Task FindWorkflowDefinitionAsync(IServiceProvider serviceProvider, CancellationToken cancellationToken) { - var serviceProvider = context.ServiceProvider; - var cancellationToken = context.CancellationToken; var workflowDefinitionStore = serviceProvider.GetRequiredService(); var filter = new WorkflowDefinitionFilter { DefinitionId = WorkflowDefinitionId }; @@ -162,29 +153,32 @@ public class WorkflowDefinitionActivity : Composite, IInitializable else filter.VersionOptions = VersionOptions.SpecificVersion(Version); - var workflowDefinition = await workflowDefinitionStore.FindAsync(filter, cancellationToken); + var workflowDefinition = + await workflowDefinitionStore.FindAsync(filter, cancellationToken) + ?? (await workflowDefinitionStore.FindAsync(new WorkflowDefinitionFilter { DefinitionId = WorkflowDefinitionId, VersionOptions = VersionOptions.Published }, cancellationToken) + ?? await workflowDefinitionStore.FindAsync(new WorkflowDefinitionFilter { DefinitionId = WorkflowDefinitionId, VersionOptions = VersionOptions.Latest }, cancellationToken)); + + return workflowDefinition; + } + + async ValueTask IInitializable.InitializeAsync(InitializationContext context) + { + var serviceProvider = context.ServiceProvider; + var cancellationToken = context.CancellationToken; + var workflowDefinition = await FindWorkflowDefinitionAsync(serviceProvider, cancellationToken); if (workflowDefinition == null) - { - // Find the latest published version. - workflowDefinition = await workflowDefinitionStore.FindAsync(new WorkflowDefinitionFilter { DefinitionId = WorkflowDefinitionId, VersionOptions = VersionOptions.Published }, cancellationToken); - - if (workflowDefinition == null) - { - // Find the latest version. - workflowDefinition = await workflowDefinitionStore.FindAsync(new WorkflowDefinitionFilter { DefinitionId = WorkflowDefinitionId, VersionOptions = VersionOptions.Latest }, cancellationToken); - } - - if (workflowDefinition == null) - throw new Exception($"Could not find workflow definition with ID {WorkflowDefinitionId}."); - } + throw new Exception($"Could not find workflow definition with ID {WorkflowDefinitionId}."); // Construct the root activity stored in the activity definitions. var materializer = serviceProvider.GetRequiredService(); var root = await materializer.MaterializeAsync(workflowDefinition, cancellationToken); - DeclareInputOutputAsVariables(context); + // Declare input and output variables. + DeclareInputAsVariables(serviceProvider, (inputDescriptor, variable) => Variables.Declare(variable)); + DeclareOutputAsVariables(serviceProvider, (outputDescriptor, variable) => Variables.Declare(variable)); + // Set the root activity. Root = root; } } \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityProvider.cs b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityProvider.cs index 73f7bfc95..2cac232c9 100644 --- a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityProvider.cs +++ b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityProvider.cs @@ -114,14 +114,16 @@ public class WorkflowDefinitionActivityProvider : IActivityProvider var inputs = definition.Inputs.Select(inputDefinition => { var nakedType = inputDefinition.Type; + var inputName = inputDefinition.Name; + var safeInputName = PropertyNameHelper.GetSafePropertyName(typeof(WorkflowDefinitionActivity), inputName); return new InputDescriptor { Type = nakedType, IsWrapped = true, - ValueGetter = activity => activity.SyntheticProperties.GetValueOrDefault(inputDefinition.Name), - ValueSetter = (activity, value) => activity.SyntheticProperties[inputDefinition.Name] = value!, - Name = inputDefinition.Name, + ValueGetter = activity => activity.SyntheticProperties.GetValueOrDefault(safeInputName), + ValueSetter = (activity, value) => activity.SyntheticProperties[safeInputName] = value!, + Name = safeInputName, DisplayName = inputDefinition.DisplayName, Description = inputDefinition.Description, Category = inputDefinition.Category, @@ -135,20 +137,24 @@ public class WorkflowDefinitionActivityProvider : IActivityProvider yield return input; } - private static IEnumerable DescribeOutputs(WorkflowDefinition definition) => - definition.Outputs.Select(outputDefinition => + private static IEnumerable DescribeOutputs(WorkflowDefinition definition) + { + return definition.Outputs.Select(outputDefinition => { var nakedType = outputDefinition.Type; + var outputName = outputDefinition.Name; + var safeOutputName = PropertyNameHelper.GetSafePropertyName(typeof(WorkflowDefinitionActivity), outputName); return new OutputDescriptor { Type = nakedType, - ValueGetter = activity => activity.SyntheticProperties.GetValueOrDefault(outputDefinition.Name), - ValueSetter = (activity, value) => activity.SyntheticProperties[outputDefinition.Name] = value!, - Name = outputDefinition.Name, + ValueGetter = activity => activity.SyntheticProperties.GetValueOrDefault(safeOutputName), + ValueSetter = (activity, value) => activity.SyntheticProperties[safeOutputName] = value!, + Name = safeOutputName, DisplayName = outputDefinition.DisplayName, Description = outputDefinition.Description, IsSynthetic = true }; }); + } } \ No newline at end of file