diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-11-06-wf-drop-member-guesses-among-equals.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-11-06-wf-drop-member-guesses-among-equals.json new file mode 100644 index 0000000000..426a9fba1d --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-11-06-wf-drop-member-guesses-among-equals.json @@ -0,0 +1 @@ +{"area": "mdl/backend", "date": "2026-11-06", "symptom": "alter workflow: `drop X outcome 'V'` / `drop X outcome ` dropped the first of two outcomes sharing that value; `drop X boundary event` could not say which of several events, so under mdl 1 it was refused with no way to drop the second; check-time guards went silent on a reference the mutator resolved by name, because resolveStoredActivity pooled a jump captioned with its target's name with the target itself", "cause": "Both mutators' drop loops stopped at the first match and never counted; the boundary-event op carried no address at all; the executor's resolveStoredActivity copied findActivityByCaption but not its preferNameMatches narrowing", "file": "mdl/backend/wfmutator/mutator.go (dropOneOutcome), mdl/backend/mcp/workflow.go (dropFromActivityArray), mdl/backend/workflow_members.go (BoundaryEventIndex), mdl/executor/validate_workflow_activity_kind.go (resolveStoredActivity)", "fix": "A value that matches more than one outcome is refused as ambiguous, nothing written, in both mutators; `boundary event N` (1-based, describe order) addresses one event, out of range refused at exec before the mutator; resolveStoredActivity prefers Name matches without @N", "insight": "A check-time resolver that mirrors a mutator must mirror every narrowing step, or the check silently abstains (it returns nil on ambiguity by design) exactly where exec acts; test the resolver on the jump-caption shape directly. A drop loop with `!found` guards is the tell for a first-match guess: count matches, then decide", "refs": "ako/mxcli#808, ako/mxcli#796", "test": "mdl/backend/wfmutator/mutator_test.go (DuplicateValueRefused, DropBoundaryEvent_ByIndex); mdl/executor/alter_workflow_member_address_test.go (DropBoundaryEventByIndex, ResolveStoredActivity_PrefersName); mdl-examples/bug-tests/808-drop-boundary-event-by-number.mdl"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-image-reference-check-folds-case.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-image-reference-check-folds-case.json new file mode 100644 index 0000000000..7151f8d258 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-04-image-reference-check-folds-case.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-11-04", "symptom": "`mxcli check --references` passed `Image: 'Atlas_Core.Content.mendix'` for the stored image `Mendix` (and `Atlas_Core.content.Mendix`, `atlas_core.Content.Mendix`); mx then reported CE1613 \"The selected image '…' no longer exists\" for each. The miss hint `it holds: mendix` printed the name lower-cased, so copying it from the message produced the CE1613", "cause": "imageRefErrors / buildImageQualifiedNames (and the script context's created collections) keyed every image and collection lower-cased on the unmeasured assumption, pinned by two tests, that Mendix resolves names case-insensitively", "file": "`mdl/executor/validate_widget_image_ref.go` (imageRefErrors, sameNameOtherCase, imagesInCollection, buildImageQualifiedNames), `mdl/executor/validate.go` (scriptContext image collections)", "insight": "Measure case-sensitivity per part before encoding it: one blank-app run with four pages (exact, wrong image case, wrong collection case, wrong module case) through `docker check --no-update-widgets` settled it — CE1613 on all three, clean on the exact one. Two tests asserted the opposite (\"Mendix resolves names case-insensitively, and so must this\") with no measurement behind them; they were inverted, not deleted. A case-only miss names the stored spelling instead of the generic hint, and the hint lists names as stored. The icon reference check (iconIndex) was already exact. Found while verifying ako/mxcli#1503. The navigation `app icon` goes through the same resolveImageRefs and carried the same assumption (TestAppIconMustResolveAtExec accepted `myfirstmodule.images.logo`): measured, main wrote `app icon Atlas_Core.Content.mendix` and mx reported CE1613 at the navigation profile, so that case is now refused too. A full `make ci-local` needs the testdata/testapp submodule (`git submodule update --init testdata/testapp`); without it TestIssue1640_RegisteredListMarkersAreWhatStudioProStores fails on main as well", "refs": ["ako/mxcli#1503"], "ce": ["CE1613"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-alter-page-set-template-params.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-alter-page-set-template-params.json new file mode 100644 index 0000000000..aa53123fd8 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-alter-page-set-template-params.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-05","symptom":"`alter page … { set (CaptionParams: ({1} = VersionNo)) on btn }` and `set (ContentParams: …)` on a dynamic text: parse error (\"no viable alternative at input '({'\"), and the visitor added \"takes a plain value, not an expression\" — a template's parameters could be written by create page and replace but not changed in place (mendixlabs/mxcli#1744)","cause":"alterPageAssignment had no alternative for paramListV3, and the page mutator no setter for a ClientTemplate's Parameters","file":"mdl/grammar/MDLParser.g4 (alterPageAssignment), mdl/visitor/visitor_alter_page.go, mdl/executor/cmd_alter_page.go (setTemplateParameters), mdl/backend/pagemutator/template_params.go; test alter_set_template_params_pedapp_test.go","insight":"Resolve the parameters with the CREATE builder (buildClientTemplateParams) seeded with mutator.EnclosingEntity(widget), not with a resolver of the ALTER path's own: a bare attribute then means in a SET what it means in create page. Prove the written property set against create page's, not only against mx check — dump both pages and diff the Forms$ClientTemplateParameter with $IDs stripped (identical here); merge compares property names (#1373). Measured on fresh 11.14.0: check, exec and mx check (0 errors) all pass.","refs":["ako/mxcli#1966","mendixlabs/mxcli#1744"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-alter-set-column-attribute.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-alter-set-column-attribute.json new file mode 100644 index 0000000000..33fa048269 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-alter-set-column-attribute.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-05","symptom":"`alter page … { set (Attribute: Note) on dgOrders column(Number) }` refused for every spelling — \"column property \\\"Attribute\\\" holds a value of kind Attribute, which ALTER cannot set from a plain value\" — and `Attribute: Assoc/Attr` was a parse error; replacing the column was the only route, and it drops column properties the replacement does not restate (ako/mxcli#1644)","cause":"setColumnPropertyMut wrote only plain-value kinds, nothing bound the column's attribute against the grid's data source, and alterPageAssignment parsed `A/B` as a division expression","file":"mdl/executor/alter_set_column_attribute.go (resolveColumnAttribute), mdl/executor/cmd_alter_page.go (IsColumn Attribute branch; attribute applied last in applySetPropertyMutator), mdl/backend/pagemutator/mutator.go (setColumnAttributeBinding), mdl/backend/mutation.go (AttributeBinding.Steps), mdl/grammar/MDLParser.g4 + visitor_alter_page.go (ATTRIBUTE: hop/path); tests alter_set_column_attribute_pedapp_test.go","insight":"A SET that changes the property an element is ADDRESSED by breaks every later step that re-finds it by the address as written — here the hidden-property reset after a column SET (#1519), which failed with \"column(Old) not found\" once the attribute moved. Apply the address-changing property last. PedApp's Data grid 2 package ships no editorConfig, so the reset never ran there and the PedApp test passed against the bug; the 11.14 measurement found it, and the regression test installs the expr-checker fixture's package to reach it (control: without the ordering it fails with the measured error). Binding mirrors #1641: the executor resolves against EnclosingEntityForChildren(grid) with misbound/foreign-qualified refusals, the mutator writes AttributeRef + EntityRef steps as CREATE writes a multi-hop column.","refs":["ako/mxcli#1644","ako/mxcli#1641","mendixlabs/mxcli#1519"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-array-rooted-mapping-returns-one-object.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-array-rooted-mapping-returns-one-object.json new file mode 100644 index 0000000000..390dcbfa43 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-array-rooted-mapping-returns-one-object.json @@ -0,0 +1 @@ +{"area":"mdl/types","date":"2026-11-05","symptom":"`$Root = import from mapping M.IM($Json)` over an array-rooted JSON structure whose mapping maps the array itself to its root entity (`create M.Root { create M.Item_Root/M.Item = JsonObject {…} }`): mx check 11.14 CE0243 \"The mapping used to return a value of type 'List of M.Root', but now returns a value of type 'M.Root'\" on the first create — the reporter saw it after re-executing; Studio Pro's \"open the action and click OK\" cleared it (mendixlabs/mxcli#1734)","cause":"types.JsonMappingRootIsList fell back to the JSON structure's root kind whenever the root path did not cross an array, so ANY array-rooted structure read as list-returning — but a root at \"(Array)\" is the array container itself, one object; only a root at \"(Array)|(Object)\" (the item, Studio Pro's own shape, #248) yields many","file":"mdl/types/mapping_shape.go (JsonMappingRootIsList: a stored root path decides alone); test mdl/types/mapping_shape_test.go","insight":"Cardinality is a property of WHERE the mapping roots, not of the structure: the same array-rooted structure gives one object or many depending on whether the root element is the array or its item. A fallback to a coarser source is only safe when the finer one is absent — here the fallback ran whenever the finer source said 'not a list', which turned its 'no' into 'unknown'. The fix is shared by the lint rules (same function). Measured on fresh 11.14.0: base 1 x CE0243 on create; fix 0 on create and after a re-execute with a renamed variable.","refs":["ako/mxcli#1938","mendixlabs/mxcli#1734","#248","#267"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-custom-date-format-check-per-property.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-custom-date-format-check-per-property.json new file mode 100644 index 0000000000..8914ac2c1a --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-custom-date-format-check-per-property.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-05","symptom":"`alter page … { set (DateFormat: Custom, CustomDateFormat: 'dd/MM/yyyy') on dp }` on a picker whose stored format is not Custom: check refused it (\"`DateFormat: Custom` needs a pattern … in the same statement\") in either order and as two SETs, so exec's pre-check refused it too; --no-check wrote it and mx check passed (mendixlabs/mxcli#1743)","cause":"setWidgetFormattingMut refused Custom on the STORED blank pattern, relying on exec applying properties in name order; check's checkSetOp dry-runs one property at a time on a fresh probe of the stored widget, where Custom never sees the pattern. The whole-statement post-check (checkDateFormatAfterSet, #977) ran only in exec","file":"mdl/backend/pagemutator/mutator.go (in-setter guard removed), mdl/executor/validate_alter_set.go (checkDateFormatSet: earlier SETs on the widget, then this one whole, then the #977 check); test alter_set_date_format_pedapp_test.go","insight":"A rule between two properties cannot live in a per-property setter: whichever setter runs first sees half the statement, and a per-property dry run sees less. Put the cross-property rule after the whole statement, and give check the SAME judgement on a probe carrying the statement's earlier SETs — exec applies them in sequence on one copy, check probes the stored widget per SET. assertAgree (check then exec, same verdict) is the test shape for every check/exec disagreement. Control: before the change the three reported forms fail assertAgree; Custom with no pattern is refused by both throughout.","refs":["ako/mxcli#1965","mendixlabs/mxcli#1743","ako/mxcli#977"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-empty-then-decision-printed-as-stored.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-empty-then-decision-printed-as-stored.json new file mode 100644 index 0000000000..19e5a3ca6f --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-empty-then-decision-printed-as-stored.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-05","symptom":"describe printed a decision whose true branch is empty as `if not(X) then end if;`; executing the description stored not(X) with the true/false outcomes swapped — behaviour unchanged, model changed, so describe -> exec was not a round trip (mendixlabs/mxcli#1742)","cause":"an 'empty-then swap' in traverseFlow / traverseFlowUntilMerge folded the decision for readability; #1930 then had to undo the fold in the condition-edit splice (decisionPrintedNegated), and alter targets carried the stored form as an Alternate","file":"mdl/executor/cmd_microflows_show_helpers.go (swap removed, negateIfCondition / negateExpression / decisionPrintedNegated deleted), mdl/executor/cmd_alter_flow.go (no fold to undo); tests flow_splice_negated_condition_pedapp_test.go (TestDescribeExec_EmptyThenDecisionRoundTrips), cmd_microflows_traverse_test.go, cmd_microflows_handles_test.go","insight":"A describe that prints a NORMALISED form instead of the stored one is a standing round-trip defect: every consumer of the text inherits a compensation (the splice's un-negation, the handles' alternates), and each new consumer is a new place to forget one. Prefer printing what is stored when the grammar can express it — `if X then else … end if;` always parsed and builds exactly Studio Pro's shape. A script written against the old folded output still means the same thing: the splice diffs the branches independently, so `if not(Y) then B` over the stored `if X then else B` sets the condition and moves B, it does not invert. Its alter TARGETS in the folded form no longer resolve — a refusal, never a wrong edit. Control: with the fold restored, the PedApp round-trip test reproduces the reporter's table exactly (not($Sale/Code != empty), outcomes swapped).","refs":["ako/mxcli#1963","mendixlabs/mxcli#1742","ako/mxcli#1930"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-external-action-include-associations-clause.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-external-action-include-associations-clause.json new file mode 100644 index 0000000000..fa6e646660 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-external-action-include-associations-clause.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-05","symptom":"a call external action argument's Associations to include had no MDL: describe did not print the tree, a new call could not have one, and — found while measuring — a `create or modify` that took the splice path rebuilt the call and dropped the stored tree, with no warning, although #1922's carry claimed to keep it","cause":"#1922 wired carryIncludedAssociations into buildMicroflowFromStmt only; a create or modify of a stored flow is applied by modifyFlowInPlace first, whose replace op rebuilds the activity through the alter-flow fragment builder and never reaches the carry","file":"mdl/grammar/domains/MDLMicroflow.g4 (includeAssociationsClause), mdl/executor/cmd_microflows_builder_calls.go (buildIncludedAssociations), external_action_carry.go (stated, carriedIncludes), cmd_alter_flow.go (replace carries, statedIncludes), cmd_flow_modify.go (warning), cmd_microflows_format_action.go (describe), validate_external_action_includes.go (MDL-ODATA10), validate_external_action_calls.go (check resolution); tests external_action_include_clause_test.go, external_action_include_splice_pedapp_test.go, mdl/backend/modelsdk/microflow_external_action_include_clause_test.go","insight":"A carry for unauthored state has as many sites as the statement has write paths: create or modify of a STORED flow is a splice first and a rebuild only on fallback, so a carry in the rebuild alone is skipped in the common case. The mock harness never caught it because its stored flow had no start/end and always fell back to the rebuild — the bug only showed on a real project. Enumerate the write paths (rebuild, splice replace, alter replace) and test the splice against the real backend (PedApp + a local $metadata). IsParent is derived from the side entity being the association's FROM end; the authored tree encodes to Studio Pro 11.16's bytes, $IDs aside, and flipping IsParent breaks that comparison. Controls: stashing the splice change fails the PedApp test with the tree dropped; removing the stated skip makes `include associations ()` keep the tree.","refs":["ako/mxcli#1914","mendixlabs/mxcli#1721","ako/mxcli#1922"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-snippetcall-dataview-argument-widget-slot.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-snippetcall-dataview-argument-widget-slot.json new file mode 100644 index 0000000000..039310998e --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-snippetcall-dataview-argument-widget-slot.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-11-05","symptom":"`snippetcall … Params: (Item = $dvOuter)` — a data view's object passed by the data view's name — passed check and exec and described back, and mx check 11.14.0 reported CE0115 \"The arguments that are passed to snippet … need to be refreshed\" (mendixlabs/mxcli#1746)","cause":"buildSnippetCallParams knew three argument kinds (snippet parameter, page variable, everything else as PageParameter); the `widget` kind classifyFlowArgValue already returns for a document widget (#1560) was dropped, so the data view name went into the PageParameter slot","file":"mdl/executor/cmd_pages_builder_v3_widgets.go (IsWidget), sdk/pages/pages_widgets_advanced.go, mdl/backend/modelsdk/widget_write.go (Widget slot), mdl/executor/cmd_pages_describe_parse.go (flowArgVariableValue); tests snippetcall_widget_argument_test.go, widget_scoped_variable_test.go","insight":"The classifier already said `widget`; the snippet-call consumer mapped only the kinds it knew and let the rest fall into the default slot — when a classifier gains a kind, grep its consumers for a switch without that case. Measure on the reporter's version: 11.12.6 reports nothing for this mapping, base build or fixed, so a measurement there would have proven nothing; 11.14.0 gives the reported 2 x CE0115 on the base build and 0 on the fix.","refs":["ako/mxcli#1968","mendixlabs/mxcli#1746","ako/mxcli#1560"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-unset-datasource-text-template-null.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-unset-datasource-text-template-null.json new file mode 100644 index 0000000000..28712386bb --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-05-unset-datasource-text-template-null.json @@ -0,0 +1 @@ +{"area":"mdl/backend/widgetobj","date":"2026-11-05","symptom":"a pluggable widget with an optional datasource X and a textTemplate bound to it (dataSource=\"X\"), both unset: check and exec pass, mx check 11.14 reports CE0463 \"The definition of this widget has changed\"; `mx update-widgets` changes only that TextTemplate, empty Forms$ClientTemplate -> null (mendixlabs/mxcli#1745)","cause":"the builder nulls a TextTemplate only when the widget's editorConfig hides it (#574) or an item rule does (#1198); a template bound to an unset data source is a third case, decided by the widget.xml binding, not by editorConfig — and the binding was already loaded as PropertyTypeIDEntry.DataSourceProperty","file":"mdl/backend/widgetobj/unset_datasource_templates.go (NullTemplatesOfUnsetDataSources), mdl/executor/widget_engine.go (step 4.11), mdl/backend/mutation.go; test unset_datasource_templates_test.go","insight":"The reporter's widget is unpublished, and no widget an app ships has a TOP-LEVEL textTemplate bound to an OPTIONAL datasource (Gallery, Timeline and TreeNode bind to required ones; HTMLElement's is hidden by editorConfig outside repeat mode). A probe .mpk is a few lines: package.xml, the widget XML pair from the issue, an empty .mjs — `mxcli widget extract --mpk` then makes it authorable. Measured on fresh 11.14.0: base 3 x CE0463 (one per page), fix 0, and the reporter's control (data source set, template unset) keeps its empty template in both. Only unset templates are nulled; authored text is the statement's.","refs":["ako/mxcli#1967","mendixlabs/mxcli#1745","#574","mendixlabs/mxcli#1198"]} diff --git a/.claude/skills/fix-issue/findings/mdl-other/2026-11-05-odata-vocabulary-alias-terms-ignored.json b/.claude/skills/fix-issue/findings/mdl-other/2026-11-05-odata-vocabulary-alias-terms-ignored.json new file mode 100644 index 0000000000..40a44781ca --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-other/2026-11-05-odata-vocabulary-alias-terms-ignored.json @@ -0,0 +1 @@ +{"area":"mdl/types","date":"2026-11-05","symptom":"`create external entities` from a $metadata that declares and writes Term=\"Capabilities.InsertRestrictions\": the entity imported as not creatable / deletable, and mx check 11.14 reported 4 x CE6630 (\"marked Creatable=True in the OData service, but False in the app\") plus CE0852 / CE9435 on the flows writing it (ako/mxcli#1977)","cause":"ParseEdmx matched every annotation term by its full namespace (`case \"Org.OData.Capabilities.V1.UpdateRestrictions\"`) and never read edmx:Reference/Include aliases, so an aliased term matched nothing and was skipped silently — 12 terms (8 Capabilities, 4 Core) across entity sets, schema-level Annotations blocks, entity types and properties","file":"mdl/types/edmx.go (normalizeAnnotationTerms, xmlReference/xmlInclude, Schema Alias); test mdl/types/edmx_vocabulary_alias_test.go","insight":"Normalise at the boundary, once: rewriting each aliased term to its full namespace right after Unmarshal fixes every consumer, the six switch sites and any added later, where teaching each switch about aliases would leave the next one to forget. An undeclared prefix is left as written (the control), so nothing is guessed. A contract fixture is a local file next to the project (MetadataUrl: 'x.xml'); measured on fresh 11.14.0: base 6 errors, fix 0. Updatable stays No by design — Mendix's own Updatable does not follow UpdateRestrictions (cmd_contract.go), and mx check agrees.","refs":["ako/mxcli#1977","ako/mxcli#1939"]} diff --git a/.claude/skills/mendix/write-workflows/SKILL.md b/.claude/skills/mendix/write-workflows/SKILL.md index 4fbfb129ff..2f71a56879 100644 --- a/.claude/skills/mendix/write-workflows/SKILL.md +++ b/.claude/skills/mendix/write-workflows/SKILL.md @@ -365,9 +365,9 @@ the same function — and the refusal names the fragment that fits the target. branch cannot write a wrong type; it leaves an ordinary build error (`CE6686`) rather than an unloadable project. `path n` addresses a **parallel split** only — on a user task it used to delete the n-th outcome (ako/mxcli#791) and is now -refused; drop a user task's outcome by its value. `drop X boundary event` names -no event, so on an activity with several it is refused under `mdl 1` (under -`mdl 0` it drops the first and warns `MDL-V1-BOUNDARYDROP`). +refused; drop a user task's outcome by its value (refused if two share it). +`drop X boundary event 2` drops the second as `describe` lists them; with no number +and several events it is refused under `mdl 1` (`mdl 0` drops the first, warns `MDL-V1-BOUNDARYDROP`). The old per-action statements (`alter workflow M.W set display 'X';`, `set activity X page …`, `insert outcome 'N' on X { }`, `drop path 'Path 2' on X`) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99952a4401..fcc12d5d5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- **`drop X boundary event N` drops one boundary event of several** (ako/mxcli#808) — numbered as `describe workflow` lists them. Without a number the op named no event, so under `mdl 1` an activity with several could not lose any of them; a number past the last is refused with nothing written. +- **Associations to include on a `call external action` argument** (ako/mxcli#1914, mendixlabs/mxcli#1721) — `command = $Cmd include associations (Odata.Quantity (Odata.Unit))` is Studio Pro's *Associations to include*: the associations whose objects are sent in the OData request with that argument, as a tree. Without them a sub-object linked to the argument was silently missing at runtime. Which end each association is on is worked out from the domain model; a self-association, or one that does not touch the entity on its side, is refused by `check -p` and `exec`. `describe` prints the clause. `include associations ()` removes them all. Left out on `create or modify`, the stored tree is kept, and `check -p` and `exec` warn (MDL-ODATA10). That now also holds when the rewrite is spliced in place, which dropped the tree. Mendix 10.11+ for a non-empty clause. See `mxcli syntax odata.consume`. - **`send external object` and `delete external object`** (ako/mxcli#1939, mendixlabs/mxcli#1735) — Studio Pro's *Send external object* and *Delete external object* activities, which save and remove an external (OData) object, now have MDL: `send external object $Obj [refresh] [on error …];` and `delete external object $Obj [refresh] [on error …];`. `describe` prints them, so a flow that holds one survives a drop + create from its description, which used to delete it silently. `check` refuses `on error continue` on either (CE6035, MDL076). See `mxcli syntax odata.write`. - **A module's version and module type from MDL** (ako/mxcli#1921) — the General tab of Studio Pro's *Module settings* dialog: `create or modify module Inventory (Version: '2.3.1', ModuleType: addon)` and `alter module Inventory set (Version: '2.4.0')`. `describe module` prints the settings that differ from a new module's (App, 1.0.0), and the location as a comment when it is *Marketplace modules*; `list modules` gains `Version` and `Type` columns. The version must be SemVer, the type `app` or `addon` (Mendix 9.12+), and a statement that restates the stored settings writes nothing. Measured on 11.16: the type is the settings' `ExportLevel` (`Source` / `Protected`), not `ProtectedModuleType`, which is `AddOn` on every module. ### Fixed +- **`alter workflow` no longer guesses which outcome to drop** (ako/mxcli#808) — `drop X outcome 'V'` with two outcomes of value `V` dropped the first; it is refused as ambiguous now. `check --references` also resolves an activity reference by name before caption, as `exec` does, so its guards no longer go silent on a target captioned like another activity's name. +- **describe → exec keeps a decision whose true branch is empty** (ako/mxcli#1963, mendixlabs/mxcli#1742) — `describe microflow` folded such a decision into `if not(X) then end if;`, and executing that stored `not(X)` with the true and false outcomes swapped: the same behaviour, a different model in Studio Pro. It is now printed as stored, `if X then else end if;`, which builds Studio Pro's shape. A script written from the old output still means the same thing; an `alter microflow` target written in the folded `if not(X) then` form no longer resolves, and is refused rather than applied. - **`test --attach` on Mendix 11.14.0 says why it is refused instead of blaming an entity** — that version's `mxbuild --serve` fails every build after its first, so since ako/mxcli#1917 the attach builds on a fresh serve process, whose first build asks for a runtime restart whatever changed. The refusal claimed "an entity or association changed" and suggested restarting the dev loop, which does not help; it now says the build could not be done incrementally and points to `--local`. The hosting app's pages survive either way. 11.15.0 builds incrementally and attaches as before. - **`describe microflow` no longer leaves out a note linked to the start event or to another note** (ako/mxcli#1933, mendixlabs/mxcli#1731) — such a note counted as linked, so it was not printed as a free note, and the element it is linked to prints no `@annotation`, so it was not printed at all; re-executing the description deleted it. It is now described as a free note, after a `-- WARNING` line saying the link itself has no MDL form and a rewrite drops it. - **Rebuilding a described decision no longer adds a merge** (ako/mxcli#1929, mendixlabs/mxcli#1727) — a `case`, `split type` or `if … elsif` one of whose branches returns is described with its own merge as a label (`end case; join shared1; merge shared1;`). Executing that as a fresh create built the decision's merge and a second one for the label, wired merge to merge, and every further describe → drop + create added another (`join junctionN; merge junctionN;`). The label's merge is now the decision's merge when nothing else reaches it, so the round trip keeps one merge. diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index 88e3be3213..86c5c2a6f3 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -484,6 +484,11 @@ func runCheckFileAgainst(cmd *cobra.Command, filePath, against string) int { // mdl 0, and an alter whose patch fails is refused under both. They // run the verdict exec and diff run, so the three agree. projectViolations := exec.CheckEntityMemberDrops(prog) + // MDL-ODATA10: a call external action argument whose stored Associations + // to include a create or modify keeps because the statement does not + // write the clause (ako/mxcli#1914) — the omitted-clause warning exec + // prints, said before the write. + projectViolations = append(projectViolations, exec.CheckCarriedIncludedAssociations(prog)...) projectViolations = append(projectViolations, exec.TypeCheckProgram(prog)...) flowReport := exec.CheckFlowVerdictsReport(prog) projectViolations = append(projectViolations, flowReport.Violations...) diff --git a/cmd/mxcli/lsp_completions_gen.go b/cmd/mxcli/lsp_completions_gen.go index 1e95c59e03..b26ce57bb7 100644 --- a/cmd/mxcli/lsp_completions_gen.go +++ b/cmd/mxcli/lsp_completions_gen.go @@ -31,6 +31,7 @@ var mdlGeneratedKeywords = []protocol.CompletionItem{ {Label: "MERGE", Kind: protocol.CompletionItemKindKeyword, Detail: "DDL keyword"}, {Label: "NORMALIZED", Kind: protocol.CompletionItemKindKeyword, Detail: "DDL keyword"}, {Label: "HANDLES", Kind: protocol.CompletionItemKindKeyword, Detail: "DDL keyword"}, + {Label: "INCLUDE", Kind: protocol.CompletionItemKindKeyword, Detail: "DDL keyword"}, {Label: "ENTITY", Kind: protocol.CompletionItemKindKeyword, Detail: "DDL keyword"}, {Label: "PERSISTENT", Kind: protocol.CompletionItemKindKeyword, Detail: "DDL keyword"}, {Label: "VIEW", Kind: protocol.CompletionItemKindKeyword, Detail: "DDL keyword"}, diff --git a/cmd/mxcli/syntax/features_integration.go b/cmd/mxcli/syntax/features_integration.go index 2892fb51ae..10fab80e40 100644 --- a/cmd/mxcli/syntax/features_integration.go +++ b/cmd/mxcli/syntax/features_integration.go @@ -32,13 +32,15 @@ func init() { }) Register(SyntaxFeature{ - Path: "odata.consume", - Features: []string{"integration/odata_client", "microflows/call_external_action", "microflows/call_external_action_arguments"}, - Summary: "Create consumed OData services and external entities", + Path: "odata.consume", + Features: []string{"integration/odata_client", "microflows/call_external_action", "microflows/call_external_action_arguments", + "microflows/call_external_action_include_associations"}, + Summary: "Create consumed OData services and external entities", Keywords: []string{ "create consumed odata service", "create odata client", "odata client", "consume odata", "external entity", "metadata url", "odata4", "headers", "proxy", "configurationmicroflow", "headersmicroflow", "service url", + "call external action", "include associations", "associations to include", }, Syntax: "CREATE CONSUMED ODATA SERVICE Module.Name (\n" + " Version: '1.0',\n" + @@ -66,6 +68,18 @@ func init() { " [INTO Module] [ENTITIES (Name1, Name2)];\n\n" + "ALTER ENTITY Module.Name SET (AllowCreateChangeLocally: true);\n\n" + "DROP EXTERNAL ENTITY [IF EXISTS] Module.Name; -- refuses a local entity\n\n" + + "$Result = CALL EXTERNAL ACTION Module.Client.Action (\n" + + " Param = $Obj [INCLUDE ASSOCIATIONS (Module.Assoc [(Module.SubAssoc, ...)], ...)],\n" + + " Other = nothing -- a blank (optional) argument\n" + + ");\n" + + "-- INCLUDE ASSOCIATIONS is Studio Pro's \"Associations to include\": the\n" + + "-- associations whose objects are sent with the argument, as a tree. A\n" + + "-- name in ( ) after an association lists those included under it, from\n" + + "-- the entity at its far end. Which end the object is on is worked out\n" + + "-- from the domain model; a self-association is refused (tick it in\n" + + "-- Studio Pro). INCLUDE ASSOCIATIONS () removes them all. Left out on\n" + + "-- CREATE OR MODIFY, the stored tree is kept and MDL-ODATA10 warns.\n" + + "-- DESCRIBE prints the clause. Mendix 10.11+ for a non-empty clause.\n\n" + "-- `SET Key = value, ...` (no parentheses) still runs and warns MDL-DEPR061;\n" + "-- `SET allow_create_change_locally = true` warns MDL-DEPR063.", Example: "mdl 1;\nCREATE CONSTANT MyModule.SvcUrl ( Type: String, DefaultValue: 'https://api.example.com/odata/v4/' );\n\nCREATE CONSUMED ODATA SERVICE MyModule.SalesforceAPI (\n Version: '1.0',\n ODataVersion: OData4,\n MetadataUrl: 'https://api.example.com/odata/$metadata',\n Timeout: 300,\n ServiceUrl: MyModule.SvcUrl\n);\n\nCREATE EXTERNAL ENTITIES FROM MyModule.SalesforceAPI INTO Integration;", diff --git a/docs-site/src/appendixes/version-compatibility.md b/docs-site/src/appendixes/version-compatibility.md index 96a8e7a2e7..35932cbff8 100644 --- a/docs-site/src/appendixes/version-compatibility.md +++ b/docs-site/src/appendixes/version-compatibility.md @@ -66,7 +66,7 @@ The tables below show exactly which features are available on each Mendix versio | Show page with arguments | `SHOW PAGE Module.Page (Param = $Value)` | 9.7+ | Yes | Yes | Yes | | Calls in a task queue | `CALL MICROFLOW ... IN QUEUE ... [RETRY FIXED (...)], [RETRY EXPONENTIAL (...)]`, `CALL JAVA ACTION ... IN QUEUE ... [RETRY ...]` | 9.10+ | Yes | Yes | Yes | | Find and filter by expression | `FIND $List WHERE ...`, `FILTER $List WHERE ...` | 9.0.3+ | Yes | Yes | Yes | -| Call external action | `CALL EXTERNAL ACTION Module.Service.Action (...)` | 9.19+ (with arguments: --) | Yes (with arguments: 10.2+) | Yes | Yes | +| Call external action | `CALL EXTERNAL ACTION Module.Service.Action (...)` | 9.19+ (with arguments: --) | Yes (with arguments: 10.2+; INCLUDE ASSOCIATIONS: 10.11+) | Yes | Yes | | Range of a list | `RANGE $List OFFSET ... LIMIT ...` | 9.24+ | Yes | Yes | Yes | | SEND REST REQUEST | `SEND REST REQUEST Module.Service.Op` | -- | 10.1+ | Yes | Yes | | SEND REST REQUEST arguments, result, body | `... WITH (...)`, `$R = SEND REST REQUEST ...`, `... BODY $Var` | -- | 10.2+, 10.4+, 10.5+ | Yes | Yes | diff --git a/docs-site/src/reference/odata/README.md b/docs-site/src/reference/odata/README.md index 0cf757761b..4e18444888 100644 --- a/docs-site/src/reference/odata/README.md +++ b/docs-site/src/reference/odata/README.md @@ -103,6 +103,34 @@ A re-import of a module that an earlier mxcli version imported the old way lists the associations that fail with `CE6618` as `drop association …;` lines. Run them, then run the import again. +## Calling an External Action + +```sql +$Response = call external action Odata.OrderService.CreateBuffer( + command = $Cmd include associations (Odata.Quantity (Odata.Unit)), + additional = nothing +); +``` + +`include associations ( … )` after an argument is Studio Pro's **Associations to +include**: the associations whose objects are serialised into the OData request +with that argument. It is a tree. A name followed by `( … )` lists the +associations included under it, reached from the entity at its far end. Without +the clause, a sub-object linked to the argument is not sent, and nothing fails +until the service receives the request. + +- Which end of each association the object is on is worked out from the domain + model, so the clause names only the associations. +- A self-association is refused, because its direction cannot be derived. Tick it + in Studio Pro instead. `describe` prints a tree that holds one as a comment. +- `include associations ()` removes them all. To remove some, restate the tree + without them. The clause is always the whole tree. +- Left out on `create or modify`, the stored tree is **kept**, and `check -p` and + `exec` warn with **MDL-ODATA10**. Deleting the clause does not remove the + associations. +- `describe microflow` prints the clause, so describe → exec restates the tree. +- A non-empty clause needs Mendix 10.11 or later. + ## Contract Browsing Statements Browse available assets from cached service contracts without network access. diff --git a/docs-wiki/bug-patterns/rewrite-drops-unauthored-state.md b/docs-wiki/bug-patterns/rewrite-drops-unauthored-state.md index 95192a6783..5865d54440 100644 --- a/docs-wiki/bug-patterns/rewrite-drops-unauthored-state.md +++ b/docs-wiki/bug-patterns/rewrite-drops-unauthored-state.md @@ -368,6 +368,17 @@ non-canonical units. Its additions are the gaps, valued with Mendix's own defaults. Its removals are keys the version does not declare, and those turned out to hide real behaviour bugs. +**A carry has as many sites as the statement has write paths.** A `create or +modify` of a *stored* flow is applied as an in-place splice first, and rebuilt +from scratch only when the splice cannot make the change. A carry wired into the +rebuild alone is therefore skipped in the common case. The splice's replace +rebuilds the activity through the alter-flow fragment builder, which never reaches +it. Associations to include shipped that way: the carry was tested, and the tests +passed, because the mock's stored flow had no start or end event and always fell +back to the rebuild. Before trusting a carry, list the write paths (rebuild, +splice replace, explicit `alter … replace`), and test the splice against a real +project, where the stored flow is a whole graph. + ## See also - [fix-issue findings](../../.claude/skills/fix-issue/findings/) — the individual diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 2d7c45f063..27d7bf4284 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -869,9 +869,9 @@ Modify an existing workflow's properties, activities, outcomes, paths, condition | Insert decision outcome | `insert into activity { outcomes 'Module.Enum.Value' -> { body } }` (or `true`, `false`, `default`) | Decision or call microflow | | Insert path | `insert into activity { path { body } }` | Parallel split; `path n` must be the next number | | Insert boundary event | `insert into activity { boundary event interrupting timer [caption ''] { body } }` | Boundary timer | -| Drop outcome | `drop activity outcome 'name';` / `drop activity outcome true;` | `true`, `false`, `default` for a decision's Boolean or default outcome | +| Drop outcome | `drop activity outcome 'name';` / `drop activity outcome true;` | `true`, `false`, `default` for a decision's Boolean or default outcome; refused as ambiguous when two outcomes have that value | | Drop path | `drop activity path 2;` | Parallel split path by number, as `describe` numbers them; refused on any other activity (a user task's branches are its outcomes) | -| Drop boundary event | `drop activity boundary event;` | For an activity with one boundary event; with several, refused under `mdl 1` and the first dropped with warning MDL-V1-BOUNDARYDROP under `mdl 0` | +| Drop boundary event | `drop activity boundary event [n];` | `n` picks one by number, as `describe` lists them. Without it: for an activity with one boundary event; with several, refused under `mdl 1` and the first dropped with warning MDL-V1-BOUNDARYDROP under `mdl 0` | **Activity references** are names (`ReviewOrder`) or captions in quotes (`'Review the order'`). Add `@n` to choose one of several matches (`ACT_Process@2`); without it a name wins over a caption that repeats it, and an ambiguous reference is refused with the matches listed. Every target is resolved before anything changes. diff --git a/docs/11-proposals/PROPOSAL_external_action_include_associations.md b/docs/11-proposals/PROPOSAL_external_action_include_associations.md index bf8a32c5ce..d9035a2a40 100644 --- a/docs/11-proposals/PROPOSAL_external_action_include_associations.md +++ b/docs/11-proposals/PROPOSAL_external_action_include_associations.md @@ -1,11 +1,11 @@ --- title: Associations to Include on a Call External Action Argument -status: draft +status: implemented --- # Proposal: Associations to Include on a Call External Action Argument -**Status:** Draft +**Status:** Implemented **Date:** 2026-11-03 **Issue:** ako/mxcli#1914 (mendixlabs/mxcli#1721) diff --git a/mdl-examples/bug-tests/1644-alter-set-column-attribute.mdl b/mdl-examples/bug-tests/1644-alter-set-column-attribute.mdl new file mode 100644 index 0000000000..001cc98ce1 --- /dev/null +++ b/mdl-examples/bug-tests/1644-alter-set-column-attribute.mdl @@ -0,0 +1,21 @@ +-- ako/mxcli#1644: `alter page … set (Attribute: …) on column(…)` was +-- refused for every spelling ("holds a value of kind Attribute, which ALTER +-- cannot set from a plain value"), and `Assoc/Attr` did not parse. It now binds +-- as CREATE binds a column's Attribute, against the grid's entity. Measured on a +-- fresh 11.14.0 app: all three SETs apply, describe prints them back, mx check +-- 0 errors, and describe -> exec of the page is Unchanged. +mdl 1; +create module BugTest1644; +create persistent entity BugTest1644.Customer (Name: String(100), City: String(100)); +create persistent entity BugTest1644.Order (Number: String(20), Note: String(200), Code: String(20), Ref: String(20)); +create association BugTest1644.Order_Customer from BugTest1644.Order to BugTest1644.Customer; +create page BugTest1644.OrderGrid ( Title: 'Orders', Layout: Atlas_Core.Atlas_Default ) { + datagrid dgOrders (DataSource: database from BugTest1644.Order) { + column (Attribute: Number, Caption: 'Number') + column (Attribute: Code, Caption: 'Code') + column (Attribute: Ref, Caption: 'Ref') + } +}; +alter page BugTest1644.OrderGrid { set (Attribute: Note) on dgOrders column(Number) }; +alter page BugTest1644.OrderGrid { set (Attribute: BugTest1644.Order.Number) on dgOrders column(Code) }; +alter page BugTest1644.OrderGrid { set (Attribute: Order_Customer/City) on dgOrders column(Ref) }; diff --git a/mdl-examples/bug-tests/1914-external-action-include-associations.mdl b/mdl-examples/bug-tests/1914-external-action-include-associations.mdl new file mode 100644 index 0000000000..d009b7f2cc --- /dev/null +++ b/mdl-examples/bug-tests/1914-external-action-include-associations.mdl @@ -0,0 +1,16 @@ +-- ako/mxcli#1914 (mendixlabs/mxcli#1721): a call external action argument's +-- Associations to include had no MDL, so a sub-DTO linked to the command was +-- not sent and nothing failed until the service received the request. +-- Measured on a fresh 11.12.6 app with the reporter's service shape (command -> +-- Quantity -> Unit, imported with `create external entities`): the call below +-- builds at 0 errors and `describe` prints the clause back; leaving the clause +-- out of a later `create or modify` keeps the tree and warns MDL-ODATA10; +-- `include associations ()` clears it. Run against a project holding the +-- BugTest1914.Bug1914 service and its imported entities. +mdl 1; +create or modify microflow BugTest1914.ACT_CreateBuffer ($Cmd: BugTest1914.CreateBufferCommand) +begin + call external action BugTest1914.Bug1914.CreateBuffer( + command = $Cmd include associations (BugTest1914.Quantity (BugTest1914.Unit_2)) + ); +end; diff --git a/mdl-examples/bug-tests/1938-array-rooted-mapping-returns-one-object.mdl b/mdl-examples/bug-tests/1938-array-rooted-mapping-returns-one-object.mdl new file mode 100644 index 0000000000..19206e1002 --- /dev/null +++ b/mdl-examples/bug-tests/1938-array-rooted-mapping-returns-one-object.mdl @@ -0,0 +1,42 @@ +-- ako/mxcli#1938 (mendixlabs/mxcli#1734): an import mapping over an ARRAY-rooted JSON +-- structure that maps the array itself to its root entity (the items a child) +-- returns ONE root object, but the import activity was written as returning a +-- list: mx check CE0243 "The mapping used to return a value of type +-- 'List of BugTest1938.Root', but now returns a value of type 'BugTest1938.Root'". +-- Measured on a fresh 11.14.0 app: the base build gives 1 x CE0243 on the first +-- create (the reporter saw it on re-execution); this build 0, on create and +-- after re-executing the microflow with a renamed result variable. +mdl 1; +create module BugTest1938; +create or modify json structure BugTest1938.JSON_structure_2 + sample $$[ + { + "curveNumber": 3, + "pointNumber": 5, + "pointIndex": 5, + "x": "2024-04-15", + "y": 1 + } +]$$; +create non-persistent entity BugTest1938.Root (); +create non-persistent entity BugTest1938.JsonObject (CurveNumber: Integer, PointNumber: Integer, PointIndex: Integer, X: String(200), Y: Integer); +create association BugTest1938.JsonObject_Root from BugTest1938.JsonObject to BugTest1938.Root; +create or modify import mapping BugTest1938.Import_mapping_2 + with json structure BugTest1938.JSON_structure_2 +{ + create BugTest1938.Root { + create BugTest1938.JsonObject_Root/BugTest1938.JsonObject = JsonObject { + CurveNumber = curveNumber, + PointNumber = pointNumber, + PointIndex = pointIndex, + X = x, + Y = y + } + } +}; +create or modify microflow BugTest1938.ACT_import () +begin + declare $Variable String = ''; + $Root = import from mapping BugTest1938.Import_mapping_2($Variable); + return; +end; diff --git a/mdl-examples/bug-tests/1963-empty-then-decision-round-trip.mdl b/mdl-examples/bug-tests/1963-empty-then-decision-round-trip.mdl new file mode 100644 index 0000000000..4c487778c7 --- /dev/null +++ b/mdl-examples/bug-tests/1963-empty-then-decision-round-trip.mdl @@ -0,0 +1,21 @@ +-- ako/mxcli#1963 (mendixlabs/mxcli#1742): describe printed a decision whose +-- true branch is empty as `if not() then end if;`, +-- and executing that description stored not() with the outcomes +-- swapped — the same behaviour, a different model. describe now prints the +-- decision as stored, `if then else end if;`, so +-- describe -> exec keeps the expression and the outcomes. +-- Run, describe, execute the description under a new name: both decisions +-- store `$Value != empty`, true -> merge, false -> the change. +mdl 1; +create module BugTest1963; +create microflow BugTest1963.Repro_DecisionNegation ($Value: String) +returns Boolean +begin + declare $Valid Boolean = true; + @caption 'Value filled?' + if $Value != empty then + else + set $Valid = false; + end if; + return $Valid; +end; diff --git a/mdl-examples/bug-tests/1965-custom-date-format-set-with-pattern.mdl b/mdl-examples/bug-tests/1965-custom-date-format-set-with-pattern.mdl new file mode 100644 index 0000000000..3cb00147c1 --- /dev/null +++ b/mdl-examples/bug-tests/1965-custom-date-format-set-with-pattern.mdl @@ -0,0 +1,11 @@ +-- ako/mxcli#1965 (mendixlabs/mxcli#1743): setting a date picker to +-- `DateFormat: Custom` with its pattern was refused by check (and so by +-- exec's pre-check) in either order and as two SETs, telling the author to +-- write the very statement they had written; --no-check wrote it correctly and +-- mx check passed. check now judges the widget as the statement leaves it. +-- Run against a project holding the page below (create it first): check and +-- exec accept all three; `set (DateFormat: Custom)` alone is still CE0493. +mdl 1; +alter page BugTest1965.Booking_Edit { set (DateFormat: Custom, CustomDateFormat: 'dd/MM/yyyy') on dpA }; +alter page BugTest1965.Booking_Edit { set (CustomDateFormat: 'dd/MM/yyyy', DateFormat: Custom) on dpB }; +alter page BugTest1965.Booking_Edit { set (CustomDateFormat: 'HH:mm') on dpC; set (DateFormat: Custom) on dpC }; diff --git a/mdl-examples/bug-tests/1966-alter-page-set-template-params.mdl b/mdl-examples/bug-tests/1966-alter-page-set-template-params.mdl new file mode 100644 index 0000000000..4406282a66 --- /dev/null +++ b/mdl-examples/bug-tests/1966-alter-page-set-template-params.mdl @@ -0,0 +1,10 @@ +-- ako/mxcli#1966 (mendixlabs/mxcli#1744): `alter page … set (CaptionParams: +-- (…))` / `set (ContentParams: (…))` was a parse error, so a text template's +-- parameters could be written by create page and replace but not changed in +-- place. Both now parse, resolve a bare attribute against the data view's +-- entity as create page does, and describe back. Measured on a fresh 11.14.0 +-- app with the reporter's pages: check and exec pass, mx check 0 errors, and +-- the parameter written is the one create page writes, $IDs aside. +mdl 1; +alter page BugTest1966.PageD { set (Caption: 'Open draft (version {1})', CaptionParams: ({1} = VersionNo)) on btnD }; +alter page BugTest1966.PageE { set (Content: 'Draft {1}', ContentParams: ({1} = VersionNo)) on txtE }; diff --git a/mdl-examples/bug-tests/1967-unset-datasource-text-template-null.mdl b/mdl-examples/bug-tests/1967-unset-datasource-text-template-null.mdl new file mode 100644 index 0000000000..e023734670 --- /dev/null +++ b/mdl-examples/bug-tests/1967-unset-datasource-text-template-null.mdl @@ -0,0 +1,27 @@ +-- ako/mxcli#1967 (mendixlabs/mxcli#1745): a pluggable widget's textTemplate +-- property bound to an OPTIONAL data source (widget.xml `dataSource="X"`) was +-- written as an empty Forms$ClientTemplate while X was unset; Studio Pro stores +-- null there, and mx check reports CE0463 "The definition of this widget has +-- changed". With X set, an unset template stays empty (the control, PageC). +-- +-- Measured on a fresh 11.14.0 app with a probe widget declaring the reporter's +-- two pairs (logicItems/logicText, accessColumns/accessColumnTitle, all +-- optional): the base build gives 3 x CE0463, one per page; this build 0. +-- Needs that widget installed and extracted (`mxcli widget extract --mpk`). +mdl 1; +create module BugTest1967; +create non-persistent entity BugTest1967.Rule (Words: String(200)); +create microflow BugTest1967.DS_Rules () returns list of BugTest1967.Rule as $L +begin + $L = create list of BugTest1967.Rule; + return $L; +end; +create page BugTest1967.PageA ( Title: 'A', Layout: Atlas_Core.Atlas_Default ) { + formprobe fcA () +}; +create page BugTest1967.PageB ( Title: 'B', Layout: Atlas_Core.Atlas_Default ) { + formprobe fcB (logicItems: microflow BugTest1967.DS_Rules, logicText: '{1}', logicTextParams: ({1} = $currentObject/Words)) +}; +create page BugTest1967.PageC ( Title: 'C', Layout: Atlas_Core.Atlas_Default ) { + formprobe fcC (logicItems: microflow BugTest1967.DS_Rules) +}; diff --git a/mdl-examples/bug-tests/1968-snippetcall-dataview-argument-widget-slot.mdl b/mdl-examples/bug-tests/1968-snippetcall-dataview-argument-widget-slot.mdl new file mode 100644 index 0000000000..cd9e4f83bd --- /dev/null +++ b/mdl-examples/bug-tests/1968-snippetcall-dataview-argument-widget-slot.mdl @@ -0,0 +1,32 @@ +-- ako/mxcli#1968 (mendixlabs/mxcli#1746): a snippet call passing a data view's +-- object by the data view's name (`Params: (Item = $dvOuter)`) was written as a +-- page-parameter mapping (PageParameter = 'dvOuter'), naming a page parameter +-- the page does not have. Measured on a fresh 11.14.0 app: the base build gives +-- 2 x CE0115 "The arguments that are passed to snippet … need to be refreshed" +-- (scA, scC); this build writes the Widget slot, 0 errors, and describe -> exec +-- of PageA is Unchanged. (11.12.6 does not report CE0115 here.) +mdl 1; +create module BugTest1968; +create persistent entity BugTest1968.Item (Name: String(50)); +create persistent entity BugTest1968.Note (Text: String(200)); +create association BugTest1968.Note_Item from BugTest1968.Note to BugTest1968.Item; +create snippet BugTest1968.SN_Item ( Params: ( $Item: BugTest1968.Item ) ) { + dataview dvSnip (DataSource: $Item) { textbox tbSnip (Label: 'Name', Attribute: Name) } +}; +create page BugTest1968.PageA ( Title: 'A', Layout: Atlas_Core.PopupLayout, Params: ( $Note: BugTest1968.Note ) ) { + dataview dvOuter (DataSource: $Note/BugTest1968.Note_Item) { + dataview dvInner (DataSource: $Note) { + snippetcall scA (Snippet: BugTest1968.SN_Item, Params: (Item = $dvOuter)) + } + } +}; +create page BugTest1968.PageB ( Title: 'B', Layout: Atlas_Core.PopupLayout, Params: ( $Note: BugTest1968.Note ) ) { + dataview dvOuterB (DataSource: $Note/BugTest1968.Note_Item) { + snippetcall scB (Snippet: BugTest1968.SN_Item, Params: (Item = $currentObject)) + } +}; +create page BugTest1968.PageC ( Title: 'C', Layout: Atlas_Core.PopupLayout, Params: ( $Note: BugTest1968.Note ) ) { + dataview dvOuterC (DataSource: $Note/BugTest1968.Note_Item) { + snippetcall scC (Snippet: BugTest1968.SN_Item, Params: (Item = $dvOuterC)) + } +}; diff --git a/mdl-examples/bug-tests/1977-odata-vocabulary-alias-annotations.mdl b/mdl-examples/bug-tests/1977-odata-vocabulary-alias-annotations.mdl new file mode 100644 index 0000000000..4a6f2c757e --- /dev/null +++ b/mdl-examples/bug-tests/1977-odata-vocabulary-alias-annotations.mdl @@ -0,0 +1,19 @@ +-- ako/mxcli#1977: a $metadata that aliases a vocabulary +-- +-- and writes its terms with the alias (Term="Capabilities.InsertRestrictions") +-- had those annotations ignored: the entity was imported as not creatable or +-- deletable. Measured on a fresh 11.14.0 app with the contract next to the +-- project as alias1977.xml (one entity set, Insert/Update/DeleteRestrictions +-- all true, aliased): the base build gives 6 errors (4 x CE6630, CE0852, +-- CE9435), this build 0. +mdl 1; +create module BugTest1977; +create constant BugTest1977.SvcUrl ( Type: String, DefaultValue: 'https://example.com/odata/' ); +create consumed odata service BugTest1977.OrderService ( Version: '1.0', ODataVersion: OData4, MetadataUrl: 'alias1977.xml', Timeout: 300, ServiceUrl: BugTest1977.SvcUrl ); +create external entities from BugTest1977.OrderService into BugTest1977; +create microflow BugTest1977.ACT_Save ($Order: BugTest1977.Order) +begin + change $Order (Number = 'x'); + send external object $Order; + return; +end; diff --git a/mdl-examples/bug-tests/808-drop-boundary-event-by-number.mdl b/mdl-examples/bug-tests/808-drop-boundary-event-by-number.mdl new file mode 100644 index 0000000000..5d4a37a19f --- /dev/null +++ b/mdl-examples/bug-tests/808-drop-boundary-event-by-number.mdl @@ -0,0 +1,43 @@ +mdl 1; +-- ============================================================================ +-- Regression: ako/mxcli#808 - a boundary event could not be addressed +-- ============================================================================ +-- +-- Symptom (before fix): +-- `drop X boundary event` named no event. On an activity with several, the +-- mutators dropped the first stored one, a guess at which the author meant; +-- under mdl 1 that was refused (MDL-V1-BOUNDARYDROP) and there was no way +-- to drop the second at all. +-- +-- After fix: +-- `drop X boundary event N` drops the N-th, numbered as `describe workflow` +-- lists them; a number past the last is refused with nothing written. +-- Expected below: callChild keeps only its interrupting timer. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/808-drop-boundary-event-by-number.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe workflow BugTest808.WF_Parent" +-- mxcli docker check -p app.mpr -- expect 0 errors +-- ============================================================================ + +create module BugTest808; +create persistent entity BugTest808.Item ( Name: String(100) ); + +create workflow BugTest808.WF_Child + parameter $WorkflowContext: BugTest808.Item +begin + wait for timer childWait addDays([%CurrentDateTime%], 1); +end workflow; + +create workflow BugTest808.WF_Parent + parameter $WorkflowContext: BugTest808.Item +begin + call workflow BugTest808.WF_Child(WorkflowContext = $WorkflowContext) as callChild + boundary event interrupting timer addDays([%CurrentDateTime%], 1) { end workflow; } + boundary event non interrupting timer addHours([%CurrentDateTime%], 4) { + wait for timer remindWait addHours([%CurrentDateTime%], 1); + }; +end workflow; + +-- The second event, the non-interrupting reminder, is the one to go. +alter workflow BugTest808.WF_Parent { drop callChild boundary event 2 }; diff --git a/mdl-examples/bug-tests/image-reference-case-sensitive.mdl b/mdl-examples/bug-tests/image-reference-case-sensitive.mdl new file mode 100644 index 0000000000..39188134dc --- /dev/null +++ b/mdl-examples/bug-tests/image-reference-case-sensitive.mdl @@ -0,0 +1,31 @@ +mdl 1; + +/** + * The image reference check folded case: on a blank 11.13.0 app, whose + * Atlas_Core.Content holds the image `Mendix`, `mxcli check --references` + * passed all three wrong-case references below, and `mxcli docker check` + * then reported each as CE1613 "The selected image '…' no longer exists". + * The miss hint also printed the image as `mendix`, so following it produced + * the CE1613. + * + * Expected (run with `-p app.mpr --references`): ImgOk passes; each of + * ImgCase, CollCase and ModCase is a reference error naming the stored + * spelling (Atlas_Core.Content.Mendix / Atlas_Core.Content). This file is not + * .fail.mdl: without a project `check` cannot resolve references and passes. + */ + +create page MyFirstModule.ImgOk (Title: 'Ok', Layout: Atlas_Core.Atlas_Default) { + image imgOk (Image: 'Atlas_Core.Content.Mendix') +}; + +create page MyFirstModule.ImgCase (Title: 'Image', Layout: Atlas_Core.Atlas_Default) { + image imgCase (Image: 'Atlas_Core.Content.mendix') +}; + +create page MyFirstModule.CollCase (Title: 'Collection', Layout: Atlas_Core.Atlas_Default) { + image imgColl (Image: 'Atlas_Core.content.Mendix') +}; + +create page MyFirstModule.ModCase (Title: 'Module', Layout: Atlas_Core.Atlas_Default) { + image imgMod (Image: 'atlas_core.Content.Mendix') +}; diff --git a/mdl/ast/ast_microflow.go b/mdl/ast/ast_microflow.go index ee7e7add8c..ee241679d8 100644 --- a/mdl/ast/ast_microflow.go +++ b/mdl/ast/ast_microflow.go @@ -763,6 +763,24 @@ type CallArgument struct { // a Java action's text-template parameter: `Subject = 'Order {1}' with // ({1} = $Order/Number)` (ako/mxcli#1451). Only call java action takes it. TemplateParams []TemplateParam + // IncludeAssociations is a call external action argument's Associations to + // include, `include associations ( … )` (ako/mxcli#1914). nil when the + // clause is not written; a non-nil empty clause states "none". + IncludeAssociations *IncludeAssociationsClause +} + +// IncludeAssociationsClause is `include associations (M.A (M.B), M.C)`: the +// tree of associations whose objects an OData external action call serialises +// with the argument. Each node's children are reached from the entity at its +// association's far end. +type IncludeAssociationsClause struct { + Associations []*IncludedAssociationDef +} + +// IncludedAssociationDef is one node of the tree. +type IncludedAssociationDef struct { + Name QualifiedName + Children []*IncludedAssociationDef } // QueueRetry is the retry policy of a queued call (mendixlabs/mxcli#1729): diff --git a/mdl/ast/ast_workflow.go b/mdl/ast/ast_workflow.go index 383ed8ddfe..d17672446e 100644 --- a/mdl/ast/ast_workflow.go +++ b/mdl/ast/ast_workflow.go @@ -475,6 +475,9 @@ func (o *InsertBoundaryEventOp) alterWorkflowOp() {} type DropBoundaryEventOp struct { ActivityRef string AtPosition int + // Index is `boundary event N`: the N-th event, 1-based, in describe order; + // 0 when the statement names none (ako/mxcli#808). + Index int } func (o *DropBoundaryEventOp) alterWorkflowOp() {} diff --git a/mdl/backend/mcp/widget.go b/mdl/backend/mcp/widget.go index 9e25487f2a..243970a11e 100644 --- a/mdl/backend/mcp/widget.go +++ b/mdl/backend/mcp/widget.go @@ -573,6 +573,8 @@ func (w *mcpWidgetBuilder) PrimitiveValues() map[string]string { return nil } func (w *mcpWidgetBuilder) EnsureRequiredObjectLists() {} func (w *mcpWidgetBuilder) ApplyPropertyVisibility(_ []types.WidgetVisibilityRule) {} +func (w *mcpWidgetBuilder) NullTemplatesOfUnsetDataSources() {} + // Finalize builds the CustomWidget shell and registers its recorded pg object on // the backend, keyed by the widget's ID so mapPageWidget can find it. func (w *mcpWidgetBuilder) Finalize(id model.ID, name, label, editable string) *pages.CustomWidget { diff --git a/mdl/backend/mcp/workflow.go b/mdl/backend/mcp/workflow.go index 793e163cb6..135049e9cf 100644 --- a/mdl/backend/mcp/workflow.go +++ b/mdl/backend/mcp/workflow.go @@ -2095,7 +2095,7 @@ func (m *mcpWorkflowMutator) InsertBoundaryEvent(activityRef string, atPos int, } // DropBoundaryEvent removes the activity's (first) boundary event. -func (m *mcpWorkflowMutator) DropBoundaryEvent(activityRef string, atPos int) error { +func (m *mcpWorkflowMutator) DropBoundaryEvent(activityRef string, atPos int, index int) error { loc, err := m.resolve(activityRef, atPos) if err != nil { return err @@ -2108,7 +2108,11 @@ func (m *mcpWorkflowMutator) DropBoundaryEvent(activityRef string, atPos int) er if len(events) == 0 { return fmt.Errorf("activity %q has no boundary events", activityRef) } - return m.apply(removeAtOp(actPath+"/boundaryEvents", 0)) + at, err := backend.BoundaryEventIndex(activityRef, index, len(events)) + if err != nil { + return err + } + return m.apply(removeAtOp(actPath+"/boundaryEvents", at)) } // --- shared helpers for nested-array ops --- @@ -2176,12 +2180,24 @@ func (m *mcpWorkflowMutator) dropFromActivityArray(actPath, field, activityRef, if err != nil { return err } + // A value does not identify an outcome uniquely, so a second match makes the + // address a guess and the drop is refused, as in wfmutator (ako/mxcli#808). + at, n := -1, 0 for i, e := range elems { if match(e) { - return m.apply(removeAtOp(actPath+"/"+field, i)) + if at < 0 { + at = i + } + n++ } } - return fmt.Errorf("%s not found on activity %q", kind, activityRef) + switch { + case n == 0: + return fmt.Errorf("%s not found on activity %q", kind, activityRef) + case n > 1: + return fmt.Errorf("%s on activity %q is ambiguous: %d outcomes match. Nothing was written", kind, activityRef, n) + } + return m.apply(removeAtOp(actPath+"/"+field, at)) } // readActivityArray reads an activity's nested array (outcomes, boundaryEvents) diff --git a/mdl/backend/mcp/workflow_test.go b/mdl/backend/mcp/workflow_test.go index 69014c88b4..0250c85135 100644 --- a/mdl/backend/mcp/workflow_test.go +++ b/mdl/backend/mcp/workflow_test.go @@ -544,7 +544,7 @@ func TestWFBoundaryEvent(t *testing.T) { } // DROP removes index 0. f2, m2 := wfMutatorFake(t) - if err := m2.DropBoundaryEvent("ReviewOrder", 0); err != nil { + if err := m2.DropBoundaryEvent("ReviewOrder", 0, 0); err != nil { t.Fatal(err) } if ops := wfUpdateOps(t, f2); !strings.Contains(ops, `"path":"/flow/activities/0/boundaryEvents"`) || !strings.Contains(ops, `"index":0`) { diff --git a/mdl/backend/mock/mock_workflow_mutator.go b/mdl/backend/mock/mock_workflow_mutator.go index 5a3401df70..96a66d3eba 100644 --- a/mdl/backend/mock/mock_workflow_mutator.go +++ b/mdl/backend/mock/mock_workflow_mutator.go @@ -29,7 +29,7 @@ type MockWorkflowMutator struct { DropBranchFunc func(activityRef string, atPos int, branchName string) error SetTemplateParametersFunc func(activityRef string, atPos int, prop string, params []string) error InsertBoundaryEventFunc func(activityRef string, atPos int, eventType string, delay string, caption string, recurrence *workflows.Recurrence, activities []workflows.WorkflowActivity) error - DropBoundaryEventFunc func(activityRef string, atPos int) error + DropBoundaryEventFunc func(activityRef string, atPos int, index int) error SaveFunc func() error } @@ -141,9 +141,9 @@ func (m *MockWorkflowMutator) InsertBoundaryEvent(activityRef string, atPos int, return nil } -func (m *MockWorkflowMutator) DropBoundaryEvent(activityRef string, atPos int) error { +func (m *MockWorkflowMutator) DropBoundaryEvent(activityRef string, atPos int, index int) error { if m.DropBoundaryEventFunc != nil { - return m.DropBoundaryEventFunc(activityRef, atPos) + return m.DropBoundaryEventFunc(activityRef, atPos, index) } return nil } diff --git a/mdl/backend/modelsdk/microflow_external_action_include_clause_test.go b/mdl/backend/modelsdk/microflow_external_action_include_clause_test.go new file mode 100644 index 0000000000..bf8d4b6c87 --- /dev/null +++ b/mdl/backend/modelsdk/microflow_external_action_include_clause_test.go @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "reflect" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/modelsdk/codec" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// ako/mxcli#1914: the tree `include associations (Odata.Quantity (Odata.Unit))` +// builds — fresh $IDs, IsParent derived from the domain model — is written as +// the bytes Studio Pro 11.16 stored for the same ticks on ako/TestApp's +// Odata.IncludeAssociationsMF, $IDs aside. The stored tree is also the source +// of the builder test's expectation: both nodes IsParent, Unit under Quantity. +func TestCallExternalAction_AuthoredTreeMatchesStudioPro(t *testing.T) { + el, stored := storedExternalAction(t, includeAssocFixture) + action := actionFromGen(el).(*microflows.CallExternalAction) + + var cmd *microflows.ExternalActionParameterMapping + for _, pm := range action.ParameterMappings { + if pm.ParameterName == "command" { + cmd = pm + } + } + if cmd == nil || len(cmd.IncludedAssociations) != 1 { + t.Fatalf("fixture: no command tree") + } + qty := cmd.IncludedAssociations[0] + if qty.Association != "Odata.Quantity" || !qty.IsParent || len(qty.IncludedAssociations) != 1 || + qty.IncludedAssociations[0].Association != "Odata.Unit" || !qty.IncludedAssociations[0].IsParent { + t.Fatalf("fixture tree is not Quantity(IsParent) > Unit(IsParent): %+v", qty) + } + + // What the builder writes for the clause. + node := func(name string, children ...*microflows.IncludedAssociation) *microflows.IncludedAssociation { + return µflows.IncludedAssociation{ + BaseElement: model.BaseElement{ID: model.ID("authored-" + name)}, Association: name, IsParent: true, + IncludedAssociations: children, + } + } + cmd.IncludedAssociations = []*microflows.IncludedAssociation{node("Odata.Quantity", node("Odata.Unit"))} + + out, err := (&codec.Encoder{}).Encode(microflowActionToGen(action)) + if err != nil { + t.Fatalf("encode: %v", err) + } + want := stripIDs(includedAssociationsByParameter(t, stored)) + got := stripIDs(includedAssociationsByParameter(t, bson.Raw(out))) + if !reflect.DeepEqual(got, want) { + t.Errorf("authored tree written as\n %v\nStudio Pro stored\n %v", got, want) + } +} + +// stripIDs drops every $ID from a decoded BSON value. +func stripIDs(v any) any { + switch x := v.(type) { + case map[string]any: + out := map[string]any{} + for k, e := range x { + if k != "$ID" { + out[k] = stripIDs(e) + } + } + return out + case bson.M: + return stripIDs(map[string]any(x)) + case bson.D: + m := map[string]any{} + for _, e := range x { + m[e.Key] = e.Value + } + return stripIDs(m) + case bson.A: + out := make([]any, len(x)) + for i, e := range x { + out[i] = stripIDs(e) + } + return out + } + return v +} diff --git a/mdl/backend/modelsdk/widget_scoped_variable_test.go b/mdl/backend/modelsdk/widget_scoped_variable_test.go index 9eea9d7f78..2fdd78e81b 100644 --- a/mdl/backend/modelsdk/widget_scoped_variable_test.go +++ b/mdl/backend/modelsdk/widget_scoped_variable_test.go @@ -149,3 +149,34 @@ func TestSnippetCallMappingLocalVariableSlot(t *testing.T) { } } } + +// ako/mxcli#1968: a data view's object passed by naming the data view is the +// Widget slot alone; in PageParameter it named a page parameter that does not +// exist, and mx check 11.14 reported CE0115. +func TestSnippetCallMappingWidgetSlot(t *testing.T) { + sc := &pages.SnippetCallWidget{ + SnippetName: "M.Snip", + ParameterMappings: []pages.SnippetParamMapping{ + {ParamName: "Item", Argument: "$dvOuter", IsWidget: true}, + {ParamName: "Car", Argument: "$Car"}, + }, + } + sc.Name = "sc1" + call, _ := docGet(encodeWidget(t, sc), "FormCall").(bson.D) + mappings, _ := docGet(call, "ParameterMappings").(bson.A) + if len(mappings) != 3 { + t.Fatalf("ParameterMappings = %#v, want [marker, 2 mappings]", mappings) + } + for i, want := range []map[string]any{ + {"Widget": "dvOuter", "PageParameter": "", "SnippetParameter": "", "LocalVariable": ""}, + {"Widget": "", "PageParameter": "Car"}, // control + } { + m, _ := mappings[i+1].(bson.D) + got := pageVariableSlots(t, docGet(m, "Variable")) + for k, v := range want { + if got[k] != v { + t.Errorf("mapping %d Variable.%s = %v, want %q", i+1, k, got[k], v) + } + } + } +} diff --git a/mdl/backend/modelsdk/widget_write.go b/mdl/backend/modelsdk/widget_write.go index c0351981d0..5b252369be 100644 --- a/mdl/backend/modelsdk/widget_write.go +++ b/mdl/backend/modelsdk/widget_write.go @@ -1038,6 +1038,14 @@ func widgetToGen(w pages.Widget) (element.Element, error) { // A snippet's own parameter, passed on from inside that snippet, // fills the SnippetParameter slot (ako/mxcli#721 L3). // A page `Variables:` entry fills LocalVariable (ako/mxcli#826). + // A data view's object, passed by naming the data view, fills + // Widget alone, as a flow call's widget argument does + // (ako/mxcli#1968); in PageParameter it was CE0115. + if pm.IsWidget { + m.SetVariable(pageVariableToGen(strings.TrimPrefix(pm.Argument, "$"), "", "")) + call.AddParameterMappings(m) + continue + } kind := "" switch { case pm.IsSnippetParameter: diff --git a/mdl/backend/mutation.go b/mdl/backend/mutation.go index fb0e7bfd5b..1c9271fc77 100644 --- a/mdl/backend/mutation.go +++ b/mdl/backend/mutation.go @@ -351,7 +351,9 @@ type WorkflowMutator interface { InsertBoundaryEvent(activityRef string, atPos int, eventType string, delay string, caption string, recurrence *workflows.Recurrence, activities []workflows.WorkflowActivity) error // DropBoundaryEvent removes the boundary event from the referenced activity. - DropBoundaryEvent(activityRef string, atPos int) error + // DropBoundaryEvent removes an activity's boundary event: the index-th + // (1-based, describe order), or the first when index is 0. + DropBoundaryEvent(activityRef string, atPos int, index int) error // Save persists the mutations to the backend. Save() error @@ -443,6 +445,11 @@ type WidgetObjectBuilder interface { // values, matching Studio Pro's editorConfig.js-driven behavior (#574). ApplyPropertyVisibility(rules []types.WidgetVisibilityRule) + // NullTemplatesOfUnsetDataSources stores TextTemplate: null for an unset + // TextTemplate bound to a data source property that holds none, as Studio + // Pro does (ako/mxcli#1967); an empty template there is CE0463. + NullTemplatesOfUnsetDataSources() + // --- Gallery-specific --- // CloneGallerySelectionProperty clones the itemSelection property with a new Selection value. @@ -529,6 +536,10 @@ type PluggablePropertyScoper interface { type AttributeBinding struct { Path string SourceVariable *pages.WidgetVariable + // Steps are the association hops of an attribute reached over + // associations (`Assoc/Attr`), written as the AttributeRef's EntityRef as + // CREATE writes a multi-hop column (ako/mxcli#1644). + Steps []pages.AttributeRefStep } // ObjectListItemSpec describes one item of an object-list property (e.g. one diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index 0ed44e0885..5d3856822e 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -2844,6 +2844,11 @@ func setColumnPropertyMut(colDoc bson.D, propKeyMap map[string]string, propKindM return fmt.Errorf("column property %q has no Value", propName) } + // A column's attribute, resolved by the executor against the grid's + // data source as CREATE resolves it (ako/mxcli#1644). + if b, ok := value.(backend.AttributeBinding); ok { + return setColumnAttributeBinding(valDoc, propName, propKindMap[typePointerID], b) + } field, settable := columnValueField(propKindMap[typePointerID]) if !settable { return fmt.Errorf( @@ -3590,13 +3595,12 @@ func setWidgetFormattingMut(widget bson.D, propName string, value any) error { if !isString || !ok { return fmt.Errorf("DateFormat is one of Date, Time, DateTime, Custom, not %v", value) } - // mxbuild: CE0493 "Date format is custom but no format string is - // specified." Set the pattern in the same statement — properties are - // applied in name order, so CustomDateFormat lands first. - if canon == "Custom" && strings.TrimSpace(bsonnav.DGetString(fi, "CustomDateFormat")) == "" { - return fmt.Errorf("`DateFormat: Custom` needs a pattern: set CustomDateFormat in the same statement, " + - "e.g. `set (DateFormat: Custom, CustomDateFormat: 'dd-MM-yyyy HH:mm') on …` (mxbuild CE0493)") - } + // mxbuild CE0493 "Date format is custom but no format string is + // specified" is judged on the widget the whole statement leaves + // (CheckDateFormat, after the SET; check does the same), not here: + // this sees one property, and refusing on the stored pattern refused + // `set (DateFormat: Custom, CustomDateFormat: …)` in check's + // one-property dry run (ako/mxcli#1965). bsonnav.DSet(fi, "DateFormat", canon) case "CustomDateFormat": s, isString := value.(string) @@ -4413,3 +4417,26 @@ func (m *Mutator) PageVariableNames() []string { } return names } + +// setColumnAttributeBinding writes a Data grid 2 column's attribute: the +// AttributeRef CREATE writes for `column (Attribute: …)`, with the association +// hops as its EntityRef (ako/mxcli#1644). A column attribute reads the grid's +// items, so a SourceVariable — a page parameter's — is never written here. +func setColumnAttributeBinding(valDoc bson.D, propName, kind string, b backend.AttributeBinding) error { + if kind != "Attribute" { + return fmt.Errorf("column property %q holds a value of kind %s, not an attribute", propName, kind) + } + if strings.Count(b.Path, ".") < 2 { + return fmt.Errorf("column property %q: %q is not a qualified attribute", propName, b.Path) + } + if !hasKey(valDoc, "AttributeRef") { + return fmt.Errorf("column property %q stores no AttributeRef to set — replace the column instead", propName) + } + bsonnav.DSet(valDoc, "AttributeRef", bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "DomainModels$AttributeRef"}, + {Key: "Attribute", Value: b.Path}, + {Key: "EntityRef", Value: widgetobj.AttributeEntityRefBSON(b.Steps)}, + }) + return nil +} diff --git a/mdl/backend/pagemutator/template_params.go b/mdl/backend/pagemutator/template_params.go new file mode 100644 index 0000000000..1277464a1a --- /dev/null +++ b/mdl/backend/pagemutator/template_params.go @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "fmt" + "strings" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/mdl/backend/widgetobj" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// SetWidgetTemplateParameters replaces the parameters of a built-in widget's +// text template — a button's caption (CaptionParams), a dynamic text's content +// (ContentParams) — with params, resolved by the caller as CREATE PAGE resolves +// them (ako/mxcli#1966). The template's text is left as it is: a statement that +// changes both sets Caption / Content too, and properties apply in name order, +// so the text is written first. +func (m *Mutator) SetWidgetTemplateParameters(widgetRef, prop string, params []*pages.ClientTemplateParameter) error { + result := m.widgetFinder(m.rawData, widgetRef) + if result == nil { + return m.widgetNotFoundError(widgetRef) + } + if len(result.colPropKeys) > 0 { + return fmt.Errorf("%s on a data grid column is not supported by alter page set: use `replace` on the column", prop) + } + var tmpl bson.D + switch strings.ToLower(prop) { + case "captionparams": + tmpl = bsonnav.DGetDoc(result.widget, "CaptionTemplate") + case "contentparams": + tmpl = bsonnav.DGetDoc(result.widget, "Content") + if tmpl != nil && bsonnav.DGetString(tmpl, "$Type") != "Forms$ClientTemplate" { + tmpl = nil + } + } + if tmpl == nil { + return fmt.Errorf("a %s has no %s: CaptionParams are a button's caption parameters, "+ + "ContentParams a dynamic text's", widgetTypeLabel(bsonnav.DGetString(result.widget, "$Type")), prop) + } + list := bson.A{int32(2)} + for _, p := range params { + list = append(list, widgetobj.SerializeColumnClientTemplateParameter(p)) + } + bsonnav.DSet(tmpl, "Parameters", list) + return nil +} diff --git a/mdl/backend/wfmutator/mutator.go b/mdl/backend/wfmutator/mutator.go index 2dc409d3f5..4ed5d45cd2 100644 --- a/mdl/backend/wfmutator/mutator.go +++ b/mdl/backend/wfmutator/mutator.go @@ -484,36 +484,48 @@ func (m *Mutator) DropOutcome(activityRef string, atPos int, outcomeName string) } outcomes := bsonnav.DGetArrayElements(bsonnav.DGet(actDoc, "Outcomes")) - found := false - var kept []any - for _, elem := range outcomes { - oDoc, ok := elem.(bson.D) - if !ok { - kept = append(kept, elem) - continue - } + kept, err := dropOneOutcome(outcomes, "outcome", outcomeName, activityRef, func(oDoc bson.D) bool { // Only an outcome that stores a string value can be addressed by one: // a path, a void or a boolean outcome reads back as "" and would match // an empty value (found reviewing ako/mxcli#791). value, hasValue := bsonnav.DGet(oDoc, "Value").(string) - typeName := bsonnav.DGetString(oDoc, "$Type") - matched := hasValue && value == outcomeName - if !matched && strings.EqualFold(outcomeName, "Default") && typeName == "Workflows$VoidConditionOutcome" { - matched = true - } - if matched && !found { - found = true - continue + if hasValue && value == outcomeName { + return true } - kept = append(kept, elem) - } - if !found { - return fmt.Errorf("outcome %q not found on activity %q", outcomeName, activityRef) + return strings.EqualFold(outcomeName, "Default") && bsonnav.DGetString(oDoc, "$Type") == "Workflows$VoidConditionOutcome" + }) + if err != nil { + return err } bsonnav.DSetArray(actDoc, "Outcomes", kept) return nil } +// dropOneOutcome removes the single outcome that matches. A value is the only +// address an outcome has, and Mendix does not keep it unique: with two outcomes +// of the same value, dropping the first was a guess at which one was meant, so +// it is refused instead (ako/mxcli#808). +func dropOneOutcome(outcomes []any, kind, value, activityRef string, matches func(bson.D) bool) ([]any, error) { + at, n := -1, 0 + for i, elem := range outcomes { + if oDoc, ok := elem.(bson.D); ok && matches(oDoc) { + if at < 0 { + at = i + } + n++ + } + } + if n == 0 { + return nil, fmt.Errorf("%s %q not found on activity %q", kind, value, activityRef) + } + if n > 1 { + return nil, fmt.Errorf("%s %q on activity %q is ambiguous: %d outcomes have that value. Nothing was written", kind, value, activityRef, n) + } + kept := make([]any, 0, len(outcomes)-1) + kept = append(kept, outcomes[:at]...) + return append(kept, outcomes[at+1:]...), nil +} + // --------------------------------------------------------------------------- // Path operations (parallel split) // --------------------------------------------------------------------------- @@ -626,47 +638,21 @@ func (m *Mutator) DropBranch(activityRef string, atPos int, branchName string) e } outcomes := bsonnav.DGetArrayElements(bsonnav.DGet(actDoc, "Outcomes")) - found := false - var kept []any - for _, elem := range outcomes { - oDoc, ok := elem.(bson.D) - if !ok { - kept = append(kept, elem) - continue - } - if !found { - typeName := bsonnav.DGetString(oDoc, "$Type") - switch strings.ToLower(branchName) { - case "true": - if typeName == "Workflows$BooleanConditionOutcome" { - if v, ok := bsonnav.DGet(oDoc, "Value").(bool); ok && v { - found = true - continue - } - } - case "false": - if typeName == "Workflows$BooleanConditionOutcome" { - if v, ok := bsonnav.DGet(oDoc, "Value").(bool); ok && !v { - found = true - continue - } - } - case "default": - if typeName == "Workflows$VoidConditionOutcome" { - found = true - continue - } - default: - if value, ok := bsonnav.DGet(oDoc, "Value").(string); ok && value == branchName { - found = true - continue - } - } + kept, err := dropOneOutcome(outcomes, "branch", branchName, activityRef, func(oDoc bson.D) bool { + typeName := bsonnav.DGetString(oDoc, "$Type") + switch strings.ToLower(branchName) { + case "true", "false": + v, ok := bsonnav.DGet(oDoc, "Value").(bool) + return typeName == "Workflows$BooleanConditionOutcome" && ok && v == strings.EqualFold(branchName, "true") + case "default": + return typeName == "Workflows$VoidConditionOutcome" + default: + value, ok := bsonnav.DGet(oDoc, "Value").(string) + return ok && value == branchName } - kept = append(kept, elem) - } - if !found { - return fmt.Errorf("branch %q not found on activity %q", branchName, activityRef) + }) + if err != nil { + return err } bsonnav.DSetArray(actDoc, "Outcomes", kept) return nil @@ -730,7 +716,7 @@ func (m *Mutator) InsertBoundaryEvent(activityRef string, atPos int, eventType s return nil } -func (m *Mutator) DropBoundaryEvent(activityRef string, atPos int) error { +func (m *Mutator) DropBoundaryEvent(activityRef string, atPos int, index int) error { actDoc, err := m.findActivityByCaption(activityRef, atPos) if err != nil { return err @@ -740,9 +726,11 @@ func (m *Mutator) DropBoundaryEvent(activityRef string, atPos int) error { if len(events) == 0 { return fmt.Errorf("activity %q has no boundary events", activityRef) } - - // Drop the first boundary event silently. - bsonnav.DSetArray(actDoc, "BoundaryEvents", events[1:]) + at, err := backend.BoundaryEventIndex(activityRef, index, len(events)) + if err != nil { + return err + } + bsonnav.DSetArray(actDoc, "BoundaryEvents", append(append([]any{}, events[:at]...), events[at+1:]...)) return nil } diff --git a/mdl/backend/wfmutator/mutator_test.go b/mdl/backend/wfmutator/mutator_test.go index 45f4d9e8dc..3716e41514 100644 --- a/mdl/backend/wfmutator/mutator_test.go +++ b/mdl/backend/wfmutator/mutator_test.go @@ -374,7 +374,7 @@ func TestWorkflowMutator_DropBoundaryEvent_Single(t *testing.T) { act := makeWfActivityWithBoundaryEvents("Review", evt) m := newMutator(makeWorkflowDoc(act)) - if err := m.DropBoundaryEvent("Review", 0); err != nil { + if err := m.DropBoundaryEvent("Review", 0, 0); err != nil { t.Fatalf("DropBoundaryEvent failed: %v", err) } @@ -391,7 +391,7 @@ func TestWorkflowMutator_DropBoundaryEvent_Multiple(t *testing.T) { act := makeWfActivityWithBoundaryEvents("Review", evt1, evt2) m := newMutator(makeWorkflowDoc(act)) - if err := m.DropBoundaryEvent("Review", 0); err != nil { + if err := m.DropBoundaryEvent("Review", 0, 0); err != nil { t.Fatalf("DropBoundaryEvent failed: %v", err) } @@ -410,7 +410,7 @@ func TestWorkflowMutator_DropBoundaryEvent_NoEvents(t *testing.T) { act := makeWfActivityWithBoundaryEvents("Review") m := newMutator(makeWorkflowDoc(act)) - err := m.DropBoundaryEvent("Review", 0) + err := m.DropBoundaryEvent("Review", 0, 0) if err == nil { t.Fatal("Expected error when dropping from activity with no boundary events") } @@ -1553,3 +1553,50 @@ func TestWorkflowMutator_InsertBoundaryEvent_EndsWithMarker(t *testing.T) { } } } + +// ako/mxcli#808: a value is an outcome's only address and Mendix does not keep +// it unique, so two outcomes with the same value make the drop a guess. It is +// refused, and the document is left as it was. +func TestWorkflowMutator_DropOutcome_DuplicateValueRefused(t *testing.T) { + act := makeWfActivityWithOutcomes("Review", "task1", + makeOutcome("Workflows$UserTaskOutcome", "Approve"), + makeOutcome("Workflows$UserTaskOutcome", "Approve")) + m := newMutator(makeWorkflowDoc(act)) + err := m.DropOutcome("Review", 0, "Approve") + if err == nil || !strings.Contains(err.Error(), "ambiguous: 2 outcomes") { + t.Fatalf("want an ambiguity refusal, got %v", err) + } + actDoc, _ := m.findActivityByCaption("Review", 0) + if n := len(bsonnav.DGetArrayElements(bsonnav.DGet(actDoc, "Outcomes"))); n != 2 { + t.Errorf("refused drop changed the outcomes: %d left", n) + } +} + +func TestWorkflowMutator_DropBranch_DuplicateValueRefused(t *testing.T) { + act := makeWfActivityWithOutcomes("Decision", "dec1", + makeOutcome("Workflows$EnumerationValueConditionOutcome", "Active"), + makeOutcome("Workflows$EnumerationValueConditionOutcome", "Active")) + m := newMutator(makeWorkflowDoc(act)) + err := m.DropBranch("Decision", 0, "Active") + if err == nil || !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("want an ambiguity refusal, got %v", err) + } +} + +// ako/mxcli#808: `boundary event N` drops the N-th event (describe order). +func TestWorkflowMutator_DropBoundaryEvent_ByIndex(t *testing.T) { + evt1 := makeWfBoundaryEvent("Workflows$InterruptingTimerBoundaryEvent") + evt2 := makeWfBoundaryEvent("Workflows$NonInterruptingTimerBoundaryEvent") + m := newMutator(makeWorkflowDoc(makeWfActivityWithBoundaryEvents("Review", evt1, evt2))) + if err := m.DropBoundaryEvent("Review", 0, 2); err != nil { + t.Fatal(err) + } + actDoc, _ := m.findActivityByCaption("Review", 0) + events := bsonnav.DGetArrayElements(bsonnav.DGet(actDoc, "BoundaryEvents")) + if len(events) != 1 || bsonnav.DGetString(events[0].(bson.D), "$Type") != "Workflows$InterruptingTimerBoundaryEvent" { + t.Fatalf("boundary event 2 did not drop the second event: %v", events) + } + if err := m.DropBoundaryEvent("Review", 0, 2); err == nil { + t.Fatal("boundary event 2 of 1 accepted") + } +} diff --git a/mdl/backend/widgetobj/builder.go b/mdl/backend/widgetobj/builder.go index c432334753..b21885a1e5 100644 --- a/mdl/backend/widgetobj/builder.go +++ b/mdl/backend/widgetobj/builder.go @@ -752,6 +752,10 @@ func setAttributeRefField(value bson.D, attributePath string, steps []pages.Attr // an attribute over associations (one EntityRefStep per hop), or nil for an // own-entity attribute. Mirrors the codec-form attributeRefWithStepsToGen and the // legacy serializeAssociationSource EntityRef structure. +// AttributeEntityRefBSON is the AttributeRef.EntityRef an attribute over +// associations stores: its hops as an IndirectEntityRef, or nil for none. +func AttributeEntityRefBSON(steps []pages.AttributeRefStep) any { return attributeEntityRefBSON(steps) } + func attributeEntityRefBSON(steps []pages.AttributeRefStep) any { if len(steps) == 0 { return nil diff --git a/mdl/backend/widgetobj/unset_datasource_templates.go b/mdl/backend/widgetobj/unset_datasource_templates.go new file mode 100644 index 0000000000..e8b8267ab8 --- /dev/null +++ b/mdl/backend/widgetobj/unset_datasource_templates.go @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: Apache-2.0 + +package widgetobj + +import ( + "sort" + "strings" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// NullTemplatesOfUnsetDataSources stores TextTemplate: null for an unset +// TextTemplate property bound to a data source (widget.xml `dataSource="X"`) +// when X holds no data source (ako/mxcli#1967, mendixlabs/mxcli#1745). Studio +// Pro writes it that way, and `mx update-widgets` changes mxcli's empty +// Forms$ClientTemplate to null and nothing else; the empty template is CE0463 +// "The definition of this widget has changed". With X set, an unset template +// stays empty (the reporter's control), so the rule keys on X, not on the +// template. +// +// Only an UNSET template is nulled: authored text is the statement's, and is +// never dropped here. Only the widget's own properties: a binding to the +// enclosing widget's data source ("../datasource", a column's) names one that +// is always set. +func NullTemplatesOfUnsetDataSources(object bson.D, propertyTypeIDs map[string]pages.PropertyTypeIDEntry) bson.D { + values := map[string]bson.D{} + for _, elem := range object { + if elem.Key != "Properties" { + continue + } + arr, _ := elem.Value.(bson.A) + for _, item := range arr { + if prop, ok := item.(bson.D); ok { + if id := propertyTypePointerID(prop); id != "" { + values[id] = widgetValueOfProperty(prop) + } + } + } + } + valueOf := func(key string) (bson.D, bool) { + entry, ok := propertyTypeIDs[key] + if !ok { + return nil, false + } + v, ok := values[strings.ReplaceAll(entry.PropertyTypeID, "-", "")] + return v, ok && v != nil + } + var keys []string + for key, entry := range propertyTypeIDs { + ds := entry.DataSourceProperty + if !strings.EqualFold(entry.ValueType, "TextTemplate") || ds == "" || strings.Contains(ds, "/") { + continue + } + dsValue, ok := valueOf(ds) + if !ok || !bsonFieldIsNil(dsValue, "DataSource") { + continue // the data source is set, or not known: leave the template + } + if tv, ok := valueOf(key); ok && isUnsetClientTemplate(tv) { + keys = append(keys, key) + } + } + sort.Strings(keys) + for _, key := range keys { + object = updateWidgetPropertyValue(object, propertyTypeIDs, key, func(val bson.D) bson.D { + return setBSONField(val, "TextTemplate", nil) + }) + } + return object +} + +// isUnsetClientTemplate reports whether a WidgetValue's TextTemplate is an +// empty Forms$ClientTemplate: no translation in Template, and no parameters. +// null is not "unset" here — there is nothing to change. +func isUnsetClientTemplate(val bson.D) bool { + var tmpl bson.D + for _, e := range val { + if e.Key == "TextTemplate" { + tmpl, _ = e.Value.(bson.D) + } + } + if tmpl == nil { + return false + } + for _, e := range tmpl { + switch e.Key { + case "Parameters": + if arr, ok := e.Value.(bson.A); ok && len(arr) > 1 { + return false + } + case "Template": + text, _ := e.Value.(bson.D) + for _, te := range text { + if te.Key == "Items" { + if arr, ok := te.Value.(bson.A); ok && len(arr) > 1 { + return false + } + } + } + } + } + return true +} + +// NullTemplatesOfUnsetDataSources applies the rule to the object being built. +func (ob *Builder) NullTemplatesOfUnsetDataSources() { + ob.object = NullTemplatesOfUnsetDataSources(ob.object, ob.propertyTypeIDs) +} diff --git a/mdl/backend/widgetobj/unset_datasource_templates_test.go b/mdl/backend/widgetobj/unset_datasource_templates_test.go new file mode 100644 index 0000000000..f491ff8d8b --- /dev/null +++ b/mdl/backend/widgetobj/unset_datasource_templates_test.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 + +package widgetobj + +import ( + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// ako/mxcli#1967 (mendixlabs/mxcli#1745): the reporter's widget pairs +// +// +// +// +// An unset logicText whose logicItems is unset is null in Studio Pro; mxcli +// wrote an empty Forms$ClientTemplate, CE0463. With logicItems set, an unset +// logicText stays an empty template (the reporter's control (c)). +func TestNullTemplatesOfUnsetDataSources(t *testing.T) { + const ( + dsID = "11111111-1111-1111-1111-111111111111" + textID = "22222222-2222-2222-2222-222222222222" + colTxtID = "33333333-3333-3333-3333-333333333333" + ) + ids := map[string]pages.PropertyTypeIDEntry{ + "logicItems": {PropertyTypeID: dsID, ValueType: "DataSource"}, + "logicText": {PropertyTypeID: textID, ValueType: "TextTemplate", DataSourceProperty: "logicItems"}, + "columnTitle": {PropertyTypeID: colTxtID, ValueType: "TextTemplate", DataSourceProperty: "../datasource"}, + } + authored := BuildClientTemplateWithTextAndParams("Rule {1}", nil) + prop := func(id string, ds, tt any) bson.D { + return bson.D{ + {Key: "$Type", Value: "CustomWidgets$WidgetProperty"}, + {Key: "TypePointer", Value: types.UUIDToBlob(id)}, + {Key: "Value", Value: bson.D{ + {Key: "$Type", Value: "CustomWidgets$WidgetValue"}, + {Key: "DataSource", Value: ds}, + {Key: "TextTemplate", Value: tt}, + }}, + } + } + object := func(ds, text any) bson.D { + return bson.D{{Key: "Properties", Value: bson.A{int32(2), + prop(dsID, ds, nil), prop(textID, nil, text), prop(colTxtID, nil, BuildEmptyClientTemplate()), + }}} + } + templateOf := func(obj bson.D, id string) any { + for _, item := range obj[0].Value.(bson.A)[1:] { + p := item.(bson.D) + if propertyTypePointerID(p) == normalizeID(id) { + for _, e := range widgetValueOfProperty(p) { + if e.Key == "TextTemplate" { + return e.Value + } + } + } + } + t.Fatalf("no property %s", id) + return nil + } + setDS := bson.D{{Key: "$Type", Value: "CustomWidgets$CustomWidgetXPathSource"}} + + if got := templateOf(NullTemplatesOfUnsetDataSources(object(nil, BuildEmptyClientTemplate()), ids), textID); got != nil { + t.Errorf("(a) data source unset, template unset: TextTemplate = %v, want null", got) + } + if got := templateOf(NullTemplatesOfUnsetDataSources(object(setDS, BuildEmptyClientTemplate()), ids), textID); got == nil { + t.Error("(c) data source set, template unset: nulled; Studio Pro keeps the empty template") + } + if got := templateOf(NullTemplatesOfUnsetDataSources(object(nil, authored), ids), textID); got == nil { + t.Error("an authored template was dropped") + } + if got := templateOf(NullTemplatesOfUnsetDataSources(object(nil, BuildEmptyClientTemplate()), ids), colTxtID); got == nil { + t.Error("a template bound to the enclosing widget's data source was nulled") + } +} diff --git a/mdl/backend/workflow_members.go b/mdl/backend/workflow_members.go index b751844d2b..23e9c75bfb 100644 --- a/mdl/backend/workflow_members.go +++ b/mdl/backend/workflow_members.go @@ -110,3 +110,17 @@ func ParallelPathIndex(caption, ref string, count int) (int, error) { } return n - 1, nil } + +// BoundaryEventIndex is the 0-based position of `boundary event N` among an +// activity's count boundary events (ako/mxcli#808). index 0 — no number written +// — is the first, the old behaviour the executor gates (MDL-V1-BOUNDARYDROP); +// a number past the last is refused rather than clamped. +func BoundaryEventIndex(ref string, index, count int) (int, error) { + if index == 0 { + return 0, nil + } + if index < 1 || index > count { + return 0, fmt.Errorf("activity %q has %d boundary event(s); `boundary event %d` names none of them", ref, count, index) + } + return index - 1, nil +} diff --git a/mdl/executor/alter_set_column_attribute.go b/mdl/executor/alter_set_column_attribute.go new file mode 100644 index 0000000000..6f4402c674 --- /dev/null +++ b/mdl/executor/alter_set_column_attribute.go @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/model" +) + +// resolveColumnAttribute binds `set (Attribute: …) on column(…)` as +// CREATE binds a column's `Attribute:` (ako/mxcli#1644), so the mutator only +// writes it — the split #1641 made for a pluggable widget's attribute. +// +// A column attribute is linked to the grid's data source: a bare name is +// qualified against the grid's entity (an inherited member by its declaring +// one), `Module.Entity.Attr` is kept but refused when the entity is not the +// grid's or a generalization of it (CE7247), `Assoc/…/Attr` gets the hops CREATE +// writes for a multi-hop column, and `$Param.Attr` is refused — a column reads +// the grid's items, not a parameter. +func resolveColumnAttribute(ctx *ExecContext, mutator backend.PageMutator, target ast.WidgetRef, value any, + moduleName string, moduleID model.ID) (backend.AttributeBinding, error) { + where := fmt.Sprintf("column `%s` attribute", target.Name()) + if expr, ok := value.(*ast.WidgetExpressionV3); ok { + return backend.AttributeBinding{}, mdlerrors.NewValidationf( + "%s: `%s` names a parameter's attribute, but a column's attribute is read from each of the grid's "+ + "items — bind an attribute of the grid's entity (`Attr`, or `Assoc/Attr` over an association)", + where, expr.Text) + } + attr, ok := value.(string) + if !ok || strings.TrimSpace(attr) == "" { + return backend.AttributeBinding{}, mdlerrors.NewValidationf( + "%s takes an attribute name, `Module.Entity.Attr` or `Assoc/Attr`", where) + } + entity := mutator.EnclosingEntityForChildren(target.Widget) + if entity == "" { + mf, nf := mutator.EnclosingDataSourceFlow(target.Widget, true) + entity = resolveDataSourceFlowEntity(ctx, moduleName, moduleID, mf, nf) + } + pb := newAlterPageBuilder(ctx, moduleName, moduleID, entity, mutator) + if strings.Contains(attr, "/") { + if entity == "" { + return backend.AttributeBinding{}, mdlerrors.NewValidationf( + "%s: the grid's data source names no entity, so the path `%s` cannot be followed", where, attr) + } + final, steps, ok := pb.resolveAssociationAttributePathForEntity(attr, entity) + if !ok { + return backend.AttributeBinding{}, mdlerrors.NewValidationf( + "%s: `%s` does not resolve from %s — each step must be an association of the entity before it, "+ + "and the last an attribute", where, attr, entity) + } + return backend.AttributeBinding{Path: final, Steps: steps}, nil + } + outer := mutator.EnclosingEntity(target.Widget) + ds := map[string]string{} + if entity != "" { + ds["datasource"] = entity + } + bs := resolveBindingScope("datasource", true, outer, ds, "") + ix := pb.attributeIndex() + if err := misboundAttributeError(where, attr, bs, outer, ds, ix); err != nil { + return backend.AttributeBinding{}, err + } + if strings.Contains(attr, ".") { + if err := foreignQualifiedAttributeError(where, attr, bs, ix); err != nil { + return backend.AttributeBinding{}, err + } + return backend.AttributeBinding{Path: attr}, nil + } + if entity == "" { + return backend.AttributeBinding{}, mdlerrors.NewValidationf( + "%s: the grid's data source names no entity to qualify `%s` with — write `Module.Entity.%s`", + where, attr, attr) + } + if err := pb.rejectAssociationAsAttribute(attr, entity, where); err != nil { + return backend.AttributeBinding{}, err + } + return backend.AttributeBinding{Path: pb.resolveAttributePathForEntity(attr, entity)}, nil +} diff --git a/mdl/executor/alter_set_column_attribute_pedapp_test.go b/mdl/executor/alter_set_column_attribute_pedapp_test.go new file mode 100644 index 0000000000..696457ad64 --- /dev/null +++ b/mdl/executor/alter_set_column_attribute_pedapp_test.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "context" + "path/filepath" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// ako/mxcli#1644: `alter page … set (Attribute: …) on column(…)` was +// refused for every spelling ("holds a value of kind Attribute, which ALTER +// cannot set from a plain value"), and `Assoc/Attr` did not parse — the only way +// was to replace the column, dropping what the replacement did not restate. It +// now binds as CREATE binds a column's Attribute: against the grid's entity. +func TestAlterPageSet_ColumnAttribute(t *testing.T) { + exec, out := openPedAppWithWidgets(t) + dir := filepath.Dir(exec.newExecContext(context.Background()).Backend.Path()) + const setup = `mdl 1; +create persistent entity MyFirstModule.Customer1644 (Name: String(100), City: String(100)); +create persistent entity MyFirstModule.Order1644 (Number: String(20), Note: String(200), Code: String(20), Ref: String(20)); +create association MyFirstModule.Order1644_Customer1644 from MyFirstModule.Order1644 to MyFirstModule.Customer1644; +create page MyFirstModule.OrderGrid1644 ( Title: 'Orders', Layout: Atlas_Core.Atlas_Default, Params: ( $Cust: MyFirstModule.Customer1644 ) ) { + datagrid dgOrders (DataSource: database from MyFirstModule.Order1644) { + column colNumber (Attribute: Number, Caption: 'Number') + column colCode (Attribute: Code, Caption: 'Code') + column colRef (Attribute: Ref, Caption: 'Ref') + } +};` + if err := agreeExec(t, exec, setup); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + alter := func(set string) string { + return "mdl 1;\nalter page MyFirstModule.OrderGrid1644 { " + set + " };" + } + assertAgree(t, exec, dir, alter("set (Attribute: Note) on dgOrders column(Number)"), "") + assertAgree(t, exec, dir, alter("set (Attribute: MyFirstModule.Order1644.Number) on dgOrders column(Code)"), "") + assertAgree(t, exec, dir, alter("set (Attribute: Order1644_Customer1644/City) on dgOrders column(Ref)"), "") + + var buf bytes.Buffer + saved := exec.output + exec.output = &buf + err := afRun(t, exec, "describe page MyFirstModule.OrderGrid1644;") + exec.output = saved + if err != nil { + t.Fatal(err) + } + d := buf.String() + for _, want := range []string{"Attribute: Note", "Attribute: Number", "Attribute: Order1644_Customer1644/City"} { + if !strings.Contains(d, want) { + t.Errorf("describe does not show %q:\n%s", want, d) + } + } + + // Refusals, in check and in exec alike. + assertAgree(t, exec, dir, alter("set (Attribute: $Cust.Name) on dgOrders column(Note)"), + "a column's attribute is read from each of the grid's items") + assertAgree(t, exec, dir, alter("set (Attribute: NoSuchAttr) on dgOrders column(Note)"), "NoSuchAttr") + assertAgree(t, exec, dir, alter("set (Attribute: MyFirstModule.Customer1644.City) on dgOrders column(Note)"), "Customer1644") +} + +// The reset of what the widget hides runs after a column SET and re-finds the +// column by its address — which is its attribute. Changing the attribute moved +// the column away from `column(Old)`, and the reset failed with "column(Old) +// not found" (measured on 11.14 with its Data grid 2 package; PedApp's ships no +// editorConfig, so its rules are installed from the expr-checker fixture here). +// The attribute is now written after the rest of the SET and its reset. +func TestAlterPageSet_ColumnAttributeWithHiddenPropertyReset(t *testing.T) { + exec, out := openPedAppWithWidgets(t) + path := exec.newExecContext(context.Background()).Backend.Path() + dir := filepath.Dir(path) + mpk := "com.mendix.widget.web.Datagrid.mpk" + if err := copyPedAppFile(filepath.Join("..", "..", "testdata", "expr-checker", "widgets", mpk), filepath.Join(dir, "widgets", mpk)); err != nil { + t.Fatal(err) + } + if rules := resolveWidgetVisibilityRules(path, pages.WidgetIDDataGrid2); len(rules) == 0 { + t.Fatal("control: the installed Data grid 2 package states no visibility rules, so the reset would not run") + } + const setup = `mdl 1; +create persistent entity MyFirstModule.Order1644b (Number: String(20), Note: String(200)); +create page MyFirstModule.OrderGrid1644b ( Title: 'Orders', Layout: Atlas_Core.Atlas_Default ) { + datagrid dgOrders (DataSource: database from MyFirstModule.Order1644b) { + column colNumber (Attribute: Number, Caption: 'Number') + } +};` + if err := agreeExec(t, exec, setup); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + for _, set := range []string{ + "set (Attribute: Note) on dgOrders column(Number)", + "set (Attribute: Number, ColumnWidth: autoFill) on dgOrders column(Note)", + } { + assertAgree(t, exec, dir, "mdl 1;\nalter page MyFirstModule.OrderGrid1644b { "+set+" };", "") + } +} diff --git a/mdl/executor/alter_set_date_format_pedapp_test.go b/mdl/executor/alter_set_date_format_pedapp_test.go new file mode 100644 index 0000000000..033716abf9 --- /dev/null +++ b/mdl/executor/alter_set_date_format_pedapp_test.go @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import "testing" + +// ako/mxcli#1965 (mendixlabs/mxcli#1743): `set (DateFormat: Custom, +// CustomDateFormat: …)` on a date picker whose stored format is not Custom was +// refused by check — and so by exec's pre-check — in either order and as two +// SETs, while --no-check wrote it correctly and mx check passed. check dry-ran +// each property alone on the stored widget, where Custom has no pattern. The +// widget is now judged as the whole statement leaves it, as exec does. +func TestCheckExecAgree_CustomDateFormatSetWithItsPattern(t *testing.T) { + exec, out, dir := openPedAppCopy(t) + const setup = `mdl 1; +create persistent entity MyFirstModule.Booking1965 (Moment: DateTime); +create page MyFirstModule.Booking1965_Edit ( Title: 'Booking', Layout: Atlas_Core.PopupLayout, Params: ( $Booking: MyFirstModule.Booking1965 ) ) { + layoutgrid lg { row { column (DesktopWidth: autofill) { + dataview dv (DataSource: $Booking) { + datepicker dpA (Label: 'A', Attribute: Moment) + datepicker dpB (Label: 'B', Attribute: Moment) + datepicker dpC (Label: 'C', Attribute: Moment) + datepicker dpD (Label: 'D', Attribute: Moment) + } + } } } +};` + if err := agreeExec(t, exec, setup); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + for _, tc := range []struct{ name, set, want string }{ + {"one SET, DateFormat first", "set (DateFormat: Custom, CustomDateFormat: 'dd/MM/yyyy') on dpA", ""}, + {"one SET, the pattern first", "set (CustomDateFormat: 'dd/MM/yyyy', DateFormat: Custom) on dpB", ""}, + {"two SETs, the pattern first", "set (CustomDateFormat: 'HH:mm') on dpC; set (DateFormat: Custom) on dpC", ""}, + // Control: Custom with no pattern anywhere is still CE0493, in both. + {"Custom with no pattern", "set (DateFormat: Custom) on dpD", "CE0493"}, + } { + t.Run(tc.name, func(t *testing.T) { + assertAgree(t, exec, dir, "mdl 1;\nalter page MyFirstModule.Booking1965_Edit { "+tc.set+" };", tc.want) + }) + } +} diff --git a/mdl/executor/alter_set_template_params_pedapp_test.go b/mdl/executor/alter_set_template_params_pedapp_test.go new file mode 100644 index 0000000000..5715c60424 --- /dev/null +++ b/mdl/executor/alter_set_template_params_pedapp_test.go @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" +) + +// ako/mxcli#1966 (mendixlabs/mxcli#1744): `alter page … set (CaptionParams: +// (…))` / `set (ContentParams: (…))` did not parse, so a text template's +// parameters could be written by create page and replace but not changed in +// place. The reporter's two statements now pass check and exec, and describe +// prints the parameters back, resolved against the data view's entity. +func TestAlterPageSet_TemplateParameters(t *testing.T) { + exec, out, dir := openPedAppCopy(t) + const setup = `mdl 1; +create persistent entity MyFirstModule.Draft1966 (Title: String(100), VersionNo: Integer); +create microflow MyFirstModule.ACT_Open1966 ($Draft: MyFirstModule.Draft1966) returns Boolean begin return true; end; +create page MyFirstModule.PageD1966 ( Title: 'D', Layout: Atlas_Core.PopupLayout, Params: ( $Draft: MyFirstModule.Draft1966 ) ) { + dataview dvD (DataSource: $Draft) { + actionbutton btnD (Caption: 'Open draft', Action: call microflow MyFirstModule.ACT_Open1966(Draft = $currentObject)) + dynamictext txtE (Content: 'Open draft (version {1})', ContentParams: ({1} = Title)) + textbox tbT (Label: 'Title', Attribute: Title) + } +};` + if err := agreeExec(t, exec, setup); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + assertAgree(t, exec, dir, "mdl 1;\nalter page MyFirstModule.PageD1966 { "+ + "set (Caption: 'Open draft (version {1})', CaptionParams: ({1} = VersionNo)) on btnD };", "") + assertAgree(t, exec, dir, "mdl 1;\nalter page MyFirstModule.PageD1966 { "+ + "set (Content: 'Draft {1}', ContentParams: ({1} = VersionNo)) on txtE };", "") + + var buf bytes.Buffer + saved := exec.output + exec.output = &buf + err := afRun(t, exec, "describe page MyFirstModule.PageD1966;") + exec.output = saved + if err != nil { + t.Fatal(err) + } + d := buf.String() + for _, want := range []string{ + "Caption: 'Open draft (version {1})',", + "CaptionParams: ({1} = VersionNo)", + "Content: 'Draft {1}', ContentParams: ({1} = VersionNo)", + } { + if !strings.Contains(d, want) { + t.Errorf("describe does not show %q:\n%s", want, d) + } + } + + // Control: a widget with no text template refuses it, in both. + assertAgree(t, exec, dir, "mdl 1;\nalter page MyFirstModule.PageD1966 { set (CaptionParams: ({1} = VersionNo)) on tbT };", + "has no CaptionParams") +} diff --git a/mdl/executor/alter_workflow_member_address_test.go b/mdl/executor/alter_workflow_member_address_test.go index 473f08aaab..c57833d04f 100644 --- a/mdl/executor/alter_workflow_member_address_test.go +++ b/mdl/executor/alter_workflow_member_address_test.go @@ -94,7 +94,7 @@ func TestAlterWorkflow_DropBoundaryEventAmongSeveral(t *testing.T) { t.Run(tc.name, func(t *testing.T) { dropped := false mut := &mock.MockWorkflowMutator{ - DropBoundaryEventFunc: func(string, int) error { dropped = true; return nil }, + DropBoundaryEventFunc: func(string, int, int) error { dropped = true; return nil }, } ctx, out := boundaryDropCtx(t, tc.events, mut) ctx.LanguageVersion = tc.v @@ -118,3 +118,78 @@ func TestAlterWorkflow_DropBoundaryEventAmongSeveral(t *testing.T) { }) } } + +// ako/mxcli#808: `boundary event N` names one event among several, in describe +// order, under either language version; out of range is refused before the +// mutator is reached. +func TestAlterWorkflow_DropBoundaryEventByIndex(t *testing.T) { + for _, tc := range []struct { + src string + v langver.Version + index int // index handed to the mutator; 0 = not reached + refused string + }{ + {`alter workflow Sales.WF { drop task1 boundary event 2 };`, langver.V1, 2, ""}, + {`alter workflow Sales.WF { drop task1 boundary event 1 };`, langver.V0, 1, ""}, + {`alter workflow Sales.WF { drop task1 boundary event 3 };`, langver.V1, 0, "has 2 boundary event"}, + {`alter workflow Sales.WF { drop task1 boundary event 3 };`, langver.V0, 0, "has 2 boundary event"}, + } { + t.Run(tc.src, func(t *testing.T) { + got := 0 + mut := &mock.MockWorkflowMutator{ + DropBoundaryEventFunc: func(_ string, _ int, index int) error { got = index; return nil }, + } + ctx, out := boundaryDropCtx(t, 2, mut) + ctx.LanguageVersion = tc.v + prog, perrs := visitor.Build(tc.src) + if len(perrs) > 0 { + t.Fatal(perrs) + } + err := execAlterWorkflow(ctx, prog.Statements[0].(*ast.AlterWorkflowStmt)) + if tc.refused != "" { + if err == nil || !strings.Contains(err.Error(), tc.refused) { + t.Fatalf("want refusal %q, got %v", tc.refused, err) + } + } else if err != nil { + t.Fatal(err) + } + if got != tc.index { + t.Errorf("mutator got index %d, want %d", got, tc.index) + } + if strings.Contains(out.String(), "MDL-V1-BOUNDARYDROP") { + t.Errorf("an addressed drop is not a guess; got warning:\n%s", out.String()) + } + }) + } +} + +func TestVisitor_BoundaryEventIndexZeroRefused(t *testing.T) { + if _, errs := visitor.Build(`alter workflow Sales.WF { drop task1 boundary event 0 };`); len(errs) == 0 { + t.Fatal("boundary event 0 accepted; indexes are 1-based") + } +} + +// ako/mxcli#808: a Name match wins over caption matches without @N, as in the +// mutator (preferNameMatches). A jump captioned with its target's name used to +// make the reference ambiguous here, so the check-time guards went silent. +func TestResolveStoredActivity_PrefersName(t *testing.T) { + task := &workflows.UserTask{BaseWorkflowActivity: workflows.BaseWorkflowActivity{ + BaseElement: model.BaseElement{ID: "a1"}, Name: "task1", Caption: "Review"}} + jump := &workflows.JumpToActivity{BaseWorkflowActivity: workflows.BaseWorkflowActivity{ + BaseElement: model.BaseElement{ID: "a2"}, Name: "jump1", Caption: "task1"}} + flow := &workflows.Flow{Activities: []workflows.WorkflowActivity{task, jump}} + if got := resolveStoredActivity(flow, "task1", 0); got != task { + t.Fatalf("task1 resolved to %v, want the task named task1", got) + } + // @N still counts every match in describe order. + if got := resolveStoredActivity(flow, "task1", 2); got != jump { + t.Fatalf("task1@2 resolved to %v, want the jump", got) + } + // Two caption matches and no name match stay ambiguous. + other := &workflows.JumpToActivity{BaseWorkflowActivity: workflows.BaseWorkflowActivity{ + BaseElement: model.BaseElement{ID: "a3"}, Name: "jump2", Caption: "Review"}} + flow.Activities = append(flow.Activities, other) + if got := resolveStoredActivity(flow, "Review", 0); got != nil { + t.Fatalf("ambiguous caption resolved to %v", got) + } +} diff --git a/mdl/executor/cmd_alter_flow.go b/mdl/executor/cmd_alter_flow.go index 0ff080a245..13b48519b4 100644 --- a/mdl/executor/cmd_alter_flow.go +++ b/mdl/executor/cmd_alter_flow.go @@ -53,6 +53,9 @@ func execAlterFlow(ctx *ExecContext, s *ast.AlterFlowStmt) error { return mdlerrors.NewBackend("save altered "+s.Kind(), err) } fmt.Fprintf(ctx.Output, "Altered %s %s\n", s.Kind(), s.Name) + for _, c := range a.carriedIncludes { + fmt.Fprintln(ctx.Output, carriedIncludesWarning(s.Name.String(), c)) + } return nil } @@ -201,6 +204,18 @@ func (a *alterFlowContext) applyTo(ctx *ExecContext, mut backend.MicroflowMutato if frag, err = a.keepStoredHandler(ctx, op, target, frag); err != nil { return fail(err) } + // A call external action replaced by one that leaves out + // `include associations` keeps the stored tree, as a create or + // modify rebuild does (ako/mxcli#1914): the splice rebuilt the + // call and dropped it. + carried, err := carryIncludedAssociations( + µflows.MicroflowObjectCollection{Objects: frag.Objects}, + µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{target.Object}}, + a.stmt.Name.String(), a.statedIncludes) + if err != nil { + return fail(err) + } + a.carriedIncludes = append(a.carriedIncludes, carried...) } if inLoop != nil { if what := notPlainFragment(frag); what != "" { @@ -329,6 +344,12 @@ type alterFlowContext struct { // one whose target was swapped for new elements (ako/mxcli#1761). The // report of a `create or modify` tells the two apart. edited map[*ast.AlterFlowOperation]bool + // statedIncludes are the external action mappings whose fragment wrote + // `include associations ( … )`, and carriedIncludes the stored trees a + // replace kept because its statement did not (ako/mxcli#1914) — told by + // whoever reports the write, not here, since check plans the same splice. + statedIncludes map[*microflows.ExternalActionParameterMapping]bool + carriedIncludes []carriedIncludes } // noteEdited records that op edited its stored target in place. @@ -771,13 +792,10 @@ func (a *alterFlowContext) splitCondition(ctx *ExecContext, op *ast.AlterFlowOpe if missing := a.unscopedRefs(expr, target.ID); len(missing) > 0 { return "", "", fmt.Errorf("the condition uses %s, which is not declared on the path to the decision", strings.Join(missing, ", ")) } - // The `if` states the condition as describe printed it. For a decision - // whose true path goes straight on, describe prints the negated condition - // with the paths swapped; the stored flows do not move, so storing the - // printed condition as it is would invert the decision (ako/mxcli#1930). - if decisionPrintedNegated(ctx, a.mf.ObjectCollection, target.ID) { - expr = negateExpression(expr) - } + // The `if` states the condition as describe prints it, which is the stored + // one: describe no longer folds a decision whose true path goes straight + // on into not(X) with its paths swapped (ako/mxcli#1963), so there is no + // fold to undo here (#1930). caption = split.Caption if caption == old.Expression { caption = expr @@ -867,6 +885,12 @@ func (a *alterFlowContext) buildFragmentVars(ctx *ExecContext, body []ast.Microf return nil, fragmentVars{}, err } frag.Pinned = fb.statedNotes + for pm := range fb.statedIncludes { + if a.statedIncludes == nil { + a.statedIncludes = map[*microflows.ExternalActionParameterMapping]bool{} + } + a.statedIncludes[pm] = true + } return frag, declaredVarsOf(fb, frag.Objects), nil } diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index 87375b73e5..8bd6d92764 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -355,6 +355,32 @@ func resolveAlterPageUnit(ctx *ExecContext, s *ast.AlterPageStmt, h *ContainerHi // ============================================================================ func applySetPropertyMutator(ctx *ExecContext, mutator backend.PageMutator, op *ast.SetPropertyOp, moduleName string, moduleID model.ID) error { + // A column's attribute is written last. The column is addressed by its + // attribute, so once it changes, `column(Old)` no longer finds it, and the + // follow-up below — which re-finds the column — failed with "column(Old) + // not found". The attribute drives none of the visibility the follow-up + // resets, so the rest is applied and reset under the address as written + // (ako/mxcli#1644). + if op.Target.IsColumn() { + for name, value := range op.Properties { + if !strings.EqualFold(name, "Attribute") { + continue + } + rest := &ast.SetPropertyOp{Target: op.Target, Properties: map[string]any{}} + for k, v := range op.Properties { + if k != name { + rest.Properties[k] = v + } + } + if len(rest.Properties) > 0 { + if err := applySetPropertyMutator(ctx, mutator, rest, moduleName, moduleID); err != nil { + return err + } + } + return applySetProperties(ctx, mutator, + &ast.SetPropertyOp{Target: op.Target, Properties: map[string]any{name: value}}, moduleName, moduleID) + } + } if err := applySetProperties(ctx, mutator, op, moduleName, moduleID); err != nil { return err } @@ -463,7 +489,20 @@ func applySetProperties(ctx *ExecContext, mutator backend.PageMutator, op *ast.S "`set %s = ` needs a pluggable widget target: `set '%s' = … on `", propName, propName) } + if params, ok := value.([]ast.ParamAssignmentV3); ok && isTemplateParamsKey(propName) { + if err := setTemplateParameters(ctx, mutator, op.Target, propName, params, moduleName, moduleID); err != nil { + return err + } + continue + } if op.Target.IsColumn() { + if strings.EqualFold(propName, "Attribute") { + b, err := resolveColumnAttribute(ctx, mutator, op.Target, value, moduleName, moduleID) + if err != nil { + return err + } + value = b + } if err := mutator.SetColumnProperty(op.Target.Widget, columnRefOf(op.Target), propName, value); err != nil { return mdlerrors.NewBackend("set "+propName+" on "+op.Target.Name(), err) } @@ -1384,3 +1423,32 @@ func newAlterPageBuilder(ctx *ExecContext, moduleName string, moduleID model.ID, isSnippet: mutator.ContainerType() == backend.ContainerSnippet, } } + +// templateParameterSetter is the page mutator capability behind +// `set (CaptionParams: …)` / `set (ContentParams: …)`. +type templateParameterSetter interface { + SetWidgetTemplateParameters(widgetRef, prop string, params []*pages.ClientTemplateParameter) error +} + +func isTemplateParamsKey(prop string) bool { + return strings.EqualFold(prop, "CaptionParams") || strings.EqualFold(prop, "ContentParams") +} + +// setTemplateParameters writes a text template's parameters on a stored +// built-in widget (ako/mxcli#1966), resolved by the builder CREATE PAGE uses +// against the entity of the data container the widget sits in — so a bare +// attribute name means here what it means in `create page`. +func setTemplateParameters(ctx *ExecContext, mutator backend.PageMutator, target ast.WidgetRef, prop string, + params []ast.ParamAssignmentV3, moduleName string, moduleID model.ID) error { + setter, ok := mutator.(templateParameterSetter) + if !ok || target.Widget == "" || target.IsColumn() || target.Item != nil { + return mdlerrors.NewUnsupported(fmt.Sprintf( + "alter page set %s is supported on a button or a dynamic text — use `replace with …` here", prop)) + } + pb := newAlterPageBuilder(ctx, moduleName, moduleID, mutator.EnclosingEntity(target.Widget), mutator) + resolved := pb.buildClientTemplateParams(params) + if err := setter.SetWidgetTemplateParameters(target.Widget, prop, resolved); err != nil { + return mdlerrors.NewBackend("set "+prop+" on "+target.Name(), err) + } + return nil +} diff --git a/mdl/executor/cmd_alter_workflow.go b/mdl/executor/cmd_alter_workflow.go index b47dd8e88c..cbe200bd46 100644 --- a/mdl/executor/cmd_alter_workflow.go +++ b/mdl/executor/cmd_alter_workflow.go @@ -243,7 +243,7 @@ func execAlterWorkflow(ctx *ExecContext, s *ast.AlterWorkflowStmt) error { } case *ast.DropBoundaryEventOp: - if err := mutator.DropBoundaryEvent(o.ActivityRef, o.AtPosition); err != nil { + if err := mutator.DropBoundaryEvent(o.ActivityRef, o.AtPosition, o.Index); err != nil { return mdlerrors.NewBackend("drop boundary event", err) } diff --git a/mdl/executor/cmd_alter_workflow_boundary.go b/mdl/executor/cmd_alter_workflow_boundary.go index 62264dfc56..84a61dd866 100644 --- a/mdl/executor/cmd_alter_workflow_boundary.go +++ b/mdl/executor/cmd_alter_workflow_boundary.go @@ -42,15 +42,25 @@ func checkBoundaryEventDrops(ctx *ExecContext, s *ast.AlterWorkflowStmt) error { } } n := len(storedBoundaryEvents(resolveStoredActivity(wf.Flow, o.ActivityRef, o.AtPosition))) + if o.Index > 0 { + // `boundary event N` names one (ako/mxcli#808): nothing to guess, + // only a place to check. + if n > 0 && o.Index > n { + return mdlerrors.NewValidation(fmt.Sprintf( + "alter workflow %s: `drop %s boundary event %d`: '%s' has %d boundary event(s). Nothing was written", + s.Name, o.ActivityRef, o.Index, o.ActivityRef, n)) + } + continue + } if n < 2 { continue } if boundaryDropAmbiguous.Applies(ctx.LanguageVersion) { return mdlerrors.NewValidation(fmt.Sprintf( "alter workflow %s: `drop %s boundary event` is refused: '%s' has %d boundary events, and the "+ - "statement does not say which. Nothing was written. Restate the workflow with "+ - "`create or modify workflow`, keeping the events to keep", - s.Name, o.ActivityRef, o.ActivityRef, n)) + "statement does not say which. Nothing was written. Name it by its place, as `describe workflow` "+ + "prints them: `drop %s boundary event 1` … `boundary event %d`", + s.Name, o.ActivityRef, o.ActivityRef, n, o.ActivityRef, n)) } fmt.Fprintf(ctx.progress(), "Warning [%s]: alter workflow %s: '%s' has %d boundary events; "+ "`drop %s boundary event` drops the first. %s\n", diff --git a/mdl/executor/cmd_flow_modify.go b/mdl/executor/cmd_flow_modify.go index 5e93c59f44..39533e4c76 100644 --- a/mdl/executor/cmd_flow_modify.go +++ b/mdl/executor/cmd_flow_modify.go @@ -190,6 +190,11 @@ func modifyFlowInPlace(ctx *ExecContext, d *flowDecl) (handled bool, err error) switch summary := patchSummary(ops, a.edited, moves, set); { case summary != "": ctx.ReportMutation("Modified", "%s: %s (%s)", d.kind(), d.name, summary) + if ctx.Output != nil { + for _, c := range a.carriedIncludes { + fmt.Fprintln(ctx.Output, carriedIncludesWarning(d.name.String(), c)) + } + } case moved: ctx.ReportMutation("Moved", "%s: %s", d.kind(), d.name) default: diff --git a/mdl/executor/cmd_microflows_build.go b/mdl/executor/cmd_microflows_build.go index f39b4de7ac..a09e772f91 100644 --- a/mdl/executor/cmd_microflows_build.go +++ b/mdl/executor/cmd_microflows_build.go @@ -472,9 +472,15 @@ func buildMicroflowFromStmt(ctx *ExecContext, s *ast.CreateMicroflowStmt, opts b } return nil, fmt.Errorf("%s", errMsg.String()) } - if err := carryIncludedAssociations(mf.ObjectCollection, existingObjects, qualifiedName); err != nil { + carried, err := carryIncludedAssociations(mf.ObjectCollection, existingObjects, qualifiedName, builder.statedIncludes) + if err != nil { return nil, err } + if !opts.Quiet && ctx.Output != nil { + for _, c := range carried { + fmt.Fprintln(ctx.Output, carriedIncludesWarning(qualifiedName, c)) + } + } return &builtFlow{ Microflow: mf, ContainerID: containerID, diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index 5aa1fa728e..56512ac319 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -33,9 +33,17 @@ type flowBuilder struct { // reports the rest without a project). assocObjectVars map[string]bool - objects []microflows.MicroflowObject - flows []*microflows.SequenceFlow - annotationFlows []*microflows.AnnotationFlow + objects []microflows.MicroflowObject + flows []*microflows.SequenceFlow + // statedIncludes are the external action parameter mappings whose + // statement wrote `include associations ( … )`: the tree is the statement's, + // so the stored one is not carried onto them (ako/mxcli#1914). + statedIncludes map[*microflows.ExternalActionParameterMapping]bool + // resolveKnownOnly is check's mode for include associations: an + // association or entity the project does not hold yet may be one an + // earlier statement creates, so only what the project decides is reported. + resolveKnownOnly bool + annotationFlows []*microflows.AnnotationFlow // storedNotes resolves `@annotation(existing: '…')` to the note of the // stored flow with that text, keyed by the text as the script writes it. @@ -220,6 +228,11 @@ type flowBuilder struct { // it — `find $L by Code = …` in a replaced loop became a find by the invalid // expression `Code = …` (CE0117). func (fb *flowBuilder) nestedBuilder(x, y int) *flowBuilder { + // One registry for the whole flow, made here so a body's marks land in + // it rather than in a map of the body's own (ako/mxcli#1914). + if fb.statedIncludes == nil { + fb.statedIncludes = map[*microflows.ExternalActionParameterMapping]bool{} + } return &flowBuilder{ posX: x, posY: y, @@ -255,6 +268,10 @@ func (fb *flowBuilder) nestedBuilder(x, y int) *flowBuilder { storedNotes: fb.storedNotes, // A body of a spliced fragment is spliced too (ako/mxcli#1769). qualifiedMembersOnly: fb.qualifiedMembersOnly, + // A call in a body that states `include associations` must reach the + // carry, which reads the flow's registry (ako/mxcli#1914). + statedIncludes: fb.statedIncludes, + resolveKnownOnly: fb.resolveKnownOnly, } } diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index a9651a003f..a7c196c84b 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -1139,6 +1139,14 @@ func (fb *flowBuilder) addCallExternalActionAction(s *ast.CallExternalActionStmt // makes every call on a nullable parameter unbuildable. mapping.CanBeEmpty = pk.canBeEmpty } + if arg.IncludeAssociations != nil { + mapping.IncludedAssociations = fb.buildIncludedAssociations( + serviceQN+"."+s.ActionName, arg.Name, mapping.ParameterEntity, arg.IncludeAssociations.Associations) + if fb.statedIncludes == nil { + fb.statedIncludes = map[*microflows.ExternalActionParameterMapping]bool{} + } + fb.statedIncludes[mapping] = true + } mappings = append(mappings, mapping) } @@ -2492,3 +2500,66 @@ func (fb *flowBuilder) optExprToString(e ast.Expression) string { } return fb.exprToString(e) } + +// buildIncludedAssociations builds an argument's Associations to include from +// `include associations ( … )` (ako/mxcli#1914). side is the entity on this +// side of each node: the parameter's entity at the root, and below a node the +// entity at its association's far end. +// +// IsParent is derived, never written: it is whether side is the association's +// FROM end, its owner (ParentPointer — CLAUDE.md, "Association Parent/Child +// Pointer Semantics"). A self-association is refused, because both ends are +// side and the direction cannot be derived; guessing it would serialise the +// wrong one. So is an association that does not touch side, and one side +// specializes both ends of. +func (fb *flowBuilder) buildIncludedAssociations(call, param, side string, defs []*ast.IncludedAssociationDef) []*microflows.IncludedAssociation { + out := make([]*microflows.IncludedAssociation, 0, len(defs)) + for _, def := range defs { + qn := def.Name.String() + where := fmt.Sprintf("call external action %s(%s): include associations %s", call, param, qn) + if side == "" { + if fb.resolveKnownOnly { + return out + } + fb.addError("%s: the parameter's entity is not known (the service's metadata does not type it), "+ + "so which end of the association it is on cannot be worked out", where) + return out + } + info := fb.lookupAssociation(def.Name.Module, def.Name.Name) + if info == nil { + if !fb.resolveKnownOnly { + fb.addError("%s: association not found", where) + } + continue + } + if info.parentEntityQN != "" && info.parentEntityQN == info.childEntityQN { + fb.addError("%s is a self-association (%s to itself): MDL cannot tell which end the object is on, "+ + "so it cannot write it. Tick it in Studio Pro; a create or modify that leaves the clause out keeps it", where, side) + continue + } + isFrom := fb.entityIsSubtypeOf(side, info.parentEntityQN) + isTo := fb.entityIsSubtypeOf(side, info.childEntityQN) + var far string + switch { + case isFrom && !isTo: + far = info.childEntityQN + case isTo && !isFrom: + far = info.parentEntityQN + case isFrom && isTo: + fb.addError("%s: %s specializes both ends (%s and %s), so which end the object is on cannot be worked out", + where, side, info.parentEntityQN, info.childEntityQN) + continue + default: + fb.addError("%s does not touch %s: it is from %s to %s. A node's associations start from the entity "+ + "at its parent's far end, and the root's from the parameter's entity", where, side, info.parentEntityQN, info.childEntityQN) + continue + } + out = append(out, µflows.IncludedAssociation{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + Association: qn, + IsParent: isFrom, + IncludedAssociations: fb.buildIncludedAssociations(call, param, far, def.Children), + }) + } + return out +} diff --git a/mdl/executor/cmd_microflows_builder_nested_test.go b/mdl/executor/cmd_microflows_builder_nested_test.go index dc1c9b0a03..d1cff647aa 100644 --- a/mdl/executor/cmd_microflows_builder_nested_test.go +++ b/mdl/executor/cmd_microflows_builder_nested_test.go @@ -35,6 +35,8 @@ var nestedBuilderInherits = map[string]bool{ "annotationsByLabel": true, "storedNotes": true, "qualifiedMembersOnly": true, + "statedIncludes": true, // the carry reads one registry (#1914) + "resolveKnownOnly": true, // Where the body is drawn: nestedBuilder's arguments, and the body's own // lanes and rows (a loop box is sized to fit, so it never wraps). diff --git a/mdl/executor/cmd_microflows_format_action.go b/mdl/executor/cmd_microflows_format_action.go index 09023f9236..452e8eb6cf 100644 --- a/mdl/executor/cmd_microflows_format_action.go +++ b/mdl/executor/cmd_microflows_format_action.go @@ -918,7 +918,8 @@ func formatAction( // optional parameter left blank as `Argument: ""`, which printed as // `name = )` and did not parse (ako/mxcli#1915). `nothing` is the // blank; `empty` (11.14's spelling of the same field) stays a value. - params = append(params, fmt.Sprintf("%s = %s", pm.ParameterName, describeCallArgument(ctx, pm.Argument))) + arg := fmt.Sprintf("%s = %s", pm.ParameterName, describeCallArgument(ctx, pm.Argument)) + params = append(params, arg+describeIncludedAssociations(ctx, pm.IncludedAssociations)) } paramStr := "" @@ -2507,3 +2508,83 @@ func workflowSelectionMDL(workflow, variable string, flag bool, flagWords string } return sel } + +// describeIncludedAssociations prints an argument's stored Associations to +// include as its `include associations ( … )` clause (ako/mxcli#1914), so +// describe -> exec restates the tree rather than relying on the carry. A tree +// holding a self-association cannot be written back — the builder refuses one, +// its direction not being derivable — so it is printed as a comment instead: +// left out, the clause carries the stored tree on create or modify. +func describeIncludedAssociations(ctx *ExecContext, tree []*microflows.IncludedAssociation) string { + if len(tree) == 0 { + return "" + } + if self := selfAssociationsIn(ctx, tree); len(self) > 0 { + return fmt.Sprintf(" /* include associations (%s): holds the self-association %s, which MDL cannot write; "+ + "create or modify keeps it while this clause is left out — change it in Studio Pro */", + formatIncludedAssociations(tree), strings.Join(self, ", ")) + } + return " include associations (" + formatIncludedAssociations(tree) + ")" +} + +// selfAssociationsIn lists the tree's associations whose two ends are one +// entity. An association the project does not resolve is not one. +func selfAssociationsIn(ctx *ExecContext, tree []*microflows.IncludedAssociation) []string { + if ctx == nil || ctx.Backend == nil { + return nil + } + var names []string + var walk func([]*microflows.IncludedAssociation) + walk = func(in []*microflows.IncludedAssociation) { + for _, ia := range in { + names = append(names, ia.Association) + walk(ia.IncludedAssociations) + } + } + walk(tree) + ends := projectAssociationEnds(ctx) + var self []string + seen := map[string]bool{} + for _, n := range names { + if e, ok := ends[n]; ok && e[0] != "" && e[0] == e[1] && !seen[n] { + seen[n] = true + self = append(self, n) + } + } + return self +} + +// projectAssociationEnds maps each association's qualified name to its +// {FROM, TO} entities. +func projectAssociationEnds(ctx *ExecContext) map[string][2]string { + out := map[string][2]string{} + modules, err := getModulesFromCache(ctx) + if err != nil { + return out + } + moduleNames := make(map[model.ID]string, len(modules)) + for _, m := range modules { + moduleNames[m.ID] = m.Name + } + dms, err := ctx.Backend.ListDomainModels() + if err != nil { + return out + } + for _, dm := range dms { + mod := moduleNames[dm.ContainerID] + if mod == "" { + continue + } + byID := make(map[model.ID]string, len(dm.Entities)) + for _, e := range dm.Entities { + byID[e.ID] = mod + "." + e.Name + } + for _, a := range dm.Associations { + out[mod+"."+a.Name] = [2]string{byID[a.ParentID], byID[a.ChildID]} + } + for _, a := range dm.CrossAssociations { + out[mod+"."+a.Name] = [2]string{byID[a.ParentID], a.ChildRef} + } + } + return out +} diff --git a/mdl/executor/cmd_microflows_handles_test.go b/mdl/executor/cmd_microflows_handles_test.go index 9f5997a333..f1160d0ece 100644 --- a/mdl/executor/cmd_microflows_handles_test.go +++ b/mdl/executor/cmd_microflows_handles_test.go @@ -167,10 +167,9 @@ func TestDescribeMicroflowWithHandles_Statement(t *testing.T) { } } -// Describe prints an if whose then-branch is empty with its condition negated -// and the branches swapped. A target written from that output must still find -// the split, and so must one written from the stored condition. -func TestMicroflowTargets_NegatedIfMatchesBothForms(t *testing.T) { +// An if whose then-branch is empty is printed as stored (ako/mxcli#1963), so +// the stored condition is both the printed form and the handle. +func TestMicroflowTargets_EmptyThenIfIsAddressedAsStored(t *testing.T) { mf := handlesFixture() // Swap the branches: true now goes straight to the merge. for _, f := range mf.ObjectCollection.Flows { @@ -184,11 +183,11 @@ func TestMicroflowTargets_NegatedIfMatchesBothForms(t *testing.T) { } ctx := &ExecContext{} got := strings.Join(formatMicroflowActivities(ctx, mf, nil, nil), "\n") - if !strings.Contains(got, "if not($N > 1) then") { - t.Fatalf("fixture should describe the split negated:\n%s", got) + if !strings.Contains(got, "if $N > 1 then") || strings.Contains(got, "not(") { + t.Fatalf("fixture should describe the split as stored:\n%s", got) } cands, _, _, _ := microflowTargets(ctx, mf, nil, nil) - for _, target := range []string{"if not($N > 1) then", "if $N > 1 then", "if * then"} { + for _, target := range []string{"if $N > 1 then", "if * then"} { c, err := mfmutator.ResolveText(cands, target) if err != nil { t.Errorf("%s: %v", target, err) @@ -203,7 +202,7 @@ func TestMicroflowTargets_NegatedIfMatchesBothForms(t *testing.T) { if c.ID == "split" { c.Caption = "" cands[i] = c - if h := mfmutator.Handle(cands, i); h != "if not($N > 1) then" { + if h := mfmutator.Handle(cands, i); h != "if $N > 1 then" { t.Errorf("handle %q, want the printed form", h) } } diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 27863323db..140b9b5154 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -1376,19 +1376,14 @@ func traverseFlow( continueAfterSplitJoin(ctx, mergeID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget, labels) return } + // A decision whose true path goes straight on is printed as stored — + // `if X then else end if;` — not folded into + // `if not(X) then …`: executing the folded form stored not(X) with + // the outcomes swapped, so describe -> exec changed the model + // (ako/mxcli#1963), and a condition edit had to undo the fold + // (#1930). The builder builds the unfolded form as Studio Pro stores it. trueFlow, falseFlow := findBranchFlows(flows) - // Empty-then swap: when the true branch goes directly to the merge - // (empty then body) and the false branch has real content, negate - // the condition and swap branches for more readable output. - // "if cond then else end if;" → "if not(cond) then end if;" - if trueFlow != nil && falseFlow != nil && mergeID != "" { - if trueFlow.DestinationID == mergeID && falseFlow.DestinationID != mergeID { - stmt = negateIfCondition(stmt) - trueFlow, falseFlow = falseFlow, trueFlow - } - } - if stmt != "" { emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest, activityMap) *lines = append(*lines, indentStr+stmt) @@ -1602,15 +1597,7 @@ func traverseFlowUntilMerge( } trueFlow, falseFlow := findBranchFlows(flows) nestedMergeID = resolveNestedMergeID(nestedMergeID, mergeID, trueFlow, falseFlow, flowsByOrigin) - - // Empty-then swap: negate when true branch is empty but false branch has content. - // Skip when both branches go directly to merge (both empty). - if trueFlow != nil && falseFlow != nil && nestedMergeID != "" && nestedMergeID == mergeID { - if trueFlow.DestinationID == nestedMergeID && falseFlow.DestinationID != nestedMergeID { - stmt = negateIfCondition(stmt) - trueFlow, falseFlow = falseFlow, trueFlow - } - } + // Printed as stored, never folded into not(X) (ako/mxcli#1963). if stmt != "" { emitObjectAnnotations(obj, lines, indentStr, annotationsByTarget, flowsByOrigin, flowsByDest, activityMap) @@ -3182,81 +3169,6 @@ func (e *Executor) traverseFlow( traverseFlow(e.newExecContext(context.Background()), currentID, activityMap, flowsByOrigin, nil, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget, mergeLabels{}) } -// negateIfCondition transforms "if then" into "if not() then". -// Used by the empty-then swap to produce readable output when Studio Pro stores -// the flow with an inverted condition (true branch empty, false branch has body). -func negateIfCondition(stmt string) string { - // stmt is always "if then" from formatActivity. - const prefix = "if " - const suffix = " then" - if strings.HasPrefix(stmt, prefix) && strings.HasSuffix(stmt, suffix) { - return prefix + negateExpression(stmt[len(prefix):len(stmt)-len(suffix)]) + suffix - } - return stmt -} - -// negateExpression is cond's negation as describe spells it: not(x) becomes -// x, anything else not(cond). It is its own inverse, which is what lets a -// condition edit undo the empty-then swap (decisionPrintedNegated). -func negateExpression(cond string) string { - // Avoid double-negation: not(not(x)) → x - // Only unwrap if the outer parens are balanced (depth returns to 0 at the final char) - if strings.HasPrefix(cond, "not(") && strings.HasSuffix(cond, ")") { - inner := cond[4 : len(cond)-1] - depth := 0 - balanced := true - for _, ch := range inner { - if ch == '(' { - depth++ - } else if ch == ')' { - depth-- - if depth < 0 { - balanced = false - break - } - } - } - if balanced && depth == 0 { - return inner - } - } - return "not(" + cond + ")" -} - -// decisionPrintedNegated reports whether describe prints the decision splitID -// with the empty-then swap: its condition negated and its paths swapped, because -// its true path goes straight to its merge and its false path does not. The -// condition an `if` states for such a decision is the negation of the one -// stored, so whatever replaces it must be negated back (ako/mxcli#1930). It -// looks in the loop bodies too, each printed from its own collection. -func decisionPrintedNegated(ctx *ExecContext, oc *microflows.MicroflowObjectCollection, splitID model.ID) bool { - if oc == nil { - return false - } - activityMap := make(map[model.ID]microflows.MicroflowObject, len(oc.Objects)) - for _, obj := range oc.Objects { - activityMap[obj.GetID()] = obj - } - if _, here := activityMap[splitID]; !here { - for _, obj := range oc.Objects { - if loop, ok := obj.(*microflows.LoopedActivity); ok && decisionPrintedNegated(ctx, loop.ObjectCollection, splitID) { - return true - } - } - return false - } - var flows []*microflows.SequenceFlow - for _, f := range oc.Flows { - if f.OriginID == splitID { - flows = append(flows, f) - } - } - mergeID := findSplitMergePoints(ctx, oc, activityMap)[splitID] - trueFlow, falseFlow := findBranchFlows(flows) - return trueFlow != nil && falseFlow != nil && mergeID != "" && - trueFlow.DestinationID == mergeID && falseFlow.DestinationID != mergeID -} - func (e *Executor) collectErrorHandlerStatements( startID model.ID, activityMap map[model.ID]microflows.MicroflowObject, diff --git a/mdl/executor/cmd_microflows_traverse_test.go b/mdl/executor/cmd_microflows_traverse_test.go index e41642252a..f588420358 100644 --- a/mdl/executor/cmd_microflows_traverse_test.go +++ b/mdl/executor/cmd_microflows_traverse_test.go @@ -1269,41 +1269,17 @@ func TestTraverseFlowWithSourceMap_RecordsRange(t *testing.T) { } // ============================================================================= -// negateIfCondition +// Empty then-branch: printed as stored, not folded (ako/mxcli#1963) // ============================================================================= -func TestNegateIfCondition(t *testing.T) { - tests := []struct { - in, want string - }{ - {"if $Active then", "if not($Active) then"}, - {"if $X = 42 then", "if not($X = 42) then"}, - {"if not($Done) then", "if $Done then"}, // double-negation removal - {"if not($A and $B) then", "if $A and $B then"}, // unwrap not() - {"if true then", "if not(true) then"}, // literal - {"something else", "something else"}, // no match — passthrough - {"if find($S,'{{') >= 0 then", "if not(find($S,'{{') >= 0) then"}, // complex expression - {"if not($A) or $B) then", "if not(not($A) or $B)) then"}, // unbalanced — do NOT unwrap - } - for _, tt := range tests { - got := negateIfCondition(tt.in) - if got != tt.want { - t.Errorf("negateIfCondition(%q) = %q, want %q", tt.in, got, tt.want) - } - } -} - -// ============================================================================= -// Empty-then swap: true branch → merge produces negated condition -// ============================================================================= - -func TestTraverseFlow_EmptyThenSwap(t *testing.T) { +func TestTraverseFlow_EmptyThenPrintedAsStored(t *testing.T) { e := newTestExecutor() // Graph: start → split → (true) → merge → end // → (false) → log → merge - // Without swap: if cond then else log end if; - // With swap: if not(cond) then log end if; + // Printed: if $Active then else log end if; — the folded + // `if not($Active) then log end if;` stored not($Active) with the + // outcomes swapped when executed (ako/mxcli#1963). activityMap := map[model.ID]microflows.MicroflowObject{ mkID("start"): µflows.StartEvent{BaseMicroflowObject: mkObj("start")}, mkID("split"): µflows.ExclusiveSplit{ @@ -1342,8 +1318,8 @@ func TestTraverseFlow_EmptyThenSwap(t *testing.T) { output += l + "\n" } - if !strings.Contains(output, "not($Active)") { - t.Errorf("expected negated condition 'not($Active)', got:\n%s", output) + if strings.Contains(output, "not(") || !strings.Contains(output, "if $Active then") { + t.Errorf("expected the stored condition, not its negation, got:\n%s", output) } if strings.Count(output, "if ") != 1 { t.Errorf("expected split header to be emitted once, got:\n%s", output) @@ -1351,8 +1327,9 @@ func TestTraverseFlow_EmptyThenSwap(t *testing.T) { if !strings.Contains(output, "log node 'Test'") { t.Errorf("expected a log statement in output, got:\n%s", output) } - if strings.Contains(output, "else") { - t.Errorf("expected no empty else block, got:\n%s", output) + then, els := strings.Index(output, "if $Active then"), strings.Index(output, "else") + if els < 0 || els > strings.Index(output, "log node 'Test'") || then > els { + t.Errorf("expected the log on the false path, after else, got:\n%s", output) } } diff --git a/mdl/executor/cmd_navigation_app_title_test.go b/mdl/executor/cmd_navigation_app_title_test.go index 0861724288..27f10642da 100644 --- a/mdl/executor/cmd_navigation_app_title_test.go +++ b/mdl/executor/cmd_navigation_app_title_test.go @@ -108,7 +108,10 @@ func TestAppIconMustResolveAtExec(t *testing.T) { want string // "" = written }{ {"MyFirstModule.Images.Logo", ""}, - {"myfirstmodule.images.logo", ""}, // Mendix resolves names case-insensitively + // Mendix resolves an image reference case-sensitively: an app icon + // spelled `Atlas_Core.Content.mendix` for the stored `Mendix` is CE1613 + // at the navigation profile (measured on 11.13.0). + {"myfirstmodule.images.logo", "app icon: image collection not found: myfirstmodule.images"}, {"none", ""}, {"MyFirstModule.Images.Nope", "app icon: image not found: Nope in image collection MyFirstModule.Images"}, {"Shop.Images.Logo", "app icon: image collection not found: Shop.Images"}, diff --git a/mdl/executor/cmd_pages_builder_v3_widgets.go b/mdl/executor/cmd_pages_builder_v3_widgets.go index e55137e5a6..134ffe6cfa 100644 --- a/mdl/executor/cmd_pages_builder_v3_widgets.go +++ b/mdl/executor/cmd_pages_builder_v3_widgets.go @@ -1638,6 +1638,9 @@ func (pb *pageBuilder) buildSnippetCallParams(sc *pages.SnippetCallWidget, snipp Argument: argument, IsSnippetParameter: kind == "snippet", IsLocalVariable: kind == "local", + // A data view of the document, named for its object + // (ako/mxcli#1968), the way a flow call's argument is (#1560). + IsWidget: kind == "widget", }) } diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index 3146ce76b2..fc59a1cd57 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -1415,7 +1415,9 @@ func extractSnippetCallParams(w map[string]any) string { } ref := extractString(mm["Parameter"]) name := ref[strings.LastIndex(ref, ".")+1:] - value := pageVariableArgValue(mm["Variable"]) + // flowArgVariableValue: a data view's object is named in Widget + // (ako/mxcli#1968), which pageVariableArgValue does not read. + value := flowArgVariableValue(mm["Variable"]) if value == "" { value = strings.TrimSpace(extractString(mm["Argument"])) } diff --git a/mdl/executor/external_action_carry.go b/mdl/executor/external_action_carry.go index cfeb89e71a..b80895df3e 100644 --- a/mdl/executor/external_action_carry.go +++ b/mdl/executor/external_action_carry.go @@ -24,8 +24,15 @@ import ( // when they differ, the rewrite is refused rather than guessed. // // A tree is not carried onto a parameter that now binds another entity: its -// associations belong to the entity it was ticked on. -func carryIncludedAssociations(rebuilt, stored *microflows.MicroflowObjectCollection, qualifiedName string) error { +// associations belong to the entity it was ticked on. Nor onto a mapping whose +// statement writes `include associations ( … )` (stated), which says the tree +// itself — `()` included, which clears it. +// +// Each carry is returned, so exec can say that a statement left the clause out +// and kept the stored tree: deleting the clause removes nothing, and the +// warning is what tells the author so. +func carryIncludedAssociations(rebuilt, stored *microflows.MicroflowObjectCollection, qualifiedName string, + stated map[*microflows.ExternalActionParameterMapping]bool) ([]carriedIncludes, error) { type slot struct { entity string tree []*microflows.IncludedAssociation @@ -51,21 +58,22 @@ func carryIncludedAssociations(rebuilt, stored *microflows.MicroflowObjectCollec } if len(conflicts) > 0 { sort.Strings(conflicts) - return mdlerrors.NewUnsupported(fmt.Sprintf( + return nil, mdlerrors.NewUnsupported(fmt.Sprintf( "microflow %s calls %s more than once with different associations to include, "+ "and MDL cannot say which call has which — rewriting it would mix them up.\n"+ " Change the microflow in Studio Pro, or give the calls the same associations first.", qualifiedName, strings.Join(conflicts, ", "))) } if len(storedTrees) == 0 { - return nil + return nil, nil } + var carried []carriedIncludes used := map[string]bool{} for _, call := range externalActionCalls(rebuilt) { for _, pm := range call.ParameterMappings { key := externalActionKey(call) + "|" + strings.ToLower(pm.ParameterName) s, ok := storedTrees[key] - if !ok || len(pm.IncludedAssociations) > 0 { + if !ok || len(pm.IncludedAssociations) > 0 || stated[pm] { continue } if s.entity != "" && pm.ParameterEntity != "" && !strings.EqualFold(s.entity, pm.ParameterEntity) { @@ -74,10 +82,48 @@ func carryIncludedAssociations(rebuilt, stored *microflows.MicroflowObjectCollec // The first copy keeps the stored $IDs; a second call to the same // action gets fresh ones, since two elements of a unit cannot share one. pm.IncludedAssociations = cloneIncludedAssociations(s.tree, !used[key]) + if !used[key] { + carried = append(carried, carriedIncludes{ + call: call.ConsumedODataService + "." + call.Name, + param: pm.ParameterName, + tree: formatIncludedAssociations(s.tree), + }) + } used[key] = true } } - return nil + return carried, nil +} + +// carriedIncludes is one argument that kept its stored Associations to include +// because its statement did not write the clause. +type carriedIncludes struct { + call, param string + tree string // as the clause writes it: "Odata.Quantity (Odata.Unit)" +} + +// carriedIncludesWarning is what exec prints for a carry (ako/mxcli#1914). +func carriedIncludesWarning(flow string, c carriedIncludes) string { + return fmt.Sprintf("Warning: %s: call external action %s(%s) keeps its stored associations to include (%s), "+ + "because the statement does not write `include associations`. State them — `%s = … include associations (%s)` — "+ + "or write `include associations ()` to remove them [%s]", + flow, c.call, c.param, c.tree, c.param, c.tree, carriedIncludesRule) +} + +// carriedIncludesRule is the rule id of the omitted-clause warning. +const carriedIncludesRule = "MDL-ODATA10" + +// formatIncludedAssociations writes a tree as the clause's contents. +func formatIncludedAssociations(in []*microflows.IncludedAssociation) string { + parts := make([]string, 0, len(in)) + for _, ia := range in { + s := ia.Association + if len(ia.IncludedAssociations) > 0 { + s += " (" + formatIncludedAssociations(ia.IncludedAssociations) + ")" + } + parts = append(parts, s) + } + return strings.Join(parts, ", ") } // externalActionCalls returns every CallExternalAction in a flow, loop bodies diff --git a/mdl/executor/external_action_include_clause_test.go b/mdl/executor/external_action_include_clause_test.go new file mode 100644 index 0000000000..8d95663a40 --- /dev/null +++ b/mdl/executor/external_action_include_clause_test.go @@ -0,0 +1,298 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1914: `include associations ( … )` on a call external action +// argument writes the argument's Associations to include. The tree Studio Pro +// stored on ako/TestApp's Odata.IncludeAssociationsMF (11.16.0) is Quantity +// ticked, Unit ticked under it, both IsParent — each written from its FROM end. +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +func TestIncludeAssociations_BuildsStudioProTree(t *testing.T) { + calls := rewriteExternalActionFlow(t, nil, `call external action Odata.Bug1914.CreateBuffer( + command = $Cmd include associations (Odata.Quantity (Odata.Unit)), + additional = empty);`) + cmd := mappingNamed(calls[0], "command") + if got, want := includedAssociationsShape(cmd.IncludedAssociations), + includedAssociationsShape(storedIncludedAssociations()); got != want { + t.Fatalf("tree = %s, want Studio Pro's %s", got, want) + } + for _, ia := range []*microflows.IncludedAssociation{cmd.IncludedAssociations[0], cmd.IncludedAssociations[0].IncludedAssociations[0]} { + if ia.ID == "" { + t.Errorf("%s has no $ID", ia.Association) + } + } + if add := mappingNamed(calls[0], "additional"); len(add.IncludedAssociations) != 0 { + t.Errorf("an argument without the clause got a tree: %+v", add.IncludedAssociations) + } +} + +// IsParent is false when the object is the association's TO end: Owner is from +// QuantityParameterType to CreateBufferCommand, and Unit is then reached from +// QuantityParameterType, Owner's far end. +func TestIncludeAssociations_IsParentFromTheToEnd(t *testing.T) { + calls := rewriteExternalActionFlow(t, nil, + `call external action Odata.Bug1914.CreateBuffer(command = $Cmd include associations (Odata.Owner (Odata.Unit)));`) + tree := mappingNamed(calls[0], "command").IncludedAssociations + if got := includedAssociationsShape(tree); got != "(Odata.Owner false (Odata.Unit true ))" { + t.Errorf("tree = %s, want Owner from its TO end, Unit from QuantityParameterType", got) + } +} + +func TestIncludeAssociations_Refusals(t *testing.T) { + for _, tc := range []struct{ name, clause, want string }{ + {"self-association", "(Odata.Self)", "Odata.Self is a self-association"}, + {"association that does not touch the entity", "(Odata.Elsewhere)", + "Odata.Elsewhere does not touch Odata.CreateBufferCommand"}, + {"nested association that does not touch the far end", "(Odata.Quantity (Odata.Owner (Odata.Elsewhere)))", + "Odata.Elsewhere does not touch Odata.CreateBufferCommand"}, + {"unknown association", "(Odata.NoSuch)", "include associations Odata.NoSuch: association not found"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, _, err := runExternalActionFlow(t, nil, + "call external action Odata.Bug1914.CreateBuffer(command = $Cmd include associations "+tc.clause+");") + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("err = %v, want %q", err, tc.want) + } + }) + } +} + +// The clause is the whole tree: it replaces the stored one, `()` clears it, +// and neither warns. Left out, the stored tree is carried and exec warns. +func TestIncludeAssociations_ClauseOverStoredTree(t *testing.T) { + t.Run("restated with less", func(t *testing.T) { + calls, out, err := runExternalActionFlow(t, storedCreateBufferCall(), + `call external action Odata.Bug1914.CreateBuffer(command = $Cmd include associations (Odata.Quantity), additional = empty);`) + if err != nil { + t.Fatal(err) + } + if got := includedAssociationsShape(mappingNamed(calls[0], "command").IncludedAssociations); got != "(Odata.Quantity true )" { + t.Errorf("tree = %s, want Quantity alone — Unit removed", got) + } + if strings.Contains(out, carriedIncludesRule) { + t.Errorf("a stated clause warned:\n%s", out) + } + }) + t.Run("empty clause clears", func(t *testing.T) { + calls, out, err := runExternalActionFlow(t, storedCreateBufferCall(), + `call external action Odata.Bug1914.CreateBuffer(command = $Cmd include associations (), additional = empty);`) + if err != nil { + t.Fatal(err) + } + if tree := mappingNamed(calls[0], "command").IncludedAssociations; len(tree) != 0 { + t.Errorf("include associations () kept %s", includedAssociationsShape(tree)) + } + if strings.Contains(out, carriedIncludesRule) { + t.Errorf("an empty clause warned:\n%s", out) + } + }) + t.Run("omitted clause carries and warns", func(t *testing.T) { + calls, out, err := runExternalActionFlow(t, storedCreateBufferCall(), + `call external action Odata.Bug1914.CreateBuffer(command = $Cmd, additional = empty);`) + if err != nil { + t.Fatal(err) + } + if tree := mappingNamed(calls[0], "command").IncludedAssociations; len(tree) != 1 { + t.Errorf("the stored tree was not carried: %s", includedAssociationsShape(tree)) + } + for _, want := range []string{carriedIncludesRule, "Odata.Bug1914.CreateBuffer(command)", + "(Odata.Quantity (Odata.Unit))", "include associations ()"} { + if !strings.Contains(out, want) { + t.Errorf("the warning does not say %q:\n%s", want, out) + } + } + }) + t.Run("control: no stored tree, no clause, no warning", func(t *testing.T) { + _, out, err := runExternalActionFlow(t, nil, + `call external action Odata.Bug1914.CreateBuffer(command = $Cmd, additional = empty);`) + if err != nil { + t.Fatal(err) + } + if strings.Contains(out, carriedIncludesRule) { + t.Errorf("warned with nothing to carry:\n%s", out) + } + }) +} + +// describe prints the stored tree as the clause, so describe -> exec restates +// it; a tree holding a self-association is printed as a comment. +func TestIncludeAssociations_Describe(t *testing.T) { + mod := mkModule("Odata") + dm := includedAssocDomainModel(mod.ID) + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { return []*domainmodel.DomainModel{dm}, nil }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + + got := formatAction(ctx, storedCreateBufferCall().Action, nil, nil) + if want := "command = $Cmd include associations (Odata.Quantity (Odata.Unit)), additional = empty"; !strings.Contains(got, want) { + t.Errorf("describe = %s\nwant it to contain %s", got, want) + } + + // A self-association cannot be written back: printed as a comment, which + // parses, so the statement still runs and the carry keeps the tree. + self := storedCreateBufferCall() + self.Action.(*microflows.CallExternalAction).ParameterMappings[0].IncludedAssociations = + []*microflows.IncludedAssociation{{Association: "Odata.Self", IsParent: true}} + got = formatAction(ctx, self.Action, nil, nil) + if strings.Contains(got, "$Cmd include associations") || + !strings.Contains(got, "/* include associations (Odata.Self): holds the self-association Odata.Self") { + t.Errorf("describe = %s\nwant the self-association as a comment", got) + } + if _, errs := visitor.Build("create microflow Odata.F ($Cmd: Odata.CreateBufferCommand)\nbegin\n" + got + "\nend;"); len(errs) > 0 { + t.Errorf("the described call does not parse: %v\n%s", errs, got) + } +} + +// A non-empty clause needs Microflows$IncludedAssociation, introduced in +// 10.11.0; `include associations ()` writes nothing and is not gated. +func TestIncludeAssociations_VersionGate(t *testing.T) { + at := func(major, minor int) *ExecContext { + ctx, _ := newMockCtx(t, withBackend(&mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ProjectVersionFunc: func() *types.ProjectVersion { + return &types.ProjectVersion{MajorVersion: major, MinorVersion: minor} + }, + })) + return ctx + } + tree := parseMicroflowStmt(t, "create microflow M.F ($Cmd: M.Cmd) begin\n"+ + " call external action M.Svc.Act(command = $Cmd include associations (M.Assoc));\nend;") + none := parseMicroflowStmt(t, "create microflow M.F ($Cmd: M.Cmd) begin\n"+ + " call external action M.Svc.Act(command = $Cmd include associations ());\nend;") + if _, err := buildMicroflowFromStmt(at(10, 10), tree, buildFlowOpts{}); err == nil || + !strings.Contains(err.Error(), "include associations") || !strings.Contains(err.Error(), "10.11") { + t.Errorf("10.10: err = %v, want the include associations refusal naming 10.11", err) + } + if _, err := buildMicroflowFromStmt(at(10, 11), tree, buildFlowOpts{}); err != nil && strings.Contains(err.Error(), "10.11") { + t.Errorf("10.11 satisfies the minimum; got: %v", err) + } + if _, err := buildMicroflowFromStmt(at(10, 10), none, buildFlowOpts{}); err != nil && strings.Contains(err.Error(), "include associations") { + t.Errorf("an empty clause writes nothing and must not be gated; got: %v", err) + } +} + +// check -p says what exec will warn: an omitted clause over a stored tree. +func TestIncludeAssociations_CheckWarnsOnCarry(t *testing.T) { + mod := mkModule("Odata") + h := mkHierarchy(mod) + mf := mkMicroflow(mod.ID, "ACT_Buffer") + mf.ObjectCollection = µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{storedCreateBufferCall()}} + ctx, _ := newMockCtx(t, withBackend(&mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return []*microflows.Microflow{mf}, nil }, + }), withHierarchy(h)) + run := func(script string) []string { + prog, errs := visitor.Build(script) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var msgs []string + for _, v := range CheckCarriedIncludedAssociations(ctx, prog) { + msgs = append(msgs, v.RuleID+": "+v.Message) + } + return msgs + } + flow := func(args string) string { + return "create or modify microflow Odata.ACT_Buffer ($Cmd: Odata.CreateBufferCommand)\nbegin\n" + + " call external action Odata.Bug1914.CreateBuffer(" + args + ");\nend;" + } + if got := run(flow("command = $Cmd, additional = empty")); len(got) != 1 || + !strings.Contains(got[0], carriedIncludesRule) || !strings.Contains(got[0], "(Odata.Quantity (Odata.Unit))") { + t.Errorf("omitted clause: %q, want one MDL-ODATA10 naming the stored tree", got) + } + for name, script := range map[string]string{ + "stated": flow("command = $Cmd include associations (Odata.Quantity), additional = empty"), + "stated empty": flow("command = $Cmd include associations (), additional = empty"), + "drop + create": "drop microflow Odata.ACT_Buffer;\n" + strings.Replace(flow("command = $Cmd"), "create or modify", "create", 1), + "another action": strings.Replace(flow("command = $Other"), "Bug1914.CreateBuffer(", "Bug1914.CreateOther(", 1), + } { + if got := run(script); len(got) != 0 { + t.Errorf("%s: %q, want no warning", name, got) + } + } +} + +// check refuses what exec's builder refuses, against the same contract and +// domain model — and leaves an association the project does not hold to exec, +// since an earlier statement may create it. +func TestIncludeAssociations_CheckAgreesWithExec(t *testing.T) { + mod := mkModule("Odata") + dm := includedAssocDomainModel(mod.ID) + ctx, _ := newMockCtx(t, withBackend(&mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + GetModuleByNameFunc: func(name string) (*model.Module, error) { + if name == mod.Name { + return mod, nil + } + return nil, nil + }, + GetDomainModelFunc: func(model.ID) (*domainmodel.DomainModel, error) { return dm, nil }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { return []*domainmodel.DomainModel{dm}, nil }, + ListConsumedODataServicesFunc: func() ([]*model.ConsumedODataService, error) { + return []*model.ConsumedODataService{{BaseElement: model.BaseElement{ID: "svc"}, ContainerID: mod.ID, + Name: "Bug1914", Metadata: includedAssocMetadata}}, nil + }, + }), withHierarchy(mkHierarchy(mod))) + check := func(clause string) string { + prog, errs := visitor.Build("create microflow Odata.F ($Cmd: Odata.CreateBufferCommand)\nbegin\n" + + " call external action Odata.Bug1914.CreateBuffer(command = $Cmd include associations " + clause + ");\nend;") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var msgs []string + for _, e := range validateExternalActionCalls(ctx, prog) { + msgs = append(msgs, e.Error()) + } + return strings.Join(msgs, "\n") + } + for clause, want := range map[string]string{ + "(Odata.Self)": "Odata.Self is a self-association", + "(Odata.Elsewhere)": "Odata.Elsewhere does not touch Odata.CreateBufferCommand", + "(Odata.Quantity (Odata.Owner))": "", // Owner touches QuantityParameterType from its FROM end + "(Odata.Quantity (Odata.Unit))": "", + "(Odata.CreatedLaterInTheScript)": "", // not in the project: exec's to resolve + } { + got := check(clause) + if want == "" && strings.Contains(got, "include associations") { + t.Errorf("%s: refused: %s", clause, got) + } + if want != "" && !strings.Contains(got, want) { + t.Errorf("%s: %q, want %q", clause, got, want) + } + } +} + +// A call in a loop body is built by a nested builder: what it states must +// reach the carry, or `include associations ()` there gets the stored tree back. +func TestIncludeAssociations_StatedInANestedBody(t *testing.T) { + calls, out, err := runExternalActionFlow(t, storedCreateBufferCall(), `while $Cmd != empty +begin + call external action Odata.Bug1914.CreateBuffer(command = $Cmd include associations (), additional = empty); + break; +end while;`) + if err != nil { + t.Fatal(err) + } + if len(calls) != 0 { + t.Fatalf("the call is in the loop, not the top level: %d", len(calls)) + } + if strings.Contains(out, carriedIncludesRule) { + t.Errorf("a stated clause in a loop body was carried over:\n%s", out) + } +} diff --git a/mdl/executor/external_action_include_splice_pedapp_test.go b/mdl/executor/external_action_include_splice_pedapp_test.go new file mode 100644 index 0000000000..6e2dd40050 --- /dev/null +++ b/mdl/executor/external_action_include_splice_pedapp_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "path/filepath" + "strings" + "testing" +) + +// ako/mxcli#1914, against the real backend: a create or modify that restates +// a stored flow takes the SPLICE path, which rebuilt the call external action +// and dropped its Associations to include — the #1922 carry only covered the +// full rebuild. Measured on PedApp with the reporter's service shape (a +// command DTO with a Quantity sub-DTO, and a Unit under it). +func TestIncludeAssociations_SpliceKeepsTheStoredTree(t *testing.T) { + exec, out, _ := openPedAppCopy(t) + metadata, err := filepath.Abs(filepath.Join("testdata", "bug1914_metadata.xml")) + if err != nil { + t.Fatal(err) + } + setup := `mdl 1; +create constant MyFirstModule.SvcUrl1914 ( Type: String, DefaultValue: 'https://example.com/odata/' ); +create consumed odata service MyFirstModule.Bug1914 ( Version: '1.0', ODataVersion: OData4, + MetadataUrl: '` + metadata + `', Timeout: 300, ServiceUrl: MyFirstModule.SvcUrl1914 ); +create external entities from MyFirstModule.Bug1914 into MyFirstModule;` + if err := agreeExec(t, exec, setup); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + flow := func(clause string) string { + return `mdl 1; +create or modify microflow MyFirstModule.ACT_Buffer1914 ($Cmd: MyFirstModule.CreateBufferCommand) +begin + call external action MyFirstModule.Bug1914.CreateBuffer(command = $Cmd` + clause + `); +end;` + } + describe := func() string { + t.Helper() + out.Reset() + if err := agreeExec(t, exec, "describe microflow MyFirstModule.ACT_Buffer1914;"); err != nil { + t.Fatal(err) + } + return out.String() + } + const tree = " include associations (MyFirstModule.Quantity (MyFirstModule.Unit_2))" + if err := agreeExec(t, exec, flow(tree)); err != nil { + t.Fatalf("create: %v\n%s", err, out.String()) + } + if d := describe(); !strings.Contains(d, "command = $Cmd"+tree+")") { + t.Fatalf("the authored tree does not describe back:\n%s", d) + } + + out.Reset() + if err := agreeExec(t, exec, flow("")); err != nil { + t.Fatalf("restate without the clause: %v\n%s", err, out.String()) + } + if !strings.Contains(out.String(), carriedIncludesRule) || !strings.Contains(out.String(), "spliced") { + t.Errorf("the splice did not warn about the carried tree:\n%s", out.String()) + } + if d := describe(); !strings.Contains(d, "command = $Cmd"+tree+")") { + t.Errorf("the splice dropped the stored tree:\n%s", d) + } + + out.Reset() + if err := agreeExec(t, exec, flow(" include associations ()")); err != nil { + t.Fatalf("clear: %v\n%s", err, out.String()) + } + if d := describe(); strings.Contains(d, "include associations") || strings.Contains(out.String(), carriedIncludesRule) { + t.Errorf("include associations () did not clear the tree quietly:\n%s", d) + } +} diff --git a/mdl/executor/external_action_included_associations_test.go b/mdl/executor/external_action_included_associations_test.go index 9674dac8f2..77a46554ac 100644 --- a/mdl/executor/external_action_included_associations_test.go +++ b/mdl/executor/external_action_included_associations_test.go @@ -76,10 +76,48 @@ func storedCreateBufferCall() *microflows.ActionActivity { }} } +// includedAssocDomainModel is the Odata module: the reporter's DTOs, the +// associations the fixture ticks (Quantity: command -> quantity, Unit: +// quantity -> unit), and three probes — Owner reaches the command from its TO +// end, Self links the command to itself, Elsewhere does not touch it. +func includedAssocDomainModel(modID model.ID) *domainmodel.DomainModel { + entity := func(name string) *domainmodel.Entity { + return &domainmodel.Entity{ + BaseElement: model.BaseElement{ID: nextID("ent")}, + Name: name, + RemoteServiceName: "Odata.Bug1914", + RemoteEntityName: name, + } + } + cmd, qty, other, unit := entity("CreateBufferCommand"), entity("QuantityParameterType"), entity("OtherCommand"), entity("Unit") + assoc := func(name string, from, to *domainmodel.Entity) *domainmodel.Association { + return &domainmodel.Association{BaseElement: model.BaseElement{ID: nextID("as")}, Name: name, ParentID: from.ID, ChildID: to.ID} + } + return &domainmodel.DomainModel{ + ContainerID: modID, + Entities: []*domainmodel.Entity{cmd, qty, other, unit}, + Associations: []*domainmodel.Association{ + assoc("Quantity", cmd, qty), assoc("Unit", qty, unit), + assoc("Owner", qty, cmd), assoc("Self", cmd, cmd), assoc("Elsewhere", other, unit), + }, + } +} + // rewriteExternalActionFlow runs CREATE OR MODIFY of Odata.ACT_Buffer with body // against a stored microflow holding stored (nil: there is no stored // microflow), and returns the CallExternalActions the rewrite wrote. func rewriteExternalActionFlow(t *testing.T, stored *microflows.ActionActivity, body string) []*microflows.CallExternalAction { + t.Helper() + calls, _, err := runExternalActionFlow(t, stored, body) + if err != nil { + t.Fatalf("create or modify: %v", err) + } + return calls +} + +// runExternalActionFlow is rewriteExternalActionFlow returning exec's output +// and error rather than failing on one. +func runExternalActionFlow(t *testing.T, stored *microflows.ActionActivity, body string) ([]*microflows.CallExternalAction, string, error) { t.Helper() mod := mkModule("Odata") h := mkHierarchy(mod) @@ -90,26 +128,28 @@ func rewriteExternalActionFlow(t *testing.T, stored *microflows.ActionActivity, mf.ObjectCollection = µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{stored}} existing = append(existing, mf) } - entity := func(name string) *domainmodel.Entity { - return &domainmodel.Entity{ - BaseElement: model.BaseElement{ID: nextID("ent")}, - Name: name, - RemoteServiceName: "Odata.Bug1914", - RemoteEntityName: name, - } - } + dm := includedAssocDomainModel(mod.ID) var written *microflows.Microflow mb := &mock.MockBackend{ IsConnectedFunc: func() bool { return true }, ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + GetModuleByNameFunc: func(name string) (*model.Module, error) { + if name == mod.Name { + return mod, nil + } + return nil, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { + if id == mod.ID { + return dm, nil + } + return nil, nil + }, ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return existing, nil }, ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { - return []*domainmodel.DomainModel{{ - ContainerID: mod.ID, - Entities: []*domainmodel.Entity{entity("CreateBufferCommand"), entity("QuantityParameterType"), entity("OtherCommand")}, - }}, nil + return []*domainmodel.DomainModel{dm}, nil }, ListConsumedODataServicesFunc: func() ([]*model.ConsumedODataService, error) { return []*model.ConsumedODataService{{ @@ -123,7 +163,7 @@ func rewriteExternalActionFlow(t *testing.T, stored *microflows.ActionActivity, CreateMicroflowFunc: func(mf *microflows.Microflow) error { written = mf; return nil }, UpdateMicroflowFunc: func(mf *microflows.Microflow) error { written = mf; return nil }, } - ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + ctx, out := newMockCtx(t, withBackend(mb), withHierarchy(h)) prog, errs := visitor.Build("create or modify microflow Odata.ACT_Buffer ($Cmd: Odata.CreateBufferCommand, $Other: Odata.OtherCommand)\nbegin\n" + body + "\nend;") @@ -131,7 +171,7 @@ func rewriteExternalActionFlow(t *testing.T, stored *microflows.ActionActivity, t.Fatalf("parse: %v", errs) } if err := execCreateMicroflow(ctx, prog.Statements[0].(*ast.CreateMicroflowStmt)); err != nil { - t.Fatalf("create or modify: %v", err) + return nil, out.String(), err } if written == nil { t.Fatal("nothing was written") @@ -144,7 +184,7 @@ func rewriteExternalActionFlow(t *testing.T, stored *microflows.ActionActivity, } } } - return calls + return calls, out.String(), nil } func mappingNamed(c *microflows.CallExternalAction, name string) *microflows.ExternalActionParameterMapping { @@ -229,8 +269,8 @@ func TestCreateOrModify_IncludedAssociationsTwoCalls(t *testing.T) { rebuilt.Objects = append(rebuilt.Objects, a) calls = append(calls, a.Action.(*microflows.CallExternalAction)) } - err := carryIncludedAssociations(rebuilt, - µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{stored}}, "Odata.ACT_Buffer") + _, err := carryIncludedAssociations(rebuilt, + µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{stored}}, "Odata.ACT_Buffer", nil) return calls, err } diff --git a/mdl/executor/flow_splice_negated_condition_pedapp_test.go b/mdl/executor/flow_splice_negated_condition_pedapp_test.go index ff4de1b66b..e7c65a371e 100644 --- a/mdl/executor/flow_splice_negated_condition_pedapp_test.go +++ b/mdl/executor/flow_splice_negated_condition_pedapp_test.go @@ -3,9 +3,12 @@ package executor import ( + "fmt" + "sort" "strings" "testing" + "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/microflows" ) @@ -55,20 +58,19 @@ func TestCreateOrModify_FoldedDecisionKeepsItsMeaning(t *testing.T) { name, stored, printed, edited, want string }{ { - // The reporter's flow. - name: "printed as not(X)", + // The reporter's flow. describe prints it as stored since + // ako/mxcli#1963, so the edited condition is stored as written. + name: "printed as stored", stored: "trim($Sale/Code) != ''", - printed: "if not(trim($Sale/Code) != '') then", - edited: "if not(trim($Sale/Reference) != '') then", + printed: "if trim($Sale/Code) != '' then", + edited: "if trim($Sale/Reference) != '' then", want: "trim($Sale/Reference) != ''", }, { - // describe takes a stored not(Z) out of the fold rather than - // printing not(not(Z)); the edit must put it back. - name: "stored not(Z), printed as Z", + name: "stored not(Z), printed as stored", stored: "not($Sale/Code = empty)", - printed: "if $Sale/Code = empty then", - edited: "if $Sale/Reference = empty then", + printed: "if not($Sale/Code = empty) then", + edited: "if not($Sale/Reference = empty) then", want: "not($Sale/Reference = empty)", }, } { @@ -127,8 +129,8 @@ begin end loop; return $Valid; end;` - const printed = "if not(trim($Sale/Code) != '') then" - const edited = "if not(trim($Sale/Reference) != '') then" + const printed = "if trim($Sale/Code) != '' then" + const edited = "if trim($Sale/Reference) != '' then" exec, out, _ := openPedAppCopy(t) if err := agreeExec(t, exec, setup); err != nil { @@ -166,3 +168,53 @@ end;` t.Fatalf("stored expression in the loop = %q, want it unchanged, %q", expr, want) } } + +// ako/mxcli#1963 (mendixlabs/mxcli#1742): describe folded a decision whose true +// path goes straight on into `if not(X) then end if;`, and +// executing that description stored not(X) with the outcomes swapped — the +// same behaviour, a different model. The reporter's steps: describe the flow, +// execute the description under a new name, compare the decisions. +func TestDescribeExec_EmptyThenDecisionRoundTrips(t *testing.T) { + exec, out, _ := openPedAppCopy(t) + if err := agreeExec(t, exec, strings.Replace(negatedSetup, "%s", "$Sale/Code != empty", 1)); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + described := describeFlow(t, exec, "VAL_Sale1930") + if strings.Contains(described, "not(") { + t.Errorf("describe folded the decision:\n%s", described) + } + src := strings.Replace(described, "VAL_Sale1930", "VAL_Sale1930_RoundTrip", 1) + if !strings.HasPrefix(strings.TrimSpace(src), "mdl ") { + src = "mdl 1;\n" + src + } + out.Reset() + if err := agreeExec(t, exec, src); err != nil { + t.Fatalf("exec of the description: %v\n%s", err, out.String()) + } + shape := func(name string) string { + t.Helper() + mf := loopEditFlowOf(t, exec, name) + kinds := map[model.ID]string{} + var split *microflows.ExclusiveSplit + for _, obj := range mf.ObjectCollection.Objects { + kinds[obj.GetID()] = fmt.Sprintf("%T", obj) + if s, ok := obj.(*microflows.ExclusiveSplit); ok { + split = s + } + } + if split == nil { + t.Fatalf("%s has no decision", name) + } + var outcomes []string + for _, f := range mf.ObjectCollection.Flows { + if f.OriginID == split.ID { + outcomes = append(outcomes, fmt.Sprintf("%v→%s", f.CaseValue, kinds[f.DestinationID])) + } + } + sort.Strings(outcomes) + return split.SplitCondition.(*microflows.ExpressionSplitCondition).Expression + " " + strings.Join(outcomes, " ") + } + if a, b := shape("VAL_Sale1930"), shape("VAL_Sale1930_RoundTrip"); a != b { + t.Errorf("the round trip changed the decision:\n original %s\n round trip %s", a, b) + } +} diff --git a/mdl/executor/snippetcall_widget_argument_test.go b/mdl/executor/snippetcall_widget_argument_test.go new file mode 100644 index 0000000000..89e37d6589 --- /dev/null +++ b/mdl/executor/snippetcall_widget_argument_test.go @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// ako/mxcli#1968 (mendixlabs/mxcli#1746): `snippetcall … Params: (Item = +// $dvOuter)` passes a data view's object by naming the data view. It was +// classified as a page parameter and written in PageParameter, a page +// parameter the page does not have: mx check CE0115. A document widget's +// name is the Widget slot's, as it is for a flow call's argument (#1560). +func TestSnippetCall_DataViewNameIsAWidgetArgument(t *testing.T) { + mod := mkModule("Mod") + snp := mkSnippetWithParam(mod.ID, "MySnippet", "Asset") + h := mkHierarchy(mod) + withContainer(h, snp.ContainerID, mod.ID) + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListFoldersFunc: func() ([]*types.FolderInfo, error) { return nil, nil }, + ListSnippetsFunc: func() ([]*pages.Snippet, error) { return []*pages.Snippet{snp}, nil }, + } + build := func(arg string) pages.SnippetParamMapping { + t.Helper() + pb := newPageBuilder(mb, h, "Mod") + pb.isDocumentWidget = func(name string) bool { return name == "dvOuter" } + pb.paramScope["Asset"] = "p1" + sc, err := pb.buildSnippetCallV3(&ast.WidgetV3{Type: "SNIPPETCALL", Name: "sc1", Properties: map[string]any{ + "Snippet": "Mod.MySnippet", + "Params": []ast.SnippetCallParam{{ParamName: "Asset", Variable: arg}}, + }}) + if err != nil { + t.Fatal(err) + } + if len(sc.ParameterMappings) != 1 { + t.Fatalf("mappings = %d, want 1", len(sc.ParameterMappings)) + } + return sc.ParameterMappings[0] + } + if m := build("$dvOuter"); !m.IsWidget || m.IsSnippetParameter || m.IsLocalVariable { + t.Errorf("$dvOuter: %+v, want a widget argument", m) + } + // Control: a page parameter of that page stays one. + if m := build("$Asset"); m.IsWidget { + t.Errorf("$Asset: %+v, want a page parameter", m) + } +} + +// describe reads the Widget slot back as `$dvOuter`, so describe -> exec +// round-trips the call; it read PageParameter / SnippetParameter / +// LocalVariable only and printed the argument as absent. +func TestSnippetCall_DescribeReadsTheWidgetSlot(t *testing.T) { + call := func(variable map[string]any) map[string]any { + return map[string]any{"FormCall": map[string]any{"ParameterMappings": []any{ + int32(2), + map[string]any{"Parameter": "ModD.SN_Item.Item", "Variable": variable}, + }}} + } + if got := extractSnippetCallParams(call(map[string]any{"Widget": "dvOuter", "PageParameter": ""})); got != "Item = $dvOuter" { + t.Errorf("widget slot: %q, want Item = $dvOuter", got) + } + if got := extractSnippetCallParams(call(map[string]any{"PageParameter": "Note"})); got != "Item = $Note" { + t.Errorf("control, page parameter: %q", got) + } +} diff --git a/mdl/executor/testdata/bug1914_metadata.xml b/mdl/executor/testdata/bug1914_metadata.xml new file mode 100644 index 0000000000..faa69ea9fb --- /dev/null +++ b/mdl/executor/testdata/bug1914_metadata.xml @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 415f74e2bb..4fd383b552 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -62,7 +62,9 @@ type scriptContext struct { queues map[string]bool // Image collections created (Module.Collection) and the images they declare - // (Module.Collection.Image), both lower-cased like buildImageQualifiedNames. + // (Module.Collection.Image), both spelled as declared, like + // buildImageQualifiedNames — Mendix resolves an image reference + // case-sensitively. // Missing, an image widget naming a collection the same script creates was // "image collection not found" and exec refused the script // (mendixlabs/mxcli#1419). @@ -382,10 +384,10 @@ func (sc *scriptContext) collectSingle(stmt ast.Statement) { } case *ast.CreateImageCollectionStmt: if s.Name.Module != "" { - qn := strings.ToLower(s.Name.String()) + qn := s.Name.String() sc.imageCollections[qn] = true for _, img := range s.Images { - sc.images[qn+"."+strings.ToLower(img.Name)] = true + sc.images[qn+"."+img.Name] = true } } case *ast.CreateWorkflowStmt: diff --git a/mdl/executor/validate_alter_set.go b/mdl/executor/validate_alter_set.go index 5b47cf1110..7e7d841d61 100644 --- a/mdl/executor/validate_alter_set.go +++ b/mdl/executor/validate_alter_set.go @@ -94,6 +94,7 @@ func validateAlterSetProperties(ctx *ExecContext, prog *ast.Program, sc *scriptC } label := fmt.Sprintf("alter %s %s", containerType, s.PageName.String()) modName := h.GetModuleName(containerID) + var earlier []*ast.SetPropertyOp for _, op := range s.Operations { set, ok := op.(*ast.SetPropertyOp) if !ok { @@ -111,11 +112,47 @@ func validateAlterSetProperties(ctx *ExecContext, prog *ast.Program, sc *scriptC continue } errs = append(errs, checkSetOp(ctx, probe, label, set, modName, containerID, sc)...) + errs = append(errs, checkDateFormatSet(ctx, probe, label, earlier, set, modName, containerID)...) + earlier = append(earlier, set) } } return errs } +// checkDateFormatSet is exec's checkDateFormatAfterSet for check: a date +// picker left `DateFormat: Custom` with no pattern is CE0493, judged on the +// widget as the statement leaves it — its earlier SETs on that widget applied +// first, then this one whole — because the format and its pattern may be set +// in either order, in one SET or in two (ako/mxcli#1965). +func checkDateFormatSet(ctx *ExecContext, p pageProbe, label string, earlier []*ast.SetPropertyOp, op *ast.SetPropertyOp, + modName string, modID model.ID) []error { + touches := false + for name := range op.Properties { + if strings.EqualFold(name, "DateFormat") || strings.EqualFold(name, "CustomDateFormat") { + touches = true + } + } + if !touches || op.Target.IsColumn() || op.Target.Item != nil || op.Target.Widget == "" { + return nil + } + probe, err := p.Probe() + if err != nil { + return nil + } + for _, e := range earlier { + if e.Target.Widget == op.Target.Widget && !e.Target.IsColumn() && e.Target.Item == nil { + _ = applySetProperties(ctx, probe, e, modName, modID) // its own errors are reported for it + } + } + if applySetProperties(ctx, probe, op, modName, modID) != nil { + return nil // reported by checkSetOp + } + if err := checkDateFormatAfterSet(probe, op); err != nil { + return []error{mdlerrors.NewValidation(fmt.Sprintf("%s: %v", label, err))} + } + return nil +} + // openPageProbe opens the stored document and returns it as a prober, or nil // when it cannot be established — an engine without the capability, a unit that // will not load. Silence, never a finding: a false "no such property" blocks a diff --git a/mdl/executor/validate_external_action_calls.go b/mdl/executor/validate_external_action_calls.go index 07a6bed23b..392ca5f920 100644 --- a/mdl/executor/validate_external_action_calls.go +++ b/mdl/executor/validate_external_action_calls.go @@ -120,10 +120,39 @@ func validateExternalActionCalls(ctx *ExecContext, prog *ast.Program) []error { if err := checkExternalActionCall(ctx, h, services, c.call); err != nil { errs = append(errs, fmt.Errorf("statement %d: %w", c.index, err)) } + for _, msg := range checkIncludedAssociations(ctx, h, c.call) { + errs = append(errs, fmt.Errorf("statement %d: %s", c.index, msg)) + } } return errs } +// checkIncludedAssociations resolves each `include associations ( … )` the +// call writes as exec's builder does (ako/mxcli#1914), so check refuses what +// exec would: a self-association, an association that does not touch the +// entity on its side, one whose side specializes both ends. +func checkIncludedAssociations(ctx *ExecContext, h *ContainerHierarchy, c externalCall) []string { + stated := false + for _, a := range c.stmt.Arguments { + if a.IncludeAssociations != nil && len(a.IncludeAssociations.Associations) > 0 { + stated = true + } + } + if !stated { + return nil + } + fb := &flowBuilder{backend: flowBuildBackend(ctx.Backend), hierarchy: h, resolveKnownOnly: true} + kinds := fb.resolveExternalActionParameterKinds(c.stmt.ServiceName, c.stmt.ActionName) + call := c.stmt.ServiceName.String() + "." + c.stmt.ActionName + for _, a := range c.stmt.Arguments { + if a.IncludeAssociations == nil { + continue + } + fb.buildIncludedAssociations(call, a.Name, kinds[strings.ToLower(a.Name)].entity, a.IncludeAssociations.Associations) + } + return fb.GetErrors() +} + func checkExternalActionCall(ctx *ExecContext, h *ContainerHierarchy, services []*model.ConsumedODataService, c externalCall) error { svcQN := c.stmt.ServiceName.String() diff --git a/mdl/executor/validate_external_action_includes.go b/mdl/executor/validate_external_action_includes.go new file mode 100644 index 0000000000..c70abf2a25 --- /dev/null +++ b/mdl/executor/validate_external_action_includes.go @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "context" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// CheckCarriedIncludedAssociations is the check -p half of the omitted-clause +// warning (MDL-ODATA10, ako/mxcli#1914): a create or modify of a stored +// microflow whose call external action argument holds Associations to include, +// restated without `include associations ( … )`. exec keeps the stored tree +// (the #1922 carry) and says so; check says it before the write, so an author +// who deleted the clause to remove the associations learns that it did not. +// +// The pairing is the carry's — service, action and parameter name — so the +// two agree on which arguments are carried. A warning, never an error: keeping +// what Studio Pro ticked is the safe outcome, and the script may rely on it. +func (e *Executor) CheckCarriedIncludedAssociations(prog *ast.Program) []linter.Violation { + if e == nil { + return nil + } + return CheckCarriedIncludedAssociations(e.newExecContext(context.Background()), prog) +} + +// CheckCarriedIncludedAssociations is the ExecContext-level entry point. +func CheckCarriedIncludedAssociations(ctx *ExecContext, prog *ast.Program) []linter.Violation { + if ctx == nil || prog == nil || !ctx.Connected() { + return nil + } + dropped := map[string]bool{} + var out []linter.Violation + for _, stmt := range prog.Statements { + switch s := stmt.(type) { + case *ast.DropMicroflowStmt: + // drop + create starts from nothing: there is no stored tree to carry. + dropped[s.Name.String()] = true + case *ast.CreateMicroflowStmt: + qn := s.Name.String() + if !s.CreateOrModify || dropped[qn] { + continue + } + stored := storedMicroflowObjects(ctx, s.Name) + if stored == nil { + continue + } + trees := map[string][]*microflows.IncludedAssociation{} + for _, call := range externalActionCalls(stored) { + for _, pm := range call.ParameterMappings { + if len(pm.IncludedAssociations) > 0 { + trees[externalActionKey(call)+"|"+strings.ToLower(pm.ParameterName)] = pm.IncludedAssociations + } + } + } + if len(trees) == 0 { + continue + } + warned := map[string]bool{} + forEachMicroflowStatement(s.Body, func(st ast.MicroflowStatement) { + call, ok := st.(*ast.CallExternalActionStmt) + if !ok { + return + } + callQN := call.ServiceName.String() + "." + call.ActionName + for _, arg := range call.Arguments { + if arg.IncludeAssociations != nil { + continue + } + key := strings.ToLower(callQN) + "|" + strings.ToLower(arg.Name) + tree, ok := trees[key] + if !ok || warned[key] { + continue + } + warned[key] = true + c := carriedIncludes{call: callQN, param: arg.Name, tree: formatIncludedAssociations(tree)} + out = append(out, linter.Violation{ + RuleID: carriedIncludesRule, + Severity: linter.SeverityWarning, + Message: strings.TrimPrefix(carriedIncludesWarning(qn, c), "Warning: "), + }) + } + }) + } + } + return out +} + +// storedMicroflowObjects is the live stored microflow's object collection, nil +// when the project has none of that name. +func storedMicroflowObjects(ctx *ExecContext, name ast.QualifiedName) *microflows.MicroflowObjectCollection { + mfs, err := microflowsNamed(ctx, name.Name) + if err != nil { + return nil + } + h, err := getHierarchy(ctx) + if err != nil || h == nil { + return nil + } + mf, ok := pickLive(mfs, + func(m *microflows.Microflow) bool { + return m.Name == name.Name && h.GetModuleName(h.FindModuleID(m.ContainerID)) == name.Module + }, + microflowExcluded) + if !ok || mf == nil { + return nil + } + return mf.ObjectCollection +} diff --git a/mdl/executor/validate_widget_image_ref.go b/mdl/executor/validate_widget_image_ref.go index a2daa3f9a8..16e1854c24 100644 --- a/mdl/executor/validate_widget_image_ref.go +++ b/mdl/executor/validate_widget_image_ref.go @@ -28,12 +28,15 @@ import ( // imageRefErrors reports the image references in refs that do not resolve. // -// known holds every image as a lower-cased three-part qualified name; -// collections holds every image collection as a lower-cased two-part one. A nil -// map means the project's collections could not be listed — then nothing is -// reported, because calling every image missing on a failed read is the wrong -// direction to be wrong in. An empty non-nil map is a real answer: the project -// has no image collections, so any reference to one is wrong. +// known holds every image as a three-part qualified name and collections every +// image collection as a two-part one, both spelled as stored: Mendix resolves +// an image reference case-sensitively in every part (measured on 11.13.0 — a +// case difference anywhere is CE1613), so the match is exact, and a miss that +// differs only in case names the stored spelling. A nil map means the +// project's collections could not be listed — then nothing is reported, +// because calling every image missing on a failed read is the wrong direction +// to be wrong in. An empty non-nil map is a real answer: the project has no +// image collections, so any reference to one is wrong. func imageRefErrors(refs []string, known map[string]bool, collections map[string]bool) []string { var errors []string for _, ref := range refs { @@ -55,14 +58,27 @@ func imageRefErrors(refs []string, known map[string]bool, collections map[string } collection := parts[0] + "." + parts[1] - if !collections[strings.ToLower(collection)] { + if !collections[collection] { + if stored := sameNameOtherCase(collections, collection); stored != "" { + errors = append(errors, fmt.Sprintf( + "image collection not found: %s (referenced as %s) — image names are "+ + "case-sensitive; the collection is %s", + collection, ref, stored)) + continue + } errors = append(errors, fmt.Sprintf( "image collection not found: %s (referenced as %s) — "+ "`show image collections` lists them", collection, ref)) continue } - if !known[strings.ToLower(ref)] { + if !known[ref] { + if stored := sameNameOtherCase(known, ref); stored != "" { + errors = append(errors, fmt.Sprintf( + "image not found: %s — image names are case-sensitive; write %s", + ref, stored)) + continue + } msg := fmt.Sprintf("image not found: %s in image collection %s", parts[2], collection) if inCollection := imagesInCollection(known, collection); len(inCollection) > 0 { msg += fmt.Sprintf(" — it holds: %s", strings.Join(inCollection, ", ")) @@ -75,12 +91,22 @@ func imageRefErrors(refs []string, known map[string]bool, collections map[string return errors } -// imagesInCollection returns the image names known for one collection, so the -// diagnostic can show what is actually there instead of only what is not. The -// names are lower-cased (that is how the set is keyed) and capped, since a -// collection can hold hundreds. +// sameNameOtherCase returns the name in set that equals name except for case, +// or "" when there is none — the stored spelling a case-only miss meant. +func sameNameOtherCase(set map[string]bool, name string) string { + for stored := range set { + if strings.EqualFold(stored, name) { + return stored + } + } + return "" +} + +// imagesInCollection returns the image names known for one collection, as +// stored, so the diagnostic can show what is actually there instead of only +// what is not. Capped, since a collection can hold hundreds. func imagesInCollection(known map[string]bool, collection string) []string { - prefix := strings.ToLower(collection) + "." + prefix := collection + "." var names []string for qn := range known { if strings.HasPrefix(qn, prefix) { @@ -95,10 +121,10 @@ func imagesInCollection(known map[string]bool, collection string) []string { return names } -// buildImageQualifiedNames returns the project's images as lower-cased -// three-part qualified names, and its image collections as lower-cased two-part -// ones. Both are nil when the collections could not be read at all — the caller -// distinguishes that from "there are none". +// buildImageQualifiedNames returns the project's images as three-part +// qualified names, and its image collections as two-part ones, spelled as +// stored. Both are nil when the collections could not be read at all — the +// caller distinguishes that from "there are none". func buildImageQualifiedNames(ctx *ExecContext) (images map[string]bool, collections map[string]bool) { h, err := getHierarchy(ctx) if err != nil { @@ -111,10 +137,10 @@ func buildImageQualifiedNames(ctx *ExecContext) (images map[string]bool, collect images = make(map[string]bool) collections = make(map[string]bool) for _, ic := range ics { - qn := strings.ToLower(h.GetQualifiedName(ic.ContainerID, ic.Name)) + qn := h.GetQualifiedName(ic.ContainerID, ic.Name) collections[qn] = true for _, img := range ic.Images { - images[qn+"."+strings.ToLower(img.Name)] = true + images[qn+"."+img.Name] = true } } return images, collections diff --git a/mdl/executor/validate_widget_image_ref_script_test.go b/mdl/executor/validate_widget_image_ref_script_test.go index 0fa42568da..bb556e80f2 100644 --- a/mdl/executor/validate_widget_image_ref_script_test.go +++ b/mdl/executor/validate_widget_image_ref_script_test.go @@ -40,10 +40,12 @@ func TestWidgetRefs_ImageInScriptCreatedCollectionIsResolved(t *testing.T) { if len(errs) != 0 { t.Fatalf("an image in a collection the script creates was refused: %v", errs) } - // Mendix resolves names case-insensitively; the script's spelling must too. + // Mendix resolves an image reference case-sensitively (a case difference + // is CE1613, measured on 11.13.0), so a case difference against the + // script's own spelling is refused, naming that spelling. errs = validateWidgetReferences(ctx, imageRefWidget("staticimage", "Image", "moda.ICONS.Pix"), sc) - if len(errs) != 0 { - t.Fatalf("a case difference against a script-created image was refused: %v", errs) + if len(errs) != 1 || !strings.Contains(errs[0], "ModA.Icons") || !strings.Contains(errs[0], "case-sensitive") { + t.Fatalf("a case difference against a script-created image: want one refusal naming ModA.Icons, got %v", errs) } // The project's own collections still resolve alongside the script's. errs = validateWidgetReferences(ctx, imageRefWidget("image", "Image", "MyFirstModule.Images.gallery"), sc) diff --git a/mdl/executor/validate_widget_image_ref_test.go b/mdl/executor/validate_widget_image_ref_test.go index 8c8d60e1d7..469c94abdc 100644 --- a/mdl/executor/validate_widget_image_ref_test.go +++ b/mdl/executor/validate_widget_image_ref_test.go @@ -27,8 +27,8 @@ import ( // reader to different places. func TestImageRefErrors_UnknownImageIsReported(t *testing.T) { - known := map[string]bool{"myfirstmodule.images._1": true} - collections := map[string]bool{"myfirstmodule.images": true} + known := map[string]bool{"MyFirstModule.Images._1": true} + collections := map[string]bool{"MyFirstModule.Images": true} errs := imageRefErrors([]string{"MyFirstModule.Images.NoSuchImage"}, known, collections) if len(errs) != 1 { @@ -58,15 +58,50 @@ func TestImageRefErrors_UnknownCollectionSaysSo(t *testing.T) { // CONTROL: an image that exists is not reported. Without this the check just // forbids the feature. func TestImageRefErrors_KnownImageIsClean(t *testing.T) { - known := map[string]bool{"myfirstmodule.images._1": true} - collections := map[string]bool{"myfirstmodule.images": true} + known := map[string]bool{"MyFirstModule.Images._1": true} + collections := map[string]bool{"MyFirstModule.Images": true} if errs := imageRefErrors([]string{"MyFirstModule.Images._1"}, known, collections); len(errs) != 0 { t.Errorf("an existing image was reported: %v", errs) } - // Mendix resolves names case-insensitively, and so must this. - if errs := imageRefErrors([]string{"myfirstmodule.IMAGES._1"}, known, collections); len(errs) != 0 { - t.Errorf("a case difference was reported: %v", errs) +} + +// Mendix resolves an image reference case-SENSITIVELY, in every part. Measured +// on a blank 11.13.0 app, whose Atlas_Core.Content holds `Mendix`: each of +// `Atlas_Core.Content.mendix`, `Atlas_Core.content.Mendix` and +// `atlas_core.Content.Mendix` is CE1613 "The selected image … no longer +// exists", and `Atlas_Core.Content.Mendix` is clean. The check folded case and +// passed all three, and its "it holds:" hint printed `mendix` — so following +// the hint produced the CE1613. A case-only miss names the stored spelling. +func TestImageRefErrors_CaseMismatchIsReportedWithTheStoredSpelling(t *testing.T) { + known := map[string]bool{"Atlas_Core.Content.Mendix": true} + collections := map[string]bool{"Atlas_Core.Content": true} + for _, tc := range []struct{ ref, want string }{ + {"Atlas_Core.Content.mendix", "Atlas_Core.Content.Mendix"}, + {"Atlas_Core.content.Mendix", "Atlas_Core.Content"}, + {"atlas_core.Content.Mendix", "Atlas_Core.Content"}, + } { + errs := imageRefErrors([]string{tc.ref}, known, collections) + if len(errs) != 1 { + t.Errorf("%s: got %d errors, want 1 — Mendix reports CE1613 for it: %v", tc.ref, len(errs), errs) + continue + } + for _, want := range []string{tc.want, "case-sensitive"} { + if !strings.Contains(errs[0], want) { + t.Errorf("%s: the message should say %q: %s", tc.ref, want, errs[0]) + } + } + } +} + +// The "it holds:" hint lists the images as stored. Lower-cased, it named +// `mendix` for a stored `Mendix`, and copying it from the message was CE1613. +func TestImageRefErrors_HintListsImagesAsStored(t *testing.T) { + known := map[string]bool{"Atlas_Core.Content.Mendix": true} + collections := map[string]bool{"Atlas_Core.Content": true} + errs := imageRefErrors([]string{"Atlas_Core.Content.mendix_logo"}, known, collections) + if len(errs) != 1 || !strings.Contains(errs[0], "it holds: Mendix") { + t.Errorf("want the hint to list Mendix as stored, got %v", errs) } } diff --git a/mdl/executor/validate_workflow_activity_kind.go b/mdl/executor/validate_workflow_activity_kind.go index 92a9f586a8..c6dddae816 100644 --- a/mdl/executor/validate_workflow_activity_kind.go +++ b/mdl/executor/validate_workflow_activity_kind.go @@ -274,11 +274,26 @@ func findStoredWorkflow(ctx *ExecContext, qn ast.QualifiedName) *workflows.Workf // resolveStoredActivity mirrors the mutator's own addressing // (wfmutator.findActivityByCaption): match on Name or Caption, search nested -// flows too, and use @N to pick among several. It returns nil rather than -// guessing when the reference is ambiguous, so the mutator's message wins. +// flows too, and use @N to pick among several. Without @N a Name match wins +// over caption matches, as the mutator's preferNameMatches does — without that +// a jump captioned with its target's name made the reference look ambiguous +// here, and every check-time guard went silent on a target the mutator +// resolved (ako/mxcli#808). It returns nil rather than guessing when the +// reference is ambiguous, so the mutator's message wins. func resolveStoredActivity(flow *workflows.Flow, ref string, atPos int) workflows.WorkflowActivity { var matches []workflows.WorkflowActivity collectStoredActivities(flow, ref, &matches) + if atPos == 0 { + var named []workflows.WorkflowActivity + for _, a := range matches { + if a.GetName() == ref { + named = append(named, a) + } + } + if len(named) > 0 { + matches = named + } + } switch { case len(matches) == 0: return nil diff --git a/mdl/executor/version_gates_flow_actions.go b/mdl/executor/version_gates_flow_actions.go index f920358b8c..45e5d3c8a8 100644 --- a/mdl/executor/version_gates_flow_actions.go +++ b/mdl/executor/version_gates_flow_actions.go @@ -30,6 +30,26 @@ var flowActionGates = []typeGate{ return ok && len(s.ConnectionArguments) > 0 }, }, + { + // `include associations ()` writes nothing and is not gated: only a + // tree needs the 10.11 type (ako/mxcli#1914). + versionGate: versionGate{"microflows", "call_external_action_include_associations", "call external action … include associations ( … )", + "this version has no associations to include on an external action argument (Microflows$IncludedAssociation): leave out the clause, or upgrade the project"}, + types: []string{"Microflows$IncludedAssociation"}, + properties: []string{"Microflows$ExternalActionParameterMapping.IncludedAssociations"}, + match: func(st ast.MicroflowStatement) bool { + s, ok := st.(*ast.CallExternalActionStmt) + if !ok { + return false + } + for _, a := range s.Arguments { + if a.IncludeAssociations != nil && len(a.IncludeAssociations.Associations) > 0 { + return true + } + } + return false + }, + }, { // The body variable is written for an operation whose body is an export // mapping, or one the builder cannot find; for a JSON, template or file diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index b19e20e509..70ee8237d4 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -833,6 +833,9 @@ func (e *PluggableWidgetEngine) Build(def *WidgetDefinition, w *ast.WidgetV3) (* if len(visibility) > 0 { builder.ApplyPropertyVisibility(visibility) } + // 4.11 A TextTemplate bound to an optional data source the statement does + // not set is null, as Studio Pro stores it (ako/mxcli#1967, CE0463). + builder.NullTemplatesOfUnsetDataSources() // 5. Build CustomWidget widgetID := model.ID(types.GenerateID()) diff --git a/mdl/grammar/MDLLexer.g4 b/mdl/grammar/MDLLexer.g4 index c4e15a7d7a..7d30cd14c8 100644 --- a/mdl/grammar/MDLLexer.g4 +++ b/mdl/grammar/MDLLexer.g4 @@ -170,6 +170,11 @@ NORMALIZED: N O R M A L I Z E D; // activity (ADR-0012). In `keyword` too, so "handles" still parses as a name. HANDLES: H A N D L E S; +// `command = $Cmd include associations (M.A (M.B))` — a call external action +// argument's Associations to include (ako/mxcli#1914). In `keyword` too, so +// "include" still parses as a name. +INCLUDE: I N C L U D E; + ENTITY: E N T I T Y; PERSISTENT: P E R S I S T E N T; VIEW: V I E W; diff --git a/mdl/grammar/MDLParser.g4 b/mdl/grammar/MDLParser.g4 index 8b4576dfb5..4dc7480e07 100644 --- a/mdl/grammar/MDLParser.g4 +++ b/mdl/grammar/MDLParser.g4 @@ -583,6 +583,14 @@ alterPageAssignment // Whether the key IS an action slot is the stored widget's call, not the // grammar's — the mutator refuses one that is not. | STRING_LITERAL alterAssignOp actionExprV3 // 'createFileAction': microflow Module.MF + // A text template's parameters, as CREATE PAGE writes them (ako/mxcli#1966): + // without this, `set (CaptionParams: ({1} = Attr))` did not parse and a + // template's parameters could only be changed by REPLACEing the widget. + | (CONTENTPARAMS | CAPTIONPARAMS) alterAssignOp paramListV3 // CaptionParams: ({1} = VersionNo) + // An attribute over associations, as CREATE PAGE takes a column's or an + // input's (ako/mxcli#1644): `Assoc/Attr` parsed as a division and was + // refused as an expression. + | ATTRIBUTE alterAssignOp attributePathHopV3 SLASH attributePathV3 // Attribute: Order_Customer/City | identifierOrKeyword alterAssignOp actionExprV3 // createFileAction: microflow Module.MF | identifierOrKeyword alterAssignOp propertyValueV3 // Caption: 'Save' | STRING_LITERAL alterAssignOp propertyValueV3 // 'showLabel': false diff --git a/mdl/grammar/domains/MDLMicroflow.g4 b/mdl/grammar/domains/MDLMicroflow.g4 index 70c62ef104..6ef528a737 100644 --- a/mdl/grammar/domains/MDLMicroflow.g4 +++ b/mdl/grammar/domains/MDLMicroflow.g4 @@ -797,12 +797,26 @@ callArgumentList // action's text-template parameter (StringTemplate(Text|Sql)) — the `with` // form of log and show message. Only call java action takes it (the visitor // refuses it elsewhere; ako/mxcli#1451). +// +// `Param = $Obj include associations (M.A (M.B), M.C)` is the argument's +// Associations to include: the associations whose objects are serialised into +// the OData request with it, as a tree. Only call external action takes it (the +// visitor refuses it elsewhere; ako/mxcli#1914). `include associations ()` +// states "none". callArgument - : (VARIABLE /* @alias MDL-DEPR006 */ | parameterName) EQUALS (NOTHING | expression templateParams?) + : (VARIABLE /* @alias MDL-DEPR006 */ | parameterName) EQUALS (NOTHING | expression templateParams? includeAssociationsClause?) | parameterName COLON /* @alias MDL-DEPR007 */ expression | expression // positional: refused by the visitor ; +includeAssociationsClause + : INCLUDE ASSOCIATIONS LPAREN (includedAssociation (COMMA includedAssociation)*)? RPAREN + ; + +includedAssociation + : qualifiedName (LPAREN includedAssociation (COMMA includedAssociation)* RPAREN)? + ; + showPageStatement : SHOW PAGE qualifiedName (LPAREN showPageArgList? RPAREN)? (FOR VARIABLE)? (WITH memberAssignmentList)? onErrorClause? ; diff --git a/mdl/grammar/domains/MDLSettings.g4 b/mdl/grammar/domains/MDLSettings.g4 index 55d147c6ce..41058f3ad6 100644 --- a/mdl/grammar/domains/MDLSettings.g4 +++ b/mdl/grammar/domains/MDLSettings.g4 @@ -679,6 +679,7 @@ keyword | MERGE | NORMALIZED | HANDLES + | INCLUDE // Query / SQL | SELECT | FROM | WHERE | JOIN | LEFT | RIGHT | INNER | OUTER | FULL | CROSS diff --git a/mdl/grammar/domains/MDLWorkflow.g4 b/mdl/grammar/domains/MDLWorkflow.g4 index d16dffc853..20eaca1481 100644 --- a/mdl/grammar/domains/MDLWorkflow.g4 +++ b/mdl/grammar/domains/MDLWorkflow.g4 @@ -436,10 +436,13 @@ alterWorkflowTarget : alterTarget alterWorkflowTargetMember? ; +// `boundary event N` is the N-th boundary event in the order `describe workflow` +// prints them, like a parallel split's `path N` (ako/mxcli#808); without N it +// is the activity's only one. alterWorkflowTargetMember : OUTCOME (STRING_LITERAL | TRUE | FALSE | DEFAULT) | PATH NUMBER_LITERAL - | BOUNDARY EVENT + | BOUNDARY EVENT NUMBER_LITERAL? ; // `Key: value`, the key naming a workflow property (Display, Description, diff --git a/mdl/types/edmx.go b/mdl/types/edmx.go index 714b83c945..94a3de6495 100644 --- a/mdl/types/edmx.go +++ b/mdl/types/edmx.go @@ -221,6 +221,9 @@ func ParseEdmx(metadataXML string) (*EdmxDocument, error) { if err := xml.Unmarshal([]byte(metadataXML), &edmx); err != nil { return nil, fmt.Errorf("failed to parse EDMX XML: %w", err) } + // Every term below is matched by its full namespace; read aliased ones + // through the document's aliases first (ako/mxcli#1977). + normalizeAnnotationTerms(&edmx) doc := &EdmxDocument{ Version: edmx.Version, @@ -736,15 +739,28 @@ func ResolveNavType(t string) (typeName string, isMany bool) { type xmlEdmx struct { XMLName xml.Name `xml:"Edmx"` Version string `xml:"Version,attr"` + References []xmlReference `xml:"Reference"` DataServices []xmlDataService `xml:"DataServices"` } +// xmlReference is an : the vocabularies (and other documents) +// this one uses, each Include optionally giving its namespace an alias. +type xmlReference struct { + Includes []xmlInclude `xml:"Include"` +} + +type xmlInclude struct { + Namespace string `xml:"Namespace,attr"` + Alias string `xml:"Alias,attr"` +} + type xmlDataService struct { Schemas []xmlSchema `xml:"Schema"` } type xmlSchema struct { Namespace string `xml:"Namespace,attr"` + Alias string `xml:"Alias,attr"` EntityTypes []xmlEntityType `xml:"EntityType"` ComplexTypes []xmlComplexType `xml:"ComplexType"` EnumTypes []xmlEnumType `xml:"EnumType"` @@ -939,3 +955,87 @@ func containsString(haystack []string, needle string) bool { } return false } + +// normalizeAnnotationTerms rewrites every annotation term written with a +// vocabulary alias to its full namespace, so the term switches below match it. +// +// CSDL lets a document alias a namespace — , or a Schema's +// own Alias — and write `Term="Capabilities.UpdateRestrictions"`. Matched by +// the full namespace only, every such term was skipped: an entity set imported +// as not creatable / updatable / deletable, which mxbuild, resolving the alias, +// reports as CE6630 (ako/mxcli#1977). A prefix the document does not declare is +// left as written; it names no vocabulary this parser knows. +func normalizeAnnotationTerms(edmx *xmlEdmx) { + aliases := map[string]string{} + for _, ref := range edmx.References { + for _, inc := range ref.Includes { + if inc.Alias != "" && inc.Namespace != "" { + aliases[inc.Alias] = inc.Namespace + } + } + } + for _, ds := range edmx.DataServices { + for _, s := range ds.Schemas { + if s.Alias != "" && s.Namespace != "" { + aliases[s.Alias] = s.Namespace + } + } + } + if len(aliases) == 0 { + return + } + canon := func(term string) string { + name, qualifier, hasQualifier := strings.Cut(term, "#") + i := strings.LastIndex(name, ".") + if i <= 0 { + return term + } + ns, ok := aliases[name[:i]] + if !ok { + return term + } + out := ns + name[i:] + if hasQualifier { + out += "#" + qualifier + } + return out + } + for d := range edmx.DataServices { + for si := range edmx.DataServices[d].Schemas { + s := &edmx.DataServices[d].Schemas[si] + for i := range s.EntityTypes { + et := &s.EntityTypes[i] + for j := range et.Annotations { + et.Annotations[j].Term = canon(et.Annotations[j].Term) + } + for j := range et.Properties { + for k := range et.Properties[j].Annotations { + et.Properties[j].Annotations[k].Term = canon(et.Properties[j].Annotations[k].Term) + } + } + } + for i := range s.ComplexTypes { + for j := range s.ComplexTypes[i].Properties { + p := &s.ComplexTypes[i].Properties[j] + for k := range p.Annotations { + p.Annotations[k].Term = canon(p.Annotations[k].Term) + } + } + } + for i := range s.EntityContainers { + for j := range s.EntityContainers[i].EntitySets { + es := &s.EntityContainers[i].EntitySets[j] + for k := range es.Annotations { + es.Annotations[k].Term = canon(es.Annotations[k].Term) + } + } + } + for i := range s.Annotations { + for j := range s.Annotations[i].Annotations { + s.Annotations[i].Annotations[j].Term = canon(s.Annotations[i].Annotations[j].Term) + } + } + } + } +} diff --git a/mdl/types/edmx_vocabulary_alias_test.go b/mdl/types/edmx_vocabulary_alias_test.go new file mode 100644 index 0000000000..761fa04724 --- /dev/null +++ b/mdl/types/edmx_vocabulary_alias_test.go @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: Apache-2.0 + +package types + +import ( + "strings" + "testing" +) + +// ako/mxcli#1977: a $metadata may declare a vocabulary alias — +// — +// and write its terms with it (Term="Capabilities.UpdateRestrictions"). The +// terms were matched by their full namespace only, so an aliased annotation +// was skipped: an entity set imported as not creatable / updatable / +// deletable, which mxbuild (resolving the alias) reports as CE6630. Every term +// is now read through the document's aliases. +const aliasMetadata = ` + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +` + +type aliasProbe struct { + insertable, updatable, deletable, topSupported string + computed, immutable bool + description string +} + +func probeAliasDoc(t *testing.T, cap, core string) aliasProbe { + t.Helper() + src := strings.NewReplacer(`Alias="CAP"`, `Alias="Capabilities"`, `Alias="CORE"`, `Alias="Core"`, + "CAP.", cap+".", "CORE.", core+".").Replace(aliasMetadata) + doc, err := ParseEdmx(src) + if err != nil { + t.Fatal(err) + } + b := func(p *bool) string { + if p == nil { + return "unset" + } + if *p { + return "true" + } + return "false" + } + var orders, lines *EdmEntitySet + for _, es := range doc.EntitySets { + switch es.Name { + case "Orders": + orders = es + case "Lines": + lines = es + } + } + et := doc.Schemas[0].EntityTypes[0] + return aliasProbe{ + insertable: b(orders.Insertable), updatable: b(orders.Updatable), topSupported: b(orders.TopSupported), + deletable: b(lines.Deletable), + computed: et.Properties[0].Computed, immutable: et.Properties[1].Immutable, + description: et.Summary, + } +} + +func TestParseEdmx_VocabularyAliases(t *testing.T) { + full := probeAliasDoc(t, "Org.OData.Capabilities.V1", "Org.OData.Core.V1") + want := aliasProbe{insertable: "true", updatable: "true", deletable: "true", topSupported: "false", + computed: true, immutable: true, description: "An order"} + if full != want { + t.Fatalf("control, full namespaces: %+v, want %+v", full, want) + } + if aliased := probeAliasDoc(t, "Capabilities", "Core"); aliased != want { + t.Errorf("aliased terms: %+v, want what the full namespaces give, %+v", aliased, want) + } + // Control: a prefix the document does not declare is not guessed at. + unknown := probeAliasDoc(t, "Caps", "Kore") + if unknown.updatable != "unset" || unknown.computed || unknown.description != "" { + t.Errorf("an undeclared prefix was read as a vocabulary: %+v", unknown) + } +} diff --git a/mdl/types/mapping_shape.go b/mdl/types/mapping_shape.go index 5c1d1406cc..53deb9bd25 100644 --- a/mdl/types/mapping_shape.go +++ b/mdl/types/mapping_shape.go @@ -40,6 +40,17 @@ func JsonMappingRootIsList(lookup JsonStructureLookup, im *model.ImportMapping) MappingRootPathCrossesArray(im.Elements[0].JsonPath) { return true, true } + // A root whose path is stored is decided by it alone: one that does not + // cross an array yields one object, even in an array-rooted structure. + // A mapping may root at "(Array)" — the array itself — with its items a + // child at "(Array)|(Object)"; it returns one root object, and answering + // "list" from the structure wrote the import as returning a list: mx check + // 11.14 CE0243 "The mapping used to return a value of type 'List of X', but + // now returns a value of type 'X'" (ako/mxcli#1938). The structure decides + // only when the mapping stores no path. + if len(im.Elements) > 0 && im.Elements[0] != nil && im.Elements[0].JsonPath != "" { + return false, true + } if im.JsonStructure != "" && lookup != nil { parts := strings.SplitN(im.JsonStructure, ".", 2) if len(parts) == 2 { diff --git a/mdl/types/mapping_shape_test.go b/mdl/types/mapping_shape_test.go new file mode 100644 index 0000000000..17fc8e7c86 --- /dev/null +++ b/mdl/types/mapping_shape_test.go @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: Apache-2.0 + +package types + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" +) + +type fixedStructure struct{ root string } + +func (f fixedStructure) GetJsonStructureByQualifiedName(string, string) (*JsonStructure, error) { + return &JsonStructure{Elements: []*JsonElement{{ElementType: f.root}}}, nil +} + +// ako/mxcli#1938 (mendixlabs/mxcli#1734): an array-rooted JSON structure whose +// mapping maps the ARRAY ITSELF to the root entity (root at "(Array)", the +// items a child at "(Array)|(Object)") returns ONE root object; the structure +// fallback answered "list" for any array-rooted structure, and the import +// activity was written as returning a list — mx check 11.14 CE0243 "The mapping +// used to return a value of type 'List of …', but now returns '…'". A root +// whose path is known is decided by the path alone. +func TestJsonMappingRootIsList_ByRootPath(t *testing.T) { + mapping := func(path string) *model.ImportMapping { + return &model.ImportMapping{ + JsonStructure: "M.JS", + Elements: []*model.ImportMappingElement{{Kind: "Object", Entity: "M.Root", JsonPath: path}}, + } + } + array := fixedStructure{root: "Array"} + for _, tc := range []struct { + path string + want bool + }{ + {"(Array)", false}, // the reporter's: the array is the root object + {"(Array)|(Object)", true}, // Studio Pro's own array-rooted mappings (#248) + {"(Object)", false}, + {"(Object)|choices|(Object)|message", true}, // rooted below an array (#267) + } { + list, known := JsonMappingRootIsList(array, mapping(tc.path)) + if !known || list != tc.want { + t.Errorf("root at %q: list=%v known=%v, want list=%v", tc.path, list, known, tc.want) + } + } + // Control: with no stored path the structure still decides. + if list, known := JsonMappingRootIsList(array, mapping("")); !known || !list { + t.Errorf("no root path, array structure: list=%v known=%v, want the structure's answer (list)", list, known) + } +} diff --git a/mdl/visitor/include_associations_clause_test.go b/mdl/visitor/include_associations_clause_test.go new file mode 100644 index 0000000000..dd21574b2e --- /dev/null +++ b/mdl/visitor/include_associations_clause_test.go @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#1914: a call external action argument's Associations to include, +// written as `include associations ( … )` after the argument. +package visitor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +func externalActionArgs(t *testing.T, call string) ([]ast.CallArgument, []error) { + t.Helper() + prog, errs := Build("create microflow M.F ($Cmd: M.Cmd)\nbegin\n" + call + "\nend;") + if len(errs) > 0 { + return nil, errs + } + body := prog.Statements[0].(*ast.CreateMicroflowStmt).Body + for _, st := range body { + if c, ok := st.(*ast.CallExternalActionStmt); ok { + return c.Arguments, nil + } + } + t.Fatalf("no call external action in %q", call) + return nil, nil +} + +func TestIncludeAssociationsClause(t *testing.T) { + args, errs := externalActionArgs(t, `$R = call external action M.Svc.Create( + command = $Cmd include associations (Odata.Quantity (Odata.Unit, Odata.Price (Odata.Currency)), Odata.Lines), + additional = empty +);`) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + if len(args) != 2 { + t.Fatalf("args = %d, want 2", len(args)) + } + if got := args[0].Value.(*ast.VariableExpr).Name; got != "Cmd" { + t.Errorf("the argument value is %q, want Cmd (the clause is not part of it)", got) + } + ic := args[0].IncludeAssociations + if ic == nil || len(ic.Associations) != 2 { + t.Fatalf("tree = %+v, want two roots", ic) + } + qty := ic.Associations[0] + if qty.Name.String() != "Odata.Quantity" || len(qty.Children) != 2 || + qty.Children[0].Name.String() != "Odata.Unit" || len(qty.Children[0].Children) != 0 || + qty.Children[1].Name.String() != "Odata.Price" || len(qty.Children[1].Children) != 1 || + qty.Children[1].Children[0].Name.String() != "Odata.Currency" { + t.Errorf("nesting lost: %+v", qty) + } + if ic.Associations[1].Name.String() != "Odata.Lines" { + t.Errorf("second root = %s", ic.Associations[1].Name) + } + if args[1].IncludeAssociations != nil { + t.Errorf("an argument without the clause has a tree: %+v", args[1].IncludeAssociations) + } +} + +func TestIncludeAssociationsClause_EmptyStatesNone(t *testing.T) { + args, errs := externalActionArgs(t, `call external action M.Svc.Create(command = $Cmd include associations ());`) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + if ic := args[0].IncludeAssociations; ic == nil || len(ic.Associations) != 0 { + t.Errorf("an empty clause must be present and empty, got %+v", ic) + } +} + +func TestIncludeAssociationsClause_Refusals(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"unqualified association", `call external action M.Svc.Create(command = $Cmd include associations (Quantity));`, + "`Quantity` in include associations is not qualified"}, + {"unqualified nested association", `call external action M.Svc.Create(command = $Cmd include associations (M.Quantity (Unit)));`, + "`Unit` in include associations is not qualified"}, + {"on another call", `call microflow M.Other(Cmd = $Cmd include associations (M.Quantity));`, + "only call external action has"}, + } { + t.Run(tc.name, func(t *testing.T) { + _, errs := externalActionArgs(t, tc.src) + joined := "" + for _, e := range errs { + joined += e.Error() + "\n" + } + if !strings.Contains(joined, tc.want) { + t.Errorf("errors = %q, want %q", joined, tc.want) + } + }) + } +} + +// "include" is a keyword now; it must still parse as a name. +func TestIncludeIsStillAName(t *testing.T) { + if _, errs := Build("create persistent entity M.E (Include: Boolean);"); len(errs) > 0 { + t.Fatalf("an attribute called Include no longer parses: %v", errs) + } +} diff --git a/mdl/visitor/visitor_alter_page.go b/mdl/visitor/visitor_alter_page.go index ae3198dd74..b19f75e4ed 100644 --- a/mdl/visitor/visitor_alter_page.go +++ b/mdl/visitor/visitor_alter_page.go @@ -133,6 +133,19 @@ func (b *Builder) buildAlterSetLayout(ctx *parser.AlterSetContext) *ast.SetLayou // buildAlterPageAssignment extracts property name and value from an assignment context. func (b *Builder) buildAlterPageAssignment(ctx *parser.AlterPageAssignmentContext) (string, interface{}) { + // Attribute: Assoc/…/Attr, the path CREATE PAGE takes (ako/mxcli#1644). + if hop := ctx.AttributePathHopV3(); hop != nil && ctx.ATTRIBUTE() != nil { + return "Attribute", unquoteQualifiedName(hop.GetText()) + "/" + buildAttributePathV3(ctx.AttributePathV3()) + } + // CaptionParams / ContentParams: a text template's parameters, the same + // list CREATE PAGE reads (ako/mxcli#1966). + if pl := ctx.ParamListV3(); pl != nil { + name := "ContentParams" + if ctx.CAPTIONPARAMS() != nil { + name = "CaptionParams" + } + return name, buildParamListV3(pl) + } // An expression-typed property takes the expression as written, from // whichever value alternative matched (visitor_widget_expression.go). if id := ctx.IdentifierOrKeyword(); id != nil && ctx.STRING_LITERAL() == nil { diff --git a/mdl/visitor/visitor_alter_workflow.go b/mdl/visitor/visitor_alter_workflow.go index 9d9205e1de..4fb0f08b46 100644 --- a/mdl/visitor/visitor_alter_workflow.go +++ b/mdl/visitor/visitor_alter_workflow.go @@ -218,7 +218,18 @@ func (b *Builder) buildAlterWorkflowDrop(ctx *parser.AlterWorkflowTargetContext) case m.PATH() != nil: return &ast.DropPathOp{PathCaption: "Path " + m.NUMBER_LITERAL().GetText(), ActivityRef: ref, AtPosition: pos} case m.BOUNDARY() != nil: - return &ast.DropBoundaryEventOp{ActivityRef: ref, AtPosition: pos} + op := &ast.DropBoundaryEventOp{ActivityRef: ref, AtPosition: pos} + if n := m.NUMBER_LITERAL(); n != nil { + idx, err := strconv.Atoi(n.GetText()) + if err != nil || idx < 1 { + tok := n.GetSymbol() + b.addError(fmt.Errorf("line %d:%d `boundary event %s`: the number is the event's place, from 1, "+ + "in the order describe workflow prints them", tok.GetLine(), tok.GetColumn(), n.GetText())) + return nil + } + op.Index = idx + } + return op } return nil } diff --git a/mdl/visitor/visitor_argument_binding.go b/mdl/visitor/visitor_argument_binding.go index 3ca39d40ed..7dbdecc98c 100644 --- a/mdl/visitor/visitor_argument_binding.go +++ b/mdl/visitor/visitor_argument_binding.go @@ -50,9 +50,45 @@ func (b *Builder) ExitCallArgument(ctx *parser.CallArgumentContext) { "call java action has; bind this parameter to an expression", tok.GetLine(), tok.GetColumn())) } + if ic := ctx.IncludeAssociationsClause(); ic != nil { + tok := ic.GetStart() + if !isExternalActionCallArgument(ctx) { + b.addError(fmt.Errorf("line %d:%d `include associations ( … )` is a call external action argument's "+ + "Associations to include, which only call external action has; remove it", + tok.GetLine(), tok.GetColumn())) + } + b.refuseUnqualifiedIncludedAssociations(ic.(*parser.IncludeAssociationsClauseContext).AllIncludedAssociation()) + } b.recordDollarArgument(ctx.VARIABLE()) } +// refuseUnqualifiedIncludedAssociations refuses a node of an include +// associations tree that names an association without its module: the tree is +// stored by qualified name, and an unqualified one does not resolve. +func (b *Builder) refuseUnqualifiedIncludedAssociations(nodes []parser.IIncludedAssociationContext) { + for _, n := range nodes { + node := n.(*parser.IncludedAssociationContext) + if qn := buildQualifiedName(node.QualifiedName()); qn.Module == "" { + tok := node.GetStart() + b.addError(fmt.Errorf("line %d:%d `%s` in include associations is not qualified: write the association "+ + "as Module.Association", tok.GetLine(), tok.GetColumn(), qn.Name)) + } + b.refuseUnqualifiedIncludedAssociations(node.AllIncludedAssociation()) + } +} + +// isExternalActionCallArgument reports whether a call argument belongs to a +// call external action — the one call whose arguments have Associations to +// include (ako/mxcli#1914). +func isExternalActionCallArgument(ctx *parser.CallArgumentContext) bool { + list := ctx.GetParent() + if list == nil { + return false + } + _, ok := list.GetParent().(*parser.CallExternalActionStatementContext) + return ok +} + // isJavaActionCallArgument reports whether a call argument belongs to a call // java action — the one call whose parameters can be text templates // (StringTemplate(Text|Sql)), filled with `with ({1} = …)` (ako/mxcli#1451). diff --git a/mdl/visitor/visitor_microflow_actions.go b/mdl/visitor/visitor_microflow_actions.go index 3742e81dc6..8270bd1a31 100644 --- a/mdl/visitor/visitor_microflow_actions.go +++ b/mdl/visitor/visitor_microflow_actions.go @@ -741,17 +741,42 @@ func buildCallArgumentList(ctx parser.ICallArgumentListContext) []ast.CallArgume // whitespace before `)` to keep. ca.Value = value ca.TemplateParams = buildTemplateParams(tp) + } else if arg.IncludeAssociationsClause() != nil { + ca.Value = value // ends at `include`, as above } else { ca.Value = appendExpressionListTrailingWhitespace(listCtx, nextParserRuleContext(allArgs, i), expr, value) } } + if ic := arg.IncludeAssociationsClause(); ic != nil { + ca.IncludeAssociations = buildIncludeAssociationsClause(ic) + } + args = append(args, ca) } return args } +// buildIncludeAssociationsClause converts `include associations ( … )`. +func buildIncludeAssociationsClause(ctx parser.IIncludeAssociationsClauseContext) *ast.IncludeAssociationsClause { + c := ctx.(*parser.IncludeAssociationsClauseContext) + clause := &ast.IncludeAssociationsClause{Associations: []*ast.IncludedAssociationDef{}} + for _, n := range c.AllIncludedAssociation() { + clause.Associations = append(clause.Associations, buildIncludedAssociation(n)) + } + return clause +} + +func buildIncludedAssociation(ctx parser.IIncludedAssociationContext) *ast.IncludedAssociationDef { + n := ctx.(*parser.IncludedAssociationContext) + def := &ast.IncludedAssociationDef{Name: buildQualifiedName(n.QualifiedName())} + for _, c := range n.AllIncludedAssociation() { + def.Children = append(def.Children, buildIncludedAssociation(c)) + } + return def +} + // buildMemberAssignmentList converts memberAssignmentList context to ChangeItem slice. func buildMemberAssignmentList(ctx parser.IMemberAssignmentListContext) []ast.ChangeItem { if ctx == nil { diff --git a/sdk/pages/pages_widgets_advanced.go b/sdk/pages/pages_widgets_advanced.go index 605455f2f2..52cb8b4674 100644 --- a/sdk/pages/pages_widgets_advanced.go +++ b/sdk/pages/pages_widgets_advanced.go @@ -187,6 +187,10 @@ type SnippetParamMapping struct { // IsLocalVariable puts Argument in the LocalVariable slot: the call passes a // page `Variables:` entry, not a parameter (ako/mxcli#826). IsLocalVariable bool + // IsWidget puts Argument in the Widget slot: the call passes a data + // view's object by naming the data view (ako/mxcli#1968). Written in + // PageParameter, it named a page parameter that does not exist — CE0115. + IsWidget bool } // SnippetCallWidget represents a snippet call widget. diff --git a/sdk/versions/mendix-10.yaml b/sdk/versions/mendix-10.yaml index 2519b0a365..7c3356a088 100644 --- a/sdk/versions/mendix-10.yaml +++ b/sdk/versions/mendix-10.yaml @@ -118,6 +118,10 @@ features: min_version: "10.2.0" mdl: "CALL EXTERNAL ACTION Module.Service.Action (Name = $Value);" notes: "Microflows$ExternalActionParameterMapping introduced in 10.2.0 (mendixmodelsdk 4.117.0); an action called with no arguments writes none (ako/mxcli#1811)." + call_external_action_include_associations: + min_version: "10.11.0" + mdl: "CALL EXTERNAL ACTION Module.Service.Action (Command = $Cmd INCLUDE ASSOCIATIONS (Module.Assoc (Module.SubAssoc)));" + notes: "Microflows$IncludedAssociation and ExternalActionParameterMapping.IncludedAssociations introduced in 10.11.0 (mendixmodelsdk); an empty clause writes nothing (ako/mxcli#1914)." send_rest_request_output: min_version: "10.4.0" mdl: "$Response = SEND REST REQUEST Module.Service.Operation;" diff --git a/sdk/versions/mendix-11.yaml b/sdk/versions/mendix-11.yaml index db79e1b984..587cbc1392 100644 --- a/sdk/versions/mendix-11.yaml +++ b/sdk/versions/mendix-11.yaml @@ -121,6 +121,10 @@ features: min_version: "10.2.0" mdl: "CALL EXTERNAL ACTION Module.Service.Action (Name = $Value);" notes: "Microflows$ExternalActionParameterMapping introduced in 10.2.0 (mendixmodelsdk 4.117.0); an action called with no arguments writes none (ako/mxcli#1811)." + call_external_action_include_associations: + min_version: "10.11.0" + mdl: "CALL EXTERNAL ACTION Module.Service.Action (Command = $Cmd INCLUDE ASSOCIATIONS (Module.Assoc (Module.SubAssoc)));" + notes: "Microflows$IncludedAssociation and ExternalActionParameterMapping.IncludedAssociations introduced in 10.11.0 (mendixmodelsdk); an empty clause writes nothing (ako/mxcli#1914)." send_rest_request_output: min_version: "10.4.0" mdl: "$Response = SEND REST REQUEST Module.Service.Operation;"