diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-11-test-callers-apply-entity-access.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-11-test-callers-apply-entity-access.json new file mode 100644 index 000000000..6360f2c3a --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-11-test-callers-apply-entity-access.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-11-11", "symptom": "ako/mxcli#2100 (mendixlabs/mxcli#1805): a .test.mdl test calling a microflow with Apply entity access = Yes failed CE0114 at build ('A microflow that does not apply entity access can only call microflows that also do not apply entity access') and MDL-SEC22 at check; the same for an applying @setup. Making the generated test microflow apply entity access then failed CE0150 on the runner's own handlers ('attributes of System.Error … cannot be accessed in microflows that apply entity access').", "cause": "Three generators wrote callers without the setting: writeTestFlow (endpoint), GenerateTestRunner (legacy MxTest.TestRunner) and CheckSource (MxTest.Check_* wrappers). The runner's verdict and @throws handlers read $latestError/Message directly, which an applying flow may not.", "file": "cmd/mxcli/testrunner/caller_entity_access.go (callerAppliesEntityAccess, errorMessageFlow helper), generator_endpoint.go (attachOnError numbers its handlers), generator.go, check_source.go, runner.go (legacy cleanup), build_attribution.go", "insight": "CE0114 has one direction, so the safe constant is 'apply' (the same call RegisterEndpoint made for #1654) — but the setting has a second, opposite constraint (CE0150), and the fix has to be measured against both: mx check on 11.12.1 at Off and Production showed an applying flow reading an ordinary entity's attribute is 0 errors, so CE0150 is the only cost. Mendix's CE0150 text prescribes the helper. The runtime half needs no measurement because the endpoint runs tests under Core.createSystemContext. A test body that reads $latestError/ itself is the one case 'apply' would newly break, so it keeps the old setting; under the legacy runner that decision is per suite because all tests share one microflow. The reported fix named two generators; the legacy runner was a third.", "refs": ["ako/mxcli#2100", "mendixlabs/mxcli#1805", "ako/mxcli#1654"], "test": "cmd/mxcli/testrunner/caller_entity_access_test.go; mdl-examples/bug-tests/test-runner-2100-*"} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-11-watch-cold-rebuild-per-change-client-source-deleted.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-11-watch-cold-rebuild-per-change-client-source-deleted.json new file mode 100644 index 000000000..30a6bda33 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-11-11-watch-cold-rebuild-per-change-client-source-deleted.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-11-11", "symptom": "`run --local --watch` on Mendix 11.16.0 took 20-22 s to apply a microflow edit and 25-28 s, with a runtime restart that drops every browser session, to apply a page edit. Every change logged \"mxbuild --serve's incremental build left web/dist without dist/pages ... restarting mxbuild --serve for a full build\" or \"mxbuild --serve cannot build this change incrementally (it wants client source its first build does not leave behind)\".", "cause": "A build in an mxbuild --serve process writes the client source its bundler reads (web/pages, layouts, nanoflows, snippets, widgets, package.json) and deletes it when the build ends. The next build regenerates only the changed documents, so the bundler has almost no input: dist/ goes from 47 files to 4, or the build fails on a missing web/pages file. serveRecovery answered both with a fresh serve process and a cold build per change, and the cold build's restartRequired restarts the runtime.", "file": "cmd/mxcli/docker/client_source_keep.go (clientSourceKeep: watch, restore, missingPageSource), cmd/mxcli/docker/serve_recovery.go (buildKeeping, isBundleFailure), cmd/mxcli/docker/runlocal.go (the boot build is watched too)", "insight": "The earlier measurement that ruled this out (webclient_legacy_paths.go, 11.14) put back the bundler config and the empty directories, not the files in them, and concluded that nothing done to the deployment helps. What settled it was copying web/ every 50 ms during a cold build and diffing the copy against web/ afterwards: 75 files existed only mid-build. Putting them back before the next /build made it complete in 2.3 s with restartRequired false. Control: that bundle against a cold build of the same model on a fresh process has the same file set and differs only in the build timestamp on each import. Keep what a successful build deleted rather than a list of directory names, so a version that leaves its source in place keeps nothing; and check dist/pages against the kept source after every build, because a missed file is bundled this time and silently absent the next. A measurement taken through a project's own ./mxcli tests the binary that link points at: `make build` replaces bin/mxcli with a new inode and leaves the hardlink on the old one, which produced one full run of 'the fix does nothing'.", "refs": ["ako/mxcli#2150"], "test": "cmd/mxcli/docker/client_source_keep_test.go"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-11-binding-through-outer-data-view.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-11-binding-through-outer-data-view.json new file mode 100644 index 000000000..093b76101 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-11-binding-through-outer-data-view.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-11-11", "refs": ["ako/mxcli#2107", "ako/mxcli#826", "ako/mxcli#833", "mendixlabs/mxcli#1591"], "ce": ["CE7247"], "symptom": "describe page -> exec of six FactoryManagement pages (Mendix 11.16) failed: a markdown viewer in an inner data view was refused \"`GenAICommons.Span.Output` is an attribute of GenAICommons.Span, not of ConversationalUI.CopyHelper … CE7247\" (and Span_TextGeneration_SlideIn's markdownViewer3 failed \"failed to resolve mapping for stringAttribute\"); an attribute-based visibility condition failed \"ConversationalUI.TraceView has no attribute HasError\".", "file": "mdl/executor/cmd_pages_parameter_binding.go (namedObjectSourceName, isNamedObjectAttributeText), cmd_pages_describe_pluggable.go and cmd_pages_describe_parse.go (print $dv.Attr / $dv/Assoc/Attr), cmd_pages_builder_v3.go (namedObjectVariable, used by resolveRootedAssociationPath), widget_engine.go (parameterBinding takes a data view; rootedAttributeBinding; BuildContext.AttributeSteps), widget_expression_value.go and validate_widget_expression_value.go (rooted data-source form on an attribute property), cmd_pages_builder_visible_when.go, cmd_pages_input_binding_context.go (MDL-WIDGET34 accepts an enclosing data view), mdl/backend (WidgetObjectBuilder.SetAttributeWithSteps in widgetobj and mcp). Tests: cmd_pages_outer_dataview_binding_2107_test.go", "cause": "Studio Pro stores both bindings with the data-view pair SourceVariable {Widget: , PageParameter: …} (markdownViewer3 also with association steps). parameterSourceName returns \"\" when Widget is set, so describe fell back to the qualified name and the rewrite resolved it against the nearest container. The `$dataView1.Attr` spelling existed for built-in inputs only (#826); the pluggable engine, Visible-by-attribute and MDL-WIDGET34 accepted a parameter only, and no pluggable attribute property could store association steps at all.", "insight": "**When a describe falls back to a qualified name, look for a SourceVariable the reader ignored.** Every one of these bindings carried the pair, and the existing spelling for it just had not been wired into every reader and writer: grep the parameter-only helpers (parameterSourceName, parameterVariable) for their callers, and widen them in ONE helper (namedObjectVariable, namedObjectSourceName) so check, describe and exec agree. Also note that `$x/Assoc/Attr` parses as an association data source, not an expression: a fix tested only through the expression form passes its unit test and still fails the real page (MDL-WIDGET42) - feed the engine the parsed shape. Controls: reverting each edit fails its test; the old binary refuses all six pages, the new one reports Unchanged page on all six; a fresh page with $dvOuter.Output, $dvOuter/Span_Trace/SystemPrompt and Visible: $dvOuter.Flag in (true) builds with mx check 0 errors on 11.16."} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-11-own-container-name-in-item-slots.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-11-own-container-name-in-item-slots.json new file mode 100644 index 000000000..f0b8343da --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-11-own-container-name-in-item-slots.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-11-11", "refs": ["ako/mxcli#2109", "mendixlabs/mxcli#1324", "mendixlabs/mxcli#122", "ako/mxcli#2095"], "ce": ["CE0117", "CE0115"], "symptom": "describe page -> exec of four FactoryManagement pages (Mendix 11.16) was refused by check on its own output: \"`actionButton1` reads `$grid1` …, but `grid1` is the data container the widget sits in directly\" (MDL-BUTTON02) and \"$grid1 cannot be stored — … an explicit one is rejected as CE0115\" (MDL-PAGEARG01). The stored pages build (untouched app: 4 unrelated errors).", "file": "mdl/executor/validate_page_button_context.go (checkButtonContextTree: control bar nearest \"\", nearestSelectable; isObjectContextContainer + combobox; checkOwnContainerName skips a show page in a selectable grid's rows), mdl/executor/cmd_pages_showpage_args.go (pageArgContext.rowSelectionOf, namesRowSelection, selectableDataGrid, enteringDataWidgetOf shared by argContextForChildren and both engine sites), mdl/executor/widget_engine.go. Tests: validate_own_container_slots_2109_test.go; updated validate_page_own_container_name_test.go (btnCtlBar) and cmd_pages_showpage_selection_arg_122_test.go", "cause": "Three slots open or move an object context that the walks modelled wrong. A selectable data grid's name is its selection in its rows as well as its control bar (Studio Pro stores Variable {Widget: grid1} from column content). A control bar takes its object from above the grid but sits INSIDE it, so the container above is an outer one, not the nearest. A combo box's option content is its own item context. And the pluggable engine entered a data widget's argument context with its own constructor, so a fix in argContextForChildren alone left exec refusing what check accepted.", "insight": "**Dump the stored slot before deciding which side is wrong, and find the property path, not just the widget.** A small walker mapping each CustomWidget TypePointer to its PropertyKey showed `Datagrid:grid1 > [columns] > [content]`, `[filtersPlaceholder]` (printed as controlbar) and `Combobox > [optionsSourceAssociationCustomContent]` - three different reasons in one rule. Then **re-measure an old 'measured' verdict with today's writer before trusting it**: btnCtlBar's CE0117 on 11.14 was an error in an EXPRESSION `$dvP`; since #2095 the argument is written as the variable pair, and a probe on 11.16 builds it - and even a button DIRECTLY in dvP passing $dvP - with 0 errors (positive control in the same build: CE1365), so MDL-BUTTON02's premise for action arguments may be stale beyond this fix. Controls: reverting each of the five edits fails its test; old binary refuses all four pages, new one reports Unchanged page on the three that MDL-WIDGET03 (#2110) does not block."} diff --git a/.claude/skills/fix-issue/findings/modelsdk/2026-11-11-system-workflow-subprocess-members.json b/.claude/skills/fix-issue/findings/modelsdk/2026-11-11-system-workflow-subprocess-members.json new file mode 100644 index 000000000..429743abb --- /dev/null +++ b/.claude/skills/fix-issue/findings/modelsdk/2026-11-11-system-workflow-subprocess-members.json @@ -0,0 +1 @@ +{"area":"modelsdk/meta","date":"2026-11-11","symptom":"describe -> exec of TestApp WorkflowCommons.Snip_WorkflowActivityRecord_Header (Mendix 11.14.0): mx check CE0117 'Error(s) in expression.' at Text 'text4', a parameter reading $WorkflowActivityRecord/System.WorkflowActivityRecord_WorkflowSubProcess/Caption","cause":"modelsdk/meta's System module (extracted from 11.6.4) had no System.WorkflowSubProcess, WorkflowSubProcessDefinition, their associations or System.WorkflowSubProcessState, so the association path could not resolve and the builder stored the parameter as an expression","file":"modelsdk/meta/system_module.go (SystemEntityDef.Introduced, ModelerSystemEntitiesIn, SystemEnumerationsIn), modelsdk/meta/system_enumerations.go, mdl/backend/modelsdk/system_module_read.go, mdl/backend/modelsdk/enumeration.go","insight":"Two measurements, two sources: attributes, types and String lengths from the model.mdp of a deploy build (mxcli docker build -o writes app/model/model.mdp; mx check does not), and the introducing version from the names in Mendix.Modeler.Core.dll per version (mxcli setup mxbuild --version X, grep, delete; ~1.6 GB each). Bisect with a control name that exists in every version (WorkflowActivityRecord_Actor), or 'absent' cannot be told from 'the model is stored elsewhere'. Result: absent 11.6.0/11.7.0, present 11.8.0 onward; 11.8.0 alone also has WorkflowActivityDetails_WorkflowSubProcess. Gate the ENTITY and derive its associations' presence from their ends (as RuntimeOnly already did), so new associations need no per-row version. A test whose count assumed every System enumeration exists for the 11.6.6 fixture is the gate working, not a regression: compare against the version-aware list.","refs":["#2141","mendixlabs/mxcli#1810"],"ce":["CE0117","CE1613"]} diff --git a/.claude/skills/mendix/system-module/SKILL.md b/.claude/skills/mendix/system-module/SKILL.md index 486b02cea..f64341e10 100644 --- a/.claude/skills/mendix/system-module/SKILL.md +++ b/.claude/skills/mendix/system-module/SKILL.md @@ -19,10 +19,10 @@ description: "Reference for the built-in System module — User, FileDocument, I | 275-332 | 3. HTTP / Web Services | | 333-359 | 4. Error Handling | | 360-383 | 7. Utility Entities | -| 384-448 | 8. Enumerations | -| 449-480 | 9. Inheritance Hierarchies | -| 481-536 | 10. Common Patterns | -| 537-539 | Source | +| 384-451 | 8. Enumerations | +| 452-483 | 9. Inheritance Hierarchies | +| 484-539 | 10. Common Patterns | +| 540-542 | Source | # Mendix System Module Reference @@ -440,6 +440,9 @@ refused. `describe` prints them as `--` comment lines for that reason. ### WorkflowState `InProgress`, `Paused`, `Completed`, `Aborted`, `Incompatible`, `Failed` +### WorkflowSubProcessState +`InProgress`, `Aborted`, `Failed`, `Completed`, `Paused` + ### WorkflowUserTaskCompletionType `Single`, `Veto`, `Consensus`, `Majority`, `Threshold`, `Microflow` diff --git a/.claude/skills/mendix/system-module/reference/workflow-and-queues.md b/.claude/skills/mendix/system-module/reference/workflow-and-queues.md index f072e3f3b..9212fe19a 100644 --- a/.claude/skills/mendix/system-module/reference/workflow-and-queues.md +++ b/.claude/skills/mendix/system-module/reference/workflow-and-queues.md @@ -168,6 +168,48 @@ Detailed audit of each workflow activity execution. | WorkflowActivityRecord_TaskTargetedUsers | System.User | Many-to-Many | | WorkflowActivityRecord_TaskAssignedUsers | System.User | Many-to-Many | | WorkflowActivityRecord_TaskTargetedGroups | System.WorkflowGroup | Many-to-Many | +| WorkflowActivityRecord_WorkflowSubProcess | System.WorkflowSubProcess | Many-to-One (11.8+) | +| WorkflowActivityRecord_WorkflowSubProcessDefinition | System.WorkflowSubProcessDefinition | Many-to-One (11.8+) | + +### System.WorkflowSubProcess + +A running or finished sub-process of a workflow. Mendix 11.8 and later +only: in an older project every name in this section is CE1613 "no longer exists". + +| Attribute | Type | Description | +|-----------|------|-------------| +| Caption | String(200) | Sub-process caption | +| StartTime | DateTime | When it started | +| EndTime | DateTime | When it ended | +| State | Enum (WorkflowSubProcessState) | Execution state | +| Reason | String | State reason | + +| Association | Target | Type | +|-------------|--------|------| +| WorkflowSubProcess_Workflow | System.Workflow | Many-to-One | +| WorkflowSubProcess_WorkflowSubProcessDefinition | System.WorkflowSubProcessDefinition | Many-to-One | + +`WorkflowUserTask_WorkflowSubProcess`, `WorkflowEndedUserTask_WorkflowSubProcess` +and `WorkflowCurrentActivity_WorkflowSubProcess` point at it from a task or activity. +Read its caption from an activity record with +`$WorkflowActivityRecord/System.WorkflowActivityRecord_WorkflowSubProcess/Caption`. + +### System.WorkflowSubProcessDefinition + +The definition of a sub-process (Mendix 11.8 and later). + +| Attribute | Type | Description | +|-----------|------|-------------| +| Caption | String(200) | Definition caption | +| IsObsolete | Boolean | Removed from the model | +| ModelGUID | String(36) | Sub-process GUID in the model | + +| Association | Target | Type | +|-------------|--------|------| +| WorkflowSubProcessDefinition_WorkflowDefinition | System.WorkflowDefinition | Many-to-One | + +`WorkflowUserTaskDefinition_WorkflowSubProcessDefinition` points at it from a user +task definition. ### System.WorkflowActivityDetails diff --git a/.claude/skills/mendix/test-microflows/SKILL.md b/.claude/skills/mendix/test-microflows/SKILL.md index 36ae31be6..7214f02fb 100644 --- a/.claude/skills/mendix/test-microflows/SKILL.md +++ b/.claude/skills/mendix/test-microflows/SKILL.md @@ -16,14 +16,14 @@ description: "Write and run MDL-based microflow tests with `mxcli test` — anno | 190-232 | Test File Formats | | 233-248 | string Operations | | 249-272 | Running Tests | -| 273-470 | How It Works | -| 471-509 | Check a test file before you run it | -| 510-631 | Writing Good Tests | -| 632-646 | Test File Organization | -| 647-659 | Interpreting Failures | -| 660-681 | CI Integration | -| 682-688 | Related Skills | -| 689-693 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | +| 273-471 | How It Works | +| 472-510 | Check a test file before you run it | +| 511-632 | Writing Good Tests | +| 633-647 | Test File Organization | +| 648-660 | Interpreting Failures | +| 661-682 | CI Integration | +| 683-689 | Related Skills | +| 690-694 | Warm test loop (`mxcli test --local [--watch]`, `--attach`, `run --local --test-endpoint`) | # Test Microflows Skill @@ -325,11 +325,12 @@ chained, `--local` ran with the app's startup logic suppressed, and a suite that depended on startup state passed under `--attach` and failed under `--local` for reasons unrelated to the code. -`MxTest.RegisterEndpoint` applies entity access, so it can call a startup -microflow that does or does not (CE0114 only forbids a non-applying microflow -calling an applying one); yours still runs with its own setting. A build error -located in `MxTest.RegisterEndpoint` or `MxTest.RegisterTestEndpoint` is reported -as an mxcli defect, not as an error in your project. +`MxTest.RegisterEndpoint` and the generated test microflows apply entity +access, so they can call a microflow that does or does not (CE0114 only forbids +a non-applying microflow calling an applying one); yours still runs with its own +setting. Exception: a test whose own error handler reads `$latestError/…` gets a +flow that does not apply it, since an applying one may not (CE0150). A build +error in an `MxTest` document mxcli generated is reported as an mxcli defect. One thing rollback does **not** cover: whatever the startup microflow writes happens at boot, outside any test's transaction, so `@cleanup rollback` does not diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f78b49c0..624399501 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **A test can call a microflow that applies entity access** (ako/mxcli#2100, mendixlabs/mxcli#1805) — the generated test microflows, the legacy runner's `MxTest.TestRunner` and the wrappers `mxcli check` sees did not apply entity access, so a test (or its `@setup`) calling a production microflow that does failed with CE0114 at build time and MDL-SEC22 at check time, and the only workaround was weakening the production microflow. They now apply it, as `MxTest.RegisterEndpoint` already did. An applying microflow may not read `$latestError/Message` (CE0150), so the runner's error handlers read the message through a generated helper, `MxTest.Test_ErrorMessage`, that does not apply entity access and is removed with the test microflows; failure messages and `@throws` matching are unchanged. A test whose own error handler reads `$latestError/…` keeps a non-applying microflow, so it builds as before. Tests run in a system context, so nothing changes at run time. +- **`run --local --watch` applies a change in about 5 s instead of 20-28 s on Mendix versions that bundle the client themselves** (ako/mxcli#2150) — measured on a blank 11.16.0 app: a microflow edit took 20-22 s and a page edit 25-28 s with a runtime restart that dropped every browser session, because each change cost a fresh `mxbuild --serve` and a cold build. mxbuild deletes the client source a build wrote, so its next build bundled a client without pages. That source is now kept while a build runs and put back before the next one: the incremental build takes about 2 s, a page edit is a reload, and an open browser tab shows it without a restart or a new login. The full build remains the fallback when the bundle still comes out incomplete. A deleted page's module stays in `dist/pages`, unused, until the next full build. On Windows the copies are read without blocking mxbuild's own deletes, and if a build still fails because `deployment/web` was in use, the kept source is given up for that run and builds go back to the full rebuild. +- **`check` accepts the data-container names Studio Pro stores in a grid row, a control bar and a combo box option** (ako/mxcli#2109) — `describe page` → `exec` of four FactoryManagement pages was refused on its own output (MDL-BUTTON02, MDL-PAGEARG01): a show page in a selectable data grid's column passing the grid's selection (`$grid1`), a control-bar button passing the data view around its grid, and a combo box's option content reading the data view around the combo box. All three build in Studio Pro's own pages. `check` and `exec` now accept them and write what Studio Pro stores, and the pages report `Unchanged page`. +- **A binding to an outer data view keeps that data view** (ako/mxcli#2107) — a markdown viewer or an attribute-based visibility condition inside a nested data view, bound to the outer one's object, was described as a bare qualified name and rewritten against the nearest data view: CE7247, or "has no attribute". `$dvOuter.Attr` and `$dvOuter/Assoc/Attr` now work on a pluggable widget's attribute property and in `Visible: … in (…)`, as on input widgets, and `describe` prints them; the six FactoryManagement pages report `Unchanged page`. - **`describe page` → `exec` no longer rewrites a page over formatting and stored keys MDL cannot state** (ako/mxcli#2106) — on FactoryManagement, 8 pages reported `Replaced page` with nothing changed: a dynamic-classes expression's trailing newlines, an XPath constraint's line breaks and spacing, the order of an enumeration condition's values, an association source's null `SourceVariable` key, and an inherited `createdDate` binding requalified to the specialization. The stored value is now kept where only its form differs, the writer stores the key Studio Pro stores, and a system member resolves to the entity that stores it. A full sweep of the app's 229 pages turns those 8 `Unchanged` and changes no other. - **`describe page` → `exec` keeps a check box's label position, a radio button group's orientation and a show page action's close count** (ako/mxcli#2106) — none had an MDL spelling, so a round trip of a Studio Pro page reset a check box with its label after the control to the default, turned every vertical radio button group horizontal, and stopped a show page button from closing the page it came from, with `mx check` green. `checkbox` takes `LabelPosition: Default|BeforeControl|AfterControl`, `radiobuttons` takes `RenderHorizontal: false`, and `show page` takes `with (NumberOfPagesToClose: n)`; `describe` prints each when it differs from the default. - **`mxcli test --local --watch` keeps applying changes on macOS and Linux mxbuild** (mendixlabs/mxcli#1787, refs mendixlabs/mxcli#1786) — the test runner's rebuild called mxbuild's serve process directly, without the recovery `run --local --watch` has, so the second build of a serve process failed on regenerated pages and every later change reported "could not be applied". The rebuild now goes through the same recovery, which also recognises mxbuild's resource-key form of that failure (`IO_PathNotFound_Path`, as macOS and Linux report it) and skips the web-bundle check, which only matters when a browser is served. diff --git a/cmd/mxcli/docker/client_source_keep.go b/cmd/mxcli/docker/client_source_keep.go new file mode 100644 index 000000000..072b2b1f6 --- /dev/null +++ b/cmd/mxcli/docker/client_source_keep.go @@ -0,0 +1,331 @@ +// SPDX-License-Identifier: Apache-2.0 + +package docker + +import ( + "io" + "io/fs" + "os" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "time" +) + +// client_source_keep.go gives mxbuild --serve's incremental build back the +// client source its own previous build deleted (ako/mxcli#2150). +// +// On the Mendix versions where mxbuild writes web/dist itself, a build writes +// the client source the bundler reads — web/pages, layouts, nanoflows, +// snippets, widgets, package.json — and deletes it when the build ends. The +// next build in the same process regenerates only the documents that changed, +// so the bundler runs on almost nothing and dist/ comes out without a single +// page. serveRecovery answers that with a fresh serve process and a cold build +// on every change. +// +// Measured by driving mxbuild 11.16.0's /build API directly on a blank app: +// +// build time restartRequired dist +// 1 cold 12.1 s true 47 files, 13 pages +// 2 same process, microflow change 1.9 s false 4 files, 0 pages +// 1' fresh process, cold 12.4 s true complete +// 2' microflow change, source put back 2.3 s false complete +// 3' page change, source put back 2.3 s false complete, the edit in its page module +// 4' new page, source put back 2.5 s false complete, the new page in dist/pages +// +// Control: 4' against a cold build of the same model on a fresh process — the +// same file set, differing only in the build timestamp each module import +// carries ("./index.js?6393…"). +// +// So the source is copied aside while a build runs and put back before the +// next. The 11.14 measurement in webclient_legacy_paths.go is not this one: it +// restored the bundler config and the empty directories, not what was in them. +// +// Nothing here names the directories. What is kept is whatever a build wrote +// under web/ that a successful build then removed, so a Mendix version that +// leaves its source in place (the rollup shape, 11.13 and earlier) keeps +// nothing and the keep switches itself off. +type clientSourceKeep struct { + webDir string + dir string // where the copies live; outside web/, which mxbuild recreates + + // off is set once a successful build removed nothing it wrote: this + // Mendix version needs no keep, and the sweeps would be pure cost. + off bool + + // restored is what restore put back before the build now running, for + // watch's stop to take out again when that build fails. + restored []string + + // sweeps counts completed sweeps, so a test can wait for one instead of + // for an amount of time. + sweeps atomic.Int64 +} + +// clientSourcePoll is how often a running build's web/ is swept. The files +// live for about 1.5 s (measured on 11.14 and 11.16), and a sweep that misses +// one is caught when the build ends, by missingPageSource. +const clientSourcePoll = 50 * time.Millisecond + +// newClientSourceKeep returns a keep for the deployment, or nil when there is +// nowhere to put the copies. Every method is a no-op on nil. +func newClientSourceKeep(deployDir string) *clientSourceKeep { + dir, err := os.MkdirTemp("", "mxcli-client-source-") + if err != nil { + return nil + } + return &clientSourceKeep{webDir: filepath.Join(deployDir, "web"), dir: dir} +} + +// Close removes the copies. +func (k *clientSourceKeep) Close() { + if k != nil { + _ = os.RemoveAll(k.dir) + } +} + +// reset empties the keep. A cold build writes every source file again, so +// before one the keep is cleared and whatever it held of deleted documents +// goes with it. +func (k *clientSourceKeep) reset() { + if k == nil { + return + } + _ = os.RemoveAll(k.dir) + _ = os.MkdirAll(k.dir, 0o755) + k.restored = nil +} + +// files lists the kept files, relative, slash-separated. +func (k *clientSourceKeep) files() []string { + if k == nil { + return nil + } + var out []string + _ = filepath.WalkDir(k.dir, func(path string, d fs.DirEntry, err error) error { + if err != nil || d.IsDir() { + return nil + } + if rel, rerr := filepath.Rel(k.dir, path); rerr == nil && !strings.HasSuffix(rel, keepTempSuffix) { + out = append(out, filepath.ToSlash(rel)) + } + return nil + }) + return out +} + +// restore puts the kept source back into web/ where it is absent, and returns +// how many files it wrote. +func (k *clientSourceKeep) restore() int { + if k == nil || k.off { + return 0 + } + k.restored = nil + for _, rel := range k.files() { + target := filepath.Join(k.webDir, filepath.FromSlash(rel)) + if _, err := os.Lstat(target); err == nil { + continue + } + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + continue + } + if err := copyFile(filepath.Join(k.dir, filepath.FromSlash(rel)), target); err == nil { + k.restored = append(k.restored, rel) + } + } + return len(k.restored) +} + +// watch copies aside what the build about to run writes under web/, until the +// returned stop is called with whether that build succeeded. +// +// After a successful build, anything kept that is still in web/ was not +// removed by the build and is dropped from the keep: only what mxbuild deletes +// needs putting back, and holding more would resurrect a file the user +// deleted. After a failed one the keep is left as it was and what restore put +// back is taken out again, so a build that died before its own cleanup does +// not leave web/ looking like another client shape (planWebClient reads it). +func (k *clientSourceKeep) watch() (stop func(ok bool)) { + if k == nil || k.off { + return func(bool) {} + } + existing := k.present() + done := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for { + k.sweep(existing) + select { + case <-done: + return + case <-time.After(clientSourcePoll): + } + } + }() + return func(ok bool) { + close(done) + wg.Wait() + k.sweep(existing) + if !ok { + for _, rel := range k.restored { + _ = os.Remove(filepath.Join(k.webDir, filepath.FromSlash(rel))) + } + k.restored = nil + return + } + k.restored = nil + kept := k.files() + for _, rel := range kept { + if _, err := os.Lstat(filepath.Join(k.webDir, filepath.FromSlash(rel))); err == nil { + _ = os.Remove(filepath.Join(k.dir, filepath.FromSlash(rel))) + } + } + if len(k.files()) == 0 { + k.off = true + return + } + // A page bundled from source the sweeps did not catch would be left + // out of the next build's bundle with nothing to say so. An empty keep + // sends that build down serveRecovery's full-build path instead, which + // fills the keep again. + if len(k.missingPageSource()) > 0 { + k.reset() + } + } +} + +// present is the set of files under web/ now, dist/ excluded. +func (k *clientSourceKeep) present() map[string]bool { + set := map[string]bool{} + k.walkWeb(func(rel string, _ fs.FileInfo) { set[rel] = true }) + return set +} + +// walkWeb visits the regular files under web/ outside dist/. Errors are +// skipped: mxbuild is deleting directories under the walk. +func (k *clientSourceKeep) walkWeb(visit func(rel string, info fs.FileInfo)) { + _ = filepath.WalkDir(k.webDir, func(path string, d fs.DirEntry, err error) error { + if err != nil { + if d != nil && d.IsDir() { + return filepath.SkipDir + } + return nil + } + rel, rerr := filepath.Rel(k.webDir, path) + if rerr != nil { + return nil + } + if d.IsDir() { + if rel == "dist" { + return filepath.SkipDir + } + return nil + } + if !d.Type().IsRegular() { + return nil + } + if info, ierr := d.Info(); ierr == nil { + visit(filepath.ToSlash(rel), info) + } + return nil + }) +} + +// keepTempSuffix marks a copy in progress, so a half-written one is never +// restored. +const keepTempSuffix = ".mxcli-part" + +// sweep copies aside every file that appeared since the build started, and +// every kept file whose source changed. A file that was already in web/ and is +// not kept is one earlier builds left alone, so it is not copied. +func (k *clientSourceKeep) sweep(existing map[string]bool) { + k.walkWeb(func(rel string, info fs.FileInfo) { + kept := filepath.Join(k.dir, filepath.FromSlash(rel)) + have, err := os.Stat(kept) + if err != nil && existing[rel] { + return + } + if err == nil && have.Size() == info.Size() && have.ModTime().Equal(info.ModTime()) { + return + } + src := filepath.Join(k.webDir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(kept), 0o755); err != nil { + return + } + part := kept + keepTempSuffix + if err := copyClientSource(src, part); err != nil { + _ = os.Remove(part) + return + } + // A file mxbuild was still writing is copied again on the next sweep; + // until then the previous copy stands rather than a truncated one. + after, err := os.Stat(src) + if err != nil || after.Size() != info.Size() || !after.ModTime().Equal(info.ModTime()) { + _ = os.Remove(part) + return + } + _ = os.Chtimes(part, info.ModTime(), info.ModTime()) + _ = os.Rename(part, kept) + }) + k.sweeps.Add(1) +} + +// copyClientSource copies a file out of web/ while mxbuild may be writing or +// deleting it; openClientSource is what keeps the copy out of mxbuild's way. +func copyClientSource(src, dst string) error { + in, err := openClientSource(src) + if err != nil { + return err + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644) + if err != nil { + return err + } + if _, err := io.Copy(out, in); err != nil { + out.Close() + return err + } + return out.Close() +} + +// disable switches the keep off for good and drops what it holds. It is the +// answer to a build that failed because web/ was in use: sweeping a directory +// mxbuild is recreating is the one thing here that can get in a build's way. +func (k *clientSourceKeep) disable() { + if k == nil { + return + } + k.reset() + k.off = true +} + +// active reports whether the keep sweeps and restores. +func (k *clientSourceKeep) active() bool { return k != nil && !k.off } + +// missingPageSource names the page modules in dist/pages that the keep holds +// no source for. The build that just ran bundled them from a file the sweeps +// missed, and the next build would silently leave them out — a page that 404s +// while the loop reports the change applied. +func (k *clientSourceKeep) missingPageSource() []string { + if k == nil || k.off { + return nil + } + entries, err := os.ReadDir(filepath.Join(k.webDir, "dist", "pages")) + if err != nil { + return nil + } + var missing []string + for _, e := range entries { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".js") { + continue + } + if _, err := os.Stat(filepath.Join(k.dir, "pages", e.Name())); err != nil { + missing = append(missing, e.Name()) + } + } + return missing +} diff --git a/cmd/mxcli/docker/client_source_keep_test.go b/cmd/mxcli/docker/client_source_keep_test.go new file mode 100644 index 000000000..fe57d1574 --- /dev/null +++ b/cmd/mxcli/docker/client_source_keep_test.go @@ -0,0 +1,375 @@ +// SPDX-License-Identifier: Apache-2.0 + +// mxbuild --serve deletes the client source a build wrote, so its next build +// bundles a client without pages and the watch loop pays a cold build per +// change (ako/mxcli#2150). The measurements are in client_source_keep.go; the +// fake below is that behaviour: a build writes the source of the documents it +// regenerates, bundles whatever source is in web/, and deletes all of it. +package docker + +import ( + "os" + "path/filepath" + "sort" + "strings" + "testing" + "time" +) + +// fakePrebuiltMxbuild is a serve process of the prebuilt shape. +type fakePrebuiltMxbuild struct { + t *testing.T + deploy string + keep *clientSourceKeep // nil: nothing is watching, so nothing to wait for + model map[string]string // page name -> content, the model now + built map[string]string // what this process last generated + // failBundle makes the bundler step fail when a restored source file has + // this content. + failBundle string + // hide names a page whose source is deleted before any sweep can see it. + hide string +} + +func (m *fakePrebuiltMxbuild) restartProcess() { m.built = nil } + +func (m *fakePrebuiltMxbuild) build() *BuildResult { + m.t.Helper() + web := filepath.Join(m.deploy, "web") + cold := m.built == nil + if cold { + m.built = map[string]string{} + } + keepWrite(m.t, filepath.Join(web, "index.html"), "") // never deleted + for name, content := range m.model { + if cold || m.built[name] != content { + if name != m.hide { + keepWrite(m.t, filepath.Join(web, "pages", name+".js"), content) + } + m.built[name] = content + } + } + keepWrite(m.t, filepath.Join(web, "package.json"), "{}") + m.waitForSweep() + + // The bundler: every source file present becomes a page module. + dist := filepath.Join(web, "dist") + _ = os.RemoveAll(dist) + keepWrite(m.t, filepath.Join(dist, "index.js"), "//") + entries, _ := os.ReadDir(filepath.Join(web, "pages")) + for _, e := range entries { + b, _ := os.ReadFile(filepath.Join(web, "pages", e.Name())) + if m.failBundle != "" && string(b) == m.failBundle { + _ = os.RemoveAll(dist) + return &BuildResult{Status: "Failure", Message: bundleCompileMessage} + } + keepWrite(m.t, filepath.Join(dist, "pages", e.Name()), string(b)) + } + if m.hide != "" && cold { + keepWrite(m.t, filepath.Join(dist, "pages", m.hide+".js"), m.model[m.hide]) + } + if len(entries) > 0 { + keepWrite(m.t, filepath.Join(dist, "widgets.css"), "/**/") + } + _ = os.RemoveAll(filepath.Join(web, "pages")) + _ = os.Remove(filepath.Join(web, "package.json")) + return &BuildResult{Status: "Success", RestartRequired: cold} +} + +// waitForSweep blocks until a sweep that started after the source was written +// has finished. Counting sweeps rather than sleeping keeps the test off the +// wall clock. +func (m *fakePrebuiltMxbuild) waitForSweep() { + if m.keep == nil || m.keep.off { + return + } + target := m.keep.sweeps.Load() + 2 + deadline := time.Now().Add(10 * time.Second) + for m.keep.sweeps.Load() < target { + if time.Now().After(deadline) { + m.t.Fatal("the keep never swept web/ during the build") + } + time.Sleep(time.Millisecond) + } +} + +func keepWrite(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } +} + +func distPages(t *testing.T, deploy string) map[string]string { + t.Helper() + out := map[string]string{} + entries, _ := os.ReadDir(filepath.Join(deploy, "web", "dist", "pages")) + for _, e := range entries { + b, _ := os.ReadFile(filepath.Join(deploy, "web", "dist", "pages", e.Name())) + out[strings.TrimSuffix(e.Name(), ".js")] = string(b) + } + return out +} + +// newKeptLoop boots a fake serve process the way RunLocal does — the boot +// build watched by the keep — and returns the recovery the watch loop builds +// through. +func newKeptLoop(t *testing.T, withKeep bool) (*fakePrebuiltMxbuild, *scriptedServe, *serveRecovery, *strings.Builder) { + t.Helper() + deploy := t.TempDir() + m := &fakePrebuiltMxbuild{t: t, deploy: deploy, model: map[string]string{"Mod.Home": "home v1", "Mod.Other": "other v1"}} + var keep *clientSourceKeep + if withKeep { + keep = newClientSourceKeep(deploy) + t.Cleanup(keep.Close) + m.keep = keep + } + f := &scriptedServe{build: func(proc, nth int) (*BuildResult, error) { + if nth == 1 && proc > 0 { + m.restartProcess() + } + return m.build(), nil + }} + stop := keep.watch() + boot := m.build() + stop(boot.OK()) + out := &strings.Builder{} + return m, f, &serveRecovery{serve: f, deployDir: deploy, out: out, keep: keep}, out +} + +// The fix: with the source put back, a change builds on the running serve +// process and the bundle is complete, the edited page included. +func TestClientSourceKeep_IncrementalBuildKeepsTheBundleComplete(t *testing.T) { + m, f, r, out := newKeptLoop(t, true) + + m.model["Mod.Home"] = "home v2" + res, err := r.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}) + if err != nil || !res.OK() { + t.Fatalf("Build = %+v, %v", res, err) + } + if f.restarts != 0 { + t.Fatalf("restarts = %d, want 0: the incremental build had its client source back\n%s", f.restarts, out) + } + if res.RestartRequired { + t.Fatalf("a page change must not ask for a runtime restart: %+v", res) + } + want := map[string]string{"Mod.Home": "home v2", "Mod.Other": "other v1"} + if got := distPages(t, m.deploy); len(got) != 2 || got["Mod.Home"] != want["Mod.Home"] || got["Mod.Other"] != want["Mod.Other"] { + t.Fatalf("dist/pages = %v, want %v", got, want) + } + + // A page added by an incremental build is kept too, so the build after it + // still bundles it. + m.model["Mod.New"] = "new v1" + if _, err := r.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}); err != nil { + t.Fatal(err) + } + m.model["Mod.Home"] = "home v3" + if _, err := r.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}); err != nil { + t.Fatal(err) + } + if got := distPages(t, m.deploy); got["Mod.New"] != "new v1" || got["Mod.Home"] != "home v3" || f.restarts != 0 { + t.Fatalf("after adding a page then editing another: dist/pages = %v, restarts = %d", got, f.restarts) + } + if _, err := os.Stat(filepath.Join(m.deploy, "web", "pages")); !os.IsNotExist(err) { + t.Fatalf("web/pages must be gone again after the build, as mxbuild leaves it: %v", err) + } +} + +// Control: the same loop without the keep is the reported behaviour — every +// change restarts mxbuild --serve. +func TestClientSourceKeep_WithoutItEveryChangeRestartsServe(t *testing.T) { + _, f, r, _ := newKeptLoop(t, false) + + // A change that regenerates no page, such as a microflow edit. + if _, err := r.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}); err != nil { + t.Fatal(err) + } + if f.restarts != 1 { + t.Fatalf("restarts = %d, want 1: without the kept source the incremental bundle has no pages", f.restarts) + } +} + +// A page whose source the sweeps missed is in this build's bundle and would be +// absent from the next one. The keep is emptied, so the next build is a full +// one and the page is not lost. +func TestClientSourceKeep_MissedSourceFallsBackToAFullBuild(t *testing.T) { + deploy := t.TempDir() + m := &fakePrebuiltMxbuild{t: t, deploy: deploy, hide: "Mod.Missed", + model: map[string]string{"Mod.Home": "home v1", "Mod.Missed": "missed v1"}} + keep := newClientSourceKeep(deploy) + t.Cleanup(keep.Close) + m.keep = keep + f := &scriptedServe{build: func(proc, nth int) (*BuildResult, error) { + if nth == 1 && proc > 0 { + m.restartProcess() + m.hide = "" // the fresh build's source is caught + } + return m.build(), nil + }} + stop := keep.watch() + stop(m.build().OK()) + r := &serveRecovery{serve: f, deployDir: deploy, out: &strings.Builder{}, keep: keep} + + // A change that regenerates no page, such as a microflow edit. + if _, err := r.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}); err != nil { + t.Fatal(err) + } + got := distPages(t, deploy) + if got["Mod.Missed"] != "missed v1" || got["Mod.Home"] != "home v1" { + t.Fatalf("dist/pages = %v: the page whose source was missed must still be served", got) + } + if f.restarts == 0 { + t.Fatal("a keep that missed a page's source must give way to a full build") + } +} + +// Kept source the bundler cannot compile is replaced by a full build rather +// than reported as the user's error. +func TestClientSourceKeep_BundlerFailureOnKeptSourceRebuildsFresh(t *testing.T) { + m, f, r, out := newKeptLoop(t, true) + m.failBundle = "other v1" // Mod.Other is unchanged, so its source is the restored copy + + m.model["Mod.Home"] = "home v2" + f.build = func(proc, nth int) (*BuildResult, error) { + if nth == 1 && proc > 0 { + m.restartProcess() + m.failBundle = "" + } + return m.build(), nil + } + res, err := r.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}) + if err != nil || !res.OK() { + t.Fatalf("Build = %+v, %v\n%s", res, err, out) + } + if f.restarts != 1 { + t.Fatalf("restarts = %d, want 1", f.restarts) + } + if got := distPages(t, m.deploy); got["Mod.Home"] != "home v2" || got["Mod.Other"] != "other v1" { + t.Fatalf("dist/pages = %v", got) + } +} + +// A failed build must not leave restored source behind in web/: planWebClient +// reads web/ to decide which client the deployment has. +func TestClientSourceKeep_FailedBuildTakesRestoredSourceOutAgain(t *testing.T) { + deploy := t.TempDir() + k := newClientSourceKeep(deploy) + t.Cleanup(k.Close) + web := filepath.Join(deploy, "web") + + stop := k.watch() + keepWrite(t, filepath.Join(web, "pages", "A.js"), "a") + keepWrite(t, filepath.Join(web, "index.html"), "") + k.sweep(map[string]bool{}) + _ = os.RemoveAll(filepath.Join(web, "pages")) + stop(true) + + if got := k.files(); len(got) != 1 || got[0] != "pages/A.js" { + t.Fatalf("kept = %v, want only pages/A.js: index.html survived the build, so it is not the keep's to restore", got) + } + if n := k.restore(); n != 1 { + t.Fatalf("restore = %d, want 1", n) + } + stop = k.watch() + stop(false) // the build failed before deleting anything + if _, err := os.Stat(filepath.Join(web, "pages", "A.js")); !os.IsNotExist(err) { + t.Fatalf("restored source must be removed after a failed build: %v", err) + } + if got := k.files(); len(got) != 1 { + t.Fatalf("a failed build must leave the keep as it was, got %v", got) + } +} + +// A Mendix version that leaves its client source in place needs no keep, and +// the keep stops sweeping. +func TestClientSourceKeep_SwitchesOffWhenNothingIsDeleted(t *testing.T) { + deploy := t.TempDir() + k := newClientSourceKeep(deploy) + t.Cleanup(k.Close) + web := filepath.Join(deploy, "web") + + stop := k.watch() + keepWrite(t, filepath.Join(web, "pages", "A.js"), "a") + keepWrite(t, filepath.Join(web, "rollup.config.mjs"), "//") + stop(true) // both files are still there + + if !k.off { + kept := k.files() + sort.Strings(kept) + t.Fatalf("the keep must switch off when the build deleted nothing it wrote; kept = %v", kept) + } + if n := k.restore(); n != 0 { + t.Fatalf("restore = %d on a switched-off keep", n) + } +} + +// A nil keep is what every caller without one passes. +func TestClientSourceKeep_NilIsANoOp(t *testing.T) { + var k *clientSourceKeep + k.reset() + k.Close() + k.watch()(true) + if k.restore() != 0 || k.files() != nil || k.missingPageSource() != nil { + t.Fatal("nil keep must do nothing") + } +} + +// On Windows a sweep can hold web/ open while mxbuild recreates it, and the +// build fails with a sharing violation. The keep gives way: the build is run +// again without it, and it stays off. +func TestClientSourceKeep_WebDirInUseDisablesTheKeepAndRebuilds(t *testing.T) { + m, f, r, out := newKeptLoop(t, true) + inner := f.build + failed := false + f.build = func(proc, nth int) (*BuildResult, error) { + if !failed { + failed = true + return &BuildResult{Status: "Failure", Message: webDirLockedMessage}, nil + } + return inner(proc, nth) + } + + res, err := r.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}) + if err != nil || !res.OK() { + t.Fatalf("Build = %+v, %v\n%s", res, err, out) + } + if r.keep.active() { + t.Fatal("the keep must stay off once it has collided with a build") + } + if len(r.keep.files()) != 0 { + t.Fatalf("a disabled keep must hold nothing, got %v", r.keep.files()) + } + if got := distPages(t, m.deploy); len(got) != 2 { + t.Fatalf("dist/pages = %v, want both pages (from the full build the loop falls back to)", got) + } + if !strings.Contains(out.String(), "deployment/web was in use") { + t.Fatalf("the fallback must be reported, got:\n%s", out) + } + // Control for the report: a build that fails some other way is not retried. + calls := len(f.reqs) + f.build = func(int, int) (*BuildResult, error) { + return &BuildResult{Status: "Failure", Message: "CE0001 something else"}, nil + } + if res, _ := r.Build(BuildRequest{ProjectFilePath: "/p/App.mpr"}); res.OK() || len(f.reqs) != calls+1 { + t.Fatalf("an unrelated failure must be returned as it is, after one build; builds = %d", len(f.reqs)-calls) + } +} + +// The copy the sweeps make reads the same bytes on every platform. +func TestCopyClientSource(t *testing.T) { + dir := t.TempDir() + src, dst := filepath.Join(dir, "a.js"), filepath.Join(dir, "b.js") + keepWrite(t, src, "export default 1") + if err := copyClientSource(src, dst); err != nil { + t.Fatal(err) + } + if b, _ := os.ReadFile(dst); string(b) != "export default 1" { + t.Fatalf("copy = %q", b) + } + if err := copyClientSource(filepath.Join(dir, "gone.js"), dst); err == nil { + t.Fatal("a source mxbuild already deleted must be an error, not an empty copy") + } +} diff --git a/cmd/mxcli/docker/client_source_open_other.go b/cmd/mxcli/docker/client_source_open_other.go new file mode 100644 index 000000000..122d7d0b4 --- /dev/null +++ b/cmd/mxcli/docker/client_source_open_other.go @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build !windows + +package docker + +import "os" + +// openClientSource opens a file under deployment/web for reading. On POSIX an +// open file does not stop anyone deleting it; see the Windows build. +func openClientSource(path string) (*os.File, error) { return os.Open(path) } diff --git a/cmd/mxcli/docker/client_source_open_windows.go b/cmd/mxcli/docker/client_source_open_windows.go new file mode 100644 index 000000000..eef2190e0 --- /dev/null +++ b/cmd/mxcli/docker/client_source_open_windows.go @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build windows + +package docker + +import ( + "os" + "syscall" +) + +// openClientSource opens a file under deployment/web for reading without +// stopping mxbuild from deleting or replacing it meanwhile. +// +// os.Open shares a file for reading and writing but not for deletion, so a copy +// in progress made mxbuild's own cleanup of that file fail with "being used by +// another process" — the failure the web client bundler caused by holding +// web/ open (#1342). With FILE_SHARE_DELETE the delete goes through and the +// file disappears when this handle closes. +func openClientSource(path string) (*os.File, error) { + p, err := syscall.UTF16PtrFromString(path) + if err != nil { + return nil, err + } + h, err := syscall.CreateFile(p, syscall.GENERIC_READ, + syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE|syscall.FILE_SHARE_DELETE, + nil, syscall.OPEN_EXISTING, syscall.FILE_ATTRIBUTE_NORMAL, 0) + if err != nil { + return nil, &os.PathError{Op: "open", Path: path, Err: err} + } + return os.NewFile(uintptr(h), path), nil +} diff --git a/cmd/mxcli/docker/client_source_open_windows_test.go b/cmd/mxcli/docker/client_source_open_windows_test.go new file mode 100644 index 000000000..4f58da80a --- /dev/null +++ b/cmd/mxcli/docker/client_source_open_windows_test.go @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build windows + +package docker + +import ( + "os" + "path/filepath" + "testing" +) + +// A copy in progress must not stop mxbuild deleting the file (ako/mxcli#2150). +// Control: the same delete fails while the file is open through os.Open. +func TestOpenClientSource_DoesNotBlockDelete(t *testing.T) { + dir := t.TempDir() + shared := filepath.Join(dir, "shared.js") + plain := filepath.Join(dir, "plain.js") + for _, p := range []string{shared, plain} { + if err := os.WriteFile(p, []byte("//"), 0o600); err != nil { + t.Fatal(err) + } + } + + f, err := openClientSource(shared) + if err != nil { + t.Fatal(err) + } + defer f.Close() + if err := os.Remove(shared); err != nil { + t.Fatalf("deleting a file openClientSource holds open must succeed: %v", err) + } + + g, err := os.Open(plain) + if err != nil { + t.Fatal(err) + } + defer g.Close() + if err := os.Remove(plain); err == nil { + t.Fatal("control: deleting a file held through os.Open was expected to fail on Windows") + } +} diff --git a/cmd/mxcli/docker/runlocal.go b/cmd/mxcli/docker/runlocal.go index f5a072902..a1660afde 100644 --- a/cmd/mxcli/docker/runlocal.go +++ b/cmd/mxcli/docker/runlocal.go @@ -861,7 +861,16 @@ func RunLocal(opts LocalRunOptions) error { fmt.Fprintf(w, "Building (first build is cold: ~10-15s for a small app, minutes for a large one; limit %s)...\n", serveBuildTimeout(opts.BuildTimeout)) scaffold := buildScaffold{} themeBefore := themeModuleFolders(opts.ProjectPath) + // Under --watch the client source this build writes and then deletes is + // kept, so the first change can already build incrementally (ako/mxcli#2150). + var keep *clientSourceKeep + if opts.Watch { + keep = newClientSourceKeep(opts.DeployDir) + defer keep.Close() + } + stopKeeping := keep.watch() build, err := serve.Build(BuildRequest{Target: TargetDeploy, ProjectFilePath: opts.ProjectPath}) + stopKeeping(err == nil && build != nil && build.OK()) if err != nil { return fmt.Errorf("initial build: %w", err) } @@ -1142,7 +1151,7 @@ func RunLocal(opts LocalRunOptions) error { // 7. Stay up until interrupted. With --watch, rebuild + hot-apply on every // project change; otherwise just keep the runtime serving. if opts.Watch { - return watchAndApply(opts, serve, rt, bundler, mxbuildPath, bootSource, scaffold) + return watchAndApply(opts, serve, rt, bundler, mxbuildPath, bootSource, scaffold, keep) } fmt.Fprintln(w, "(run with --watch to rebuild and hot-apply on changes; Ctrl-C to stop)") if waitForInterruptOrExit(rt.Exited()) { @@ -1499,7 +1508,7 @@ func settleSourceWith(source func() time.Time, seen time.Time, sigCh <-chan os.S // // scaffold is the theme source builds wrote themselves, which is not a change // (see buildScaffold); each rebuild adds to it. -func watchAndApply(opts LocalRunOptions, serve *ServeServer, rt *LocalRuntime, bundler *bundlerSupervisor, mxbuildPath string, bootSource time.Time, scaffold buildScaffold) error { +func watchAndApply(opts LocalRunOptions, serve *ServeServer, rt *LocalRuntime, bundler *bundlerSupervisor, mxbuildPath string, bootSource time.Time, scaffold buildScaffold, keep *clientSourceKeep) error { w := opts.Stdout // Every recovery re-bundle goes through the supervisor under --watch, so a // one-shot never runs alongside the incremental bundler (#971). @@ -1507,7 +1516,7 @@ func watchAndApply(opts LocalRunOptions, serve *ServeServer, rt *LocalRuntime, b DeployDir: opts.DeployDir, MxBuildPath: mxbuildPath, Timeout: opts.WebClientTimeout, Stdout: opts.Stdout, }) - builds := &serveRecovery{serve: serve, deployDir: opts.DeployDir, out: w} + builds := &serveRecovery{serve: serve, deployDir: opts.DeployDir, out: w, keep: keep} sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) diff --git a/cmd/mxcli/docker/serve_recovery.go b/cmd/mxcli/docker/serve_recovery.go index 436edd8b5..f906da54a 100644 --- a/cmd/mxcli/docker/serve_recovery.go +++ b/cmd/mxcli/docker/serve_recovery.go @@ -37,8 +37,12 @@ import ( // 4 same process, rm web/ Failure - // 5 fresh process true complete again // -// So nothing done to the deployment from outside helps, and a fresh process -// always does. The cost of one is a ~5s start plus a cold ~18s build per change, +// So nothing done to dist/ from outside helps, and a fresh process always does. +// What does help is done before the build: client_source_keep.go puts back the +// client source the previous build deleted, and then the incremental build is +// complete (ako/mxcli#2150, measured on 11.16.0). The restart below is what is +// left when there is no kept source, or the bundle is incomplete all the same. +// The cost of one is a ~5s start plus a cold ~18s build per change, // against a 4s reload that serves no pages — correct beats fast. // // The same process also keeps a theme error past its fix, in every shape @@ -80,6 +84,11 @@ type serveRecovery struct { // succeeds with an incomplete web/dist is a good build and is not repeated. headless bool + // keep holds the client source mxbuild deletes after each build and puts + // it back before the next, so an incremental build bundles a complete + // client (client_source_keep.go). Nil keeps nothing. + keep *clientSourceKeep + // confirmedErrors is the error signature a fresh process last confirmed // for a suspect incremental failure (see suspectIncrementalErrors). The same // errors again are the model's, not the process's, and are not rechecked @@ -135,7 +144,8 @@ func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { } before, prebuilt := readPrebuiltBundle(r.deployDir) - res, err := r.serve.Build(req) + restored := r.keep.restore() + res, err := r.buildKeeping(req) if err != nil { return res, err } @@ -184,6 +194,15 @@ func (r *serveRecovery) Build(req BuildRequest) (*BuildResult, error) { return r.recheckIncrementalErrors(req, res) } + if restored > 0 && !freshProcess && len(res.Errors()) == 0 && isBundleFailure(res) { + // The bundler failed on source the keep put back, and the model has no + // error of its own: the kept source may be stale or a copy may be bad. + // A cold build writes all of it again. + fmt.Fprintln(r.out, " the app bundle did not compile from the client source kept from earlier builds; "+ + "restarting mxbuild --serve for a full build...") + return r.rebuildFresh(req) + } + if legacyClientBuildHint(r.deployDir, res.Message, string(res.Raw)) != "" { fmt.Fprintln(r.out, " mxbuild --serve cannot build this change incrementally (it wants client source its "+ "first build does not leave behind); restarting mxbuild --serve for a full build...") @@ -232,7 +251,10 @@ func (r *serveRecovery) rebuildFresh(req BuildRequest) (*BuildResult, error) { if err := r.restart(); err != nil { return nil, err } - res, err := r.serve.Build(req) + // A cold build writes every client source file, so the keep starts over: + // nothing of a document deleted since survives in it. + r.keep.reset() + res, err := r.buildKeeping(req) if err == nil { r.observe(res) if res != nil && res.OK() { @@ -242,6 +264,35 @@ func (r *serveRecovery) rebuildFresh(req BuildRequest) (*BuildResult, error) { return res, err } +// buildKeeping runs one build while the keep copies aside the client source +// that build writes and then deletes. +func (r *serveRecovery) buildKeeping(req BuildRequest) (*BuildResult, error) { + watching := r.keep.active() + stop := r.keep.watch() + res, err := r.serve.Build(req) + stop(err == nil && res != nil && res.OK()) + if watching && err == nil && webDirInUse(res) { + // Windows refuses to delete a file or directory another process has + // open, and the keep was reading web/ while mxbuild rebuilt it. The + // keep is given up rather than raced again: builds go back to the full + // rebuild per change, which is slow and always worked. + fmt.Fprintln(r.out, " deployment/web was in use while mxbuild rebuilt it; mxcli no longer keeps the client source "+ + "between builds in this run (changes take a full build again), and builds once more...") + r.keep.disable() + return r.serve.Build(req) + } + return res, err +} + +// bundleCompileMessage is mxbuild's message when its bundler fails. +const bundleCompileMessage = "Compilation of the app bundle failed" + +// isBundleFailure reports whether a failed build failed in the bundler. +func isBundleFailure(res *BuildResult) bool { + return res != nil && !res.OK() && + (strings.Contains(res.Message, bundleCompileMessage) || strings.Contains(string(res.Raw), bundleCompileMessage)) +} + func (r *serveRecovery) restart() error { r.themeFailed = false // whatever the old process held went with it if err := r.serve.Restart(); err != nil { diff --git a/cmd/mxcli/testrunner/build_attribution.go b/cmd/mxcli/testrunner/build_attribution.go index e35042828..fc42c5912 100644 --- a/cmd/mxcli/testrunner/build_attribution.go +++ b/cmd/mxcli/testrunner/build_attribution.go @@ -227,6 +227,7 @@ func buildFailureHint(other []docker.BuildProblem) string { var runnerDocuments = map[string]string{ endpointStartupFlow: "to register the test endpoint at startup and then run your after-startup microflow", endpointRegisterAction: "to register the test endpoint", + errorMessageFlow: "to read a raised error's message for the test microflows", } // runnerDocumentPattern matches the `document` MxBuild reports — the document's diff --git a/cmd/mxcli/testrunner/caller_entity_access.go b/cmd/mxcli/testrunner/caller_entity_access.go new file mode 100644 index 000000000..e8e6f90d5 --- /dev/null +++ b/cmd/mxcli/testrunner/caller_entity_access.go @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: Apache-2.0 + +// The generated test callers apply entity access (ako/mxcli#2100, upstream +// mendixlabs/mxcli#1805). +// +// A test calls the project's microflows, and MxBuild refuses a microflow that +// does not apply entity access calling one that does (CE0114). The callers — +// each endpoint test microflow, the legacy runner's MxTest.TestRunner, and the +// MxTest.Check_* wrappers `mxcli check` and the editor see — were written +// without the setting, so a test could not call a production microflow that +// applies it, and changing the production microflow to suit the harness is no +// fix. The rule has one direction (MDL-SEC22's table; RegisterEndpoint applies +// entity access for the same reason, ako/mxcli#1654), so an applying caller +// may call either kind. +// +// Applying entity access costs one thing, measured with mx check on 11.12.1 at +// security Off and Production: the flow may not read an attribute of +// System.Error or System.SoapFault (CE0150). The generated handlers read the +// raised message, so they pass $latestError to errorMessageFlow, a helper that +// does not apply entity access — what CE0150's own text prescribes. An applying +// flow reading an ordinary entity's attribute is 0 errors at both levels, so +// nothing else in a test body is affected. +// +// Nothing changes at run time: every test runs under a system context +// (Core.createSystemContext), which entity access does not restrict. +package testrunner + +import ( + "regexp" + "strconv" + "strings" + + "github.com/mendixlabs/mxcli/mdl/langver" +) + +const applyEntityAccessAnnotation = "@applyentityaccess" + +// callerAppliesEntityAccess reports whether the generated caller of these +// tests applies entity access: yes, unless a test body itself reads an error +// object's attributes. Such a body — a custom handler inspecting $latestError — +// builds today in a non-applying caller and would fail CE0150 in an applying +// one, so it keeps the setting it had. A test that both reads the error and +// calls an applying microflow cannot be served by either setting; it has to +// pass the error to a non-applying microflow of its own, as Mendix says. +func callerAppliesEntityAccess(tests ...TestCase) bool { + for _, tc := range tests { + if readsErrorAttributes(tc.MDL, tc.Version) { + return false + } + } + return true +} + +// errorAttributeAccess is a member read on the variables Mendix binds to a +// raised error inside a handler, matched without regard to case so no spelling +// the model accepts slips past: a miss would cost the test its build. +var errorAttributeAccess = regexp.MustCompile(`(?i)\$latest(error|soapfault)\s*/`) + +// readsErrorAttributes reports whether a test body reads an attribute of the +// error being handled, outside string literals and comments. +func readsErrorAttributes(body string, v langver.Version) bool { + return errorAttributeAccess.MatchString(maskLiteralsAndComments(body, v)) +} + +// maskLiteralsAndComments blanks the text inside string literals and comments, +// with the same rules as scanLiteralState, so a pattern matched against the +// result only sees code. +func maskLiteralsAndComments(body string, v langver.Version) string { + b := []byte(body) + inString, inLineComment, inBlockComment := false, false, false + for i := 0; i < len(b); i++ { + c := b[i] + switch { + case c == '\n': + inLineComment = false + case inLineComment: + b[i] = ' ' + case inBlockComment: + if c == '*' && i+1 < len(b) && b[i+1] == '/' { + inBlockComment = false + b[i], b[i+1] = ' ', ' ' + i++ + continue + } + b[i] = ' ' + case inString: + switch { + case c == '\\' && v < langver.V1 && i+1 < len(b): + b[i] = ' ' + if b[i+1] != '\n' { + b[i+1] = ' ' + } + i++ + case c == '\'': + inString = false + default: + b[i] = ' ' + } + case c == '\'': + inString = true + case c == '-' && i+1 < len(b) && b[i+1] == '-': + inLineComment = true + b[i] = ' ' + case c == '/' && i+1 < len(b) && b[i+1] == '*': + inBlockComment = true + b[i], b[i+1] = ' ', ' ' + i++ + } + } + return string(b) +} + +// suiteHasFlows reports whether a generated script writes any test microflow, +// and so needs errorMessageFlow: a test whose @expect did not compile gets none. +func suiteHasFlows(suite *TestSuite) bool { + for _, tc := range suite.Tests { + if len(tc.AssertionErrors) == 0 { + return true + } + } + return false +} + +// writeErrorMessageFlow defines errorMessageFlow. It does not apply entity +// access — that is its whole purpose. It is written with the test microflows +// rather than with the endpoint, so a run attached to an endpoint an older +// mxcli installed still has it, and it is removed with them. +func writeErrorMessageFlow(b *strings.Builder) { + b.WriteString("/** Returns a raised error's message to a test microflow, which applies entity access and may not read it (CE0150). */\n") + b.WriteString(createFlow + " " + errorMessageFlow + " (RaisedError: System.Error)\n") + b.WriteString("RETURNS String AS $Message\n") + b.WriteString("BEGIN\n") + b.WriteString(" DECLARE $Message String = $RaisedError/Message;\n") + b.WriteString(" RETURN $Message;\n") + writeFlowEnd(b) +} + +// errorMessageVar names the variable the nth handler of a test reads the +// message into. scope keeps the legacy runner's tests, which share one +// microflow, apart; the endpoint's tests each have their own. +func errorMessageVar(scope string, n int) string { + return "$MxTestErrorMessage" + scope + "_" + strconv.Itoa(n) +} + +// readErrorMessage is the handler line that puts the message of the error +// being handled in msgVar. +func readErrorMessage(msgVar string) string { + return " " + msgVar + " = CALL MICROFLOW " + errorMessageFlow + "(RaisedError = $latestError);" +} diff --git a/cmd/mxcli/testrunner/caller_entity_access_test.go b/cmd/mxcli/testrunner/caller_entity_access_test.go new file mode 100644 index 000000000..e563c60a6 --- /dev/null +++ b/cmd/mxcli/testrunner/caller_entity_access_test.go @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "regexp" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// The generated test callers apply entity access, and read a raised error's +// message through a helper that does not (ako/mxcli#2100, mendixlabs/mxcli#1805). +// +// Measured with mx check on 11.12.1, at security Off and Production alike: +// +// - a microflow that does not apply entity access calling one that does is +// CE0114, so a test could not call a production microflow that applies it; +// - a microflow that applies entity access reading $latestError/Message is +// CE0150 ("attributes of System.Error … cannot be accessed in microflows +// that apply entity access. Pass the object to a microflow that does not +// apply entity access if you want to inspect it."); +// - an applying flow passing $latestError to a non-applying helper that +// returns $Error/Message is 0 errors — Mendix's own prescription. +// +// The tests run under a system context, which entity access does not restrict, +// so the setting changes what compiles and nothing about what a test sees. + +const callerSuiteSrc = `mdl 1; +/** + * @test calls an applying microflow + * @expect $ok + */ +$ok = call microflow MyModule.ACT_Applying(); +$ok2 = call microflow MyModule.ACT_Applying(); +/ + +/** + * @test throws with a message + * @throws 'must be positive' + */ +$r = call microflow MyModule.Apply(Amount = -5.0); +/ +` + +func callerSuite(t *testing.T, src string) *TestSuite { + t.Helper() + tests, err := parseMDLTests(src, "t.test.mdl") + if err != nil { + t.Fatalf("parse: %v", err) + } + return &TestSuite{Name: "t", Tests: tests} +} + +// microflowsIn parses a generated script and returns its microflows by name. +func microflowsIn(t *testing.T, mdl string) map[string]*ast.CreateMicroflowStmt { + t.Helper() + prog, errs := visitor.Build(mdl) + if len(errs) > 0 { + t.Fatalf("generated MDL does not parse: %v\n%s", errs[0], mdl) + } + flows := map[string]*ast.CreateMicroflowStmt{} + for _, s := range prog.Statements { + if mf, ok := s.(*ast.CreateMicroflowStmt); ok { + flows[mf.Name.String()] = mf + } + } + return flows +} + +func applies(mf *ast.CreateMicroflowStmt) bool { + return mf.ApplyEntityAccess != nil && *mf.ApplyEntityAccess +} + +// outsideHelper is the script with the helper's own definition removed: the +// helper is the one place that may read an error's attributes. +func outsideHelper(mdl string) string { + start := strings.Index(mdl, createFlow+" "+errorMessageFlow) + if start < 0 { + return mdl + } + end := strings.Index(mdl[start:], "END;") + return mdl[:start] + mdl[start+end:] +} + +var errorAttributeRead = regexp.MustCompile(`\$latest(Error|SoapFault)/`) + +func TestEndpointTestFlowsApplyEntityAccess(t *testing.T) { + suite := callerSuite(t, callerSuiteSrc) + mdl := GenerateTestFlows(suite) + flows := microflowsIn(t, mdl) + for _, tc := range suite.Tests { + mf := flows[testFlowName(tc)] + if mf == nil { + t.Fatalf("no %s in:\n%s", testFlowName(tc), mdl) + } + if !applies(mf) { + t.Errorf("%s does not apply entity access, so it cannot call a microflow that does (CE0114)", testFlowName(tc)) + } + } + helper := flows[errorMessageFlow] + if helper == nil { + t.Fatalf("no %s in:\n%s", errorMessageFlow, mdl) + } + if applies(helper) { + t.Errorf("%s applies entity access, so it cannot read the error's message either (CE0150)", errorMessageFlow) + } + if n := strings.Count(mdl, createFlow+" "+errorMessageFlow); n != 1 { + t.Errorf("%s is defined %d times, want once", errorMessageFlow, n) + } + if m := errorAttributeRead.FindString(outsideHelper(mdl)); m != "" { + t.Errorf("an applying test flow reads %q directly (CE0150):\n%s", m, mdl) + } + // The verdict and the @throws comparison still carry the message. + for _, want := range []string{ + "CALL MICROFLOW " + errorMessageFlow + "(RaisedError = $latestError)", + "'" + verdictFailPrefix + "exception during execution: ' + $MxTestErrorMessage_1", + "contains($MxTestErrorMessage_1, 'must be positive')", + } { + if !strings.Contains(mdl, want) { + t.Errorf("generated flows do not contain %q:\n%s", want, mdl) + } + } +} + +// Every handler reads the message into its own variable: a body with two +// calls gets two handlers in one microflow. +func TestErrorMessageVariablePerHandler(t *testing.T) { + mdl := GenerateTestFlows(callerSuite(t, callerSuiteSrc)) + for _, v := range []string{"$MxTestErrorMessage_1 = CALL", "$MxTestErrorMessage_2 = CALL"} { + if !strings.Contains(mdl, v) { + t.Errorf("want %q in:\n%s", v, mdl) + } + } +} + +func TestLegacyTestRunnerAppliesEntityAccess(t *testing.T) { + mdl := GenerateTestRunner(callerSuite(t, callerSuiteSrc)) + flows := microflowsIn(t, mdl) + runner := flows[mxTestRunner] + if runner == nil { + t.Fatalf("no %s in:\n%s", mxTestRunner, mdl) + } + if !applies(runner) { + t.Errorf("%s does not apply entity access (CE0114)", mxTestRunner) + } + if helper := flows[errorMessageFlow]; helper == nil || applies(helper) { + t.Errorf("want a non-applying %s, got %+v", errorMessageFlow, helper) + } + if m := errorAttributeRead.FindString(outsideHelper(mdl)); m != "" { + t.Errorf("the applying runner reads %q directly (CE0150):\n%s", m, mdl) + } + // Two tests share the runner, so their message variables must differ. + if !strings.Contains(mdl, "contains($MxTestErrorMessage_2_1, 'must be positive')") { + t.Errorf("the @throws comparison does not read the helper's result:\n%s", mdl) + } +} + +func TestCheckWrapperAppliesEntityAccess(t *testing.T) { + cs, err := CheckSource(callerSuiteSrc, "t.test.mdl") + if err != nil { + t.Fatal(err) + } + flows := microflowsIn(t, cs.MDL) + n := 0 + for name, mf := range flows { + if !strings.HasPrefix(name, mxTestModule+".Check_") { + continue + } + n++ + if !applies(mf) { + t.Errorf("%s does not apply entity access, so check reports MDL-SEC22 for a call the run accepts", name) + } + } + if n != 2 { + t.Errorf("found %d check wrappers, want 2:\n%s", n, cs.MDL) + } +} + +// A test whose own handler reads the error's attributes keeps a caller that +// does not apply entity access: applying it would fail that read (CE0150) in a +// test that builds today. The control is the same text inside a string, which +// reads nothing. +func TestBodyReadingErrorAttributesKeepsCallerNonApplying(t *testing.T) { + const reads = `mdl 1; +/** + * @test inspects the error itself + * @expect $msg != empty + */ +declare $msg String = empty; +$r = call microflow MyModule.Fail() on error without rollback begin + set $msg = $latestError/Message; +end error; +/ +` + control := strings.Replace(reads, "set $msg = $latestError/Message;", "set $msg = '$latestError/Message';", 1) + + for _, c := range []struct { + name, src string + want bool + }{{"reads the error", reads, false}, {"control: only a string", control, true}} { + t.Run(c.name, func(t *testing.T) { + suite := callerSuite(t, c.src) + if got := applies(microflowsIn(t, GenerateTestFlows(suite))[testFlowName(suite.Tests[0])]); got != c.want { + t.Errorf("endpoint test flow applies entity access = %v, want %v", got, c.want) + } + if got := applies(microflowsIn(t, GenerateTestRunner(suite))[mxTestRunner]); got != c.want { + t.Errorf("legacy runner applies entity access = %v, want %v", got, c.want) + } + cs, err := CheckSource(c.src, "t.test.mdl") + if err != nil { + t.Fatal(err) + } + for name, mf := range microflowsIn(t, cs.MDL) { + if got := applies(mf); got != c.want { + t.Errorf("check wrapper %s applies entity access = %v, want %v", name, got, c.want) + } + } + }) + } +} + +// The helper is generated for the run, so cleanup removes it with the test +// flows, and a build error in it is mxcli's. +func TestErrorMessageFlowIsCleanedUpAndAttributed(t *testing.T) { + got := generatedTestFlowNames([]string{"Test_test_1", strings.TrimPrefix(errorMessageFlow, mxTestModule+"."), "MyOwnFlow"}) + if strings.Join(got, ",") != testFlowPrefix+"test_1,"+errorMessageFlow { + t.Errorf("discovered for cleanup: %v", got) + } + if names := suiteTestFlowNames(suiteOf("test_1")); !containsString(names, errorMessageFlow) { + t.Errorf("the fallback drop list misses %s: %v", errorMessageFlow, names) + } + if cmds := cleanupCommands(projectState{}, true); !containsString(cmds, "DROP MICROFLOW "+errorMessageFlow) { + t.Errorf("legacy cleanup in a pre-existing MxTest module does not drop %s: %v", errorMessageFlow, cmds) + } + + ce := problem("CE0109", "Undefined variable 'x'.", "MxTest", "Microflow '"+strings.TrimPrefix(errorMessageFlow, mxTestModule+".")+"'", "End event") + _, err := resultsForBuildFailure(buildFailure(failedBuild(ce)), suiteOf("test_1")) + if err == nil || !strings.Contains(err.Error(), "defect in mxcli") || strings.Contains(err.Error(), "left behind") { + t.Errorf("an error in %s is not reported as mxcli's: %v", errorMessageFlow, err) + } +} + +func containsString(list []string, s string) bool { + for _, x := range list { + if x == s { + return true + } + } + return false +} diff --git a/cmd/mxcli/testrunner/check_source.go b/cmd/mxcli/testrunner/check_source.go index 96f67429f..60910c2a8 100644 --- a/cmd/mxcli/testrunner/check_source.go +++ b/cmd/mxcli/testrunner/check_source.go @@ -119,7 +119,13 @@ func CheckSource(content, path string) (CheckedSource, error) { // `/` is refused under mdl 1, and under mdl 0 it drew an MDL-V1-SLASH // warning for a separator that is the test format's, not a terminator // the author wrote. - place(out, first-1, fmt.Sprintf("CREATE OR MODIFY MICROFLOW %s.%s () BEGIN", mxTestModule, checkFlowName(tc, i))) + // The wrapper applies entity access as the run's caller does, on the + // same line so no body line moves; see callerAppliesEntityAccess. + wrapper := fmt.Sprintf("CREATE OR MODIFY MICROFLOW %s.%s () BEGIN", mxTestModule, checkFlowName(tc, i)) + if callerAppliesEntityAccess(tc) { + wrapper = applyEntityAccessAnnotation + " " + wrapper + } + place(out, first-1, wrapper) place(out, first+len(body), "END;") } diff --git a/cmd/mxcli/testrunner/cleanup_leftovers.go b/cmd/mxcli/testrunner/cleanup_leftovers.go index 6f68ac160..8a66b29c4 100644 --- a/cmd/mxcli/testrunner/cleanup_leftovers.go +++ b/cmd/mxcli/testrunner/cleanup_leftovers.go @@ -95,6 +95,9 @@ func suiteTestFlowNames(suite *TestSuite) []string { for _, tc := range suite.Tests { names = append(names, testFlowName(tc)) } + if len(names) > 0 { + names = append(names, errorMessageFlow) + } return names } diff --git a/cmd/mxcli/testrunner/endpoint.go b/cmd/mxcli/testrunner/endpoint.go index bec837e82..63cfdaa61 100644 --- a/cmd/mxcli/testrunner/endpoint.go +++ b/cmd/mxcli/testrunner/endpoint.go @@ -31,6 +31,12 @@ const ( endpointStartupFlow = mxTestModule + ".RegisterEndpoint" // testFlowPrefix prefixes every generated per-test microflow. testFlowPrefix = mxTestModule + ".Test_" + // errorMessageFlow returns a raised error's message to a generated test + // caller, which applies entity access and so may not read it itself + // (CE0150, ako/mxcli#2100). It carries the generated prefix so every path + // that finds and removes the generated flows removes it too, and no test ID + // (test_N) can produce its name. + errorMessageFlow = testFlowPrefix + "ErrorMessage" ) // newEndpointToken returns a fresh 256-bit token as hex. diff --git a/cmd/mxcli/testrunner/generator.go b/cmd/mxcli/testrunner/generator.go index bae60d8d9..423775edf 100644 --- a/cmd/mxcli/testrunner/generator.go +++ b/cmd/mxcli/testrunner/generator.go @@ -112,6 +112,15 @@ func GenerateTestRunner(suite *TestSuite) string { // Module creation (idempotent — only if MxTest module doesn't exist) b.WriteString("CREATE MODULE MxTest;\n\n") + if suiteHasFlows(suite) { + writeErrorMessageFlow(&b) + b.WriteString("\n") + } + // One microflow runs every test, so it applies entity access only if all of + // them allow it; see callerAppliesEntityAccess. + if callerAppliesEntityAccess(suite.Tests...) { + b.WriteString(applyEntityAccessAnnotation + "\n") + } b.WriteString(createFlow + " MxTest.TestRunner ()\n") b.WriteString("RETURNS Boolean AS $AllPassed\n") b.WriteString("BEGIN\n") @@ -224,7 +233,7 @@ func writeThrowsTestBlock(b *strings.Builder, tc TestCase, suffix string) { renamedMDL := renameVariables(tc.MDL, varNames, suffix) lines := strings.Split(renamedMDL, "\n") - rewritten := rewriteForThrowsTest(lines, didThrowVar, tc.Throws, tc.Version) + rewritten := rewriteForThrowsTest(lines, didThrowVar, suffix, tc.Throws, tc.Version) writeBodyLines(b, rewritten, tc.Version) // Check that it did throw @@ -373,12 +382,14 @@ func renameExpect(exp Expect, names map[string]bool, suffix string) Expect { // with a return value (required when the microflow returns a type). Which calls // get one is attachOnError's decision, shared with the endpoint runner. func rewriteWithErrorHandling(lines []string, testID string, v langver.Version) []string { - return attachOnError(lines, v, []string{ - fmt.Sprintf(" LOG ERROR NODE 'MXTEST' 'MXTEST:FAIL:%s:Exception during execution';", - escapeMDLString(testID)), - " SET $TestFailed = true;", - " SET $AllPassed = false;", - " RETURN $AllPassed;", + return attachOnError(lines, v, func(int) []string { + return []string{ + fmt.Sprintf(" LOG ERROR NODE 'MXTEST' 'MXTEST:FAIL:%s:Exception during execution';", + escapeMDLString(testID)), + " SET $TestFailed = true;", + " SET $AllPassed = false;", + " RETURN $AllPassed;", + } }) } @@ -388,19 +399,22 @@ func rewriteWithErrorHandling(lines []string, testID string, v langver.Version) // runner reports a wrong expectation the same way the endpoint runner does — as // "expected an exception but none was thrown", which is the only outcome its log // protocol has for a throws test that did not get what it asked for. -func rewriteForThrowsTest(lines []string, didThrowVar string, want string, v langver.Version) []string { - return attachOnError(lines, v, throwsFlagHandler(didThrowVar, want)) +func rewriteForThrowsTest(lines []string, didThrowVar, suffix, want string, v langver.Version) []string { + return attachOnError(lines, v, func(n int) []string { + return throwsFlagHandler(didThrowVar, want, errorMessageVar(suffix, n)) + }) } // throwsFlagHandler sets the did-throw flag, gated on the expected message when // the annotation carried one. See throwsHandler for why the match is a substring -// and where $latestError comes from. -func throwsFlagHandler(didThrowVar, want string) []string { +// and where the message in msgVar comes from. +func throwsFlagHandler(didThrowVar, want, msgVar string) []string { if want == "" { return []string{fmt.Sprintf(" SET %s = true;", didThrowVar)} } return []string{ - fmt.Sprintf(" IF contains(%s, '%s') THEN", latestErrorMessage, escapeMDLString(want)), + readErrorMessage(msgVar), + fmt.Sprintf(" IF contains(%s, '%s') THEN", msgVar, escapeMDLString(want)), fmt.Sprintf(" SET %s = true;", didThrowVar), " END IF;", } diff --git a/cmd/mxcli/testrunner/generator_endpoint.go b/cmd/mxcli/testrunner/generator_endpoint.go index f63f780c7..cfeabeaa6 100644 --- a/cmd/mxcli/testrunner/generator_endpoint.go +++ b/cmd/mxcli/testrunner/generator_endpoint.go @@ -50,6 +50,10 @@ func GenerateTestFlows(suite *TestSuite) string { v := suiteVersion(suite) writeScriptHeader(&b, v) b.WriteString("\n") + if suiteHasFlows(suite) { + writeErrorMessageFlow(&b) + b.WriteString("\n") + } for _, tc := range suite.Tests { // A test with an uncompilable @expect gets no microflow. The runner // reports it as an ERROR from the parse message, which is more useful @@ -66,6 +70,9 @@ func GenerateTestFlows(suite *TestSuite) string { // writeTestFlow writes one test's microflow. func writeTestFlow(b *strings.Builder, tc TestCase, v langver.Version) { fmt.Fprintf(b, "/** %s */\n", escapeMDLComment(tc.Name)) + if callerAppliesEntityAccess(tc) { + b.WriteString(applyEntityAccessAnnotation + "\n") + } fmt.Fprintf(b, "%s %s ()\n", createFlow, testFlowName(tc)) b.WriteString("RETURNS String AS $Verdict\n") b.WriteString("BEGIN\n") @@ -222,15 +229,18 @@ func failVerdictExpr(exp Expect) string { // // The verdict carries the raised message: a bare "exception during execution" // left the user guessing, e.g. at an unresolvable [%CurrentUser%] token (tests -// run in a system context with no user). $latestError/Message is the same -// expression throwsHandler uses, measured there. +// run in a system context with no user). The message is read through +// errorMessageFlow, as in throwsHandler: the test flow applies entity access. func rewriteBodyForVerdict(lines []string, tc TestCase) []string { - handler := []string{ - fmt.Sprintf(" SET $Verdict = '%s' + %s;", - escapeMDLString(verdictFailPrefix+"exception during execution: "), latestErrorMessage), - " RETURN $Verdict;", - } - return attachOnError(lines, tc.Version, handler) + return attachOnError(lines, tc.Version, func(n int) []string { + msg := errorMessageVar("", n) + return []string{ + readErrorMessage(msg), + fmt.Sprintf(" SET $Verdict = '%s' + %s;", + escapeMDLString(verdictFailPrefix+"exception during execution: "), msg), + " RETURN $Verdict;", + } + }) } // rewriteBodyForThrows attaches an ON ERROR handler that clears the pre-set @@ -240,7 +250,7 @@ func rewriteBodyForVerdict(lines []string, tc TestCase) []string { // replaces it with one naming both sides, so a failing @throws says what was // raised instead of only what was expected. func rewriteBodyForThrows(lines []string, want string, v langver.Version) []string { - return attachOnError(lines, v, throwsHandler(want)) + return attachOnError(lines, v, func(n int) []string { return throwsHandler(want, errorMessageVar("", n)) }) } // throwsHandler builds the ON ERROR body of an @throws test. @@ -248,31 +258,28 @@ func rewriteBodyForThrows(lines []string, want string, v langver.Version) []stri // $latestError is the error variable Mendix makes available inside a handler; // its Message member is what the annotation is compared against. Measured // against mxbuild 11.6.0 and 11.13.0 at 0 errors, with `$noSuchThing/Message` as -// the control (CE0109, "Undefined variable"). +// the control (CE0109, "Undefined variable"). The handler reads it into msg +// through errorMessageFlow, because the test microflow applies entity access. // // The comparison is `contains`, so an expectation is a substring of the raised // message: a Mendix error message routinely carries an activity name or an // object id the test cannot predict, and demanding equality would make almost // every real expectation unwritable. -func throwsHandler(want string) []string { +func throwsHandler(want, msgVar string) []string { if want == "" { return []string{fmt.Sprintf(" SET $Verdict = '%s';", verdictPass)} } msg := verdictFailPrefix + "expected an error containing " + quoteForMessage(want) + ", actual: " return []string{ - fmt.Sprintf(" IF contains(%s, '%s') THEN", latestErrorMessage, escapeMDLString(want)), + readErrorMessage(msgVar), + fmt.Sprintf(" IF contains(%s, '%s') THEN", msgVar, escapeMDLString(want)), fmt.Sprintf(" SET $Verdict = '%s';", verdictPass), " ELSE", - fmt.Sprintf(" SET $Verdict = '%s' + %s;", escapeMDLString(msg), latestErrorMessage), + fmt.Sprintf(" SET $Verdict = '%s' + %s;", escapeMDLString(msg), msgVar), " END IF;", } } -// latestErrorMessage is the MDL expression for the message of the error being -// handled. Named once because it appears in both branches of every @throws -// handler and in the legacy runner. -const latestErrorMessage = "$latestError/Message" - // quoteForMessage renders the expected message for a human-readable verdict, // before that verdict is itself escaped for MDL. func quoteForMessage(s string) string { @@ -309,7 +316,10 @@ func quoteForMessage(s string) string { // semicolons) of its own, and `on error` may be inside a string or a comment. // A body that does not parse is returned unchanged — no handler can make it // parse, and the error the author then sees is about their own text. -func attachOnError(lines []string, v langver.Version, handler []string) []string { +// +// handler is called once per wrapped call with its number, from 1 in source +// order, so a handler that declares a variable can give each its own name. +func attachOnError(lines []string, v langver.Version, handler func(n int) []string) []string { body := strings.Join(lines, "\n") prefix := bodyParsePrefix(v) stmts, ok := visitor.FlowStatements(prefix + body + bodyParseSuffix) @@ -320,7 +330,9 @@ func attachOnError(lines []string, v langver.Version, handler []string) []string runes := []rune(body) // The handler is laid out at the indentation of the line its call starts // on, so a call nested in an IF or a loop reads as nested in --verbose. + n := 0 block := func(callStart int) string { + n++ lineStart := callStart for lineStart > 0 && runes[lineStart-1] != '\n' { lineStart-- @@ -332,7 +344,7 @@ func attachOnError(lines []string, v langver.Version, handler []string) []string pad := string(runes[lineStart:indent]) var b strings.Builder b.WriteString("ON ERROR BEGIN\n") - for _, l := range handler { + for _, l := range handler(n) { b.WriteString(pad + l + "\n") } b.WriteString(pad + "END ERROR") diff --git a/cmd/mxcli/testrunner/generator_endpoint_test.go b/cmd/mxcli/testrunner/generator_endpoint_test.go index baceffc9d..8f497057f 100644 --- a/cmd/mxcli/testrunner/generator_endpoint_test.go +++ b/cmd/mxcli/testrunner/generator_endpoint_test.go @@ -25,8 +25,9 @@ func TestGenerateTestFlowsOneMicroflowPerTest(t *testing.T) { t.Errorf("generated MDL is missing %q", want) } } - if n := strings.Count(mdl, "CREATE OR MODIFY MICROFLOW"); n != 2 { - t.Errorf("got %d microflows, want one per test (2)", n) + // Plus the one errorMessageFlow every handler shares (ako/mxcli#2100). + if n := strings.Count(mdl, "CREATE OR MODIFY MICROFLOW "+testFlowPrefix+"test_"); n != 2 { + t.Errorf("got %d test microflows, want one per test (2)", n) } } @@ -113,9 +114,15 @@ func TestGenerateTestFlowsFailVerdictCarriesErrorMessage(t *testing.T) { }}} mdl := GenerateTestFlows(suite) - want := "SET $Verdict = '" + verdictFailPrefix + "exception during execution: ' + " + latestErrorMessage + ";" - if !strings.Contains(mdl, want) { - t.Errorf("the error handler does not carry $latestError/Message into the verdict; want %q in:\n%s", want, mdl) + // The message comes through errorMessageFlow: the test flow applies entity + // access and may not read $latestError/Message itself (ako/mxcli#2100). + for _, want := range []string{ + readErrorMessage("$MxTestErrorMessage_1"), + "SET $Verdict = '" + verdictFailPrefix + "exception during execution: ' + $MxTestErrorMessage_1;", + } { + if !strings.Contains(mdl, want) { + t.Errorf("the error handler does not carry the raised message into the verdict; want %q in:\n%s", want, mdl) + } } } @@ -200,6 +207,7 @@ func TestEndpointCleanupCommands(t *testing.T) { "ALTER SETTINGS RUNTIME (AfterStartupMicroflow: '')", "DROP MICROFLOW MxTest.Test_test_1", "DROP MICROFLOW MxTest.Test_test_2", + "DROP MICROFLOW " + errorMessageFlow, "DROP MICROFLOW " + endpointStartupFlow, "DROP JAVA ACTION " + endpointRegisterAction, }, diff --git a/cmd/mxcli/testrunner/handshake_test.go b/cmd/mxcli/testrunner/handshake_test.go index 0d1a25a9c..8450ecd27 100644 --- a/cmd/mxcli/testrunner/handshake_test.go +++ b/cmd/mxcli/testrunner/handshake_test.go @@ -149,7 +149,7 @@ func TestGenerateEndpointMDLNoChainWhenNone(t *testing.T) { func TestDropTestFlows(t *testing.T) { got := dropTestFlows("", &TestSuite{Tests: []TestCase{{ID: "test_1"}, {ID: "test_2"}}}) - want := []string{"DROP MICROFLOW MxTest.Test_test_1", "DROP MICROFLOW MxTest.Test_test_2"} + want := []string{"DROP MICROFLOW MxTest.Test_test_1", "DROP MICROFLOW MxTest.Test_test_2", "DROP MICROFLOW " + errorMessageFlow} if len(got) != len(want) { t.Fatalf("got %q, want %q", got, want) } diff --git a/cmd/mxcli/testrunner/runner.go b/cmd/mxcli/testrunner/runner.go index c8052c59a..8e5861226 100644 --- a/cmd/mxcli/testrunner/runner.go +++ b/cmd/mxcli/testrunner/runner.go @@ -941,7 +941,7 @@ func cleanupCommands(st projectState, mxTestPresent bool) []string { if st.createdMxTest { cmds = append(cmds, "DROP MODULE "+mxTestModule) } else { - cmds = append(cmds, "DROP MICROFLOW "+mxTestRunner) + cmds = append(cmds, "DROP MICROFLOW "+mxTestRunner, "DROP MICROFLOW "+errorMessageFlow) } return cmds } diff --git a/cmd/mxcli/testrunner/runner_cleanup_test.go b/cmd/mxcli/testrunner/runner_cleanup_test.go index ced296bb9..f8d17fba1 100644 --- a/cmd/mxcli/testrunner/runner_cleanup_test.go +++ b/cmd/mxcli/testrunner/runner_cleanup_test.go @@ -188,6 +188,7 @@ func TestCleanupCommands(t *testing.T) { want: []string{ "ALTER SETTINGS RUNTIME (AfterStartupMicroflow: 'Mod.Flow')", "DROP MICROFLOW MxTest.TestRunner", + "DROP MICROFLOW " + errorMessageFlow, }, }, { diff --git a/cmd/mxcli/testrunner/throws_test.go b/cmd/mxcli/testrunner/throws_test.go index a293f11cd..721d29f9d 100644 --- a/cmd/mxcli/testrunner/throws_test.go +++ b/cmd/mxcli/testrunner/throws_test.go @@ -83,13 +83,13 @@ $result = call microflow M.MF_Import(Json = '{}'); suite := &TestSuite{Name: "s", Tests: []TestCase{tc}} mdl := GenerateTestFlows(suite) - want := "IF contains($latestError/Message, 'the object was not found') THEN" + want := "IF contains($MxTestErrorMessage_1, 'the object was not found') THEN" if !strings.Contains(mdl, want) { t.Errorf("generated handler does not compare the message.\nwant a line: %s\ngot:\n%s", want, mdl) } // The failing branch must name what was actually raised — a verdict that // says only what was expected tells you nothing about what came back. - if !strings.Contains(mdl, "actual: ' + $latestError/Message") { + if !strings.Contains(mdl, "actual: ' + $MxTestErrorMessage_1") { t.Errorf("generated handler does not report the raised message:\n%s", mdl) } } @@ -106,7 +106,7 @@ $result = call microflow M.MF_Import(Json = '{}'); `) mdl := GenerateTestRunner(&TestSuite{Name: "s", Tests: []TestCase{tc}}) - if !strings.Contains(mdl, "contains($latestError/Message, 'not found')") { + if !strings.Contains(mdl, "contains($MxTestErrorMessage_1_1, 'not found')") { t.Errorf("legacy runner does not compare the message:\n%s", mdl) } } @@ -122,7 +122,7 @@ $result = call microflow M.MF_Import(Json = '{}'); / `) mdl := GenerateTestFlows(&TestSuite{Name: "s", Tests: []TestCase{tc}}) - if !strings.Contains(mdl, "contains($latestError/Message, 'it''''s missing')") { + if !strings.Contains(mdl, "contains($MxTestErrorMessage_1, 'it''''s missing')") { t.Errorf("apostrophe not doubled for MDL:\n%s", mdl) } } diff --git a/docs-site/src/reference/page/create-page.md b/docs-site/src/reference/page/create-page.md index e62da012f..0d445871b 100644 --- a/docs-site/src/reference/page/create-page.md +++ b/docs-site/src/reference/page/create-page.md @@ -238,7 +238,14 @@ DATAGRID dgOrders (DataSource: DATABASE FROM Sales.Order, Selection: Single) { A `SHOW PAGE` in the control bar takes the selection the same way — `Action: SHOW PAGE Sales.Order_Detail(Order = $dgOrders)` opens the page with -the selected row, and the button is disabled until a row is selected. +the selected row, and the button is disabled until a row is selected. In a grid +with `Selection`, a `SHOW PAGE` in a column's custom content may pass the +grid's selection by name as well, which is what Studio Pro stores there; +`$currentObject` is the row. + +A control bar takes its object from above its grid, but sits inside the grid, +so the data view around the grid is an outer one there: `$dvOrder` from the +control bar of a grid in data view `dvOrder` is valid. `mxcli check -p ` reports a missing argument before mxbuild does. @@ -288,6 +295,14 @@ attribute property set by its own key too, such as a Custom chart's attribute (`Module.Other.Attr`) is refused, because stored without the parameter it is mx check CE7247. +An **enclosing data view** is named the same way: inside a data view nested in +`dvOuter`, `$dvOuter.Attr` reads the outer data view's object, and +`$dvOuter/Assoc/Attr` reads over an association from it. This works on input +widgets, on a pluggable widget's attribute property +(`markdown md (stringAttribute: $dvOuter.Output)`) and in an attribute-based +visibility condition (`Visible: $dvOuter.HasError in (true)`). It is the form +Studio Pro stores for these bindings. + ## Parameters `module.Name` diff --git a/docs-site/src/tools/running-tests.md b/docs-site/src/tools/running-tests.md index 2700263d8..2d26da205 100644 --- a/docs-site/src/tools/running-tests.md +++ b/docs-site/src/tools/running-tests.md @@ -450,9 +450,29 @@ The generated startup flow, `MxTest.RegisterEndpoint`, **applies entity access**, so it can call your after-startup microflow whichever way that one is set: Mendix refuses a microflow that does not apply entity access calling one that does (CE0114), and allows the other direction. Your microflow still runs -with its own setting. A build error located in `MxTest.RegisterEndpoint` or -`MxTest.RegisterTestEndpoint` is reported as mxcli's, not your project's — those -documents exist only for the run. +with its own setting. + +The generated test microflows apply entity access for the same reason, so a +test, and its `@setup` microflows, can call a production microflow that applies +it without you changing that microflow's setting. `mxcli check` sees the test +the same way, so it does not report MDL-SEC22 for such a call. Tests run in a +system context, which entity access does not restrict, so what a test reads and +writes is the same either way. + +An applying microflow may not read the attributes of `System.Error` +(CE0150), so the runner's own error handlers pass `$latestError` to a generated +helper, `MxTest.Test_ErrorMessage`, which does not apply entity access and +returns the message. A test whose **own** error handler reads `$latestError/…` +keeps a test microflow that does not apply entity access, so it builds as it +did; if that test also calls a microflow that applies entity access, no setting +serves both, and the handler has to pass `$latestError` to a microflow of yours +that does not apply it, as CE0150's own text says. Under `--legacy-runner` the +whole suite shares one microflow, so one such test keeps all of them +non-applying. + +A build error located in `MxTest.RegisterEndpoint`, +`MxTest.RegisterTestEndpoint` or `MxTest.Test_ErrorMessage` is reported as +mxcli's, not your project's — those documents exist only for the run. ### `@cleanup`: what happens to a test's data diff --git a/mdl-examples/bug-tests/2107-binding-through-outer-data-view.mdl b/mdl-examples/bug-tests/2107-binding-through-outer-data-view.mdl new file mode 100644 index 000000000..6cd2bcebe --- /dev/null +++ b/mdl-examples/bug-tests/2107-binding-through-outer-data-view.mdl @@ -0,0 +1,51 @@ +mdl 1; +-- ============================================================================ +-- Bug ako/mxcli#2107: a binding to an outer data view resolved against the nearest +-- ============================================================================ +-- +-- Symptom (before fix): describe page -> exec of six FactoryManagement pages +-- (Mendix 11.16) failed. Studio Pro stores the binding with the data-view pair +-- SourceVariable {Widget: , PageParameter: …}; describe printed +-- the bare qualified name and the rewrite read it against the NEAREST data view: +-- "`GenAICommons.Span.Output` is an attribute of GenAICommons.Span, not of +-- ConversationalUI.CopyHelper … CE7247" (markdown viewer) +-- "ConversationalUI.TraceView has no attribute HasError" (Visible by attribute) +-- +-- After fix: `$dvOuter.Attr`, and the rooted `$dvOuter/Assoc/Attr`, on a +-- pluggable widget's attribute property and in `Visible: … in (…)`, store the +-- data-view pair (with the association steps); describe prints them so, and +-- the six pages report `Unchanged page`. mx check 0 errors on Mendix 11.16.0 +-- for this script (needs the Markdown widget, as Conversational UI ships it). +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/2107-binding-through-outer-data-view.mdl -p app.mpr +-- mxcli docker check -p app.mpr -- expect 0 errors +-- ============================================================================ + +create module BugTest2107; +create module role BugTest2107.User; + +create persistent entity BugTest2107.Span (Output: String(unlimited), Flag: Boolean default false); +create persistent entity BugTest2107.Trace (SystemPrompt: String(unlimited)); +create association BugTest2107.Span_Trace from BugTest2107.Span to BugTest2107.Trace; +create non-persistent entity BugTest2107.Helper (Note: String(100)); + +create microflow BugTest2107.DS_Helper () returns BugTest2107.Helper +begin + $Helper = create BugTest2107.Helper (Note = 'x'); + return $Helper; +end; +grant execute on microflow BugTest2107.DS_Helper to BugTest2107.User; + +create page BugTest2107.Span_Details (Params: ($Span: BugTest2107.Span), Title: 'Span', Layout: Atlas_Core.PopupLayout) { + dataview dvOuter (DataSource: $Span) { + dataview dvInner (DataSource: microflow BugTest2107.DS_Helper()) { + markdown mdOutput (stringAttribute: $dvOuter.Output) + markdown mdPrompt (stringAttribute: $dvOuter/Span_Trace/SystemPrompt) + container cFlag (Visible: $dvOuter.Flag in (true)) { + dynamictext txtFlag (Content: 'flagged') + } + } + } +}; +grant view on page BugTest2107.Span_Details to BugTest2107.User; diff --git a/mdl-examples/bug-tests/2109-own-container-name-in-item-slots.mdl b/mdl-examples/bug-tests/2109-own-container-name-in-item-slots.mdl new file mode 100644 index 000000000..1fcbdd85b --- /dev/null +++ b/mdl-examples/bug-tests/2109-own-container-name-in-item-slots.mdl @@ -0,0 +1,64 @@ +mdl 1; +-- ============================================================================ +-- Bug ako/mxcli#2109: check refused Studio Pro's own data-container names +-- ============================================================================ +-- +-- Symptom (before fix): describe page -> exec of four FactoryManagement pages +-- (Mendix 11.16) was refused by check on its own output: +-- "`actionButton1` reads `$grid1` …, but `grid1` is the data container the +-- widget sits in directly — its name is a variable only inside a data +-- container nested below it (CE0117)" [MDL-BUTTON02] +-- "$grid1 cannot be stored — … an explicit one is rejected as CE0115" [MDL-PAGEARG01] +-- The stored pages build. Measured shapes: +-- - a button in a SELECTABLE data grid's column passing `$grid1`: Studio Pro +-- stores Variable {Widget: grid1}, the grid's selection, as from its +-- control bar (MxModelReflection.MxObjectType_Select); +-- - `$dataView3` from the filter bar (control bar) of a grid inside dataView3 +-- (OIDC.OIDC_Client_NewEdit): the bar sits inside the grid, so dataView3 is +-- an outer container there; +-- - `$dataView2` in a combo box's option content inside dataView2 +-- (AgentCommons.Version_NewEdit): the option is the combo box's own context. +-- +-- After fix: check accepts all three; exec writes the variable forms Studio +-- Pro stores; mx check 0 errors on Mendix 11.16.0 for this script. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/2109-own-container-name-in-item-slots.mdl -p app.mpr +-- mxcli docker check -p app.mpr -- expect 0 errors +-- ============================================================================ + +create module BugTest2109; +create module role BugTest2109.User; + +create persistent entity BugTest2109.Gate (Name: String(100)); +create persistent entity BugTest2109.Item (Name: String(100)); + +create microflow BugTest2109.ACT_Open ($Gate: BugTest2109.Gate) returns Boolean +begin + return true; +end; +grant execute on microflow BugTest2109.ACT_Open to BugTest2109.User; + +create page BugTest2109.Item_Details (Params: ($Item: BugTest2109.Item), Title: 'Item', Layout: Atlas_Core.PopupLayout) { + dataview dvItem (DataSource: $Item) { + dynamictext txtName (Content: '{1}', ContentParams: ({1} = Name)) + } +}; +grant view on page BugTest2109.Item_Details to BugTest2109.User; + +create page BugTest2109.Overview (Params: ($Gate: BugTest2109.Gate), Title: 'Overview', Layout: Atlas_Core.PopupLayout) { + dataview dvGate (DataSource: $Gate) { + datagrid dgGates (DataSource: database from BugTest2109.Gate) { + controlbar { + actionbutton btnOpen (Caption: 'Open', Action: call microflow BugTest2109.ACT_Open(Gate = $dvGate)) + } + column (Attribute: Name) + } + } + datagrid dgItems (DataSource: database from BugTest2109.Item, Selection: Single) { + column (Attribute: Name, ShowContentAs: customContent) { + actionbutton btnView (Caption: 'View', Action: show page BugTest2109.Item_Details(Item = $dgItems)) + } + } +}; +grant view on page BugTest2109.Overview to BugTest2109.User; diff --git a/mdl-examples/bug-tests/system-workflow-subprocess-members.mdl b/mdl-examples/bug-tests/system-workflow-subprocess-members.mdl new file mode 100644 index 000000000..4612e2e26 --- /dev/null +++ b/mdl-examples/bug-tests/system-workflow-subprocess-members.mdl @@ -0,0 +1,29 @@ +mdl 1; +-- ============================================================================ +-- Bug: System.WorkflowSubProcess was missing from mxcli's System module +-- ============================================================================ +-- +-- Symptom (before fix): System.WorkflowSubProcess, WorkflowSubProcessDefinition, +-- their associations and System.WorkflowSubProcessState (Mendix 11.8+) were +-- unknown to mxcli. A binding over System.WorkflowActivityRecord_WorkflowSubProcess +-- could not resolve and was written as an expression: describe -> exec of +-- TestApp's WorkflowCommons.Snip_WorkflowActivityRecord_Header failed mx check +-- [CE0117] "Error(s) in expression." at Text 'text4' +-- +-- After fix: the members are known for Mendix 11.8 and later (measured from +-- each version's modeler assembly; attributes and lengths from the 11.14.0 +-- model.mdp), the binding is stored as association steps, and this script is +-- mx check 0 errors on 11.14.0. Before 11.8 the names stay unknown, as they +-- are to Mendix. +-- +-- Usage (Mendix 11.8 or later): +-- mxcli exec mdl-examples/bug-tests/system-workflow-subprocess-members.mdl -p app.mpr +-- mxcli docker check -p app.mpr -- expect 0 errors +-- ============================================================================ + +create module BugTestSubProcess; + +create snippet BugTestSubProcess.SN_ActivityHeader (Params: ($Activity: System.WorkflowActivityRecord)) { + dynamictext txtSubProcess (Content: 'Sub-process: {1}', ContentParams: ({1} = $Activity/System.WorkflowActivityRecord_WorkflowSubProcess/Caption)) + dynamictext txtDefinition (Content: 'Definition: {1}', ContentParams: ({1} = $Activity/System.WorkflowActivityRecord_WorkflowSubProcessDefinition/Caption)) +}; diff --git a/mdl-examples/bug-tests/test-runner-2100-applying-callee.test.mdl b/mdl-examples/bug-tests/test-runner-2100-applying-callee.test.mdl new file mode 100644 index 000000000..6d9e17cec --- /dev/null +++ b/mdl-examples/bug-tests/test-runner-2100-applying-callee.test.mdl @@ -0,0 +1,44 @@ +-- ako/mxcli#2100 (upstream mendixlabs/mxcli#1805): the generated test +-- microflows did not apply entity access, so a test could not call a +-- production microflow that does: MxBuild refused every one with CE0114, and +-- `mxcli check` with MDL-SEC22. Applying it alone then failed CE0150 on the +-- runner's own handlers, which read $latestError/Message. +-- +-- Setup: test-runner-2100-setup.mdl. Then: +-- +-- mxcli test mdl-examples/bug-tests/test-runner-2100-applying-callee.test.mdl -p app.mpr --local +-- +-- Measured on 11.12.1 (--local --db-type hsqldb). Before the fix: every test +-- calling Acc.* reported "CE0114 … (at Call microflow activity 'ACT_…')" and +-- none ran. After: 01 and 03 PASS, 02 FAILs with "exception during execution: +-- Amount must be positive" and 04 with "expected an error containing +-- 'something else', actual: Amount must be positive" — the messages arrive +-- through the generated MxTest.Test_ErrorMessage helper. +mdl 1; +/** + * @test 01 calls a microflow that applies entity access + * @expect $ok + */ +$ok = call microflow Acc.ACT_IsReady(); +/ + +/** + * @test 02 an unexpected error fails with its message + * @expect $ok + */ +$ok = call microflow Acc.ACT_Refuse(Amount = -1.0); +/ + +/** + * @test 03 @throws matches the message of an applying microflow + * @throws 'must be positive' + */ +$ok = call microflow Acc.ACT_Refuse(Amount = -1.0); +/ + +/** + * @test 04 @throws with the wrong message fails, naming the actual one + * @throws 'something else' + */ +$ok = call microflow Acc.ACT_Refuse(Amount = -1.0); +/ diff --git a/mdl-examples/bug-tests/test-runner-2100-setup.mdl b/mdl-examples/bug-tests/test-runner-2100-setup.mdl new file mode 100644 index 000000000..9191506b6 --- /dev/null +++ b/mdl-examples/bug-tests/test-runner-2100-setup.mdl @@ -0,0 +1,32 @@ +-- ako/mxcli#2100 (upstream mendixlabs/mxcli#1805): the setup script for +-- test-runner-2100-applying-callee.test.mdl. Apply it before running the tests: +-- +-- mxcli exec mdl-examples/bug-tests/test-runner-2100-setup.mdl -p app.mpr +-- +-- Production microflows that apply entity access, which the generated test +-- microflows (and check's wrappers) could not call. One raises an error, so +-- the runner's handlers have to read its message from an applying flow. +mdl 1; +create module Acc; + +create java action Acc.JA_Fail(Message: String) returns Boolean +as $$ +throw new com.mendix.core.CoreException(Message); +$$; + +/** Applies entity access, as many production actions do. */ +@applyentityaccess +create microflow Acc.ACT_IsReady () returns Boolean +begin + return true; +end; + +/** Applies entity access and refuses a negative amount. */ +@applyentityaccess +create microflow Acc.ACT_Refuse (Amount: Decimal) returns Boolean +begin + if $Amount < 0 then + $Failed = call java action Acc.JA_Fail(Message = 'Amount must be positive'); + end if; + return true; +end; diff --git a/mdl/backend/mcp/widget.go b/mdl/backend/mcp/widget.go index 243970a11..991359b65 100644 --- a/mdl/backend/mcp/widget.go +++ b/mdl/backend/mcp/widget.go @@ -165,6 +165,17 @@ func (w *mcpWidgetBuilder) SetAttribute(propertyKey, attributePath string) { } } +// SetAttributeWithSteps: the MCP payload's attribute has no association +// steps, so a path over associations is reported unsupported rather than sent +// as a direct attribute. +func (w *mcpWidgetBuilder) SetAttributeWithSteps(propertyKey, attributePath string, steps []pages.AttributeRefStep) { + if len(steps) > 0 { + w.note("SetAttributeWithSteps(" + propertyKey + ")") + return + } + w.SetAttribute(propertyKey, attributePath) +} + // SetSourceVariable: the MCP widget payload has no slot for a property's // parameter source, so a `$Param.Attr` binding is reported unsupported rather // than sent as one read from the enclosing object. diff --git a/mdl/backend/modelsdk/enumeration.go b/mdl/backend/modelsdk/enumeration.go index 1b1f2ac7f..b4abddc1d 100644 --- a/mdl/backend/modelsdk/enumeration.go +++ b/mdl/backend/modelsdk/enumeration.go @@ -27,7 +27,7 @@ func (b *Backend) ListEnumerations() ([]*model.Enumeration, error) { // ListJavaActions. Without them `describe enumeration System.X` reports // "not found" and `check --references` rejects an attribute typed against // one, while `describe entity` prints that very type (#1102). - out = append(out, meta.BuildSystemEnumerations()...) + out = append(out, meta.BuildSystemEnumerationsIn(b.systemVersionAtLeast())...) return out, nil } diff --git a/mdl/backend/modelsdk/enumeration_system_test.go b/mdl/backend/modelsdk/enumeration_system_test.go index a22c31a02..d727df25b 100644 --- a/mdl/backend/modelsdk/enumeration_system_test.go +++ b/mdl/backend/modelsdk/enumeration_system_test.go @@ -68,9 +68,17 @@ func TestListEnumerations_IncludesSystemModule(t *testing.T) { t.Errorf("stored enumerations = %d, want 7 (the fixture's own)", stored) } - if got, want := len(enums)-stored, len(meta.SystemEnumerations); got != want { + // The fixture is Mendix 11.6.6, so only the enumerations its System module + // has: System.WorkflowSubProcessState arrived in 11.8 and must not be listed. + if got, want := len(enums)-stored, len(meta.SystemEnumerationsIn(b.systemVersionAtLeast())); got != want { t.Errorf("synthesized System enumerations = %d, want %d", got, want) } + if findSystemEnum(enums, "WorkflowSubProcessState") != nil { + t.Error("System.WorkflowSubProcessState listed for an 11.6.6 project; it was introduced in 11.8") + } + if len(meta.SystemEnumerationsIn(b.systemVersionAtLeast())) >= len(meta.SystemEnumerations) { + t.Error("control: the version filter dropped nothing for 11.6.6") + } activityType := findSystemEnum(enums, "WorkflowActivityType") if activityType == nil { diff --git a/mdl/backend/modelsdk/system_module_read.go b/mdl/backend/modelsdk/system_module_read.go index 40de4ba3a..3e3b82a49 100644 --- a/mdl/backend/modelsdk/system_module_read.go +++ b/mdl/backend/modelsdk/system_module_read.go @@ -27,7 +27,7 @@ import ( func buildSystemDomainModel(atLeast func(major, minor int) bool) *domainmodel.DomainModel { dm := &domainmodel.DomainModel{ContainerID: model.ID(meta.SystemModuleID)} dm.ID = model.ID(meta.SystemDomainModelID) - for _, e := range meta.ModelerSystemEntities() { + for _, e := range meta.ModelerSystemEntitiesIn(atLeast) { ent := &domainmodel.Entity{ Name: e.Name, Persistable: e.Persistable, diff --git a/mdl/backend/modelsdk/system_module_read_test.go b/mdl/backend/modelsdk/system_module_read_test.go index a8a7ec2c0..a2fa11190 100644 --- a/mdl/backend/modelsdk/system_module_read_test.go +++ b/mdl/backend/modelsdk/system_module_read_test.go @@ -102,3 +102,33 @@ func TestSystemDomainModel_SystemMemberFlagsReachTheModel(t *testing.T) { t.Errorf("System.UserRole stores no system members, got %+v", ur) } } + +// The workflow sub-process members exist from Mendix 11.8 (measured from each +// version's modeler assembly). TestApp (11.14.0) reads +// System.WorkflowActivityRecord_WorkflowSubProcess/Caption, which could not +// resolve while the System module lacked them: the binding was written as an +// expression, mx check CE0117. Before 11.8 the names must stay absent, or a +// check accepts what the build then refuses as CE1613. +func TestSystemDomainModel_WorkflowSubProcessByVersion(t *testing.T) { + atLeast := func(major, minor int) func(int, int) bool { + return func(ma, mi int) bool { return major > ma || major == ma && minor >= mi } + } + has := func(dm *domainmodel.DomainModel) (entity, assoc bool) { + for _, e := range dm.Entities { + entity = entity || e.Name == "WorkflowSubProcess" + } + for _, a := range dm.Associations { + assoc = assoc || a.Name == "WorkflowActivityRecord_WorkflowSubProcess" + } + return + } + if e, a := has(buildSystemDomainModel(atLeast(11, 14))); !e || !a { + t.Errorf("11.14: entity %v, association %v; want both", e, a) + } + if e, a := has(buildSystemDomainModel(atLeast(11, 8))); !e || !a { + t.Errorf("11.8: entity %v, association %v; want both", e, a) + } + if e, a := has(buildSystemDomainModel(atLeast(11, 7))); e || a { + t.Errorf("11.7: entity %v, association %v; want neither", e, a) + } +} diff --git a/mdl/backend/mutation.go b/mdl/backend/mutation.go index b4d3f5e80..37da3f9a4 100644 --- a/mdl/backend/mutation.go +++ b/mdl/backend/mutation.go @@ -392,6 +392,10 @@ type WidgetObjectBuilder interface { // and deferred to Finalize, which returns the aggregate result. SetAttribute(propertyKey string, attributePath string) + // SetAttributeWithSteps is SetAttribute for an attribute reached over + // associations: the AttributeRef's EntityRef holds one step per hop, as + // a rooted path `$Name/Assoc/Attr` stores it beside its SourceVariable. + SetAttributeWithSteps(propertyKey string, attributePath string, steps []pages.AttributeRefStep) SetAssociation(propertyKey string, assocPath string, entityName string) // SetSourceVariable names the object an attribute or association property // reads, when that is not the enclosing data context: a page or snippet diff --git a/mdl/backend/widgetobj/builder.go b/mdl/backend/widgetobj/builder.go index b21885a1e..24f60fd11 100644 --- a/mdl/backend/widgetobj/builder.go +++ b/mdl/backend/widgetobj/builder.go @@ -136,6 +136,15 @@ func (ob *Builder) SetAttribute(propertyKey string, attributePath string) { }) } +func (ob *Builder) SetAttributeWithSteps(propertyKey string, attributePath string, steps []pages.AttributeRefStep) { + if attributePath == "" { + return + } + ob.object = updateWidgetPropertyValue(ob.object, ob.propertyTypeIDs, propertyKey, func(val bson.D) bson.D { + return setAttributeRefField(val, attributePath, steps) + }) +} + func (ob *Builder) SetAssociation(propertyKey string, assocPath string, entityName string) { if assocPath == "" { return diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index 7c838f82f..331ea041c 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -2573,13 +2573,8 @@ func (pb *pageBuilder) resolveRootedAssociationPath(attrRef string) (wv pages.Wi if !ok { return pages.WidgetVariable{}, "", nil, false } - var start string - if v, entity, isParam := pb.parameterVariable(root); isParam { - wv, start = v, entity - } else if v, isDV := pb.dataViewVariables[root]; isDV { - wv, start = v, pb.paramEntityNames[root] - } - if start == "" { + wv, start, named := pb.namedObjectVariable(root) + if !named { return pages.WidgetVariable{}, "", nil, false } finalQN, steps, ok = pb.resolveAssociationPathFrom(path, start) @@ -3017,6 +3012,27 @@ func (pb *pageBuilder) parameterVariable(name string) (pages.WidgetVariable, str return pages.WidgetVariable{Variable: name, Kind: pb.parameterSlotKind(name)}, entity, true } +// namedObjectVariable is parameterVariable widened to an enclosing data view: +// the `$name` of a `$name.Attr` binding read from a page or snippet parameter, +// or else from a data view the widget sits inside, whose SourceVariable is the +// data-view pair {Widget: name, PageParameter: …} (ako/mxcli#826, #2107). It +// returns the SourceVariable to store and the object's entity. +func (pb *pageBuilder) namedObjectVariable(name string) (pages.WidgetVariable, string, bool) { + if wv, entity, ok := pb.parameterVariable(name); ok { + return wv, entity, true + } + name = strings.TrimPrefix(name, "$") + wv, known := pb.dataViewVariables[name] + if !known || pb.isDeclaredParameter(name) { + return pages.WidgetVariable{}, "", false + } + entity := pb.paramEntityNames[name] + if entity == "" { + return pages.WidgetVariable{}, "", false + } + return wv, entity, true +} + // documentKind names what is being built, for messages. func (pb *pageBuilder) documentKind() string { if pb.isSnippet { diff --git a/mdl/executor/cmd_pages_builder_visible_when.go b/mdl/executor/cmd_pages_builder_visible_when.go index fbeabf3cb..0be6c22ed 100644 --- a/mdl/executor/cmd_pages_builder_visible_when.go +++ b/mdl/executor/cmd_pages_builder_visible_when.go @@ -70,11 +70,16 @@ func (pb *pageBuilder) visibleWhenSettings(w *ast.WidgetV3, vw *ast.VisibleWhenV // the snippet with no data container around it. entity, attrName := pb.entityContext, vw.Attribute var source *pages.WidgetVariable + // + // `$dataView1.Attr` reads an enclosing data view's object: the data-view pair + // {Widget, PageParameter}, as on FactoryManagement's + // ConversationalUI.Trace_Details (ako/mxcli#2107). if name, rest, ok := namedObjectBinding(vw.Attribute); ok { - wv, paramEntity, isParam := pb.parameterVariable(name) + wv, paramEntity, isParam := pb.namedObjectVariable(name) if !isParam || strings.Contains(rest, ".") { - return nil, mdlerrors.NewValidationf("%s: `$%s` is not a parameter of this %s — `Visible: $Param.Attr in (…)` "+ - "reads an attribute of a page or snippet parameter; inside a data container name the attribute bare", + return nil, mdlerrors.NewValidationf("%s: `$%s` is not a parameter of this %s or an enclosing data view — "+ + "`Visible: $Param.Attr in (…)` reads an attribute of a page or snippet parameter, `$dataView1.Attr` one of "+ + "a data view the widget is inside; for the nearest data container's object name the attribute bare", where, name, pb.documentKind()) } entity, attrName, source = paramEntity, rest, &wv diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index bb94b4bb7..947b7b464 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -39,8 +39,10 @@ func extractConditionalSettings(ctx *ExecContext, widget *rawWidget, w map[strin if attr, ok := cvs["Attribute"].(string); ok && attr != "" { widget.VisibleAttr = describeAttr(ctx, attr) // Read from a page or snippet parameter (TestApp - // Snip_UserTask_NameColumnWithIcon): `$Param.Attr in (…)`. - if param := parameterSourceName(cvs["SourceVariable"]); param != "" { + // Snip_UserTask_NameColumnWithIcon): `$Param.Attr in (…)`, or from + // an enclosing data view (FactoryManagement + // ConversationalUI.Trace_Details): `$dataView6.Attr in (…)`. + if param := namedObjectSourceName(cvs["SourceVariable"]); param != "" { widget.VisibleAttr = parameterAttributeMDL(param, attr) } for _, c := range getBsonArrayElements(cvs["Conditions"]) { diff --git a/mdl/executor/cmd_pages_describe_pluggable.go b/mdl/executor/cmd_pages_describe_pluggable.go index bedac40b1..96f27b936 100644 --- a/mdl/executor/cmd_pages_describe_pluggable.go +++ b/mdl/executor/cmd_pages_describe_pluggable.go @@ -1345,10 +1345,20 @@ func extractExplicitProperties(ctx *ExecContext, w map[string]any, enclosingEnti if attrRef, ok := value["AttributeRef"].(map[string]any); ok && attrRef != nil { if attr := extractString(attrRef["Attribute"]); attr != "" { described := describeAttr(ctx, attr) - if param := parameterSourceName(value["SourceVariable"]); param != "" && attrRef["EntityRef"] == nil { - // Read from a page or snippet parameter: `$Param.Attr`. The - // qualified name re-executes without the SourceVariable — - // CE7247 (mendixlabs/mxcli#1591). + if root := namedObjectSourceName(value["SourceVariable"]); root != "" && attrRef["EntityRef"] != nil { + // The same object, with association steps: `$Root/Assoc/Attr` + // (ako/mxcli#833), as FactoryManagement's + // ConversationalUI.Span_TextGeneration_SlideIn stores it + // (ako/mxcli#2107). + if path := columnAttributeFromRef(ctx, attrRef); strings.Contains(path, "/") { + described = "$" + root + "/" + path + } + } else if param := namedObjectSourceName(value["SourceVariable"]); param != "" && attrRef["EntityRef"] == nil { + // Read from a page or snippet parameter, `$Param.Attr`, or an + // enclosing data view, `$dataView1.Attr`. The qualified name + // re-executes without the SourceVariable and binds against + // the nearest container — CE7247 (mendixlabs/mxcli#1591, + // ako/mxcli#2107). described = parameterAttributeMDL(param, attr) } else if schemas[typePointerID].DataSourceProperty == "" && strings.Contains(enclosing, ".") && !strings.EqualFold(attributeEntity(attr), enclosing) { diff --git a/mdl/executor/cmd_pages_input_binding_context.go b/mdl/executor/cmd_pages_input_binding_context.go index daf351ec1..016d4b532 100644 --- a/mdl/executor/cmd_pages_input_binding_context.go +++ b/mdl/executor/cmd_pages_input_binding_context.go @@ -306,8 +306,10 @@ func validatePageVariableBindings(widgets []*ast.WidgetV3, variables []ast.PageV // validateNamedObjectBindings is MDL-WIDGET34 for `$name.Attr` on a whole page // or snippet, where the parameters are known: a name that is neither one of -// them nor — on a built-in input — an enclosing data view names nothing, and -// exec refuses it. `Visible: $name.Attr in (…)` reads a parameter only. +// them nor — on a built-in input, or in `Visible: $name.Attr in (…)` — an +// enclosing data view names nothing, and exec refuses it. The visibility +// setting stores the data-view pair as an input does (FactoryManagement +// ConversationalUI.Trace_Details, ako/mxcli#2107). func validateNamedObjectBindings(widgets []*ast.WidgetV3, params []string, locationPrefix string) []linter.Violation { declared := make(map[string]bool, len(params)) for _, p := range params { @@ -382,9 +384,9 @@ func validateNamedObjectBindings(widgets []*ast.WidgetV3, params []string, locat } } if vw, ok := w.Properties["VisibleWhen"].(*ast.VisibleWhenV3); ok && vw != nil { - if name, rest, ok := namedObjectBinding(vw.Attribute); ok && (!declared[name] || strings.Contains(rest, ".")) { - report(fmt.Sprintf("%s `%s`: `Visible: %s in (…)` — `$%s` is not a parameter of this document, "+ - "so the condition would read nothing", kind, w.Name, vw.Attribute, name)) + if name, rest, ok := namedObjectBinding(vw.Attribute); ok && (!(declared[name] || dataViews[name]) || strings.Contains(rest, ".")) { + report(fmt.Sprintf("%s `%s`: `Visible: %s in (…)` — `$%s` is not a parameter of this document "+ + "or a data view the widget is inside, so the condition would read nothing", kind, w.Name, vw.Attribute, name)) } } inner := dataViews diff --git a/mdl/executor/cmd_pages_outer_dataview_binding_2107_test.go b/mdl/executor/cmd_pages_outer_dataview_binding_2107_test.go new file mode 100644 index 000000000..ad6793caf --- /dev/null +++ b/mdl/executor/cmd_pages_outer_dataview_binding_2107_test.go @@ -0,0 +1,307 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#2107: a widget bound to an attribute of an OUTER data view's object. +// Studio Pro stores the binding with the data-view pair as its source, +// SourceVariable {Widget: , PageParameter: } +// (FactoryManagement, Mendix 11.16): +// +// ConversationalUI.Span_ToolSpan_SlideIn markdownViewer1.stringAttribute +// AttributeRef GenAICommons.Span.Output, SourceVariable {Widget: dataView1, PageParameter: ToolSpan} +// ConversationalUI.Trace_Details container112 Visible +// Attribute GenAICommons.Trace.HasError, SourceVariable {Widget: dataView6, PageParameter: Trace} +// +// describe printed the bare qualified name, which the rewrite resolved against the +// NEAREST data view: "`GenAICommons.Span.Output` is an attribute of +// GenAICommons.Span, not of ConversationalUI.CopyHelper … CE7247", and +// "ConversationalUI.TraceView has no attribute HasError". The pair already has +// an MDL spelling on input widgets, `$dataView1.Attr` (ako/mxcli#826). +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" + "github.com/mendixlabs/mxcli/sdk/pages" + "go.mongodb.org/mongo-driver/v2/bson" +) + +func outerDataViewPair(widget, param string) map[string]any { + return map[string]any{"$Type": "Forms$PageVariable", "Widget": widget, "PageParameter": param} +} + +func markdownInNestedDataView(sourceVariable any) map[string]any { + md := map[string]any{ + "$Type": "CustomWidgets$CustomWidget", + "Name": "markdownViewer1", + "Type": map[string]any{ + "WidgetId": "com.mendix.widget.web.markdown.Markdown", + "ObjectType": map[string]any{"PropertyTypes": []any{ + map[string]any{"$ID": "pt-str", "PropertyKey": "stringAttribute", + "ValueType": map[string]any{"Type": "Attribute"}}, + }}, + }, + "Object": map[string]any{"Properties": []any{ + map[string]any{"TypePointer": "pt-str", "Value": map[string]any{ + "AttributeRef": map[string]any{"Attribute": "GenAICommons.Span.Output"}, + "SourceVariable": sourceVariable, + }}, + }}, + } + inner := map[string]any{ + "$Type": "Forms$DataView", "Name": "dataView3", + "DataSource": map[string]any{"$Type": "Forms$DataViewSource", + "EntityRef": map[string]any{"$Type": "DomainModels$DirectEntityRef", "Entity": "ConversationalUI.CopyHelper"}}, + "Widgets": []any{md}, + } + return map[string]any{ + "$Type": "Forms$DataView", "Name": "dataView1", + "DataSource": map[string]any{"$Type": "Forms$DataViewSource", + "EntityRef": map[string]any{"$Type": "DomainModels$DirectEntityRef", "Entity": "GenAICommons.ToolSpan"}, + "SourceVariable": map[string]any{"$Type": "Forms$PageVariable", "PageParameter": "ToolSpan"}}, + "Widgets": []any{inner}, + } +} + +func describedMarkdownAttribute(t *testing.T, sourceVariable any) string { + t.Helper() + ctx, _ := newMockCtx(t) + outer := parseRawWidget(ctx, markdownInNestedDataView(sourceVariable)) + if len(outer) != 1 || len(outer[0].Children) != 1 || len(outer[0].Children[0].Children) != 1 { + t.Fatalf("want dataView1 > dataView3 > markdown, got %+v", outer) + } + for _, p := range outer[0].Children[0].Children[0].ExplicitProperties { + if p.Key == "stringAttribute" { + return p.Value + } + } + t.Fatal("no stringAttribute described") + return "" +} + +func TestDescribePluggableAttributeThroughOuterDataView(t *testing.T) { + if got := describedMarkdownAttribute(t, outerDataViewPair("dataView1", "ToolSpan")); got != "$dataView1.Output" { + t.Errorf("stringAttribute = %q, want $dataView1.Output (the stored data-view pair)", got) + } + // Control: with no SourceVariable the qualified name stays, as before. + if got := describedMarkdownAttribute(t, nil); got != "GenAICommons.Span.Output" { + t.Errorf("stringAttribute without a source = %q, want GenAICommons.Span.Output", got) + } +} + +func describedVisibleAttr(t *testing.T, sourceVariable any) string { + t.Helper() + ctx, _ := newMockCtx(t) + var w rawWidget + extractConditionalSettings(ctx, &w, map[string]any{"ConditionalVisibilitySettings": map[string]any{ + "Attribute": "GenAICommons.Trace.HasError", + "SourceVariable": sourceVariable, + "Conditions": []any{int32(2), + map[string]any{"AttributeValue": "true", "EditableVisible": true}, + map[string]any{"AttributeValue": "false", "EditableVisible": false}}, + }}) + return w.VisibleAttr +} + +func TestDescribeVisibleAttributeThroughOuterDataView(t *testing.T) { + if got := describedVisibleAttr(t, outerDataViewPair("dataView6", "Trace")); got != "$dataView6.HasError" { + t.Errorf("Visible attribute = %q, want $dataView6.HasError", got) + } + // Control: a parameter source stays `$Param.Attr`. + if got := describedVisibleAttr(t, map[string]any{"$Type": "Forms$PageVariable", "PageParameter": "Trace"}); got != "$Trace.HasError" { + t.Errorf("Visible attribute from a parameter = %q, want $Trace.HasError", got) + } +} + +// The builder half: `$dataView1.Attr` on a Visible-by-attribute setting and on +// a pluggable widget's attribute key writes the data-view pair, as it already +// did on an input widget. + +func TestBuildVisibleWhen_ThroughOuterDataView(t *testing.T) { + pb := scopedPB(false) + pb.paramEntityNames["dataView1"] = "M.Job" + defer pb.registerDataViewVariable("dataView1", &pages.DataViewSource{ParameterName: "Task"})() + cvs, err := buildVisibleWhen(t, pb, "$dataView1.Status", "Running") + if err != nil { + t.Fatalf("build: %v", err) + } + if cvs == nil || cvs.Attribute != "M.Job.Status" { + t.Fatalf("settings = %+v, want Attribute M.Job.Status", cvs) + } + if want := (pages.WidgetVariable{Widget: "dataView1", Variable: "Task"}); cvs.SourceVariable == nil || *cvs.SourceVariable != want { + t.Errorf("SourceVariable = %+v, want %+v", cvs.SourceVariable, want) + } + // Control: a name that is neither a parameter nor an enclosing data view. + if _, err := buildVisibleWhen(t, scopedPB(false), "$dataView9.Status", "Running"); err == nil { + t.Error("Visible: $dataView9.Status built; want a refusal") + } +} + +func TestBuildPluggableAttribute_ThroughOuterDataView(t *testing.T) { + e := scopeEngine(t) + e.pageBuilder.paramEntityNames["dvOuter"] = "Sales.Customer" + defer e.pageBuilder.registerDataViewVariable("dvOuter", &pages.DataViewSource{ParameterName: "Ctx"})() + w := twoSourceWidget(map[string]any{"staticAttribute": &ast.WidgetExpressionV3{Text: "$dvOuter.Name"}}) + widget, err := e.Build(multiSourceDef(), w) + if err != nil { + t.Fatalf("Build: %v", err) + } + val := widgetValueFor(widget.RawObject, "Sales.Customer.Name") + if val == nil { + t.Fatal("no WidgetValue binds Sales.Customer.Name — the binding was dropped") + } + sv, ok := dGet(val, "SourceVariable").(bson.D) + if !ok { + t.Fatalf("SourceVariable = %#v, want the data-view pair", dGet(val, "SourceVariable")) + } + for k, want := range map[string]any{"Widget": "dvOuter", "PageParameter": "Ctx"} { + if got := dGet(sv, k); got != want { + t.Errorf("SourceVariable.%s = %#v, want %#v", k, got, want) + } + } + // Control: a name that is no data view and no parameter is still refused. + w = twoSourceWidget(map[string]any{"staticAttribute": &ast.WidgetExpressionV3{Text: "$dvNope.Name"}}) + if _, err := scopeEngine(t).Build(multiSourceDef(), w); err == nil { + t.Error("staticAttribute: $dvNope.Name built; want a refusal") + } +} + +// The check half: MDL-WIDGET34 accepted `Visible: $name.Attr in (…)` for a +// parameter only, so describe's `$dataView6.HasError` would be refused. +func TestCheck_VisibleAttributeThroughOuterDataView(t *testing.T) { + widget34 := func(src string) int { + n := 0 + for _, v := range ValidateProgram(parseMDL(t, src), "") { + if v.RuleID == "MDL-WIDGET34" { + n++ + } + } + return n + } + page := func(inner string) string { + return `mdl 1; +create page M.Trace ( Title: 'T', Layout: Atlas_Core.PopupLayout, Params: ( $Trace: M.Job ) ) { + dataview dvOuter ( DataSource: $Trace ) { + dataview dvInner ( DataSource: microflow M.DS_View() ) { + container c1 ( Visible: $dvOuter.Status in (Running) ) + } + } + ` + inner + ` +};` + } + if n := widget34(page("")); n != 0 { + t.Errorf("`$dvOuter.Status` inside dvOuter: %d MDL-WIDGET34, want 0", n) + } + // Control: the same name outside dvOuter reads nothing. + if n := widget34(page("container c2 ( Visible: $dvOuter.Status in (Running) )")); n != 1 { + t.Errorf("`$dvOuter.Status` outside dvOuter: %d MDL-WIDGET34, want 1", n) + } +} + +// The pair with association steps: ConversationalUI.Span_TextGeneration_SlideIn's +// markdownViewer3 reads GenAICommons.Trace.SystemPrompt over Span_Trace from +// dataView1's object, SourceVariable {Widget: dataView1, PageParameter: +// ModelSpan}. The rooted spelling is #833's `$Root/Assoc/Attr`. +func TestDescribePluggableAttributePathThroughOuterDataView(t *testing.T) { + ctx, _ := newMockCtx(t) + w := markdownInNestedDataView(outerDataViewPair("dataView1", "ModelSpan")) + md := w["Widgets"].([]any)[0].(map[string]any)["Widgets"].([]any)[0].(map[string]any) + prop := md["Object"].(map[string]any)["Properties"].([]any)[0].(map[string]any) + prop["Value"].(map[string]any)["AttributeRef"] = map[string]any{ + "Attribute": "GenAICommons.Trace.SystemPrompt", + "EntityRef": map[string]any{"$Type": "DomainModels$IndirectEntityRef", "Steps": []any{int32(2), + map[string]any{"$Type": "DomainModels$EntityRefStep", "Association": "GenAICommons.Span_Trace", "DestinationEntity": "GenAICommons.Trace"}}}, + } + outer := parseRawWidget(ctx, w) + var got string + for _, p := range outer[0].Children[0].Children[0].ExplicitProperties { + if p.Key == "stringAttribute" { + got = p.Value + } + } + if !strings.HasPrefix(got, "$dataView1/") || !strings.HasSuffix(got, "Span_Trace/SystemPrompt") { + t.Errorf("stringAttribute = %q, want $dataView1/…Span_Trace/SystemPrompt", got) + } +} + +func associationEngine(t *testing.T) *PluggableWidgetEngine { + t.Helper() + real := &modelsdkbackend.Backend{} + mod := &model.Module{BaseElement: model.BaseElement{ID: model.ID("mod-sales")}, Name: "Sales"} + b := &mock.MockBackend{ + LoadWidgetTemplateFunc: real.LoadWidgetTemplate, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { + return []*domainmodel.DomainModel{{ + BaseElement: model.BaseElement{ID: model.ID("dm-sales")}, + ContainerID: mod.ID, + Entities: []*domainmodel.Entity{ + {BaseElement: model.BaseElement{ID: model.ID("e-invoice")}, Name: "Invoice", + Attributes: []*domainmodel.Attribute{{Name: "Reference"}, {Name: "Label"}}}, + {BaseElement: model.BaseElement{ID: model.ID("e-order")}, Name: "Order", + Attributes: []*domainmodel.Attribute{{Name: "Number"}, {Name: "Label"}}}, + {BaseElement: model.BaseElement{ID: model.ID("e-customer")}, Name: "Customer", + Attributes: []*domainmodel.Attribute{{Name: "Name"}, {Name: "Label"}}}, + }, + Associations: []*domainmodel.Association{{Name: "Order_Customer", ParentID: "e-order", ChildID: "e-customer", + Type: domainmodel.AssociationTypeReference}}, + }}, nil + }, + } + pb := &pageBuilder{backend: b, entityContext: "Sales.Invoice", paramEntityNames: map[string]string{}, widgetScope: map[string]model.ID{}} + e := NewPluggableWidgetEngine(b, pb) + pb.pluggableEngine = e + return e +} + +func TestBuildPluggableAttributePath_ThroughOuterDataView(t *testing.T) { + e := associationEngine(t) + e.pageBuilder.paramEntityNames["dvOuter"] = "Sales.Order" + defer e.pageBuilder.registerDataViewVariable("dvOuter", &pages.DataViewSource{ParameterName: "Ctx"})() + w := twoSourceWidget(map[string]any{"staticAttribute": &ast.WidgetExpressionV3{Text: "$dvOuter/Order_Customer/Name"}}) + widget, err := e.Build(multiSourceDef(), w) + if err != nil { + t.Fatalf("Build: %v", err) + } + val := widgetValueFor(widget.RawObject, "Sales.Customer.Name") + if val == nil { + t.Fatal("no WidgetValue binds Sales.Customer.Name — the binding was dropped") + } + ref, _ := dGet(val, "AttributeRef").(bson.D) + entityRef, _ := dGet(ref, "EntityRef").(bson.D) + steps, _ := dGet(entityRef, "Steps").(bson.A) + var step bson.D + if len(steps) == 2 { + step, _ = steps[1].(bson.D) + } + if dGet(step, "Association") != "Sales.Order_Customer" { + t.Errorf("EntityRef steps = %#v, want one hop over Sales.Order_Customer", dGet(ref, "EntityRef")) + } + sv, _ := dGet(val, "SourceVariable").(bson.D) + if dGet(sv, "Widget") != "dvOuter" || dGet(sv, "PageParameter") != "Ctx" { + t.Errorf("SourceVariable = %#v, want the data-view pair", dGet(val, "SourceVariable")) + } +} + +// As the parser hands it over: `stringAttribute: $dataView1/Span_Trace/SystemPrompt` +// parses as an association data source rooted at the data view, which the +// engine refused as "an attribute property … cannot hold the expression" +// (MDL-WIDGET42) on Span_TextGeneration_SlideIn. +func TestBuildPluggableAttributePath_ParsedForm(t *testing.T) { + e := associationEngine(t) + e.pageBuilder.paramEntityNames["dvOuter"] = "Sales.Order" + defer e.pageBuilder.registerDataViewVariable("dvOuter", &pages.DataViewSource{ParameterName: "Ctx"})() + w := twoSourceWidget(map[string]any{"staticAttribute": &ast.DataSourceV3{ + Type: "association", Reference: "Order_Customer/Name", ContextVariable: "dvOuter"}}) + widget, err := e.Build(multiSourceDef(), w) + if err != nil { + t.Fatalf("Build: %v", err) + } + if widgetValueFor(widget.RawObject, "Sales.Customer.Name") == nil { + t.Fatal("no WidgetValue binds Sales.Customer.Name") + } +} diff --git a/mdl/executor/cmd_pages_parameter_binding.go b/mdl/executor/cmd_pages_parameter_binding.go index dbf9de5ea..679b15734 100644 --- a/mdl/executor/cmd_pages_parameter_binding.go +++ b/mdl/executor/cmd_pages_parameter_binding.go @@ -48,7 +48,7 @@ func namedObjectBinding(s string) (name, rest string, ok bool) { // one shape is still an expression. func parameterAttributeValue(raw any) (string, bool) { e, ok := raw.(*ast.WidgetExpressionV3) - if !ok || !isParameterAttributeText(e.Text) { + if !ok || !isNamedObjectAttributeText(e.Text) { return "", false } return e.Text, true @@ -89,6 +89,21 @@ func parameterSourceName(sv any) string { return extractString(m["PageParameter"]) } +// namedObjectSourceName is the `$name` of the object a stored SourceVariable +// names: the data view of a data-view pair {Widget, PageParameter}, or else a +// page or snippet parameter — "" for none, or for a page variable. MDL spells +// the pair `$dataView1.Attr` (ako/mxcli#826, #2107). +func namedObjectSourceName(sv any) string { + m, ok := sv.(map[string]any) + if !ok || m == nil || extractString(m["LocalVariable"]) != "" { + return "" + } + if widget := extractString(m["Widget"]); widget != "" { + return widget + } + return parameterSourceName(sv) +} + // customWidgetPropertyParameter is the parameter a CustomWidget property's // WidgetValue reads (its SourceVariable), or "". func customWidgetPropertyParameter(w map[string]any, propertyKey string) string { @@ -114,6 +129,19 @@ func parameterAttributeMDL(name, attrQN string) string { return "$" + name + "." + mdlIdent(shortAttributeName(attrQN)) } +// isNamedObjectAttributeText reports whether text binds an attribute of a named +// object: `$Name.Attr`, or the rooted association path `$Name/Assoc/Attr` +// (ako/mxcli#833), which a pluggable attribute property takes as well +// (FactoryManagement ConversationalUI.Span_TextGeneration_SlideIn, +// ako/mxcli#2107). Neither is a Mendix expression to an attribute property. +func isNamedObjectAttributeText(text string) bool { + if isParameterAttributeText(text) { + return true + } + _, _, rooted := rootedAssociationPath(text) + return rooted +} + // isParameterAttributeText reports whether text is the `$Param.Attr` shape. // namedObjectBinding alone takes `$Ctx.Name + 'x'` too, which is an expression. func isParameterAttributeText(text string) bool { diff --git a/mdl/executor/cmd_pages_showpage_args.go b/mdl/executor/cmd_pages_showpage_args.go index 9ada8c67b..67d7445a8 100644 --- a/mdl/executor/cmd_pages_showpage_args.go +++ b/mdl/executor/cmd_pages_showpage_args.go @@ -62,6 +62,12 @@ type pageArgContext struct { // selection `$` passes to a page as an explicit mapping naming the // grid (mendixlabs/mxcli#122). The row object is not the control bar's. selectionOf string + // rowSelectionOf is set inside a selectable data grid's rows: the grid's + // name, whose selection `$` a show page in a column passes as the + // same explicit mapping. Studio Pro stores exactly that (Variable {Widget: + // grid1}) on FactoryManagement's MxModelReflection.MxObjectType_Select, + // and it builds (ako/mxcli#2109). The row object stays $currentObject. + rowSelectionOf string } // inDataWidget names the data widget whose context c is. @@ -77,12 +83,42 @@ func (c pageArgContext) namesSelection(value string) bool { return ok && c.selectionOf != "" && strings.EqualFold(name, c.selectionOf) } +// namesRowSelection reports whether value is `$` written in that grid's +// rows, where the grid has a selection: a show page argument only. +func (c pageArgContext) namesRowSelection(value string) bool { + name, ok := strings.CutPrefix(value, "$") + return ok && c.rowSelectionOf != "" && strings.EqualFold(name, c.rowSelectionOf) +} + +// selectableDataGrid reports whether w is a data grid with a selection, whose +// name is then a variable in its rows as well as in its control bar. +func selectableDataGrid(w *ast.WidgetV3) bool { + switch strings.ToLower(w.Type) { + case "datagrid", "datagrid2": + default: + return false + } + sel := w.GetSelection() + return sel != "" && !strings.EqualFold(sel, "None") +} + // enteringDataWidget is the context below a data-bound widget bound to ds and // yielding entity. func enteringDataWidget(ds *ast.DataSourceV3, entity string) pageArgContext { return pageArgContext{known: true, present: true, varName: contextVarFor(ds), entity: entity} } +// enteringDataWidgetOf is enteringDataWidget for the rows of data widget w, by +// name: a selectable data grid's name is its selection there too. The one +// constructor the check walk and both builders use, so the three agree. +func enteringDataWidgetOf(w *ast.WidgetV3, ds *ast.DataSourceV3, entity string) pageArgContext { + c := enteringDataWidget(ds, entity).inDataWidget(w.Name) + if selectableDataGrid(w) { + c.rowSelectionOf = w.Name + } + return c +} + // atDocumentRoot is the context at the top of a page, snippet or layout the pass // walks in full: knowable, and empty. func atDocumentRoot() pageArgContext { @@ -126,7 +162,7 @@ func argContextForChildren(w *ast.WidgetV3, parent pageArgContext) pageArgContex if ds := w.GetDataSource(); ds != nil { // The entity is only used to word the refusal; the executor's builder // overwrites this with one that carries it. - c := enteringDataWidget(ds, "").inDataWidget(w.Name) + c := enteringDataWidgetOf(w, ds, "") c.docParams = parent.docParams return c } @@ -303,7 +339,7 @@ func refuseShowPageArgument(widgetName, target, argName, argValue string, c page func (pb *pageBuilder) namedParameterArgument(value string) (variable, kind string) { variable, kind = pb.classifyFlowArgValue(value) if kind != "parameter" && kind != "snippet" { - if pb.argCtx.namesSelection(value) { + if pb.argCtx.namesSelection(value) || pb.argCtx.namesRowSelection(value) { variable, kind = value, "widget" } else { return "", "" @@ -361,7 +397,7 @@ func validateShowPageArguments(w *ast.WidgetV3, c pageArgContext, locationPrefix var out []linter.Violation for _, arg := range action.Args { strVal, ok := arg.Value.(string) - if !ok || c.binds(strVal) || c.namesDocumentParameter(strVal) || c.namesSelection(strVal) { + if !ok || c.binds(strVal) || c.namesDocumentParameter(strVal) || c.namesSelection(strVal) || c.namesRowSelection(strVal) { continue } out = append(out, linter.Violation{ diff --git a/mdl/executor/cmd_pages_showpage_selection_arg_122_test.go b/mdl/executor/cmd_pages_showpage_selection_arg_122_test.go index e46c9b95e..597c03662 100644 --- a/mdl/executor/cmd_pages_showpage_selection_arg_122_test.go +++ b/mdl/executor/cmd_pages_showpage_selection_arg_122_test.go @@ -75,8 +75,12 @@ func TestValidateShowPageArguments_ControlBarPassesGridSelection(t *testing.T) { } } -// The selection is the control bar's, not the row's: a column button passing -// its own grid's name is not the measured shape and stays refused. +// A column button passing its own selectable grid's name passes the grid's +// selection too: Studio Pro stores Variable {Widget: grid1} there +// (FactoryManagement MxModelReflection.MxObjectType_Select, which builds), and +// so does the builder now, at 0 errors on 11.16 (ako/mxcli#2109). This used to +// be refused as an unmeasured shape. A grid without a selection still has no +// `$grid` to pass (TestCheck_GridSelectionFromColumnContent). func TestValidateShowPageArguments_GridSelectionOnlyInItsControlBar(t *testing.T) { src := `create page Mod.Overview (title: 'O', layout: Atlas_Core.Atlas_Default) { datagrid dgOrders (datasource: database from Mod.Order, selection: single) { @@ -90,8 +94,8 @@ func TestValidateShowPageArguments_GridSelectionOnlyInItsControlBar(t *testing.T t.Fatalf("parse: %v", errs) } s := prog.Statements[0].(*ast.CreatePageStmtV3) - if got := pageArgViolations(t, s.Widgets); len(got) != 1 { - t.Errorf("row-scoped button passing $dgOrders: %d MDL-PAGEARG01, want 1", len(got)) + if got := pageArgViolations(t, s.Widgets); len(got) != 0 { + t.Errorf("row-scoped button passing its selectable grid's selection $dgOrders: %d MDL-PAGEARG01, want 0", len(got)) } } diff --git a/mdl/executor/validate_own_container_slots_2109_test.go b/mdl/executor/validate_own_container_slots_2109_test.go new file mode 100644 index 000000000..aa1945456 --- /dev/null +++ b/mdl/executor/validate_own_container_slots_2109_test.go @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#2109: check refused its own describe output on four FactoryManagement +// pages (Mendix 11.16), which Studio Pro authored and which build clean. The +// stored shapes, measured: +// +// - MxModelReflection.MxObjectType_Select, OIDC.OIDC_Client_NewEdit, +// UserCommons.ClaimsForClaimEntityAttribute: a button in a Data grid 2 +// column's content, in a grid with `Selection: Single`, whose show page +// mapping stores Variable {Widget: grid1} — the grid's SELECTION, the shape +// a control bar button stores for `$grid` (mendixlabs/mxcli#122). +// - AgentCommons.Version_NewEdit: container5 in a combo box's +// optionsSourceAssociationCustomContent, inside dataView2, with +// `Visible: $currentObject = $dataView2`. The option content is the combo +// box's own item context, so $dataView2 is an OUTER container there. +// +// check said: "`actionButton1` reads `$grid1` …, but `grid1` is the data +// container the widget sits in directly — its name is a variable only inside +// a data container nested below it" (MDL-BUTTON02), and refused the argument +// as an explicit mapping (MDL-PAGEARG01, "an explicit one is rejected as CE0115"). +package executor + +import ( + "strings" + "testing" +) + +func ownContainerRules(t *testing.T, src string) []string { + t.Helper() + var out []string + for _, v := range ValidateProgram(parseMDL(t, src), "") { + if v.RuleID == "MDL-BUTTON02" || v.RuleID == "MDL-PAGEARG01" { + out = append(out, v.RuleID+": "+v.Message) + } + } + return out +} + +func gridColumnButtonPage(selection string) string { + return `mdl 1; +create page M.Select ( Title: 'Select', Layout: Atlas_Core.PopupLayout ) { + datagrid grid1 ( DataSource: database from M.Item` + selection + ` ) { + column ( Attribute: Name, ShowContentAs: customContent ) { + actionbutton actionButton1 ( Caption: 'View', Action: show page M.Item_Details(Item = $grid1) ) + } + } +};` +} + +func TestCheck_GridSelectionFromColumnContent(t *testing.T) { + if got := ownContainerRules(t, gridColumnButtonPage(", Selection: Single")); len(got) != 0 { + t.Errorf("a selectable grid's selection refused from its column content:\n%s", strings.Join(got, "\n")) + } + // Control: a grid with no selection has no `$grid1` to pass. + if got := ownContainerRules(t, gridColumnButtonPage("")); len(got) == 0 { + t.Error("$grid1 accepted in a grid with no selection") + } +} + +func comboBoxOptionContentPage(visible string) string { + return `mdl 1; +create page M.Edit ( Title: 'Edit', Layout: Atlas_Core.PopupLayout, Params: ( $Agent: M.Agent ) ) { + dataview dataView2 ( DataSource: $Agent ) { + combobox comboBox3 ( Attribute: Agent_Version_InUse, DataSource: database from M.Version, optionsSourceAssociationCustomContentType: 'yes' ) { + optionssourceassociationcustomcontent { + container container5 ( Visible: ` + visible + ` ) + } + } + container container9 ( Visible: $currentObject = $dataView2 ) + } +};` +} + +func TestCheck_ComboBoxOptionContentIsItsOwnContext(t *testing.T) { + got := ownContainerRules(t, comboBoxOptionContentPage("$currentObject = $dataView2")) + // container9 sits directly in dataView2 and is still refused (the control); + // container5, in the combo box's option content, is not. + if len(got) != 1 || !strings.Contains(got[0], "container9") { + t.Errorf("want only container9 refused, got:\n%s", strings.Join(got, "\n")) + } +} diff --git a/mdl/executor/validate_page_button_context.go b/mdl/executor/validate_page_button_context.go index 5a6b6cdd6..b8915bf53 100644 --- a/mdl/executor/validate_page_button_context.go +++ b/mdl/executor/validate_page_button_context.go @@ -25,7 +25,7 @@ func ValidatePageButtonContext(prog *ast.Program) []linter.Violation { var out []linter.Violation for _, stmt := range prog.Statements { if label, widgets, ok := documentWidgets(stmt); ok { - out = append(out, checkButtonContextTree(widgets, "", false, "", label)...) + out = append(out, checkButtonContextTree(widgets, "", false, "", false, label)...) } } return out @@ -48,9 +48,14 @@ func ValidatePageButtonContext(prog *ast.Program) []linter.Violation { // control bar inherits the context from above the grid, not from the grid. // // nearest is the name of the data container whose object the widget sits in -// ("" at the top), for MDL-BUTTON02 (mendixlabs/mxcli#1324). It moves with the -// same rule as inContext: a control bar keeps the context from above its grid. -func checkButtonContextTree(widgets []*ast.WidgetV3, controlBarOf string, inContext bool, nearest, locationPrefix string) []linter.Violation { +// ("" at the top), for MDL-BUTTON02 (mendixlabs/mxcli#1324). Inside a control +// bar it is "": the bar takes its object from above its grid but sits inside +// the grid, so no container's name is its own there (ako/mxcli#2109). +// +// nearestSelectable says the nearest container is a data grid with a +// selection: its name then is a variable in its rows too, the selection, which +// a show page argument may pass (ako/mxcli#2109). +func checkButtonContextTree(widgets []*ast.WidgetV3, controlBarOf string, inContext bool, nearest string, nearestSelectable bool, locationPrefix string) []linter.Violation { var out []linter.Violation for _, w := range widgets { if w == nil { @@ -60,19 +65,24 @@ func checkButtonContextTree(widgets []*ast.WidgetV3, controlBarOf string, inCont out = append(out, checkControlBarAction(a, w.Name, controlBarOf, locationPrefix)...) } if nearest != "" { - out = append(out, checkOwnContainerName(w, nearest, locationPrefix)...) + out = append(out, checkOwnContainerName(w, nearest, nearestSelectable, locationPrefix)...) } childContext := inContext || isObjectContextContainer(w) - childNearest := nearest + childNearest, childSelectable := nearest, nearestSelectable if isObjectContextContainer(w) && w.Name != "" { - childNearest = w.Name + childNearest, childSelectable = w.Name, selectableDataGrid(w) } for _, c := range w.Children { - childOf, ctx, near := controlBarOf, childContext, childNearest + childOf, ctx, near, sel := controlBarOf, childContext, childNearest, childSelectable if c != nil && strings.EqualFold(c.Type, "controlbar") { - childOf, ctx, near = w.Name, inContext, nearest + // The control bar's object comes from above the grid, but the + // control bar sits INSIDE the grid, so the container above is + // an outer one there: `$dataView3` from the filter bar of a + // grid in dataView3 is Studio Pro's own spelling and builds + // (FactoryManagement OIDC.OIDC_Client_NewEdit, ako/mxcli#2109). + childOf, ctx, near, sel = w.Name, inContext, "", false } - out = append(out, checkButtonContextTree([]*ast.WidgetV3{c}, childOf, ctx, near, locationPrefix)...) + out = append(out, checkButtonContextTree([]*ast.WidgetV3{c}, childOf, ctx, near, sel, locationPrefix)...) } } return out @@ -95,7 +105,12 @@ var ownNameExprProps = []struct{ key, written string }{ // arguments (and a chained THEN action's), a data source's flow arguments, // Visible, Editable and DynamicClasses are CE0117 "Error(s) in expression.", // and a data source's XPath `where` is CE0161 "Error(s) in XPath constraint.". -func checkOwnContainerName(w *ast.WidgetV3, nearest, locationPrefix string) []linter.Violation { +// +// A selectable data grid's name in its rows is the grid's selection, which a +// show page argument stores as Variable {Widget: }: Studio Pro writes it +// there and it builds (ako/mxcli#2109), so that one slot is not judged. Other +// slots are, as no stored example shows them reading the selection. +func checkOwnContainerName(w *ast.WidgetV3, nearest string, nearestSelectable bool, locationPrefix string) []linter.Violation { var out []linter.Violation flag := func(slot, expr, code string) { if !exprReadsVariable(expr, nearest) { @@ -113,6 +128,9 @@ func checkOwnContainerName(w *ast.WidgetV3, nearest, locationPrefix string) []li }) } for a := w.GetAction(); a != nil; a = a.ThenAction { + if nearestSelectable && a.Type == "showPage" { + continue + } for _, arg := range a.Args { if v, ok := arg.Value.(string); ok { flag(a.Type+" action argument", v, "CE0117") @@ -174,10 +192,12 @@ func isExprIdentByte(c byte) bool { // isObjectContextContainer reports whether w gives its (non-control-bar) // children a current object: a data view's object, a list view or gallery -// item, a grid row (column content). +// item, a grid row (column content), a combo box option (its custom content: +// AgentCommons.Version_NewEdit reads `$dataView2` there, the data view +// around the combo box, and builds; ako/mxcli#2109). func isObjectContextContainer(w *ast.WidgetV3) bool { switch strings.ToLower(w.Type) { - case "dataview", "listview", "gallery", "templategrid", "datagrid", "datagrid2": + case "dataview", "listview", "gallery", "templategrid", "datagrid", "datagrid2", "combobox": return true } return false diff --git a/mdl/executor/validate_page_own_container_name_test.go b/mdl/executor/validate_page_own_container_name_test.go index b92225201..fa2bd1c6b 100644 --- a/mdl/executor/validate_page_own_container_name_test.go +++ b/mdl/executor/validate_page_own_container_name_test.go @@ -56,6 +56,14 @@ func TestValidatePageButtonContext_OwnDataViewName_Issue1324(t *testing.T) { // data container decides — a plain container is transparent, a control bar // takes the context from ABOVE its grid (so a grid's own selection stays // spellable there), and a row or item of a nested list is a new context. +// +// btnCtlBar is no longer flagged (ako/mxcli#2109). Its CE0117 was an error in +// an EXPRESSION `$dvP`; since #2095 the argument is written as the variable +// pair {Widget: dvP, PageParameter: Gate}, which builds with 0 errors on +// 11.16 (probe page, with a CE1365 positive control in the same build), and +// is what Studio Pro stores for a control-bar button there +// (FactoryManagement OIDC.OIDC_Client_NewEdit actionButton7, which builds). +// A control bar sits inside its grid, so the container above is an outer one. func TestValidatePageButtonContext_OwnContainerName_MeasuredShapes(t *testing.T) { src := `create page G52.Probe (Title: 'Probe', Layout: Atlas_Core.PopupLayout, Params: ( $Gate: G52.Gate )) { dataview dvP (DataSource: $Gate) { @@ -92,7 +100,7 @@ func TestValidatePageButtonContext_OwnContainerName_MeasuredShapes(t *testing.T) } };` got := strings.Join(ownContainerFlagged(t, src), ",") - want := "btnCtlBar,btnDgOwnCol,btnGaOwn,btnInContainer,btnLvOwn,btnOwnPath" + want := "btnDgOwnCol,btnGaOwn,btnInContainer,btnLvOwn,btnOwnPath" if got != want { t.Fatalf("flagged %q, want %q (mx check 11.14.0's CE0117 set)", got, want) } diff --git a/mdl/executor/validate_widget_expression_value.go b/mdl/executor/validate_widget_expression_value.go index 4a08a385f..787d5b62b 100644 --- a/mdl/executor/validate_widget_expression_value.go +++ b/mdl/executor/validate_widget_expression_value.go @@ -58,6 +58,11 @@ func validateWidgetExpressionValuesIn(w *ast.WidgetV3, def *WidgetDefinition, it continue } text = src + // `$Name/Assoc/Attr` parses as a data source; on an attribute + // property it is a rooted attribute path (ako/mxcli#2107). + if root, path, rooted := rootedAttributeValue(v); rooted { + text, isExpr = "$"+root+"/"+path, true + } default: continue } @@ -77,7 +82,7 @@ func validateWidgetExpressionValuesIn(w *ast.WidgetV3, def *WidgetDefinition, it switch { case op == "expression": continue - case isExpr && isParameterAttributeText(text) && (op == "attribute" || !known): + case isExpr && isNamedObjectAttributeText(text) && (op == "attribute" || !known): // `$Param.Attr` reads an attribute of a page or snippet parameter // (mendixlabs/mxcli#1591); exec resolves the parameter. It is no // Mendix expression (that is `$Param/Attr`), so a key whose kind is diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index b867974c9..e5c361b9a 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -278,6 +278,10 @@ type BuildContext struct { // SourceVariable names the parameter an attribute or association binding // reads (`$Param.Attr`); nil for one read from the enclosing object. SourceVariable *pages.WidgetVariable + // AttributeSteps are the association hops of a rooted attribute path, + // `$Name/Assoc/Attr`, stored as the AttributeRef's EntityRef beside + // SourceVariable (ako/mxcli#2107). + AttributeSteps []pages.AttributeRefStep // ImageEntity is the entity an image-typed property binds to when it was // written `$currentObject` rather than as a collection entry (#1476). ImageEntity string @@ -527,7 +531,7 @@ func (e *PluggableWidgetEngine) Build(def *WidgetDefinition, w *ast.WidgetV3) (* e.recordDataSourceEntity(propKey, entityName) if entityName != "" { e.pageBuilder.entityContext = entityName - e.pageBuilder.argCtx = enteringDataWidget(ds, entityName).inDataWidget(w.Name) + e.pageBuilder.argCtx = enteringDataWidgetOf(w, ds, entityName) } } } @@ -747,6 +751,17 @@ func (e *PluggableWidgetEngine) Build(def *WidgetDefinition, w *ast.WidgetV3) (* // re-applies a property authored by its schema key AFTER every // datasource has run, so the shared context here is always the LAST // datasource's entity, which is what #647 wrote for every name. + if _, _, rooted := rootedAssociationPath(strVal); rooted { + // `$Name/Assoc/Attr`: an association path read from a named + // object (ako/mxcli#833, #2107). + wv, finalQN, steps, err := e.rootedAttributeBinding(w, propName, strVal) + if err != nil { + return nil, err + } + builder.SetAttributeWithSteps(propName, finalQN, steps) + builder.SetSourceVariable(propName, &wv) + break + } if name, rest, named := namedObjectBinding(strVal); named { // `$Param.Attr`: the attribute of a page or snippet parameter, // whatever encloses the widget — the AttributeRef plus a @@ -1059,7 +1074,11 @@ var templateAttrPlaceholderRe = regexp.MustCompile(`\{[A-Za-z][A-Za-z0-9_]*\}`) func (e *PluggableWidgetEngine) applyOperation(builder backend.WidgetObjectBuilder, opName string, propKey string, ctx *BuildContext) error { switch opName { case "attribute": - builder.SetAttribute(propKey, ctx.AttributePath) + if len(ctx.AttributeSteps) > 0 { + builder.SetAttributeWithSteps(propKey, ctx.AttributePath, ctx.AttributeSteps) + } else { + builder.SetAttribute(propKey, ctx.AttributePath) + } if ctx.SourceVariable != nil && ctx.AttributePath != "" { builder.SetSourceVariable(propKey, ctx.SourceVariable) } @@ -1497,13 +1516,15 @@ func refuseAmbiguousGenericDataSource(def *WidgetDefinition, w *ast.WidgetV3, ds } // parameterBinding resolves the `$name` of a `$name.Attr` binding on a pluggable -// widget property to the page or snippet parameter it names: the SourceVariable -// to store beside the reference, and the parameter's entity. A name that is no -// parameter is refused — the data-view pair (#826) is measured on the built-in -// inputs only, and anything else would be written as a binding that reads +// widget property to the object it names: a page or snippet parameter, or an +// enclosing data view — the data-view pair Studio Pro stores on a markdown +// viewer in an inner data view (FactoryManagement ConversationalUI +// Span_*_SlideIn, ako/mxcli#2107) as on the built-in inputs (#826). It returns +// the SourceVariable to store beside the reference and the object's entity. +// Any other name is refused: it would be written as a binding that reads // nothing. func (e *PluggableWidgetEngine) parameterBinding(w *ast.WidgetV3, propertyKey, attr, name string) (pages.WidgetVariable, string, error) { - wv, entity, ok := e.pageBuilder.parameterVariable(name) + wv, entity, ok := e.pageBuilder.namedObjectVariable(name) if !ok { return pages.WidgetVariable{}, "", notAParameterError( fmt.Sprintf("%s `%s` property `%s`", strings.ToLower(w.Type), w.Name, propertyKey), @@ -1512,12 +1533,30 @@ func (e *PluggableWidgetEngine) parameterBinding(w *ast.WidgetV3, propertyKey, a return wv, entity, nil } +// rootedAttributeBinding resolves `$Name/Assoc/Attr` on a pluggable widget's +// attribute property — an association path read from a parameter or an +// enclosing data view, the shape Studio Pro stores as the AttributeRef's +// steps beside a SourceVariable naming the object (FactoryManagement +// ConversationalUI.Span_TextGeneration_SlideIn, ako/mxcli#2107). +func (e *PluggableWidgetEngine) rootedAttributeBinding(w *ast.WidgetV3, propertyKey, attr string) (pages.WidgetVariable, string, []pages.AttributeRefStep, error) { + wv, finalQN, steps, ok := e.pageBuilder.resolveRootedAssociationPath(attr) + if !ok { + root, _, _ := rootedAssociationPath(attr) + return pages.WidgetVariable{}, "", nil, mdlerrors.NewValidationf( + "%s `%s` property `%s`: `%s` — `$%s/…` reads an association path from a parameter of this %s or an "+ + "enclosing data view, and `$%s` is neither, or the path does not resolve from it", + strings.ToLower(w.Type), w.Name, propertyKey, attr, root, e.pageBuilder.documentKind(), root) + } + return wv, finalQN, steps, nil +} + // notAParameterError refuses a `$name.Attr` binding whose name is no page or // snippet parameter. where names the widget property, for the message. func notAParameterError(where, attr, name, documentKind string) error { return mdlerrors.NewValidationf( - "%s: `%s` — `$%s` is not a parameter of this %s; `$Param.Attr` reads an attribute "+ - "of a page or snippet parameter. Inside a data container, name the attribute bare", + "%s: `%s` — `$%s` is not a parameter of this %s or an enclosing data view; `$Param.Attr` reads an "+ + "attribute of a page or snippet parameter, `$dataView1.Attr` one of a data view the widget is inside. "+ + "For the nearest data container's object, name the attribute bare", where, attr, name, documentKind) } @@ -1688,7 +1727,14 @@ func (e *PluggableWidgetEngine) resolveMapping(mapping PropertyMapping, w *ast.W } } } - if name, rest, named := namedObjectBinding(attr); named { + if _, _, rooted := rootedAssociationPath(attr); rooted { + // `$Name/Assoc/Attr` (ako/mxcli#833, #2107). + wv, finalQN, steps, err := e.rootedAttributeBinding(w, mapping.PropertyKey, attr) + if err != nil { + return nil, err + } + ctx.AttributePath, ctx.AttributeSteps, ctx.SourceVariable = finalQN, steps, &wv + } else if name, rest, named := namedObjectBinding(attr); named { // `$Param.Attr`: read from a page or snippet parameter, whatever // encloses the widget (see cmd_pages_parameter_binding.go). wv, entity, err := e.parameterBinding(w, mapping.PropertyKey, attr, name) @@ -1776,7 +1822,7 @@ func (e *PluggableWidgetEngine) resolveMapping(mapping PropertyMapping, w *ast.W e.recordDataSourceEntity(mapping.PropertyKey, entityName) if entityName != "" { e.pageBuilder.entityContext = entityName - e.pageBuilder.argCtx = enteringDataWidget(ds, entityName).inDataWidget(w.Name) + e.pageBuilder.argCtx = enteringDataWidgetOf(w, ds, entityName) if w.Name != "" { e.pageBuilder.paramEntityNames[w.Name] = entityName } diff --git a/mdl/executor/widget_expression_value.go b/mdl/executor/widget_expression_value.go index 3a547423f..efe752db0 100644 --- a/mdl/executor/widget_expression_value.go +++ b/mdl/executor/widget_expression_value.go @@ -69,6 +69,11 @@ func scalarPropertyText(w *ast.WidgetV3, key string, raw any, op string) (string } return "", expressionInScalarPropertyError(w, key, op, v.Text) case *ast.DataSourceV3: + // `$Name/Assoc/Attr` parses as a data source; on an attribute property + // it is a rooted attribute path, resolved by the engine (ako/mxcli#2107). + if root, path, rooted := rootedAttributeValue(v); rooted && op == "attribute" { + return "$" + root + "/" + path, nil + } src, hasSrc := w.ExpressionSource(key) if op == "expression" && hasSrc { return src, nil diff --git a/mdl/roundtrip/testapp_allowlist_test.go b/mdl/roundtrip/testapp_allowlist_test.go index 85e5fe7aa..c556a42c5 100644 --- a/mdl/roundtrip/testapp_allowlist_test.go +++ b/mdl/roundtrip/testapp_allowlist_test.go @@ -115,8 +115,6 @@ var testAppKnownFailures = map[string]knownFailure{ "enumeration WorkflowCommons.Enum_NotificationArea_RenderAs": {laws: []law{lawGetPut}, issue: "#721", why: "enumeration: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "export mapping Services.OrdersExportMapping": {laws: []law{lawGetPut}, issue: "#721", why: "export mapping: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "page WorkflowCommons.WorkflowAttachment_View": {laws: []law{lawExec}, issue: "#721", why: "page: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "page WorkflowCommons.WorkflowDefinition_View": {laws: []law{lawExec}, issue: "#721", why: "page: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "page WorkflowCommons.WorkflowSubProcess_View": {laws: []law{lawExec}, issue: "#721", why: "page: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "rest client Clients.OrdersRestClient": {laws: []law{lawParse}, issue: "#721", why: "rest client: breaks parse on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "rule Rules.Rule1": {laws: []law{lawGetPut}, issue: "#721", why: "rule: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "rule Rules.Rule2": {laws: []law{lawGetPut}, issue: "#721", why: "rule: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, @@ -128,14 +126,9 @@ var testAppKnownFailures = map[string]knownFailure{ "snippet WorkflowCommons.Snip_TimelineViewer_Timeline_Tasks": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowActivityRecord_ActivityIcon": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowActivityRecord_Details": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes; executes since a widget bound to a snippet parameter is described as $Param.Attr — getput not yet triaged"}, - "snippet WorkflowCommons.Snip_WorkflowActivityRecord_Header": {laws: []law{lawGetPut}, issue: "#721 #833", why: "text4 reads System.WorkflowSubProcess over System.WorkflowActivityRecord_WorkflowSubProcess, which the System module meta (modelsdk/meta/system_module.go) lacks, so the path cannot resolve and is written as an expression (#833)"}, "snippet WorkflowCommons.Snip_WorkflowActivity_Details_ActivityType": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "snippet WorkflowCommons.Snip_WorkflowSubProcess_Details": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "snippet WorkflowCommons.Snip_WorkflowSubProcess_IncompatibleWarning": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "snippet WorkflowCommons.Snip_WorkflowSubProcess_State": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_Workflow_CommentsAndAttachments": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_Workflow_CommentsAndAttachments_Admin": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "snippet WorkflowCommons.Snip_Workflow_SubProcessDetails": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, } // createOrModifyProbeKnownFailures is the allowlist of diff --git a/modelsdk/meta/system_enumerations.go b/modelsdk/meta/system_enumerations.go index 896a7aa26..88cfe77b1 100644 --- a/modelsdk/meta/system_enumerations.go +++ b/modelsdk/meta/system_enumerations.go @@ -40,8 +40,15 @@ import ( // developer actually wrote. The cost is that `search` — which indexes captions — // still does not match these; that needs real caption data, not a guess. func BuildSystemEnumerations() []*model.Enumeration { - out := make([]*model.Enumeration, 0, len(SystemEnumerations)) - for _, def := range SystemEnumerations { + return BuildSystemEnumerationsIn(nil) +} + +// BuildSystemEnumerationsIn is BuildSystemEnumerations for one project +// version, given as its IsAtLeast; nil means the current platform. +func BuildSystemEnumerationsIn(atLeast func(major, minor int) bool) []*model.Enumeration { + defs := SystemEnumerationsIn(atLeast) + out := make([]*model.Enumeration, 0, len(defs)) + for _, def := range defs { e := &model.Enumeration{ ContainerID: model.ID(SystemModuleID), // The model carries the LOCAL name; the module comes from diff --git a/modelsdk/meta/system_module.go b/modelsdk/meta/system_module.go index f084e9de8..1afe16ba0 100644 --- a/modelsdk/meta/system_module.go +++ b/modelsdk/meta/system_module.go @@ -101,6 +101,18 @@ func (a SystemAssocDef) InVersion(atLeast func(major, minor int) bool) bool { return true } +// introducedIn reports whether a member introduced in version introduced ("" = +// always) exists for atLeast; nil atLeast answers for the current platform. +func introducedIn(introduced string, atLeast func(major, minor int) bool) bool { + return introduced == "" || atLeast == nil || atLeast(parseMajorMinor(introduced)) +} + +// InVersion reports whether the entity exists in a project whose Mendix version +// satisfies atLeast; nil (version unknown) answers for the current platform. +func (e SystemEntityDef) InVersion(atLeast func(major, minor int) bool) bool { + return introducedIn(e.Introduced, atLeast) +} + func parseMajorMinor(v string) (int, int) { var major, minor int fmt.Sscanf(v, "%d.%d", &major, &minor) @@ -131,6 +143,13 @@ type SystemEntityDef struct { // backend that speaks to the runtime metamodel instead. RuntimeOnly bool + // Introduced is the first Mendix version, as "major.minor", whose System + // module has this entity; empty means every supported version. Measured, + // like SystemAssocDef.Introduced. An association is in a version only when + // both its ends are, so the associations of a newer entity need no flag of + // their own (see ModelerSystemAssociationsIn). + Introduced string + // The four system members an entity may store — System.owner, // System.changedBy, createdDate, changedDate. On a user entity these are the // AutoOwner / AutoChangedBy / AutoCreatedDate / AutoChangedDate pseudo-types; @@ -232,9 +251,15 @@ func systemEntityByName(name string) (SystemEntityDef, bool) { // full list minus the runtime-only rows. Every path that resolves a name against // the model wants this one; see SystemEntityDef.RuntimeOnly. func ModelerSystemEntities() []SystemEntityDef { + return ModelerSystemEntitiesIn(nil) +} + +// ModelerSystemEntitiesIn is ModelerSystemEntities for one project version, +// given as its IsAtLeast; nil means the current platform. +func ModelerSystemEntitiesIn(atLeast func(major, minor int) bool) []SystemEntityDef { out := make([]SystemEntityDef, 0, len(SystemEntities)) for _, e := range SystemEntities { - if !e.RuntimeOnly { + if !e.RuntimeOnly && e.InVersion(atLeast) { out = append(out, e) } } @@ -254,9 +279,11 @@ func ModelerSystemAssociations() []SystemAssocDef { // ModelerSystemAssociationsIn is ModelerSystemAssociations for one project // version, given as its IsAtLeast; nil means the current platform. func ModelerSystemAssociationsIn(atLeast func(major, minor int) bool) []SystemAssocDef { + // An end that is runtime-only, or not yet in this version, takes the + // association with it: derived, not flagged per association. runtimeOnly := make(map[string]bool, len(SystemEntities)) for _, e := range SystemEntities { - if e.RuntimeOnly { + if e.RuntimeOnly || !e.InVersion(atLeast) { runtimeOnly[e.Name] = true } } @@ -270,6 +297,14 @@ func ModelerSystemAssociationsIn(atLeast func(major, minor int) bool) []SystemAs return out } +// workflowSubProcessIntroduced is the first Mendix version whose System module +// has the workflow sub-process members, measured from the modeler assembly of +// each version (the ako/mxcli#1810 method): absent from 11.6.0 and 11.7.0, present +// in 11.8.0, 11.9.0, 11.10.0, 11.12.0 and 11.14.0. 11.8.0 also has +// WorkflowActivityDetails_WorkflowSubProcess, gone from 11.9.0 on; it is not +// modelled, as its ends and type were never measured. +const workflowSubProcessIntroduced = "11.8" + // SystemEntities lists all entities in the System module. // // Entities, attributes and types were extracted from Mendix Studio Pro 11.6.4 @@ -426,6 +461,23 @@ var SystemEntities = []SystemEntityDef{ {Name: "Retry", Type: "String", Length: 200}, {Name: "ScheduledEventName", Type: "String", Length: 200}, }}, + // Workflow sub-processes (introduced in workflowSubProcessIntroduced). + // Attributes, types and lengths measured from the System domain model in + // the deployment/model/model.mdp mxbuild 11.14.0 writes. TestApp's + // WorkflowCommons reads WorkflowSubProcess.Caption over + // WorkflowActivityRecord_WorkflowSubProcess. + {Name: "WorkflowSubProcessDefinition", Persistable: true, Introduced: workflowSubProcessIntroduced, Attributes: []SystemAttrDef{ + {Name: "Caption", Type: "String", Length: 200}, + {Name: "IsObsolete", Type: "Boolean"}, + {Name: "ModelGUID", Type: "String", Length: 36}, + }}, + {Name: "WorkflowSubProcess", Persistable: true, Introduced: workflowSubProcessIntroduced, Attributes: []SystemAttrDef{ + {Name: "Caption", Type: "String", Length: 200}, + {Name: "StartTime", Type: "DateTime"}, + {Name: "EndTime", Type: "DateTime"}, + {Name: "State", Type: "Enumeration", EnumQN: "System.WorkflowSubProcessState"}, + {Name: "Reason", Type: "String"}, + }}, {Name: "WorkflowDefinition", Persistable: true, Attributes: []SystemAttrDef{ {Name: "Name", Type: "String", Length: 200}, {Name: "Title", Type: "String", Length: 200}, @@ -665,6 +717,20 @@ var SystemAssociations = []SystemAssocDef{ {Name: "WorkflowActivity_SubWorkflow", Parent: "WorkflowActivity", Child: "Workflow", Type: "Reference", Owner: "Default"}, {Name: "WorkflowActivityUserTaskOutcome_WorkflowActivity", Parent: "WorkflowActivityUserTaskOutcome", Child: "WorkflowActivity", Type: "Reference", Owner: "Default"}, {Name: "WorkflowActivityUserTaskOutcome_User", Parent: "WorkflowActivityUserTaskOutcome", Child: "User", Type: "Reference", Owner: "Default"}, + // Workflow sub-process associations, measured from the 11.14.0 model.mdp. + // They exist exactly where their sub-process end does (no Introduced of + // their own); WorkflowActivity_WorkflowSubProcess is runtime-only through + // WorkflowActivity. + {Name: "WorkflowSubProcessDefinition_WorkflowDefinition", Parent: "WorkflowSubProcessDefinition", Child: "WorkflowDefinition", Type: "Reference", Owner: "Default"}, + {Name: "WorkflowUserTaskDefinition_WorkflowSubProcessDefinition", Parent: "WorkflowUserTaskDefinition", Child: "WorkflowSubProcessDefinition", Type: "Reference", Owner: "Default"}, + {Name: "WorkflowActivityRecord_WorkflowSubProcessDefinition", Parent: "WorkflowActivityRecord", Child: "WorkflowSubProcessDefinition", Type: "Reference", Owner: "Default"}, + {Name: "WorkflowSubProcess_WorkflowSubProcessDefinition", Parent: "WorkflowSubProcess", Child: "WorkflowSubProcessDefinition", Type: "Reference", Owner: "Default"}, + {Name: "WorkflowSubProcess_Workflow", Parent: "WorkflowSubProcess", Child: "Workflow", Type: "Reference", Owner: "Default"}, + {Name: "WorkflowActivityRecord_WorkflowSubProcess", Parent: "WorkflowActivityRecord", Child: "WorkflowSubProcess", Type: "Reference", Owner: "Default"}, + {Name: "WorkflowUserTask_WorkflowSubProcess", Parent: "WorkflowUserTask", Child: "WorkflowSubProcess", Type: "Reference", Owner: "Default"}, + {Name: "WorkflowEndedUserTask_WorkflowSubProcess", Parent: "WorkflowEndedUserTask", Child: "WorkflowSubProcess", Type: "Reference", Owner: "Default"}, + {Name: "WorkflowCurrentActivity_WorkflowSubProcess", Parent: "WorkflowCurrentActivity", Child: "WorkflowSubProcess", Type: "Reference", Owner: "Default"}, + {Name: "WorkflowActivity_WorkflowSubProcess", Parent: "WorkflowActivity", Child: "WorkflowSubProcess", Type: "Reference", Owner: "Default"}, {Name: "Thumbnail_Image", Parent: "Thumbnail", Child: "Image", Type: "Reference", Owner: "Both"}, {Name: "BackgroundJob_Session", Parent: "BackgroundJob", Child: "Session", Type: "Reference", Owner: "Default"}, {Name: "BackgroundJob_XASInstance", Parent: "BackgroundJob", Child: "XASInstance", Type: "Reference", Owner: "Default"}, @@ -676,6 +742,21 @@ var SystemAssociations = []SystemAssocDef{ type SystemEnumDef struct { Name string // qualified name, e.g. "System.EventStatus" Values []string // enumeration value names + // Introduced is the first Mendix version ("major.minor") with this + // enumeration; empty means every supported version. + Introduced string +} + +// SystemEnumerationsIn lists the System enumerations of one project version, +// given as its IsAtLeast; nil means the current platform. +func SystemEnumerationsIn(atLeast func(major, minor int) bool) []SystemEnumDef { + out := make([]SystemEnumDef, 0, len(SystemEnumerations)) + for _, e := range SystemEnumerations { + if introducedIn(e.Introduced, atLeast) { + out = append(out, e) + } + } + return out } // SystemEnumerations lists all enumerations in the System module. @@ -721,6 +802,10 @@ var SystemEnumerations = []SystemEnumDef{ "MultiInputUserTask", "WaitForNotification", "WaitForTimer", "EndOfBoundaryEventPath", "NonInterruptingTimerEvent", "InterruptingTimerEvent", }}, + // Measured from the 11.14.0 model.mdp, in its stored order. + {Name: "System.WorkflowSubProcessState", Introduced: workflowSubProcessIntroduced, Values: []string{ + "InProgress", "Aborted", "Failed", "Completed", "Paused", + }}, {Name: "System.WorkflowCurrentActivityAction", Values: []string{ "DoNothing", "JumpTo", }}, diff --git a/modelsdk/meta/system_subprocess_test.go b/modelsdk/meta/system_subprocess_test.go new file mode 100644 index 000000000..b6a1ffffd --- /dev/null +++ b/modelsdk/meta/system_subprocess_test.go @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: Apache-2.0 + +package meta + +import "testing" + +// The workflow sub-process members of the System module — two entities, their +// associations and System.WorkflowSubProcessState — were missing, so a binding +// over System.WorkflowActivityRecord_WorkflowSubProcess could not resolve: +// describe -> exec of TestApp's WorkflowCommons.Snip_WorkflowActivityRecord_Header +// wrote text4 as an expression, mx check CE0117 "Error(s) in expression.". +// +// Measured from the System domain model in the deployment/model/model.mdp +// mxbuild 11.14.0 writes; the version they first appear in from the modeler +// assembly of each Mendix version (the #1810 method). +func atLeastVersion(major, minor int) func(int, int) bool { + return func(ma, mi int) bool { return major > ma || major == ma && minor >= mi } +} + +func TestWorkflowSubProcessMembers(t *testing.T) { + ent, ok := systemEntityByName("WorkflowSubProcess") + if !ok { + t.Fatal("System.WorkflowSubProcess is not declared") + } + want := map[string]string{"Caption": "String", "StartTime": "DateTime", "EndTime": "DateTime", "State": "Enumeration", "Reason": "String"} + for _, a := range ent.Attributes { + if want[a.Name] != a.Type { + t.Errorf("WorkflowSubProcess.%s is %s, measured %s", a.Name, a.Type, want[a.Name]) + } + delete(want, a.Name) + } + if len(want) > 0 { + t.Errorf("WorkflowSubProcess lacks %v", want) + } + if _, ok := systemEntityByName("WorkflowSubProcessDefinition"); !ok { + t.Error("System.WorkflowSubProcessDefinition is not declared") + } + + has := func(atLeast func(int, int) bool, name string) bool { + for _, a := range ModelerSystemAssociationsIn(atLeast) { + if a.Name == name { + return true + } + } + return false + } + hasEntity := func(atLeast func(int, int) bool, name string) bool { + for _, e := range ModelerSystemEntitiesIn(atLeast) { + if e.Name == name { + return true + } + } + return false + } + current := atLeastVersion(11, 14) + if !has(current, "WorkflowActivityRecord_WorkflowSubProcess") || !hasEntity(current, "WorkflowSubProcess") { + t.Error("11.14 must have WorkflowSubProcess and WorkflowActivityRecord_WorkflowSubProcess") + } + if has(nil, "WorkflowActivity_WorkflowSubProcess") { + t.Error("WorkflowActivity_WorkflowSubProcess ends on a runtime-only entity and must not be modeler-visible") + } + // Before the version that introduced them, the names do not exist and a + // model naming one is CE1613; the association goes with its end entity. + before := atLeastVersion(subProcessBeforeMajor, subProcessBeforeMinor) + if hasEntity(before, "WorkflowSubProcess") || has(before, "WorkflowActivityRecord_WorkflowSubProcess") { + t.Errorf("%d.%d has no workflow sub-processes", subProcessBeforeMajor, subProcessBeforeMinor) + } + if !has(before, "WorkflowActivityRecord_Actor") { + t.Error("control: an unrelated association must stay") + } + for _, e := range SystemEnumerationsIn(before) { + if e.Name == "System.WorkflowSubProcessState" { + t.Errorf("System.WorkflowSubProcessState listed for %d.%d", subProcessBeforeMajor, subProcessBeforeMinor) + } + } +} + +// The last version measured WITHOUT the sub-process members. +const subProcessBeforeMajor, subProcessBeforeMinor = 11, 7 diff --git a/modelsdk/meta/testdata/system_string_lengths.txt b/modelsdk/meta/testdata/system_string_lengths.txt index da0f268c1..f572d8ec1 100644 --- a/modelsdk/meta/testdata/system_string_lengths.txt +++ b/modelsdk/meta/testdata/system_string_lengths.txt @@ -127,6 +127,10 @@ WorkflowRecord Description 0 WorkflowRecord Name 200 WorkflowRecord Reason 0 WorkflowRecord WorkflowKey 200 +WorkflowSubProcess Caption 200 +WorkflowSubProcess Reason 0 +WorkflowSubProcessDefinition Caption 200 +WorkflowSubProcessDefinition ModelGUID 36 WorkflowUserTask Description 0 WorkflowUserTask Name 0 WorkflowUserTask Outcome 200