Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile all expressions at once when validating. #268

Merged
merged 6 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 66 additions & 1 deletion src/Test/TestCases.Workflows/ExpressionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Activities.Validation;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Xunit;

namespace TestCases.Workflows;
Expand Down Expand Up @@ -47,7 +48,8 @@ static ExpressionTests()
// There's no programmatic way (that I know of) to add assembly references when creating workflows like in these tests.
// Adding the custom assembly directly to the expression validator to simulate XAML reference.
// The null is for testing purposes.
VbExpressionValidator.Instance = new VbExpressionValidator(new() { typeof(ClassWithCollectionProperties).Assembly, null });
VbExpressionValidator.Instance.AddRequiredAssembly(typeof(ClassWithCollectionProperties).Assembly);
CSharpExpressionValidator.Instance.AddRequiredAssembly(typeof(ClassWithCollectionProperties).Assembly);
}

[Theory]
Expand Down Expand Up @@ -424,6 +426,69 @@ public void CSRoslynValidator_ValidatesMoreThan16Arguments()
var result = ActivityValidationServices.Validate(sequence, _useValidator);
result.Errors.ShouldBeEmpty();
}
[Fact]
public void VB_Multithreaded_NoError()
{
var activities = new List<Activity>();
var tasks = new List<Task>();
var results = new List<ValidationResults>();
for (var i = 0; i < 20; i++)
{
var seq = new Sequence();
seq.Variables.Add(new Variable<int>("sum"));
for (var j = 0; j < 1000; j++)
{
seq.Activities.Add(new Assign
{
To = new OutArgument<int>(new VisualBasicReference<int>("sum")),
Value = new InArgument<int>(new VisualBasicValue<int>($"sum + {j}"))
});
}
activities.Add(seq);
}
foreach (var activity in activities)
{
tasks.Add(Task.Run(() =>
{
results.Add(ActivityValidationServices.Validate(activity, _useValidator));
}));
}
Task.WaitAll(tasks.ToArray());

results.All(r => !r.Errors.Any() && !r.Warnings.Any()).ShouldBeTrue();
}

[Fact]
public void CS_Multithreaded_NoError()
{
var activities = new List<Activity>();
var tasks = new List<Task>();
var results = new List<ValidationResults>();
for (var i = 0; i < 20; i++)
{
var seq = new Sequence();
seq.Variables.Add(new Variable<int>("sum"));
for (var j = 0; j < 1000; j++)
{
seq.Activities.Add(new Assign
{
To = new OutArgument<int>(new CSharpReference<int>("sum")),
Value = new InArgument<int>(new CSharpValue<int>($"sum + {j}"))
});
}
activities.Add(seq);
}
foreach (var activity in activities)
{
tasks.Add(Task.Run(() =>
{
results.Add(ActivityValidationServices.Validate(activity, _useValidator));
aoltean16 marked this conversation as resolved.
Show resolved Hide resolved
}));
}
Task.WaitAll(tasks.ToArray());

results.All(r => !r.Errors.Any() && !r.Warnings.Any()).ShouldBeTrue();
}

[Fact]
public void VBReferenceTypeIsChecked()
Expand Down
76 changes: 76 additions & 0 deletions src/Test/TestCases.Workflows/ValidationExtensionsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
using Microsoft.CSharp.Activities;
using Shouldly;
using System;
using System.Activities;
using System.Activities.Statements;
using System.Activities.Validation;
using System.Collections.Generic;
using System.Linq;
using Xunit;

namespace TestCases.Workflows;

public class ValidationExtensionsTests
{
private readonly ValidationSettings _useValidator = new() { ForceExpressionCache = false };

[Fact]
public void OnlyOneInstanceOfExtensionTypeIsAdded()
{
var seq = new Sequence();
for (var j = 0; j < 10000; j++)
{
seq.Activities.Add(new ActivityWithValidationExtension());
}

var result = ActivityValidationServices.Validate(seq, _useValidator);
result.Errors.Count.ShouldBe(1);
result.Errors.First().Message.ShouldContain(nameof(MockValidationExtension));
}

[Fact]
public void ValidationErrorsAreConcatenated()
{
var seq = new Sequence()
{
Activities =
{
new ActivityWithValidationExtension(),
new ActivityWithValidationError(),
new WriteLine { Text = new InArgument<string>(new CSharpValue<string>("var1")) }
}
};

var result = ActivityValidationServices.Validate(seq, _useValidator);
result.Errors.Count.ShouldBe(3);
result.Errors.ShouldContain(error => error.Message.Contains(nameof(ActivityWithValidationError)));
result.Errors.ShouldContain(error => error.Message.Contains(nameof(MockValidationExtension)));
result.Errors.ShouldContain(error => error.Message.Contains("The name 'var1' does not exist in the current context"));
}

class ActivityWithValidationError : CodeActivity
{
protected override void Execute(CodeActivityContext context) => throw new NotImplementedException();

protected override void CacheMetadata(CodeActivityMetadata metadata) => metadata.AddValidationError(nameof(ActivityWithValidationError));
}

class ActivityWithValidationExtension : CodeActivity
{
protected override void Execute(CodeActivityContext context) => throw new NotImplementedException();

protected override void CacheMetadata(CodeActivityMetadata metadata)
{
if (metadata.Environment.IsValidating)
{
metadata.Environment.Extensions.GetOrAdd(() => new MockValidationExtension());
}
}
}

class MockValidationExtension : IValidationExtension
{
public IEnumerable<ValidationError> PostValidate(Activity activity) =>
new List<ValidationError>() { new ValidationError(nameof(MockValidationExtension)) };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ internal sealed class ActivityLocationReferenceEnvironment : LocationReferenceEn
private Dictionary<string, LocationReference> _declarations;
private List<LocationReference> _unnamedDeclarations;

public ActivityLocationReferenceEnvironment() { }
public ActivityLocationReferenceEnvironment()
{
Extensions = new();
}

public ActivityLocationReferenceEnvironment(LocationReferenceEnvironment parent)
{
Expand All @@ -22,6 +25,7 @@ public ActivityLocationReferenceEnvironment(LocationReferenceEnvironment parent)
CompileExpressions = parent.CompileExpressions;
IsValidating = parent.IsValidating;
InternalRoot = parent.Root;
Extensions = parent.Extensions;
}
}

Expand Down
71 changes: 71 additions & 0 deletions src/UiPath.Workflow.Runtime/EnvironmentExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// This file is part of Core WF which is licensed under the MIT license.
// See LICENSE file in the project root for full license information.

namespace System.Activities
{
internal class EnvironmentExtensions
{
private readonly Dictionary<Type, object> _extensions = new();

/// <summary>
/// Gets the specified extension.
/// If the extension does not exist,
/// it will invoke the <paramref name="createExtensionFactory"/> parameter
/// </summary>
/// <typeparam name="TExtension">The type of the extension</typeparam>
/// <param name="createExtensionFactory">The factory to create the extension</param>
/// <exception cref="ArgumentNullException"></exception>
public TExtension GetOrAdd<TExtension>(Func<TExtension> createExtensionFactory)
where TExtension : class
{
var type = typeof(TExtension);
if (_extensions.TryGetValue(type, out object extension))
{
return extension as TExtension;
}

return CreateAndAdd();

TExtension CreateAndAdd()
{
var extension = createExtensionFactory();
if (extension is null)
throw new ArgumentNullException(nameof(extension));

_extensions[type] = extension;
return extension;
}
}

/// <summary>
/// Retrieves the extension registered for the given type
/// or null otherwise
/// </summary>
/// <typeparam name="T">The type of the extension.</typeparam>
public T Get<T>() where T : class
{
if (_extensions.TryGetValue(typeof(T), out object extension))
return extension as T;
return null;
}

/// <summary>
/// Adds the specified extension to the list.
/// The extension is treated as a singleton,
/// so if a second extension with the same type is added, it will
/// throw an <see cref="InvalidOperationException"/>
/// </summary>
/// <typeparam name="TExtension">The type of the extension</typeparam>
/// <param name="extension">The extension</param>
/// <exception cref="InvalidOperationException"></exception>
public void Add<TExtension>(TExtension extension) where TExtension : class
{
if (_extensions.ContainsKey(typeof(TExtension)))
throw new InvalidOperationException($"Service '{typeof(TExtension).FullName}' already exists");

_extensions[typeof(TExtension)] = extension;
}

internal IReadOnlyCollection<object> All => _extensions.Values;
}
}
2 changes: 2 additions & 0 deletions src/UiPath.Workflow.Runtime/LocationReferenceEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ public abstract class LocationReferenceEnvironment
/// </summary>
internal bool IsValidating { get; set; }

internal EnvironmentExtensions Extensions { get; set; }

public abstract Activity Root { get; }

public LocationReferenceEnvironment Parent { get; protected set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ namespace System.Activities.Validation;

public static class ActivityValidationServices
{
internal static readonly ReadOnlyCollection<Activity> EmptyChildren = new(Array.Empty<Activity>());
private static readonly ValidationSettings defaultSettings = new();

internal static readonly ReadOnlyCollection<Activity> EmptyChildren = new(Array.Empty<Activity>());
internal static ReadOnlyCollection<ValidationError> EmptyValidationErrors = new(new List<ValidationError>(0));

public static ValidationResults Validate(Activity toValidate) => Validate(toValidate, defaultSettings);
Expand Down Expand Up @@ -245,7 +246,7 @@ private static string GenerateExceptionString(IList<ValidationError> validationE
return exceptionString;
}

static internal string GenerateValidationErrorPrefix(Activity toValidate, ActivityUtilities.ActivityCallStack parentChain, ProcessActivityTreeOptions options, out Activity source)
internal static string GenerateValidationErrorPrefix(Activity toValidate, ActivityUtilities.ActivityCallStack parentChain, ProcessActivityTreeOptions options, out Activity source)
{
bool parentVisible = true;
string prefix = "";
Expand Down Expand Up @@ -451,9 +452,12 @@ internal ValidationResults InternalValidate()
{
// We want to add the CacheMetadata errors to our errors collection
ActivityUtilities.CacheRootMetadata(_rootToValidate, _environment, _options, new ActivityUtilities.ProcessActivityCallback(ValidateElement), ref _errors);
}
}

return new ValidationResults(_errors);
return new ValidationResults(GetValidationExtensionResults().Concat(_errors ?? Array.Empty<ValidationError>()).ToList());

IEnumerable<ValidationError> GetValidationExtensionResults() =>
_environment.Extensions.All.OfType<IValidationExtension>().SelectMany(validationExtension => validationExtension.PostValidate(_rootToValidate));
}

private void ValidateElement(ActivityUtilities.ChildActivity childActivity, ActivityUtilities.ActivityCallStack parentChain)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace System.Activities.Validation
{
internal interface IValidationExtension
{
IEnumerable<ValidationError> PostValidate(Activity activity);
}
}
Loading