From 22c1aea4728ce2b6b354a86a85eaed280679446b Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Fri, 11 Sep 2026 22:15:47 -0700 Subject: [PATCH] fix(bpmn): the document PUT preserves the definition's non-BPMN metadata (#8063) * fix(bpmn): preserve definition metadata when the document PUT rebinds it The document PUT edits a definition's BPMN document, not the whole definition, so it must not reset the author's name, description, variables, options and custom properties the way a whole-definition import intentionally does. BpmnInterchangeDocumentService.ImportAsync now accepts the existing definition to preserve that metadata from, which ImportDocumentAsync passes; POST bpmn/import stays a whole-definition import. Co-Authored-By: Claude Opus 5 * fix(bpmn): preserve IsReadonly and restore ImportAsync's public signature Review findings on the prior metadata-preservation fix: IsReadonly was still overwritten by the document PUT because it was missing from the preserved field set, and the public ImportAsync(string, string?, string?, string?, CancellationToken) signature had been changed in place by appending an optional preserveMetadataFrom parameter. Restore that signature exactly and move the shared import logic into a private ImportCoreAsync(..., preserveMetadataFrom, CancellationToken), called by both the public ImportAsync and ImportDocumentAsync, with IsReadonly now carried alongside the other non-BPMN metadata. Extend the endpoint tests to also set and assert inputs, outputs, outcomes, tool version and IsReadonly on both the preserve and replace paths. Co-Authored-By: Claude Opus 5 * fix(bpmn): refuse document PUT when the target definition has vanished ImportDocumentAsync fell back to the whole-definition import path when its definition lookup returned null, silently creating a definition under the requested id with reset metadata instead of reporting that the PUT's target disappeared (e.g. deleted between the endpoint's existence/ETag check and this lookup). It now throws BpmnDefinitionNotFoundException, which the shared exception cascade maps to 404 for the document PUT endpoint; POST bpmn/import is unaffected since it never passes a preservation source. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- doc/wiki/bpmn-workflows.md | 15 +- .../Bpmn/BpmnImportExceptionCascade.cs | 6 + .../BpmnDefinitionNotFoundException.cs | 16 +++ .../BpmnInterchangeDocumentService.cs | 97 ++++++++++++- .../Endpoints/BpmnInterchangeEndpointTests.cs | 135 ++++++++++++++++++ .../Interchange/BpmnDocumentRoundTripTests.cs | 21 +++ .../BpmnExportAvailabilityTests.cs | 4 +- 7 files changed, 286 insertions(+), 8 deletions(-) create mode 100644 src/modules/Elsa.Bpmn.Interchange/Exceptions/BpmnDefinitionNotFoundException.cs diff --git a/doc/wiki/bpmn-workflows.md b/doc/wiki/bpmn-workflows.md index 3036873a1..46b0211fa 100644 --- a/doc/wiki/bpmn-workflows.md +++ b/doc/wiki/bpmn-workflows.md @@ -90,7 +90,7 @@ An exported `.bpmn` is self-contained: all binding configuration, including inpu | `POST bpmn/import` | `write:workflow-definitions` | Uploads a single `.bpmn` file and persists it as a new or updated workflow definition (as a draft; it is not published). Optional form fields: `DefinitionId` (update an existing definition instead of creating one), `Name`, `ProcessId` (required when the document declares more than one process). | | `GET bpmn/definitions/{definitionId}/export` | `read:workflow-definitions` | Writes the workflow definition's BPMN source back out as `.bpmn` XML. Optional `VersionOptions` query parameter (`Latest`, `Published`, or a specific version), defaulting to `Latest`. | | `GET bpmn/definitions/{definitionId}/document` | `read:workflow-definitions` | Reads the workflow definition's stored BPMN source with the `Bpmn.Model`/`Bpmn.Interchange` reader and returns the whole `bpmnDefinitions` document as the library's own JSON (payload format `1.0.0`), rather than as `.bpmn` XML. Same refusals as `Export` when the definition was never imported from BPMN or its stored source is stale. Carries an `ETag` response header for the returned revision — see below. | -| `PUT bpmn/definitions/{definitionId}/document` | `write:workflow-definitions` | Accepts a `bpmnDefinitions` JSON document — the shape `GET` on the same route returns — writes it back out as `.bpmn` XML, and runs it through the same path `Import` runs: analyze, capability check, bind, persist as a new draft, refresh the stored source. Never edits a published version in place, exactly like `Import`. Returns the same `Id`/`DefinitionId`/`Version`/`Analysis` shape `Import` returns, plus the new `ETag`. Requires an `If-Match` request header — see below. | +| `PUT bpmn/definitions/{definitionId}/document` | `write:workflow-definitions` | Accepts a `bpmnDefinitions` JSON document — the shape `GET` on the same route returns — writes it back out as `.bpmn` XML, and runs it through the same path `Import` runs: analyze, capability check, bind, persist as a new draft, refresh the stored source. Only the activity graph and the `Bpmn:*` custom properties change; the definition's name, description, variables, inputs, outputs, outcomes, options, tool version and any other custom property are carried forward unchanged, unlike `POST bpmn/import`, which stays a whole-definition import (see below). Never edits a published version in place, exactly like `Import`. Returns the same `Id`/`DefinitionId`/`Version`/`Analysis` shape `Import` returns, plus the new `ETag`. Requires an `If-Match` request header — see below. | Both `Analyze` and `Import` require exactly one uploaded file; zero or more than one returns `400 Bad Request`. @@ -109,7 +109,9 @@ exactly what is stored returns the same `ETag` `GET` did. The first `PUT` after uploaded bytes with the writer's own rendering of the same document, so the stored document, and with it the `ETag`, changes once even when nothing was edited. A save that changes only the definition's other properties — its name, description or variables, say — leaves the document and the graph untouched and does not change the `ETag`; a -document `PUT`, like `Import`, resets those properties regardless. +document `PUT` leaves those properties as that save left them, since it edits the document, not the rest of the +definition — see the next section. `POST bpmn/import` with the same `DefinitionId` is different: it is a +whole-definition import and resets them from the document, same as it always has. `PUT` requires an `If-Match` request header carrying the `ETag` a prior `GET` (or `PUT`) returned: @@ -139,6 +141,15 @@ A document that declares more than one `` is re-imported against the sa imported with — recorded on the workflow definition the first time it is imported, whether from `Import` or from a `document` `PUT`, so an edit to a multi-process document does not have to name the process again on every save. +**The document `PUT` edits the BPMN document, not the whole definition.** Only the activity graph the newly bound +document produces and the `Bpmn:*` custom properties (`SourceXml`, `SourceVersion`, `SourceProcessId`) change; the +definition's name, description, variables, inputs, outputs, outcomes, options, tool version and every other custom +property are carried forward exactly as they stood before the `PUT`. This is what lets Studio's binding UX (elsa- +studio#1001) save a binding change through this endpoint without silently resetting metadata the author set some +other way — a renamed definition, variables added on the draft, a description. `POST bpmn/import` with a +`DefinitionId` is unaffected by this: it remains a whole-definition import, building the definition from the +uploaded document alone and replacing all of the above, exactly as it always has. + ### Capability refusal at import A BPMN document can declare behaviour (e.g. certain multi-instance or event-subprocess shapes) that needs a host diff --git a/src/modules/Elsa.Bpmn.Interchange/Endpoints/Bpmn/BpmnImportExceptionCascade.cs b/src/modules/Elsa.Bpmn.Interchange/Endpoints/Bpmn/BpmnImportExceptionCascade.cs index f169c0a88..40ef20fd0 100644 --- a/src/modules/Elsa.Bpmn.Interchange/Endpoints/Bpmn/BpmnImportExceptionCascade.cs +++ b/src/modules/Elsa.Bpmn.Interchange/Endpoints/Bpmn/BpmnImportExceptionCascade.cs @@ -31,6 +31,12 @@ internal static class BpmnImportExceptionCascade { result = await import(); } + catch (BpmnDefinitionNotFoundException exception) + { + addError(exception.Message); + await sendErrorsAsync(StatusCodes.Status404NotFound, cancellationToken); + return null; + } catch (BpmnInterchangeException exception) { addError(exception.Message); diff --git a/src/modules/Elsa.Bpmn.Interchange/Exceptions/BpmnDefinitionNotFoundException.cs b/src/modules/Elsa.Bpmn.Interchange/Exceptions/BpmnDefinitionNotFoundException.cs new file mode 100644 index 000000000..96b22b2a8 --- /dev/null +++ b/src/modules/Elsa.Bpmn.Interchange/Exceptions/BpmnDefinitionNotFoundException.cs @@ -0,0 +1,16 @@ +namespace Elsa.Bpmn.Interchange.Exceptions; + +/// +/// Thrown when is asked to edit the BPMN +/// document of a workflow definition that no longer exists. +/// +/// +/// edits an existing definition by +/// passing it as the shared import logic's preserveMetadataFrom parameter; a missing preservation source must +/// never fall through to the whole-definition import path preserveMetadataFrom: null takes; that path would +/// silently create a definition under the requested id with reset metadata instead of reporting that the PUT's +/// target disappeared. If the definition is deleted between the PUT endpoint's own existence/ETag check and this +/// method's lookup, this is what makes that race surface as a refusal rather than a silent, metadata-resetting +/// import. +/// +public class BpmnDefinitionNotFoundException(string message) : Exception(message); diff --git a/src/modules/Elsa.Bpmn.Interchange/Services/BpmnInterchangeDocumentService.cs b/src/modules/Elsa.Bpmn.Interchange/Services/BpmnInterchangeDocumentService.cs index 20f5f3b39..402af7383 100644 --- a/src/modules/Elsa.Bpmn.Interchange/Services/BpmnInterchangeDocumentService.cs +++ b/src/modules/Elsa.Bpmn.Interchange/Services/BpmnInterchangeDocumentService.cs @@ -6,10 +6,13 @@ using Elsa.Bpmn.Activities; using Elsa.Bpmn.Hosting; using Elsa.Bpmn.Interchange.Binding; using Elsa.Bpmn.Interchange.Exceptions; +using Elsa.Common.Models; using Elsa.Extensions; using Elsa.Workflows.Management; using Elsa.Workflows.Management.Entities; +using Elsa.Workflows.Management.Mappers; using Elsa.Workflows.Management.Models; +using Elsa.Workflows.Models; namespace Elsa.Bpmn.Interchange.Services; @@ -57,6 +60,17 @@ namespace Elsa.Bpmn.Interchange.Services; /// caller has, which is worse than refusing. /// /// +/// A whole-definition import and a document edit disagree about what else gets replaced. +/// builds the WorkflowDefinitionModel from the document alone, so Import — which persists a whole +/// definition from a document, name and all — intentionally replaces the definition's name, description, variables, +/// inputs, outputs, outcomes, options, tool version, read-only flag and custom properties with what that model carries. +/// is different: it edits the BPMN document of an existing definition, so +/// it passes that definition to the shared import logic's preserveMetadataFrom parameter, which carries all +/// of the above onto the result unchanged. Either way, only the bound activity graph and the three custom properties +/// this service owns (, , +/// ) come from the import itself. +/// +/// /// Capability refusal happens here, at import, not at BpmnGraph.Build. /// is the static half of the same check BpmnGraph.Build performs at first execution: it needs only the /// definition, not bound work or a host snapshot. Running it at import means an unrunnable diagram is rejected before @@ -72,7 +86,8 @@ public sealed class BpmnInterchangeDocumentService( BpmnXmlWriter writer, BpmnWorkBinder binder, IWorkflowDefinitionImporter importer, - IWorkflowDefinitionStore store) + IWorkflowDefinitionStore store, + VariableDefinitionMapper variableDefinitionMapper) { /// The workflow definition custom property the original BPMN XML is carried under, for . public const string SourceXmlCustomPropertyKey = "Bpmn:SourceXml"; @@ -144,7 +159,40 @@ public sealed class BpmnInterchangeDocumentService( /// The document cannot be read, or declares more than one process and does not pick one. /// The document needs a host capability this deployment does not declare. /// A work binding cannot be turned into an Elsa activity. - public async Task ImportAsync(string xml, string? definitionId, string? name, string? processId, CancellationToken cancellationToken) + public Task ImportAsync(string xml, string? definitionId, string? name, string? processId, CancellationToken cancellationToken) => + ImportCoreAsync(xml, definitionId, name, processId, preserveMetadataFrom: null, cancellationToken); + + /// + /// The shared import logic behind both the public and : + /// reads a document, refuses it if the host cannot run what it declares, and binds it into the + /// scope a workflow definition's root becomes. + /// + /// The BPMN 2.0 XML to import. + /// The workflow definition to update, or null/empty to create a new one. + /// The workflow definition's display name, defaulting to the process's own BPMN name or id. + /// + /// The process to bind when the document declares more than one; not needed when it declares exactly one. + /// + /// + /// When set, the definition this import must otherwise leave untouched: its name, description, variables, + /// inputs, outputs, outcomes, options, tool version, read-only flag and custom properties are carried onto the + /// imported definition as-is, and only the bound activity graph and the , + /// and custom + /// properties this method owns change. This is what passes so the document PUT + /// edits the BPMN document without silently resetting the rest of the definition; left null for a + /// whole-definition import, where the model built from the document alone is the intended contract. + /// + /// The cancellation token. + /// The document cannot be read, or declares more than one process and does not pick one. + /// The document needs a host capability this deployment does not declare. + /// A work binding cannot be turned into an Elsa activity. + private async Task ImportCoreAsync( + string xml, + string? definitionId, + string? name, + string? processId, + WorkflowDefinition? preserveMetadataFrom, + CancellationToken cancellationToken) { var result = reader.Read(xml, new BpmnImportOptions { ProcessId = processId }); var rootDefinition = ResolveRootDefinition(result.Definitions, processId); @@ -160,10 +208,25 @@ public sealed class BpmnInterchangeDocumentService( var model = new WorkflowDefinitionModel { DefinitionId = definitionId ?? string.Empty, - Name = string.IsNullOrWhiteSpace(name) ? rootDefinition.Name ?? rootDefinition.ProcessId : name, + Name = preserveMetadataFrom is not null + ? preserveMetadataFrom.Name + : string.IsNullOrWhiteSpace(name) ? rootDefinition.Name ?? rootDefinition.ProcessId : name, Root = process }; + if (preserveMetadataFrom is not null) + { + model.Description = preserveMetadataFrom.Description; + model.Variables = variableDefinitionMapper.Map(preserveMetadataFrom.Variables).ToList(); + model.Inputs = preserveMetadataFrom.Inputs; + model.Outputs = preserveMetadataFrom.Outputs; + model.Outcomes = preserveMetadataFrom.Outcomes; + model.Options = preserveMetadataFrom.Options; + model.ToolVersion = preserveMetadataFrom.ToolVersion; + model.IsReadonly = preserveMetadataFrom.IsReadonly; + model.CustomProperties = new Dictionary(preserveMetadataFrom.CustomProperties); + } + var importResult = await importer.ImportAsync(new SaveWorkflowDefinitionRequest { Model = model, Publish = false }, cancellationToken); // The definition's final Version is only known once the importer/publisher has assigned and persisted it — @@ -237,6 +300,15 @@ public sealed class BpmnInterchangeDocumentService( /// , the same path Import runs. Analyze-then-commit sharing this one code path /// with the read side is what keeps a preview unable to disagree with what this actually persists. /// + /// + /// Unlike a whole-definition import, this edits the BPMN document of an existing definition: the caller + /// is changing a binding, not replacing the definition. So 's current metadata — + /// name, description, variables, inputs, outputs, outcomes, options, tool version, read-only flag and custom + /// properties other than the ones this service owns — is carried onto the result unchanged; see the shared + /// import logic's preserveMetadataFrom parameter, which this passes the existing definition to. Only the activity graph + /// and the // + /// custom properties move. + /// /// The edited document, deserialized through the library's own JSON converters. /// The workflow definition to update. /// @@ -245,13 +317,28 @@ public sealed class BpmnInterchangeDocumentService( /// finds the value that was used the first time. /// /// The cancellation token. + /// + /// The workflow definition to edit no longer exists — e.g. it was deleted between the PUT endpoint's own + /// existence/ETag check and this lookup. A missing preservation source must never fall through to the + /// whole-definition import path, which would silently create a definition under + /// with reset metadata instead of reporting that this PUT's target disappeared. + /// /// The document declares more than one process and does not pick one. /// The document needs a host capability this deployment does not declare. /// A work binding cannot be turned into an Elsa activity. - public Task ImportDocumentAsync(BpmnDefinitions document, string definitionId, string? processId, CancellationToken cancellationToken) + public async Task ImportDocumentAsync(BpmnDefinitions document, string definitionId, string? processId, CancellationToken cancellationToken) { + var filter = WorkflowDefinitionHandle.ByDefinitionId(definitionId, VersionOptions.Latest).ToFilter(); + var existingDefinition = await store.FindAsync(filter, cancellationToken); + + if (existingDefinition is null) + { + throw new BpmnDefinitionNotFoundException( + $"Workflow definition '{definitionId}' does not exist, so its BPMN document cannot be edited."); + } + var xml = writer.Write(document); - return ImportAsync(xml, definitionId, name: null, processId, cancellationToken); + return await ImportCoreAsync(xml, definitionId, name: null, processId, preserveMetadataFrom: existingDefinition, cancellationToken); } /// diff --git a/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Endpoints/BpmnInterchangeEndpointTests.cs b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Endpoints/BpmnInterchangeEndpointTests.cs index d01caca0f..1a0428839 100644 --- a/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Endpoints/BpmnInterchangeEndpointTests.cs +++ b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Endpoints/BpmnInterchangeEndpointTests.cs @@ -16,6 +16,7 @@ using Elsa.Workflows; using Elsa.Workflows.Activities; using Elsa.Workflows.Management; using Elsa.Workflows.Management.Entities; +using Elsa.Workflows.Memory; using Elsa.Workflows.Models; using FastEndpoints; using Microsoft.AspNetCore.Authentication; @@ -472,6 +473,59 @@ public class BpmnInterchangeEndpointTests(ITestOutputHelper testOutputHelper) : Assert.Contains("order-process", exportedXml); } + [Fact] + public async Task DocumentPut_WithABindingChange_PreservesTheDefinitionsNonBpmnMetadata() + { + var definitionId = await ImportCamundaOrderProcessAsync(); + await SetNonBpmnMetadataOnDraftAsync(definitionId); + var metadataBeforePut = await CaptureMetadataAsync(definitionId); + + var (etag, documentJson) = await GetDocumentAsync(definitionId); + var putResponse = await PutDocumentAsync(definitionId, WithNotifyWarehouseTextChanged(documentJson), etag); + + Assert.Equal(HttpStatusCode.OK, putResponse.StatusCode); + + // Every field SetNonBpmnMetadataOnDraftAsync set survived the PUT unchanged. + var metadataAfterPut = await CaptureMetadataAsync(definitionId); + Assert.Equal(metadataBeforePut, metadataAfterPut); + + // ...while the graph reflects the binding change the PUT carried. + var exportResponse = await GetAuthenticatedAsync($"bpmn/definitions/{definitionId}/export", "workflows/definitions:view"); + Assert.Equal(HttpStatusCode.OK, exportResponse.StatusCode); + var exportedXml = await exportResponse.Content.ReadAsStringAsync(); + Assert.Contains("Notifying the warehouse, rebound via document PUT", exportedXml); + } + + [Fact] + public async Task Import_WithTheSameDefinitionId_StillReplacesTheDefinitionsMetadata() + { + var definitionId = await ImportCamundaOrderProcessAsync(); + await SetNonBpmnMetadataOnDraftAsync(definitionId); + var metadataBeforeImport = await CaptureMetadataAsync(definitionId); + + // POST bpmn/import with a DefinitionId is a whole-definition import, unlike the document PUT above: it must + // keep replacing everything SetNonBpmnMetadataOnDraftAsync set, exactly as it did before that PUT preserved + // it. + using var content = new MultipartFormDataContent(); + AddBpmnFile(content, ReadAsset("camunda-order-process.bpmn"), "file"); + content.Add(new StringContent(definitionId), "DefinitionId"); + var importResponse = await PostAuthenticatedAsync("bpmn/import", content, "workflows/definitions:write"); + Assert.Equal(HttpStatusCode.OK, importResponse.StatusCode); + + var metadataAfterImport = await CaptureMetadataAsync(definitionId); + Assert.NotEqual(metadataBeforeImport, metadataAfterImport); + Assert.Equal("Order Process", metadataAfterImport.Name); + Assert.Null(metadataAfterImport.Description); + Assert.Equal(string.Empty, metadataAfterImport.VariableSummary); + Assert.Null(metadataAfterImport.UsableAsActivity); + Assert.Equal(string.Empty, metadataAfterImport.InputSummary); + Assert.Equal(string.Empty, metadataAfterImport.OutputSummary); + Assert.Equal(string.Empty, metadataAfterImport.OutcomeSummary); + Assert.Null(metadataAfterImport.ToolVersion); + Assert.False(metadataAfterImport.IsReadonly); + Assert.Null(metadataAfterImport.CustomPropertyValue); + } + [Fact] public async Task DocumentPut_WhenAuthenticatedWithoutTheRequiredPermission_ReturnsForbidden() { @@ -655,6 +709,87 @@ public class BpmnInterchangeEndpointTests(ITestOutputHelper testOutputHelper) : return document.ToJsonString(); } + /// + /// Changes the literal text the NotifyWarehouse task's elsa:activityBinding configures its bound + /// with — a real binding change, the kind Elsa Studio's binding UX makes, as opposed to + /// 's layout-only edit. + /// + private static string WithNotifyWarehouseTextChanged(string documentJson) => + documentJson.Replace("Notifying the warehouse", "Notifying the warehouse, rebound via document PUT"); + + /// + /// Renames the latest draft of and sets a description, a variable, an + /// activity-usable option, an input, an output, an outcome, a tool version, the read-only flag and a custom + /// property on it — the non-BPMN metadata a document PUT must leave untouched, set the way Studio's own + /// definitions API would ( then + /// ). + /// + private async Task SetNonBpmnMetadataOnDraftAsync(string definitionId) + { + using var scope = _app!.Services.CreateScope(); + var publisher = scope.ServiceProvider.GetRequiredService(); + var draft = await publisher.GetDraftAsync(definitionId, VersionOptions.Latest); + Assert.NotNull(draft); + + draft!.Name = "Renamed by the author, not by BPMN"; + draft.Description = "Handles a customer order end to end."; + draft.Variables = [new Variable("OrderReference", "unset")]; + draft.Options.UsableAsActivity = true; + draft.Inputs = [new InputDefinition { Name = "CustomerId", Type = typeof(string) }]; + draft.Outputs = [new OutputDefinition { Name = "OrderId", Type = typeof(string) }]; + draft.Outcomes = ["Fulfilled"]; + draft.ToolVersion = new Version(1, 2, 3); + draft.IsReadonly = true; + draft.CustomProperties["Custom:Owner"] = "fulfillment-team-lead"; + + await publisher.SaveDraftAsync(draft); + } + + /// A snapshot of everything sets, for before/after comparison. + private sealed record CapturedMetadata( + string? Name, + string? Description, + string VariableSummary, + bool? UsableAsActivity, + string InputSummary, + string OutputSummary, + string OutcomeSummary, + Version? ToolVersion, + bool IsReadonly, + string? CustomPropertyValue); + + private async Task CaptureMetadataAsync(string definitionId) + { + var definition = await FindLatestDefinitionAsync(definitionId); + var variableSummary = string.Join(";", definition.Variables.Select(variable => $"{variable.Name}={variable.Value}")); + var inputSummary = string.Join(";", definition.Inputs.Select(input => $"{input.Name}:{input.Type}")); + var outputSummary = string.Join(";", definition.Outputs.Select(output => $"{output.Name}:{output.Type}")); + var outcomeSummary = string.Join(";", definition.Outcomes); + definition.CustomProperties.TryGetValue("Custom:Owner", out var customPropertyValue); + + return new( + definition.Name, + definition.Description, + variableSummary, + definition.Options.UsableAsActivity, + inputSummary, + outputSummary, + outcomeSummary, + definition.ToolVersion, + definition.IsReadonly, + customPropertyValue); + } + + private async Task FindLatestDefinitionAsync(string definitionId) + { + using var scope = _app!.Services.CreateScope(); + var store = scope.ServiceProvider.GetRequiredService(); + var filter = WorkflowDefinitionHandle.ByDefinitionId(definitionId, VersionOptions.Latest).ToFilter(); + var definition = await store.FindAsync(filter); + Assert.NotNull(definition); + return definition!; + } + /// /// Saves the latest draft of the way the workflow-definition save endpoint Studio's /// designer calls does — , a re-serialized root, then diff --git a/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnDocumentRoundTripTests.cs b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnDocumentRoundTripTests.cs index f5cf9f60e..a27e2beaa 100644 --- a/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnDocumentRoundTripTests.cs +++ b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnDocumentRoundTripTests.cs @@ -2,6 +2,7 @@ using System.Text; using System.Xml.Linq; using Bpmn.Model; using Elsa.Bpmn.Interchange.Binding; +using Elsa.Bpmn.Interchange.Exceptions; using Elsa.Bpmn.Interchange.IntegrationTests.Scenarios.Binding; using Elsa.Bpmn.Interchange.IntegrationTests.Support; using Elsa.Bpmn.Interchange.Services; @@ -108,6 +109,26 @@ public class BpmnDocumentRoundTripTests : BpmnBindingTestBase Assert.Contains("Archiving the order", addedBinding.Descendants(Elsa + "input").Single().Value); } + [Fact(DisplayName = "Importing a document against a definition id that does not exist refuses rather than creating one")] + public async Task ImportDocumentAsync_WhenTheDefinitionDoesNotExist_ThrowsAndCreatesNothing() + { + var xml = ReadAsset("camunda-order-process.bpmn"); + var imported = await DocumentService.ImportAsync(xml, definitionId: null, name: null, processId: null, CancellationToken.None); + Assert.True(imported.ImportResult.Succeeded, string.Join("; ", imported.ImportResult.ValidationErrors.Select(error => error.Message))); + + var stored = await FindLatestAsync(imported.ImportResult.WorkflowDefinition.DefinitionId); + var document = DocumentService.ReadDocument(stored); + + var missingDefinitionId = $"{Guid.NewGuid()}-does-not-exist"; + + await Assert.ThrowsAsync(() => + DocumentService.ImportDocumentAsync(document, missingDefinitionId, processId: null, CancellationToken.None)); + + var filter = WorkflowDefinitionHandle.ByDefinitionId(missingDefinitionId, VersionOptions.Latest).ToFilter(); + var afterAttempt = await DefinitionStore.FindAsync(filter); + Assert.Null(afterAttempt); + } + /// Everything camunda-order-process.bpmn carries that an edit must not disturb: foreign attributes, foreign extension elements, the elsa: binding on the untouched task, and BPMN DI waypoints. private static void AssertContentEquivalent(XDocument expected, XDocument actual) { diff --git a/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnExportAvailabilityTests.cs b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnExportAvailabilityTests.cs index 37d84e1c8..91d2eb923 100644 --- a/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnExportAvailabilityTests.cs +++ b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnExportAvailabilityTests.cs @@ -9,6 +9,7 @@ using Elsa.Testing.Shared; using Elsa.Workflows.Management; using Elsa.Workflows.Management.Entities; using Elsa.Workflows.Management.Filters; +using Elsa.Workflows.Management.Mappers; using Elsa.Workflows.Management.Models; using Elsa.Workflows.Models; using Microsoft.Extensions.DependencyInjection; @@ -140,7 +141,8 @@ public class BpmnExportAvailabilityTests(ITestOutputHelper testOutputHelper) : B services.GetRequiredService(), services.GetRequiredService(), importer, - store); + store, + services.GetRequiredService()); var xml = BpmnAssetReader.Read("camunda-order-process.bpmn");