Merge pull request #7741 from elsa-workflows/sfmskywalker-fix-optout-publish-validation
Add opt-out for publish-on-validation-error failure
This commit is contained in:
commit
1417e797ec
|
|
@ -106,12 +106,12 @@
|
|||
<PackageVersion Include="coverlet.collector" Version="6.0.4" PrivateAssets="All"/>
|
||||
<PackageVersion Include="coverlet.msbuild" Version="6.0.4" PrivateAssets="All"/>
|
||||
<PackageVersion Include="Cronos" Version="0.11.1"/>
|
||||
<PackageVersion Include="CShells" Version="0.0.24-preview.132"/>
|
||||
<PackageVersion Include="CShells.Abstractions" Version="0.0.24-preview.132"/>
|
||||
<PackageVersion Include="CShells.AspNetCore" Version="0.0.24-preview.132"/>
|
||||
<PackageVersion Include="CShells.AspNetCore.Abstractions" Version="0.0.24-preview.132"/>
|
||||
<PackageVersion Include="CShells.FastEndpoints" Version="0.0.24-preview.132"/>
|
||||
<PackageVersion Include="CShells.FastEndpoints.Abstractions" Version="0.0.24-preview.132"/>
|
||||
<PackageVersion Include="CShells" Version="0.0.28"/>
|
||||
<PackageVersion Include="CShells.Abstractions" Version="0.0.28"/>
|
||||
<PackageVersion Include="CShells.AspNetCore" Version="0.0.28"/>
|
||||
<PackageVersion Include="CShells.AspNetCore.Abstractions" Version="0.0.28"/>
|
||||
<PackageVersion Include="CShells.FastEndpoints" Version="0.0.28"/>
|
||||
<PackageVersion Include="CShells.FastEndpoints.Abstractions" Version="0.0.28"/>
|
||||
<PackageVersion Include="Elsa.Platform.PackageManifest.Generator" Version="0.0.1-preview.53"/>
|
||||
<PackageVersion Include="Datadog.Trace.Bundle" Version="3.32.0"/>
|
||||
<PackageVersion Include="DistributedLock" Version="2.7.1"/>
|
||||
|
|
|
|||
|
|
@ -12,6 +12,8 @@
|
|||
<packageSourceMapping>
|
||||
<packageSource key="NuGet official package source">
|
||||
<package pattern="*" />
|
||||
<package pattern="CShells" />
|
||||
<package pattern="CShells.*" />
|
||||
</packageSource>
|
||||
<packageSource key="cshells-feedz">
|
||||
<package pattern="CShells" />
|
||||
|
|
|
|||
|
|
@ -41,6 +41,8 @@ internal class BulkPublish(
|
|||
var alreadyPublished = new List<string>();
|
||||
var skipped = new List<string>();
|
||||
var updatedConsumers = new List<string>();
|
||||
var failed = new List<string>();
|
||||
var warnings = new Dictionary<string, ICollection<string>>();
|
||||
var publishableDefinitions = new List<(string DefinitionId, WorkflowDefinition Definition)>();
|
||||
|
||||
var definitions = (await store.FindManyAsync(new WorkflowDefinitionFilter
|
||||
|
|
@ -88,13 +90,23 @@ internal class BulkPublish(
|
|||
foreach (var (definitionId, definition) in publishableDefinitions)
|
||||
{
|
||||
var result = await workflowDefinitionPublisher.PublishAsync(definition, cancellationToken);
|
||||
|
||||
if (!result.Succeeded)
|
||||
{
|
||||
failed.Add(definitionId);
|
||||
continue;
|
||||
}
|
||||
|
||||
published.Add(definitionId);
|
||||
|
||||
if (result.ValidationErrors.Count > 0)
|
||||
warnings[definitionId] = result.ValidationErrors.Select(x => x.Message).ToList();
|
||||
|
||||
if (result.AffectedWorkflows.WorkflowDefinitions.Count > 0)
|
||||
updatedConsumers.AddRange(result.AffectedWorkflows.WorkflowDefinitions.Select(x => x.DefinitionId));
|
||||
}
|
||||
|
||||
return new(published, alreadyPublished, notFound, skipped, updatedConsumers);
|
||||
return new(published, alreadyPublished, notFound, skipped, updatedConsumers, failed, warnings);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,11 +5,13 @@ internal class Request
|
|||
public ICollection<string> DefinitionIds { get; set; } = default!;
|
||||
}
|
||||
|
||||
internal class Response(ICollection<string> published, ICollection<string> alreadyPublished, ICollection<string> notFound, ICollection<string> skipped, ICollection<string> updatedConsumers)
|
||||
internal class Response(ICollection<string> published, ICollection<string> alreadyPublished, ICollection<string> notFound, ICollection<string> skipped, ICollection<string> updatedConsumers, ICollection<string> failed, IDictionary<string, ICollection<string>> warnings)
|
||||
{
|
||||
public ICollection<string> Published { get; } = published;
|
||||
public ICollection<string> AlreadyPublished { get; } = alreadyPublished;
|
||||
public ICollection<string> NotFound { get; } = notFound;
|
||||
public ICollection<string> Skipped { get; } = skipped;
|
||||
public ICollection<string> UpdatedConsumers { get; } = updatedConsumers;
|
||||
public ICollection<string> Failed { get; } = failed;
|
||||
public IDictionary<string, ICollection<string>> Warnings { get; } = warnings;
|
||||
}
|
||||
|
|
@ -116,7 +116,8 @@ internal class Post(
|
|||
|
||||
var mappedDefinition = await linker.MapAsync(draft, cancellationToken);
|
||||
var affectedWorkflows = result?.AffectedWorkflows?.WorkflowDefinitions ?? [];
|
||||
var response = new Response(mappedDefinition, false, affectedWorkflows.Count);
|
||||
var validationErrors = result?.ValidationErrors.Select(e => e.Message).ToList() ?? [];
|
||||
var response = new Response(mappedDefinition, false, affectedWorkflows.Count, validationErrors);
|
||||
await HttpContext.Response.WriteAsJsonAsync(response, serializerOptions, cancellationToken);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,4 +2,4 @@ using Elsa.Workflows.Api.Models;
|
|||
|
||||
namespace Elsa.Workflows.Api.Endpoints.WorkflowDefinitions.Post;
|
||||
|
||||
internal record Response(LinkedWorkflowDefinitionModel WorkflowDefinition, bool AlreadyPublished, int ConsumingWorkflowCount);
|
||||
internal record Response(LinkedWorkflowDefinitionModel WorkflowDefinition, bool AlreadyPublished, int ConsumingWorkflowCount, ICollection<string> ValidationErrors);
|
||||
|
|
@ -60,8 +60,19 @@ internal class Publish(
|
|||
|
||||
var isPublished = definition.IsPublished;
|
||||
var result = !isPublished ? await workflowDefinitionPublisher.PublishAsync(definition, cancellationToken) : null;
|
||||
|
||||
if (result is { Succeeded: false })
|
||||
{
|
||||
foreach (var validationError in result.ValidationErrors)
|
||||
AddError(validationError.Message);
|
||||
|
||||
await Send.ErrorsAsync(400, cancellationToken);
|
||||
return;
|
||||
}
|
||||
|
||||
var validationErrors = result?.ValidationErrors.Select(e => e.Message).ToList() ?? [];
|
||||
var mappedDefinition = await linker.MapAsync(definition, cancellationToken);
|
||||
var response = new Response(mappedDefinition, isPublished, result?.AffectedWorkflows.WorkflowDefinitions.Count ?? 0);
|
||||
var response = new Response(mappedDefinition, isPublished, result?.AffectedWorkflows.WorkflowDefinitions.Count ?? 0, validationErrors);
|
||||
await Send.OkAsync(response, cancellationToken);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,4 +7,4 @@ internal class Request
|
|||
public string DefinitionId { get; set; } = default!;
|
||||
}
|
||||
|
||||
internal record Response(LinkedWorkflowDefinitionModel WorkflowDefinition, bool AlreadyPublished, int ConsumingWorkflowCount);
|
||||
internal record Response(LinkedWorkflowDefinitionModel WorkflowDefinition, bool AlreadyPublished, int ConsumingWorkflowCount, ICollection<string> ValidationErrors);
|
||||
|
|
@ -61,6 +61,7 @@ public class WorkflowManagementFeature(IModule module) : FeatureBase(module)
|
|||
private string CompressionAlgorithm { get; set; } = nameof(None);
|
||||
private LogPersistenceMode LogPersistenceMode { get; set; } = LogPersistenceMode.Include;
|
||||
private bool IsReadOnlyMode { get; set; }
|
||||
private bool FailOnValidationErrors { get; set; } = true;
|
||||
|
||||
/// <summary>
|
||||
/// A set of activity types to make available to the system.
|
||||
|
|
@ -219,6 +220,17 @@ public class WorkflowManagementFeature(IModule module) : FeatureBase(module)
|
|||
return this;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Enables or disables failing workflow publication when the workflow has validation errors.
|
||||
/// Defaults to <c>true</c> as of 3.8.0 (publication fails when validation errors are present).
|
||||
/// Set to <c>false</c> to allow publication to succeed, returning validation errors as warnings.
|
||||
/// </summary>
|
||||
public WorkflowManagementFeature UseFailOnValidationErrors(bool enabled = true)
|
||||
{
|
||||
FailOnValidationErrors = enabled;
|
||||
return this;
|
||||
}
|
||||
|
||||
public WorkflowManagementFeature UseWorkflowDefinitionPublisher(Func<IServiceProvider, IWorkflowDefinitionPublisher> workflowDefinitionPublisher)
|
||||
{
|
||||
_workflowDefinitionPublisher = workflowDefinitionPublisher;
|
||||
|
|
@ -313,6 +325,7 @@ public class WorkflowManagementFeature(IModule module) : FeatureBase(module)
|
|||
options.CompressionAlgorithm = CompressionAlgorithm;
|
||||
options.LogPersistenceMode = LogPersistenceMode;
|
||||
options.IsReadOnlyMode = IsReadOnlyMode;
|
||||
options.FailOnValidationErrors = FailOnValidationErrors;
|
||||
});
|
||||
|
||||
Services.Configure<ExpressionOptions>(options => options.RegisterTypeAlias(typeof(ClrWorkflowMaterializerContext), nameof(ClrWorkflowMaterializerContext)));
|
||||
|
|
|
|||
|
|
@ -32,4 +32,14 @@ public class ManagementOptions
|
|||
/// A mode that does not allow editing workflows.
|
||||
/// </summary>
|
||||
public bool IsReadOnlyMode { get; set; }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Determines whether publishing a workflow definition fails when the workflow has validation errors.
|
||||
/// When <c>true</c> (the default as of 3.8.0), publishing fails if any validation errors are present.
|
||||
/// When <c>false</c>, publishing is allowed to succeed and any validation errors are returned as warnings
|
||||
/// on the publish result. This allows publishing workflows that intentionally leave required properties
|
||||
/// blank (for example, an empty Cron expression used to disable a trigger). In 3.6.x and 3.7.x this
|
||||
/// defaulted to <c>false</c> (opt-in); as of 3.8.0 it defaults to <c>true</c> (opt-out).
|
||||
/// </summary>
|
||||
public bool FailOnValidationErrors { get; set; } = true;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,7 +8,9 @@ using Elsa.Workflows.Management.Filters;
|
|||
using Elsa.Workflows.Management.Materializers;
|
||||
using Elsa.Workflows.Management.Models;
|
||||
using Elsa.Workflows.Management.Notifications;
|
||||
using Elsa.Workflows.Management.Options;
|
||||
using Elsa.Workflows.Models;
|
||||
using Microsoft.Extensions.Options;
|
||||
|
||||
namespace Elsa.Workflows.Management.Services;
|
||||
|
||||
|
|
@ -20,7 +22,8 @@ public class WorkflowDefinitionPublisher(
|
|||
IIdentityGenerator identityGenerator,
|
||||
IActivitySerializer activitySerializer,
|
||||
IMediator mediator,
|
||||
ISystemClock systemClock)
|
||||
ISystemClock systemClock,
|
||||
IOptions<ManagementOptions> options)
|
||||
: IWorkflowDefinitionPublisher
|
||||
{
|
||||
/// <inheritdoc />
|
||||
|
|
@ -87,7 +90,7 @@ public class WorkflowDefinitionPublisher(
|
|||
var workflowGraph = await workflowDefinitionService.MaterializeWorkflowAsync(definition, cancellationToken);
|
||||
var validationErrors = (await workflowValidator.ValidateAsync(workflowGraph.Workflow, cancellationToken)).ToList();
|
||||
|
||||
if (validationErrors.Any())
|
||||
if (validationErrors.Any() && options.Value.FailOnValidationErrors)
|
||||
return new(false, validationErrors, new([]));
|
||||
|
||||
await mediator.SendAsync(new WorkflowDefinitionPublishing(definition), cancellationToken);
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
using Elsa.Testing.Shared;
|
||||
using Elsa.Extensions;
|
||||
using Elsa.Testing.Shared;
|
||||
using Elsa.Workflows.Management;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Xunit.Abstractions;
|
||||
|
|
@ -8,10 +9,12 @@ namespace Elsa.Workflows.IntegrationTests.Scenarios.ImportAndPublish;
|
|||
public class ImportAndPublishCronTests
|
||||
{
|
||||
private readonly CapturingTextWriter _capturingTextWriter = new();
|
||||
private readonly ITestOutputHelper _testOutputHelper;
|
||||
private readonly IServiceProvider _services;
|
||||
|
||||
public ImportAndPublishCronTests(ITestOutputHelper testOutputHelper)
|
||||
{
|
||||
_testOutputHelper = testOutputHelper;
|
||||
_services = new TestApplicationBuilder(testOutputHelper)
|
||||
.WithCapturingTextWriter(_capturingTextWriter)
|
||||
.Build();
|
||||
|
|
@ -53,4 +56,29 @@ public class ImportAndPublishCronTests
|
|||
Assert.Single(result.ValidationErrors);
|
||||
Assert.Equal("Error when parsing cron expression: The given cron expression has an invalid format. Seconds: Value must be a number between 0 and 59 (all inclusive).", result.ValidationErrors.Single().Message);
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "Cron workflow with bad cron expression should publish successfully when FailOnValidationErrors is disabled.")]
|
||||
public async Task ImportAndPublish_ShouldSucceed_WithBadCronExpression_WhenFailOnValidationErrorsDisabled()
|
||||
{
|
||||
// Opt out of strict publishing.
|
||||
var services = new TestApplicationBuilder(_testOutputHelper)
|
||||
.WithCapturingTextWriter(_capturingTextWriter)
|
||||
.ConfigureElsa(elsa => elsa.UseWorkflowManagement(management => management.UseFailOnValidationErrors(false)))
|
||||
.Build();
|
||||
|
||||
// Populate registries.
|
||||
await services.PopulateRegistriesAsync();
|
||||
|
||||
// Import workflow.
|
||||
var workflowDefinition = await services.ImportWorkflowDefinitionAsync($"Scenarios/ImportAndPublish/Workflows/bad-cron-expression.json");
|
||||
|
||||
// Publish.
|
||||
IWorkflowDefinitionPublisher workflowDefinitionPublisher = services.GetRequiredService<IWorkflowDefinitionPublisher>();
|
||||
var result = await workflowDefinitionPublisher.PublishAsync(workflowDefinition);
|
||||
|
||||
// Assert: publishing succeeds while the validation error is surfaced as a warning.
|
||||
Assert.True(result.Succeeded);
|
||||
Assert.Single(result.ValidationErrors);
|
||||
Assert.Equal("Error when parsing cron expression: The given cron expression has an invalid format. Seconds: Value must be a number between 0 and 59 (all inclusive).", result.ValidationErrors.Single().Message);
|
||||
}
|
||||
}
|
||||
|
|
@ -9,10 +9,12 @@ namespace Elsa.Workflows.IntegrationTests.Scenarios.ImportAndPublish;
|
|||
public class ImportAndPublishHttpEndpointsTests
|
||||
{
|
||||
private readonly CapturingTextWriter _capturingTextWriter = new();
|
||||
private readonly ITestOutputHelper _testOutputHelper;
|
||||
private readonly IServiceProvider _services;
|
||||
|
||||
public ImportAndPublishHttpEndpointsTests(ITestOutputHelper testOutputHelper)
|
||||
{
|
||||
_testOutputHelper = testOutputHelper;
|
||||
_services = new TestApplicationBuilder(testOutputHelper)
|
||||
.WithCapturingTextWriter(_capturingTextWriter)
|
||||
.ConfigureElsa(configure => configure.UseHttp())
|
||||
|
|
@ -65,4 +67,41 @@ public class ImportAndPublishHttpEndpointsTests
|
|||
Assert.Single(result.ValidationErrors);
|
||||
Assert.Equal("The /test path and get method are already in use by another workflow!", result.ValidationErrors.Single().Message);
|
||||
}
|
||||
|
||||
[Fact(DisplayName = "Http endpoint workflow with duplicate path and method should publish successfully when FailOnValidationErrors is disabled.")]
|
||||
public async Task ImportAndPublish_ShouldSucceed_WithTwoHttpEndpointSamePathMethod_WhenFailOnValidationErrorsDisabled()
|
||||
{
|
||||
// Opt out of strict publishing.
|
||||
var services = new TestApplicationBuilder(_testOutputHelper)
|
||||
.WithCapturingTextWriter(_capturingTextWriter)
|
||||
.ConfigureElsa(configure => configure
|
||||
.UseHttp()
|
||||
.UseWorkflowManagement(management => management.UseFailOnValidationErrors(false)))
|
||||
.Build();
|
||||
|
||||
// Populate registries.
|
||||
await services.PopulateRegistriesAsync();
|
||||
|
||||
// Import first workflow.
|
||||
var workflowDefinition = await services.ImportWorkflowDefinitionAsync($"Scenarios/ImportAndPublish/Workflows/http-workflow.json");
|
||||
|
||||
// Publish first workflow.
|
||||
IWorkflowDefinitionPublisher workflowDefinitionPublisher = services.GetRequiredService<IWorkflowDefinitionPublisher>();
|
||||
var result = await workflowDefinitionPublisher.PublishAsync(workflowDefinition);
|
||||
|
||||
// Assert first workflow.
|
||||
Assert.True(result.Succeeded);
|
||||
Assert.Empty(result.ValidationErrors);
|
||||
|
||||
// Import second workflow.
|
||||
workflowDefinition = await services.ImportWorkflowDefinitionAsync($"Scenarios/ImportAndPublish/Workflows/http-workflow.json");
|
||||
|
||||
// Publish second workflow.
|
||||
result = await workflowDefinitionPublisher.PublishAsync(workflowDefinition);
|
||||
|
||||
// Assert: publishing succeeds while the validation error is surfaced as a warning.
|
||||
Assert.True(result.Succeeded);
|
||||
Assert.Single(result.ValidationErrors);
|
||||
Assert.Equal("The /test path and get method are already in use by another workflow!", result.ValidationErrors.Single().Message);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue