diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-23-nightly-11-14-11-15-runlifecycle-wants-serve-rebuild-hint.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-23-nightly-11-14-11-15-runlifecycle-wants-serve-rebuild-hint.json new file mode 100644 index 000000000..7d42ba239 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-23-nightly-11-14-11-15-runlifecycle-wants-serve-rebuild-hint.json @@ -0,0 +1 @@ +{"area":"cmd/mxcli","date":"2026-10-23","symptom":"The nightly goes red on the Mendix 11.14.0 and 11.15.0 legs only, in `TestRunLifecycle_DetachStatusExecWaitStop`: `wait (Mendix 11.14+): want build #2 to fail with the serve-rebuild hint; exit 0, \"applied: build #2 via restart in 48.1s (runtime restarted; browser sessions dropped)\"`. Every other leg, push CI and the unit tests are green","cause":"Not a product defect: a nightly-only test pinned the 11.14+ mxbuild --serve regression as a FAILED build #2, and cf135bfc (serveRecovery) made the watch loop restart mxbuild --serve and build on the fresh process when an incremental build fails for want of client source, so build #2 now applies. The lifecycle test needs a cached mxbuild and a runtime, so it runs only in the nightly matrix, never in the PR that changed the behaviour","file":"`cmd/mxcli/cmd_run_lifecycle_integration_test.go` (`wantBuild2`); the behaviour is `cmd/mxcli/docker/serve_recovery.go`","insight":"A test that pins a documented failure is a contract with the code that explains it: improving that failure flips the test, and when the test only runs nightly the flip lands a day later on main. Whoever changes serve_recovery.go, webclient_legacy_paths.go or the watch loop should grep for `serveRebuildsBroken` and run the lifecycle test on 11.14+. The new assertion keeps the 'when mxbuild fixes it, this branch should go' property by requiring the recovery's own log line ('cannot build this change incrementally') next to the applied build, not just an applied build. Reproduce in about 2 minutes: `mxcli setup mxbuild --version 11.14.0`, JAVA_HOME at a JDK 25 (11.14's serve refuses 21 with 'JDK 25 not found'), then `MXCLI_RUNLIFE_VERSION=11.14.0 go test -tags integration -run TestRunLifecycle_DetachStatusExecWaitStop ./cmd/mxcli/`. Controls: the old test against current code fails at line 157 with the nightly's exact message; the new test passes all six steps; the new test with the legacy-client restart branch stubbed fails with mxbuild's 'Could not find a part of the path …/web/pages/MyFirstModule.Hello.js'","refs":["mendixlabs/mxcli nightly run 39915484754","ako/mxcli nightly run 39914908250"],"test":"cmd/mxcli/cmd_run_lifecycle_integration_test.go"} diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-23-alter-set-pluggable-attribute-property.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-23-alter-set-pluggable-attribute-property.json new file mode 100644 index 000000000..0c8a75cc4 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-23-alter-set-pluggable-attribute-property.json @@ -0,0 +1 @@ +{"area":"mdl/backend","date":"2026-10-23","symptom":"`alter page … set (eventDataAttribute: JsonData) on chart1` was refused for every spelling: \"pluggable property \\\"eventDataAttribute\\\" holds a value of kind Attribute, which ALTER cannot set from a plain value — rewrite the widget with CREATE OR REPLACE PAGE (or ALTER PAGE REPLACE) instead\"; `$Ctx.EventData` failed earlier, at parse, as \"takes a plain value, not an expression\". check --references refused the same, so one binding could only be fixed by rewriting the whole widget.","cause":"Which entity a bare attribute name belongs to is the widget's binding rule applied to the property's datasource link and the scopes around it — not a fact of the BSON the page mutator holds — so setPluggableWidgetPropertyMut had nothing to qualify a name with and refused the kind outright. The ALTER SET visitor refused every expression-shaped value except DynamicClasses / DynamicCellClass, `$Param.Attr` included.","fix":"The mutator reports the property instead of binding it: PluggablePropertyScope (an optional backend.PluggablePropertyScoper) gives the kind, DataSourceProperty link, and the linked and enclosing sources' entity or flow. The executor (resolvePluggableSetValue) binds by CREATE's own functions — resolveBindingScope, misboundAttributeError (CE1613 and CE7247 refusals word for word), parameterVariable for `$Param.Attr`, resolveAttributePathForEntity for inherited members — and hands the mutator a backend.AttributeBinding, which writes AttributeRef and SourceVariable (null when bound to the context object). The visitor passes the strict `$Param.Attr` shape (ast.IsParameterAttributeText) through as a WidgetExpressionV3; on any other property the executor refuses it. check dry-runs the same applySetProperties, so it follows with no code of its own.","insight":"When a mutator refuses a kind because it 'cannot know' something, check whether the executor already knows it: here the whole binding rule existed for CREATE and only the property's link and scopes had to be read out of the stored document. Keep the mutator a writer of resolved values and the refusals stay shared — a second binding rule in the mutator is the duplicate-resolver drift that this area keeps paying for. The SourceVariable has to be written as null when a binding returns to the context object: leaving the stale `$Ctx` in place was measured as CE7247 \"Attribute 'MyFirstModule.ChartEntry.JsonData' is not an attribute of entity 'MyFirstModule.ChartContext'\" on 11.12.5, with describe printing `$Ctx.JsonData`. Wrong turn to skip: relaxing the visitor's expression refusal in general — only the one `$Param.Attr` shape is a binding.","issue":"ako/mxcli#1622","refs":["ako/mxcli#1502","mendixlabs/mxcli#1591"],"ce":["CE7247","CE1613"],"file":"mdl/backend/pagemutator/pluggable_attribute.go, mdl/backend/pagemutator/mutator.go (setPluggableWidgetPropertyMut), mdl/executor/alter_set_pluggable_attribute.go, mdl/executor/cmd_alter_page.go (applySetProperties), mdl/visitor/visitor_alter_page.go, mdl/ast/ast_page_v3.go (IsParameterAttributeText)","test":"mdl/backend/pagemutator/pluggable_attribute_set_1622_test.go, mdl/executor/alter_set_pluggable_attribute_1622_test.go, mdl/visitor/visitor_alter_set_parameter_attribute_test.go, mdl-examples/bug-tests/1622-alter-set-pluggable-attribute.mdl"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-23-inherited-association-member-bare.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-23-inherited-association-member-bare.json new file mode 100644 index 000000000..7e960baef --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-23-inherited-association-member-bare.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-23", "symptom": "`find $l by Other.Person_Dept = $d` over HR.Emp (HR.Emp extends Other.Person) built clean, but describe printed `find $l by Person_Dept = $d`; executed as a new microflow that passed check -p and exec and mxbuild then failed `[CE1613] \"The selected attribute 'HR.Emp.Person_Dept' no longer exists.\"` (11.12.5). A hand-written bare inherited association in a find or a create's members stored the same attribute (ako/mxcli#1637)", "cause": "An association lives in its FROM entity's module, so one inherited from a generalization in another module is not in the specialization's module. resolveMemberChange looked a bare name up only there, then fell through to an attribute of the entity. resolveAttributeInEntityHierarchy already walked the chain for attributes; nothing did for associations", "file": "`mdl/executor/cmd_microflows_builder_actions.go` (`resolveInheritedAssociation`, tried after the inherited-attribute lookup in resolveMemberChange's bare-name branch)", "insight": "Fix the reader the short form is executed by, not the printer: describe has no variable types for a list operation, but the builder knows the entity, so making the bare name resolve through the chain makes describe's existing output correct, leaves every describe golden untouched, and lets the member speller (which shortens only when the short name resolves to the same member) agree with describe for free — its test was the cheapest proof, the round trip needs a FRESH build because re-running over the stored flow is spliced as 'Unchanged'. Assert on the stored unit bytes (`HR.Emp.Person_Dept` absent, `Other.Person_Dept` present): describe prints attribute and association alike, so it cannot show the damage. Not covered: the same bare name in two modules on one chain still resolves to the entity's own module first, and check -p still passes a member that names nothing", "refs": ["ako/mxcli#1637", "ako/mxcli#662", "mdl-examples/bug-tests/1637-inherited-association-member.mdl"], "ce": ["CE1613"], "rules": []} diff --git a/.claude/skills/mendix/alter-page/SKILL.md b/.claude/skills/mendix/alter-page/SKILL.md index b16e00d6a..c05f0d8da 100644 --- a/.claude/skills/mendix/alter-page/SKILL.md +++ b/.claude/skills/mendix/alter-page/SKILL.md @@ -243,7 +243,7 @@ so a silent write would build cleanly and then fail to open. > `Editable` on a non-input widget is rejected. This mirrors CREATE PAGE's > `visible:` — see the create-page skill for enum-value rules. -**Pluggable widget properties** use quoted names to set values in the widget's `Object.Properties[]`. Boolean values are stored as `"yes"`/`"no"` in BSON. +**Pluggable widget properties** use quoted names to set values in the widget's `Object.Properties[]`. Boolean values are stored as `"yes"`/`"no"` in BSON. An **attribute** property binds as in CREATE: `Attr` (the bound entity's), `Module.Entity.Attr`, or `$Param.Attr` — `set (eventDataAttribute: $Ctx.EventData) on chart1`; an attribute the bound entity lacks is refused (CE1613, CE7247). **Column property names are case-insensitive** in MDL — `set (caption: …)` and `set (Caption: …)` both work. The internal BSON keys are dictated by the widget schema and stay case-sensitive on the storage side. diff --git a/CHANGELOG.md b/CHANGELOG.md index 98d50887d..718b4b96b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **A find, filter or create by an association inherited from another module keeps that association when written short** (ako/mxcli#1637) — over `HR.Emp extends Other.Person`, `describe` prints `find $l by Other.Person_Dept = $d` as `find $l by Person_Dept = $d`, and the builder looked a bare member up only in `HR`, so executing that description as a new microflow — or writing the name short by hand — stored the attribute `HR.Emp.Person_Dept`: `check -p` and exec passed, then mxbuild failed with CE1613 (11.12.5). Re-running over the stored flow said "Unchanged", so only a rebuild showed it. A bare member name now resolves through the generalization chain to an association of an ancestor in another module, after the inherited attributes. - **`find` / `filter` by `System.owner` or `System.changedBy` builds, and describe prints System associations back so they re-execute** (ako/mxcli#1629) — `$m = find $l by System.owner = $User;` was refused at exec with advice to create a System association, though mxbuild builds it (11.12.5). It now resolves to the System association; on an entity whose generalization root does not store the member, `check -p` and exec refuse it with the CE0091 "No member selected." mxbuild would report, naming the `alter entity` that fixes it. `describe` printed a stored one as `by owner`, and a System association inherited by a specialization of `System.User` as `by User_Language` — which re-executed as an attribute of the specialization, CE1613. A System association in a find or filter is now printed qualified (`by System.User_Language`), and running the description again changes nothing. - **`marketplace diff` and `update` no longer fail with "path is too long" under the default Windows `%TEMP%`** (mendixlabs/mxcli#1486) — `mx` refuses to write any file whose path is 260 characters or longer, on every platform (measured on Linux with mx 11.12.1: a 259-character path is written, a 260-character one refused with the reported *"The specified file name or path is too long"*), and the reference project was built in `%TEMP%\mxdiff\target-ref`. The deepest files mx writes there are bundled `node_modules` trees — 181 characters in the 11.12.1 blank app, about 202 in NanoflowCommons 7.x — so that layout fitted 7.x for no user name at all. References are now built under the shortest directory the user owns (`%USERPROFILE%\.mxcli\t` on Windows, `/tmp` on Linux and macOS, or `TMP`/`TMPDIR` when shorter), named in one letter beneath it; never a drive-root folder such as `C:\mxt`, whose children every local account may modify. A package deep enough to hit the limit anyway gets an error naming the directory, its length, what it leaves for the package, and how to move it. `marketplace install` builds its reference the same way. - **`marketplace update` and `install` no longer write the package's metadata into the app root** (mendixlabs/mxcli#1487) — after updating Nanoflow Commons 6.2.1 → 7.4.0, `manifest.json`, `License.txt` and `SiemensMendixNanoflowCommons__7.4.0__READMEOSS_.html` sat beside the `.mpr` as untracked files, because every archive entry but `project.mpr` and `package.xml` was copied into the project. Only the files the package's `package.xml` lists under `` are installed now — exactly what `mx module-import` installs, measured on 11.12.1 with ExternalDatabaseConnector 6.3.0 and BusinessEvents 3.12.1, which includes passing over a listed file the archive lacks. What is left out (`License.txt`, `manifest.json`, the READMEOSS report, `Mendix EULA.docx`) is named on one line of the output. diff --git a/cmd/mxcli/cmd_run_lifecycle_integration_test.go b/cmd/mxcli/cmd_run_lifecycle_integration_test.go index 5aaf8a0d6..f780c3368 100644 --- a/cmd/mxcli/cmd_run_lifecycle_integration_test.go +++ b/cmd/mxcli/cmd_run_lifecycle_integration_test.go @@ -40,9 +40,10 @@ import ( // // On Mendix 11.14+ every build after the first in one mxbuild --serve process // fails (an mxbuild regression, measured without mxcli: webclient_legacy_paths.go). -// There the steps that wait for build #2 assert that documented failure — the -// build fails and the run log explains why — instead of an applied build. When an -// mxbuild fixes it, those assertions fail and this branch should go. +// mxcli recovers by restarting mxbuild --serve and building on the fresh process +// (serve_recovery.go), so build #2 still applies there; the steps that wait for it +// also assert that the run log shows that recovery. When an mxbuild fixes the +// regression, those assertions fail and this branch should go. func TestRunLifecycle_DetachStatusExecWaitStop(t *testing.T) { version := os.Getenv("MXCLI_RUNLIFE_VERSION") if version == "" { @@ -109,22 +110,23 @@ func TestRunLifecycle_DetachStatusExecWaitStop(t *testing.T) { return f } - // wantBuild2 checks a `run wait` that should report build #2: applied, or on - // 11.14+ failed with mxcli's explanation of the mxbuild regression in the log. - // orApplied also accepts an applied build #2 on 11.14+, for a step whose - // claim is only that build #2 ran. - wantBuild2 := func(step, out string, code int, orApplied bool) { + // wantBuild2 checks a `run wait` that should report build #2 applied. On + // 11.14+ that build only applies because mxcli restarted mxbuild --serve after + // the incremental build failed, so the run log must show that recovery. + // orDirect drops that requirement, for a step whose claim is only that build + // #2 ran. + wantBuild2 := func(step, out string, code int, orDirect bool) { t.Helper() - if serveRebuildsBroken && !(orApplied && code == 0 && strings.HasPrefix(out, "applied: build #2")) { - log, _ := os.ReadFile(docker.RunLogPath(mpr)) - if code != 1 || !strings.HasPrefix(out, "failed: build #2") || !strings.Contains(string(log), "first build does not") { - t.Fatalf("%s (Mendix 11.14+): want build #2 to fail with the serve-rebuild hint; exit %d, %q\n%s", step, code, out, tailOf(docker.RunLogPath(mpr))) - } - return - } if code != 0 || !strings.HasPrefix(out, "applied: build #2") { t.Fatalf("%s: exit %d, %q\n%s", step, code, out, tailOf(docker.RunLogPath(mpr))) } + if serveRebuildsBroken && !orDirect { + log, _ := os.ReadFile(docker.RunLogPath(mpr)) + if !strings.Contains(string(log), "cannot build this change incrementally") { + t.Fatalf("%s (Mendix 11.14+): build #2 applied without mxcli restarting mxbuild --serve; "+ + "if this mxbuild builds incrementally again, drop the 11.14+ branch\n%s", step, tailOf(docker.RunLogPath(mpr))) + } + } } // 1. detach: one line, exit 0, and the app answers. @@ -205,11 +207,11 @@ func TestRunLifecycle_DetachStatusExecWaitStop(t *testing.T) { if err := bg.Wait(); err != nil { t.Fatalf("detach (boot race): %v\n%s", err, tailOf(docker.RunLogPath(mpr))) } - // What this step claims is that the change is not lost: build #2 runs. On - // 11.14+ this build was measured applying (via restart) where step 4's failed; - // the likely difference is how much of the change the boot build had already - // read, which the test does not control. So either outcome passes there; - // "nothing to apply" never does. + // What this step claims is that the change is not lost: build #2 applies. On + // 11.14+ this build was measured applying without a serve restart where step + // 4's needed one; the likely difference is how much of the change the boot + // build had already read, which the test does not control. So the recovery is + // not required in the log here; "nothing to apply" never passes. out, code = mxcli("run", "wait", "-p", mpr, "--timeout", "2m") wantBuild2("a change made during the boot", out, code, true) if out, code := mxcli("run", "stop", "-p", mpr); code != 0 { diff --git a/docs-site/src/reference/page/alter-page.md b/docs-site/src/reference/page/alter-page.md index 8c7cffd35..e1fa47d6f 100644 --- a/docs-site/src/reference/page/alter-page.md +++ b/docs-site/src/reference/page/alter-page.md @@ -82,6 +82,15 @@ For page-level properties (like `Title`), omit the `ON` clause. For pluggable widget properties, use quoted property names (e.g., `'showLabel'`). +A pluggable widget's **attribute** property is bound the way `CREATE PAGE` binds it. +A bare name (`set (eventDataAttribute: JsonData) on chart1`) belongs to the entity +the property binds to: the widget's own datasource when the property is linked to +one, otherwise the enclosing data container. `Module.Entity.Attr` must be an +attribute of that entity or of one of its generalizations. `$Param.Attr` reads an +attribute of a page or snippet parameter instead. A name the bound entity lacks +(CE1613), and another entity's qualified attribute (CE7247), are refused by +`check` and `exec` alike. + ### INSERT BEFORE / INSERT AFTER Inserts new widgets immediately before or after a named widget within its parent container. The new widgets use the same syntax as in `CREATE PAGE`. diff --git a/mdl-examples/bug-tests/1622-alter-set-pluggable-attribute.mdl b/mdl-examples/bug-tests/1622-alter-set-pluggable-attribute.mdl new file mode 100644 index 000000000..f6a6b1e3e --- /dev/null +++ b/mdl-examples/bug-tests/1622-alter-set-pluggable-attribute.mdl @@ -0,0 +1,55 @@ +mdl 1; +-- ============================================================================ +-- ALTER PAGE SET on a pluggable widget's attribute property (ako/mxcli#1622) +-- ============================================================================ +-- +-- Symptom (before fix), Mendix 11.12.5: every spelling was refused. +-- set (eventDataAttribute: JsonData) on chart1 +-- → pluggable property "eventDataAttribute" holds a value of kind +-- Attribute, which ALTER cannot set from a plain value — rewrite the +-- widget with CREATE OR REPLACE PAGE (or ALTER PAGE REPLACE) instead +-- set (eventDataAttribute: $Ctx.EventData) on chart1 +-- → Parse error: property eventDataAttribute takes a plain value, not an +-- expression +-- +-- Fix: the mutator reports the property's kind, datasource link and scopes; +-- the executor binds the value by CREATE's rule and hands it the resolved +-- AttributeRef (+ SourceVariable for `$Param.Attr`). check --references +-- dry-runs the same path. Measured on mxbuild 11.12.5: +-- after each SET below → 0 errors +-- bound back to JsonData with the $Ctx +-- SourceVariable left in place (fault) → CE7247 +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/1622-alter-set-pluggable-attribute.mdl -p app.mpr +-- mxcli docker check -p app.mpr → 0 errors +-- mxcli -p app.mpr -c "describe page BugTest1622.ChartPage" +-- → eventDataAttribute: $Ctx.EventData +-- `set (eventDataAttribute: EventData) on chart1` is refused (CE1613), and +-- `set (eventDataAttribute: BugTest1622.ChartContext.EventData)` too (CE7247). +-- ============================================================================ + +create module BugTest1622; + +create non-persistent entity BugTest1622.ChartContext (EventData: String(unlimited)); +create non-persistent entity BugTest1622.ChartEntry (JsonData: String(unlimited), JsonLayout: String(unlimited)); + +create page BugTest1622.ChartPage ( + Title: 'Chart', + Layout: Atlas_Core.Atlas_Default, + Params: ( $Ctx: BugTest1622.ChartContext, $Entry: BugTest1622.ChartEntry ) +) { + dataview dvEntry (DataSource: $Entry) { + customchart chart1 ( + dataAttribute: JsonData, + layoutAttribute: JsonLayout, + eventDataAttribute: JsonLayout + ) + } +}; + +-- A bare name: an attribute of the data view's entity. +alter page BugTest1622.ChartPage { set (eventDataAttribute: JsonData) on chart1; }; + +-- An attribute of the other page parameter. +alter page BugTest1622.ChartPage { set (eventDataAttribute: $Ctx.EventData) on chart1; }; diff --git a/mdl-examples/bug-tests/1637-inherited-association-member.mdl b/mdl-examples/bug-tests/1637-inherited-association-member.mdl new file mode 100644 index 000000000..d1f073211 --- /dev/null +++ b/mdl-examples/bug-tests/1637-inherited-association-member.mdl @@ -0,0 +1,40 @@ +mdl 1; +-- ============================================================================ +-- ako/mxcli#1637 — a member association inherited from another module +-- ============================================================================ +-- +-- An association lives in its FROM entity's module. HR.Emp extends Other.Person, +-- so it inherits Other.Person_Dept — an association of module Other. describe +-- prints a find's association short (`find $l by Person_Dept = $d`), and the +-- builder looked a bare name up only in HR, so executing that description (or +-- writing the name bare by hand) stored the ATTRIBUTE HR.Emp.Person_Dept: +-- [CE1613] "The selected attribute 'HR.Emp.Person_Dept' no longer exists." +-- (measured on 11.12.5; re-running over the stored flow said "Unchanged", so +-- only a rebuild showed it). A bare name now resolves through the +-- generalization chain, after the inherited attributes. Each form below builds +-- clean on 11.12.5. +-- ============================================================================ + +create module B1637Base; +create module B1637; + +create persistent entity B1637Base.Person ("Name": String(50)); +create persistent entity B1637Base.Dept ("Name": String(50)); +create association B1637Base.Person_Dept from B1637Base.Person to B1637Base.Dept; + +create persistent entity B1637.Emp extends B1637Base.Person ("Code": String(10)); + +-- Qualified, as written — describe prints it as `by Person_Dept`. +create or modify microflow B1637.Emp_FirstInDept ($Emps: List of B1637.Emp, $Dept: B1637Base.Dept) +returns B1637.Emp as $Emp +begin + $Emp = find $Emps by B1637Base.Person_Dept = $Dept; +end; + +-- The short spelling describe prints, in a find and in a create. +create or modify microflow B1637.Emp_HireInto ($Emps: List of B1637.Emp, $Dept: B1637Base.Dept) +returns B1637.Emp as $New +begin + $Found = filter $Emps by Person_Dept = $Dept; + $New = create B1637.Emp (Code = 'new', Person_Dept = $Dept); +end; diff --git a/mdl/ast/ast_page_v3.go b/mdl/ast/ast_page_v3.go index 5a1f181c5..15b6f2f24 100644 --- a/mdl/ast/ast_page_v3.go +++ b/mdl/ast/ast_page_v3.go @@ -3,6 +3,7 @@ package ast import ( + "regexp" "strings" "github.com/mendixlabs/mxcli/mdl/types" @@ -208,6 +209,18 @@ func (w *WidgetV3) ExpressionSource(key string) (string, bool) { return "", false } +// parameterAttributeTextRe is `$Param.Attr` and nothing more: one variable, +// one attribute name (quoted when it is a keyword). `$Ctx.Name + 'x'` is an +// expression, and so is `$Ctx/Name`. +var parameterAttributeTextRe = regexp.MustCompile(`^\$[A-Za-z_][A-Za-z0-9_]*\.(?:[A-Za-z_][A-Za-z0-9_]*|"[A-Za-z_][A-Za-z0-9_]*")$`) + +// IsParameterAttributeText reports whether text is the `$Param.Attr` shape: an +// attribute of a page or snippet parameter, the binding an attribute property +// takes. It is no Mendix expression, which would read `$Param/Attr`. +func IsParameterAttributeText(text string) bool { + return parameterAttributeTextRe.MatchString(text) +} + // WidgetExpressionV3 is a generic widget property written as a Mendix // expression that no other value form accepts — `dynamicBarColor: if // $currentObject/Hot then 'red' else 'blue'`. Only an Expression-typed property diff --git a/mdl/backend/mutation.go b/mdl/backend/mutation.go index be6327d08..3ce937842 100644 --- a/mdl/backend/mutation.go +++ b/mdl/backend/mutation.go @@ -472,6 +472,44 @@ type ColumnHiddenPropertyResetter interface { ResetHiddenColumnProperties(gridRef, columnRef string, rules []types.WidgetVisibilityRule, setProps []string) ([]string, error) } +// SourceScope is what one data source hands to the widgets it governs: the +// entity it names, or the microflow/nanoflow whose return entity it is — which +// lives in the flow document, so only a caller holding the model can resolve it. +type SourceScope struct { + Entity string + Microflow string + Nanoflow string +} + +// PluggablePropertyScope describes one top-level property of a stored pluggable +// widget, for a SET that has to bind it: its kind, the widget's own datasource +// it is linked to (widget.xml `dataSource="…"`, "" when none), and the scope of +// that datasource and of the enclosing data container. +type PluggablePropertyScope struct { + Key string // the property key as the template spells it + Kind string // ValueType.Type + DataSource string // the linked datasource property key, "" when linked to none + Linked SourceScope + Enclosing SourceScope +} + +// PluggablePropertyScoper is implemented by a PageMutator that can describe a +// stored pluggable widget's property, so the executor can resolve an attribute +// binding for ALTER … SET by the rule CREATE uses (ako/mxcli#1622). ok +// is false when widgetRef is not a pluggable widget or has no such property. +// Optional: the Studio Pro MCP backend has no pluggable SET path. +type PluggablePropertyScoper interface { + PluggablePropertyScope(widgetRef, propKey string) (PluggablePropertyScope, bool) +} + +// AttributeBinding is the value SetWidgetProperty takes for an Attribute-kind +// pluggable property: the attribute path already resolved by the executor, and +// the page or snippet parameter it is read from (nil for the context object). +type AttributeBinding struct { + Path string + SourceVariable *pages.WidgetVariable +} + // ObjectListItemSpec describes one item of an object-list property (e.g. one // Accordion group, one PopupMenu basicItem, one Maps marker). The backend // applies these specs to the template's nested PropertyTypeIDs to produce diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index 79904b0b1..ac5ed4836 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -3913,6 +3913,10 @@ func setPluggableWidgetPropertyMut(widget bson.D, propName string, value any) er // the runtime all read the template (mendixlabs/mxcli#1201). An empty // kind — a document with no ValueType — stays primitive, as before. kind := propKindMap[typePointerID] + if b, ok := value.(backend.AttributeBinding); ok { + // Resolved by the executor, which knows the binding scope (#1622). + return setAttributeBindingValue(valDoc, propName, kind, b) + } field, settable := columnValueField(kind) if !settable { return fmt.Errorf( diff --git a/mdl/backend/pagemutator/pluggable_attribute.go b/mdl/backend/pagemutator/pluggable_attribute.go new file mode 100644 index 000000000..674bd9b30 --- /dev/null +++ b/mdl/backend/pagemutator/pluggable_attribute.go @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "fmt" + "strings" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/mdl/backend/widgetobj" + "github.com/mendixlabs/mxcli/mdl/bsonutil" +) + +// ALTER … SET on a pluggable widget's attribute property (ako/mxcli#1622). +// +// Which entity a bare attribute name belongs to is not a fact of the BSON: it +// is the widget's binding rule (the executor's resolveBindingScope) applied to +// the property's datasource link and the scopes around the widget. So the +// mutator does not bind; it reports what the rule needs — PluggablePropertyScope +// — and writes what the executor resolved — a backend.AttributeBinding. + +// PluggablePropertyScope describes propKey of the pluggable widget widgetRef: +// its kind, the widget's own datasource it is linked to, and the scope of that +// datasource and of the nearest enclosing one. The key is matched +// case-insensitively, as the setter matches it (#1069). +func (m *Mutator) PluggablePropertyScope(widgetRef, propKey string) (backend.PluggablePropertyScope, bool) { + result := m.widgetFinder(m.rawData, widgetRef) + if result == nil || len(result.colPropKeys) > 0 || + bsonnav.DGetString(result.widget, "$Type") != "CustomWidgets$CustomWidget" { + return backend.PluggablePropertyScope{}, false + } + objType := bsonnav.DGetDoc(bsonnav.DGetDoc(result.widget, "Type"), "ObjectType") + for _, pt := range bsonnav.DGetArrayElements(bsonnav.DGet(objType, "PropertyTypes")) { + ptDoc, ok := pt.(bson.D) + if !ok { + continue + } + key := bsonnav.DGetString(ptDoc, "PropertyKey") + if key == "" || !strings.EqualFold(key, propKey) { + continue + } + vt := bsonnav.DGetDoc(ptDoc, "ValueType") + scope := backend.PluggablePropertyScope{ + Key: key, + Kind: bsonnav.DGetString(vt, "Type"), + DataSource: bsonnav.DGetString(vt, "DataSourceProperty"), + } + if scope.DataSource != "" { + scope.Linked = toSourceScope(resolveSourceScope(m.rawData, pluggablePropertyDataSource(result.widget, scope.DataSource))) + } + if ds, ok := findNearestDataSourceDoc(m.rawData, widgetRef); ok { + scope.Enclosing = toSourceScope(resolveSourceScope(m.rawData, ds)) + } + return scope, true + } + return backend.PluggablePropertyScope{}, false +} + +func toSourceScope(s sourceScope) backend.SourceScope { + return backend.SourceScope{Entity: s.Entity, Microflow: s.Microflow, Nanoflow: s.Nanoflow} +} + +// pluggablePropertyDataSource is the DataSource document stored under the +// widget's property dsKey, or nil. +func pluggablePropertyDataSource(widget bson.D, dsKey string) bson.D { + keys := buildPropKeyMap(widget) + for _, p := range bsonnav.DGetArrayElements(bsonnav.DGet(bsonnav.DGetDoc(widget, "Object"), "Properties")) { + pd, ok := p.(bson.D) + if !ok || keys[bsonnav.ExtractBinaryIDFromDoc(bsonnav.DGet(pd, "TypePointer"))] != dsKey { + continue + } + return bsonnav.DGetDoc(bsonnav.DGetDoc(pd, "Value"), "DataSource") + } + return nil +} + +// setAttributeBindingValue writes a resolved attribute binding into an +// Attribute-kind property's WidgetValue: the AttributeRef, and the +// SourceVariable naming the parameter it is read from — or null, so a value +// bound back to the context object is no longer read from the parameter. +func setAttributeBindingValue(valDoc bson.D, propName, kind string, b backend.AttributeBinding) error { + if kind != "Attribute" { + return fmt.Errorf("pluggable property %q holds a value of kind %s, not an attribute", propName, kind) + } + if b.Path == "" { + return fmt.Errorf("pluggable property %q: no attribute to bind", propName) + } + // Both slots are part of every WidgetValue Studio Pro writes. A value + // without them cannot be completed in place, and a SourceVariable left + // unwritten would keep reading the old parameter. + for _, key := range []string{"AttributeRef", "SourceVariable"} { + if !hasKey(valDoc, key) { + return fmt.Errorf("pluggable property %q stores no %s to set — rewrite the widget with "+ + "CREATE OR REPLACE PAGE (or ALTER PAGE REPLACE) instead", propName, key) + } + } + bsonnav.DSet(valDoc, "AttributeRef", bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "DomainModels$AttributeRef"}, + {Key: "Attribute", Value: b.Path}, + {Key: "EntityRef", Value: nil}, + }) + var sv any + if b.SourceVariable != nil && (b.SourceVariable.Widget != "" || b.SourceVariable.Variable != "") { + sv = widgetobj.PageVariableBSON(b.SourceVariable.Widget, b.SourceVariable.Variable, b.SourceVariable.Kind) + } + bsonnav.DSet(valDoc, "SourceVariable", sv) + return nil +} diff --git a/mdl/backend/pagemutator/pluggable_attribute_set_1622_test.go b/mdl/backend/pagemutator/pluggable_attribute_set_1622_test.go new file mode 100644 index 000000000..07f69916c --- /dev/null +++ b/mdl/backend/pagemutator/pluggable_attribute_set_1622_test.go @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// ako/mxcli#1622: `alter page … set (eventDataAttribute: …) on chart1` was +// refused for every spelling — "pluggable property "eventDataAttribute" holds a +// value of kind Attribute, which ALTER cannot set from a plain value". The +// mutator holds only BSON, so it cannot tell which entity a bare name belongs +// to; the executor resolves the binding and hands it an AttributeBinding, and +// the mutator reports what it needs for that: the property's kind, its +// datasource link, and the scope of the linked and the enclosing sources. + +// chartInDataView is a data view over M.Entry holding a Custom chart whose +// dataAttribute is linked to the chart's own datasource (over M.Point) and +// whose eventDataAttribute is linked to none, so it binds to the data view. +func chartInDataView() bson.D { + dsID, dataID, eventID := idBin(0x40), idBin(0x41), idBin(0x42) + propType := func(id bson.Binary, key, kind, link string) bson.D { + vt := bson.D{{Key: "Type", Value: kind}} + if link != "" { + vt = append(vt, bson.E{Key: "DataSourceProperty", Value: link}) + } + return bson.D{{Key: "$ID", Value: id}, {Key: "PropertyKey", Value: key}, {Key: "ValueType", Value: vt}} + } + attrValue := func(attr string) bson.D { + var ref any + if attr != "" { + ref = bson.D{ + {Key: "$ID", Value: idBin(0x50)}, + {Key: "$Type", Value: "DomainModels$AttributeRef"}, + {Key: "Attribute", Value: attr}, + {Key: "EntityRef", Value: nil}, + } + } + return bson.D{ + {Key: "AttributeRef", Value: ref}, + {Key: "Expression", Value: ""}, + {Key: "PrimitiveValue", Value: ""}, + {Key: "SourceVariable", Value: nil}, + } + } + chart := bson.D{ + {Key: "$Type", Value: "CustomWidgets$CustomWidget"}, + {Key: "Name", Value: "chart1"}, + {Key: "Type", Value: bson.D{ + {Key: "ObjectType", Value: bson.D{ + {Key: "PropertyTypes", Value: bson.A{int32(2), + propType(dsID, "datasource", "DataSource", ""), + propType(dataID, "dataAttribute", "Attribute", "datasource"), + propType(eventID, "eventDataAttribute", "Attribute", ""), + }}, + }}, + }}, + {Key: "Object", Value: bson.D{ + {Key: "Properties", Value: bson.A{int32(2), + bson.D{{Key: "TypePointer", Value: dsID}, {Key: "Value", Value: bson.D{ + {Key: "DataSource", Value: bson.D{{Key: "EntityRef", Value: directEntityRef("M.Point")}}}, + }}}, + bson.D{{Key: "TypePointer", Value: dataID}, {Key: "Value", Value: attrValue("")}}, + bson.D{{Key: "TypePointer", Value: eventID}, {Key: "Value", Value: attrValue("M.Entry.Old")}}, + }}, + }}, + } + dv := bson.D{ + {Key: "$Type", Value: "Forms$DataView"}, + {Key: "Name", Value: "dvEntry"}, + {Key: "DataSource", Value: bson.D{ + {Key: "$Type", Value: "Forms$DataViewSource"}, + {Key: "EntityRef", Value: directEntityRef("M.Entry")}, + }}, + {Key: "Widgets", Value: bson.A{int32(2), chart}}, + } + return pageWith(dv) +} + +func chartValue(t *testing.T, m *Mutator, key string) bson.D { + t.Helper() + result := m.widgetFinder(m.rawData, "chart1") + if result == nil { + t.Fatal("chart1 not found") + } + keys := buildPropKeyMap(result.widget) + for _, p := range bsonnav.DGetArrayElements(bsonnav.DGet(bsonnav.DGetDoc(result.widget, "Object"), "Properties")) { + pd := p.(bson.D) + if keys[bsonnav.ExtractBinaryIDFromDoc(bsonnav.DGet(pd, "TypePointer"))] == key { + return bsonnav.DGetDoc(pd, "Value") + } + } + t.Fatalf("no property %s", key) + return nil +} + +func TestPluggablePropertyScope(t *testing.T) { + m := &Mutator{rawData: chartInDataView(), widgetFinder: findBsonWidget} + + got, ok := m.PluggablePropertyScope("chart1", "eventDataAttribute") + want := backend.PluggablePropertyScope{Key: "eventDataAttribute", Kind: "Attribute", + Enclosing: backend.SourceScope{Entity: "M.Entry"}} + if !ok || got != want { + t.Errorf("eventDataAttribute: got %+v, %v; want %+v", got, ok, want) + } + + // Linked to the chart's own datasource; the key matched case-insensitively, + // as the setter matches it (#1069). + got, ok = m.PluggablePropertyScope("chart1", "DataAttribute") + want = backend.PluggablePropertyScope{Key: "dataAttribute", Kind: "Attribute", DataSource: "datasource", + Linked: backend.SourceScope{Entity: "M.Point"}, Enclosing: backend.SourceScope{Entity: "M.Entry"}} + if !ok || got != want { + t.Errorf("dataAttribute: got %+v, %v; want %+v", got, ok, want) + } + + for _, c := range []struct{ widget, key string }{{"chart1", "noSuchKey"}, {"dvEntry", "Name"}, {"nope", "x"}} { + if _, ok := m.PluggablePropertyScope(c.widget, c.key); ok { + t.Errorf("%s.%s: ok, want false", c.widget, c.key) + } + } +} + +func TestSetPluggableAttributeBinding(t *testing.T) { + m := &Mutator{rawData: chartInDataView(), widgetFinder: findBsonWidget} + + // `$Ctx.EventData`: the AttributeRef plus the parameter it is read from. + err := m.SetWidgetProperty("chart1", "eventDataAttribute", backend.AttributeBinding{ + Path: "M.Context.EventData", SourceVariable: &pages.WidgetVariable{Variable: "Ctx"}}) + if err != nil { + t.Fatalf("set: %v", err) + } + val := chartValue(t, m, "eventDataAttribute") + ref := bsonnav.DGetDoc(val, "AttributeRef") + if bsonnav.DGetString(ref, "$Type") != "DomainModels$AttributeRef" || + bsonnav.DGetString(ref, "Attribute") != "M.Context.EventData" || bsonnav.DGet(ref, "EntityRef") != nil { + t.Errorf("AttributeRef = %v, want DomainModels$AttributeRef M.Context.EventData with a null EntityRef", ref) + } + sv := bsonnav.DGetDoc(val, "SourceVariable") + if bsonnav.DGetString(sv, "$Type") != "Forms$PageVariable" || bsonnav.DGetString(sv, "PageParameter") != "Ctx" { + t.Errorf("SourceVariable = %v, want a Forms$PageVariable naming page parameter Ctx", sv) + } + + // Bound back to the context object: the SourceVariable goes, or the value + // would still be read from $Ctx. + if err := m.SetWidgetProperty("chart1", "eventDataAttribute", backend.AttributeBinding{Path: "M.Entry.JsonData"}); err != nil { + t.Fatalf("set: %v", err) + } + val = chartValue(t, m, "eventDataAttribute") + if got := bsonnav.DGetString(bsonnav.DGetDoc(val, "AttributeRef"), "Attribute"); got != "M.Entry.JsonData" { + t.Errorf("Attribute = %q, want M.Entry.JsonData", got) + } + if sv := bsonnav.DGet(val, "SourceVariable"); sv != nil { + t.Errorf("SourceVariable = %v, want null", sv) + } + + // An empty AttributeRef slot is filled, not skipped. + if err := m.SetWidgetProperty("chart1", "dataAttribute", backend.AttributeBinding{Path: "M.Point.X"}); err != nil { + t.Fatalf("set: %v", err) + } + if got := bsonnav.DGetString(bsonnav.DGetDoc(chartValue(t, m, "dataAttribute"), "AttributeRef"), "Attribute"); got != "M.Point.X" { + t.Errorf("dataAttribute = %q, want M.Point.X", got) + } +} + +// Control: an unresolved value on an Attribute property is still refused — the +// mutator cannot tell which entity a bare name belongs to. +func TestSetPluggableAttributePlainValueStillRefused(t *testing.T) { + m := &Mutator{rawData: chartInDataView(), widgetFinder: findBsonWidget} + err := m.SetWidgetProperty("chart1", "eventDataAttribute", "JsonData") + if err == nil || !strings.Contains(err.Error(), "kind Attribute") { + t.Fatalf("want the kind-Attribute refusal, got %v", err) + } + if got := bsonnav.DGetString(bsonnav.DGetDoc(chartValue(t, m, "eventDataAttribute"), "AttributeRef"), "Attribute"); got != "M.Entry.Old" { + t.Errorf("Attribute = %q after a refusal, want the stored M.Entry.Old", got) + } +} + +// An AttributeBinding on a property of another kind is a caller bug, refused +// rather than stringified into a PrimitiveValue. +func TestSetPluggableAttributeBindingOnOtherKindRefused(t *testing.T) { + m := &Mutator{rawData: chartInDataView(), widgetFinder: findBsonWidget} + if err := m.SetWidgetProperty("chart1", "datasource", backend.AttributeBinding{Path: "M.Point.X"}); err == nil { + t.Fatal("an attribute binding on a DataSource property was accepted") + } +} diff --git a/mdl/executor/alter_set_pluggable_attribute.go b/mdl/executor/alter_set_pluggable_attribute.go new file mode 100644 index 000000000..41f0c22e0 --- /dev/null +++ b/mdl/executor/alter_set_pluggable_attribute.go @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/model" +) + +// ALTER … SET on a pluggable widget's attribute property (ako/mxcli#1622). +// +// `set (eventDataAttribute: …) on chart1` was refused for every spelling: the +// mutator holds only BSON, and which entity a bare name belongs to is the +// widget's binding rule, not a fact of the document. So the mutator describes +// the property (backend.PluggablePropertyScoper) and this resolves the binding +// by the rule CREATE writes by — resolveBindingScope, misboundAttributeError, +// `$Param.Attr` — before handing it a backend.AttributeBinding. check +// --references dry-runs the same path against a copy of the document, so it +// refuses exactly what exec refuses. + +// resolvePluggableSetValue returns the value SetWidgetProperty should receive +// for prop of widgetRef: an AttributeBinding for an Attribute-kind pluggable +// property, value unchanged for anything else. A `$Param.Attr` value on a +// property that takes no attribute is refused: the visitor lets that one +// expression shape through only for the attribute case. +func resolvePluggableSetValue(ctx *ExecContext, mutator backend.PageMutator, widgetRef, prop string, value any, moduleName string, moduleID model.ID) (any, error) { + expr, isExpr := value.(*ast.WidgetExpressionV3) + var scope backend.PluggablePropertyScope + found := false + if scoper, ok := mutator.(backend.PluggablePropertyScoper); ok && widgetRef != "" { + scope, found = scoper.PluggablePropertyScope(widgetRef, prop) + } + if !found || scope.Kind != "Attribute" { + if isExpr { + return nil, mdlerrors.NewValidationf( + "property %s takes a plain value, not an expression: %s — `$Param.Attr` binds an attribute "+ + "property of a pluggable widget, and %s is not one", prop, expr.Text, prop) + } + return value, nil + } + + pb := newAlterPageBuilder(ctx, moduleName, moduleID, "", mutator) + where := fmt.Sprintf("widget `%s` property `%s`", widgetRef, scope.Key) + + if isExpr { + // `$Param.Attr`: the attribute of a page or snippet parameter, whatever + // encloses the widget — the AttributeRef plus a SourceVariable naming + // the parameter, as CREATE writes it. + name, rest, _ := namedObjectBinding(expr.Text) + wv, entity, ok := pb.parameterVariable(name) + if !ok { + return nil, notAParameterError(where, expr.Text, name, pb.documentKind()) + } + if err := pb.rejectAssociationAsAttribute(rest, entity, where); err != nil { + return nil, err + } + return backend.AttributeBinding{Path: pb.resolveAttributePathForEntity(rest, entity), SourceVariable: &wv}, nil + } + + attr, ok := value.(string) + if !ok || attr == "" { + return nil, mdlerrors.NewValidationf("%s takes an attribute name, `Module.Entity.Attr` or `$Param.Attr`", where) + } + if strings.ContainsAny(attr, "/$") { + return nil, mdlerrors.NewUnsupported(fmt.Sprintf( + "%s: alter page set cannot bind the path `%s` — use `replace with …`, which rebuilds the "+ + "widget through the CREATE PAGE path", where, attr)) + } + + // The property's scope, by the rule CREATE uses: its linked datasource's + // entity, or the enclosing data container's for an unlinked property. + outer := sourceScopeEntity(ctx, scope.Enclosing, moduleName, moduleID) + dsEntities := map[string]string{} + if scope.DataSource != "" { + if entity := sourceScopeEntity(ctx, scope.Linked, moduleName, moduleID); entity != "" { + dsEntities[scope.DataSource] = entity + } + } + bs := resolveBindingScope(scope.DataSource, true, outer, dsEntities, "") + if err := misboundAttributeError(where, attr, bs, outer, dsEntities, pb.attributeIndex()); err != nil { + return nil, err + } + if strings.Contains(attr, ".") { + return backend.AttributeBinding{Path: attr}, nil + } + if bs.Entity == "" { + return nil, mdlerrors.NewValidationf( + "%s: cannot tell which entity `%s` belongs to — the property's data source names none that "+ + "can be read here. Qualify it (`Module.Entity.%s`) or bind a parameter's (`$Param.%s`)", + where, attr, attr, attr) + } + if err := pb.rejectAssociationAsAttribute(attr, bs.Entity, where); err != nil { + return nil, err + } + return backend.AttributeBinding{Path: pb.resolveAttributePathForEntity(attr, bs.Entity)}, nil +} + +// sourceScopeEntity is the entity a data source supplies: the one it names, or +// its microflow's or nanoflow's return entity. +func sourceScopeEntity(ctx *ExecContext, s backend.SourceScope, moduleName string, moduleID model.ID) string { + if s.Entity != "" { + return s.Entity + } + return resolveDataSourceFlowEntity(ctx, moduleName, moduleID, s.Microflow, s.Nanoflow) +} diff --git a/mdl/executor/alter_set_pluggable_attribute_1622_test.go b/mdl/executor/alter_set_pluggable_attribute_1622_test.go new file mode 100644 index 000000000..c29730c9b --- /dev/null +++ b/mdl/executor/alter_set_pluggable_attribute_1622_test.go @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" +) + +// ako/mxcli#1622: ALTER PAGE SET on a pluggable widget's attribute property. +// The mutator reports the property's scope; the executor resolves the binding +// by the rule CREATE uses — resolveBindingScope, misboundAttributeError, +// `$Param.Attr` — and hands the mutator an AttributeBinding. Before, every +// spelling reached the mutator as a plain value and was refused. + +// scopedMutator is a MockPageMutator that also describes pluggable properties. +type scopedMutator struct { + *mock.MockPageMutator + scopes map[string]backend.PluggablePropertyScope +} + +func (m *scopedMutator) PluggablePropertyScope(widgetRef, propKey string) (backend.PluggablePropertyScope, bool) { + s, ok := m.scopes[widgetRef+"."+strings.ToLower(propKey)] + return s, ok +} + +// chartSetMutator: chart1 inside a data view over Sales.Invoice, on a page with +// $Ctx: Sales.Customer. eventDataAttribute is linked to no datasource; +// dataAttribute to the chart's own datasource over Sales.Order; title is a +// String property. +func chartSetMutator(got *any) *scopedMutator { + invoice := backend.SourceScope{Entity: "Sales.Invoice"} + return &scopedMutator{ + MockPageMutator: &mock.MockPageMutator{ + ParamScopeFunc: func() (map[string]model.ID, map[string]string) { + return map[string]model.ID{"Ctx": "e-customer"}, map[string]string{"Ctx": "Sales.Customer"} + }, + SetWidgetPropertyFunc: func(_, _ string, v any) error { *got = v; return nil }, + }, + scopes: map[string]backend.PluggablePropertyScope{ + "chart1.eventdataattribute": {Key: "eventDataAttribute", Kind: "Attribute", Enclosing: invoice}, + "chart1.dataattribute": {Key: "dataAttribute", Kind: "Attribute", DataSource: "datasource", + Linked: backend.SourceScope{Entity: "Sales.Order"}, Enclosing: invoice}, + "chart1.title": {Key: "title", Kind: "String", Enclosing: invoice}, + }, + } +} + +func setOnChart(t *testing.T, prop string, value any) (any, error) { + t.Helper() + var got any + m := chartSetMutator(&got) + op := &ast.SetPropertyOp{Target: ast.WidgetRef{Widget: "chart1"}, Properties: map[string]any{prop: value}} + err := applySetProperties(scopeCheckCtx(t), m, op, "Sales", model.ID("mod-sales")) + return got, err +} + +func TestAlterSet_PluggableAttributeIsResolved(t *testing.T) { + for _, tc := range []struct { + prop string + value any + path string + param string + }{ + // Bare: against the property's own scope — the enclosing data view for + // an unlinked property, the linked datasource for a linked one. + {"eventDataAttribute", "Reference", "Sales.Invoice.Reference", ""}, + {"dataAttribute", "Number", "Sales.Order.Number", ""}, + // Qualified with the bound entity: as written. + {"eventDataAttribute", "Sales.Invoice.Label", "Sales.Invoice.Label", ""}, + // `$Param.Attr`: the parameter's entity, and the parameter itself. + {"eventDataAttribute", &ast.WidgetExpressionV3{Text: "$Ctx.Name"}, "Sales.Customer.Name", "Ctx"}, + } { + got, err := setOnChart(t, tc.prop, tc.value) + if err != nil { + t.Errorf("%s = %v: %v", tc.prop, tc.value, err) + continue + } + b, ok := got.(backend.AttributeBinding) + if !ok { + t.Errorf("%s = %v: mutator got %#v, want a backend.AttributeBinding", tc.prop, tc.value, got) + continue + } + if b.Path != tc.path { + t.Errorf("%s = %v: Path = %q, want %q", tc.prop, tc.value, b.Path, tc.path) + } + gotParam := "" + if b.SourceVariable != nil { + gotParam = b.SourceVariable.Variable + } + if gotParam != tc.param { + t.Errorf("%s = %v: SourceVariable = %+v, want parameter %q", tc.prop, tc.value, b.SourceVariable, tc.param) + } + } +} + +// The refusals are CREATE's, word for word: a bare name the bound entity lacks +// (CE1613), a qualified attribute of another entity (CE7247), a `$name` that is +// no parameter. +func TestAlterSet_PluggableAttributeRefusals(t *testing.T) { + for _, tc := range []struct { + prop string + value any + want []string + }{ + {"eventDataAttribute", "Name", []string{"`Name`", "Sales.Invoice", "CE1613"}}, + {"dataAttribute", "Reference", []string{"`Reference`", "Sales.Order", "Sales.Invoice (the enclosing data container)"}}, + {"eventDataAttribute", "Sales.Customer.Name", []string{"CE7247", "$Param.Name"}}, + {"eventDataAttribute", &ast.WidgetExpressionV3{Text: "$Nope.Name"}, []string{"$Nope", "not a parameter"}}, + // An expression on a property that is not an attribute. + {"title", &ast.WidgetExpressionV3{Text: "$Ctx.Name"}, []string{"title", "not an expression"}}, + } { + got, err := setOnChart(t, tc.prop, tc.value) + if err == nil { + t.Errorf("%s = %v: accepted (%#v), want a refusal", tc.prop, tc.value, got) + continue + } + for _, w := range tc.want { + if !strings.Contains(err.Error(), w) { + t.Errorf("%s = %v: error should mention %q, got: %v", tc.prop, tc.value, w, err) + } + } + if got != nil { + t.Errorf("%s = %v: refused, but the mutator was still called with %#v", tc.prop, tc.value, got) + } + } +} + +// Control: a property of another kind passes through unchanged. +func TestAlterSet_PluggableNonAttributePassesThrough(t *testing.T) { + got, err := setOnChart(t, "title", "Sales") + if err != nil || got != "Sales" { + t.Errorf("title = 'Sales': got %#v, %v; want the plain value", got, err) + } +} diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index 27bdcbd78..3da530011 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -472,7 +472,13 @@ func applySetProperties(ctx *ExecContext, mutator backend.PageMutator, op *ast.S return mdlerrors.NewBackend("set "+propName+" on "+op.Target.Name(), err) } } else { - if err := mutator.SetWidgetProperty(op.Target.Widget, propName, value); err != nil { + // An attribute property of a pluggable widget is bound here, by the + // rule CREATE uses; the mutator only writes it (#1622). + resolved, err := resolvePluggableSetValue(ctx, mutator, op.Target.Widget, propName, value, moduleName, moduleID) + if err != nil { + return err + } + if err := mutator.SetWidgetProperty(op.Target.Widget, propName, resolved); err != nil { return mdlerrors.NewBackend("set "+propName+" on "+op.Target.Name(), err) } } diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index aeb9c2c92..d297c802c 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -2483,6 +2483,8 @@ func (fb *flowBuilder) resolveMemberChange(mc *microflows.MemberChange, memberNa mc.AttributeQualifiedName = memberName } else if attrQN, ok := fb.resolveAttributeInEntityHierarchy(entityQN, memberName); ok { mc.AttributeQualifiedName = attrQN + } else if assocQN, ok := fb.resolveInheritedAssociation(entityQN, memberName); ok { + mc.AssociationQualifiedName = assocQN } else { mc.AttributeQualifiedName = entityQN + "." + memberName } @@ -2548,6 +2550,59 @@ func (fb *flowBuilder) resolveAttributeInEntityHierarchy(entityQN, attrName stri return "", false } +// resolveInheritedAssociation finds a bare association name among the +// associations entityQN inherits from a generalization in ANOTHER module, and +// returns its qualified name. An association lives in its FROM entity's +// module, so one inherited from Other.Person is in Other — the lookup by name +// in the specialization's own module cannot see it, and the bare name was +// written as an attribute of the specialization, which mxbuild reports as +// CE1613 (ako/mxcli#1637). describe prints a find's association short, so its +// output executes through here. The entity's own module was searched by name +// already; a match must involve the ancestor, at either end. +func (fb *flowBuilder) resolveInheritedAssociation(entityQN, name string) (string, bool) { + if fb == nil || fb.backend == nil || entityQN == "" || name == "" { + return "", false + } + ownModule, _, _ := strings.Cut(entityQN, ".") + seen := make(map[string]bool) + for currentQN := entityQN; currentQN != ""; { + if seen[currentQN] { + return "", false + } + seen[currentQN] = true + module, entityName, ok := strings.Cut(currentQN, ".") + if !ok { + return "", false + } + mod, err := fb.backend.GetModuleByName(module) + if err != nil || mod == nil { + return "", false + } + dm, err := fb.backend.GetDomainModel(mod.ID) + if err != nil || dm == nil { + return "", false + } + entity := dm.FindEntityByName(entityName) + if entity == nil { + return "", false + } + if module != ownModule { + for _, a := range dm.Associations { + if a != nil && a.Name == name && (a.ParentID == entity.ID || a.ChildID == entity.ID) { + return module + "." + name, true + } + } + for _, a := range dm.CrossAssociations { + if a != nil && a.Name == name && a.ParentID == entity.ID { + return module + "." + name, true + } + } + } + currentQN = entity.GeneralizationRef + } + return "", false +} + // entityIsSubtypeOf reports whether candidateQN is the same as ancestorQN or // inherits from it through the generalization chain. The walk consults the // domain model the same way resolveAttributeInEntityHierarchy does. diff --git a/mdl/executor/cmd_pages_parameter_binding.go b/mdl/executor/cmd_pages_parameter_binding.go index c3cdcc5b2..a836ac2bb 100644 --- a/mdl/executor/cmd_pages_parameter_binding.go +++ b/mdl/executor/cmd_pages_parameter_binding.go @@ -20,7 +20,6 @@ package executor import ( - "regexp" "strings" "github.com/mendixlabs/mxcli/mdl/ast" @@ -115,12 +114,8 @@ func parameterAttributeMDL(name, attrQN string) string { return "$" + name + "." + mdlIdent(shortAttributeName(attrQN)) } -// parameterAttributeTextRe is `$Param.Attr` and nothing more: one variable, -// one attribute name (quoted when it is a keyword). namedObjectBinding alone -// takes `$Ctx.Name + 'x'` too, which is an expression. -var parameterAttributeTextRe = regexp.MustCompile(`^\$[A-Za-z_][A-Za-z0-9_]*\.(?:[A-Za-z_][A-Za-z0-9_]*|"[A-Za-z_][A-Za-z0-9_]*")$`) - // 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 { - return parameterAttributeTextRe.MatchString(text) + return ast.IsParameterAttributeText(text) } diff --git a/mdl/executor/inherited_association_member_test.go b/mdl/executor/inherited_association_member_test.go new file mode 100644 index 000000000..dcdb7e501 --- /dev/null +++ b/mdl/executor/inherited_association_member_test.go @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// ako/mxcli#1637: describe prints a find's association member short — `by +// Person_Dept` for Other.Person_Dept — and the builder resolved a bare name +// only in the list entity's own module. An association inherited from a +// generalization in another module was therefore not found, and the bare name +// was written as an attribute of the entity. Measured on 11.12.5: the +// description of a clean find, executed as a new microflow, passed check -p +// and exec and then failed +// +// [CE1613] "The selected attribute 'HR.Emp.Person_Dept' no longer exists." +// +// Re-running it over the stored flow said "Unchanged", so only a rebuild (a +// copy, a fresh project) showed it. +const inheritedAssocHead = `mdl 1; +create module Other; +create module HR; +create persistent entity Other.Person (Name: String(50)); +create persistent entity Other.Dept (Name: String(50)); +create association Other.Person_Dept from Other.Person to Other.Dept; +create persistent entity HR.Emp extends Other.Person (Code: String(10)); +create microflow HR.FindByDept ($l: List of HR.Emp, $d: Other.Dept) returns Boolean +begin + $m = find $l by Other.Person_Dept = $d; + return true; +end; +` + +// microflowUnit returns a stored microflow's unit bytes. +func microflowUnit(t *testing.T, exec *Executor, module, name string) []byte { + t.Helper() + ctx := exec.newExecContext(context.Background()) + h, err := getHierarchy(ctx) + if err != nil { + t.Fatal(err) + } + all, err := ctx.Backend.ListMicroflows() + if err != nil { + t.Fatal(err) + } + for _, m := range all { + if m.Name == name && h.GetModuleName(h.FindModuleID(m.ContainerID)) == module { + raw, err := ctx.Backend.GetRawUnitBytes(m.ID) + if err != nil { + t.Fatal(err) + } + return append([]byte(nil), raw...) + } + } + t.Fatalf("%s.%s not found", module, name) + return nil +} + +// assertStoresInheritedAssociation fails when the unit references the member +// as an attribute of HR.Emp instead of the association Other.Person_Dept. +func assertStoresInheritedAssociation(t *testing.T, unit []byte, what string) { + t.Helper() + if bytes.Contains(unit, []byte("HR.Emp.Person_Dept")) { + t.Errorf("%s: stored as the attribute HR.Emp.Person_Dept — mxbuild reports CE1613", what) + } + if !bytes.Contains(unit, []byte("Other.Person_Dept")) { + t.Errorf("%s: does not reference the association Other.Person_Dept", what) + } +} + +func TestInheritedAssociation_DescribeExecutesAsTheSameMember(t *testing.T) { + exec, out, dir := openPedAppCopy(t) + assertAgree(t, exec, dir, inheritedAssocHead, "") + + out.Reset() + if err := agreeExec(t, exec, "describe microflow HR.FindByDept;"); err != nil { + t.Fatalf("describe: %v", err) + } + described := out.String() + if !strings.Contains(described, "$m = find $l by Person_Dept = $d;") { + t.Fatalf("describe output changed shape; this test runs it back as written:\n%s", described) + } + + // Execute the description as a new microflow: a fresh build, with nothing + // stored to splice against. + i := strings.Index(described, "create or modify microflow HR.FindByDept") + if i < 0 { + t.Fatalf("no create statement in the description:\n%s", described) + } + copySrc := "mdl 1;\n" + strings.Replace(described[i:], "microflow HR.FindByDept ", "microflow HR.FindByDeptCopy ", 1) + assertAgree(t, exec, dir, copySrc, "") + assertStoresInheritedAssociation(t, microflowUnit(t, exec, "HR", "FindByDeptCopy"), "the executed description") +} + +// The same bare name written by hand, in a find and in a create's members. +func TestInheritedAssociation_BareNameResolves(t *testing.T) { + exec, _, dir := openPedAppCopy(t) + const flows = `create microflow HR.Bare ($l: List of HR.Emp, $d: Other.Dept) returns Boolean +begin + $m = find $l by Person_Dept = $d; + $e = create HR.Emp (Code = 'x', Person_Dept = $d); + return true; +end; +` + assertAgree(t, exec, dir, inheritedAssocHead+flows, "") + unit := microflowUnit(t, exec, "HR", "Bare") + assertStoresInheritedAssociation(t, unit, "a hand-written bare name") +} + +// diff-then-patch compares a declared flow in describe's spelling. describe +// prints `Person_Dept`; the speller shortens the declared Other.Person_Dept only +// when the short name resolves to the same member, which it did not — so the +// declared find and its own stored activity were spelled differently. +func TestInheritedAssociation_SpellerAgreesWithDescribe(t *testing.T) { + exec, _, _ := openPedAppCopy(t) + if err := agreeExec(t, exec, inheritedAssocHead); err != nil { + t.Fatalf("setup: %v", err) + } + prog, errs := visitor.Build(`create microflow HR.F ($l: List of HR.Emp, $d: Other.Dept) +begin + $m = find $l by Other.Person_Dept = $d; +end; +`) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs[0]) + } + f := prog.Statements[0].(*ast.CreateMicroflowStmt) + ctx := exec.newExecContext(context.Background()) + got := describedMemberSpellings(ctx, f.Parameters, f.Body, nil) + name, _ := listOperationFieldName(got[0].(*ast.ListOperationStmt).Condition.(*ast.BinaryExpr).Left) + if name != "Person_Dept" { + t.Errorf("declared find spelled %q; describe prints Person_Dept", name) + } +} + +// Control: an inherited ATTRIBUTE of the same bare name is still the attribute — +// the association lookup comes after it. +func TestInheritedAssociation_AttributeStillWins(t *testing.T) { + exec, _, dir := openPedAppCopy(t) + const src = `mdl 1; +create module Other; +create module HR; +create persistent entity Other.Person (Name: String(50)); +create persistent entity HR.Emp extends Other.Person (Code: String(10)); +create microflow HR.ByName ($l: List of HR.Emp) returns Boolean +begin + $m = find $l by Name = 'x'; + return true; +end; +` + assertAgree(t, exec, dir, src, "") + if unit := microflowUnit(t, exec, "HR", "ByName"); !bytes.Contains(unit, []byte("Other.Person.Name")) { + t.Errorf("find by an inherited attribute no longer stores Other.Person.Name") + } +} diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index 5b268134d..39c8dc005 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -1378,14 +1378,22 @@ func refuseAmbiguousGenericDataSource(def *WidgetDefinition, w *ast.WidgetV3, ds func (e *PluggableWidgetEngine) parameterBinding(w *ast.WidgetV3, propertyKey, attr, name string) (pages.WidgetVariable, string, error) { wv, entity, ok := e.pageBuilder.parameterVariable(name) if !ok { - return pages.WidgetVariable{}, "", mdlerrors.NewValidationf( - "%s `%s` property `%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", - strings.ToLower(w.Type), w.Name, propertyKey, attr, name, e.pageBuilder.documentKind()) + return pages.WidgetVariable{}, "", notAParameterError( + fmt.Sprintf("%s `%s` property `%s`", strings.ToLower(w.Type), w.Name, propertyKey), + attr, name, e.pageBuilder.documentKind()) } return wv, entity, 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", + where, attr, name, documentKind) +} + // entityContextFor returns the entity that propertyKey's value binds against. // See bindingScopeFor. func (e *PluggableWidgetEngine) entityContextFor(propertyKey string) string { diff --git a/mdl/visitor/visitor_alter_page.go b/mdl/visitor/visitor_alter_page.go index 10cb31be5..ae3198dd7 100644 --- a/mdl/visitor/visitor_alter_page.go +++ b/mdl/visitor/visitor_alter_page.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" + "github.com/antlr4-go/antlr/v4" + "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/grammar/parser" ) @@ -141,6 +143,12 @@ func (b *Builder) buildAlterPageAssignment(ctx *parser.AlterPageAssignmentContex } } if expr := ctx.Expression(); expr != nil { + // `$Param.Attr` binds an attribute property to a page or snippet + // parameter (ako/mxcli#1622); the executor, which knows the + // property's kind, resolves or refuses it. + if text := stripExpressionIdentifierQuotes(ruleSourceText(expr.(antlr.ParserRuleContext))); ast.IsParameterAttributeText(text) { + return name, &ast.WidgetExpressionV3{Text: text} + } b.addError(widgetExpressionNotAllowed(name, expr)) return "", nil } diff --git a/mdl/visitor/visitor_alter_set_parameter_attribute_test.go b/mdl/visitor/visitor_alter_set_parameter_attribute_test.go new file mode 100644 index 000000000..d43a07e27 --- /dev/null +++ b/mdl/visitor/visitor_alter_set_parameter_attribute_test.go @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// ako/mxcli#1622: `set (eventDataAttribute: $Ctx.EventData) on chart1` was a +// parse error — "takes a plain value, not an expression" — so an attribute of +// a page parameter could not be bound by ALTER at all. `$Param.Attr` is the +// spelling CREATE takes for that binding; it now reaches the executor, which +// knows the property's kind and decides. Any longer expression is refused here +// as before. +func TestAlterSet_ParameterAttributeReachesTheExecutor(t *testing.T) { + for src, want := range map[string]string{ + `$Ctx.EventData`: "$Ctx.EventData", + `$Ctx."Type"`: "$Ctx.Type", // identifier quotes stripped, as in CREATE + } { + prog, errs := Build(`alter page M.P { set (eventDataAttribute: ` + src + `) on chart1; };`) + if len(errs) > 0 { + t.Errorf("%s: %v", src, errs) + continue + } + op := prog.Statements[0].(*ast.AlterPageStmt).Operations[0].(*ast.SetPropertyOp) + got, ok := op.Properties["eventDataAttribute"].(*ast.WidgetExpressionV3) + if !ok || got.Text != want { + t.Errorf("%s: value = %#v, want *ast.WidgetExpressionV3{Text: %q}", src, op.Properties["eventDataAttribute"], want) + } + } + + // Controls: an expression is still refused at parse. + for _, src := range []string{`$Ctx.EventData + 'x'`, `$Ctx/EventData`} { + _, errs := Build(`alter page M.P { set (eventDataAttribute: ` + src + `) on chart1; };`) + if len(errs) == 0 || !strings.Contains(errs[0].Error(), "takes a plain value, not an expression") { + t.Errorf("%s: want the plain-value refusal, got %v", src, errs) + } + } +}