diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 92a698438..4947fb831 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -728,3 +728,5 @@ {"area": "mdl/executor", "date": "2026-09-26", "symptom": "`describe fragment from page M.P widget w` (and `from snippet`) fails for every container and every widget — including ones `describe page` prints — with `not found in page M.P`, a message that does not even name the widget", "cause": "Visitor stores DescribeFragmentFromStmt.ContainerType as \"PAGE\"/\"SNIPPET\"; describeFragmentFrom switched on \"page\"/\"snippet\" with no default, so neither branch ran, the widget list stayed empty, and the fall-through reported the widget missing. Third instance of this split: ALTER PAGE (#402), DESCRIBE/ALTER STYLING (#631)", "file": "`mdl/executor/cmd_fragments.go` (`describeFragmentFrom`)", "insight": "The mismatch hid behind a plausible error because a switch on the discriminator had no default: an unmatched container type looked like an empty container, and an empty container looks like a missing widget. Normalise with strings.ToLower where the discriminator is consumed (the house convention — cmd_styling, cmd_alter_page, validate_alter_* all do) AND make the default an error, so the next casing drift fails loudly instead of reporting the wrong thing. The existing mock tests hand-built the AST and so agreed with the handler; only a test that goes visitor.Build → NewRegistry().Dispatch pins the contract between the two layers (cmd_fragments_from_test.go). Verified on Evora: Administration.Account_Edit/textBox6 and AgentCommons.Snippet_Agent_Details/dataView7 now describe", "refs": ["#402", "#631"]} {"area": "mdl/executor", "date": "2026-09-26", "symptom": "describe output that does not re-parse or loses data (ako/mxcli#707): an entity string default or validation message containing ' was emitted unescaped; so were module-role descriptions, published OData/REST Path/Version/Namespace/Summary/Folder, and REST client BaseUrl/Path/header values; an agent `mcp service` block with a Description lacked the comma after `Enabled`; workflow decision / parallel split captions came back only as `-- caption` comments (replay reset them to 'Decision' / 'Parallel split'); `describe demo user` emitted `password '***'`, which replay stored as the password; `describe settings` printed `DatabasePassword = ''`.", "cause": "Hand-rolled `'%s'` emit sites that put the quotes and the escaping in different places (the #1006 source-scan guard covered only cmd_workflows.go); a block emitter with no separator logic, unlike its sibling; captions treated as commentary although the grammar has `comment '…'` for both activities; secrets printed as data, with a placeholder the writer took literally.", "file": "mdl/executor/cmd_entities_describe.go, cmd_security.go, cmd_security_write.go, cmd_odata.go, cmd_published_rest.go, cmd_rest_clients.go, cmd_agenteditor_agents.go, cmd_workflows.go, cmd_settings.go", "fix": "Every emit site uses mdlQuoted; TestDescribers_HaveNoHandRolledStringLiterals now scans all seven describer files. MCP block writes the comma like the tool block. workflowCaptionClauses emits `comment '…'` for a non-default caption and computes the name clause against the caption the writer will store. DatabasePassword is omitted with a comment (create or modify is a patch, so replay keeps it). Demo users are described as `create or modify … password '***'`, and the executor treats '***' as 'keep the stored password', refusing it for a user that does not exist.", "insight": "Assert round trips by reparsing describe output with the real visitor and comparing the AST value to the stored one, not by substring. For secrets the right placeholder is one the WRITER understands: omission works where the create is a patch (configuration); where the grammar requires the value (demo user) give the placeholder a meaning (keep stored) and refuse it where that meaning is empty, so a replay can neither leak nor silently set a credential.", "test": "mdl/executor/issue707_describe_roundtrip_test.go"} {"area": "mdl/executor", "date": "2026-09-26", "symptom": "describe microflow \u2026 with handles (ako/mxcli#713) printed no handle for an activity inside an `on error { \u2026 }` block, and the alter-target resolver counted such activities after the whole main flow: on SUB_Feedback_PostToAppInsights `return * @1` picked `return $Response`, although describe prints the handler's `return empty` first. Also `$Response` did not address a REST call whose output is on its result handling (and cast, create list, web service, workflow, XML/JSON, database-query outputs).", "cause": "Error-handler bodies are rendered by collectErrorHandlerStatements, a second describer that returned bare strings and never wrote the source map, so those nodes had no line to rank or print a handle at; unranked candidates were appended last. The output-variable switch was copied from actionOutputVariableName, which had drifted from the formatter.", "file": "mdl/executor/cmd_microflows_show_helpers.go; mdl/backend/mfmutator/target.go", "fix": "collectErrorHandlerStatementSpans reports each handler-body object's statement span; emitActivityStatement and emitCommentedErrorHandler record them in the source map (additive entries in ELK sourceMap too). mfmutator.OutputVariable reads the variable where the formatter does, for every action it prints as `$X = \u2026`. A comment rendering (`-- Unsupported \u2026`) is no statement, so it never becomes a handle.", "insight": "Any node the describer prints through a side path must enter the source map, or everything keyed on print order (ordinals, handles, ELK highlighting) silently disagrees with the text. Check ranking against a Studio Pro flow that has a handler body, not just VAL_Feedback.", "test": "TestDescribeWithHandles_ErrorHandlerBody, TestMicroflowTargets_PedAppEveryFlowRanksAndResolves, TestOutputVariable_EveryActionDescribePrintsAnAssignmentFor, TestCandidate_CommentRenderingIsNoStatement"} +{"area": "mdl/executor", "date": "2026-09-27", "symptom": "describe microflow \u2026 with handles printed `-- handle: commit $Order on error {` for an activity with a custom error handler (TestApp Services.SaveOrder). The handle could not be used: an `alter microflow` target ends at the `{` that opens a fragment, so `insert after commit $Order on error { \u2026 }` parsed the handler brace as the fragment.", "cause": "printedStatement ended an action's statement at a line ending in `;` or `{` and kept the `{`, which belongs to the error-handler block describe opens, not to the statement.", "file": "mdl/executor/cmd_microflows_handles.go", "fix": "printedStatement strips the trailing `{` of an error-handler block opener, so the handle is `commit $Order on error`, which parses as a target and still matches the activity.", "insight": "A handle is only useful if it can be written back as a target in the grammar that consumes it; test a printed handle by parsing it, not only by resolving it.", "test": "TestPrintedStatement_ErrorHandlerBlockOpenerIsNotPartOfTheStatement"} +{"area": "mdl/executor", "date": "2026-09-27", "symptom": "alter microflow (ako/mxcli#736) reported \"Altered microflow\" for statements mx check then rejected: two fragments in one statement declaring the same variable gave CE0111 Duplicate variable name; a fragment reading $X in the same statement as `drop $X` (either order) gave CE0109 Undefined variable. A loop fragment was refused as reading its own iterator, and drop/replace of a Studio Pro loop with more than one body activity was refused by the dangling-reference guard.", "cause": "The scope checks compared each operation with the flow as stored, never with what earlier operations of the same statement had declared, read or removed. The iterator of a fragment's loop is on its LoopSource, not an action output. Loop body flows are stored in the unit's Flows list, not in the loop, so removing the loop left them pointing at removed objects.", "file": "mdl/executor/cmd_alter_flow.go; mdl/backend/mfmutator/splice.go", "fix": "alterFlowContext tracks declaredByOps / readByOps / removedByOps across the statement's operations and checks each later operation against them; checkFragmentScope counts a fragment loop's iterator as its own; Drop and Replace remove graph.bodyFlows of a loop with it.", "insight": "A per-operation check against the stored document is only sound for a one-operation statement; every multi-operation test needs a case where operation 2 depends on operation 1. mx check on a copied TestApp is the cheap oracle: the CE numbers appear the moment a hygiene hole is hit.", "test": "TestAlterMicroflow_PedApp_ScopeSpansTheStatement; TestAlterMicroflow_PedApp_LoopFragmentDeclaresItsIterator; TestSplice_DropOrReplaceALoopTakesItsBodyFlows"} diff --git a/.claude/skills/mendix/write-microflows/SKILL.md b/.claude/skills/mendix/write-microflows/SKILL.md index 1bb0fe887..1c5397194 100644 --- a/.claude/skills/mendix/write-microflows/SKILL.md +++ b/.claude/skills/mendix/write-microflows/SKILL.md @@ -283,6 +283,14 @@ declare $status Enumeration(Module.OrderStatus) = Module.OrderStatus.Open; > The `calendar*Between` functions (`calendarMonthsBetween`, `calendarYearsBetween`) > return whole units (Integer) and are fine to assign directly. +### Changing a variable: always `set` + +`set $Counter = $Counter + 1;` changes a variable. `$x = …` without `set` is an +activity that creates `$x` — required under `mdl 1;` (`MDL-V1-SET` otherwise). +List operations and aggregates are one statement per activity and never nest: +`$Open = filter $Orders by Status = M.Status.Open;` then `$N = count $Open;` — +see [`reference/data-operations.md`](reference/data-operations.md#one-statement-per-activity). + ### ❌ INCORRECT Syntax ```mdl @@ -676,8 +684,8 @@ close page on error { return; }; of them; `continue` is fine on `declare`, `set`, `retrieve`, `delete` and `call microflow`. Measured on 11.14.0 — note that create-*variable* and change-*variable* accept `continue` while change-*object* does not. -- **The list-operation and aggregate forms of `set`** (`$x = head($l)`, - `$n = count($l)`) have no error handling in Mendix at all — **MDL077**. +- **List operations and aggregates** (`$x = head $l;`, `$n = count $l;`) + have no error handling in Mendix at all — **MDL077**. **In a nanoflow, almost none of them take a clause at all.** `change`, `log`, `show page`, `close page`, `show message` and `validation feedback` are CE6035 diff --git a/.claude/skills/mendix/write-microflows/reference/data-operations.md b/.claude/skills/mendix/write-microflows/reference/data-operations.md index 6ed4590d2..e10bf2cff 100644 --- a/.claude/skills/mendix/write-microflows/reference/data-operations.md +++ b/.claude/skills/mendix/write-microflows/reference/data-operations.md @@ -117,18 +117,65 @@ add head($SourceItems) to $Items; Use expression-valued `add` only when the expression returns an object compatible with the target list element type. +### One statement per activity + +Every list operation and aggregate is **one Studio Pro activity**, and it is +written as one statement that mirrors it: the keyword is the operation's name +in the activity's dialog, and the inputs are the ones the dialog asks for. The +operand is always a **variable**, because the dialog selects a variable — so +one activity cannot be nested inside another, just as it cannot be drawn. + +| Studio Pro activity: operation | MDL statement | +|---|---| +| List operation: Filter | `$Open = filter $Orders by Status = Shop.Status.Open;` | +| List operation: Filter by expression | `$Big = filter $Orders where $currentObject/Total > 1000;` | +| List operation: Find | `$Order = find $Orders by Number = $Number;` | +| List operation: Find by expression | `$Late = find $Orders where $currentObject/DueDate < [%CurrentDateTime%];` | +| List operation: Sort | `$Sorted = sort $Orders by OrderDate desc, Number asc;` | +| List operation: Head / Tail | `$First = head $Orders;` / `$Rest = tail $Orders;` | +| List operation: Range | `$Page = range $Orders offset 20 limit 10;` | +| List operation: Union / Intersect / Subtract | `$All = union $A with $B;` / `$Both = intersect $A with $B;` / `$Left = subtract $B from $A;` | +| List operation: Contains / Equals | `$Has = contains $Order in $Orders;` / `$Same = equals $A and $B;` | +| Aggregate list: Count | `$N = count $Orders;` | +| Aggregate list: Sum / Average / Minimum / Maximum | `$Total = sum $Orders by Amount;` or `$Total = sum $Orders of $currentObject/Amount * 1.21;` | +| Aggregate list: All / Any | `$AllPaid = all $Orders where $currentObject/Paid;` | +| Aggregate list: Reduce | `$Csv = reduce $Orders from '' as String using $currentResult + $currentObject/Name;` | + +`by` picks a member (the dialog's attribute or association selector) and `where` +/ `of` take an expression — the "… by expression" variants. `subtract $B from $A` +is A minus B. + +Two statements, never one nested call: + +```mdl +$Approved = filter $Orders where $currentObject/Status = Shop.Status.Approved; +$Count = count $Approved; +``` + +The older **call forms** (`$x = filter($L, …)`, `$n = count($L)`, `sum($L.Attr)`) +still parse and build the same activity, but they are deprecated +(`MDL-DEPR003`, `MDL-DEPR004`) — do not write them. Under the `mdl 1;` header: + +- `set` is **mandatory** to change a variable: `set $Total = $Total + 1;`. A + statement without `set` is only ever an activity (`MDL-V1-SET` under mdl 0). +- `$x = find(…)` and `$x = contains(…)` are refused: the call is also Mendix's + **string** function. `set $Pos = find($Text, 'a');` is the string function; + `$Match = find $L where …;` is the list operation (`MDL-V1-LIST`). +- a nested call, or a list operation after `set`, is refused (`MDL-V1-LIST`). + Without the header a nested operand is refused at check time as `MDL-LISTOP02`. + ### `range` — paging a list -`range` takes the **offset first, then the amount**, and Mendix requires at -least one of them: +`range` takes an `offset` and a `limit` (Studio Pro's *Offset* and *Amount*), and +Mendix requires at least one of them: ```mdl -$Page = range($Sorted, $Offset, $PageSize); -- skip $Offset, take $PageSize -$First = range($Sorted, 0, 10); -- first 10 -$Rest = range($Sorted, $Offset); -- skip $Offset, take the rest +$Page = range $Sorted offset $Offset limit $PageSize; -- skip $Offset, take $PageSize +$First = range $Sorted limit 10; -- first 10 +$Rest = range $Sorted offset $Offset; -- skip $Offset, take the rest ``` -`range($List)` with no bound builds nothing useful and fails with **CE6520** +`range $List;` with no bound builds nothing useful and fails with **CE6520** ("Amount and offset are not specified. Either amount or offset or both must be specified."); `mxcli check` refuses it first as **MDL068**. To use the whole list, drop the activity and use the list variable directly. @@ -142,89 +189,95 @@ retrieve $Page from Sales.Order where [Status = 'Open'] sort by OrderDate desc limit $PageSize offset $Offset; ``` -Note the clause order there is `limit` then `offset` — the reverse of `range`'s -argument order, because each mirrors the Mendix editor it comes from. - -### `filter` / `find` test one item at a time — `$currentObject` +### `filter` / `find` — `by` a member, or `where` an expression over `$currentObject` -A FILTER/FIND predicate is an expression Mendix evaluates once per item, with -the item bound to **`$currentObject`**. That is the only iterator name there is: +`by Member = value` is Studio Pro's *Filter* / *Find*: an attribute or +association of the list's entity and the value it must have. Anything else is +the *by expression* operation, written after `where`, which Mendix evaluates +once per item with the item bound to **`$currentObject`** — the only iterator +name there is: ```mdl -$Pending = filter($Orders, $currentObject/Status = 'Pending'); -$Large = filter($Orders, $currentObject/Amount > 1000); -$Match = find($Orders, $currentObject/OrderNumber = $Wanted); +$Pending = filter $Orders by Status = Shop.Status.Pending; +$Large = filter $Orders where $currentObject/Amount > 1000; +$Match = find $Orders by OrderNumber = $Wanted; ``` -A **bare attribute name** means the same thing — mxcli resolves it against the -list's entity and writes `$currentObject/Attr`: +A **bare attribute name** after `where` means the same thing — mxcli resolves it +against the list's entity and writes `$currentObject/Attr`: ```mdl -$Open = filter($Orders, Status != 'Closed'); -- stored as $currentObject/Status +$Open = filter $Orders where Status != 'Closed'; -- stored as $currentObject/Status ``` -Two things are refused rather than passed through to the build: +`by` accepts only `Member = value`; `filter $L by Amount > 3` is refused with a +pointer to `where`. Two things are refused after `where` rather than passed +through to the build: - a bare name that is **not** a member of the list's entity (this used to reach mxbuild as `CE0117 "Error(s) in expression."`); -- any **other** iterator name — `filter($L, $item/Amount > 0)` is `MDL-LISTOP01`, - pre-empting `CE0109 "Undefined variable 'item'"`. +- any **other** iterator name — `filter $L where $item/Amount > 0` is + `MDL-LISTOP01`, pre-empting `CE0109 "Undefined variable 'item'"`. `$item` is still fine when it is genuinely in scope, which is how the O(N) lookup -idiom is written: inside `loop $item in $L`, `find($Others, Key = $item/Key)` +idiom is written: inside `loop $item in $L`, `find $Others by Key = $item/Key` navigates the **loop's** variable on the right-hand side. -**`sort` is not an expression.** It takes attribute names directly, so a bare -attribute is the only spelling — `sort($Orders, CreateDate desc)`. Writing -`$currentObject/` there is wrong. - +**`sort` is not an expression.** It takes attribute names directly — +`sort $Orders by CreateDate desc` — and any word works as a name there, so an +attribute called `Count` or `Date` needs no quotes. Writing `$currentObject/` +there is wrong. -### `contains` is overloaded — string vs list +### `contains` — string function vs list operation -`contains(a, b)` is both a **string** function (`contains(haystack, needle)` → substring test) and a **list** operation (`contains(list, object)` → membership test). mxcli picks the right serialization automatically: +The list operation is `contains $Object in $List`, and it creates its own +Boolean output variable, so do not declare it first. The **string** function +`contains(haystack, needle)` is an expression, assigned with `set` to a variable +declared first: ```mdl +-- LIST contains — the statement creates $Found +$Found = contains $Item in $Items; + -- STRING contains — assign to a PRE-DECLARED Boolean (a Change Variable action) declare $HasAt Boolean = false; set $HasAt = contains($Email, '@'); - --- LIST contains — do NOT pre-declare the output (the list op creates it) -set $Found = contains($Items, $Item); ``` -The distinction: a **literal or computed** second argument is always the string function. When both arguments are plain variables, the input variable's declared type decides — a **String** input becomes the string function (Change Variable, so declare the Boolean first), anything else stays a list operation (which creates its own output variable, so leave it undeclared). Getting the declare wrong is what triggers `CE0111 "Duplicate variable name"`. +Under `mdl 1;` that is the whole rule. Without the header, the call form +`$Found = contains($Items, $Item)` is still read as the list operation when both +arguments are plain variables and the first is not a declared String — the +guess that `mdl 1` removes. Getting the declare wrong is what triggers `CE0111 +"Duplicate variable name"`. ### Aggregates — all eight, including `reduce`, `all` and `any` An Aggregate list activity folds a list into one value. `count` takes only the -list; the rest take either an **attribute** or an **expression** over -`$currentObject`. +list; `sum`, `average`, `minimum` and `maximum` aggregate an attribute (`by`) or +an expression over `$currentObject` (`of`) — the dialog's *Aggregate with*. ```mdl -$Count = count($Orders); -$Total = sum($Orders.Amount); -- attribute form -$Total = sum($Orders, $currentObject/Amount * 1.21); -- expression form -$Avg = average($Orders.Amount); -$Min = minimum($Orders.Amount); -$Max = maximum($Orders.Amount); +$Count = count $Orders; +$Total = sum $Orders by Amount; -- attribute +$Total = sum $Orders of $currentObject/Amount * 1.21; -- expression +$Avg = average $Orders by Amount; +$Min = minimum $Orders by Amount; +$Max = maximum $Orders by Amount; -- Boolean predicates over every item. No seed, always Boolean. -$AllPaid = all($Orders, $currentObject/Paid); -$AnyLate = any($Orders, $currentObject/DueDate < [%CurrentDateTime%]); +$AllPaid = all $Orders where $currentObject/Paid; +$AnyLate = any $Orders where $currentObject/DueDate < [%CurrentDateTime%]; -- REDUCE folds with a running total. $currentResult is the accumulator. -$Discounted = reduce( - $Orders, - $currentResult + $currentObject/Amount * 0.9, - initial: 0, - returns: Decimal -); +$Discounted = reduce $Orders from 0 as Decimal + using $currentResult + $currentObject/Amount * 0.9; ``` -**`reduce` needs `initial:` and `returns:` and neither can be inferred.** Mendix -stores both beside the expression, and the fold is meaningless without a seed -and a result type — so MDL makes them mandatory rather than guessing. `all` and -`any` take neither: they never accumulate, and always fold to Boolean. +**`reduce` needs the initial value (`from`) and the return type (`as`), and +neither can be inferred.** Mendix stores both beside the expression, and the +fold is meaningless without a seed and a result type — so MDL makes them +mandatory rather than guessing. `all` and `any` take neither: they never +accumulate, and always fold to Boolean. Do not reach for `reduce` where `sum` will do. It exists for folds Mendix has no dedicated function for — running a string together, or carrying a value forward diff --git a/cmd/mxcli/lsp_completions_gen.go b/cmd/mxcli/lsp_completions_gen.go index 739fd48d8..19568e7d7 100644 --- a/cmd/mxcli/lsp_completions_gen.go +++ b/cmd/mxcli/lsp_completions_gen.go @@ -166,6 +166,7 @@ var mdlGeneratedKeywords = []protocol.CompletionItem{ {Label: "REDUCE", Kind: protocol.CompletionItemKindKeyword, Detail: "Microflow keyword"}, {Label: "ANY", Kind: protocol.CompletionItemKindKeyword, Detail: "Microflow keyword"}, {Label: "INITIAL", Kind: protocol.CompletionItemKindKeyword, Detail: "Microflow keyword"}, + {Label: "USING", Kind: protocol.CompletionItemKindKeyword, Detail: "Microflow keyword"}, {Label: "LIST", Kind: protocol.CompletionItemKindKeyword, Detail: "Microflow keyword"}, {Label: "REMOVE", Kind: protocol.CompletionItemKindKeyword, Detail: "Microflow keyword"}, {Label: "EQUALS", Kind: protocol.CompletionItemKindKeyword, Detail: "Microflow keyword"}, diff --git a/cmd/mxcli/syntax/features_microflow.go b/cmd/mxcli/syntax/features_microflow.go index bc7dc3a74..637de99d4 100644 --- a/cmd/mxcli/syntax/features_microflow.go +++ b/cmd/mxcli/syntax/features_microflow.go @@ -176,8 +176,8 @@ func init() { "-- FEEDBACK -> MDL076. A custom handler IS accepted on all of them, and\n" + "-- CONTINUE is fine on DECLARE, SET, RETRIEVE, DELETE and CALL MICROFLOW.\n" + "--\n" + - "-- The list-operation and aggregate forms of SET ($x = head($l),\n" + - "-- $n = count($l)) have no error handling in Mendix at all -> MDL077.\n" + + "-- List operations and aggregates ($x = head $l, $n = count $l) have\n" + + "-- no error handling in Mendix at all -> MDL077.\n" + "--\n" + "-- IN A NANOFLOW only DECLARE and SET take a clause at all. CHANGE, LOG,\n" + "-- SHOW PAGE, CLOSE PAGE, SHOW MESSAGE and VALIDATION FEEDBACK are CE6035\n" + @@ -383,9 +383,67 @@ func init() { // A CE number in the cached `syntax --json` index leads an agent // debugging the build error back to the right topic (issue #1002). "$currentObject", "predicate", "CE0117", "CE0109", "MDL-LISTOP01", + "contains", "equals", "by", "where", "MDL-DEPR003", "MDL-DEPR004", "MDL-V1-LIST", }, - Syntax: "$List = CREATE LIST OF Module.Entity;\nADD $Item TO $List;\nREMOVE $Item FROM $List;\n$Result = HEAD($List);\n$Result = TAIL($List);\n$Result = FIND($List, predicate);\n$Result = FILTER($List, predicate);\n$Result = SORT($List, attr ASC);\n$Result = UNION($L1, $L2);\n$Result = INTERSECT($L1, $L2);\n$Result = SUBTRACT($L1, $L2);\n$Result = RANGE($List, offset, amount);\n$Result = RANGE($List, offset);\n\n-- Aggregates. Mendix has eight; each takes an attribute or an expression\n-- over $currentObject.\n$Count = COUNT($List);\n$Sum = SUM($List.Attr);\n$Sum = SUM($List, expression);\n$Avg = AVERAGE($List.Attr);\n$Min = MINIMUM($List.Attr);\n$Max = MAXIMUM($List.Attr);\n$AllMatch = ALL($List, boolean-expression);\n$AnyMatch = ANY($List, boolean-expression);\n\n-- REDUCE folds the list into one value. $currentResult is the running\n-- total; both INITIAL and RETURNS are required and cannot be inferred.\n$Folded = REDUCE($List, expression, initial: value, returns: Type);\n\n-- RANGE takes OFFSET first, then AMOUNT, and needs at least ONE of them:\n-- RANGE($L, $Offset, $Amount) page: skip $Offset, take $Amount\n-- RANGE($L, 0, $Amount) first $Amount\n-- RANGE($L, $Offset) skip $Offset, take the rest\n-- RANGE($L) with no bound is CE6520 at build time (mxcli check: MDL068).\n\nA FIND/FILTER predicate is evaluated once per item, and Mendix binds the\nitem to $currentObject -- the same variable the aggregate expressions above\nuse, and the only iterator name there is:\n\n FILTER($Orders, $currentObject/Amount > 0)\n\nA bare attribute name means the same thing; mxcli resolves it against the\nlist's entity and writes $currentObject/Attr. A name that is not a member\nof that entity is refused, and naming any other variable is MDL-LISTOP01.\n\nSORT is not an expression -- it takes attribute names directly, so a bare\nattribute is the only spelling there.\n\nEvery list operation above is a separate ACTIVITY, and an activity stores\nits list as a VARIABLE. They do not nest: COUNT(FILTER($L, ...)) is not a\nshorter spelling of two statements, it is a list argument Mendix cannot\nstore. mxcli refuses it as MDL-LISTOP02; give the inner operation its own\nstatement and pass the variable:\n\n $Approved = FILTER($Orders, $currentObject/Status = 'Approved');\n $Count = COUNT($Approved);", - Example: "$AllOrders = CREATE LIST OF MyModule.Order;\nADD $NewOrder TO $AllOrders;\n$First = HEAD($AllOrders);\n\n-- The item under test is $currentObject\n$Pending = FILTER($AllOrders, $currentObject/Status = 'Pending');\n$Large = FILTER($AllOrders, $currentObject/Amount > 1000);\n\n-- A bare attribute name is resolved against the list's entity\n$Open = FILTER($AllOrders, Status != 'Closed');\n\n-- SORT takes attribute names, not an expression\n$Sorted = SORT($Pending, CreateDate DESC);\n$Page = RANGE($Sorted, $Offset, $PageSize);\n$Total = SUM($AllOrders.Amount);\n$AllPaid = ALL($AllOrders, $currentObject/Paid);\n$AnyLate = ANY($AllOrders, $currentObject/DueDate < [%CurrentDateTime%]);\n\n-- List operations do not nest -- one statement each (MDL-LISTOP02)\n-- WRONG: $Count = COUNT(FILTER($AllOrders, $currentObject/Paid));\n$Paid = FILTER($AllOrders, $currentObject/Paid);\n$Count = COUNT($Paid);\n$Discounted = REDUCE(\n $AllOrders,\n $currentResult + $currentObject/Amount * 0.9,\n initial: 0,\n returns: Decimal\n);", + Syntax: "$List = CREATE LIST OF Module.Entity;\nADD $Item TO $List;\nREMOVE $Item FROM $List;\n\n" + + "-- One statement per Studio Pro activity. The keyword is the operation's\n" + + "-- name and the operand is always a variable, as in the activity's dialog.\n" + + "-- List operation:\n" + + "$Result = HEAD $List;\n$Result = TAIL $List;\n" + + "$Result = FIND $List BY Member = value; -- Find (attribute or association)\n" + + "$Result = FIND $List WHERE expression; -- Find by expression\n" + + "$Result = FILTER $List BY Member = value; -- Filter\n" + + "$Result = FILTER $List WHERE expression; -- Filter by expression\n" + + "$Result = SORT $List BY Attr DESC, Attr2 ASC;\n" + + "$Result = UNION $L1 WITH $L2;\n$Result = INTERSECT $L1 WITH $L2;\n" + + "$Result = SUBTRACT $L2 FROM $L1; -- $L1 minus $L2\n" + + "$Bool = CONTAINS $Object IN $List;\n$Bool = EQUALS $L1 AND $L2;\n" + + "$Result = RANGE $List OFFSET offset LIMIT amount;\n\n" + + "-- Aggregate list: BY an attribute, or OF an expression over $currentObject.\n" + + "$Count = COUNT $List;\n$Sum = SUM $List BY Attr;\n$Sum = SUM $List OF expression;\n" + + "$Avg = AVERAGE $List BY Attr;\n$Min = MINIMUM $List BY Attr;\n$Max = MAXIMUM $List BY Attr;\n" + + "$AllMatch = ALL $List WHERE boolean-expression;\n$AnyMatch = ANY $List WHERE boolean-expression;\n\n" + + "-- REDUCE folds the list into one value. $currentResult is the running\n" + + "-- total; the initial value and the type are required and cannot be inferred.\n" + + "$Folded = REDUCE $List FROM initial AS Type USING expression;\n\n" + + "-- RANGE needs at least ONE bound:\n" + + "-- RANGE $L OFFSET $Offset LIMIT $Amount page: skip $Offset, take $Amount\n" + + "-- RANGE $L LIMIT $Amount first $Amount\n" + + "-- RANGE $L OFFSET $Offset skip $Offset, take the rest\n" + + "-- RANGE with no bound is CE6520 at build time (mxcli check: MDL068).\n\n" + + "BY names a member and the value it must have; anything else goes after\n" + + "WHERE, which is evaluated once per item with the item bound to\n" + + "$currentObject -- the same variable the aggregate expressions use:\n\n" + + " FILTER $Orders WHERE $currentObject/Amount > 0\n\n" + + "A bare attribute name after WHERE means the same thing; mxcli resolves it\n" + + "against the list's entity and writes $currentObject/Attr. Naming any\n" + + "other variable there is MDL-LISTOP01.\n\n" + + "Because the operand is a variable, activities cannot nest. Give each\n" + + "its own statement:\n\n" + + " $Approved = FILTER $Orders WHERE $currentObject/Status = 'Approved';\n" + + " $Count = COUNT $Approved;\n\n" + + "The call forms ($x = FILTER($L, ...), $n = COUNT($L)) are deprecated\n" + + "aliases (MDL-DEPR003, MDL-DEPR004). $x = FIND(...) and CONTAINS(...) are\n" + + "also Mendix's string functions: under `mdl 1;` the call form is refused\n" + + "and `SET $x = find($Text, 'a')` is always the string function\n" + + "(MDL-V1-LIST). Without the header a nested call is refused as\n" + + "MDL-LISTOP02.", + Example: "$AllOrders = CREATE LIST OF MyModule.Order;\nADD $NewOrder TO $AllOrders;\n$First = HEAD $AllOrders;\n\n" + + "-- Filter by member, and by expression over $currentObject\n" + + "$Pending = FILTER $AllOrders BY Status = MyModule.OrderStatus.Pending;\n" + + "$Large = FILTER $AllOrders WHERE $currentObject/Amount > 1000;\n" + + "$Match = FIND $AllOrders BY OrderNumber = $Number;\n\n" + + "-- SORT takes attribute names, not an expression\n" + + "$Sorted = SORT $Pending BY CreateDate DESC;\n" + + "$Page = RANGE $Sorted OFFSET $Offset LIMIT $PageSize;\n" + + "$Total = SUM $AllOrders BY Amount;\n" + + "$AllPaid = ALL $AllOrders WHERE $currentObject/Paid;\n" + + "$AnyLate = ANY $AllOrders WHERE $currentObject/DueDate < [%CurrentDateTime%];\n\n" + + "-- One statement per activity\n" + + "$Paid = FILTER $AllOrders WHERE $currentObject/Paid;\n" + + "$Count = COUNT $Paid;\n" + + "$Discounted = REDUCE $AllOrders FROM 0 AS Decimal\n" + + " USING $currentResult + $currentObject/Amount * 0.9;", SeeAlso: []string{"microflow.retrieve"}, }) @@ -400,6 +458,41 @@ func init() { Example: "LOG INFO NODE 'OrderService' 'Order created successfully';\nLOG WARNING 'Customer not found';\nLOG ERROR 'Failed to process {1}' WITH (\n {1} = $OrderNumber\n);", }) + Register(SyntaxFeature{ + Path: "microflow.alter", + Summary: "Patch a stored microflow or nanoflow: insert, replace or drop activities in place", + Keywords: []string{ + "alter microflow", "alter nanoflow", "insert after", "insert before", + "replace", "drop activity", "patch microflow", "splice", "handle", + }, + Syntax: "ALTER MICROFLOW|NANOFLOW Module.Name {\n" + + " INSERT AFTER|BEFORE <target> { <statements> }\n" + + " REPLACE <target> WITH { <statements> }\n" + + " DROP <target>;\n" + + "};\n\n" + + "-- <target> addresses one activity by content, as `describe microflow ... with handles` prints it:\n" + + "-- $Var the activity that outputs $Var\n" + + "-- 'Caption' a decision or an activity with a custom caption\n" + + "-- <statement> a statement pattern; * matches any run of tokens\n" + + "-- followed by @n when it matches more than one. Targets are resolved against the\n" + + "-- stored flow before any operation runs; an ambiguous or unknown target is an error.\n" + + "-- Only the new activities, the rewired flows and the objects moved to make room change;\n" + + "-- every other element keeps its $ID, position and curve.\n" + + "-- Refused: insert after a decision, insert before an activity several flows enter,\n" + + "-- drop/replace of a decision or of an activity with an error handler, anything inside\n" + + "-- a loop body, a fragment that returns, and a fragment variable that clashes with one\n" + + "-- the flow has or reads one not declared on the path. Over --mcp only insert is supported.", + Example: "alter microflow FeedbackModule.VAL_Feedback {\n" + + " insert after $IsValidEmail { log info node 'Feedback' 'Email checked'; }\n" + + " replace set $ValidFeedback = false @3 with {\n" + + " set $ValidFeedback = false;\n" + + " log warning node 'Feedback' 'Email rejected';\n" + + " }\n" + + " drop log debug node 'Feedback' *;\n" + + "};", + SeeAlso: []string{"microflow"}, + }) + Register(SyntaxFeature{ Path: "microflow.show-page", Summary: "Open and close pages from microflows", diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 1abf19730..6ef196712 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -483,6 +483,8 @@ rather than updating the first. | Describe microflow | `describe microflow Module.Name;` | Full MDL with activities | | Describe microflow (normalized) | `describe microflow Module.Name normalized;` | Folds crossed branches into one condition instead of flattening them. Opt-in: the output re-executes to an equivalent graph with fewer nodes and a different layout | | Describe microflow (with handles) | `describe microflow Module.Name with handles;` | Prints `-- handle: <target>` above each activity: its content address for `alter microflow` — output `$Var`, `'Caption'`, or a statement pattern with `*` wildcards (anchored at both ends), plus `@n` when several match. Comments only; cannot be combined with `normalized` | +| Insert into a stored microflow | `alter microflow Module.Name { insert after <target> { <statements> } };` | Also `insert before`, and `alter nanoflow`. A graph splice into the stored flow, not a rebuild: only the new activities, the two rewired flows and the objects moved to make room change; every other element keeps its `$ID`, position and curve. `<target>` is a handle from `describe … with handles`, resolved before any operation runs. Refused: after a decision, before an activity several flows enter, inside a loop body, a fragment that returns, a variable the flow already has or one not declared on the path | +| Replace or drop an activity | `alter microflow Module.Name { replace <target> with { <statements> } drop <target>; };` | Flows into the activity are re-pointed at the replacement (or at its successor, for `drop`). Refused for a decision, an end event, an activity with an error handler, and an activity whose output variable is still read. Over `--mcp` only `insert` is supported | | Describe nanoflow | `describe nanoflow Module.Name;` | Full MDL with activities | | Rename microflow | `rename microflow Module.Old to New;` | Updates all references | | Rename nanoflow | `rename nanoflow Module.Old to New;` | Updates all references | diff --git a/mdl/ast/ast_alter_flow.go b/mdl/ast/ast_alter_flow.go new file mode 100644 index 000000000..b6534dbd4 --- /dev/null +++ b/mdl/ast/ast_alter_flow.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 + +package ast + +// ============================================================================ +// ALTER MICROFLOW / ALTER NANOFLOW — a graph splice into the stored flow +// ============================================================================ + +// AlterFlowStmt represents: +// +// alter microflow|nanoflow Module.Name { +// insert after|before <target> { <statements> } +// replace <target> with { <statements> } +// drop <target>; +// } +// +// (ADR-0012 decision 3). Targets are content addresses, resolved against the +// flow as stored before any operation applies. +type AlterFlowStmt struct { + Nanoflow bool + Name QualifiedName + Operations []*AlterFlowOperation +} + +func (s *AlterFlowStmt) isStatement() {} + +// Kind is "microflow" or "nanoflow". +func (s *AlterFlowStmt) Kind() string { + if s.Nanoflow { + return "nanoflow" + } + return "microflow" +} + +// AlterFlowOpKind names an operation of an AlterFlowStmt. +type AlterFlowOpKind string + +const ( + AlterFlowInsertAfter AlterFlowOpKind = "insert after" + AlterFlowInsertBefore AlterFlowOpKind = "insert before" + AlterFlowReplace AlterFlowOpKind = "replace" + AlterFlowDrop AlterFlowOpKind = "drop" +) + +// AlterFlowOperation is one operation of an AlterFlowStmt. +type AlterFlowOperation struct { + Op AlterFlowOpKind + // Target is the content address as written (`$IsValidEmail`, + // `'Email is Valid?'`, `log * node 'Debug' *`, with an optional `@n`). + // mfmutator.ParseTarget reads it. + Target string + // Body is the fragment, for insert and replace. + Body []MicroflowStatement +} diff --git a/mdl/ast/ast_microflow.go b/mdl/ast/ast_microflow.go index 277f7c5f8..ade57f74e 100644 --- a/mdl/ast/ast_microflow.go +++ b/mdl/ast/ast_microflow.go @@ -851,21 +851,30 @@ type SortSpec struct { Ascending bool // True for ASC, false for DESC } -// ListOperationStmt represents list operations like HEAD, TAIL, FIND, etc. -// $Var = HEAD($List) -// $Var = FIND($List, condition) -// $Var = SORT($List, attr ASC) -// $Var = UNION($List1, $List2) +// ListOperationStmt is one Studio Pro List operation activity: +// $Var = head $List +// $Var = find $List by Member = value | where condition +// $Var = sort $List by attr asc +// $Var = union $List1 with $List2 +// The call forms ($Var = HEAD($List), …) build the same node. type ListOperationStmt struct { - OutputVariable string // Output variable name - Operation ListOperationType // Operation type - InputVariable string // Input list variable (first operand) - SecondVariable string // Second operand for UNION, INTERSECT, SUBTRACT, CONTAINS, EQUALS - Condition Expression // Condition for FIND/FILTER - SortSpecs []SortSpec // Sort specifications for SORT - OffsetExpr Expression // Offset expression for RANGE - LimitExpr Expression // Limit expression for RANGE - Annotations *ActivityAnnotations // Optional @position, @caption, @color, @annotation + OutputVariable string // Output variable name + Operation ListOperationType // Operation type + InputVariable string // Input list variable (first operand) + SecondVariable string // Second operand for UNION, INTERSECT, SUBTRACT, CONTAINS, EQUALS + Condition Expression // Condition for FIND/FILTER + // ByExpression is set for FIND/FILTER whose condition is an expression over + // $currentObject (Studio Pro's "Find by expression" / "Filter by + // expression"). When false, a condition of the shape `Member = value` (see + // IsMemberEquality) is the "by member" operation, and any other condition + // falls back to the expression operation, which is what the function form + // has always done. The statement form sets it from the linking word: `where` + // sets it, `by` does not. + ByExpression bool + SortSpecs []SortSpec // Sort specifications for SORT + OffsetExpr Expression // Offset expression for RANGE + LimitExpr Expression // Limit expression for RANGE + Annotations *ActivityAnnotations // Optional @position, @caption, @color, @annotation // ErrorHandling is recorded only so the clause can be REFUSED. Mendix's // ListOperationsAction has no ErrorHandlingType, so an ON ERROR here has // nowhere to go; parsing it and reporting it beats dropping it silently. @@ -877,6 +886,24 @@ type ListOperationStmt struct { func (s *ListOperationStmt) isMicroflowStatement() {} +// IsMemberEquality reports whether a FIND/FILTER condition names a member of +// the list's entity and a value, `Member = value` — the shape Studio Pro's Find +// and Filter operations take (an attribute or association, and the value it must +// have). It is the single test both the visitor (which linking word a function +// form is a respelling of) and the flow builder (which operation to write) use, +// so the two cannot disagree. +func IsMemberEquality(cond Expression) bool { + binary, ok := cond.(*BinaryExpr) + if !ok || binary.Operator != "=" { + return false + } + switch binary.Left.(type) { + case *IdentifierExpr, *QualifiedNameExpr: + return true + } + return false +} + // UnresolvedOperand is a list operand that the visitor could not reduce to a // variable name. // @@ -932,10 +959,11 @@ func (t AggregateListOperationType) String() string { } } -// AggregateListStmt represents aggregate operations: COUNT, SUM, AVERAGE, etc. -// $Count = COUNT($List) -// $Sum = SUM($List/Attr) -// $Sum = SUM($List, $currentObject/Price * 2) // expression form +// AggregateListStmt is one Studio Pro Aggregate list activity: +// $Count = count $List +// $Sum = sum $List by Attr +// $Sum = sum $List of $currentObject/Price * 2 // expression form +// The call forms ($Count = COUNT($List), …) build the same node. type AggregateListStmt struct { OutputVariable string // Output variable name Operation AggregateListOperationType // Operation type diff --git a/mdl/backend/backend.go b/mdl/backend/backend.go index a57e80e3a..f9a0cd760 100644 --- a/mdl/backend/backend.go +++ b/mdl/backend/backend.go @@ -37,5 +37,6 @@ type FullBackend interface { AgentEditorBackend PageMutationBackend WorkflowMutationBackend + MicroflowMutationBackend WidgetBuilderBackend } diff --git a/mdl/backend/mcp/microflow_mutator.go b/mdl/backend/mcp/microflow_mutator.go new file mode 100644 index 000000000..4bbe61ad1 --- /dev/null +++ b/mdl/backend/mcp/microflow_mutator.go @@ -0,0 +1,499 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mcp + +import ( + "encoding/json" + "fmt" + "strconv" + "strings" + + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/primitive" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/mfmutator" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// ALTER MICROFLOW / NANOFLOW over MCP (plan item 4.2f). +// +// The splice itself is the shared engine (mfmutator), run on the stored form of +// the flow as the local .mpr holds it — the same decisions, placement and +// refusals as on the modelsdk backend. What differs is the write: Studio Pro +// takes no unit bytes, so Save diffs the spliced document against the stored +// one and sends the difference as ped_update_document path operations on the +// live document: positions set, new objects and flows added, rewired flow ends +// set, removed elements removed. PED addresses list entries by index, and those +// indexes are the stored order — so before anything is sent, the live document +// is read back and compared with the local one, and a mismatch (the .mpr is +// behind Studio Pro) refuses the write rather than patching the wrong element. + +// OpenMicroflowForMutation opens a microflow or nanoflow for splicing. +func (b *Backend) OpenMicroflowForMutation(unitID model.ID) (backend.MicroflowMutator, error) { + raw, err := b.reader.GetRawUnitBytes(unitID) + if err != nil { + return nil, fmt.Errorf("alter over MCP reads the stored flow from the local project: %w", err) + } + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + return nil, err + } + docType, _ := docValue(d, "$Type").(string) + name, _ := docValue(d, "Name").(string) + qn, err := b.flowQualifiedName(unitID, docType, name) + if err != nil { + return nil, err + } + deps := &mcpFlowDeps{b: b, docType: docType, qn: qn, stored: d, objects: map[string]microflows.MicroflowObject{}, + flows: map[string]*microflows.SequenceFlow{}, annotations: map[string]*microflows.AnnotationFlow{}} + var work bson.D + if err := bson.Unmarshal(raw, &work); err != nil { + return nil, err + } + m, err := mfmutator.New(work, unitID, deps) + if err != nil { + return nil, err + } + return &mcpFlowMutator{Mutator: m, qn: qn}, nil +} + +// mcpFlowMutator is the shared splice, limited to what PED applies safely: +// inserts. A PED update that fails is rolled back EXCEPT for its removals, +// which persist (PED's own warning, and measured: a failed update left the +// removed activity gone together with the flows attached to it). A drop or a +// replace needs a removal, so over MCP they are refused rather than risked on +// the live model; run them against the .mpr instead. +type mcpFlowMutator struct { + *mfmutator.Mutator + qn string +} + +func (m *mcpFlowMutator) Replace(model.ID, *backend.MicroflowFragment) error { + return fmt.Errorf("replace is not supported by the MCP backend yet: Studio Pro does not roll back a removal when an update fails; run without --mcp to alter %s in the .mpr", m.qn) +} + +func (m *mcpFlowMutator) Drop(model.ID) error { + return fmt.Errorf("drop is not supported by the MCP backend yet: Studio Pro does not roll back a removal when an update fails; run without --mcp to alter %s in the .mpr", m.qn) +} + +func (b *Backend) flowQualifiedName(id model.ID, docType, name string) (string, error) { + var container model.ID + switch docType { + case microflowDocType: + mf, err := b.GetMicroflow(id) + if err != nil { + return "", err + } + container = mf.ContainerID + case "Microflows$Nanoflow": + nfs, err := b.reader.ListNanoflows() + if err != nil { + return "", err + } + for _, nf := range nfs { + if nf.ID == id { + container = nf.ContainerID + } + } + default: + return "", fmt.Errorf("unit %s is a %s, not a microflow or nanoflow", id, docType) + } + mod, err := b.moduleNameForContainer(container) + if err != nil { + return "", err + } + return mod + "." + name, nil +} + +// mcpFlowDeps is mfmutator.Deps for the MCP backend. Serialize produces only +// what the splice reads (identity, type, geometry, pointers) and keeps the +// domain object, which is what PED is sent; SaveUnit turns the spliced +// document into PED operations. +type mcpFlowDeps struct { + b *Backend + docType string + qn string + stored bson.D + + objects map[string]microflows.MicroflowObject + flows map[string]*microflows.SequenceFlow + annotations map[string]*microflows.AnnotationFlow +} + +func binaryOf(id model.ID) primitive.Binary { + return primitive.Binary{Subtype: 0, Data: types.UUIDToBlob(string(id))} +} + +func (d *mcpFlowDeps) SerializeObject(obj microflows.MicroflowObject) (bson.D, error) { + d.objects[string(obj.GetID())] = obj + p := obj.GetPosition() + w, h := 0, 0 + if s, ok := obj.(interface{ GetSize() model.Size }); ok { + w, h = s.GetSize().Width, s.GetSize().Height + } + return bson.D{ + {Key: "$ID", Value: binaryOf(obj.GetID())}, + {Key: "$Type", Value: "mcp-new-object"}, + {Key: "RelativeMiddlePoint", Value: fmt.Sprintf("%d;%d", p.X, p.Y)}, + {Key: "Size", Value: fmt.Sprintf("%d;%d", w, h)}, + }, nil +} + +func (d *mcpFlowDeps) SerializeSequenceFlow(f *microflows.SequenceFlow) (bson.D, error) { + d.flows[string(f.ID)] = f + return bson.D{ + {Key: "$ID", Value: binaryOf(f.ID)}, + {Key: "$Type", Value: "Microflows$SequenceFlow"}, + {Key: "DestinationConnectionIndex", Value: int32(f.DestinationConnectionIndex)}, + {Key: "DestinationPointer", Value: binaryOf(f.DestinationID)}, + {Key: "IsErrorHandler", Value: f.IsErrorHandler}, + {Key: "Line", Value: bson.D{ + {Key: "$Type", Value: "Microflows$BezierCurve"}, + {Key: "DestinationControlVector", Value: f.DestinationControlVector}, + {Key: "OriginControlVector", Value: f.OriginControlVector}, + }}, + {Key: "OriginConnectionIndex", Value: int32(f.OriginConnectionIndex)}, + {Key: "OriginPointer", Value: binaryOf(f.OriginID)}, + }, nil +} + +func (d *mcpFlowDeps) SerializeAnnotationFlow(f *microflows.AnnotationFlow) (bson.D, error) { + d.annotations[string(f.ID)] = f + return bson.D{ + {Key: "$ID", Value: binaryOf(f.ID)}, + {Key: "$Type", Value: "Microflows$AnnotationFlow"}, + {Key: "DestinationPointer", Value: binaryOf(f.DestinationID)}, + {Key: "OriginPointer", Value: binaryOf(f.OriginID)}, + }, nil +} + +// SaveUnit sends the difference between the stored and the spliced document. +func (d *mcpFlowDeps) SaveUnit(_ string, contents []byte) error { + var spliced bson.D + if err := bson.Unmarshal(contents, &spliced); err != nil { + return err + } + ops, err := d.b.flowPatchOps(d, spliced) + if err != nil { + return err + } + if len(ops) == 0 { + return nil + } + if err := d.b.checkLiveFlowMatches(d.docType, d.qn, d.stored); err != nil { + return err + } + // Only inserts reach here (mcpFlowMutator refuses the rest), so there is + // nothing to remove; a removal would mean the splice did something this + // path was not built for, and PED would not roll it back on a failure. + for _, op := range ops { + if op.Operation.Type == "remove" { + return fmt.Errorf("%s: refusing to send a removal over MCP", d.qn) + } + } + // One update: PED applies it all or, on a failure, nothing. + if err := d.b.pedUpdateDoc(d.docType, d.qn, ops...); err != nil { + return err + } + return d.b.pedCheckDocument(d.docType, d.qn) +} + +// flowList returns a unit's top-level objects or flows, in stored order, with +// their $IDs. +func flowList(d bson.D, objects bool) (ids []string, docs []bson.D) { + list := docValue(d, "Flows") + if objects { + oc, _ := docValue(d, "ObjectCollection").(bson.D) + list = docValue(oc, "Objects") + } + a, _ := list.(bson.A) + for i, el := range a { + if i == 0 { + if _, marker := el.(int32); marker { + continue + } + } + e, ok := el.(bson.D) + if !ok { + continue + } + b, _ := docValue(e, "$ID").(primitive.Binary) + ids = append(ids, types.BlobToUUID(b.Data)) + docs = append(docs, e) + } + return ids, docs +} + +func docValue(d bson.D, key string) any { + for _, e := range d { + if e.Key == key { + return e.Value + } + } + return nil +} + +func pointerID(d bson.D, key string) string { + b, _ := docValue(d, key).(primitive.Binary) + return types.BlobToUUID(b.Data) +} + +func storedPointToPED(s string) map[string]int { + x, y, _ := strings.Cut(s, ";") + px, _ := strconv.Atoi(x) + py, _ := strconv.Atoi(y) + return map[string]int{"x": px, "y": py} +} + +func pedVector(s string) map[string]int { + p := storedPointToPED(s) + return map[string]int{"width": p["x"], "height": p["y"]} +} + +var pedSides = []string{"Top", "Right", "Bottom", "Left"} + +// pedQualifiedFlowType maps the storage $Type of a flow object to the +// qualified name PED reports for it, where the two differ (CLAUDE.md, "BSON +// Storage Names vs Qualified Names"). +var pedQualifiedFlowType = map[string]string{ + "Microflows$MicroflowParameter": "Microflows$MicroflowParameterObject", +} + +// flowPatchOps derives the PED operations that turn the stored document into +// the spliced one. Order matters, because PED addresses entries by index: +// everything that uses a stored index (positions, rewired flows) and every +// append goes first, while indexes are still the stored ones; removals go +// last, highest index first. +func (b *Backend) flowPatchOps(d *mcpFlowDeps, spliced bson.D) ([]pedOpEntry, error) { + var ops []pedOpEntry + set := func(path string, v any) { + ops = append(ops, pedOpEntry{Path: path, Operation: pedOperation{Type: "set", Value: v}}) + } + oldObjIDs, oldObjs := flowList(d.stored, true) + newObjIDs, newObjs := flowList(spliced, true) + oldFlowIDs, oldFlows := flowList(d.stored, false) + newFlowIDs, newFlows := flowList(spliced, false) + + path := map[string]string{} + oldObjIndex := map[string]int{} + for i, id := range oldObjIDs { + oldObjIndex[id] = i + path[id] = fmt.Sprintf("/objectCollection/objects/%d", i) + } + survivingObj := map[string]bool{} + var added []string + for i, id := range newObjIDs { + if idx, ok := oldObjIndex[id]; ok { + survivingObj[id] = true + before, _ := docValue(oldObjs[idx], "RelativeMiddlePoint").(string) + after, _ := docValue(newObjs[i], "RelativeMiddlePoint").(string) + if before != after { + set(path[id]+"/relativeMiddlePoint", storedPointToPED(after)) + } + continue + } + added = append(added, id) + } + for j, id := range added { + path[id] = fmt.Sprintf("/objectCollection/objects/%d", len(oldObjIDs)+j) + } + for _, id := range added { + obj, ok := d.objects[id] + if !ok { + return nil, fmt.Errorf("internal: spliced object %s was not serialized", id) + } + idPath := map[model.ID]string{} + m, err := b.mapObjectTree(obj, path[id], idPath) + if err != nil { + return nil, err + } + var sets []pedOpEntry + skeletonObject(m, path[id], func(p string, v any) { + sets = append(sets, pedOpEntry{Path: p, Operation: pedOperation{Type: "set", Value: v}}) + }) + ops = append(ops, pedOpEntry{Path: "/objectCollection/objects", Operation: pedOperation{Type: "add", Value: m}}) + ops = append(ops, sets...) + // The skeleton constructor ignores a position; set it on the stored element. + p := obj.GetPosition() + set(path[id]+"/relativeMiddlePoint", map[string]int{"x": p.X, "y": p.Y}) + for nested, np := range idPath { + path[string(nested)] = np + } + } + ref := func(id string) (string, error) { + p, ok := path[id] + if !ok { + return "", fmt.Errorf("a flow points at %s, which is not a top-level object PED can address", id) + } + return "$id(" + p + ")", nil + } + + oldFlowIndex := map[string]int{} + for i, id := range oldFlowIDs { + oldFlowIndex[id] = i + } + survivingFlow := map[string]bool{} + var addedFlows []string + for i, id := range newFlowIDs { + idx, ok := oldFlowIndex[id] + if !ok { + addedFlows = append(addedFlows, id) + continue + } + survivingFlow[id] = true + fp := fmt.Sprintf("/flows/%d", idx) + o, n := oldFlows[idx], newFlows[i] + for _, end := range []string{"Origin", "Destination"} { + key := strings.ToLower(end) + if pointerID(o, end+"Pointer") != pointerID(n, end+"Pointer") { + r, err := ref(pointerID(n, end+"Pointer")) + if err != nil { + return nil, err + } + set(fp+"/"+key, r) + } + if fmt.Sprint(docValue(o, end+"ConnectionIndex")) != fmt.Sprint(docValue(n, end+"ConnectionIndex")) { + set(fp+"/"+key+"ConnectionIndex", docValue(n, end+"ConnectionIndex")) + } + ol, _ := docValue(o, "Line").(bson.D) + nl, _ := docValue(n, "Line").(bson.D) + if ov, nv := fmt.Sprint(docValue(ol, end+"ControlVector")), fmt.Sprint(docValue(nl, end+"ControlVector")); ov != nv && nv != "" { + set(fp+"/line/"+key+"ControlVector", pedVector(nv)) + } + } + } + for j, id := range addedFlows { + fp := fmt.Sprintf("/flows/%d", len(oldFlowIDs)+j) + if af, ok := d.annotations[id]; ok { + o, err := ref(string(af.OriginID)) + if err != nil { + return nil, err + } + dst, err := ref(string(af.DestinationID)) + if err != nil { + return nil, err + } + ops = append(ops, pedOpEntry{Path: "/flows", Operation: pedOperation{Type: "add", Value: map[string]any{ + "$Type": "Microflows$AnnotationFlow", "originId": o, "destinationId": dst}}}) + continue + } + f, ok := d.flows[id] + if !ok { + return nil, fmt.Errorf("internal: spliced flow %s was not serialized", id) + } + o, err := ref(string(f.OriginID)) + if err != nil { + return nil, err + } + dst, err := ref(string(f.DestinationID)) + if err != nil { + return nil, err + } + v := map[string]any{"$Type": "Microflows$SequenceFlow", "originId": o, "destinationId": dst} + if f.OriginConnectionIndex >= 0 && f.OriginConnectionIndex < 4 { + v["originConnectionSide"] = pedSides[f.OriginConnectionIndex] + } + if f.DestinationConnectionIndex >= 0 && f.DestinationConnectionIndex < 4 { + v["destinationConnectionSide"] = pedSides[f.DestinationConnectionIndex] + } + cv, err := mapCaseValue(f.CaseValue) + if err != nil { + return nil, err + } + if cv != nil { + v["caseValue"] = cv + } + ops = append(ops, pedOpEntry{Path: "/flows", Operation: pedOperation{Type: "add", Value: v}}) + if f.IsErrorHandler { + set(fp+"/isErrorHandler", true) + } + if f.OriginControlVector != "" { + set(fp+"/line/originControlVector", pedVector(f.OriginControlVector)) + } + if f.DestinationControlVector != "" { + set(fp+"/line/destinationControlVector", pedVector(f.DestinationControlVector)) + } + } + + for i := len(oldFlowIDs) - 1; i >= 0; i-- { + if !survivingFlow[oldFlowIDs[i]] { + idx := i + ops = append(ops, pedOpEntry{Path: "/flows", Operation: pedOperation{Type: "remove", Index: &idx}}) + } + } + for i := len(oldObjIDs) - 1; i >= 0; i-- { + if !survivingObj[oldObjIDs[i]] { + idx := i + ops = append(ops, pedOpEntry{Path: "/objectCollection/objects", Operation: pedOperation{Type: "remove", Index: &idx}}) + } + } + return ops, nil +} + +// checkLiveFlowMatches compares the live document's objects and flows with the +// stored ones the splice ran on: the same count, and each object the same type +// at the same position. PED addresses them by index, so a live document that +// has moved on since the .mpr was saved would have the patch applied to other +// elements than the ones the splice chose. +func (b *Backend) checkLiveFlowMatches(docType, qn string, stored bson.D) error { + res, err := b.client.CallTool("ped_read_document", map[string]any{ + "documentType": docType, + "documentName": qn, + "paths": []string{"/objectCollection/objects", "/flows"}, + }) + if err != nil { + return err + } + if res.IsError { + return fmt.Errorf("ped_read_document %s: %s", qn, pedStripReminder(res.Text)) + } + var body struct { + Results []struct { + Path string `json:"path"` + Result json.RawMessage `json:"result"` + } `json:"results"` + } + if err := json.Unmarshal([]byte(pedStripReminder(res.Text)), &body); err != nil { + return fmt.Errorf("read %s from Studio Pro: %w", qn, err) + } + stale := func(what string) error { + return fmt.Errorf("%s in Studio Pro no longer matches the local project (%s); save in Studio Pro so the .mpr is current, then retry", qn, what) + } + _, objs := flowList(stored, true) + _, flows := flowList(stored, false) + for _, r := range body.Results { + var live []map[string]any + if err := json.Unmarshal(r.Result, &live); err != nil { + return fmt.Errorf("read %s %s from Studio Pro: %w", qn, r.Path, err) + } + switch r.Path { + case "/flows": + if len(live) != len(flows) { + return stale(fmt.Sprintf("%d flows live, %d stored", len(live), len(flows))) + } + case "/objectCollection/objects": + if len(live) != len(objs) { + return stale(fmt.Sprintf("%d objects live, %d stored", len(live), len(objs))) + } + for i, o := range objs { + typ, _ := docValue(o, "$Type").(string) + if q, ok := pedQualifiedFlowType[typ]; ok { + typ = q + } + if live[i]["$Type"] != typ { + return stale(fmt.Sprintf("object %d is a %v live, a %s stored", i, live[i]["$Type"], typ)) + } + rp, _ := docValue(o, "RelativeMiddlePoint").(string) + want := storedPointToPED(rp) + pt, _ := live[i]["relativeMiddlePoint"].(map[string]any) + if fmt.Sprint(pt["x"]) != strconv.Itoa(want["x"]) || fmt.Sprint(pt["y"]) != strconv.Itoa(want["y"]) { + return stale(fmt.Sprintf("object %d is at %v live, at %s stored", i, pt, rp)) + } + } + } + } + return nil +} diff --git a/mdl/backend/mcp/microflow_mutator_test.go b/mdl/backend/mcp/microflow_mutator_test.go new file mode 100644 index 000000000..7fad8b990 --- /dev/null +++ b/mdl/backend/mcp/microflow_mutator_test.go @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mcp + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/primitive" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/mfmutator" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +func mfID(name string) primitive.Binary { + b := make([]byte, 16) + copy(b, name) + return primitive.Binary{Data: b} +} + +func mfUUID(name string) string { return types.BlobToUUID(mfID(name).Data) } + +func mfObj(name, typ string, x int) bson.D { + return bson.D{ + {Key: "$ID", Value: mfID(name)}, + {Key: "$Type", Value: typ}, + {Key: "RelativeMiddlePoint", Value: fmt.Sprintf("%d;200", x)}, + {Key: "Size", Value: "120;60"}, + } +} + +func mfFlow(name, from, to string) bson.D { + return bson.D{ + {Key: "$ID", Value: mfID(name)}, + {Key: "$Type", Value: "Microflows$SequenceFlow"}, + {Key: "DestinationConnectionIndex", Value: int32(3)}, + {Key: "DestinationPointer", Value: mfID(to)}, + {Key: "IsErrorHandler", Value: false}, + {Key: "Line", Value: bson.D{{Key: "$Type", Value: "Microflows$BezierCurve"}, + {Key: "DestinationControlVector", Value: "-30;0"}, {Key: "OriginControlVector", Value: "30;0"}}}, + {Key: "OriginConnectionIndex", Value: int32(1)}, + {Key: "OriginPointer", Value: mfID(from)}, + } +} + +// storedReduce is the shape of TestApp's Microflows.MicroflowReduce, the flow +// the live check ran on: a start, three activities 170 apart, an end. +func storedReduce() bson.D { + return bson.D{ + {Key: "$ID", Value: mfID("unit")}, + {Key: "$Type", Value: "Microflows$Microflow"}, + {Key: "Flows", Value: bson.A{int32(3), mfFlow("f0", "start", "a"), mfFlow("f1", "a", "b"), mfFlow("f2", "b", "end")}}, + {Key: "Name", Value: "Reduce"}, + {Key: "ObjectCollection", Value: bson.D{ + {Key: "$ID", Value: mfID("oc")}, + {Key: "$Type", Value: "Microflows$MicroflowObjectCollection"}, + {Key: "Objects", Value: bson.A{int32(3), + mfObj("start", "Microflows$StartEvent", -98), + mfObj("end", "Microflows$EndEvent", 564), + mfObj("a", "Microflows$ActionActivity", 214), + mfObj("b", "Microflows$ActionActivity", 404), + }}, + }}, + } +} + +func openFlowMutator(t *testing.T, b *Backend) (*mcpFlowMutator, *mcpFlowDeps) { + t.Helper() + stored := storedReduce() + raw, _ := bson.Marshal(stored) + var storedD, work bson.D + _ = bson.Unmarshal(raw, &storedD) + _ = bson.Unmarshal(raw, &work) + deps := &mcpFlowDeps{b: b, docType: microflowDocType, qn: "M.Reduce", stored: storedD, + objects: map[string]microflows.MicroflowObject{}, flows: map[string]*microflows.SequenceFlow{}, + annotations: map[string]*microflows.AnnotationFlow{}} + m, err := mfmutator.New(work, "unit", deps) + if err != nil { + t.Fatal(err) + } + return &mcpFlowMutator{Mutator: m, qn: "M.Reduce"}, deps +} + +func logFragment() *backend.MicroflowFragment { + id := model.ID(types.GenerateID()) + act := &microflows.ActionActivity{ + BaseActivity: microflows.BaseActivity{BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: id}, + Position: model.Point{X: 360, Y: 200}, + Size: model.Size{Width: 120, Height: 60}, + }}, + Action: &microflows.LogMessageAction{LogLevel: "Info", LogNodeName: "'Reduce'", + MessageTemplate: &model.Text{Translations: map[string]string{"en_US": "reduced"}}}, + } + return &backend.MicroflowFragment{Objects: []microflows.MicroflowObject{act}, Entry: id, Exit: id} +} + +// The splice becomes PED path operations addressed by the stored indexes: +// the moved end, the new activity (added bare, then its action and position +// set, as PED's skeleton constructor requires), the rewired flow's end, and +// the new flow — and no removal. +func TestFlowPatchOps_InsertAfter(t *testing.T) { + var sent []any + f := newFakePED(t, func(name string, args map[string]any) (string, bool) { + switch name { + case "ped_read_document": + return `{"results":[{"path":"/objectCollection/objects","result":[` + + `{"$Type":"Microflows$StartEvent","relativeMiddlePoint":{"x":-98,"y":200}},` + + `{"$Type":"Microflows$EndEvent","relativeMiddlePoint":{"x":564,"y":200}},` + + `{"$Type":"Microflows$ActionActivity","relativeMiddlePoint":{"x":214,"y":200}},` + + `{"$Type":"Microflows$ActionActivity","relativeMiddlePoint":{"x":404,"y":200}}]},` + + `{"path":"/flows","result":[{},{},{}]}]}`, false + case "ped_update_document": + sent = append(sent, args["operations"]) + return "SUCCESS: All operations have been performed successfully.", false + case "ped_check_errors": + return "No errors found.", false + } + return "SUCCESS", false + }) + b := &Backend{client: f.connectClient(t)} + m, _ := openFlowMutator(t, b) + if err := m.InsertAfter(model.ID(mfUUID("a")), logFragment()); err != nil { + t.Fatal(err) + } + if err := m.Save(); err != nil { + t.Fatalf("save: %v", err) + } + if len(sent) != 1 { + t.Fatalf("want one ped_update_document, got %d", len(sent)) + } + js, _ := json.Marshal(sent[0]) + got := string(js) + var ops []pedOpEntry + if err := json.Unmarshal(js, &ops); err != nil { + t.Fatal(err) + } + has := func(path, typ, value string) bool { + for _, op := range ops { + v, _ := json.Marshal(op.Operation.Value) + if op.Path == path && op.Operation.Type == typ && strings.Contains(string(v), value) { + return true + } + } + return false + } + for _, w := range []struct{ path, typ, value string }{ + {"/objectCollection/objects/3/relativeMiddlePoint", "set", `"x":534`}, // b moved along + {"/objectCollection/objects", "add", `"Microflows$ActionActivity"`}, + {"/objectCollection/objects/4/action", "set", `"Microflows$LogMessageAction"`}, + {"/flows/1/destination", "set", `$id(/objectCollection/objects/4)`}, + {"/flows", "add", `"destinationId":"$id(/objectCollection/objects/3)"`}, + {"/flows", "add", `"originId":"$id(/objectCollection/objects/4)"`}, + } { + if !has(w.path, w.typ, w.value) { + t.Errorf("no %s %s with %s in:\n%s", w.typ, w.path, w.value, got) + } + } + if strings.Contains(got, `"remove"`) { + t.Errorf("an insert sent a removal:\n%s", got) + } +} + +// PED addresses by index, so a live document that differs from the stored one +// must refuse the write before anything is sent. +func TestFlowPatchOps_StaleLiveDocumentIsRefused(t *testing.T) { + updates := 0 + f := newFakePED(t, func(name string, _ map[string]any) (string, bool) { + switch name { + case "ped_read_document": + return `{"results":[{"path":"/objectCollection/objects","result":[{},{},{},{},{}]},{"path":"/flows","result":[{},{},{}]}]}`, false + case "ped_update_document": + updates++ + } + return "SUCCESS", false + }) + b := &Backend{client: f.connectClient(t)} + m, _ := openFlowMutator(t, b) + if err := m.InsertAfter(model.ID(mfUUID("a")), logFragment()); err != nil { + t.Fatal(err) + } + err := m.Save() + if err == nil || !strings.Contains(err.Error(), "no longer matches the local project") { + t.Fatalf("want the stale-document refusal, got %v", err) + } + if updates != 0 { + t.Error("an update was sent to a document that did not match") + } +} + +// Drop and replace need a removal, which PED does not roll back when an +// update fails; over MCP they are refused. +func TestMCPFlowMutator_RefusesRemovals(t *testing.T) { + m, _ := openFlowMutator(t, &Backend{}) + if err := m.Drop(model.ID(mfUUID("a"))); err == nil || !strings.Contains(err.Error(), "not supported by the MCP backend") { + t.Errorf("drop: %v", err) + } + if err := m.Replace(model.ID(mfUUID("a")), logFragment()); err == nil || !strings.Contains(err.Error(), "not supported by the MCP backend") { + t.Errorf("replace: %v", err) + } +} diff --git a/mdl/backend/mcp/unsupported_gen.go b/mdl/backend/mcp/unsupported_gen.go index a1dc96a9c..abae5e3a3 100644 --- a/mdl/backend/mcp/unsupported_gen.go +++ b/mdl/backend/mcp/unsupported_gen.go @@ -992,6 +992,11 @@ func (unsupportedBackend) MoveViewEntitySourceDocument(_ string, _ model.ID, _ s return } +func (unsupportedBackend) OpenMicroflowForMutation(_ model.ID) (r0 backend.MicroflowMutator, err1 error) { + err1 = errUnsupported("OpenMicroflowForMutation") + return +} + func (unsupportedBackend) OpenPageForMutation(_ model.ID) (r0 backend.PageMutator, err1 error) { err1 = errUnsupported("OpenPageForMutation") return diff --git a/mdl/backend/mfmutator/splice.go b/mdl/backend/mfmutator/splice.go new file mode 100644 index 000000000..011cf3e1e --- /dev/null +++ b/mdl/backend/mfmutator/splice.go @@ -0,0 +1,962 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mfmutator + +// # The graph splice (plan item 4.2b) +// +// An `alter microflow` operation edits the STORED document, never a rebuild of +// it. The rebuild (`UpdateMicroflow`, then re-pairing IDs by type and position) +// is what deleted merges, reset curves and moved $IDs onto other nodes on a +// Studio Pro-authored flow (ADR-0012, Context). So the splice works on the raw +// BSON tree of the unit as read from storage: +// +// - the fragment's objects are the only elements it builds, and they are +// appended to the collection that holds the target; +// - the sequence flows around the target are rewired by changing their +// pointers, connection sides and the control vector at the rewired end — +// every other property of a rewired flow (its $ID, case value, error-handler +// flag, the curve at its other end) is kept; +// - nodes are moved only to make room, and only by a translation; +// - nothing else is touched, and no $ID is ever rewritten. A removed element +// leaves no reference behind: Save refuses a document in which any binary +// still names an element that was removed (CLAUDE.md rule 1), and one in +// which two elements share an $ID. +// +// What the splice cannot do safely it refuses, with the reason: an insert +// after a decision (which branch?), before an activity several flows enter +// (which path?), a drop or replace of an activity with an error handler (the +// handler would be orphaned), and — for now — anything inside a loop body, +// whose coordinates are relative to the loop box. + +import ( + "bytes" + "fmt" + "strconv" + "strings" + + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/primitive" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// Deps supplies the engine-specific steps: turning a new object or flow into +// its stored BSON form, and writing the unit back. Everything between — which +// flows to rewire, where to place, what to remove — is decided here, once, for +// every storage engine. +type Deps interface { + SerializeObject(obj microflows.MicroflowObject) (bson.D, error) + SerializeSequenceFlow(f *microflows.SequenceFlow) (bson.D, error) + SerializeAnnotationFlow(f *microflows.AnnotationFlow) (bson.D, error) + // SaveUnit writes the patched unit. The implementation must go through the + // storage engine's reconciling write (canon.Reconcile), like every write. + SaveUnit(unitID string, contents []byte) error +} + +// Mutator splices fragments into one microflow or nanoflow unit. +type Mutator struct { + deps Deps + unitID model.ID + doc bson.D + // removed holds the $IDs of every element an operation took out, so Save + // can prove that nothing still points at one of them. + removed map[string]bool +} + +var _ backend.MicroflowMutator = (*Mutator)(nil) + +// New returns a Mutator over a decoded Microflows$Microflow or +// Microflows$Nanoflow unit. +func New(doc bson.D, unitID model.ID, deps Deps) (*Mutator, error) { + switch t := dString(doc, "$Type"); t { + case "Microflows$Microflow", "Microflows$Nanoflow": + default: + return nil, fmt.Errorf("unit %s is a %s, not a microflow or nanoflow", unitID, t) + } + if dDoc(doc, "ObjectCollection") == nil { + return nil, fmt.Errorf("unit %s has no object collection", unitID) + } + return &Mutator{deps: deps, unitID: unitID, doc: doc, removed: map[string]bool{}}, nil +} + +// Bytes returns the patched unit, after the integrity checks Save applies. +func (m *Mutator) Bytes() ([]byte, error) { + if err := m.checkIntegrity(); err != nil { + return nil, err + } + return bson.Marshal(m.doc) +} + +// Save writes the patched unit through Deps. +func (m *Mutator) Save() error { + out, err := m.Bytes() + if err != nil { + return err + } + return m.deps.SaveUnit(string(m.unitID), out) +} + +// --------------------------------------------------------------------------- +// The graph view +// --------------------------------------------------------------------------- + +// node is one object of the flow as stored. +type node struct { + id string // model ID form (types.BlobToUUID of the $ID) + typ string + doc bson.D + pos point + size point + // loop is the $ID of the loop whose body holds the node, "" at top level. + loop string +} + +// flowRef is one entry of the unit's Flows list. +type flowRef struct { + id string + typ string + doc bson.D + origin string + dest string + isErr bool +} + +func (f flowRef) isSequence() bool { return f.typ == "Microflows$SequenceFlow" } + +type graph struct { + nodes map[string]*node + // order is the nodes in storage order, so a shift visits them the same + // way every run. + order []*node + flows []flowRef +} + +func (m *Mutator) graph() *graph { + g := &graph{nodes: map[string]*node{}} + var walk func(oc bson.D, loop string) + walk = func(oc bson.D, loop string) { + for _, el := range arrayElements(dGet(oc, "Objects")) { + d, ok := el.(bson.D) + if !ok { + continue + } + n := &node{ + id: binaryID(dGet(d, "$ID")), + typ: dString(d, "$Type"), + doc: d, + pos: parsePoint(dString(d, "RelativeMiddlePoint")), + size: parsePoint(dString(d, "Size")), + loop: loop, + } + if n.id != "" { + g.nodes[n.id] = n + g.order = append(g.order, n) + } + if n.typ == "Microflows$LoopedActivity" { + if inner := dDoc(d, "ObjectCollection"); inner != nil { + walk(inner, n.id) + } + } + } + } + walk(dDoc(m.doc, "ObjectCollection"), "") + for _, el := range arrayElements(dGet(m.doc, "Flows")) { + d, ok := el.(bson.D) + if !ok { + continue + } + isErr, _ := dGet(d, "IsErrorHandler").(bool) + g.flows = append(g.flows, flowRef{ + id: binaryID(dGet(d, "$ID")), + typ: dString(d, "$Type"), + doc: d, + origin: binaryID(dGet(d, "OriginPointer")), + dest: binaryID(dGet(d, "DestinationPointer")), + isErr: isErr, + }) + } + return g +} + +// outgoing returns the sequence flows leaving id, split into normal and +// error-handler flows. +func (g *graph) outgoing(id string) (normal, errs []flowRef) { + for _, f := range g.flows { + if f.isSequence() && f.origin == id { + if f.isErr { + errs = append(errs, f) + } else { + normal = append(normal, f) + } + } + } + return normal, errs +} + +// incoming returns the sequence flows entering id. +func (g *graph) incoming(id string) []flowRef { + var out []flowRef + for _, f := range g.flows { + if f.isSequence() && f.dest == id { + out = append(out, f) + } + } + return out +} + +// annotationFlows returns the annotation flows attached to id. +func (g *graph) annotationFlows(id string) []flowRef { + var out []flowRef + for _, f := range g.flows { + if !f.isSequence() && (f.origin == id || f.dest == id) { + out = append(out, f) + } + } + return out +} + +func (g *graph) node(id model.ID) (*node, error) { + n, ok := g.nodes[string(id)] + if !ok { + return nil, fmt.Errorf("activity %s is not in the stored flow (was it dropped by an earlier operation?)", id) + } + if n.loop != "" { + return nil, fmt.Errorf("%s is inside a loop body; alter does not splice inside a loop yet — "+ + "address the loop itself, or rewrite the loop with create or modify", describeNode(n)) + } + return n, nil +} + +// --------------------------------------------------------------------------- +// Operations +// --------------------------------------------------------------------------- + +// InsertAfter splices frag onto the flow leaving target. +func (m *Mutator) InsertAfter(target model.ID, frag *backend.MicroflowFragment) error { + g := m.graph() + x, err := g.node(target) + if err != nil { + return err + } + normal, _ := g.outgoing(x.id) + switch { + case len(normal) == 0: + return fmt.Errorf("nothing follows %s: it ends the flow; insert before it instead", describeNode(x)) + case len(normal) > 1: + return fmt.Errorf("%s has %d outgoing flows (it is a decision): insert after it would not say which branch; "+ + "insert before the first activity of the branch instead", describeNode(x), len(normal)) + } + return m.spliceOnFlow(g, normal[0], frag) +} + +// InsertBefore splices frag onto the flow entering target. +func (m *Mutator) InsertBefore(target model.ID, frag *backend.MicroflowFragment) error { + g := m.graph() + y, err := g.node(target) + if err != nil { + return err + } + in := g.incoming(y.id) + switch { + case len(in) == 0: + return fmt.Errorf("no flow enters %s; there is no path to insert on", describeNode(y)) + case len(in) > 1: + return fmt.Errorf("%d flows enter %s: insert before it would not say on which path; "+ + "insert after one of its predecessors instead", len(in), describeNode(y)) + } + return m.spliceOnFlow(g, in[0], frag) +} + +// spliceOnFlow puts frag on flow f (origin X, destination Y): f keeps its +// origin end and now enters the fragment's entry; a new flow leaves the +// fragment's exit and enters Y where f used to. +func (m *Mutator) spliceOnFlow(g *graph, f flowRef, frag *backend.MicroflowFragment) error { + x, y := g.nodes[f.origin], g.nodes[f.dest] + if x == nil || y == nil { + return fmt.Errorf("flow %s points at an object that is not in the flow", f.id) + } + if x.loop != "" || y.loop != "" { + return fmt.Errorf("the flow runs inside a loop body; alter does not splice inside a loop yet") + } + fb, err := fragmentGeometry(frag) + if err != nil { + return err + } + ax, s, err := flowAxis(x, y) + if err != nil { + return err + } + + // Make room: the fragment plus a gap on either side has to fit between + // X's and Y's facing edges; what is missing is added by moving everything + // past the midpoint of that gap further along the axis. + a := x.pos.get(ax) + s*x.size.get(ax)/2 + b := y.pos.get(ax) - s*y.size.get(ax)/2 + need := fb.length(ax) + 2*minGap + if deficit := need - s*(b-a); deficit > 0 { + m.shift(g, ax, s, float64(a+b)/2, s*deficit, "") + b += s * deficit + } + + // Place the fragment centred in the gap, its entry level with the flow. + cross := ax.other() + mid := point{} + mid = mid.set(ax, (a+b)/2) + mid = mid.set(cross, (x.pos.get(cross)+y.pos.get(cross))/2) + fb.placeCentred(ax, mid) + if err := fb.checkRoom(g, ""); err != nil { + return err + } + + entrySide, exitSide := side(ax, -s), side(ax, s) + destIdx, destVec := flowEnd(f.doc, "Destination") + if err := m.addFragment(frag); err != nil { + return err + } + // f now enters the fragment. + setPointer(f.doc, "DestinationPointer", frag.Entry) + setInt(f.doc, "DestinationConnectionIndex", entrySide) + setVector(f.doc, "DestinationControlVector", sideVector(entrySide)) + // And a new flow carries on from the fragment to Y. + return m.addFlow(&microflows.SequenceFlow{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + OriginID: frag.Exit, + DestinationID: model.ID(y.id), + OriginConnectionIndex: exitSide, + DestinationConnectionIndex: destIdx, + OriginControlVector: sideVector(exitSide), + DestinationControlVector: destVec, + }) +} + +// Replace puts frag where target is: every flow that entered target enters +// the fragment, the flow that left it leaves the fragment's exit, and +// annotations attached to target are attached to the fragment's entry. +func (m *Mutator) Replace(target model.ID, frag *backend.MicroflowFragment) error { + g := m.graph() + x, err := g.node(target) + if err != nil { + return err + } + out, err := removable(g, x, "replace") + if err != nil { + return err + } + y := g.nodes[out.dest] + if y == nil { + return fmt.Errorf("flow %s points at an object that is not in the flow", out.id) + } + fb, err := fragmentGeometry(frag) + if err != nil { + return err + } + ax, s, err := flowAxis(x, y) + if err != nil { + return err + } + // The fragment starts where target started; if it is longer, everything + // past target moves along by the difference. + near := x.pos.get(ax) - s*x.size.get(ax)/2 + if extra := fb.length(ax) - x.size.get(ax); extra > 0 { + m.shift(g, ax, s, float64(x.pos.get(ax)), s*extra, x.id) + } + centre := point{} + centre = centre.set(ax, near+s*fb.length(ax)/2) + centre = centre.set(ax.other(), x.pos.get(ax.other())) + fb.placeCentred(ax, centre) + if err := fb.checkRoom(g, x.id); err != nil { + return err + } + + if err := m.addFragment(frag); err != nil { + return err + } + for _, in := range g.incoming(x.id) { + setPointer(in.doc, "DestinationPointer", frag.Entry) + } + setPointer(out.doc, "OriginPointer", frag.Exit) + for _, af := range g.annotationFlows(x.id) { + key := "DestinationPointer" + if af.origin == x.id { + key = "OriginPointer" + } + setPointer(af.doc, key, frag.Entry) + } + m.removeFlows(g.bodyFlows(x.id)) + return m.removeObject(x) +} + +// Drop removes target and joins the flows that entered it to the object it +// led to. Annotation lines attached to it go with it; the annotations stay. +func (m *Mutator) Drop(target model.ID) error { + g := m.graph() + x, err := g.node(target) + if err != nil { + return err + } + out, err := removable(g, x, "drop") + if err != nil { + return err + } + destIdx, destVec := flowEnd(out.doc, "Destination") + destPtr := dGet(out.doc, "DestinationPointer") + for _, in := range g.incoming(x.id) { + if in.origin == out.dest { + return fmt.Errorf("dropping %s would leave a flow from an object to itself", describeNode(x)) + } + } + for _, in := range g.incoming(x.id) { + setBinary(in.doc, "DestinationPointer", destPtr) + setInt(in.doc, "DestinationConnectionIndex", destIdx) + setVector(in.doc, "DestinationControlVector", destVec) + } + drop := g.bodyFlows(x.id) + drop[out.id] = true + for _, af := range g.annotationFlows(x.id) { + drop[af.id] = true + } + m.removeFlows(drop) + return m.removeObject(x) +} + +// bodyFlows returns the flows that run inside loop's body, at any depth. They +// are stored in the unit's Flows list, not in the loop, so taking the loop out +// has to take them too; left behind they would point at removed objects. +func (g *graph) bodyFlows(loop string) map[string]bool { + inside := func(id string) bool { + for n := g.nodes[id]; n != nil && n.loop != ""; n = g.nodes[n.loop] { + if n.loop == loop { + return true + } + } + return false + } + out := map[string]bool{} + for _, f := range g.flows { + if inside(f.origin) || inside(f.dest) { + out[f.id] = true + } + } + return out +} + +// removable checks that x can be taken out of the flow and returns the one +// flow that leaves it. +func removable(g *graph, x *node, verb string) (flowRef, error) { + switch x.typ { + case "Microflows$ActionActivity", "Microflows$LoopedActivity": + default: + return flowRef{}, fmt.Errorf("cannot %s %s: only an activity or a loop can be, because a decision or an end event "+ + "changes the shape of the flow", verb, describeNode(x)) + } + normal, errs := g.outgoing(x.id) + if len(errs) > 0 { + return flowRef{}, fmt.Errorf("cannot %s %s: it has an error handler, which would be left with no activity; "+ + "rewrite the flow with create or modify instead", verb, describeNode(x)) + } + if len(normal) != 1 { + return flowRef{}, fmt.Errorf("cannot %s %s: it has %d outgoing flows, not one", verb, describeNode(x), len(normal)) + } + return normal[0], nil +} + +// --------------------------------------------------------------------------- +// Tree edits +// --------------------------------------------------------------------------- + +// addFragment serializes the fragment and appends it: its objects to the top +// level collection (appended, so no stored object changes list position), its +// flows to the Flows list. +func (m *Mutator) addFragment(frag *backend.MicroflowFragment) error { + oc := dDoc(m.doc, "ObjectCollection") + objs := arrayElements(dGet(oc, "Objects")) + for _, obj := range frag.Objects { + d, err := m.deps.SerializeObject(obj) + if err != nil { + return fmt.Errorf("serialize %T: %w", obj, err) + } + if d == nil { + return fmt.Errorf("cannot write a %T into a flow", obj) + } + objs = append(objs, d) + } + if !setArray(oc, "Objects", objs) { + return fmt.Errorf("the object collection has no Objects list") + } + for _, f := range frag.Flows { + if err := m.addFlow(f); err != nil { + return err + } + } + for _, af := range frag.AnnotationFlows { + d, err := m.deps.SerializeAnnotationFlow(af) + if err != nil { + return fmt.Errorf("serialize annotation flow: %w", err) + } + if err := m.appendFlowDoc(d); err != nil { + return err + } + } + return nil +} + +func (m *Mutator) addFlow(f *microflows.SequenceFlow) error { + d, err := m.deps.SerializeSequenceFlow(f) + if err != nil { + return fmt.Errorf("serialize sequence flow: %w", err) + } + return m.appendFlowDoc(d) +} + +func (m *Mutator) appendFlowDoc(d bson.D) error { + if d == nil { + return fmt.Errorf("a flow serialized to nothing") + } + flows := arrayElements(dGet(m.doc, "Flows")) + flows = append(flows, d) + if !setArray(m.doc, "Flows", flows) { + return fmt.Errorf("the unit has no Flows list") + } + return nil +} + +func (m *Mutator) removeFlows(ids map[string]bool) { + var keep []any + for _, el := range arrayElements(dGet(m.doc, "Flows")) { + if d, ok := el.(bson.D); ok && ids[binaryID(dGet(d, "$ID"))] { + m.markRemoved(d) + continue + } + keep = append(keep, el) + } + setArray(m.doc, "Flows", keep) +} + +// removeObject takes x out of the top-level collection. +func (m *Mutator) removeObject(x *node) error { + oc := dDoc(m.doc, "ObjectCollection") + var keep []any + found := false + for _, el := range arrayElements(dGet(oc, "Objects")) { + if d, ok := el.(bson.D); ok && binaryID(dGet(d, "$ID")) == x.id { + m.markRemoved(d) + found = true + continue + } + keep = append(keep, el) + } + if !found { + return fmt.Errorf("%s is not in the top-level collection", describeNode(x)) + } + setArray(oc, "Objects", keep) + return nil +} + +// markRemoved records the $ID of d and of every element nested in it. +func (m *Mutator) markRemoved(v any) { + switch t := v.(type) { + case bson.D: + for _, e := range t { + if e.Key == "$ID" { + if b, ok := e.Value.(primitive.Binary); ok { + m.removed[string(b.Data)] = true + } + continue + } + m.markRemoved(e.Value) + } + case bson.A: + for _, el := range t { + m.markRemoved(el) + } + } +} + +// shift moves every top-level node past the cut (along axis ax, in the +// direction of s) by delta. The sweep keeps everything on either side of the +// cut exactly as it was relative to its neighbours, so only the flows that +// cross the cut get longer; a flow's control vectors are relative to its ends, +// so no curve changes. skip is a node that is about to be removed. +func (m *Mutator) shift(g *graph, ax axis, s int, cut float64, delta int, skip string) { + for _, n := range g.order { + if n.loop != "" || n.id == skip { + continue + } + if float64(s)*(float64(n.pos.get(ax))-cut) <= 0 { + continue + } + n.pos = n.pos.set(ax, n.pos.get(ax)+delta) + dSet(n.doc, "RelativeMiddlePoint", n.pos.String()) + } +} + +// checkIntegrity is the rule-1 guard: no binary anywhere in the unit may +// still name an element that was removed, and no two elements may share an +// $ID. Either would make the document unopenable, and neither is caught by +// anything cheaper than Studio Pro. +func (m *Mutator) checkIntegrity() error { + seen := map[string]bool{} + var dup, dangling string + var walk func(v any, key string) + walk = func(v any, key string) { + switch t := v.(type) { + case bson.D: + for _, e := range t { + walk(e.Value, e.Key) + } + case bson.A: + for _, el := range t { + walk(el, key) + } + case primitive.Binary: + if len(t.Data) != 16 { + return + } + k := string(t.Data) + if key == "$ID" { + if seen[k] && dup == "" { + dup = types.BlobToUUID(t.Data) + } + seen[k] = true + return + } + if m.removed[k] && dangling == "" { + dangling = key + " -> " + types.BlobToUUID(t.Data) + } + } + } + walk(m.doc, "") + if dup != "" { + return fmt.Errorf("refusing to write: two elements would share $ID %s", dup) + } + if dangling != "" { + return fmt.Errorf("refusing to write: a reference still points at a removed element (%s)", dangling) + } + return nil +} + +// --------------------------------------------------------------------------- +// Geometry +// --------------------------------------------------------------------------- + +// minGap is the free space kept on each side of an inserted fragment, the +// edge-to-edge distance mxcli's own layout leaves between activities. +const minGap = 40 + +type axis int + +const ( + axisX axis = iota + axisY +) + +func (a axis) other() axis { return 1 - a } + +type point struct{ X, Y int } + +func (p point) get(a axis) int { + if a == axisX { + return p.X + } + return p.Y +} + +func (p point) set(a axis, v int) point { + if a == axisX { + p.X = v + } else { + p.Y = v + } + return p +} + +func (p point) String() string { return strconv.Itoa(p.X) + ";" + strconv.Itoa(p.Y) } + +func parsePoint(s string) point { + x, y, ok := strings.Cut(s, ";") + if !ok { + return point{} + } + px, _ := strconv.Atoi(strings.TrimSpace(x)) + py, _ := strconv.Atoi(strings.TrimSpace(y)) + return point{px, py} +} + +// flowAxis is the direction a flow from x to y runs: the axis along which the +// centres are further apart, and the sign along it. +func flowAxis(x, y *node) (axis, int, error) { + dx, dy := y.pos.X-x.pos.X, y.pos.Y-x.pos.Y + switch { + case dx == 0 && dy == 0: + return 0, 0, fmt.Errorf("%s and %s are drawn on the same spot; cannot tell which way the flow runs", describeNode(x), describeNode(y)) + case abs(dx) >= abs(dy): + return axisX, sign(dx), nil + default: + return axisY, sign(dy), nil + } +} + +// Connection indexes, as Mendix numbers the sides of a box. +const ( + sideTop = 0 + sideRight = 1 + sideBottom = 2 + sideLeft = 3 +) + +// side is the side of a box that faces direction s along ax. +func side(ax axis, s int) int { + switch { + case ax == axisX && s > 0: + return sideRight + case ax == axisX: + return sideLeft + case s > 0: + return sideBottom + default: + return sideTop + } +} + +// sideVector is the control vector Studio Pro draws a straight flow end with: +// perpendicular to the side, pointing out of the box. +func sideVector(sd int) string { + switch sd { + case sideTop: + return "0;-15" + case sideRight: + return "15;0" + case sideBottom: + return "0;15" + default: + return "-15;0" + } +} + +// fragmentBox is the fragment's layout as the builder produced it, to be +// translated into place. +type fragmentBox struct { + frag *backend.MicroflowFragment + min, max point +} + +func fragmentGeometry(frag *backend.MicroflowFragment) (*fragmentBox, error) { + if frag == nil || len(frag.Objects) == 0 || frag.Entry == "" || frag.Exit == "" { + return nil, fmt.Errorf("the fragment is empty") + } + fb := &fragmentBox{frag: frag} + first := true + for _, obj := range frag.Objects { + p, sz := obj.GetPosition(), objectSize(obj) + lo := point{p.X - sz.X/2, p.Y - sz.Y/2} + hi := point{p.X + sz.X/2, p.Y + sz.Y/2} + if first { + fb.min, fb.max, first = lo, hi, false + continue + } + fb.min = point{min(fb.min.X, lo.X), min(fb.min.Y, lo.Y)} + fb.max = point{max(fb.max.X, hi.X), max(fb.max.Y, hi.Y)} + } + return fb, nil +} + +func (fb *fragmentBox) length(ax axis) int { return fb.max.get(ax) - fb.min.get(ax) } + +// placeCentred translates the fragment so that its box is centred on c along +// ax, and its entry object sits on c across it. +func (fb *fragmentBox) placeCentred(ax axis, c point) { + var entry point + for _, obj := range fb.frag.Objects { + if obj.GetID() == fb.frag.Entry { + entry = point{obj.GetPosition().X, obj.GetPosition().Y} + } + } + d := point{} + d = d.set(ax, c.get(ax)-(fb.min.get(ax)+fb.max.get(ax))/2) + d = d.set(ax.other(), c.get(ax.other())-entry.get(ax.other())) + for _, obj := range fb.frag.Objects { + p := obj.GetPosition() + obj.SetPosition(model.Point{X: p.X + d.X, Y: p.Y + d.Y}) + } + fb.min = point{fb.min.X + d.X, fb.min.Y + d.Y} + fb.max = point{fb.max.X + d.X, fb.max.Y + d.Y} +} + +// checkRoom refuses a placement that would draw the fragment over an object +// that stays where it is. The shift clears the space past the cut; an object +// that already sat inside the gap (a branch drawn below the main line, say) +// is not moved, and drawing over it would hide it in Studio Pro. skip is the +// object a replace removes. +func (fb *fragmentBox) checkRoom(g *graph, skip string) error { + for _, n := range g.order { + if n.loop != "" || n.id == skip { + continue + } + lo := point{n.pos.X - n.size.X/2, n.pos.Y - n.size.Y/2} + hi := point{n.pos.X + n.size.X/2, n.pos.Y + n.size.Y/2} + if lo.X < fb.max.X && fb.min.X < hi.X && lo.Y < fb.max.Y && fb.min.Y < hi.Y { + return fmt.Errorf("there is no free room for the fragment: at (%d, %d)-(%d, %d) it would be drawn over %s; "+ + "move that object aside in Studio Pro first", fb.min.X, fb.min.Y, fb.max.X, fb.max.Y, describeNode(n)) + } + } + return nil +} + +func objectSize(obj microflows.MicroflowObject) point { + if s, ok := obj.(interface{ GetSize() model.Size }); ok { + sz := s.GetSize() + return point{sz.Width, sz.Height} + } + return point{} +} + +// flowEnd reads a flow's connection index and control vector at one end +// ("Origin" or "Destination"). +func flowEnd(d bson.D, end string) (int, string) { + idx := 0 + switch v := dGet(d, end+"ConnectionIndex").(type) { + case int32: + idx = int(v) + case int64: + idx = int(v) + case int: + idx = v + } + vec := "" + if line := dDoc(d, "Line"); line != nil { + vec = dString(line, end+"ControlVector") + } + return idx, vec +} + +func abs(v int) int { + if v < 0 { + return -v + } + return v +} + +func sign(v int) int { + if v < 0 { + return -1 + } + return 1 +} + +func describeNode(n *node) string { + name := strings.TrimPrefix(n.typ, "Microflows$") + if c := dString(n.doc, "Caption"); c != "" && name != "ActionActivity" { + name += " '" + c + "'" + } + return fmt.Sprintf("the %s at (%d, %d)", name, n.pos.X, n.pos.Y) +} + +// --------------------------------------------------------------------------- +// bson.D helpers. The unit is decoded with bson v1 into bson.D, whose nested +// documents are bson.D and whose arrays are bson.A with Mendix's leading +// int32 list marker. +// --------------------------------------------------------------------------- + +func dGet(d bson.D, key string) any { + for _, e := range d { + if e.Key == key { + return e.Value + } + } + return nil +} + +func dDoc(d bson.D, key string) bson.D { + v, _ := dGet(d, key).(bson.D) + return v +} + +func dString(d bson.D, key string) string { + v, _ := dGet(d, key).(string) + return v +} + +func dSet(d bson.D, key string, v any) bool { + for i := range d { + if d[i].Key == key { + d[i].Value = v + return true + } + } + return false +} + +// arrayElements returns the elements of a Mendix list, without its marker. +func arrayElements(v any) []any { + a, ok := v.(bson.A) + if !ok || len(a) == 0 { + return nil + } + if _, isMarker := a[0].(int32); isMarker { + return append([]any(nil), a[1:]...) + } + return append([]any(nil), a...) +} + +// setArray replaces a list's elements, keeping its stored marker. +func setArray(d bson.D, key string, elements []any) bool { + a, ok := dGet(d, key).(bson.A) + if !ok { + return false + } + out := bson.A{} + if len(a) > 0 { + if marker, isMarker := a[0].(int32); isMarker { + out = append(out, marker) + } + } + return dSet(d, key, append(out, elements...)) +} + +func binaryID(v any) string { + b, ok := v.(primitive.Binary) + if !ok || len(b.Data) != 16 { + return "" + } + return types.BlobToUUID(b.Data) +} + +// setPointer points key at the element id, keeping the binary subtype the +// stored pointer uses. +func setPointer(d bson.D, key string, id model.ID) { + sub := byte(0) + if b, ok := dGet(d, key).(primitive.Binary); ok { + sub = b.Subtype + } + dSet(d, key, primitive.Binary{Subtype: sub, Data: types.UUIDToBlob(string(id))}) +} + +func setBinary(d bson.D, key string, v any) { + if b, ok := v.(primitive.Binary); ok { + dSet(d, key, primitive.Binary{Subtype: b.Subtype, Data: bytes.Clone(b.Data)}) + } +} + +// setInt writes an integer property with the width it is stored in. +func setInt(d bson.D, key string, v int) { + switch dGet(d, key).(type) { + case int64: + dSet(d, key, int64(v)) + default: + dSet(d, key, int32(v)) + } +} + +// setVector sets a control vector on the flow's line. A flow without a +// Bezier line (a pre-10 document) has nothing to set. +func setVector(d bson.D, key, v string) { + if v == "" { + return + } + if line := dDoc(d, "Line"); line != nil { + dSet(line, key, v) + } +} diff --git a/mdl/backend/mfmutator/splice_test.go b/mdl/backend/mfmutator/splice_test.go new file mode 100644 index 000000000..a665bff52 --- /dev/null +++ b/mdl/backend/mfmutator/splice_test.go @@ -0,0 +1,404 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mfmutator + +import ( + "bytes" + "fmt" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/primitive" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// These tests run the splice on hand-built documents, for the shapes the +// Studio Pro fixture does not have (loops, error handlers, vertical flows). +// The acceptance test on a Studio Pro-drawn flow is in the executor package +// (cmd_alter_flow_pedapp_test.go). + +// uid returns a deterministic element id for a short name. +func uid(name string) string { + b := make([]byte, 16) + copy(b, name) + return types.BlobToUUID(b) +} + +func bin(name string) primitive.Binary { + return primitive.Binary{Subtype: 0, Data: types.UUIDToBlob(uid(name))} +} + +func obj(name, typ string, x, y int) bson.D { + w, h := 120, 60 + if typ != "Microflows$ActionActivity" && typ != "Microflows$LoopedActivity" { + w, h = 20, 20 + } + return bson.D{ + {Key: "$ID", Value: bin(name)}, + {Key: "$Type", Value: typ}, + {Key: "RelativeMiddlePoint", Value: fmt.Sprintf("%d;%d", x, y)}, + {Key: "Size", Value: fmt.Sprintf("%d;%d", w, h)}, + } +} + +func flow(name, from, to string, fromSide, toSide int32, isErr bool) bson.D { + return bson.D{ + {Key: "$ID", Value: bin(name)}, + {Key: "$Type", Value: "Microflows$SequenceFlow"}, + {Key: "CaseValues", Value: bson.A{int32(2), bson.D{{Key: "$ID", Value: bin(name + "c")}, {Key: "$Type", Value: "Microflows$NoCase"}}}}, + {Key: "DestinationConnectionIndex", Value: toSide}, + {Key: "DestinationPointer", Value: bin(to)}, + {Key: "IsErrorHandler", Value: isErr}, + {Key: "Line", Value: bson.D{ + {Key: "$ID", Value: bin(name + "l")}, + {Key: "$Type", Value: "Microflows$BezierCurve"}, + {Key: "DestinationControlVector", Value: "-30;0"}, + {Key: "OriginControlVector", Value: "30;0"}, + }}, + {Key: "OriginConnectionIndex", Value: fromSide}, + {Key: "OriginPointer", Value: bin(from)}, + } +} + +func unit(objects []bson.D, flows []bson.D) bson.D { + objs := bson.A{int32(3)} + for _, o := range objects { + objs = append(objs, o) + } + fl := bson.A{int32(3)} + for _, f := range flows { + fl = append(fl, f) + } + return bson.D{ + {Key: "$ID", Value: bin("unit")}, + {Key: "$Type", Value: "Microflows$Microflow"}, + {Key: "Flows", Value: fl}, + {Key: "ObjectCollection", Value: bson.D{ + {Key: "$ID", Value: bin("oc")}, + {Key: "$Type", Value: "Microflows$MicroflowObjectCollection"}, + {Key: "Objects", Value: objs}, + }}, + } +} + +// fakeDeps serializes the way the codec does for the properties the splice +// reads, and records what was saved. +type fakeDeps struct{ saved []byte } + +func (d *fakeDeps) SerializeObject(o microflows.MicroflowObject) (bson.D, error) { + typ := "Microflows$ActionActivity" + if _, ok := o.(*microflows.ExclusiveMerge); ok { + typ = "Microflows$ExclusiveMerge" + } + p := o.GetPosition() + sz := objectSize(o) + return bson.D{ + {Key: "$ID", Value: primitive.Binary{Data: types.UUIDToBlob(string(o.GetID()))}}, + {Key: "$Type", Value: typ}, + {Key: "RelativeMiddlePoint", Value: fmt.Sprintf("%d;%d", p.X, p.Y)}, + {Key: "Size", Value: fmt.Sprintf("%d;%d", sz.X, sz.Y)}, + }, nil +} + +func (d *fakeDeps) SerializeSequenceFlow(f *microflows.SequenceFlow) (bson.D, error) { + return bson.D{ + {Key: "$ID", Value: primitive.Binary{Data: types.UUIDToBlob(string(f.ID))}}, + {Key: "$Type", Value: "Microflows$SequenceFlow"}, + {Key: "DestinationConnectionIndex", Value: int32(f.DestinationConnectionIndex)}, + {Key: "DestinationPointer", Value: primitive.Binary{Data: types.UUIDToBlob(string(f.DestinationID))}}, + {Key: "IsErrorHandler", Value: f.IsErrorHandler}, + {Key: "Line", Value: bson.D{ + {Key: "DestinationControlVector", Value: f.DestinationControlVector}, + {Key: "OriginControlVector", Value: f.OriginControlVector}, + }}, + {Key: "OriginConnectionIndex", Value: int32(f.OriginConnectionIndex)}, + {Key: "OriginPointer", Value: primitive.Binary{Data: types.UUIDToBlob(string(f.OriginID))}}, + }, nil +} + +func (d *fakeDeps) SerializeAnnotationFlow(f *microflows.AnnotationFlow) (bson.D, error) { + return bson.D{ + {Key: "$ID", Value: primitive.Binary{Data: types.UUIDToBlob(string(f.ID))}}, + {Key: "$Type", Value: "Microflows$AnnotationFlow"}, + {Key: "DestinationPointer", Value: primitive.Binary{Data: types.UUIDToBlob(string(f.DestinationID))}}, + {Key: "OriginPointer", Value: primitive.Binary{Data: types.UUIDToBlob(string(f.OriginID))}}, + }, nil +} + +func (d *fakeDeps) SaveUnit(_ string, contents []byte) error { + d.saved = contents + return nil +} + +func newMutator(t *testing.T, doc bson.D) (*Mutator, *fakeDeps) { + t.Helper() + raw, err := bson.Marshal(doc) + if err != nil { + t.Fatal(err) + } + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + deps := &fakeDeps{} + m, err := New(d, "unit", deps) + if err != nil { + t.Fatal(err) + } + return m, deps +} + +// oneActivity is a single-activity fragment in builder coordinates. +func oneActivity() *backend.MicroflowFragment { + id := model.ID(types.GenerateID()) + act := &microflows.ActionActivity{BaseActivity: microflows.BaseActivity{BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: id}, + Position: model.Point{X: 360, Y: 200}, + Size: model.Size{Width: 120, Height: 60}, + }}} + return &backend.MicroflowFragment{Objects: []microflows.MicroflowObject{act}, Entry: id, Exit: id} +} + +func line() []bson.D { + return []bson.D{ + obj("start", "Microflows$StartEvent", 100, 200), + obj("a", "Microflows$ActionActivity", 250, 200), + obj("b", "Microflows$ActionActivity", 420, 200), + obj("end", "Microflows$EndEvent", 600, 200), + } +} + +func lineFlows() []bson.D { + return []bson.D{ + flow("f1", "start", "a", 1, 3, false), + flow("f2", "a", "b", 1, 3, false), + flow("f3", "b", "end", 1, 3, false), + } +} + +// The control every splice test leans on: decoding a unit and encoding it +// again with nothing spliced gives back the stored bytes exactly. +func TestSplice_UntouchedUnitRoundTripsExactly(t *testing.T) { + raw, _ := bson.Marshal(unit(line(), lineFlows())) + m, _ := newMutator(t, unit(line(), lineFlows())) + out, err := m.Bytes() + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(out, raw) { + t.Error("an unmodified unit did not round-trip byte for byte") + } +} + +func TestSplice_InsertAfterKeepsTheErrorHandlerFlow(t *testing.T) { + objs := append(line(), obj("handler", "Microflows$ActionActivity", 250, 350)) + flows := append(lineFlows(), flow("err", "a", "handler", 2, 0, true)) + m, _ := newMutator(t, unit(objs, flows)) + errBefore, _ := bson.Marshal(flow("err", "a", "handler", 2, 0, true)) + + frag := oneActivity() + if err := m.InsertAfter(model.ID(uid("a")), frag); err != nil { + t.Fatal(err) + } + g := m.graph() + for _, f := range g.flows { + switch f.id { + case uid("err"): + got, _ := bson.Marshal(f.doc) + if !bytes.Equal(got, errBefore) { + t.Error("the error-handler flow changed") + } + case uid("f2"): + if f.dest != string(frag.Entry) { + t.Error("the normal flow out of a was not rewired to the fragment") + } + } + } +} + +func TestSplice_VerticalFlowUsesTopAndBottom(t *testing.T) { + objs := []bson.D{ + obj("a", "Microflows$ActionActivity", 200, 100), + obj("b", "Microflows$ActionActivity", 200, 200), + obj("below", "Microflows$ActionActivity", 200, 300), + obj("beside", "Microflows$ActionActivity", 500, 100), + } + flows := []bson.D{flow("f", "a", "b", 2, 0, false), flow("g", "b", "below", 2, 0, false)} + m, _ := newMutator(t, unit(objs, flows)) + frag := oneActivity() + if err := m.InsertAfter(model.ID(uid("a")), frag); err != nil { + t.Fatal(err) + } + g := m.graph() + if p := g.nodes[uid("b")].pos; p.X != 200 || p.Y <= 200 { + t.Errorf("b should have moved down, is at %v", p) + } + if p := g.nodes[uid("beside")].pos; p.X != 500 || p.Y != 100 { + t.Errorf("an object above the cut moved to %v", p) + } + for _, f := range g.flows { + if f.id == uid("f") { + if idx, _ := flowEnd(f.doc, "Destination"); idx != sideTop { + t.Errorf("the rewired flow enters the fragment on side %d, want top", idx) + } + } + if f.origin == string(frag.Exit) { + if idx, _ := flowEnd(f.doc, "Origin"); idx != sideBottom { + t.Errorf("the new flow leaves the fragment on side %d, want bottom", idx) + } + } + } +} + +func TestSplice_Refusals(t *testing.T) { + loop := obj("loop", "Microflows$LoopedActivity", 420, 200) + loop = append(loop, bson.E{Key: "ObjectCollection", Value: bson.D{ + {Key: "$ID", Value: bin("loopoc")}, + {Key: "$Type", Value: "Microflows$MicroflowObjectCollection"}, + {Key: "Objects", Value: bson.A{int32(3), obj("inner", "Microflows$ActionActivity", 100, 60), obj("inner2", "Microflows$ActionActivity", 260, 60)}}, + }}) + withLoop := []bson.D{obj("start", "Microflows$StartEvent", 100, 200), obj("a", "Microflows$ActionActivity", 250, 200), loop} + loopFlows := []bson.D{flow("f1", "start", "a", 1, 3, false), flow("f2", "a", "loop", 1, 3, false), flow("fi", "inner", "inner2", 1, 3, false)} + + twoIn := append(line(), obj("c", "Microflows$ActionActivity", 250, 350)) + twoInFlows := append(lineFlows(), flow("f4", "c", "b", 0, 2, false)) + + withHandler := append(line(), obj("handler", "Microflows$ActionActivity", 250, 350)) + handlerFlows := append(lineFlows(), flow("err", "a", "handler", 2, 0, true)) + + cases := []struct { + name string + objs []bson.D + flows []bson.D + op func(m *Mutator) error + want string + }{ + {"insert inside a loop", withLoop, loopFlows, + func(m *Mutator) error { return m.InsertAfter(model.ID(uid("inner")), oneActivity()) }, "inside a loop"}, + {"drop inside a loop", withLoop, loopFlows, + func(m *Mutator) error { return m.Drop(model.ID(uid("inner"))) }, "inside a loop"}, + {"insert before a join", twoIn, twoInFlows, + func(m *Mutator) error { return m.InsertBefore(model.ID(uid("b")), oneActivity()) }, "2 flows enter"}, + {"insert after the end", line(), lineFlows(), + func(m *Mutator) error { return m.InsertAfter(model.ID(uid("end")), oneActivity()) }, "ends the flow"}, + {"drop an activity with an error handler", withHandler, handlerFlows, + func(m *Mutator) error { return m.Drop(model.ID(uid("a"))) }, "error handler"}, + {"replace an activity with an error handler", withHandler, handlerFlows, + func(m *Mutator) error { return m.Replace(model.ID(uid("a")), oneActivity()) }, "error handler"}, + {"drop the end event", line(), lineFlows(), + func(m *Mutator) error { return m.Drop(model.ID(uid("end"))) }, "cannot drop"}, + {"an empty fragment", line(), lineFlows(), + func(m *Mutator) error { return m.InsertAfter(model.ID(uid("a")), &backend.MicroflowFragment{}) }, "empty"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + m, _ := newMutator(t, unit(tc.objs, tc.flows)) + before, _ := bson.Marshal(m.doc) + err := tc.op(m) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("want an error containing %q, got %v", tc.want, err) + } + after, _ := bson.Marshal(m.doc) + if !bytes.Equal(before, after) { + t.Error("a refused operation changed the document") + } + }) + } +} + +// CLAUDE.md rule 1: an element is never removed while anything still points +// at it. A pointer the splice does not know about (here, a made-up property +// on another object) is found by value, and the write is refused. +func TestSplice_DropRefusesADanglingReference(t *testing.T) { + objs := line() + objs[3] = append(objs[3], bson.E{Key: "SomePointer", Value: bin("a")}) + m, deps := newMutator(t, unit(objs, lineFlows())) + if err := m.Drop(model.ID(uid("a"))); err != nil { + t.Fatalf("drop: %v", err) + } + err := m.Save() + if err == nil || !strings.Contains(err.Error(), "still points at a removed element") { + t.Fatalf("want the dangling-reference refusal, got %v", err) + } + if deps.saved != nil { + t.Error("a unit with a dangling reference was saved") + } +} + +// Control for the test above: the same drop without the stray pointer saves. +func TestSplice_DropJoinsTheFlows(t *testing.T) { + m, deps := newMutator(t, unit(line(), lineFlows())) + if err := m.Drop(model.ID(uid("a"))); err != nil { + t.Fatalf("drop: %v", err) + } + if err := m.Save(); err != nil { + t.Fatalf("save: %v", err) + } + if bytes.Contains(deps.saved, types.UUIDToBlob(uid("a"))) { + t.Error("the dropped activity's $ID is still in the unit") + } + g := m.graph() + for _, f := range g.flows { + if f.id == uid("f1") && f.dest != uid("b") { + t.Errorf("the flow into the dropped activity now enters %s, want b", f.dest) + } + if f.id == uid("f2") { + t.Error("the flow out of the dropped activity is still there") + } + } +} + +// A loop's body flows are stored in the unit's Flows list, not in the loop. +// Dropping or replacing the loop takes them with it; left behind, they point at +// the removed body objects and Save refuses the unit (a Studio Pro-authored +// loop with two body activities, ACT_ConflictedWorkflowHelper_ApplyJumpTo in +// TestApp, hit exactly that). +func TestSplice_DropOrReplaceALoopTakesItsBodyFlows(t *testing.T) { + build := func() ([]bson.D, []bson.D) { + loop := obj("loop", "Microflows$LoopedActivity", 420, 200) + loop = append(loop, bson.E{Key: "ObjectCollection", Value: bson.D{ + {Key: "$ID", Value: bin("loopoc")}, + {Key: "$Type", Value: "Microflows$MicroflowObjectCollection"}, + {Key: "Objects", Value: bson.A{int32(3), obj("inner", "Microflows$ActionActivity", 100, 60), obj("inner2", "Microflows$ActionActivity", 260, 60)}}, + }}) + objs := []bson.D{ + obj("start", "Microflows$StartEvent", 100, 200), + obj("a", "Microflows$ActionActivity", 250, 200), + loop, + obj("end", "Microflows$EndEvent", 700, 200), + } + flows := []bson.D{ + flow("f1", "start", "a", 1, 3, false), + flow("f2", "a", "loop", 1, 3, false), + flow("fi", "inner", "inner2", 1, 3, false), + flow("f3", "loop", "end", 1, 3, false), + } + return objs, flows + } + ops := map[string]func(m *Mutator) error{ + "drop": func(m *Mutator) error { return m.Drop(model.ID(uid("loop"))) }, + "replace": func(m *Mutator) error { return m.Replace(model.ID(uid("loop")), oneActivity()) }, + } + for name, op := range ops { + t.Run(name, func(t *testing.T) { + objs, flows := build() + m, deps := newMutator(t, unit(objs, flows)) + if err := op(m); err != nil { + t.Fatalf("%s: %v", name, err) + } + if err := m.Save(); err != nil { + t.Fatalf("save: %v", err) + } + for _, gone := range []string{"loop", "inner", "inner2", "fi"} { + if bytes.Contains(deps.saved, types.UUIDToBlob(uid(gone))) { + t.Errorf("%s is still in the unit", gone) + } + } + }) + } +} diff --git a/mdl/backend/microflow_mutation.go b/mdl/backend/microflow_mutation.go new file mode 100644 index 000000000..2737d66c9 --- /dev/null +++ b/mdl/backend/microflow_mutation.go @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 + +package backend + +import ( + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// MicroflowFragment is what an `alter microflow` insert or replace splices in: +// the objects and flows a fragment of MDL builds to, written exactly as +// `create microflow` writes the same statements, and the two objects the +// surrounding flow connects to. Entry is where the flow into the fragment +// ends; Exit is where the flow out of it starts. For a one-activity fragment +// they are the same object. +// +// Positions are the builder's own; the mutator translates the fragment into +// place (plan item 4.2c) before writing it. +type MicroflowFragment struct { + Objects []microflows.MicroflowObject + Flows []*microflows.SequenceFlow + AnnotationFlows []*microflows.AnnotationFlow + Entry, Exit model.ID +} + +// MicroflowMutator splices into one stored microflow or nanoflow (ADR-0012 +// decision 3). Targets are activity IDs, already resolved from their content +// address by mfmutator.Resolve. Every operation edits the stored document in +// place: untouched elements stay byte-identical, and nothing is rebuilt. +// Call Save to persist. +type MicroflowMutator interface { + // InsertAfter splices frag onto the one flow that leaves target. + InsertAfter(target model.ID, frag *MicroflowFragment) error + // InsertBefore splices frag onto the one flow that enters target. + InsertBefore(target model.ID, frag *MicroflowFragment) error + // Replace puts frag in target's place and removes target. + Replace(target model.ID, frag *MicroflowFragment) error + // Drop removes target and joins its incoming flows to its successor. + Drop(target model.ID) error + // Save writes the patched unit. + Save() error +} + +// MicroflowMutationBackend opens a microflow or nanoflow for splicing. +type MicroflowMutationBackend interface { + // OpenMicroflowForMutation loads a Microflows$Microflow or + // Microflows$Nanoflow unit and returns a mutator over its stored form. + OpenMicroflowForMutation(unitID model.ID) (MicroflowMutator, error) +} diff --git a/mdl/backend/mock/backend.go b/mdl/backend/mock/backend.go index b5fa2477a..cef40cf4a 100644 --- a/mdl/backend/mock/backend.go +++ b/mdl/backend/mock/backend.go @@ -341,6 +341,9 @@ type MockBackend struct { // WorkflowMutationBackend OpenWorkflowForMutationFunc func(unitID model.ID) (backend.WorkflowMutator, error) + // MicroflowMutationBackend + OpenMicroflowForMutationFunc func(unitID model.ID) (backend.MicroflowMutator, error) + // WidgetSerializationBackend // WidgetBuilderBackend diff --git a/mdl/backend/mock/mock_mutation.go b/mdl/backend/mock/mock_mutation.go index f9d9f53c6..a0ee642bc 100644 --- a/mdl/backend/mock/mock_mutation.go +++ b/mdl/backend/mock/mock_mutation.go @@ -32,6 +32,17 @@ func (m *MockBackend) OpenWorkflowForMutation(unitID model.ID) (backend.Workflow return nil, fmt.Errorf("MockBackend.OpenWorkflowForMutation not configured") } +// --------------------------------------------------------------------------- +// MicroflowMutationBackend +// --------------------------------------------------------------------------- + +func (m *MockBackend) OpenMicroflowForMutation(unitID model.ID) (backend.MicroflowMutator, error) { + if m.OpenMicroflowForMutationFunc != nil { + return m.OpenMicroflowForMutationFunc(unitID) + } + return nil, fmt.Errorf("MockBackend.OpenMicroflowForMutation not configured") +} + // --------------------------------------------------------------------------- // WidgetSerializationBackend // --------------------------------------------------------------------------- diff --git a/mdl/backend/modelsdk/microflow_mutator_write.go b/mdl/backend/modelsdk/microflow_mutator_write.go new file mode 100644 index 000000000..21f8a2810 --- /dev/null +++ b/mdl/backend/modelsdk/microflow_mutator_write.go @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "fmt" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/mfmutator" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/modelsdk/codec" + "github.com/mendixlabs/mxcli/modelsdk/element" + genMf "github.com/mendixlabs/mxcli/modelsdk/gen/microflows" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// OpenMicroflowForMutation loads a microflow or nanoflow unit and returns the +// shared graph splice (mfmutator) over its stored bytes. New objects and flows +// are encoded with the same converters `create microflow` uses, so a fragment +// is written exactly as the same statements would be in a create; the patched +// unit is written through the reconciling writer as a patch. +func (b *Backend) OpenMicroflowForMutation(unitID model.ID) (backend.MicroflowMutator, error) { + if b.writer == nil { + return nil, fmt.Errorf("OpenMicroflowForMutation: not connected for writing") + } + raw, err := b.reader.GetRawUnitBytes(string(unitID)) + if err != nil { + return nil, fmt.Errorf("OpenMicroflowForMutation: load unit: %w", err) + } + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + return nil, fmt.Errorf("OpenMicroflowForMutation: unmarshal: %w", err) + } + return mfmutator.New(d, unitID, codecMicroflowDeps{b: b}) +} + +// codecMicroflowDeps implements mfmutator.Deps for the modelsdk (codec) backend. +type codecMicroflowDeps struct{ b *Backend } + +var _ mfmutator.Deps = codecMicroflowDeps{} + +func (d codecMicroflowDeps) SerializeObject(obj microflows.MicroflowObject) (bson.D, error) { + el := microflowObjectToGen(obj) + if el == nil { + return nil, nil + } + assignFlowObjectIDs(el) + return encodeFlowElementToD(el) +} + +func (d codecMicroflowDeps) SerializeSequenceFlow(f *microflows.SequenceFlow) (bson.D, error) { + el := sequenceFlowToGen(f, d.b.majorVersion()) + assignID(el) + if sf, ok := el.(*genMf.SequenceFlow); ok { + for _, cv := range sf.CaseValuesItems() { + assignID(cv) + } + assignID(sf.Line()) + } + return encodeFlowElementToD(el) +} + +func (d codecMicroflowDeps) SerializeAnnotationFlow(f *microflows.AnnotationFlow) (bson.D, error) { + el := annotationFlowToGen(f, d.b.majorVersion()) + assignID(el) + if af, ok := el.(*genMf.AnnotationFlow); ok { + assignID(af.Line()) + } + return encodeFlowElementToD(el) +} + +// SaveUnit writes the spliced unit as a patch: the reconciling writer elides +// an unchanged unit and guards storage GUIDs as for every write, but does not +// re-pair element $IDs the splice deliberately kept (canon.ContentsOwnElementIDs). +func (d codecMicroflowDeps) SaveUnit(unitID string, contents []byte) error { + return d.b.writer.UpdateRawUnitPatch(unitID, contents) +} + +func encodeFlowElementToD(el element.Element) (bson.D, error) { + raw, err := (&codec.Encoder{}).Encode(el) + if err != nil { + return nil, err + } + var out bson.D + if err := bson.Unmarshal(raw, &out); err != nil { + return nil, err + } + return out, nil +} diff --git a/mdl/backend/modelsdk/unimplemented_gen.go b/mdl/backend/modelsdk/unimplemented_gen.go index 232eed4a2..b7b3f4f78 100644 --- a/mdl/backend/modelsdk/unimplemented_gen.go +++ b/mdl/backend/modelsdk/unimplemented_gen.go @@ -891,6 +891,11 @@ func (unimplemented) MoveViewEntitySourceDocument(_ string, _ model.ID, _ string return errUnimplemented("MoveViewEntitySourceDocument") } +func (unimplemented) OpenMicroflowForMutation(_ model.ID) (backend.MicroflowMutator, error) { + var r0 backend.MicroflowMutator + return r0, errUnimplemented("OpenMicroflowForMutation") +} + func (unimplemented) OpenPageForMutation(_ model.ID) (backend.PageMutator, error) { var r0 backend.PageMutator return r0, errUnimplemented("OpenPageForMutation") @@ -1118,6 +1123,10 @@ func (unimplemented) UpdateJsonStructure(_ *types.JsonStructure) error { return errUnimplemented("UpdateJsonStructure") } +func (unimplemented) UpdateLayout(_ *pages.Layout) error { + return errUnimplemented("UpdateLayout") +} + func (unimplemented) UpdateMenuDocument(_ *types.MenuDocument) error { return errUnimplemented("UpdateMenuDocument") } @@ -1224,3 +1233,8 @@ func (unimplemented) WriteJavaScriptSourceFile(_ string, _ string, _ string, _ [ func (unimplemented) WriteJavaSourceFile(_ string, _ string, _ string, _ []*types.JavaActionParameter, _ types.CodeActionReturnType, _ []string, _ string) error { return errUnimplemented("WriteJavaSourceFile") } + +func (unimplemented) WriteViewEntitySourceDocument(_ model.ID, _ string, _ string, _ string, _ string) (model.ID, error) { + var r0 model.ID + return r0, errUnimplemented("WriteViewEntitySourceDocument") +} diff --git a/mdl/deprecation/deprecation.go b/mdl/deprecation/deprecation.go index 50d5c47eb..217a682be 100644 --- a/mdl/deprecation/deprecation.go +++ b/mdl/deprecation/deprecation.go @@ -62,20 +62,33 @@ type Entry struct { CanonicalExample string } -// Rewrite replaces one keyword token of the deprecated form with another. It is -// the only shape the seeded entries need; a richer one is added with the first -// entry that needs it. +// Rewrite is the mechanical rewrite from the deprecated form to the canonical +// one. It is either a keyword swap (Token and Replacement) or, where the two +// forms differ in shape rather than in one word, a structural rewrite that +// Structural names. A structural rewrite is implemented against the parse tree, +// never as text substitution, and its correctness rests on the same test as a +// swap: Example and CanonicalExample must build the same statements. type Rewrite struct { // Token is the keyword to replace, lower-case. Token string // Replacement is the keyword written in its place, lower-case. Replacement string + // Structural describes a rewrite that is not a keyword swap, e.g. + // "call form to statement form". Empty for a keyword swap. + Structural string } // Codes of the registered entries, for the visitor to record. const ( CreateOrReplace = "MDL-DEPR001" Show = "MDL-DEPR002" + // ListOperationFunctionForm is `$x = head($L)` and the other list + // operations written as calls; find and contains are excluded, because the + // call form clashes with the string functions (see mdl/visitor, MDL-V1-LIST). + ListOperationFunctionForm = "MDL-DEPR003" + // AggregateFunctionForm is `$n = count($L)` and the other aggregates + // written as calls. + AggregateFunctionForm = "MDL-DEPR004" ) // entries is the registry. Append only: a code is never reused or renumbered, @@ -108,6 +121,31 @@ var entries = []Entry{ Example: "show entities in M;", CanonicalExample: "list entities in M;", }, + { + Code: ListOperationFunctionForm, + Old: "$x = <operation>($List, …)", + Canonical: "$x = <operation> $List …", + Rewrite: Rewrite{Structural: "call form to statement form: head/tail $L; filter/find $L by Member = v " + + "(when the condition has that shape) or where <expr>; sort $L by …; union/intersect $A with $B; " + + "subtract($A, $B) -> subtract $B from $A; equals $A and $B; range($L, o, n) -> range $L offset o limit n"}, + RemovedIn: 2, + Note: "A list operation is one Studio Pro activity whose operand is a variable, so the statement form " + + "cannot nest. find(…) and contains(…) are not reported here: the call form is also the string " + + "function, so they are version-gated instead (MDL-V1-LIST).", + Example: "create microflow M.F ($L: List of M.E) begin $H = head($L); end;", + CanonicalExample: "create microflow M.F ($L: List of M.E) begin $H = head $L; end;", + }, + { + Code: AggregateFunctionForm, + Old: "$n = <function>($List, …)", + Canonical: "$n = <function> $List …", + Rewrite: Rewrite{Structural: "call form to statement form: count $L; sum|average|minimum|maximum " + + "$L by Attr (for $L.Attr) or of <expr>; all|any $L where <expr>; reduce $L from <initial> as <type> using <expr>"}, + RemovedIn: 2, + Note: "An aggregate is one Studio Pro Aggregate list activity whose operand is a variable.", + Example: "create microflow M.F ($L: List of M.E) begin $N = count($L); end;", + CanonicalExample: "create microflow M.F ($L: List of M.E) begin $N = count $L; end;", + }, } // All returns every registered entry, in code order. diff --git a/mdl/deprecation/deprecation_test.go b/mdl/deprecation/deprecation_test.go index 8084f5c43..de608703f 100644 --- a/mdl/deprecation/deprecation_test.go +++ b/mdl/deprecation/deprecation_test.go @@ -27,8 +27,11 @@ func TestRegistryEntriesAreWellFormed(t *testing.T) { if got, ok := Lookup(e.Code); !ok || got.Code != e.Code { t.Errorf("Lookup(%q) = %v, %v", e.Code, got.Code, ok) } - if e.Old == "" || e.Canonical == "" || e.Rewrite.Token == "" || e.Rewrite.Replacement == "" || - e.Example == "" || e.CanonicalExample == "" { + swap := e.Rewrite.Token != "" && e.Rewrite.Replacement != "" + if swap == (e.Rewrite.Structural != "") { + t.Errorf("%s: a rewrite is either a keyword swap or structural, exactly one: %+v", e.Code, e.Rewrite) + } + if e.Old == "" || e.Canonical == "" || e.Example == "" || e.CanonicalExample == "" { t.Errorf("%s is incomplete: %+v", e.Code, e) } // ADR-0011: an alias warns under the version that deprecates it (1) diff --git a/mdl/executor/cmd_alter_flow.go b/mdl/executor/cmd_alter_flow.go new file mode 100644 index 000000000..d8aac4309 --- /dev/null +++ b/mdl/executor/cmd_alter_flow.go @@ -0,0 +1,504 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "regexp" + "sort" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/mfmutator" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// execAlterFlow handles `alter microflow|nanoflow Module.Name { … }`: a patch +// of the stored flow (ADR-0012 decision 3), never a rebuild. +// +// Every target is resolved against the flow AS STORED, before any operation +// runs, so an address means what `describe … with handles` showed: an +// ambiguity or a miss refuses the whole statement before anything changes, and +// a later operation cannot address what an earlier one inserted. +func execAlterFlow(ctx *ExecContext, s *ast.AlterFlowStmt) error { + if !ctx.Connected() { + return mdlerrors.NewNotConnected() + } + if !ctx.ConnectedForWrite() { + return mdlerrors.NewNotConnectedWrite() + } + a, err := loadAlterFlow(ctx, s) + if err != nil { + return err + } + + targets := make([]mfmutator.Candidate, len(s.Operations)) + for i, op := range s.Operations { + c, err := mfmutator.ResolveText(a.cands, op.Target) + if err != nil { + return mdlerrors.NewValidation(fmt.Sprintf("alter %s %s: %s %s: %v", s.Kind(), s.Name, op.Op, op.Target, err)) + } + targets[i] = c + } + + mut, err := ctx.Backend.OpenMicroflowForMutation(a.mf.ID) + if err != nil { + return mdlerrors.NewBackend("open "+s.Kind()+" for alter", err) + } + for i, op := range s.Operations { + target := targets[i] + fail := func(err error) error { + return mdlerrors.NewValidation(fmt.Sprintf("alter %s %s: %s %s: %v", s.Kind(), s.Name, op.Op, op.Target, err)) + } + if op.Op == ast.AlterFlowDrop { + if err := a.checkOutputUnused(target, nil); err != nil { + return fail(err) + } + if err := mut.Drop(target.ID); err != nil { + return fail(err) + } + a.noteRemoved(target, nil) + continue + } + frag, err := a.buildFragment(ctx, op.Body) + if err != nil { + return fail(err) + } + if err := a.checkFragmentScope(ctx, op, target, frag); err != nil { + return fail(err) + } + switch op.Op { + case ast.AlterFlowInsertAfter: + err = mut.InsertAfter(target.ID, frag) + case ast.AlterFlowInsertBefore: + err = mut.InsertBefore(target.ID, frag) + case ast.AlterFlowReplace: + if err = a.checkOutputUnused(target, frag); err == nil { + err = mut.Replace(target.ID, frag) + } + default: + err = fmt.Errorf("unknown operation") + } + if err != nil { + return fail(err) + } + if op.Op == ast.AlterFlowReplace { + a.noteRemoved(target, frag) + } + a.noteFragment(ctx, frag) + } + if err := mut.Save(); err != nil { + return mdlerrors.NewBackend("save altered "+s.Kind(), err) + } + fmt.Fprintf(ctx.Output, "Altered %s %s\n", s.Kind(), s.Name) + return nil +} + +// alterFlowContext is what the operations of one statement share: the stored +// flow (a nanoflow wrapped as a microflow, the way describe renders one), its +// addressable activities, and the name maps rendering needs. +type alterFlowContext struct { + stmt *ast.AlterFlowStmt + mf *microflows.Microflow + cands []mfmutator.Candidate + entityNames map[model.ID]string + microflowNames map[model.ID]string + + // What the statement's earlier operations did to the variables, so a later + // one is checked against the flow as it will be written, not as stored: + // the variables their fragments declare, the ones their fragments read + // (with who reads them), and the stored outputs they took away. + declaredByOps map[string]bool + readByOps map[string][]string + removedByOps map[string]bool +} + +// noteRemoved records that target's output is gone, unless the fragment that +// replaces it declares it again. +func (a *alterFlowContext) noteRemoved(target mfmutator.Candidate, replacement *backend.MicroflowFragment) { + v := target.OutputVariable + if v == "" || fragmentDeclares(replacement, v) { + return + } + a.removedByOps[v] = true +} + +// noteFragment records what an inserted or replacing fragment declares and +// reads. +func (a *alterFlowContext) noteFragment(ctx *ExecContext, frag *backend.MicroflowFragment) { + for _, obj := range frag.Objects { + if act, ok := obj.(*microflows.ActionActivity); ok { + if v := mfmutator.OutputVariable(act.Action); v != "" { + a.declaredByOps[v] = true + } + } + text := formatActivity(ctx, obj, a.entityNames, a.microflowNames) + for _, m := range variableRef.FindAllStringSubmatch(text, -1) { + a.readByOps[m[1]] = append(a.readByOps[m[1]], text) + } + } +} + +func fragmentDeclares(frag *backend.MicroflowFragment, v string) bool { + if frag == nil { + return false + } + for _, obj := range frag.Objects { + if act, ok := obj.(*microflows.ActionActivity); ok && mfmutator.OutputVariable(act.Action) == v { + return true + } + } + return false +} + +func loadAlterFlow(ctx *ExecContext, s *ast.AlterFlowStmt) (*alterFlowContext, error) { + h, err := getHierarchy(ctx) + if err != nil { + return nil, mdlerrors.NewBackend("build hierarchy", err) + } + a := &alterFlowContext{stmt: s, entityNames: getEntityNames(ctx, h), + declaredByOps: map[string]bool{}, readByOps: map[string][]string{}, removedByOps: map[string]bool{}} + // A copy: nanoflow names are added below, and the cached map is shared. + a.microflowNames = map[model.ID]string{} + for id, n := range getMicroflowNames(ctx, h) { + a.microflowNames[id] = n + } + inModule := func(container model.ID, name string) bool { + return h.GetModuleName(h.FindModuleID(container)) == s.Name.Module && name == s.Name.Name + } + if s.Nanoflow { + nfs, err := ctx.Backend.ListNanoflows() + if err != nil { + return nil, mdlerrors.NewBackend("list nanoflows", err) + } + for _, nf := range nfs { + a.microflowNames[nf.ID] = h.GetQualifiedName(nf.ContainerID, nf.Name) + } + nf, ok := pickLive(nfs, + func(nf *microflows.Nanoflow) bool { return inModule(nf.ContainerID, nf.Name) }, + func(nf *microflows.Nanoflow) bool { return nf.Excluded }) + if !ok { + return nil, mdlerrors.NewNotFound("nanoflow", s.Name.String()) + } + a.mf = &microflows.Microflow{ + BaseElement: nf.BaseElement, + ContainerID: nf.ContainerID, + Name: nf.Name, + Parameters: nf.Parameters, + ReturnType: nf.ReturnType, + ReturnVariableName: nf.ReturnVariableName, + ObjectCollection: nf.ObjectCollection, + } + } else { + mfs, err := ctx.Backend.ListMicroflows() + if err != nil { + return nil, mdlerrors.NewBackend("list microflows", err) + } + mf, ok := pickLive(mfs, + func(mf *microflows.Microflow) bool { return inModule(mf.ContainerID, mf.Name) }, + func(mf *microflows.Microflow) bool { return mf.Excluded }) + if !ok { + return nil, mdlerrors.NewNotFound("microflow", s.Name.String()) + } + a.mf = mf + } + if a.mf.ObjectCollection == nil { + return nil, mdlerrors.NewValidation(fmt.Sprintf("%s %s has no flow to alter", s.Kind(), s.Name)) + } + a.cands, _, _, _ = microflowTargets(ctx, a.mf, a.entityNames, a.microflowNames) + return a, nil +} + +// buildFragment builds a fragment's statements with the builder `create +// microflow` uses, seeded with the variables the stored flow declares, and +// cuts it out of the start and end events the builder wraps it in. +func (a *alterFlowContext) buildFragment(ctx *ExecContext, body []ast.MicroflowStatement) (*backend.MicroflowFragment, error) { + if len(body) == 0 { + return nil, fmt.Errorf("the fragment is empty; use drop to remove an activity") + } + varTypes, declared := a.storedVariables(ctx) + hierarchy, _ := getHierarchy(ctx) + restServices, _ := loadRestServices(ctx) + fb := &flowBuilder{ + textLang: authoringLanguage(ctx), + posX: 200, + posY: 200, + baseY: 200, + spacing: HorizontalSpacing, + varTypes: varTypes, + declaredVars: declared, + measurer: &layoutMeasurer{varTypes: varTypes}, + backend: ctx.Backend, + hierarchy: hierarchy, + restServices: restServices, + isNanoflow: a.stmt.Nanoflow, + } + oc := fb.buildFlowGraph(body, nil) + if errs := fb.GetErrors(); len(errs) > 0 { + return nil, fmt.Errorf("the fragment has errors:\n - %s", strings.Join(errs, "\n - ")) + } + if fb.endsWithReturn { + return nil, fmt.Errorf("the fragment ends the flow with a return, so nothing would lead on to the rest of it; " + + "a return inside an inserted fragment is not supported yet") + } + return cutFragment(oc) +} + +// cutFragment removes the builder's start event and final end event, and says +// where the fragment is entered and left. Several paths reaching the end (an +// if without a merge before it, an error handler that rejoins at the end) are +// joined by a merge, which becomes the exit. +func cutFragment(oc *microflows.MicroflowObjectCollection) (*backend.MicroflowFragment, error) { + var start, end microflows.MicroflowObject + ends := 0 + for _, obj := range oc.Objects { + switch obj.(type) { + case *microflows.StartEvent: + start = obj + case *microflows.EndEvent: + ends++ + end = obj + } + } + if start == nil || end == nil { + return nil, fmt.Errorf("the fragment does not continue: its last statement ends the flow, so nothing would lead on to the rest of it") + } + if ends > 1 { + return nil, fmt.Errorf("the fragment returns; a return inside an inserted fragment is not supported yet") + } + frag := &backend.MicroflowFragment{} + var intoEnd []*microflows.SequenceFlow + for _, f := range oc.Flows { + switch { + case f.OriginID == start.GetID(): + if frag.Entry != "" { + return nil, fmt.Errorf("the fragment starts with more than one flow") + } + frag.Entry = f.DestinationID + case f.DestinationID == end.GetID(): + intoEnd = append(intoEnd, f) + default: + frag.Flows = append(frag.Flows, f) + } + } + for _, obj := range oc.Objects { + if obj != start && obj != end { + frag.Objects = append(frag.Objects, obj) + } + } + frag.AnnotationFlows = oc.AnnotationFlows + switch { + case frag.Entry == "" || frag.Entry == end.GetID() || len(frag.Objects) == 0: + return nil, fmt.Errorf("the fragment builds no activity") + case len(intoEnd) == 0: + return nil, fmt.Errorf("no path through the fragment leads on to the rest of the flow") + case len(intoEnd) == 1: + frag.Exit = intoEnd[0].OriginID + default: + p := end.GetPosition() + merge := &microflows.ExclusiveMerge{BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + Position: p, + Size: model.Size{Width: MergeSize, Height: MergeSize}, + }} + for _, f := range intoEnd { + f.DestinationID = merge.ID + frag.Flows = append(frag.Flows, f) + } + frag.Objects = append(frag.Objects, merge) + frag.Exit = merge.ID + } + return frag, nil +} + +// storedVariables returns the variables the stored flow declares, in the two +// maps the builder keeps: entity-typed ones with their entity (a change or a +// member access resolves attributes through it), and the rest as declared. +func (a *alterFlowContext) storedVariables(ctx *ExecContext) (varTypes, declared map[string]string) { + varTypes, declared = map[string]string{}, map[string]string{} + add := func(name string, dt microflows.DataType) { + if name == "" { + return + } + t := "Unknown" + if dt != nil { + t = formatMicroflowDataType(ctx, dt, a.entityNames) + } + switch dt.(type) { + case *microflows.ObjectType, *microflows.ListType: + varTypes[name] = t + default: + declared[name] = t + } + } + for _, p := range a.mf.Parameters { + add(p.Name, p.Type) + } + for _, c := range a.cands { + act, ok := c.Object.(*microflows.ActionActivity) + if !ok || c.OutputVariable == "" { + continue + } + switch x := act.Action.(type) { + case *microflows.CreateVariableAction: + add(c.OutputVariable, x.DataType) + case *microflows.CreateObjectAction: + if x.EntityQualifiedName != "" { + varTypes[c.OutputVariable] = x.EntityQualifiedName + } else if n, ok := a.entityNames[x.EntityID]; ok { + varTypes[c.OutputVariable] = n + } else { + declared[c.OutputVariable] = "Object" + } + default: + declared[c.OutputVariable] = "Unknown" + } + } + return varTypes, declared +} + +// systemVariables are in scope everywhere they exist at all; the platform +// reports a misuse (a $latestError outside an error handler) itself. +var systemVariables = map[string]bool{ + "currentUser": true, "currentSession": true, "currentObject": true, "currentDeviceType": true, + "latestError": true, "latestHttpResponse": true, "latestSoapFault": true, +} + +var variableRef = regexp.MustCompile(`\$([A-Za-z_][A-Za-z0-9_]*)`) + +// checkFragmentScope is plan item 4.2d: the fragment is checked in the scope +// of its insertion point. A variable it declares that the flow already has is +// an error (it would shadow or clash with the stored one); a variable it uses +// that is not declared upstream of where it goes, nor by the fragment itself, +// is an error too, since the fragment would read something that does not exist +// yet on that path. +func (a *alterFlowContext) checkFragmentScope(ctx *ExecContext, op *ast.AlterFlowOperation, target mfmutator.Candidate, frag *backend.MicroflowFragment) error { + existing := map[string]bool{} + for _, p := range a.mf.Parameters { + existing[p.Name] = true + } + for _, c := range a.cands { + if c.OutputVariable != "" { + existing[c.OutputVariable] = true + } + } + own := map[string]bool{} + for _, obj := range frag.Objects { + // A loop's iterator exists only inside the loop, which the fragment + // brings along; it reads it there, so it is the fragment's own. + if loop, ok := obj.(*microflows.LoopedActivity); ok { + if src, ok := loop.LoopSource.(*microflows.IterableList); ok && src.VariableName != "" { + own[src.VariableName] = true + } + continue + } + act, ok := obj.(*microflows.ActionActivity) + if !ok { + continue + } + v := mfmutator.OutputVariable(act.Action) + if v == "" { + continue + } + replacingSame := op.Op == ast.AlterFlowReplace && v == target.OutputVariable + if existing[v] && !replacingSame { + return fmt.Errorf("the fragment declares $%s, which the %s already has; choose another name", v, a.stmt.Kind()) + } + if a.declaredByOps[v] { + return fmt.Errorf("the fragment declares $%s, which an earlier operation of this alter already declares; choose another name", v) + } + own[v] = true + } + + inScope := map[string]bool{} + for _, p := range a.mf.Parameters { + inScope[p.Name] = true + } + for id := range a.upstreamOf(target.ID, op.Op == ast.AlterFlowInsertAfter) { + for _, c := range a.cands { + if c.ID == id && c.OutputVariable != "" { + inScope[c.OutputVariable] = true + } + } + } + var missing []string + seen := map[string]bool{} + for _, obj := range frag.Objects { + for _, m := range variableRef.FindAllStringSubmatch(formatActivity(ctx, obj, a.entityNames, a.microflowNames), -1) { + v := m[1] + if seen[v] || systemVariables[v] || own[v] || (inScope[v] && !a.removedByOps[v]) { + continue + } + seen[v] = true + missing = append(missing, "$"+v) + } + } + if len(missing) > 0 { + sort.Strings(missing) + where := "before " + op.Target + if op.Op == ast.AlterFlowInsertAfter { + where = "after " + op.Target + } + return fmt.Errorf("the fragment uses %s, which is not declared on the path %s", strings.Join(missing, ", "), where) + } + return nil +} + +// upstreamOf returns every object from which id can be reached along the +// stored flows — the activities whose outputs exist when the flow gets there. +// id itself is included only when including says so (an insert after it runs +// once it has). +func (a *alterFlowContext) upstreamOf(id model.ID, including bool) map[model.ID]bool { + preds := map[model.ID][]model.ID{} + for _, f := range a.mf.ObjectCollection.Flows { + preds[f.DestinationID] = append(preds[f.DestinationID], f.OriginID) + } + out := map[model.ID]bool{} + queue := append([]model.ID(nil), preds[id]...) + for len(queue) > 0 { + n := queue[0] + queue = queue[1:] + if out[n] { + continue + } + out[n] = true + queue = append(queue, preds[n]...) + } + if including { + out[id] = true + } + return out +} + +// checkOutputUnused refuses to take away an activity whose output variable +// another activity still reads — unless the replacement declares it again. +func (a *alterFlowContext) checkOutputUnused(target mfmutator.Candidate, replacement *backend.MicroflowFragment) error { + v := target.OutputVariable + if v == "" || fragmentDeclares(replacement, v) { + return nil + } + if readers := a.readByOps[v]; len(readers) > 0 { + return fmt.Errorf("$%s is read by what an earlier operation of this alter adds: %s", v, strings.Join(readers, "; ")) + } + ref := regexp.MustCompile(`\$` + regexp.QuoteMeta(v) + `\b`) + var users []string + for _, c := range a.cands { + if c.ID == target.ID { + continue + } + for _, text := range append([]string{c.Statement}, c.Alternates...) { + if ref.MatchString(text) { + users = append(users, c.Statement) + break + } + } + } + if len(users) > 0 { + return fmt.Errorf("$%s is still used by: %s", v, strings.Join(users, "; ")) + } + return nil +} diff --git a/mdl/executor/cmd_alter_flow_pedapp_test.go b/mdl/executor/cmd_alter_flow_pedapp_test.go new file mode 100644 index 000000000..791677917 --- /dev/null +++ b/mdl/executor/cmd_alter_flow_pedapp_test.go @@ -0,0 +1,645 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "context" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/primitive" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// The acceptance test of plan item 4.2 (ako/mxcli#736) runs on the Studio +// Pro-authored PedApp fixture, because only a flow Studio Pro drew can show +// what a rebuild loses: its merges, its curves, its object order, its $IDs. + +// openPedAppFixture opens a private copy of testdata/pedapp. +func openPedAppFixture(t *testing.T) (*Executor, *bytes.Buffer) { + t.Helper() + src := filepath.Join("..", "..", "testdata", "pedapp") + if _, err := os.Stat(filepath.Join(src, "PedApp.mpr")); err != nil { + t.Skipf("PedApp fixture not found: %v", err) + } + dir := t.TempDir() + if err := copyPedAppFile(filepath.Join(src, "PedApp.mpr"), filepath.Join(dir, "PedApp.mpr")); err != nil { + t.Fatal(err) + } + if err := copyPedAppTree(filepath.Join(src, "mprcontents"), filepath.Join(dir, "mprcontents")); err != nil { + t.Fatal(err) + } + out := &bytes.Buffer{} + exec := New(out) + exec.SetBackendFactory(func() backend.FullBackend { return modelsdkbackend.New() }) + if err := exec.Execute(&ast.ConnectStmt{Path: filepath.Join(dir, "PedApp.mpr")}); err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { _ = exec.Execute(&ast.DisconnectStmt{}) }) + return exec, out +} + +func afRun(t *testing.T, exec *Executor, src string) error { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse %q: %v", src, errs[0]) + } + for _, s := range prog.Statements { + if err := exec.Execute(s); err != nil { + return err + } + } + return nil +} + +// valFeedbackUnit returns VAL_Feedback's unit ID and stored bytes. +func valFeedbackUnit(t *testing.T, exec *Executor) (model.ID, []byte) { + t.Helper() + ctx := exec.newExecContext(context.Background()) + h, err := getHierarchy(ctx) + if err != nil { + t.Fatal(err) + } + all, err := ctx.Backend.ListMicroflows() + if err != nil { + t.Fatal(err) + } + for _, m := range all { + if m.Name == "VAL_Feedback" && h.GetModuleName(h.FindModuleID(m.ContainerID)) == "FeedbackModule" { + raw, err := ctx.Backend.GetRawUnitBytes(m.ID) + if err != nil { + t.Fatal(err) + } + return m.ID, append([]byte(nil), raw...) + } + } + t.Fatal("FeedbackModule.VAL_Feedback not found") + return "", nil +} + +// flowView indexes a stored flow's top-level objects and flows by $ID. +type flowView struct { + doc bson.D + objs map[string]bson.D + flows map[string]bson.D + order []string // object ids in storage order +} + +func parseFlowView(t *testing.T, raw []byte) flowView { + t.Helper() + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + v := flowView{doc: d, objs: map[string]bson.D{}, flows: map[string]bson.D{}} + oc, _ := afGet(d, "ObjectCollection").(bson.D) + for _, el := range afList(afGet(oc, "Objects")) { + o := el.(bson.D) + id := afIDOf(afGet(o, "$ID")) + v.objs[id] = o + v.order = append(v.order, id) + } + for _, el := range afList(afGet(d, "Flows")) { + f := el.(bson.D) + v.flows[afIDOf(afGet(f, "$ID"))] = f + } + return v +} + +func afGet(d bson.D, k string) any { + for _, e := range d { + if e.Key == k { + return e.Value + } + } + return nil +} + +func afList(v any) []any { + a, _ := v.(bson.A) + if len(a) > 0 { + if _, ok := a[0].(int32); ok { + return a[1:] + } + } + return a +} + +func afIDOf(v any) string { + b, _ := v.(primitive.Binary) + return types.BlobToUUID(b.Data) +} + +func afMarshal(t *testing.T, v any) []byte { + t.Helper() + b, err := bson.Marshal(v) + if err != nil { + t.Fatal(err) + } + return b +} + +// without returns d minus the named keys, for comparing the rest. +func afWithout(d bson.D, keys ...string) bson.D { + var out bson.D + for _, e := range d { + skip := false + for _, k := range keys { + skip = skip || e.Key == k + } + if !skip { + out = append(out, e) + } + } + return out +} + +// changedKeys lists the keys whose values differ between two elements (one +// level deep, plus the Line's vectors, which is where a flow keeps its curve). +func afChangedKeys(t *testing.T, a, b bson.D) []string { + t.Helper() + var out []string + keys := map[string]bool{} + for _, e := range a { + keys[e.Key] = true + } + for _, e := range b { + keys[e.Key] = true + } + for k := range keys { + av, bv := afGet(a, k), afGet(b, k) + if k == "Line" { + al, _ := av.(bson.D) + bl, _ := bv.(bson.D) + for _, lk := range []string{"OriginControlVector", "DestinationControlVector"} { + if fmt.Sprint(afGet(al, lk)) != fmt.Sprint(afGet(bl, lk)) { + out = append(out, "Line."+lk) + } + } + if !bytes.Equal(afMarshal(t, afWithout(al, "OriginControlVector", "DestinationControlVector")), + afMarshal(t, afWithout(bl, "OriginControlVector", "DestinationControlVector"))) { + out = append(out, "Line") + } + continue + } + if !bytes.Equal(afMarshal(t, bson.D{{Key: "v", Value: av}}), afMarshal(t, bson.D{{Key: "v", Value: bv}})) { + out = append(out, k) + } + } + return afSort(out) +} + +func afSort(s []string) []string { + for i := 1; i < len(s); i++ { + for j := i; j > 0 && s[j] < s[j-1]; j-- { + s[j], s[j-1] = s[j-1], s[j] + } + } + return s +} + +func afPoint(d bson.D) (int, int) { + s, _ := afGet(d, "RelativeMiddlePoint").(string) + x, y, _ := strings.Cut(s, ";") + px, _ := strconv.Atoi(x) + py, _ := strconv.Atoi(y) + return px, py +} + +func afSize(d bson.D) (int, int) { + s, _ := afGet(d, "Size").(string) + x, y, _ := strings.Cut(s, ";") + px, _ := strconv.Atoi(x) + py, _ := strconv.Atoi(y) + return px, py +} + +// objectAt returns the id of the stored top-level object at (x, y). +func (v flowView) objectAt(t *testing.T, x, y int, typ string) string { + t.Helper() + for _, id := range v.order { + o := v.objs[id] + if px, py := afPoint(o); px == x && py == y && afGet(o, "$Type") == typ { + return id + } + } + t.Fatalf("no %s at (%d, %d)", typ, x, y) + return "" +} + +// The acceptance test of plan item 4.2 (ako/mxcli#736): one `log` inserted +// after $IsValidEmail in the Studio Pro-drawn VAL_Feedback. Only the new +// activity, the two flows around it and the positions moved to make room may +// differ; every other element — its $ID, its curve, every merge — must come +// through byte-identical. +func TestAlterMicroflow_PedApp_InsertAfterChangesOnlyTheSplice(t *testing.T) { + exec, _ := openPedAppFixture(t) + _, raw := valFeedbackUnit(t, exec) + before := parseFlowView(t, raw) + javaCall := before.objectAt(t, 980, 200, "Microflows$ActionActivity") + split := before.objectAt(t, 1155, 200, "Microflows$ExclusiveSplit") + + script := "alter microflow FeedbackModule.VAL_Feedback {\n" + + " insert after $IsValidEmail { log info node 'Feedback' 'Email checked'; }\n" + + "};" + if n := strings.Count(script, "\n") + 1; n > 5 { + t.Fatalf("the acceptance script is %d lines; the plan allows 5", n) + } + if err := afRun(t, exec, script); err != nil { + t.Fatalf("alter: %v", err) + } + _, rawAfter := valFeedbackUnit(t, exec) + after := parseFlowView(t, rawAfter) + + // The document around the flow is untouched. + if !bytes.Equal(afMarshal(t, afWithout(before.doc, "ObjectCollection", "Flows")), + afMarshal(t, afWithout(after.doc, "ObjectCollection", "Flows"))) { + t.Error("a property of the microflow document itself changed") + } + ocBefore, _ := afGet(before.doc, "ObjectCollection").(bson.D) + ocAfter, _ := afGet(after.doc, "ObjectCollection").(bson.D) + if !bytes.Equal(afMarshal(t, afWithout(ocBefore, "Objects")), afMarshal(t, afWithout(ocAfter, "Objects"))) { + t.Error("a property of the object collection changed") + } + + // Objects: every stored one survives with its $ID and in its place in the + // list; exactly one is new, a log activity; the rest differ at most in + // position, and only by the shift that made room. + for i, id := range before.order { + if after.order[i] != id { + t.Fatalf("stored object %d moved in the list: %s became %s", i, id, after.order[i]) + } + } + if got := len(after.order) - len(before.order); got != 1 { + t.Fatalf("want exactly one new object, got %d", got) + } + newID := after.order[len(after.order)-1] + newObj := after.objs[newID] + if action, _ := afGet(newObj, "Action").(bson.D); afGet(newObj, "$Type") != "Microflows$ActionActivity" || + afGet(action, "$Type") != "Microflows$LogMessageAction" { + t.Fatalf("the new object is not a log activity: %v", afGet(newObj, "$Type")) + } + shifted := 0 + javaX, _ := afPoint(before.objs[javaCall]) + for _, id := range before.order { + b, a := before.objs[id], after.objs[id] + changed := afChangedKeys(t, b, a) + if len(changed) == 0 { + continue + } + if len(changed) != 1 || changed[0] != "RelativeMiddlePoint" { + t.Errorf("object %s (%v) changed more than its position: %v", id, afGet(b, "$Type"), changed) + continue + } + bx, by := afPoint(b) + ax, ay := afPoint(a) + if ay != by || ax <= bx || bx <= javaX { + t.Errorf("object %s moved from (%d,%d) to (%d,%d): only objects past the insertion point may move, and only along the flow", + id, bx, by, ax, ay) + } + shifted++ + } + if shifted == 0 { + t.Error("nothing was shifted, yet the gap after $IsValidEmail is too narrow for an activity: placement did not run") + } + // The new activity overlaps nothing. + nx, ny := afPoint(newObj) + nw, nh := afSize(newObj) + for _, id := range after.order { + if id == newID { + continue + } + o := after.objs[id] + ox, oy := afPoint(o) + ow, oh := afSize(o) + if abs(nx-ox)*2 < nw+ow && abs(ny-oy)*2 < nh+oh { + t.Errorf("the new activity at (%d,%d) overlaps %v at (%d,%d)", nx, ny, afGet(o, "$Type"), ox, oy) + } + } + + // Flows: every stored flow survives; one is new (log -> split); one is + // rewired (java call -> log), and only at its destination end. + var added []string + for id := range after.flows { + if _, ok := before.flows[id]; !ok { + added = append(added, id) + } + } + if len(added) != 1 { + t.Fatalf("want exactly one new flow, got %d", len(added)) + } + nf := after.flows[added[0]] + if afIDOf(afGet(nf, "OriginPointer")) != newID || afIDOf(afGet(nf, "DestinationPointer")) != split { + t.Error("the new flow does not run from the log to 'Email is Valid?'") + } + rewired := 0 + for id, b := range before.flows { + a, ok := after.flows[id] + if !ok { + t.Errorf("stored flow %s is gone", id) + continue + } + changed := afChangedKeys(t, b, a) + if len(changed) == 0 { + continue + } + rewired++ + if afIDOf(afGet(b, "OriginPointer")) != javaCall || afIDOf(afGet(a, "DestinationPointer")) != newID { + t.Errorf("flow %s changed but is not the flow out of $IsValidEmail: %v", id, changed) + } + for _, k := range changed { + switch k { + case "DestinationPointer", "DestinationConnectionIndex", "Line.DestinationControlVector": + default: + t.Errorf("the rewired flow changed %s; only its destination end may change", k) + } + } + } + if rewired != 1 { + t.Errorf("want exactly one rewired flow, got %d", rewired) + } + // The new flow ends where the rewired one used to. + var oldIn bson.D + for id, b := range before.flows { + if afIDOf(afGet(b, "OriginPointer")) == javaCall { + oldIn = b + _ = id + } + } + if fmt.Sprint(afGet(nf, "DestinationConnectionIndex")) != fmt.Sprint(afGet(oldIn, "DestinationConnectionIndex")) { + t.Error("the new flow does not enter 'Email is Valid?' on the side the old flow did") + } + + // And the result reads back: describe shows the log between the two. + var buf bytes.Buffer + exec.output = &buf + if err := afRun(t, exec, "describe microflow FeedbackModule.VAL_Feedback;"); err != nil { + t.Fatal(err) + } + body := buf.String() + iCall := strings.Index(body, "$IsValidEmail = call java action") + iLog := strings.Index(body, "log info node 'Feedback' 'Email checked';") + iSplit := strings.Index(body, "@caption 'Email is Valid?'") + if iCall < 0 || iLog < iCall || iSplit < iLog { + t.Errorf("describe does not show the log between the call and the decision:\n%s", body) + } +} + +// The control: an alter with no operations reads the unit, patches nothing +// and writes nothing — the stored bytes survive the decode/encode round trip +// exactly, so any difference the test above sees is the splice's. +func TestAlterMicroflow_PedApp_EmptyAlterChangesNothing(t *testing.T) { + exec, _ := openPedAppFixture(t) + _, raw := valFeedbackUnit(t, exec) + if err := afRun(t, exec, "alter microflow FeedbackModule.VAL_Feedback { };"); err != nil { + t.Fatalf("alter: %v", err) + } + _, rawAfter := valFeedbackUnit(t, exec) + if !bytes.Equal(raw, rawAfter) { + t.Error("an empty alter changed the stored unit") + } +} + +// Drop joins the flow into the dropped activity to the one after it, and +// takes away only the activity and the flow that left it. +func TestAlterMicroflow_PedApp_Drop(t *testing.T) { + exec, _ := openPedAppFixture(t) + _, raw := valFeedbackUnit(t, exec) + before := parseFlowView(t, raw) + target := before.objectAt(t, 1305, 460, "Microflows$ActionActivity") + mergeAfter := before.objectAt(t, 1460, 460, "Microflows$ExclusiveMerge") + + if err := afRun(t, exec, "alter microflow FeedbackModule.VAL_Feedback { drop set $ValidFeedback = false @3; };"); err != nil { + t.Fatalf("alter: %v", err) + } + _, rawAfter := valFeedbackUnit(t, exec) + after := parseFlowView(t, rawAfter) + + if _, ok := after.objs[target]; ok { + t.Fatal("the dropped activity is still there") + } + if len(after.objs) != len(before.objs)-1 || len(after.flows) != len(before.flows)-1 { + t.Fatalf("want one object and one flow fewer, got %d->%d objects, %d->%d flows", + len(before.objs), len(after.objs), len(before.flows), len(after.flows)) + } + for id, b := range before.objs { + if id == target { + continue + } + if !bytes.Equal(afMarshal(t, b), afMarshal(t, after.objs[id])) { + t.Errorf("object %s changed", id) + } + } + rewired := 0 + for id, b := range before.flows { + a, ok := after.flows[id] + if !ok { + if afIDOf(afGet(b, "OriginPointer")) != target { + t.Errorf("flow %s is gone but did not leave the dropped activity", id) + } + continue + } + if changed := afChangedKeys(t, b, a); len(changed) > 0 { + rewired++ + if afIDOf(afGet(b, "DestinationPointer")) != target || afIDOf(afGet(a, "DestinationPointer")) != mergeAfter { + t.Errorf("flow %s changed but is not the flow into the dropped activity: %v", id, changed) + } + } + } + if rewired != 1 { + t.Errorf("want one rewired flow, got %d", rewired) + } + if bytes.Contains(rawAfter, uuidBytes(target)) { + t.Error("the unit still contains the dropped activity's $ID") + } +} + +func uuidBytes(id string) []byte { return types.UUIDToBlob(id) } + +// Replace puts a two-activity fragment where one activity was: the flows in +// and out are re-pointed, everything past it moves along to make room. +func TestAlterMicroflow_PedApp_Replace(t *testing.T) { + exec, _ := openPedAppFixture(t) + _, raw := valFeedbackUnit(t, exec) + before := parseFlowView(t, raw) + target := before.objectAt(t, 1305, 460, "Microflows$ActionActivity") + + err := afRun(t, exec, `alter microflow FeedbackModule.VAL_Feedback { + replace set $ValidFeedback = false @3 with { + set $ValidFeedback = false; + log warning node 'Feedback' 'Email rejected'; + } + };`) + if err != nil { + t.Fatalf("alter: %v", err) + } + _, rawAfter := valFeedbackUnit(t, exec) + after := parseFlowView(t, rawAfter) + if _, ok := after.objs[target]; ok { + t.Fatal("the replaced activity is still there") + } + if got := len(after.objs) - len(before.objs); got != 1 { + t.Errorf("want one object more (two in, one out), got %+d", got) + } + if got := len(after.flows) - len(before.flows); got != 1 { + t.Errorf("want one flow more (the fragment's own), got %+d", got) + } + for id := range before.flows { + if _, ok := after.flows[id]; !ok { + t.Errorf("stored flow %s is gone; replace keeps the flows in and out", id) + } + } + if bytes.Contains(rawAfter, uuidBytes(target)) { + t.Error("the unit still contains the replaced activity's $ID") + } + var buf bytes.Buffer + exec.output = &buf + if err := afRun(t, exec, "describe microflow FeedbackModule.VAL_Feedback;"); err != nil { + t.Fatal(err) + } + if !strings.Contains(buf.String(), "log warning node 'Feedback' 'Email rejected';") { + t.Errorf("describe does not show the replacement:\n%s", buf.String()) + } +} + +// What the splice cannot do safely, it refuses — before writing anything. +func TestAlterMicroflow_PedApp_Refusals(t *testing.T) { + cases := []struct{ name, op, want string }{ + {"insert after a decision", `insert after 'Email is Valid?' { log info 'x'; }`, "which branch"}, + {"drop a decision", `drop 'Email is Valid?';`, "cannot drop"}, + {"drop a variable still read", `drop $IsValidEmail;`, "still used"}, + {"drop the end", `drop return $ValidFeedback;`, "cannot drop"}, + {"declare an existing variable", `insert after $IsValidEmail { declare $ValidFeedback Boolean = true; }`, "already has"}, + {"use a variable not yet declared", `insert after $ValidFeedback { log info 'x {1}' with ({1} = toString($IsValidEmail)); }`, "not declared on the path"}, + {"ambiguous target", `drop set $ValidFeedback = false;`, "add an ordinal"}, + {"unknown target", `drop $Nope;`, "no activity matches"}, + {"a fragment that returns", `insert after $IsValidEmail { return false; }`, "ends the flow"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + exec, _ := openPedAppFixture(t) + _, raw := valFeedbackUnit(t, exec) + err := afRun(t, exec, "alter microflow FeedbackModule.VAL_Feedback { "+tc.op+" };") + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("want an error containing %q, got %v", tc.want, err) + } + if _, rawAfter := valFeedbackUnit(t, exec); !bytes.Equal(raw, rawAfter) { + t.Error("a refused alter changed the stored unit") + } + }) + } +} + +var _ = microflows.ActionActivity{} + +// alter nanoflow goes through the same splice: the unit is a +// Microflows$Nanoflow with the same object collection and flows. +func TestAlterNanoflow_PedApp_InsertBefore(t *testing.T) { + exec, _ := openPedAppFixture(t) + err := afRun(t, exec, `alter nanoflow FeedbackModule.ACT_Feedback_ClearImage { + insert before call javascript action * { declare $Cleared Boolean = true; } + };`) + if err != nil { + t.Fatalf("alter: %v", err) + } + var buf bytes.Buffer + exec.output = &buf + if err := afRun(t, exec, "describe nanoflow FeedbackModule.ACT_Feedback_ClearImage;"); err != nil { + t.Fatal(err) + } + body := buf.String() + iChange := strings.Index(body, "change $Feedback") + iDeclare := strings.Index(body, "declare $Cleared Boolean = true;") + iCall := strings.Index(body, "call javascript action") + if iChange < 0 || iDeclare < iChange || iCall < iDeclare { + t.Errorf("describe does not show the declare between the change and the call:\n%s", body) + } +} + +// The scope check covers the whole statement, not each operation against the +// stored flow alone. Measured with mx check 11.14 on TestApp before the fix: +// two inserts declaring the same variable gave CE0111 "Duplicate variable +// name", and an insert reading a variable a drop in the same statement took +// away gave CE0109 "Undefined variable" — both after "Altered microflow". +func TestAlterMicroflow_PedApp_ScopeSpansTheStatement(t *testing.T) { + t.Run("two fragments declare the same variable", func(t *testing.T) { + exec, _ := openPedAppFixture(t) + _, raw := valFeedbackUnit(t, exec) + err := afRun(t, exec, `alter microflow FeedbackModule.VAL_Feedback { + insert after $IsValidEmail { declare $Dup Boolean = true; } + insert after $ValidFeedback { declare $Dup Boolean = false; } + };`) + if err == nil || !strings.Contains(err.Error(), "$Dup") { + t.Fatalf("want the clash on $Dup refused, got %v", err) + } + if _, after := valFeedbackUnit(t, exec); !bytes.Equal(raw, after) { + t.Error("a refused alter changed the stored unit") + } + }) + + // SUB_Feedback_Sanitize reads its nine $Sanitized* outputs in one change; + // replacing that change first leaves $SanitizedPageName unread, so only a + // fragment of the second statement reads it. + const unread = `alter microflow FeedbackModule.SUB_Feedback_Sanitize { + replace change $Feedback (Subject = $SanitizedSubject, Description = $SanitizedDescription, SubmitterUUID = $SanitizedSubmitterUUID, SubmitterEmail = $SanitizedSubmitterEmail, SubmitterDisplayName = $SanitizedSubmitterDisplayName, ActiveUserRoles = $SanitizedActiveUserRoles, PageName = $SanitizedPageName, Browser = $SanitizedBrowser, EnvironmentURL = $SanitizedEnvironmentURL) with { log info 'sanitized'; } + };` + const use = `insert after $SanitizedSubmitterUUID { log info 'page {1}' with ({1} = $SanitizedPageName); }` + const drop = `drop $SanitizedPageName;` + for name, ops := range map[string]string{ + "insert a reader, then drop the producer": use + "\n" + drop, + "drop the producer, then insert a reader": drop + "\n" + use, + } { + t.Run(name, func(t *testing.T) { + exec, _ := openPedAppFixture(t) + if err := afRun(t, exec, unread); err != nil { + t.Fatalf("setup: %v", err) + } + err := afRun(t, exec, "alter microflow FeedbackModule.SUB_Feedback_Sanitize {\n"+ops+"\n};") + if err == nil || !strings.Contains(err.Error(), "$SanitizedPageName") { + t.Fatalf("want the read of a dropped $SanitizedPageName refused, got %v", err) + } + }) + } + // Control: each operation on its own is accepted. + t.Run("control", func(t *testing.T) { + for _, op := range []string{use, drop} { + exec, _ := openPedAppFixture(t) + if err := afRun(t, exec, unread); err != nil { + t.Fatalf("setup: %v", err) + } + if err := afRun(t, exec, "alter microflow FeedbackModule.SUB_Feedback_Sanitize {\n"+op+"\n};"); err != nil { + t.Errorf("%s: %v", op, err) + } + } + }) +} + +// A loop in the fragment declares its iterator; the scope check must count it +// as the fragment's own, or every loop fragment is refused as reading an +// undeclared variable. +func TestAlterMicroflow_PedApp_LoopFragmentDeclaresItsIterator(t *testing.T) { + exec, _ := openPedAppFixture(t) + err := afRun(t, exec, `alter microflow FeedbackModule.VAL_Feedback { + insert after $IsValidEmail { + $Items = create list of FeedbackModule.Feedback; + loop $Item in $Items begin log info 'item'; end loop; + } + };`) + if err != nil { + t.Fatalf("alter: %v", err) + } +} diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index ec6316e90..49248739b 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -1803,10 +1803,12 @@ func (fb *flowBuilder) addListOperationAction(s *ast.ListOperationStmt) model.ID } func (fb *flowBuilder) listAttributeOperation(s *ast.ListOperationStmt, filter bool) microflows.ListOperation { - binary, ok := s.Condition.(*ast.BinaryExpr) - if !ok || binary.Operator != "=" { + // `find $L where …` / `filter $L where …` is always the by-expression + // operation, even when the expression happens to read `Member = value`. + if s.ByExpression || !ast.IsMemberEquality(s.Condition) { return nil } + binary := s.Condition.(*ast.BinaryExpr) fieldName, ok := listOperationFieldName(binary.Left) if !ok || fieldName == "" { return nil diff --git a/mdl/executor/cmd_microflows_builder_list_activity_test.go b/mdl/executor/cmd_microflows_builder_list_activity_test.go new file mode 100644 index 000000000..90faa8766 --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_list_activity_test.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// `filter $L where Status = 'Open'` is Studio Pro's "Filter by expression" +// even though its expression reads `Member = value`; `filter $L by …` with the +// same condition is "Filter" by member (#733). The linking word decides, not +// the shape of the condition. +func TestFilterWhereIsAlwaysByExpression(t *testing.T) { + build := func(byExpression bool) microflows.ListOperation { + fb := predicateBuilder(t) + stmt := &ast.ListOperationStmt{ + Operation: ast.ListOpFilter, + InputVariable: "L", + OutputVariable: "R", + Condition: bare("Status", "=", "'Open'"), + ByExpression: byExpression, + } + oc := fb.buildFlowGraph([]ast.MicroflowStatement{stmt}, + &ast.MicroflowReturnType{Type: ast.DataType{Kind: ast.TypeBoolean}}) + if errs := fb.GetErrors(); len(errs) > 0 { + t.Fatalf("build errors: %v", errs) + } + for _, obj := range oc.Objects { + if act, ok := obj.(*microflows.ActionActivity); ok { + if lo, ok := act.Action.(*microflows.ListOperationAction); ok { + return lo.Operation + } + } + } + t.Fatal("no list operation built") + return nil + } + + op, ok := build(true).(*microflows.FilterOperation) + if !ok { + t.Fatalf("where: built %T, want *microflows.FilterOperation", build(true)) + } + if !strings.Contains(op.Expression, "$currentObject/Status") { + t.Errorf("where: expression %q does not read the member off $currentObject", op.Expression) + } + // Control: the same condition after `by` (or in the call form) is by member. + if got, ok := build(false).(*microflows.FilterByAttributeOperation); !ok || got.Attribute != "Shop.Order.Status" { + t.Errorf("by: built %#v, want Filter by attribute Shop.Order.Status", build(false)) + } +} diff --git a/mdl/executor/cmd_microflows_format_action.go b/mdl/executor/cmd_microflows_format_action.go index e8de99f43..c1bd5c00b 100644 --- a/mdl/executor/cmd_microflows_format_action.go +++ b/mdl/executor/cmd_microflows_format_action.go @@ -10,6 +10,7 @@ import ( "sort" "strings" + "github.com/mendixlabs/mxcli/mdl/langver" "github.com/mendixlabs/mxcli/mdl/visitor" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/microflows" @@ -441,6 +442,9 @@ func formatAction( attrName = parts[len(parts)-1] } } + if describeLanguage(ctx) >= langver.V1 { + return formatAggregateActivity(ctx, a, fn, attrName, outputVar, entityNames) + } // REDUCE carries the fold Mendix stores beside the expression. Both parts // are required, so they are rendered even when empty rather than dropped — // a reduce that describes without them cannot be executed back (#1004). @@ -1136,6 +1140,11 @@ func formatListOperation(ctx *ExecContext, op microflows.ListOperation, outputVa if op == nil { return fmt.Sprintf("$%s = list operation ...;", outputVar) } + if describeLanguage(ctx) >= langver.V1 { + if stmt, ok := formatListActivity(op, outputVar); ok { + return stmt + } + } switch o := op.(type) { case *microflows.HeadOperation: diff --git a/mdl/executor/cmd_microflows_format_list_activity.go b/mdl/executor/cmd_microflows_format_list_activity.go new file mode 100644 index 000000000..80f79a5c5 --- /dev/null +++ b/mdl/executor/cmd_microflows_format_list_activity.go @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/langver" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// describeLanguage is the MDL language version describe writes: the newest +// frozen version (the one whose header describe emits, langver.HeaderLine), +// or the version of the script the describe runs in when that is newer. +// +// While mdl 1 is a preview, a plain `describe` therefore keeps writing the +// mdl 0 forms, because its output carries no header that would make the newer +// forms mean what they say (ADR-0011). Inside an `mdl 1;` script it writes the +// mdl 1 forms, which the same script can execute back. +func describeLanguage(ctx *ExecContext) langver.Version { + v := langver.Frozen + if ctx != nil && ctx.LanguageVersion > v { + v = ctx.LanguageVersion + } + return v +} + +// formatListActivity renders a List operation activity as the statement that +// mirrors it (PROPOSAL_mdl_beta_syntax_freeze.md §4, #733). It returns false +// for an activity the statement form cannot express, which then falls back to +// the call form. +func formatListActivity(op microflows.ListOperation, outputVar string) (string, bool) { + out := "$" + outputVar + switch o := op.(type) { + case *microflows.HeadOperation: + return fmt.Sprintf("%s = head $%s;", out, o.ListVariable), true + case *microflows.TailOperation: + return fmt.Sprintf("%s = tail $%s;", out, o.ListVariable), true + case *microflows.FindOperation: + return fmt.Sprintf("%s = find $%s where %s;", out, o.ListVariable, describeExpr(o.Expression)), true + case *microflows.FilterOperation: + return fmt.Sprintf("%s = filter $%s where %s;", out, o.ListVariable, describeExpr(o.Expression)), true + case *microflows.FindByAttributeOperation: + return formatMemberListActivity("find", out, o.ListVariable, o.Attribute, o.Association, o.Expression) + case *microflows.FilterByAttributeOperation: + return formatMemberListActivity("filter", out, o.ListVariable, o.Attribute, o.Association, o.Expression) + case *microflows.SortOperation: + if len(o.Sorting) == 0 { + return "", false + } + items := make([]string, 0, len(o.Sorting)) + for _, s := range o.Sorting { + dir := "asc" + if s.Direction == microflows.SortDirectionDescending { + dir = "desc" + } + name := s.AttributeQualifiedName + if i := strings.LastIndex(name, "."); i >= 0 { + name = name[i+1:] + } + if name == "" { + return "", false + } + items = append(items, mdlIdent(name)+" "+dir) + } + return fmt.Sprintf("%s = sort $%s by %s;", out, o.ListVariable, strings.Join(items, ", ")), true + case *microflows.UnionOperation: + return fmt.Sprintf("%s = union $%s with $%s;", out, o.ListVariable1, o.ListVariable2), true + case *microflows.IntersectOperation: + return fmt.Sprintf("%s = intersect $%s with $%s;", out, o.ListVariable1, o.ListVariable2), true + case *microflows.SubtractOperation: + // Studio Pro's Subtract is the first list minus the second. + return fmt.Sprintf("%s = subtract $%s from $%s;", out, o.ListVariable2, o.ListVariable1), true + case *microflows.ContainsOperation: + return fmt.Sprintf("%s = contains $%s in $%s;", out, o.ObjectVariable, o.ListVariable), true + case *microflows.EqualsOperation: + return fmt.Sprintf("%s = equals $%s and $%s;", out, o.ListVariable1, o.ListVariable2), true + case *microflows.ListRangeOperation: + stmt := fmt.Sprintf("%s = range $%s", out, o.ListVariable) + if o.OffsetExpression != "" { + stmt += " offset " + describeExpr(o.OffsetExpression) + } + if o.LimitExpression != "" { + stmt += " limit " + describeExpr(o.LimitExpression) + } + return stmt + ";", true + } + return "", false +} + +// formatMemberListActivity renders Find / Filter by member: `by Member = value`. +func formatMemberListActivity(verb, out, list, attribute, association, value string) (string, bool) { + field := extractFieldName(attribute, association) + if value == "" { + return "", false + } + if field == "" { + return fmt.Sprintf("%s = %s $%s where %s;", out, verb, list, describeExpr(value)), true + } + return fmt.Sprintf("%s = %s $%s by %s = %s;", out, verb, list, field, describeExpr(value)), true +} + +// formatAggregateActivity renders an Aggregate list activity as the statement +// that mirrors it. fn is the function's MDL keyword and attrName the short +// name of the aggregated attribute, both already resolved by the caller. +func formatAggregateActivity(ctx *ExecContext, a *microflows.AggregateListAction, fn, attrName, outputVar string, entityNames map[model.ID]string) string { + out, list := "$"+outputVar, "$"+a.InputVariable + switch a.Function { + case microflows.AggregateFunctionCount: + return fmt.Sprintf("%s = count %s;", out, list) + case microflows.AggregateFunctionReduce: + initial := describeExpr(a.ReduceInitialValue) + if initial == "" { + initial = "empty" + } + return fmt.Sprintf("%s = reduce %s from %s as %s using %s;", out, list, initial, + formatMicroflowDataType(ctx, a.ReduceReturnType, entityNames), describeExpr(a.Expression)) + case microflows.AggregateFunctionAll, microflows.AggregateFunctionAny: + return fmt.Sprintf("%s = %s %s where %s;", out, fn, list, describeExpr(a.Expression)) + } + if a.UseExpression && a.Expression != "" { + return fmt.Sprintf("%s = %s %s of %s;", out, fn, list, describeExpr(a.Expression)) + } + return fmt.Sprintf("%s = %s %s by %s;", out, fn, list, mdlIdent(attrName)) +} diff --git a/mdl/executor/cmd_microflows_format_list_activity_test.go b/mdl/executor/cmd_microflows_format_list_activity_test.go new file mode 100644 index 000000000..7a76cebae --- /dev/null +++ b/mdl/executor/cmd_microflows_format_list_activity_test.go @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/langver" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// describe prints a List operation / Aggregate list activity as the statement +// that mirrors it when it describes under mdl 1, and keeps the call form while +// it describes under mdl 0 — which, while mdl 1 is a preview, is every describe +// outside an `mdl 1;` script (#733). +func TestDescribeListActivityUnderMdl1(t *testing.T) { + cases := []struct { + action microflows.MicroflowAction + mdl1 string + mdl0 string + wantKind string // the statement type the mdl 1 output parses back to + }{ + {listOp(&microflows.HeadOperation{ListVariable: "Orders"}, "First"), + "$First = head $Orders;", "$First = head($Orders);", "list"}, + {listOp(&microflows.TailOperation{ListVariable: "Orders"}, "Rest"), + "$Rest = tail $Orders;", "$Rest = tail($Orders);", "list"}, + {listOp(&microflows.FilterOperation{ListVariable: "Orders", Expression: "$currentObject/Total > 1000"}, "Big"), + "$Big = filter $Orders where $currentObject/Total > 1000;", "$Big = filter($Orders, $currentObject/Total > 1000);", "list"}, + {listOp(&microflows.FindOperation{ListVariable: "Orders", Expression: "$currentObject/Number < 3"}, "Late"), + "$Late = find $Orders where $currentObject/Number < 3;", "$Late = find($Orders, $currentObject/Number < 3);", "list"}, + {listOp(&microflows.FilterByAttributeOperation{ListVariable: "Orders", Attribute: "M.Order.Status", Expression: "M.Status.Open"}, "Open"), + "$Open = filter $Orders by Status = M.Status.Open;", "$Open = filter($Orders, Status = M.Status.Open);", "list"}, + {listOp(&microflows.FindByAttributeOperation{ListVariable: "Orders", Association: "M.Order_Customer", Expression: "$Customer"}, "Match"), + "$Match = find $Orders by Order_Customer = $Customer;", "$Match = find($Orders, Order_Customer = $Customer);", "list"}, + {listOp(&microflows.SortOperation{ListVariable: "Orders", Sorting: []*microflows.SortItem{ + {AttributeQualifiedName: "M.Order.Date", Direction: microflows.SortDirectionDescending}, + {AttributeQualifiedName: "M.Order.Number", Direction: microflows.SortDirectionAscending}, + }}, "Sorted"), + `$Sorted = sort $Orders by "Date" desc, Number asc;`, `$Sorted = sort($Orders, "Date" desc, Number asc);`, "list"}, + {listOp(&microflows.ListRangeOperation{ListVariable: "Orders", OffsetExpression: "20", LimitExpression: "10"}, "Page"), + "$Page = range $Orders offset 20 limit 10;", "$Page = range($Orders, 20, 10);", "list"}, + {listOp(&microflows.ListRangeOperation{ListVariable: "Orders", LimitExpression: "10"}, "Page"), + "$Page = range $Orders limit 10;", "$Page = range($Orders, 0, 10);", "list"}, + {listOp(&microflows.UnionOperation{ListVariable1: "A", ListVariable2: "B"}, "All"), + "$All = union $A with $B;", "$All = union($A, $B);", "list"}, + {listOp(&microflows.IntersectOperation{ListVariable1: "A", ListVariable2: "B"}, "Both"), + "$Both = intersect $A with $B;", "$Both = intersect($A, $B);", "list"}, + {listOp(&microflows.SubtractOperation{ListVariable1: "A", ListVariable2: "B"}, "Left"), + "$Left = subtract $B from $A;", "$Left = subtract($A, $B);", "list"}, + {listOp(&microflows.ContainsOperation{ListVariable: "Orders", ObjectVariable: "Order"}, "Has"), + "$Has = contains $Order in $Orders;", "$Has = contains($Orders, $Order);", "list"}, + {listOp(&microflows.EqualsOperation{ListVariable1: "A", ListVariable2: "B"}, "Same"), + "$Same = equals $A and $B;", "$Same = equals($A, $B);", "list"}, + {&microflows.AggregateListAction{InputVariable: "Orders", OutputVariable: "N", Function: microflows.AggregateFunctionCount}, + "$N = count $Orders;", "$N = count($Orders);", "aggregate"}, + {&microflows.AggregateListAction{InputVariable: "Orders", OutputVariable: "Total", Function: microflows.AggregateFunctionSum, AttributeQualifiedName: "M.Order.Amount"}, + "$Total = sum $Orders by Amount;", "$Total = sum($Orders.Amount);", "aggregate"}, + {&microflows.AggregateListAction{InputVariable: "Orders", OutputVariable: "Total", Function: microflows.AggregateFunctionAverage, UseExpression: true, Expression: "$currentObject/Price * 2"}, + "$Total = average $Orders of $currentObject/Price * 2;", "$Total = average($Orders, $currentObject/Price * 2);", "aggregate"}, + {&microflows.AggregateListAction{InputVariable: "Orders", OutputVariable: "Paid", Function: microflows.AggregateFunctionAll, UseExpression: true, Expression: "$currentObject/Paid"}, + "$Paid = all $Orders where $currentObject/Paid;", "$Paid = all($Orders, $currentObject/Paid);", "aggregate"}, + {&microflows.AggregateListAction{InputVariable: "Orders", OutputVariable: "Late", Function: microflows.AggregateFunctionAny, UseExpression: true, Expression: "$currentObject/Late"}, + "$Late = any $Orders where $currentObject/Late;", "$Late = any($Orders, $currentObject/Late);", "aggregate"}, + {&microflows.AggregateListAction{InputVariable: "Orders", OutputVariable: "Csv", Function: microflows.AggregateFunctionReduce, UseExpression: true, + Expression: "$currentResult + $currentObject/Name", ReduceInitialValue: "''", ReduceReturnType: &microflows.StringType{}}, + "$Csv = reduce $Orders from '' as String using $currentResult + $currentObject/Name;", + "$Csv = reduce($Orders, $currentResult + $currentObject/Name, initial: '', returns: String);", "aggregate"}, + } + for _, tc := range cases { + t.Run(tc.mdl1, func(t *testing.T) { + if got := formatAction(&ExecContext{LanguageVersion: langver.V1}, tc.action, nil, nil); got != tc.mdl1 { + t.Errorf("mdl 1 describe:\n got %s\n want %s", got, tc.mdl1) + } + // Control: outside an mdl 1 script describe keeps the call form. + for _, ctx := range []*ExecContext{nil, {LanguageVersion: langver.V0}} { + if got := formatAction(ctx, tc.action, nil, nil); got != tc.mdl0 { + t.Errorf("mdl 0 describe:\n got %s\n want %s", got, tc.mdl0) + } + } + + // The mdl 1 output parses back under the header, with nothing to warn about. + src := "mdl 1;\ncreate microflow M.A ($Orders: List of M.Order, $A: List of M.Order, $B: List of M.Order, " + + "$Order: M.Order, $Customer: M.Customer)\nbegin\n " + tc.mdl1 + "\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("the mdl 1 describe output does not parse: %v", errs) + } + if len(prog.Deprecations) > 0 || len(prog.LanguageNotes) > 0 { + t.Errorf("the mdl 1 describe output warns: %v %v", prog.Deprecations, prog.LanguageNotes) + } + }) + } +} + +func listOp(op microflows.ListOperation, out string) *microflows.ListOperationAction { + return &microflows.ListOperationAction{Operation: op, OutputVariable: out} +} + +// The describe language is the newest frozen version unless the describe runs +// inside a script whose header names a newer one. +func TestDescribeLanguage(t *testing.T) { + if got := describeLanguage(nil); got != langver.Frozen { + t.Errorf("describeLanguage(nil) = %v, want the frozen %v", got, langver.Frozen) + } + if got := describeLanguage(&ExecContext{LanguageVersion: langver.V1}); got != langver.V1 { + t.Errorf("under mdl 1 = %v", got) + } +} diff --git a/mdl/executor/cmd_microflows_handles.go b/mdl/executor/cmd_microflows_handles.go index 2172169fa..1c871e241 100644 --- a/mdl/executor/cmd_microflows_handles.go +++ b/mdl/executor/cmd_microflows_handles.go @@ -88,9 +88,15 @@ func printedStatement(obj microflows.MicroflowObject, body []string, r elkSource return strings.Join(parts, " ") } default: - if strings.HasSuffix(line, ";") || strings.HasSuffix(line, "{") { + if strings.HasSuffix(line, ";") { return strings.Join(parts, " ") } + if strings.HasSuffix(line, "{") { + // The `{` opens the error handler block. It is not part of + // the statement: a handle is written as an alter target, and a + // target ends where a fragment's `{` begins. + return strings.TrimSpace(strings.TrimSuffix(strings.Join(parts, " "), "{")) + } } if len(parts) >= 50 { break diff --git a/mdl/executor/cmd_microflows_handles_test.go b/mdl/executor/cmd_microflows_handles_test.go index 056cdd56e..10ae2ce01 100644 --- a/mdl/executor/cmd_microflows_handles_test.go +++ b/mdl/executor/cmd_microflows_handles_test.go @@ -308,3 +308,18 @@ func TestDescribeWithHandles_ErrorHandlerBody(t *testing.T) { t.Errorf("with handles minus the handle lines differs from plain describe:\n%s\n---\n%s", strings.Join(stripped, "\n"), strings.Join(plain, "\n")) } } + +// A handle has to be writable as an `alter microflow` target, and a target +// ends at the `{` that opens a fragment. So the `{` describe prints after an +// activity with a custom error handler is not part of its statement. +func TestPrintedStatement_ErrorHandlerBlockOpenerIsNotPartOfTheStatement(t *testing.T) { + body := []string{" commit $Order on error {", " return false;", " };"} + obj := &microflows.ActionActivity{} + got := printedStatement(obj, body, elkSourceRange{StartLine: 0, EndLine: 2}) + if got != "commit $Order on error" { + t.Errorf("printed statement %q, want %q", got, "commit $Order on error") + } + if _, err := mfmutator.ParseTarget(got); err != nil { + t.Errorf("the handle does not parse as a target: %v", err) + } +} diff --git a/mdl/executor/register_stubs.go b/mdl/executor/register_stubs.go index 130cefb93..aac52f5ba 100644 --- a/mdl/executor/register_stubs.go +++ b/mdl/executor/register_stubs.go @@ -519,6 +519,9 @@ func registerLintHandlers(r *Registry) { } func registerAlterPageHandlers(r *Registry) { + r.Register(&ast.AlterFlowStmt{}, func(ctx *ExecContext, stmt ast.Statement) error { + return execAlterFlow(ctx, stmt.(*ast.AlterFlowStmt)) + }) r.Register(&ast.AlterPageStmt{}, func(ctx *ExecContext, stmt ast.Statement) error { return execAlterPage(ctx, stmt.(*ast.AlterPageStmt)) }) diff --git a/mdl/executor/registry_test.go b/mdl/executor/registry_test.go index 9340826a3..a59e8e787 100644 --- a/mdl/executor/registry_test.go +++ b/mdl/executor/registry_test.go @@ -177,6 +177,7 @@ func allKnownStatements() []ast.Statement { &ast.AlterODataClientStmt{}, &ast.AlterODataServiceStmt{}, &ast.AlterPageStmt{}, + &ast.AlterFlowStmt{}, &ast.AlterPagesLayoutStmt{}, &ast.AlterPagesStylingStmt{}, &ast.AlterProjectSecurityStmt{}, diff --git a/mdl/executor/stmt_summary.go b/mdl/executor/stmt_summary.go index 362f91603..bcb135d79 100644 --- a/mdl/executor/stmt_summary.go +++ b/mdl/executor/stmt_summary.go @@ -198,6 +198,9 @@ func stmtSummary(stmt ast.Statement) string { case *ast.AlterStylingStmt: return fmt.Sprintf("alter styling on %s %s widget %s", s.ContainerType, s.ContainerName, s.WidgetName) + case *ast.AlterFlowStmt: + return fmt.Sprintf("alter %s %s", s.Kind(), s.Name) + // ALTER PAGE / ALTER SNIPPET case *ast.AlterPageStmt: ct := s.ContainerType diff --git a/mdl/executor/validate_deprecations.go b/mdl/executor/validate_deprecations.go index cd2723846..5464a3445 100644 --- a/mdl/executor/validate_deprecations.go +++ b/mdl/executor/validate_deprecations.go @@ -33,7 +33,7 @@ func ValidateDeprecations(prog *ast.Program) []linter.Violation { Severity: linter.SeverityWarning, Message: fmt.Sprintf("line %d: `%s`%s is deprecated; write `%s` — same meaning. "+ "Refused from `mdl %d`.", d.Line, e.Old, on, e.Canonical, e.RemovedIn), - Suggestion: fmt.Sprintf("Replace `%s` with `%s`. %s", e.Rewrite.Token, e.Rewrite.Replacement, e.Note), + Suggestion: deprecationSuggestion(e), }) } return out @@ -54,3 +54,12 @@ func ApplyDeprecationPolicy(violations []linter.Violation, policy deprecation.Po } return out } + +// deprecationSuggestion says how to rewrite a deprecated spelling: the keyword +// to swap, or for a structural rewrite, the rewrite itself. +func deprecationSuggestion(e deprecation.Entry) string { + if e.Rewrite.Structural != "" { + return fmt.Sprintf("Rewrite the %s. %s", e.Rewrite.Structural, e.Note) + } + return fmt.Sprintf("Replace `%s` with `%s`. %s", e.Rewrite.Token, e.Rewrite.Replacement, e.Note) +} diff --git a/mdl/executor/validate_microflow_listop_source.go b/mdl/executor/validate_microflow_listop_source.go index 313a68d33..f984b9ea8 100644 --- a/mdl/executor/validate_microflow_listop_source.go +++ b/mdl/executor/validate_microflow_listop_source.go @@ -39,6 +39,11 @@ import ( // The control for all of them is the reporter's own workaround, which builds // the same two activities explicitly and passes at 0 errors. // +// Retired for `mdl 1;` scripts (#733): there a list operation is one statement +// per activity whose operand is a VARIABLE, so a nested call does not parse and +// the visitor refuses the call forms that could nest (MDL-V1-LIST). This rule is +// reachable only from mdl 0 scripts, and goes with them. +// // Why refuse rather than materialise an implicit variable: the refusal covers // every spelling from one rule, including the literal operand, which no amount // of materialising would fix. And it needs no project — the answer is in the diff --git a/mdl/grammar/MDLLexer.g4 b/mdl/grammar/MDLLexer.g4 index cf037486c..027bd425e 100644 --- a/mdl/grammar/MDLLexer.g4 +++ b/mdl/grammar/MDLLexer.g4 @@ -257,6 +257,8 @@ MAXIMUM: M A X I M U M; REDUCE: R E D U C E; ANY: A N Y; INITIAL: I N I T I A L; +// reduce $L from <initial> as <type> using <expression> (#733). +USING: U S I N G; LIST: L I S T; REMOVE: R E M O V E; EQUALS_OP: E Q U A L S; diff --git a/mdl/grammar/MDLParser.g4 b/mdl/grammar/MDLParser.g4 index 472d38961..2aab0c61f 100644 --- a/mdl/grammar/MDLParser.g4 +++ b/mdl/grammar/MDLParser.g4 @@ -173,6 +173,11 @@ alterStatement // there; a scroll-container region is addressed as `layoutContainer.top`, // because a region has no Name of its own. | ALTER alterDocumentType qualifiedName LBRACE alterOperation+ RBRACE + // The generic ALTER on a microflow or nanoflow (ADR-0012 decision 3): a + // graph splice into the stored flow. Its targets are content addresses + // (`$Var`, `'Caption'`, a statement pattern) and its fragments are + // microflow statements, so it has its own operation rule. + | ALTER (MICROFLOW | NANOFLOW) qualifiedName LBRACE alterFlowOperation* RBRACE | alterPagesLayoutStatement | alterPagesStylingStatement | ALTER WORKFLOW qualifiedName alterWorkflowAction+ SEMICOLON? @@ -312,6 +317,36 @@ alterTarget | STRING_LITERAL (AT NUMBER_LITERAL)? ; +/** + * `alter microflow` / `alter nanoflow` operations (ADR-0012 decision 3, + * ako/mxcli#736): + * + * ```mdl + * alter microflow FeedbackModule.VAL_Feedback { + * insert after $IsValidEmail { log info node 'Feedback' 'Email checked'; } + * insert before 'Email is Valid?' { … } + * replace commit $Order with { commit $Order with events; } + * drop log * node 'Debug' *; + * } + * ``` + * + * A fragment is written exactly as the same statements are in `create + * microflow`. A target is a content address, resolved by mfmutator: `$Var` + * (the activity that outputs it), `'Caption'`, or a statement pattern with `*` + * wildcards, each optionally followed by `@n`. A pattern is any run of tokens, + * so the target is taken as raw text up to the `{`, `with` or `;` that ends it; + * that is why `drop` needs its semicolon. + */ +alterFlowOperation + : INSERT (AFTER | BEFORE) alterFlowTarget LBRACE microflowBody RBRACE SEMICOLON? + | REPLACE alterFlowTarget WITH LBRACE microflowBody RBRACE SEMICOLON? + | DROP alterFlowTarget SEMICOLON + ; + +alterFlowTarget + : ~(LBRACE | RBRACE | SEMICOLON | WITH)+ + ; + // ALTER PAGES [IN <module>] SET LAYOUT = Module.Layout [MAP (...)] [WHERE LAYOUT = Module.Old] // // The bulk form is the real one: an app has one layout and many pages, so diff --git a/mdl/grammar/domains/MDLMicroflow.g4 b/mdl/grammar/domains/MDLMicroflow.g4 index f96b8fd3d..427c59914 100644 --- a/mdl/grammar/domains/MDLMicroflow.g4 +++ b/mdl/grammar/domains/MDLMicroflow.g4 @@ -878,10 +878,46 @@ transformJsonStatement // ============================================================================= /** - * List operations that return a single item or a modified list. + * List operations that return a single item or a modified list: one statement + * per Studio Pro "List operation" activity (PROPOSAL_mdl_beta_syntax_freeze.md + * §4, #733). The operand is always a variable, as it is in the activity's + * dialog, so one activity cannot be nested inside another. */ listOperationStatement - : VARIABLE EQUALS listOperation + : VARIABLE EQUALS listOperationActivity + // The call form. A respelling for every operation but find and contains, + // whose call form is also the string function: the visitor version-gates + // those instead (MDL-V1-LIST), since no rewrite can know which was meant. + | VARIABLE EQUALS listOperation /* @alias MDL-DEPR003 */ + ; + +listOperationActivity + : HEAD VARIABLE // $x = head $L + | TAIL VARIABLE // $x = tail $L + | FIND VARIABLE listOperationCondition // $x = find $L by Number = 3 + | FILTER VARIABLE listOperationCondition // $x = filter $L where $currentObject/Paid + | SORT VARIABLE BY listSortItem (COMMA listSortItem)* // $x = sort $L by Date desc, Number + | UNION VARIABLE WITH VARIABLE // $x = union $A with $B + | INTERSECT VARIABLE WITH VARIABLE // $x = intersect $A with $B + | SUBTRACT VARIABLE FROM VARIABLE // $x = subtract $B from $A (A minus B) + | CONTAINS VARIABLE IN VARIABLE // $b = contains $Object in $L + | EQUALS_OP VARIABLE AND VARIABLE // $b = equals $A and $B + | RANGE VARIABLE (OFFSET expression)? (LIMIT expression)? // $x = range $L offset 20 limit 10 + ; + +// `by` picks a member (Studio Pro's Find / Filter: an attribute or association +// and the value it must have), written `Member = value`; the visitor refuses +// any other shape. `where` takes an expression over $currentObject (Find by +// expression / Filter by expression). +listOperationCondition + : BY expression + | WHERE expression + ; + +// A sort attribute may be any word, so an attribute called Count or Date needs +// no quotes here. +listSortItem + : identifierOrKeyword (ASC | DESC)? ; listOperation @@ -910,7 +946,19 @@ sortSpec * Aggregate operations on lists. */ aggregateListStatement - : VARIABLE EQUALS listAggregateOperation + : VARIABLE EQUALS aggregateListActivity + | VARIABLE EQUALS listAggregateOperation /* @alias MDL-DEPR004 */ + ; + +/** + * One Studio Pro "Aggregate list" activity. `by` aggregates an attribute and + * `of` an expression (the dialog's "Aggregate with: Attribute / Expression"). + */ +aggregateListActivity + : COUNT VARIABLE // $n = count $L + | (SUM | AVERAGE | MINIMUM | MAXIMUM) VARIABLE (BY identifierOrKeyword | OF expression) // $t = sum $L by Amount + | (ALL | ANY) VARIABLE WHERE expression // $b = all $L where $currentObject/Paid + | REDUCE VARIABLE FROM expression AS dataType USING expression // $s = reduce $L from '' as String using … ; listAggregateOperation diff --git a/mdl/grammar/domains/MDLSettings.g4 b/mdl/grammar/domains/MDLSettings.g4 index 87da00016..e09bfa3c5 100644 --- a/mdl/grammar/domains/MDLSettings.g4 +++ b/mdl/grammar/domains/MDLSettings.g4 @@ -643,7 +643,7 @@ keyword | COUNT | SUM | AVG | MIN | MAX | DISTINCT | ALL | ASC | DESC | UNION | INTERSECT | SUBTRACT | EXISTS | CAST | COALESCE | TRIM | LENGTH | CONTAINS | MATCH - | AVERAGE | MINIMUM | MAXIMUM | REDUCE | ANY | INITIAL + | AVERAGE | MINIMUM | MAXIMUM | REDUCE | ANY | INITIAL | USING | IS_NULL | IS_NOT_NULL | NOT_NULL | HEAD | TAIL | FIND | SORT | EMPTY | LIST_OF | LIST_KW | EQUALS_OP diff --git a/mdl/roundtrip/list_activity_test.go b/mdl/roundtrip/list_activity_test.go new file mode 100644 index 000000000..cb3b55fc2 --- /dev/null +++ b/mdl/roundtrip/list_activity_test.go @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build integration + +package roundtrip + +import ( + "regexp" + "strings" + "testing" +) + +// listActivityRows is one statement per row of the §4 Microflows table of +// PROPOSAL_mdl_beta_syntax_freeze.md, written against PedApp's +// FeedbackModule.Feedback. Each is also the exact line describe must print +// back under mdl 1 (#733). +var listActivityRows = []string{ + "$Open = filter $Feedbacks by Subject = 'Open';", + "$Wide = filter $Feedbacks where $currentObject/ScreenWidth > 1000;", + "$Match = find $Feedbacks by ScreenWidth = 3;", + "$Tall = find $Feedbacks where $currentObject/ScreenHeight > 2;", + "$Sorted = sort $Feedbacks by Subject desc, ScreenWidth asc;", + "$First = head $Feedbacks;", + "$Rest = tail $Feedbacks;", + "$Page = range $Feedbacks offset 20 limit 10;", + "$All = union $Feedbacks with $Others;", + "$Both = intersect $Feedbacks with $Others;", + "$Left = subtract $Others from $Feedbacks;", + "$Has = contains $One in $Feedbacks;", + "$Same = equals $Feedbacks and $Others;", + "$N = count $Feedbacks;", + "$Total = sum $Feedbacks by ScreenWidth;", + "$Double = sum $Feedbacks of $currentObject/ScreenWidth * 2;", + "$Avg = average $Feedbacks by ScreenWidth;", + "$Min = minimum $Feedbacks by ScreenHeight;", + "$Max = maximum $Feedbacks by ScreenHeight;", + "$AllShown = all $Feedbacks where $currentObject/_showEmail;", + "$AnyShown = any $Feedbacks where $currentObject/_showEmail;", + "$Csv = reduce $Feedbacks from '' as String using $currentResult + $currentObject/Subject;", +} + +const listActivityTarget = "microflow MyFirstModule.ListActivities" + +func listActivityScript() string { + return "mdl 1;\ncreate microflow MyFirstModule.ListActivities (\n" + + " $Feedbacks: List of FeedbackModule.Feedback,\n" + + " $Others: List of FeedbackModule.Feedback,\n" + + " $One: FeedbackModule.Feedback\n)\nreturns Boolean\nbegin\n " + + strings.Join(listActivityRows, "\n ") + "\n return $Has;\nend;\n" +} + +var positionLine = regexp.MustCompile(`(?m)^\s*@(position|anchor|curve|merge)\b.*\n`) + +// Every row of the §4 table parses, executes on the Studio Pro-authored +// fixture, and reads back through describe under the header as the same +// statement; the described microflow executes back to the same description. +func TestPedAppListActivitiesUnderMdl1(t *testing.T) { + h := newHarness(t) + defer h.close() + + if err := h.exec(listActivityScript()); err != nil { + t.Fatalf("exec under mdl 1: %v\n%s", err, h.out.String()) + } + first := h.describeUnder("mdl 1;", listActivityTarget) + body := positionLine.ReplaceAllString(first, "") + for _, row := range listActivityRows { + if !strings.Contains(body, " "+row+"\n") { + t.Errorf("describe under mdl 1 does not print %q:\n%s", row, body) + } + } + + // PutGet: executing the mdl 1 description describes back to itself. + if err := h.exec("mdl 1;\n" + first); err != nil { + t.Fatalf("exec the mdl 1 description: %v\n%s", err, first) + } + if again := h.describeUnder("mdl 1;", listActivityTarget); again != first { + t.Errorf("describe -> exec -> describe changed the microflow:\n%s", lineDiff(first, again)) + } + + // Control: outside an mdl 1 script describe keeps the call form while mdl 1 + // is a preview, so the rows above are not what a plain describe prints. + plain, err := h.describe(listActivityTarget) + if err != nil { + t.Fatal(err) + } + if strings.Contains(plain, "= head $Feedbacks;") || !strings.Contains(plain, "= head($Feedbacks);") { + t.Errorf("a plain describe must keep the call form while mdl 1 is a preview:\n%s", plain) + } +} + +// describeUnder runs describe inside a script with the given header. +func (h *harness) describeUnder(header, target string) string { + h.t.Helper() + if err := h.exec(header + "\ndescribe " + target + ";"); err != nil { + h.t.Fatalf("describe %s under %q: %v", target, header, err) + } + return h.out.String() +} diff --git a/mdl/visitor/visitor_alter.go b/mdl/visitor/visitor_alter.go index c878412ef..859174a7f 100644 --- a/mdl/visitor/visitor_alter.go +++ b/mdl/visitor/visitor_alter.go @@ -17,6 +17,10 @@ func (b *Builder) ExitAlterStatement(ctx *parser.AlterStatementContext) { b.exitAlterDocumentStatement(ctx) return } + if ctx.MICROFLOW() != nil || ctx.NANOFLOW() != nil { + b.exitAlterFlowStatement(ctx) + return + } // Handle ALTER PAGES … SET LAYOUT (the bulk repoint) if sub := ctx.AlterPagesStylingStatement(); sub != nil { diff --git a/mdl/visitor/visitor_alter_flow.go b/mdl/visitor/visitor_alter_flow.go new file mode 100644 index 000000000..784848fda --- /dev/null +++ b/mdl/visitor/visitor_alter_flow.go @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/grammar/parser" +) + +// exitAlterFlowStatement builds an AlterFlowStmt from +// ALTER MICROFLOW|NANOFLOW Module.Name { insert / replace / drop }. +func (b *Builder) exitAlterFlowStatement(ctx *parser.AlterStatementContext) { + stmt := &ast.AlterFlowStmt{Nanoflow: ctx.NANOFLOW() != nil} + if qn := ctx.QualifiedName(); qn != nil { + stmt.Name = buildQualifiedName(qn) + } + for _, opCtx := range ctx.AllAlterFlowOperation() { + op := opCtx.(*parser.AlterFlowOperationContext) + o := &ast.AlterFlowOperation{Target: alterFlowTargetText(op.AlterFlowTarget())} + switch { + case op.INSERT() != nil && op.BEFORE() != nil: + o.Op = ast.AlterFlowInsertBefore + case op.INSERT() != nil: + o.Op = ast.AlterFlowInsertAfter + case op.REPLACE() != nil: + o.Op = ast.AlterFlowReplace + default: + o.Op = ast.AlterFlowDrop + } + if body := op.MicroflowBody(); body != nil { + o.Body = buildMicroflowBody(body) + } + stmt.Operations = append(stmt.Operations, o) + } + b.statements = append(b.statements, stmt) +} + +// alterFlowTargetText returns a target as the author wrote it — whitespace and +// quoting included — since a statement pattern is matched token by token +// against describe's rendering, and the grammar only knows it as a run of +// arbitrary tokens. +func alterFlowTargetText(ctx parser.IAlterFlowTargetContext) string { + if ctx == nil { + return "" + } + start, stop := ctx.GetStart(), ctx.GetStop() + if start == nil || stop == nil { + return strings.TrimSpace(ctx.GetText()) + } + return strings.TrimSpace(start.GetInputStream().GetText(start.GetStart(), stop.GetStop())) +} diff --git a/mdl/visitor/visitor_alter_flow_test.go b/mdl/visitor/visitor_alter_flow_test.go new file mode 100644 index 000000000..2391ffc99 --- /dev/null +++ b/mdl/visitor/visitor_alter_flow_test.go @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// alter microflow / nanoflow (ADR-0012 decision 3, ako/mxcli#736): targets are +// content addresses kept as written, fragments are microflow statements. +func TestAlterFlow_OperationsAndTargets(t *testing.T) { + prog, errs := Build(`alter microflow FeedbackModule.VAL_Feedback { + insert after $IsValidEmail { log info node 'Feedback' 'Email checked'; } + insert before 'Email is Valid?' @2 { declare $n Integer = 1; set $n = 2; } + replace log * node 'Debug' * with { log warning node 'Debug' 'x'; } + drop set $ValidFeedback = false @3; + };`) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + stmt, ok := prog.Statements[0].(*ast.AlterFlowStmt) + if !ok { + t.Fatalf("want *ast.AlterFlowStmt, got %T", prog.Statements[0]) + } + if stmt.Nanoflow || stmt.Name.String() != "FeedbackModule.VAL_Feedback" { + t.Errorf("header: nanoflow=%v name=%s", stmt.Nanoflow, stmt.Name) + } + want := []struct { + op ast.AlterFlowOpKind + target string + body int + }{ + {ast.AlterFlowInsertAfter, "$IsValidEmail", 1}, + {ast.AlterFlowInsertBefore, "'Email is Valid?' @2", 2}, + {ast.AlterFlowReplace, "log * node 'Debug' *", 1}, + {ast.AlterFlowDrop, "set $ValidFeedback = false @3", 0}, + } + if len(stmt.Operations) != len(want) { + t.Fatalf("want %d operations, got %d", len(want), len(stmt.Operations)) + } + for i, w := range want { + got := stmt.Operations[i] + if got.Op != w.op || got.Target != w.target || len(got.Body) != w.body { + t.Errorf("op %d: got %q %q body=%d, want %q %q body=%d", i, got.Op, got.Target, len(got.Body), w.op, w.target, w.body) + } + } +} + +func TestAlterFlow_Nanoflow(t *testing.T) { + prog, errs := Build(`alter nanoflow M.NF { drop $X; }`) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + stmt := prog.Statements[0].(*ast.AlterFlowStmt) + if !stmt.Nanoflow || stmt.Operations[0].Op != ast.AlterFlowDrop || stmt.Operations[0].Target != "$X" { + t.Errorf("got %+v %+v", stmt, stmt.Operations[0]) + } +} diff --git a/mdl/visitor/visitor_deprecations_test.go b/mdl/visitor/visitor_deprecations_test.go index 6bb425c07..300b8850d 100644 --- a/mdl/visitor/visitor_deprecations_test.go +++ b/mdl/visitor/visitor_deprecations_test.go @@ -50,6 +50,10 @@ func TestRegistryExamplesRecordTheirCode(t *testing.T) { t.Errorf("Example and CanonicalExample build different statements:\n old: %#v\n canon: %#v", old.Statements, canon.Statements) } + // A structural rewrite is proven by the AST comparison above alone. + if e.Rewrite.Structural != "" { + return + } // The rewrite is a token swap; the canonical example must be exactly it. re := regexp.MustCompile(`(?i)\b` + regexp.QuoteMeta(e.Rewrite.Token) + `\b`) if got := re.ReplaceAllString(e.Example, e.Rewrite.Replacement); got != e.CanonicalExample { diff --git a/mdl/visitor/visitor_list_activities.go b/mdl/visitor/visitor_list_activities.go new file mode 100644 index 000000000..06c06bfa5 --- /dev/null +++ b/mdl/visitor/visitor_list_activities.go @@ -0,0 +1,330 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "fmt" + "strconv" + "strings" + + "github.com/antlr4-go/antlr/v4" + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/deprecation" + "github.com/mendixlabs/mxcli/mdl/grammar/parser" + "github.com/mendixlabs/mxcli/mdl/langver" +) + +// List operations and aggregates are statements that mirror Studio Pro's +// activities: one "List operation" or "Aggregate list" activity per statement, +// named after the operation and taking the inputs its dialog asks for +// (PROPOSAL_mdl_beta_syntax_freeze.md §4 and §5 item 3, #733): +// +// $Open = filter $Orders by Status = Shop.Status.Open; +// $N = count $Open; +// +// The operand is always a variable, so one activity cannot be nested inside +// another. The call form (`filter($Orders, …)`) keeps parsing: +// +// - for every operation except find and contains it is a respelling, a +// deprecated alias (MDL-DEPR003 / MDL-DEPR004) that builds the same AST; +// - `find(…)` and `contains(…)` are also Mendix's string functions, and which +// one a call means is guessed today from its arguments. That is not a +// respelling, so it is version-gated (listCallForm): kept and warned under +// mdl 0, refused under mdl 1; +// - so is a call after `set`, or nested in another call: under mdl 0 it is +// turned into an activity (and a nested operand is refused by MDL-LISTOP02), +// under mdl 1 `set` always assigns an expression and such a call is refused. +// +// `set` is mandatory for reassignment under mdl 1 (setIsMandatory), which is +// what removes the ambiguity: `$x = …` without it is only ever an activity. + +// listCallForm is the change of meaning for list operations and aggregates +// written as calls where the call is not a plain respelling of one activity. +var listCallForm = langver.Change{ + Code: "MDL-V1-LIST", + Since: langver.V1, + Old: "a list operation or aggregate written as a call — `find(…)` or `contains(…)`, " + + "a call after `set`, or one call nested in another — is turned into a List operation " + + "or Aggregate list activity, guessing from its arguments whether a string function was meant,", + New: "an error: a List operation or Aggregate list is one statement per activity whose operand is " + + "a variable (`$x = find $L where …;`, `$n = count $L;`), and after `set` a call is always " + + "the expression function", +} + +// setIsMandatory is the new rejection of a reassignment without `set`. +var setIsMandatory = langver.Change{ + Code: "MDL-V1-SET", + Since: langver.V1, + Old: "`$x = <expression>` without `set` changes the variable", + New: "an error: a reassignment is written `set $x = <expression>;`", +} + +// scriptLanguageVersion reads the `mdl <n>;` header of the script that tree is +// part of. The header can only be the first statement, so it is on the root. +// +// It exists for the statement builders, which are free functions without the +// Builder: they must build the AST of the version the script is written in. +// A missing or malformed header is mdl 0; a malformed one is reported by +// ExitLanguageHeader. +func scriptLanguageVersion(tree antlr.Tree) langver.Version { + for tree != nil { + if prog, ok := tree.(*parser.ProgramContext); ok { + h, ok := prog.LanguageHeader().(*parser.LanguageHeaderContext) + if !ok || h == nil || h.IDENTIFIER() == nil || h.NUMBER_LITERAL() == nil || + !strings.EqualFold(h.IDENTIFIER().GetText(), "mdl") { + return langver.V0 + } + n, err := strconv.Atoi(h.NUMBER_LITERAL().GetText()) + if v := langver.Version(n); err == nil && v.Known() { + return v + } + return langver.V0 + } + tree = tree.GetParent() + } + return langver.V0 +} + +// buildListOperationActivity fills stmt from the statement form. +func buildListOperationActivity(act *parser.ListOperationActivityContext, stmt *ast.ListOperationStmt) { + vars := act.AllVARIABLE() + varName := func(i int) string { + if i < len(vars) { + return strings.TrimPrefix(vars[i].GetText(), "$") + } + return "" + } + stmt.InputVariable = varName(0) + condition := func() { + c, ok := act.ListOperationCondition().(*parser.ListOperationConditionContext) + if !ok || c == nil { + return + } + stmt.Condition = buildSourceExpression(c.Expression()) + stmt.ByExpression = c.WHERE() != nil + } + switch { + case act.HEAD() != nil: + stmt.Operation = ast.ListOpHead + case act.TAIL() != nil: + stmt.Operation = ast.ListOpTail + case act.FIND() != nil: + stmt.Operation = ast.ListOpFind + condition() + case act.FILTER() != nil: + stmt.Operation = ast.ListOpFilter + condition() + case act.SORT() != nil: + stmt.Operation = ast.ListOpSort + for _, item := range act.AllListSortItem() { + it := item.(*parser.ListSortItemContext) + stmt.SortSpecs = append(stmt.SortSpecs, ast.SortSpec{ + Attribute: identifierOrKeywordText(it.IdentifierOrKeyword()), + Ascending: it.DESC() == nil, + }) + } + case act.UNION() != nil: + stmt.Operation, stmt.SecondVariable = ast.ListOpUnion, varName(1) + case act.INTERSECT() != nil: + stmt.Operation, stmt.SecondVariable = ast.ListOpIntersect, varName(1) + case act.SUBTRACT() != nil: + // subtract $B from $A: the result is A minus B, and A is the first list. + stmt.Operation, stmt.InputVariable, stmt.SecondVariable = ast.ListOpSubtract, varName(1), varName(0) + case act.CONTAINS() != nil: + // contains $Object in $List: the list is the activity's first operand. + stmt.Operation, stmt.InputVariable, stmt.SecondVariable = ast.ListOpContains, varName(1), varName(0) + case act.EQUALS_OP() != nil: + stmt.Operation, stmt.SecondVariable = ast.ListOpEquals, varName(1) + case act.RANGE() != nil: + stmt.Operation = ast.ListOpRange + if act.OFFSET() != nil { + stmt.OffsetExpr = buildSourceExpression(act.Expression(0)) + } + if act.LIMIT() != nil { + stmt.LimitExpr = buildSourceExpression(act.Expression(len(act.AllExpression()) - 1)) + } + } +} + +// buildAggregateListActivity fills stmt from the statement form. +func buildAggregateListActivity(act *parser.AggregateListActivityContext, stmt *ast.AggregateListStmt) { + if v := act.VARIABLE(); v != nil { + stmt.InputVariable = strings.TrimPrefix(v.GetText(), "$") + } + exprs := act.AllExpression() + switch { + case act.COUNT() != nil: + stmt.Operation = ast.AggregateCount + return + case act.SUM() != nil: + stmt.Operation = ast.AggregateSum + case act.AVERAGE() != nil: + stmt.Operation = ast.AggregateAverage + case act.MINIMUM() != nil: + stmt.Operation = ast.AggregateMinimum + case act.MAXIMUM() != nil: + stmt.Operation = ast.AggregateMaximum + case act.ALL() != nil, act.ANY() != nil: + stmt.Operation = ast.AggregateAll + if act.ANY() != nil { + stmt.Operation = ast.AggregateAny + } + stmt.IsExpression = true + if len(exprs) > 0 { + stmt.Expression = buildSourceExpression(exprs[0]) + } + return + case act.REDUCE() != nil: + // reduce $L from <initial> as <type> using <expression> + stmt.Operation = ast.AggregateReduce + stmt.IsExpression = true + if len(exprs) > 0 { + stmt.InitialValue = buildSourceExpression(exprs[0]) + } + if len(exprs) > 1 { + stmt.Expression = buildSourceExpression(exprs[1]) + } + if dt := act.DataType(); dt != nil { + t := buildDataType(dt) + stmt.ReturnType = &t + } + return + } + // sum / average / minimum / maximum: `by Attribute` or `of <expression>`. + if act.OF() != nil && len(exprs) > 0 { + stmt.IsExpression = true + stmt.Expression = buildSourceExpression(exprs[0]) + } else if id := act.IdentifierOrKeyword(); id != nil { + stmt.Attribute = identifierOrKeywordText(id) + } +} + +// unwrapSource returns the expression a SourceExpr carries. +func unwrapSource(e ast.Expression) ast.Expression { + if s, ok := e.(*ast.SourceExpr); ok { + return s.Expression + } + return e +} + +// listCallName returns the upper-cased name of a list-operation or aggregate +// call, or "" when value is not one. +func listCallName(value ast.Expression) string { + call, ok := unwrapSource(value).(*ast.FunctionCallExpr) + if !ok { + return "" + } + switch name := strings.ToUpper(call.Name); name { + case "HEAD", "TAIL", "FIND", "FILTER", "SORT", "UNION", "INTERSECT", "SUBTRACT", + "CONTAINS", "EQUALS", "RANGE", "COUNT", "SUM", "AVERAGE", "MINIMUM", "MAXIMUM": + return name + } + return "" +} + +func isStringOverload(name string) bool { return name == "FIND" || name == "CONTAINS" } + +// ExitListOperationStatement reports the call form: a deprecated alias, or for +// find/contains a version-gated construct; and checks that `by` names a member. +func (b *Builder) ExitListOperationStatement(ctx *parser.ListOperationStatementContext) { + target := "" + if v := ctx.VARIABLE(); v != nil { + target = v.GetText() + } + if op, ok := ctx.ListOperation().(*parser.ListOperationContext); ok && op != nil { + if op.FIND() != nil || op.CONTAINS() != nil { + if b.gate(listCallForm, ctx) { + b.addError(findContainsCallError(ctx.GetStart().GetLine(), target, op.FIND() != nil)) + } + return + } + b.recordDeprecation(deprecation.ListOperationFunctionForm, op.GetStart(), strings.ToLower(op.GetStart().GetText())) + return + } + act, ok := ctx.ListOperationActivity().(*parser.ListOperationActivityContext) + if !ok || act == nil { + return + } + c, ok := act.ListOperationCondition().(*parser.ListOperationConditionContext) + if !ok || c == nil || c.BY() == nil { + return + } + if !ast.IsMemberEquality(unwrapSource(buildExpression(c.Expression()))) { + b.addError(fmt.Errorf("line %d: `by` names a member and the value it must have, `by Member = value`; "+ + "`%s` is not of that shape. For any other condition write `where <expression>`, which is "+ + "Studio Pro's \"by expression\" operation and uses $currentObject", + c.GetStart().GetLine(), strings.TrimSpace(extractExpressionText(c.Expression())))) + } +} + +func findContainsCallError(line int, target string, find bool) error { + if find { + return fmt.Errorf("line %d: `%s = find(…)` is refused under mdl 1: the call is both the string function "+ + "and the List operation, one statement per activity. For the string function write "+ + "`set %s = find(…);` (the variable is declared first); for the List operation write "+ + "`%s = find $List by Member = value;` or `%s = find $List where <expression>;`", + line, target, target, target, target) + } + return fmt.Errorf("line %d: `%s = contains(…)` is refused under mdl 1: the call is both the string function "+ + "and the List operation, one statement per activity. For the string function write "+ + "`set %s = contains(…);` (the variable is declared first); for the List operation write "+ + "`%s = contains $Object in $List;`", line, target, target, target) +} + +// ExitAggregateListStatement reports the call form, a deprecated alias. +func (b *Builder) ExitAggregateListStatement(ctx *parser.AggregateListStatementContext) { + if op, ok := ctx.ListAggregateOperation().(*parser.ListAggregateOperationContext); ok && op != nil { + b.recordDeprecation(deprecation.AggregateFunctionForm, op.GetStart(), strings.ToLower(op.GetStart().GetText())) + } +} + +// ExitSetStatement gates the two constructs whose meaning mdl 1 changes: an +// assignment without `set`, and a list-operation or aggregate call as the value. +func (b *Builder) ExitSetStatement(ctx *parser.SetStatementContext) { + v := ctx.VARIABLE() + if v == nil || ctx.Expression() == nil { + // An attribute target (`$o/A = v`) is a respelling of `change`, not a + // reassignment; it is left to that alias. + return + } + target := v.GetText() + line := ctx.GetStart().GetLine() + value := buildExpression(ctx.Expression()) + name := listCallName(value) + valueText := strings.TrimSpace(extractExpressionText(ctx.Expression())) + + switch { + case name != "" && !(isStringOverload(name) && ctx.SET() != nil): + // A call the mdl 0 builder turns into an activity (when it does). + if call, ok := unwrapSource(value).(*ast.FunctionCallExpr); ok && + buildListOrAggregateStatement(strings.TrimPrefix(target, "$"), call) == nil { + // The string reading of find/contains: an expression either way. + break + } + if b.gate(listCallForm, ctx) { + if isStringOverload(name) { + b.addError(findContainsCallError(line, target, name == "FIND")) + return + } + b.addError(fmt.Errorf("line %d: `%s = %s` is refused under mdl 1: a list operation or aggregate is "+ + "one statement per activity, and its operand is a variable, never another call or an "+ + "expression. Write `%s = %s $List …;`, with each inner call as a statement of its own "+ + "(e.g. `$Open = filter $Orders where …;` then `$N = count $Open;`)", + line, target, valueText, target, strings.ToLower(name))) + } + return + case name != "" && ctx.SET() != nil: + // `set $x = find(…)` / `contains(…)`: under mdl 1 always the string + // function; under mdl 0 an activity when the arguments look like one. + if call, ok := unwrapSource(value).(*ast.FunctionCallExpr); ok && + buildListOrAggregateStatement(strings.TrimPrefix(target, "$"), call) != nil { + b.gate(listCallForm, ctx) + } + return + } + if ctx.SET() == nil && b.gate(setIsMandatory, ctx) { + b.addError(fmt.Errorf("line %d: a reassignment says `set` under mdl 1: write `set %s = %s;`. "+ + "A List operation or Aggregate list activity is written without it, as its own statement "+ + "(`%s = filter $List where …;`)", line, target, valueText, target)) + } +} diff --git a/mdl/visitor/visitor_list_activity_test.go b/mdl/visitor/visitor_list_activity_test.go new file mode 100644 index 000000000..2d1ade9f9 --- /dev/null +++ b/mdl/visitor/visitor_list_activity_test.go @@ -0,0 +1,359 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "reflect" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/deprecation" +) + +// List operations and aggregates are statements that mirror Studio Pro's +// activities (PROPOSAL_mdl_beta_syntax_freeze.md §4, ADR-0010 R2; #733). + +const listActivityParams = "$Orders: List of M.Order, $A: List of M.Order, $B: List of M.Order, " + + "$Order: M.Order, $Number: Integer, $S: String" + +// listActivityMicroflow wraps body in a microflow whose parameters every case +// below can name. +func listActivityMicroflow(header, body string) string { + return header + "create microflow M.F (" + listActivityParams + ")\nbegin\n " + body + "\nend;" +} + +// buildListActivity parses one statement inside a microflow and returns the +// program and the microflow's first body statement. +func buildListActivity(t *testing.T, header, body string) (*ast.Program, ast.MicroflowStatement, []error) { + t.Helper() + prog, errs := Build(listActivityMicroflow(header, body)) + if prog == nil || len(prog.Statements) == 0 { + return prog, nil, errs + } + mf, ok := prog.Statements[0].(*ast.CreateMicroflowStmt) + if !ok || len(mf.Body) == 0 { + return prog, nil, errs + } + return prog, mf.Body[0], errs +} + +func noteCodes(prog *ast.Program) []string { + var out []string + for _, n := range prog.LanguageNotes { + out = append(out, n.Code) + } + return out +} + +// Every row of the §4 table parses under both versions and builds the activity +// it names, with no deprecation and no language note. +func TestListActivityStatementsBuild(t *testing.T) { + cases := []struct { + src string + check func(t *testing.T, s ast.MicroflowStatement) + }{ + {"$Open = filter $Orders by Status = M.Status.Open;", func(t *testing.T, s ast.MicroflowStatement) { + lo := s.(*ast.ListOperationStmt) + if lo.Operation != ast.ListOpFilter || lo.InputVariable != "Orders" || lo.ByExpression || !ast.IsMemberEquality(lo.Condition) { + t.Errorf("got %#v", lo) + } + }}, + {"$Big = filter $Orders where $currentObject/Total > 1000;", func(t *testing.T, s ast.MicroflowStatement) { + lo := s.(*ast.ListOperationStmt) + if lo.Operation != ast.ListOpFilter || !lo.ByExpression || lo.Condition == nil { + t.Errorf("got %#v", lo) + } + }}, + {"$Match = find $Orders by Number = $Number;", func(t *testing.T, s ast.MicroflowStatement) { + lo := s.(*ast.ListOperationStmt) + if lo.Operation != ast.ListOpFind || lo.ByExpression || lo.OutputVariable != "Match" { + t.Errorf("got %#v", lo) + } + }}, + {"$Late = find $Orders where $currentObject/Number < 3;", func(t *testing.T, s ast.MicroflowStatement) { + lo := s.(*ast.ListOperationStmt) + if lo.Operation != ast.ListOpFind || !lo.ByExpression { + t.Errorf("got %#v", lo) + } + }}, + {"$Sorted = sort $Orders by Date desc, Number asc, Count;", func(t *testing.T, s ast.MicroflowStatement) { + lo := s.(*ast.ListOperationStmt) + want := []ast.SortSpec{{Attribute: "Date"}, {Attribute: "Number", Ascending: true}, {Attribute: "Count", Ascending: true}} + if lo.Operation != ast.ListOpSort || !reflect.DeepEqual(lo.SortSpecs, want) { + t.Errorf("got %#v", lo) + } + }}, + {"$First = head $Orders;", func(t *testing.T, s ast.MicroflowStatement) { + lo := s.(*ast.ListOperationStmt) + if lo.Operation != ast.ListOpHead || lo.InputVariable != "Orders" { + t.Errorf("got %#v", lo) + } + }}, + {"$Rest = tail $Orders;", func(t *testing.T, s ast.MicroflowStatement) { + if lo := s.(*ast.ListOperationStmt); lo.Operation != ast.ListOpTail { + t.Errorf("got %#v", lo) + } + }}, + {"$Page = range $Orders offset 20 limit 10;", func(t *testing.T, s ast.MicroflowStatement) { + lo := s.(*ast.ListOperationStmt) + if lo.Operation != ast.ListOpRange || lo.OffsetExpr == nil || lo.LimitExpr == nil { + t.Errorf("got %#v", lo) + } + }}, + {"$All = union $A with $B;", listPair(ast.ListOpUnion, "A", "B")}, + {"$Both = intersect $A with $B;", listPair(ast.ListOpIntersect, "A", "B")}, + // subtract $B from $A is A minus B: the first list is A. + {"$Left = subtract $B from $A;", listPair(ast.ListOpSubtract, "A", "B")}, + // contains $Order in $Orders: the list is the first operand. + {"$Has = contains $Order in $Orders;", listPair(ast.ListOpContains, "Orders", "Order")}, + {"$Same = equals $A and $B;", listPair(ast.ListOpEquals, "A", "B")}, + {"$N = count $Orders;", func(t *testing.T, s ast.MicroflowStatement) { + ag := s.(*ast.AggregateListStmt) + if ag.Operation != ast.AggregateCount || ag.InputVariable != "Orders" { + t.Errorf("got %#v", ag) + } + }}, + {"$Total = sum $Orders by Amount;", func(t *testing.T, s ast.MicroflowStatement) { + ag := s.(*ast.AggregateListStmt) + if ag.Operation != ast.AggregateSum || ag.Attribute != "Amount" || ag.IsExpression || ag.InputVariable != "Orders" { + t.Errorf("got %#v", ag) + } + }}, + {"$Total = sum $Orders of $currentObject/Price * $currentObject/Quantity;", func(t *testing.T, s ast.MicroflowStatement) { + ag := s.(*ast.AggregateListStmt) + if ag.Operation != ast.AggregateSum || !ag.IsExpression || ag.Expression == nil { + t.Errorf("got %#v", ag) + } + }}, + {"$Avg = average $Orders by Amount;", aggregateOp(ast.AggregateAverage)}, + {"$Min = minimum $Orders by Amount;", aggregateOp(ast.AggregateMinimum)}, + {"$Max = maximum $Orders of $currentObject/Amount;", aggregateOp(ast.AggregateMaximum)}, + {"$AllPaid = all $Orders where $currentObject/Paid;", aggregateOp(ast.AggregateAll)}, + {"$AnyLate = any $Orders where $currentObject/Late;", aggregateOp(ast.AggregateAny)}, + {"$Csv = reduce $Orders from '' as String using $currentResult + $currentObject/Name;", func(t *testing.T, s ast.MicroflowStatement) { + ag := s.(*ast.AggregateListStmt) + if ag.Operation != ast.AggregateReduce || ag.InitialValue == nil || ag.ReturnType == nil || + ag.ReturnType.Kind != ast.TypeString || ag.Expression == nil { + t.Errorf("got %#v", ag) + } + }}, + } + for _, header := range []string{"", "mdl 1;\n"} { + for _, tc := range cases { + t.Run(strings.TrimSpace(header+" "+tc.src), func(t *testing.T) { + prog, stmt, errs := buildListActivity(t, header, tc.src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + if stmt == nil { + t.Fatal("no statement built") + } + tc.check(t, stmt) + if d := deprecationCodes(prog); len(d) != 0 { + t.Errorf("canonical form recorded deprecations %v", d) + } + if n := noteCodes(prog); len(n) != 0 { + t.Errorf("canonical form recorded language notes %v", n) + } + }) + } + } +} + +func listPair(op ast.ListOperationType, first, second string) func(*testing.T, ast.MicroflowStatement) { + return func(t *testing.T, s ast.MicroflowStatement) { + lo := s.(*ast.ListOperationStmt) + if lo.Operation != op || lo.InputVariable != first || lo.SecondVariable != second { + t.Errorf("got %#v, want %v(%s, %s)", lo, op, first, second) + } + } +} + +func aggregateOp(op ast.AggregateListOperationType) func(*testing.T, ast.MicroflowStatement) { + return func(t *testing.T, s ast.MicroflowStatement) { + ag := s.(*ast.AggregateListStmt) + if ag.Operation != op || ag.InputVariable != "Orders" { + t.Errorf("got %#v", ag) + } + } +} + +// The function form is a deprecated respelling of the statement: both build the +// same AST, and only the function form records the registry code. find and +// contains are not here — they clash with the string functions (see +// TestFindContainsFunctionFormIsVersionGated). +func TestListFunctionFormIsAnAlias(t *testing.T) { + cases := []struct{ old, canon, code string }{ + {"$H = head($Orders);", "$H = head $Orders;", deprecation.ListOperationFunctionForm}, + {"$T = tail($Orders);", "$T = tail $Orders;", deprecation.ListOperationFunctionForm}, + {"$F = filter($Orders, Status = M.Status.Open);", "$F = filter $Orders by Status = M.Status.Open;", deprecation.ListOperationFunctionForm}, + {"$F = filter($Orders, $currentObject/Total > 10);", "$F = filter $Orders where $currentObject/Total > 10;", deprecation.ListOperationFunctionForm}, + {"$F = sort($Orders, OrderDate desc, Number);", "$F = sort $Orders by OrderDate desc, Number;", deprecation.ListOperationFunctionForm}, + {"$U = union($A, $B);", "$U = union $A with $B;", deprecation.ListOperationFunctionForm}, + {"$U = intersect($A, $B);", "$U = intersect $A with $B;", deprecation.ListOperationFunctionForm}, + {"$U = subtract($A, $B);", "$U = subtract $B from $A;", deprecation.ListOperationFunctionForm}, + {"$E = equals($A, $B);", "$E = equals $A and $B;", deprecation.ListOperationFunctionForm}, + {"$R = range($Orders, 20, 10);", "$R = range $Orders offset 20 limit 10;", deprecation.ListOperationFunctionForm}, + {"$N = count($Orders);", "$N = count $Orders;", deprecation.AggregateFunctionForm}, + {"$N = sum($Orders.Amount);", "$N = sum $Orders by Amount;", deprecation.AggregateFunctionForm}, + {"$N = average($Orders, $currentObject/Amount * 2);", "$N = average $Orders of $currentObject/Amount * 2;", deprecation.AggregateFunctionForm}, + {"$N = all($Orders, $currentObject/Paid);", "$N = all $Orders where $currentObject/Paid;", deprecation.AggregateFunctionForm}, + {"$N = reduce($Orders, $currentResult + 1, initial: 0, returns: Integer);", "$N = reduce $Orders from 0 as Integer using $currentResult + 1;", deprecation.AggregateFunctionForm}, + } + for _, header := range []string{"", "mdl 1;\n"} { + for _, tc := range cases { + t.Run(strings.TrimSpace(header+" "+tc.old), func(t *testing.T) { + oldProg, oldStmt, errs := buildListActivity(t, header, tc.old) + if len(errs) > 0 { + t.Fatalf("old form: %v", errs) + } + canonProg, canonStmt, errs := buildListActivity(t, header, tc.canon) + if len(errs) > 0 { + t.Fatalf("canonical form: %v", errs) + } + if !reflect.DeepEqual(oldStmt, canonStmt) { + t.Errorf("forms build different statements:\n old: %#v\n canon: %#v", oldStmt, canonStmt) + } + if got := deprecationCodes(oldProg); !reflect.DeepEqual(got, []string{tc.code}) { + t.Errorf("old form recorded %v, want [%s]", got, tc.code) + } + if got := deprecationCodes(canonProg); len(got) != 0 { + t.Errorf("canonical form recorded %v", got) + } + if n := noteCodes(oldProg); len(n) != 0 { + t.Errorf("an alias is not a change of meaning, but recorded notes %v", n) + } + }) + } + } +} + +// `$x = find(a, b)` and `$x = contains(a, b)` are either the list operation or +// the string function, decided today by what the arguments look like. Under +// mdl 0 that meaning is kept and warned; under mdl 1 the form is refused. +func TestFindContainsFunctionFormIsVersionGated(t *testing.T) { + for _, src := range []string{ + "$M = find($Orders, Number = $Number);", + "$M = contains($Orders, $Order);", + } { + t.Run("mdl 0 "+src, func(t *testing.T) { + prog, stmt, errs := buildListActivity(t, "", src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + if _, ok := stmt.(*ast.ListOperationStmt); !ok { + t.Errorf("mdl 0 must keep the list operation, got %T", stmt) + } + if got := noteCodes(prog); !reflect.DeepEqual(got, []string{listCallForm.Code}) { + t.Errorf("notes = %v, want [%s]", got, listCallForm.Code) + } + if d := deprecationCodes(prog); len(d) != 0 { + t.Errorf("find/contains are not aliases, but recorded %v", d) + } + }) + t.Run("mdl 1 "+src, func(t *testing.T) { + _, _, errs := buildListActivity(t, "mdl 1;\n", src) + if len(errs) == 0 { + t.Fatal("accepted under mdl 1") + } + if msg := errs[0].Error(); !strings.Contains(msg, "set $M =") || !strings.Contains(msg, "List operation") { + t.Errorf("the error should name both readings: %v", msg) + } + }) + } +} + +// Under mdl 1 `set` always assigns an expression, so `set $x = find(…)` is the +// string function whatever its arguments look like. +func TestSetFindIsTheStringFunctionUnderMdl1(t *testing.T) { + body := "declare $I Integer = 0;\n set $I = find($S, $S);" + prog, errs := Build(listActivityMicroflow("mdl 1;\n", body)) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + stmt := prog.Statements[0].(*ast.CreateMicroflowStmt).Body[1] + if _, ok := stmt.(*ast.MfSetStmt); !ok { + t.Errorf("mdl 1: set $I = find($S, $S) built %T, want *ast.MfSetStmt", stmt) + } + + // Control: under mdl 0 the same text keeps today's list-operation reading + // and warns that its meaning differs under mdl 1. + prog, errs = Build(listActivityMicroflow("", body)) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + if _, ok := prog.Statements[0].(*ast.CreateMicroflowStmt).Body[1].(*ast.ListOperationStmt); !ok { + t.Errorf("mdl 0 must keep the list operation reading") + } + if got := noteCodes(prog); !reflect.DeepEqual(got, []string{listCallForm.Code}) { + t.Errorf("notes = %v, want [%s]", got, listCallForm.Code) + } +} + +// Nesting one activity inside another cannot be written under mdl 1; under +// mdl 0 it keeps parsing (MDL-LISTOP02 refuses it at check time) and warns. +func TestNestedListOperationRefusedUnderMdl1(t *testing.T) { + for _, src := range []string{ + "$N = count(filter($Orders, Status = M.Status.Open));", + "set $N = count(filter($Orders, Status = M.Status.Open));", + "set $H = head($Orders);", + } { + t.Run("mdl 1 "+src, func(t *testing.T) { + _, _, errs := buildListActivity(t, "mdl 1;\n", src) + if len(errs) == 0 { + t.Fatal("accepted under mdl 1") + } + if msg := errs[0].Error(); !strings.Contains(msg, "one statement per activity") { + t.Errorf("error should explain the statement form: %v", msg) + } + }) + t.Run("mdl 0 "+src, func(t *testing.T) { + prog, _, errs := buildListActivity(t, "", src) + if len(errs) > 0 { + t.Fatalf("mdl 0 must keep parsing: %v", errs) + } + if got := noteCodes(prog); !reflect.DeepEqual(got, []string{listCallForm.Code}) { + t.Errorf("notes = %v, want [%s]", got, listCallForm.Code) + } + }) + } +} + +// `set` is mandatory for reassignment under mdl 1. +func TestSetIsMandatoryUnderMdl1(t *testing.T) { + body := "declare $I Integer = 0;\n $I = 5;" + _, errs := Build(listActivityMicroflow("mdl 1;\n", body)) + if len(errs) == 0 || !strings.Contains(errs[0].Error(), "set $I = 5") { + t.Fatalf("mdl 1 must refuse a reassignment without set, got %v", errs) + } + + prog, errs := Build(listActivityMicroflow("", body)) + if len(errs) > 0 { + t.Fatalf("mdl 0 must keep parsing: %v", errs) + } + if got := noteCodes(prog); !reflect.DeepEqual(got, []string{setIsMandatory.Code}) { + t.Errorf("notes = %v, want [%s]", got, setIsMandatory.Code) + } + + // Control: with `set`, neither version says anything. + for _, header := range []string{"", "mdl 1;\n"} { + prog, errs := Build(listActivityMicroflow(header, "declare $I Integer = 0;\n set $I = 5;")) + if len(errs) > 0 || len(prog.LanguageNotes) > 0 { + t.Errorf("%q: set $I = 5 gave errors %v, notes %v", header, errs, prog.LanguageNotes) + } + } + // An attribute target is `change`'s territory (plan item 3.x), not this rule. + prog, errs = Build(listActivityMicroflow("mdl 1;\n", "$Order/Number = 5;")) + if len(errs) > 0 || len(prog.LanguageNotes) > 0 { + t.Errorf("$Order/Number = 5 gave errors %v, notes %v", errs, prog.LanguageNotes) + } +} + +// `by` picks a member: anything but `Member = value` belongs after `where`. +func TestFilterByNeedsMemberEquality(t *testing.T) { + _, _, errs := buildListActivity(t, "", "$F = filter $Orders by $currentObject/Total > 3;") + if len(errs) == 0 || !strings.Contains(errs[0].Error(), "where") { + t.Fatalf("want an error pointing at `where`, got %v", errs) + } +} diff --git a/mdl/visitor/visitor_microflow_actions.go b/mdl/visitor/visitor_microflow_actions.go index 0df0fd391..534fb67ac 100644 --- a/mdl/visitor/visitor_microflow_actions.go +++ b/mdl/visitor/visitor_microflow_actions.go @@ -803,7 +803,20 @@ func buildListOperationStatement(ctx parser.IListOperationStatementContext) *ast stmt.OutputVariable = strings.TrimPrefix(v.GetText(), "$") } - // Get the list operation + // The statement form: one Studio Pro activity (#733). + if act, ok := listOpCtx.ListOperationActivity().(*parser.ListOperationActivityContext); ok && act != nil { + buildListOperationActivity(act, stmt) + return stmt + } + + // The call form. For find/filter it is a respelling of `by` when the + // condition reads `Member = value` and of `where` otherwise, so it records + // which, exactly as the flow builder has always decided (ast.IsMemberEquality). + defer func() { + if stmt.Operation == ast.ListOpFind || stmt.Operation == ast.ListOpFilter { + stmt.ByExpression = !ast.IsMemberEquality(stmt.Condition) + } + }() if opCtx := listOpCtx.ListOperation(); opCtx != nil { op := opCtx.(*parser.ListOperationContext) @@ -947,7 +960,13 @@ func buildAggregateListStatement(ctx parser.IAggregateListStatementContext) *ast stmt.OutputVariable = strings.TrimPrefix(v.GetText(), "$") } - // Get the aggregate operation + // The statement form: one Studio Pro Aggregate list activity (#733). + if act, ok := aggrCtx.AggregateListActivity().(*parser.AggregateListActivityContext); ok && act != nil { + buildAggregateListActivity(act, stmt) + return stmt + } + + // The call form, a deprecated alias of the above. if opCtx := aggrCtx.ListAggregateOperation(); opCtx != nil { op := opCtx.(*parser.ListAggregateOperationContext) diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index 34cca43af..38bd1709b 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -802,8 +802,10 @@ func buildSetStatementNode(ctx parser.ISetStatementContext) ast.MicroflowStateme valueExpr = buildExpression(expr) } - // Check if the expression is a list operation or aggregate function. - if funcCall, ok := valueExpr.(*ast.FunctionCallExpr); ok { + // Check if the expression is a list operation or aggregate function. Under + // mdl 1 `set` always assigns an expression: a list-operation call there is + // refused (ExitSetStatement), and find/contains are the string functions. + if funcCall, ok := valueExpr.(*ast.FunctionCallExpr); ok && !listCallForm.Applies(scriptLanguageVersion(setCtx)) { if stmt := buildListOrAggregateStatement(targetVar, funcCall); stmt != nil { return recordUnresolvedOperands(stmt, funcCall.Arguments) } @@ -857,20 +859,24 @@ func buildListOrAggregateStatement(targetVar string, funcCall *ast.FunctionCallE // (CE0111). Ledger #63. When both arguments are plain variables the kind // is ambiguous here; the flow builder disambiguates String-typed inputs. if !isStringLiteralArg(funcCall.Arguments, 1) { + cond := getArgumentExpression(funcCall.Arguments, 1) return &ast.ListOperationStmt{ OutputVariable: targetVar, Operation: ast.ListOpFind, InputVariable: extractVariableName(funcCall.Arguments, 0), - Condition: getArgumentExpression(funcCall.Arguments, 1), + Condition: cond, + ByExpression: !ast.IsMemberEquality(cond), } } // Falls through to the default MfSetStmt (string find expression). case "FILTER": + cond := getArgumentExpression(funcCall.Arguments, 1) return &ast.ListOperationStmt{ OutputVariable: targetVar, Operation: ast.ListOpFilter, InputVariable: extractVariableName(funcCall.Arguments, 0), - Condition: getArgumentExpression(funcCall.Arguments, 1), + Condition: cond, + ByExpression: !ast.IsMemberEquality(cond), } case "SORT": stmt := &ast.ListOperationStmt{ diff --git a/modelsdk/canon/identity.go b/modelsdk/canon/identity.go index 8c9143d9b..8a8cb0c96 100644 --- a/modelsdk/canon/identity.go +++ b/modelsdk/canon/identity.go @@ -36,6 +36,23 @@ type Option func(*reconcileOpts) type reconcileOpts struct { contentsOwnTranslations bool contentsOwnStorageGUIDs bool + contentsOwnElementIDs bool +} + +// ContentsOwnElementIDs tells Reconcile that the write is a PATCH of the stored +// document, not a rebuild: every element that survived kept its stored $ID, and +// every new element carries a fresh one on purpose. So the structural transplant +// must not run. +// +// The transplant exists for rebuilds, whose elements all arrive with random +// $IDs and are paired back by type and position. On a patch that pairing is not +// a no-op but a hazard: drop one sequence flow and every flow after it pairs with +// its predecessor, taking that flow's $ID — identities move onto other nodes, +// which is what ADR-0012 measured the microflow rebuild doing (51 of 161). The +// graph splice of `alter microflow` (mfmutator) is the caller; it has its own +// guard that no reference is left dangling. Elision still applies. +func ContentsOwnElementIDs() Option { + return func(o *reconcileOpts) { o.contentsOwnElementIDs = true } } // ContentsOwnTranslations tells Reconcile that the write already accounts for @@ -114,7 +131,9 @@ func Reconcile(contents, stored []byte, opts ...Option) (out []byte, unchanged b // version control as a whole-document replacement. TransplantIDs puts the // stored ids back on the elements that still correspond, rewriting every // reference with them. - contents = TransplantIDs(contents, stored) + if !o.contentsOwnElementIDs { + contents = TransplantIDs(contents, stored) + } // And the nested identity property the transplant does not cover: every // Workflows$* element carries a PersistentId that both engines re-mint on diff --git a/modelsdk/canon/patch_ids_test.go b/modelsdk/canon/patch_ids_test.go new file mode 100644 index 000000000..cae29e2f3 --- /dev/null +++ b/modelsdk/canon/patch_ids_test.go @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: Apache-2.0 + +package canon + +import ( + "bytes" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" +) + +// seqFlows builds a flow document whose Flows list holds one sequence flow per +// id in ids, each from origin i to origin i+1. Every flow has the same $Type +// and shape, which is exactly what makes the structural pairing positional. +func seqFlows(t *testing.T, ids ...byte) []byte { + t.Helper() + flows := bson.A{int32(3)} + for i, id := range ids { + flows = append(flows, bson.D{ + {Key: "$Type", Value: "Microflows$SequenceFlow"}, + {Key: "$ID", Value: bin(id)}, + {Key: "OriginPointer", Value: bin(byte(100 + i))}, + {Key: "DestinationPointer", Value: bin(byte(101 + i))}, + }) + } + return marshal(t, bson.D{ + {Key: "$Type", Value: "Microflows$Microflow"}, + {Key: "$ID", Value: bin(1)}, + {Key: "Flows", Value: flows}, + }) +} + +// A patch of the stored document — an `alter microflow` drop — keeps every +// surviving element's $ID already. The structural transplant must not then +// re-pair those elements: with one flow gone, the flows after it pair +// positionally with their predecessors and each takes its neighbour's $ID, +// moving identities onto other nodes (ADR-0012's "51 of 161 $IDs changed"). +func TestReconcile_PatchOwnsElementIDs(t *testing.T) { + stored := seqFlows(t, 10, 20, 30, 40) + patched := seqFlows(t, 10, 30, 40) // flow 20 dropped; 30 and 40 keep their $IDs + + // Control: the rebuild path pairs by position and moves $IDs, which is + // the defect the option exists for. + rebuilt, _ := Reconcile(patched, stored) + if ids := idSet(t, rebuilt); ids[blobToUUID(bin(40).Data)] { + t.Fatalf("control: expected the transplant to move $ID 40 onto another flow; the test would prove nothing") + } + + out, unchanged := Reconcile(patched, stored, ContentsOwnElementIDs()) + if unchanged { + t.Fatal("a patch that dropped a flow reported no change") + } + if !bytes.Equal(out, patched) { + t.Errorf("a patch that owns its element $IDs was rewritten by Reconcile") + } +} diff --git a/modelsdk/mpr/writer_core.go b/modelsdk/mpr/writer_core.go index 9d9e0b23c..ec92cb7b8 100644 --- a/modelsdk/mpr/writer_core.go +++ b/modelsdk/mpr/writer_core.go @@ -828,6 +828,17 @@ func (w *Writer) UpdateRawUnitOwningTranslations(unitID string, contents []byte) return w.updateUnit(unitID, contents, canon.ContentsOwnTranslations()) } +// UpdateRawUnitPatch is UpdateRawUnit for a write that PATCHED the stored bytes +// in place rather than rebuilding them — the graph splice of `alter microflow`. +// Such a write already carries every stored $ID and translation it means to +// keep, so neither is carried back: re-pairing element $IDs structurally would +// move identities onto other elements after a drop (canon.ContentsOwnElementIDs), +// and carrying translations would undo a deliberate removal. Elision and the +// storage-GUID guard apply as for every write. +func (w *Writer) UpdateRawUnitPatch(unitID string, contents []byte) error { + return w.updateUnit(unitID, contents, canon.ContentsOwnElementIDs(), canon.ContentsOwnTranslations()) +} + // UpdateRawUnitOwningStorageGUIDs is UpdateRawUnit for a write that deliberately // transplants storage GUIDs onto elements that keep their $ID — the marketplace // module update, which carries a module's existing GUIDs onto the documents