diff --git a/Directory.Packages.props b/Directory.Packages.props index f008be8d6..49db077d8 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -106,12 +106,12 @@ - - - - - - + + + + + + diff --git a/NuGet.Config b/NuGet.Config index 3a1e84216..590436810 100644 --- a/NuGet.Config +++ b/NuGet.Config @@ -12,6 +12,8 @@ + + diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Endpoint.cs index da23172a1..053491b2e 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Endpoint.cs @@ -41,6 +41,8 @@ internal class BulkPublish( var alreadyPublished = new List(); var skipped = new List(); var updatedConsumers = new List(); + var failed = new List(); + var warnings = new Dictionary>(); 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); } } diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Models.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Models.cs index 03a4f9f4c..879a86321 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Models.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/BulkPublish/Models.cs @@ -5,11 +5,13 @@ internal class Request public ICollection DefinitionIds { get; set; } = default!; } -internal class Response(ICollection published, ICollection alreadyPublished, ICollection notFound, ICollection skipped, ICollection updatedConsumers) +internal class Response(ICollection published, ICollection alreadyPublished, ICollection notFound, ICollection skipped, ICollection updatedConsumers, ICollection failed, IDictionary> warnings) { public ICollection Published { get; } = published; public ICollection AlreadyPublished { get; } = alreadyPublished; public ICollection NotFound { get; } = notFound; public ICollection Skipped { get; } = skipped; public ICollection UpdatedConsumers { get; } = updatedConsumers; + public ICollection Failed { get; } = failed; + public IDictionary> Warnings { get; } = warnings; } \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Endpoint.cs index a0895762e..1dfd307c7 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Endpoint.cs @@ -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); } diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Models.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Models.cs index baceba1df..847f83179 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Models.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Post/Models.cs @@ -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); \ No newline at end of file +internal record Response(LinkedWorkflowDefinitionModel WorkflowDefinition, bool AlreadyPublished, int ConsumingWorkflowCount, ICollection ValidationErrors); \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Endpoint.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Endpoint.cs index 299534025..c5f0116b1 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Endpoint.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Endpoint.cs @@ -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); } } diff --git a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Models.cs b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Models.cs index a4ed95916..2554d668a 100644 --- a/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Models.cs +++ b/src/modules/Elsa.Workflows.Api/Endpoints/WorkflowDefinitions/Publish/Models.cs @@ -7,4 +7,4 @@ internal class Request public string DefinitionId { get; set; } = default!; } -internal record Response(LinkedWorkflowDefinitionModel WorkflowDefinition, bool AlreadyPublished, int ConsumingWorkflowCount); \ No newline at end of file +internal record Response(LinkedWorkflowDefinitionModel WorkflowDefinition, bool AlreadyPublished, int ConsumingWorkflowCount, ICollection ValidationErrors); \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Management/Features/WorkflowManagementFeature.cs b/src/modules/Elsa.Workflows.Management/Features/WorkflowManagementFeature.cs index 0a26da083..c544636ee 100644 --- a/src/modules/Elsa.Workflows.Management/Features/WorkflowManagementFeature.cs +++ b/src/modules/Elsa.Workflows.Management/Features/WorkflowManagementFeature.cs @@ -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; /// /// A set of activity types to make available to the system. @@ -219,6 +220,17 @@ public class WorkflowManagementFeature(IModule module) : FeatureBase(module) return this; } + /// + /// Enables or disables failing workflow publication when the workflow has validation errors. + /// Defaults to true as of 3.8.0 (publication fails when validation errors are present). + /// Set to false to allow publication to succeed, returning validation errors as warnings. + /// + public WorkflowManagementFeature UseFailOnValidationErrors(bool enabled = true) + { + FailOnValidationErrors = enabled; + return this; + } + public WorkflowManagementFeature UseWorkflowDefinitionPublisher(Func 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(options => options.RegisterTypeAlias(typeof(ClrWorkflowMaterializerContext), nameof(ClrWorkflowMaterializerContext))); diff --git a/src/modules/Elsa.Workflows.Management/Options/ManagementOptions.cs b/src/modules/Elsa.Workflows.Management/Options/ManagementOptions.cs index aca77eb19..47db863e5 100644 --- a/src/modules/Elsa.Workflows.Management/Options/ManagementOptions.cs +++ b/src/modules/Elsa.Workflows.Management/Options/ManagementOptions.cs @@ -32,4 +32,14 @@ public class ManagementOptions /// A mode that does not allow editing workflows. /// public bool IsReadOnlyMode { get; set; } -} \ No newline at end of file + + /// + /// Determines whether publishing a workflow definition fails when the workflow has validation errors. + /// When true (the default as of 3.8.0), publishing fails if any validation errors are present. + /// When false, 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 false (opt-in); as of 3.8.0 it defaults to true (opt-out). + /// + public bool FailOnValidationErrors { get; set; } = true; +} diff --git a/src/modules/Elsa.Workflows.Management/Services/WorkflowDefinitionPublisher.cs b/src/modules/Elsa.Workflows.Management/Services/WorkflowDefinitionPublisher.cs index 007fc855c..30afc6d7a 100644 --- a/src/modules/Elsa.Workflows.Management/Services/WorkflowDefinitionPublisher.cs +++ b/src/modules/Elsa.Workflows.Management/Services/WorkflowDefinitionPublisher.cs @@ -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 options) : IWorkflowDefinitionPublisher { /// @@ -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); diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/ImportAndPublish/ImportAndPublishCronTests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/ImportAndPublish/ImportAndPublishCronTests.cs index e61fa08b1..2d41f788e 100644 --- a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/ImportAndPublish/ImportAndPublishCronTests.cs +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/ImportAndPublish/ImportAndPublishCronTests.cs @@ -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(); + 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); + } } \ No newline at end of file diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/ImportAndPublish/ImportAndPublishHttpEndpointsTests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/ImportAndPublish/ImportAndPublishHttpEndpointsTests.cs index d1b5a322e..29d127f92 100644 --- a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/ImportAndPublish/ImportAndPublishHttpEndpointsTests.cs +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/ImportAndPublish/ImportAndPublishHttpEndpointsTests.cs @@ -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(); + 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); + } } \ No newline at end of file