From 1b1ab207399a5034ed03cd24d886aa93ba4f2f5c Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Thu, 23 Apr 2026 22:51:16 +0200 Subject: [PATCH 1/2] fix: preserve commit error handling through describe/exec/describe `commit $X on error rollback;` could roundtrip as plain `commit $X;` after rewriting an MPR. The writer omitted `ErrorHandlingType` when it equaled rollback, treating it as implicit. The parser then defaulted an absent field to an empty value, so the next describe had no rollback setting to print. Make parser and writer behavior symmetric: default absent commit `ErrorHandlingType` to rollback while parsing, and always write commit `ErrorHandlingType`, defaulting to rollback. Tests cover parser and writer preservation for commit rollback handling. --- sdk/mpr/parser_microflow.go | 5 +++-- sdk/mpr/parser_microflow_test.go | 34 +++++++++++++++++++++++++++++ sdk/mpr/writer_microflow_actions.go | 11 ++++------ 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/sdk/mpr/parser_microflow.go b/sdk/mpr/parser_microflow.go index 2077b07a..6e2029b8 100644 --- a/sdk/mpr/parser_microflow.go +++ b/sdk/mpr/parser_microflow.go @@ -714,8 +714,9 @@ func parseCommitAction(raw map[string]any) *microflows.CommitObjectsAction { action.CommitVariable = extractString(raw["CommitVariableName"]) action.WithEvents = extractBool(raw["WithEvents"], false) action.RefreshInClient = extractBool(raw["RefreshInClient"], false) - if errType, ok := raw["ErrorHandlingType"].(string); ok { - action.ErrorHandlingType = microflows.ErrorHandlingType(errType) + action.ErrorHandlingType = microflows.ErrorHandlingType(extractString(raw["ErrorHandlingType"])) + if action.ErrorHandlingType == "" { + action.ErrorHandlingType = microflows.ErrorHandlingTypeRollback } return action } diff --git a/sdk/mpr/parser_microflow_test.go b/sdk/mpr/parser_microflow_test.go index b0f7df0e..41307a84 100644 --- a/sdk/mpr/parser_microflow_test.go +++ b/sdk/mpr/parser_microflow_test.go @@ -47,3 +47,37 @@ func TestParseSequenceFlow_NewCaseValueNoCase(t *testing.T) { t.Fatalf("expected *NoCase, got %T", flow.CaseValue) } } + +func TestParseCommitAction_ErrorHandlingTypeExplicit(t *testing.T) { + action := parseCommitAction(map[string]any{ + "$ID": "commit-1", + "CommitVariableName": "Order", + "WithEvents": true, + "RefreshInClient": false, + "ErrorHandlingType": "Continue", + }) + + if action.ErrorHandlingType != microflows.ErrorHandlingTypeContinue { + t.Errorf("expected Continue, got %q", action.ErrorHandlingType) + } + if action.CommitVariable != "Order" { + t.Errorf("expected CommitVariable Order, got %q", action.CommitVariable) + } +} + +func TestParseCommitAction_ErrorHandlingTypeDefaultsToRollback(t *testing.T) { + // When ErrorHandlingType is absent from BSON, the describer must still + // emit "on error rollback" — matching Mendix Studio Pro's default. + // Without this default, describe → exec → describe drops the suffix + // because the writer omits the field when it equals Rollback. + action := parseCommitAction(map[string]any{ + "$ID": "commit-1", + "CommitVariableName": "Order", + "WithEvents": false, + "RefreshInClient": false, + }) + + if action.ErrorHandlingType != microflows.ErrorHandlingTypeRollback { + t.Errorf("expected default Rollback, got %q", action.ErrorHandlingType) + } +} diff --git a/sdk/mpr/writer_microflow_actions.go b/sdk/mpr/writer_microflow_actions.go index 25c7ab08..a6030ef7 100644 --- a/sdk/mpr/writer_microflow_actions.go +++ b/sdk/mpr/writer_microflow_actions.go @@ -126,17 +126,14 @@ func serializeMicroflowAction(action microflows.MicroflowAction) bson.D { return doc case *microflows.CommitObjectsAction: - doc := bson.D{ + return bson.D{ {Key: "$ID", Value: idToBsonBinary(string(a.ID))}, {Key: "$Type", Value: "Microflows$CommitAction"}, {Key: "CommitVariableName", Value: a.CommitVariable}, + {Key: "ErrorHandlingType", Value: stringOrDefault(string(a.ErrorHandlingType), "Rollback")}, + {Key: "RefreshInClient", Value: a.RefreshInClient}, + {Key: "WithEvents", Value: a.WithEvents}, } - if a.ErrorHandlingType != "" && a.ErrorHandlingType != microflows.ErrorHandlingTypeRollback { - doc = append(doc, bson.E{Key: "ErrorHandlingType", Value: string(a.ErrorHandlingType)}) - } - doc = append(doc, bson.E{Key: "RefreshInClient", Value: a.RefreshInClient}) - doc = append(doc, bson.E{Key: "WithEvents", Value: a.WithEvents}) - return doc case *microflows.DeleteObjectAction: return bson.D{ From 1881f510076cda6ce117bd085c57f1a403898cfd Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Sun, 26 Apr 2026 23:55:37 +0200 Subject: [PATCH 2/2] test: add bug-test reproducer for commit on-error rollback roundtrip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an MDL script under mdl-examples/bug-tests/ that exercises describe → exec → describe fixpoint for `commit $X on error rollback;`, plus a negative control using `on error continue;`. Co-Authored-By: Claude Opus 4.7 --- ...308-commit-on-error-rollback-roundtrip.mdl | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 mdl-examples/bug-tests/308-commit-on-error-rollback-roundtrip.mdl diff --git a/mdl-examples/bug-tests/308-commit-on-error-rollback-roundtrip.mdl b/mdl-examples/bug-tests/308-commit-on-error-rollback-roundtrip.mdl new file mode 100644 index 00000000..564a86d0 --- /dev/null +++ b/mdl-examples/bug-tests/308-commit-on-error-rollback-roundtrip.mdl @@ -0,0 +1,53 @@ +-- ============================================================================ +-- Bug #308: `commit ... on error rollback` lost during describe → exec → describe +-- ============================================================================ +-- +-- Symptom (before fix): +-- Authoring `commit $X on error rollback;` and then re-executing the +-- describe output silently dropped the `on error rollback` clause — +-- subsequent describes printed plain `commit $X;`. In Studio Pro the +-- "Action call action" still showed Rollback, but mxcli's view of the +-- model diverged from the BSON, so any further mutation could mis-write +-- the field. +-- +-- Root cause: +-- The commit writer omitted `ErrorHandlingType` when its value was +-- Rollback (treating Rollback as implicit). The parser then left the +-- absent field as the empty string, so the describer printed `commit $X` +-- without a clause. +-- +-- After fix: +-- Parser defaults absent commit `ErrorHandlingType` to Rollback; the +-- writer always emits the field, defaulting to Rollback. The behaviour +-- is symmetric and round-trips cleanly. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/308-commit-on-error-rollback-roundtrip.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe microflow BugTest308.MF_CommitRollback" +-- The output must contain `on error rollback` and must be a fixpoint +-- under describe → exec → describe. +-- ============================================================================ + +create module BugTest308; + +create entity BugTest308.Item ( + Name : string(100) +); +/ + +create microflow BugTest308.MF_CommitRollback ( + $Item: BugTest308.Item +) +begin + commit $Item on error rollback; +end; +/ + +-- Negative control: explicit `on error continue` must survive equally. +create microflow BugTest308.MF_CommitContinue ( + $Item: BugTest308.Item +) +begin + commit $Item on error continue; +end; +/