Bugfix: error thrown in ReadSyntheticInputs
Direct cause: typeName was missing in synthetic input property Root cause: worklfow used as activity defined input with reserved name 'Metadata' Why fix was needed? Because the uncaught exception was preventing other workflows from being published. Even though this exception occurs, it should not prevent publishing other workflows. Solution introduced here: - Collect exceptions that prevent an activity from being in the expected state. In this case, we expected all syntehtic input properties to be valid, but they are not. - Return the list of exceptions to the caller, along with an IActivity instance that does not throw exceptions - The caller, in this case ActivityJsonConverter, can log any exceptions, whilst not breaking the loop with uncaught exceptions.
This commit is contained in:
parent
05d40e3a2a
commit
4cd2c4cda6
|
|
@ -202,8 +202,9 @@ public class ElsaScriptCompiler(IActivityRegistryLookupService activityRegistryL
|
|||
else
|
||||
{
|
||||
// No positional arguments, use default constructor
|
||||
var activityConstructorContext = new ActivityConstructorContext(activityDescriptor, ActivityActivator.Create);
|
||||
activity = activityDescriptor.Constructor(activityConstructorContext);
|
||||
var activityConstructorContext = new ActivityConstructorContext(activityDescriptor, (t) => new(ActivityActivator.Create(t)));
|
||||
var activityConstructionResult = activityDescriptor.Constructor(activityConstructorContext);
|
||||
activity = activityConstructionResult.Activity;
|
||||
}
|
||||
|
||||
// Set named argument properties
|
||||
|
|
|
|||
|
|
@ -6,14 +6,14 @@ namespace Elsa.Workflows;
|
|||
public interface IActivityFactory
|
||||
{
|
||||
[Obsolete("Use the CreateActivity method on the ActivityConstructorContext context itself")]
|
||||
IActivity Create(Type type, ActivityConstructorContext context);
|
||||
ActivityConstructionResult Create(Type type, ActivityConstructorContext context);
|
||||
}
|
||||
|
||||
[Obsolete("Use the CreateActivity method on the ActivityConstructorContext context itself")]
|
||||
public static class ActivityFactoryExtensions
|
||||
{
|
||||
[Obsolete("Use the CreateActivity method on the ActivityConstructorContext context itself")]
|
||||
public static IActivity CreateActivity<T>(this IActivityFactory factory, ActivityConstructorContext context)
|
||||
public static ActivityConstructionResult CreateActivity<T>(this IActivityFactory factory, ActivityConstructorContext context)
|
||||
{
|
||||
return factory.Create(typeof(T), context);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
namespace Elsa.Workflows.Exceptions;
|
||||
|
||||
public sealed class InvalidActivityDescriptorInputException(string message) : Exception(message)
|
||||
{
|
||||
}
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
namespace Elsa.Workflows.Models;
|
||||
|
||||
public class ActivityConstructionResult(IActivity activity, IEnumerable<Exception>? exceptions = null)
|
||||
{
|
||||
public bool HasExceptions => Exceptions.Any();
|
||||
|
||||
public IActivity Activity { get; } = activity;
|
||||
public IEnumerable<Exception> Exceptions { get; } = exceptions ?? [];
|
||||
|
||||
public ActivityConstructionResult<TActivity> Cast<TActivity>() where TActivity : IActivity => new ((TActivity)Activity, Exceptions);
|
||||
}
|
||||
|
||||
public sealed class ActivityConstructionResult<TActivity>(TActivity activity, IEnumerable<Exception>? exceptions = null) : ActivityConstructionResult(activity, exceptions)
|
||||
where TActivity : IActivity
|
||||
{
|
||||
public new TActivity Activity { get; } = activity;
|
||||
}
|
||||
|
|
@ -3,15 +3,17 @@ using System.Text.Json.Nodes;
|
|||
using Elsa.Expressions.Helpers;
|
||||
using Elsa.Extensions;
|
||||
using Elsa.Workflows.Attributes;
|
||||
using Elsa.Workflows.Exceptions;
|
||||
using Elsa.Workflows.Memory;
|
||||
using Humanizer;
|
||||
using Microsoft.Extensions.Logging;
|
||||
|
||||
namespace Elsa.Workflows.Models;
|
||||
|
||||
public record ActivityConstructorContext(ActivityDescriptor ActivityDescriptor, Func<Type, IActivity> ActivityFactory)
|
||||
public record ActivityConstructorContext(ActivityDescriptor ActivityDescriptor, Func<Type, ActivityConstructionResult> ActivityFactory)
|
||||
{
|
||||
public T CreateActivity<T>() where T : IActivity => (T)ActivityFactory(typeof(T));
|
||||
public IActivity CreateActivity(Type type) => ActivityFactory(type);
|
||||
public ActivityConstructionResult<T> CreateActivity<T>() where T : IActivity => ActivityFactory(typeof(T)).Cast<T>();
|
||||
public ActivityConstructionResult CreateActivity(Type type) => ActivityFactory(type);
|
||||
}
|
||||
|
||||
public static class JsonActivityConstructorContextHelper
|
||||
|
|
@ -21,13 +23,15 @@ public static class JsonActivityConstructorContextHelper
|
|||
return new ActivityConstructorContext(activityDescriptor, type => CreateActivity(activityDescriptor, type, element, serializerOptions));
|
||||
}
|
||||
|
||||
public static T CreateActivity<T>(ActivityDescriptor activityDescriptor, JsonElement element, JsonSerializerOptions serializerOptions) where T : IActivity
|
||||
public static ActivityConstructionResult<T> CreateActivity<T>(ActivityDescriptor activityDescriptor, JsonElement element, JsonSerializerOptions serializerOptions) where T : IActivity
|
||||
{
|
||||
return (T)CreateActivity(activityDescriptor, typeof(T), element, serializerOptions);
|
||||
return CreateActivity(activityDescriptor, typeof(T), element, serializerOptions).Cast<T>();
|
||||
}
|
||||
|
||||
public static IActivity CreateActivity(ActivityDescriptor activityDescriptor, Type type, JsonElement element, JsonSerializerOptions serializerOptions)
|
||||
public static ActivityConstructionResult CreateActivity(ActivityDescriptor activityDescriptor, Type type, JsonElement element, JsonSerializerOptions serializerOptions)
|
||||
{
|
||||
var exceptions = new List<Exception>();
|
||||
|
||||
// 1) Grab the raw text
|
||||
var raw = element.GetRawText();
|
||||
|
||||
|
|
@ -56,14 +60,14 @@ public static class JsonActivityConstructorContextHelper
|
|||
composite.Setup();
|
||||
|
||||
// 9) Existing synthetic inputs/outputs routines, using the cleanedElement
|
||||
ReadSyntheticInputs(activityDescriptor, activity, cleanedElement, serializerOptions);
|
||||
ReadSyntheticInputs(activityDescriptor, activity, cleanedElement, serializerOptions, exceptions);
|
||||
ReadSyntheticOutputs(activityDescriptor, activity, cleanedElement);
|
||||
|
||||
// 10) Finally re‑apply those flags
|
||||
activity.SetCanStartWorkflow(canStartWorkflow);
|
||||
activity.SetRunAsynchronously(runAsynchronously);
|
||||
|
||||
return activity;
|
||||
return new(activity, exceptions);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -140,7 +144,7 @@ public static class JsonActivityConstructorContextHelper
|
|||
}
|
||||
}
|
||||
|
||||
private static void ReadSyntheticInputs(ActivityDescriptor activityDescriptor, IActivity activity, JsonElement activityRoot, JsonSerializerOptions options)
|
||||
private static void ReadSyntheticInputs(ActivityDescriptor activityDescriptor, IActivity activity, JsonElement activityRoot, JsonSerializerOptions options, List<Exception> exceptions)
|
||||
{
|
||||
foreach (var inputDescriptor in activityDescriptor.Inputs.Where(x => x.IsSynthetic))
|
||||
{
|
||||
|
|
@ -152,6 +156,15 @@ public static class JsonActivityConstructorContextHelper
|
|||
if (!activityRoot.TryGetProperty(propertyName, out var propertyElement) || propertyElement.ValueKind == JsonValueKind.Null || propertyElement.ValueKind == JsonValueKind.Undefined)
|
||||
continue;
|
||||
|
||||
if(propertyElement.ValueKind == JsonValueKind.Object && !propertyElement.TryGetProperty("typeName", out var typeNameElement))
|
||||
{
|
||||
var exception = new InvalidActivityDescriptorInputException(
|
||||
$"Activity descriptor '{activityDescriptor.Name}' has invalid input property '{propertyName}'; missing required property 'typeName'"
|
||||
);
|
||||
exceptions.Add(exception);
|
||||
continue;
|
||||
}
|
||||
|
||||
var isWrapped = propertyElement.ValueKind == JsonValueKind.Object && propertyElement.GetProperty("typeName").ValueKind != JsonValueKind.Undefined;
|
||||
|
||||
if (isWrapped)
|
||||
|
|
|
|||
|
|
@ -69,7 +69,7 @@ public class ActivityDescriptor
|
|||
/// Instantiates a concrete instance of an <see cref="IActivity"/>.
|
||||
/// </summary>
|
||||
[JsonIgnore]
|
||||
public Func<ActivityConstructorContext, IActivity> Constructor { get; set; } = null!;
|
||||
public Func<ActivityConstructorContext, ActivityConstructionResult> Constructor { get; set; } = null!;
|
||||
|
||||
/// <summary>
|
||||
/// The kind of activity.
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ using Elsa.Workflows.Helpers;
|
|||
using Elsa.Workflows.Models;
|
||||
using Elsa.Workflows.Serialization.Helpers;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging;
|
||||
|
||||
namespace Elsa.Workflows.Serialization.Converters;
|
||||
|
||||
|
|
@ -38,13 +39,16 @@ public class ActivityJsonConverter(
|
|||
}
|
||||
|
||||
var clonedOptions = GetClonedOptions(options);
|
||||
var logger = serviceProvider.GetRequiredService<ILogger<ActivityJsonConverter>>();
|
||||
|
||||
// If the activity type is not found, create a NotFoundActivity instead.
|
||||
if (activityDescriptor == null)
|
||||
{
|
||||
var notFoundActivityDescriptor = activityRegistry.Find<NotFoundActivity>()!;
|
||||
var notFoundActivity = JsonActivityConstructorContextHelper.CreateActivity<NotFoundActivity>(notFoundActivityDescriptor, activityRoot, clonedOptions);
|
||||
var notFoundActivityResult = JsonActivityConstructorContextHelper.CreateActivity<NotFoundActivity>(notFoundActivityDescriptor, activityRoot, clonedOptions);
|
||||
LogExceptionsIfAny(notFoundActivityResult);
|
||||
|
||||
var notFoundActivity = notFoundActivityResult.Activity;
|
||||
notFoundActivity.Type = notFoundActivityTypeName;
|
||||
notFoundActivity.Version = 1;
|
||||
notFoundActivity.MissingTypeName = activityTypeName;
|
||||
|
|
@ -56,8 +60,20 @@ public class ActivityJsonConverter(
|
|||
}
|
||||
|
||||
var context = JsonActivityConstructorContextHelper.Create(activityDescriptor, activityRoot, clonedOptions);
|
||||
var activity = activityDescriptor.Constructor(context);
|
||||
return activity;
|
||||
var activityResult = activityDescriptor.Constructor(context);
|
||||
LogExceptionsIfAny(activityResult);
|
||||
|
||||
return activityResult.Activity;
|
||||
}
|
||||
|
||||
void LogExceptionsIfAny(ActivityConstructionResult result)
|
||||
{
|
||||
if (!result.HasExceptions)
|
||||
return;
|
||||
|
||||
var logger = serviceProvider.GetRequiredService<ILogger<ActivityJsonConverter>>();
|
||||
foreach (var exception in result.Exceptions)
|
||||
logger.LogWarning("An exception was thrown while constructing activity with id '{activityId}': {Message}", result.Activity.Id, exception.Message);
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
|
|
|
|||
|
|
@ -92,9 +92,9 @@ public class ActivityDescriber(IPropertyDefaultValueResolver defaultValueResolve
|
|||
},
|
||||
Constructor = context =>
|
||||
{
|
||||
var activity = context.CreateActivity(activityType);
|
||||
activity.Type = fullTypeName;
|
||||
return activity;
|
||||
var activityResult = context.CreateActivity(activityType);
|
||||
activityResult.Activity.Type = fullTypeName;
|
||||
return activityResult;
|
||||
},
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ namespace Elsa.Workflows;
|
|||
public class ActivityFactory : IActivityFactory
|
||||
{
|
||||
/// <inheritdoc />
|
||||
public IActivity Create(Type type, ActivityConstructorContext context)
|
||||
public ActivityConstructionResult Create(Type type, ActivityConstructorContext context)
|
||||
{
|
||||
return context.CreateActivity(type);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -57,7 +57,8 @@ public class WorkflowDefinitionActivityDescriptorFactory
|
|||
},
|
||||
Constructor = context =>
|
||||
{
|
||||
var activity = context.CreateActivity<WorkflowDefinitionActivity>();
|
||||
var activityResult = context.CreateActivity<WorkflowDefinitionActivity>();
|
||||
var activity = activityResult.Activity;
|
||||
activity.Type = typeName;
|
||||
activity.WorkflowDefinitionId = definition.DefinitionId;
|
||||
activity.WorkflowDefinitionVersionId = definition.Id;
|
||||
|
|
@ -65,7 +66,7 @@ public class WorkflowDefinitionActivityDescriptorFactory
|
|||
activity.LatestAvailablePublishedVersion = latestPublishedDefinition?.Version ?? definition.Version;
|
||||
activity.LatestAvailablePublishedVersionId = latestPublishedDefinition?.Id ?? definition.Id;
|
||||
|
||||
return activity;
|
||||
return activityResult;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -57,12 +57,13 @@ public class HostMethodActivityDescriber(IActivityDescriber activityDescriber) :
|
|||
|
||||
descriptor.Constructor = context =>
|
||||
{
|
||||
var activity = context.CreateActivity<HostMethodActivity>();
|
||||
var activityResult = context.CreateActivity<HostMethodActivity>();
|
||||
var activity = activityResult.Activity;
|
||||
activity.Type = activityTypeName;
|
||||
activity.HostType = hostType;
|
||||
activity.MethodName = methodName;
|
||||
activity.RunAsynchronously ??= descriptor.RunAsynchronously;
|
||||
return activity;
|
||||
return activityResult;
|
||||
};
|
||||
|
||||
descriptor.Inputs.Clear();
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Serialization;
|
||||
using Elsa.Workflows.Exceptions;
|
||||
using Elsa.Workflows.Models;
|
||||
|
||||
namespace Elsa.Workflows.Core.UnitTests.Models;
|
||||
|
|
@ -14,7 +15,7 @@ public sealed class JsonActivityConstructorContextHelperTests
|
|||
[InlineData("Metadata")]
|
||||
[InlineData("_CustomProperties")]
|
||||
[InlineData("CustomProperties")]
|
||||
public void When_InputName_Is_ReservedKeyWord_Then_ThrowsException_WhenReadingInputs(string reservedInputName)
|
||||
public void When_InputName_Is_ReservedKeyWord_Then_AddsException_WhenReadingInputs(string reservedInputName)
|
||||
{
|
||||
// Arrange
|
||||
var activityDescriptor = new ActivityDescriptor
|
||||
|
|
@ -31,15 +32,15 @@ public sealed class JsonActivityConstructorContextHelperTests
|
|||
};
|
||||
var jsonElement = GetJsonElementWithReservedInputName(reservedInputName);
|
||||
|
||||
// Act & Assert
|
||||
Assert.Throws<KeyNotFoundException>(() =>
|
||||
{
|
||||
_ = JsonActivityConstructorContextHelper.CreateActivity<WorkflowAsActivity>(
|
||||
activityDescriptor,
|
||||
jsonElement,
|
||||
_serializerOptions
|
||||
);
|
||||
});
|
||||
// Act
|
||||
var result = JsonActivityConstructorContextHelper.CreateActivity<WorkflowAsActivity>(
|
||||
activityDescriptor,
|
||||
jsonElement,
|
||||
_serializerOptions
|
||||
);
|
||||
|
||||
// Assert
|
||||
Assert.Single(result.Exceptions, e => e is InvalidActivityDescriptorInputException);
|
||||
}
|
||||
|
||||
private static JsonElement GetJsonElementWithReservedInputName(string inputName)
|
||||
|
|
|
|||
|
|
@ -125,7 +125,7 @@ public sealed class ActivityJsonConverterTests
|
|||
|
||||
static IActivityRegistry CreateActivityRegistry(string typeName, IActivity activity, int? version = null)
|
||||
{
|
||||
var descriptor = new ActivityDescriptor { Constructor = _ => activity };
|
||||
var descriptor = new ActivityDescriptor { Constructor = _ => new(activity) };
|
||||
var activityRegistry = Substitute.For<IActivityRegistry>();
|
||||
|
||||
if (version.HasValue)
|
||||
|
|
@ -140,7 +140,7 @@ public sealed class ActivityJsonConverterTests
|
|||
{
|
||||
var descriptor = new ActivityDescriptor
|
||||
{
|
||||
Constructor = _ => WorkflowAsActivity,
|
||||
Constructor = _ => new(WorkflowAsActivity),
|
||||
CustomProperties = { [customPropertyName] = customPropertyValue }
|
||||
};
|
||||
|
||||
|
|
@ -334,6 +334,11 @@ public sealed class ActivityJsonConverterTests
|
|||
private static ActivityJsonConverter CreateSut(IActivityRegistry activityRegistry)
|
||||
{
|
||||
var expressionDescriptorRegistry = new ExpressionDescriptorRegistry([]);
|
||||
var serviceProvider = Substitute.For<IServiceProvider>();
|
||||
serviceProvider
|
||||
.GetService(typeof(ILogger<ActivityJsonConverter>))!
|
||||
.Returns(CreateLogger<ActivityJsonConverter>());
|
||||
|
||||
return new(
|
||||
activityRegistry,
|
||||
expressionDescriptorRegistry,
|
||||
|
|
@ -342,7 +347,7 @@ public sealed class ActivityJsonConverterTests
|
|||
new(expressionDescriptorRegistry),
|
||||
CreateLogger<ActivityWriter>()
|
||||
),
|
||||
Substitute.For<IServiceProvider>()
|
||||
serviceProvider
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue