diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-commit-drops-nested-folders-named-like-build-folders.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-commit-drops-nested-folders-named-like-build-folders.json new file mode 100644 index 0000000000..b6fbea8c48 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-commit-drops-nested-folders-named-like-build-folders.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-28", "symptom": "`mxcli git commit` silently leaves a user file out of the commit when any folder on its path is named like a build folder: `resources/deployment/endpoints.xml` (or `javascriptsource/theme-cache/x.js`) stays untracked after the commit, with only `Left out 1 file(s) Studio Pro never commits (project-settings.user.json, *.mpr.lock, caches)` to show for it", "cause": "isJunkPath matched `.mendix-cache`, `deployment`, `theme-cache` and `.mxcli` on every path segment, so any folder of that name at any depth was dropped. Those are the project's own folders, the ones Studio Pro, mxbuild and mxcli create directly beside the .mpr; the file-name rules (lock, backup, user settings) are the ones that rightly match at any level", "file": "`cmd/mxcli/cmd_git_commit.go` (`isJunkPath(rel, projDir)`, projDir from `path.Dir(gitNoteRepo.MPRPath)`, which already resolves the project folder inside the repository, symlinks included); tests `TestGitCommit_NestedFolderNamedLikeJunkIsCommitted`, `TestGitCommit_JunkAnchoredAtProjectInSubfolder` in `cmd/mxcli/gitdoctor_test.go`", "insight": "**Anchor a path rule where the thing it names is created**: the build and cache folders only ever appear beside the .mpr, and a rule that matches the name at every depth also catches user folders that share it. **The report that something was left out did not name it**, which is what made the drop silent; a nested-folder fixture next to the root one (the existing test only had root `deployment/`) is the control that tells the two apart. Test the project-in-a-subfolder layout too: the anchor is the project's folder, not the repository root, and the same names outside the project are not its own", "refs": ["ako/mxcli#1670", "mendixlabs/mxcli#1669"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-commit-dry-run-unstages-the-users-staging.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-commit-dry-run-unstages-the-users-staging.json new file mode 100644 index 0000000000..80af6632b5 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-commit-dry-run-unstages-the-users-staging.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-28", "symptom": "`mxcli git commit -m … -p app.mpr --dry-run` unstages everything the user had staged: a file added with `git add` before the dry run is no longer staged afterwards (`git diff --cached` is empty), although the command committed nothing and documents that a dry run writes nothing", "cause": "The dry run staged everything in the real index (`git add -A`) to report it, then undid that with a plain `git reset -q`, which resets the index to HEAD rather than to what it was before the run", "file": "`cmd/mxcli/cmd_git_commit.go` (`commitRun`: a dry run stages into a temporary copy of the index through GIT_INDEX_FILE, `indexCopy`); `cmd/mxcli/gitnote.go` (`gitOutInEnv`); test `TestGitCommit_DryRunKeepsTheUsersStaging` in `cmd/mxcli/gitdoctor_test.go`", "insight": "**The existing dry-run test started from an empty index**, so 'nothing staged afterwards' was both the correct result and the bug's result; a test of an undo has to start from a state the undo can get wrong. **Do not undo, isolate**: staging into a GIT_INDEX_FILE copy leaves the real index untouched byte for byte (stat data and intent-to-add entries included), which no reset or write-tree/read-tree round trip quite does. An empty file is not a valid index, so with no index yet the temporary path is left free for git to create", "refs": ["ako/mxcli#1670", "mendixlabs/mxcli#1669"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-doctor-judges-notes-against-an-unfetched-ref.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-doctor-judges-notes-against-an-unfetched-ref.json new file mode 100644 index 0000000000..0e730ec6a9 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-doctor-judges-notes-against-an-unfetched-ref.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-28", "symptom": "`mxcli git doctor` on a fresh clone (or any CI checkout) reports every commit as `[ ] without a note` and exits 1, while the same report says `[x] origin has refs/notes/mx_metadata`; `--fix` then writes computed notes over commits the remote already annotates, and the `git push origin refs/notes/mx_metadata` it tells you to run is rejected as non-fast-forward", "cause": "doctorNotes judged each commit against the LOCAL refs/notes/mx_metadata only, and nothing fetched it: git clone and a plain git fetch never fetch refs/notes/*. The remote check had already run `git ls-remote refs/notes/mx_metadata` and printed the tip, but that result was not passed on, so an empty local ref read as 'no commit has a note'", "file": "`cmd/mxcli/gitdoctor.go` (`doctorRun` passes the remote's notes commit to `doctorNotes`; `hasNotesCommit`: the local ref is, or descends from, that commit); test `TestGitDoctor_UnfetchedNotesAreNotMissing` in `cmd/mxcli/gitdoctor_test.go`", "insight": "**Every existing notes test built its repository with `git init`, never `git clone`**, so the local notes ref was always the only one there was; the defect lives exactly in the gap between a repository that writes notes and one that receives them. A fixture with a bare remote, a pushed note and a fresh clone reproduced it on the first run. **Do not judge, and do not write, against a ref that is behind the remote**: Studio Pro's own fetch force-replaces the local notes ref with the remote's, so the remote's notes are the ones that count; a note written over a remote-annotated commit is either refused on push or thrown away on Studio Pro's next fetch. **Fetch as a fast-forward only** (`git fetch refs/notes/mx_metadata:refs/notes/mx_metadata`, no `+`): it never discards a note that exists only locally, which a force-fetch would; when the local ref has diverged the fetch is refused and the doctor says so instead of writing. Report mode stays read-only: it names the unfetched ref and the command rather than fetching", "refs": ["ako/mxcli#1670", "mendixlabs/mxcli#1669"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-doctor-judges-the-insteadof-rewritten-remote-url.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-doctor-judges-the-insteadof-rewritten-remote-url.json new file mode 100644 index 0000000000..608387c55c --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-doctor-judges-the-insteadof-rewritten-remote-url.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-28", "symptom": "With a global `url.git@github.com:.insteadOf https://github.com/` rule (a common developer setup), `mxcli git doctor` reports an HTTPS remote as `origin: git@github.com:org/app.git — SSH; Studio Pro needs HTTPS`, exits 1, and `--fix` \"rewrites\" the configured URL to HTTPS on every run (dropping its `.git` suffix) without changing anything git uses", "cause": "The remote check read `git remote get-url`, which applies url..insteadOf, and --fix set `remote..url`, which the rule rewrites again on the next read, so the check could never pass. The same read made the two SSH tests depend on the developer's and the sandbox's git config (ako/mxcli#1765 isolated the tests; the product question stayed open)", "file": "`cmd/mxcli/gitdoctor.go` (`doctorRun`: judge `git config --get remote..url`; report a differing `git remote get-url` as an [i] insteadOf line); tests `TestGitDoctor_InsteadOfRewriteIsNotAnSSHRemote`, `TestGitDoctor_ConfiguredSSHRemoteFoundBehindRewrite` in `cmd/mxcli/gitdoctor_test.go`", "insight": "**A check and its fix must read and write the same thing.** `remote get-url` is the effective URL, `remote set-url` writes the configured one; with a rewrite in between, a fix can never make its own check pass, and a loop like that is the tell. Judge what you can change (the configured URL) and report what you cannot (the rule, which applies to every git command on the machine). **Put the rule in the test repository's own config and use a host nobody rewrites** (git.example.com): the test then shows the behaviour without inheriting the developer's or the CI sandbox's rewrites of github.com, which point in opposite directions. Set GIT_SSH_COMMAND=false when the effective URL is SSH, or ls-remote reaches for the network", "refs": ["ako/mxcli#1670", "ako/mxcli#1765"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-doctor-tests-read-ambient-insteadof.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-doctor-tests-read-ambient-insteadof.json new file mode 100644 index 0000000000..85b1854a18 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-git-doctor-tests-read-ambient-insteadof.json @@ -0,0 +1 @@ +{"area":"cmd/mxcli","date":"2026-10-28","symptom":"`make test` failed on main right after #1670 (git doctor) merged, in a hosted sandbox only: TestGitDoctor_SSHRemoteRewrittenByFix and TestGitDoctor_SSHURLFormRewrittenByFix reported `remote url = \"https://github.com/org/app.git\\n\", want https://github.com/org/app`; CI was green","cause":"`git remote get-url` applies url..insteadOf, and the tests ran git under the developer's own configuration. The sandbox sets GIT_CONFIG_COUNT/KEY/VALUE rewriting git@github.com: and ssh://git@github.com/ to https://github.com/, so the SSH remote the test added already read back as HTTPS and --fix never touched it; a ~/.gitconfig rewriting https://github.com/ to git@github.com: (the usual developer setup) fails the same two tests the other way, `git@github.com:org/app`","file":"`cmd/mxcli/gitdoctor_test.go` (`doctorEnv`: GIT_CONFIG_GLOBAL=os.DevNull, GIT_CONFIG_NOSYSTEM=1, GIT_CONFIG_COUNT=0)","insight":"A test that shells out to git inherits three configuration sources besides the repository's — system, global, and the GIT_CONFIG_COUNT environment — and isolating HOME or the identity variables covers none of the third. Reproduce each source separately, because they fail in opposite directions: env rewrite on this host, then `env -u GIT_CONFIG_* HOME=git@ insteadOf>`, with an empty HOME as the passing control. Both failed before the fix and pass after. Not fixed, and a product question rather than a test one: with a global https->git@ rewrite, `git doctor` reads the effective URL, so it reports SSH and `--fix` sets an HTTPS URL that the rewrite turns straight back into SSH, every run","refs":["ako/mxcli#1670"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-legacy-runner-later-expect-pass-clears-earlier-fail.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-legacy-runner-later-expect-pass-clears-earlier-fail.json new file mode 100644 index 0000000000..42148292a5 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-legacy-runner-later-expect-pass-clears-earlier-fail.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-28", "symptom": "On the after-startup runner (every Docker run, and `--local --legacy-runner`) a test with more than one `@expect` is reported PASS when an earlier assertion fails and a later one passes, and `mxcli test` exits 0. Measured at unit level: log FAIL:test_1:Expected $r = 1, actual: 2 then PASS:test_1 gave status=PASS message=\"\" AllPassed=true. PASS then FAIL reported correctly; the endpoint runner (`--local`, `--attach`, `--watch`) was never affected", "cause": "`writeExpectAssertion` logs a verdict line per assertion (MXTEST:PASS or MXTEST:FAIL), and `ParseLogResults` treated every line as the test's verdict: each case overwrote Status, and the PASS case also cleared Message, so the last line won. The generated flow did compute `$AllPassed = false` and return it — nothing reads that value", "file": "`cmd/mxcli/testrunner/results.go` (`ParseLogResults`, new `applyVerdictLine`)", "insight": "A log protocol that emits several partial verdicts per test needs a monotonic fold, not last-write-wins: FAIL/ERROR is sticky, the first failure's message is reported (the endpoint runner's semantics — its assertions run only while $Verdict is PASS and its skipped-assertions guard never replaces a failure), and further failures are counted onto the message. The plausible wrong turn is 'first verdict line wins': the skipped-assertions guard (#1448) logs its FAIL AFTER the PASS lines of the assertions that did run, so first-wins turns that false green back on — sticky failure, not first line, is the rule. Fixed in the parser rather than the generator: it also corrects logs from runners generated before the fix, and a one-verdict-per-test generator would change a flow shape whose verdicts were measured on 11.13.0 for no additional coverage, since parser and generator ship in one binary. A RUN line resets the fold so a failure cannot outlive the run it was logged in. Second time this parser took a verdict from the wrong line (after #1448's RUN-without-verdict read as the zero status, PASS): when touching `ParseLogResults`, enumerate the line ORDERS the generator can produce per test, not just the line kinds", "refs": ["ako/mxcli#1753", "ako/mxcli#1448"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-legacy-runner-lower-case-call-variable-not-renamed.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-legacy-runner-lower-case-call-variable-not-renamed.json new file mode 100644 index 0000000000..cd1b375027 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-28-legacy-runner-lower-case-call-variable-not-renamed.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-28", "symptom": "On the after-startup runner (Docker, `--local --legacy-runner`) two tests in one file that both assign the same variable with a lower-case keyword — `$r = call microflow …`, mdl 1's and `fmt`'s spelling — fail injection with MDL063 (CE0111 \"Duplicate variable name\" from mxbuild); the same file runs under the endpoint runner, which gives each test its own flow", "cause": "The legacy runner compiles every test into one microflow and renames each test's variables with a `_N` suffix, but which variables to rename was decided by `assignPattern`, which matched only upper-case `CALL|CREATE` — the spelling of the runner's own early examples. A lower-case assignment was left unrenamed and declared once per test", "file": "`cmd/mxcli/testrunner/generator.go` (`assignPattern`, now `(?mi)…(?:call|create)\\b`)", "insight": "The cheapest detector was a sweep, not a reading: every `.test.mdl` in the repo through `GenerateTestRunner` and `executor.ValidateProgram` — 10 of the 16 in mdl-examples/ drew MDL063 before the fix, 2 after. Those 2 (`retrieve $Brands`, `retrieve $Request` + `$Request = head(…)`) are the remaining, separate gap: `assignPattern` never matched `retrieve $x`, `declare $x`, list operations (`$x = filter(…)`), `send rest request` or `execute database query` in ANY case, so a variable those define in two tests still collides; the durable fix is to take the declared names from the parsed body (`visitor.FlowStatements`, as `attachOnError` already does), not a regex. Two existing tests (`TestThrowsCatchesEveryCallKind`, `TestVerdictCatchesEveryCallKind`) matched the legacy output against the unrenamed lower-case statement and passed only because of the defect — a test that asserts a generator's text verbatim can encode the bug it should catch. The `\\b` matters once the match is case-insensitive (`$r = callback`). Found alongside ako/mxcli#1753, whose bug-test therefore uses a distinct variable per test", "refs": ["ako/mxcli#1753"], "rules": ["MDL063"], "ce": ["CE0111"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-28-module-role-bare-create-or-modify-erases-description.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-28-module-role-bare-create-or-modify-erases-description.json new file mode 100644 index 0000000000..cae8652875 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-28-module-role-bare-create-or-modify-erases-description.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-28","symptom":"A bare `create or modify module role M.R;` (no description clause) reported `Modified module role: M.R` and erased the role's stored Description; `describe` → `exec` was unaffected because describe prints the description, and grants (on the entity, keyed by role name) were untouched.","cause":"CreateModuleRoleStmt.Description was a plain string, so 'no clause' and `description ''` were the same value, and execCreateModuleRole's OR MODIFY branch passed it to Backend.AddModuleRole, which overwrites the existing role in place.","file":"mdl/ast/ast_security.go (Description *string), mdl/visitor/visitor_security.go, mdl/executor/cmd_security_write.go (execCreateModuleRole)","fix":"Description is a *string, nil when unstated, as CreateUserRoleStmt's properties have been since ako/mxcli#707; OR MODIFY writes the stored description when it is nil, so the write is elided and the run reports Unchanged. `description ''` clears it. Adopting an auto-provisioned role still drops its marker description.","insight":"A neighbouring statement already had the right shape: the user-role code even said 'the rest keep their stored values, as a module role's missing description does', which was false — a comment asserting a sibling's behaviour is a claim to test, not evidence. The tell for this class is an optional clause parsed into a value type: grep the AST for optional clauses stored as string/bool without a *T or a ...Set flag, then follow each to an OR MODIFY branch. mx check is 0 errors with the fault (measured, 11.13.0), and describe → exec cannot show it because describe restates the value; only a hand-written minimal statement does.","refs":["ako/mxcli#1218"],"test":"mdl/executor/issue1218_module_role_description_test.go, mdl/visitor/visitor_security_test.go"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-28-published-odata-is-a-10-4-document.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-28-published-odata-is-a-10-4-document.json new file mode 100644 index 0000000000..87ed75f192 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-28-published-odata-is-a-10-4-document.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-28", "symptom": "On Mendix 9.x `create published odata service` was refused at write time with `this Mendix 9.24.30 project cannot store ODataPublish$PublishedODataService2.IncludeMetadataByDefault (Mendix 10.8.0+)` (10-odata and 18-folder on 9.24), after the earlier statements of the script were already written", "cause": "mxcli writes a published OData service as ODataPublish$PublishedODataService2, a document type introduced in 10.4.0; before it the service is Rest$PublishedODataService (deleted 10.4.0), a different document with its own children. The generated version data has no type-level floor, so the property floor could only trip over the first versioned key and named the wrong thing", "file": "`mdl/executor/version_gates.go` (CreateODataServiceStmt gate), `sdk/versions/mendix-10.yaml` / `mendix-11.yaml` (integration.published_odata_service 10.4.0)", "insight": "**Before lowering a refused property, check the type it sits on exists in that version.** Lowering IncludeMetadataByDefault was the obvious fix and would have made it worse: with the floor off, mx check 9.24.30 stops loading the project at all (`TypeCacheUnknownTypeException: The type cache does not contain a type with qualified name ODataPublish$PublishedODataService2`). mendixmodelsdk says it directly: `PublishedODataService2.versionInfo = {introduced: \"10.4.0\"}`, `rest.PublishedODataService` `deleted: \"10.4.0\"`. The generated tables carry property floors only, so a whole-type floor has to be a version gate (check-time, MDL-VERSION01, before anything is written) — the same gap lets Workflows$EndOfParallelSplitPathActivity (10.21) through on 9.x. Measured: 10-odata and 18-folder on 9.24 now fail at the gate (no write); on 10.24.24 and 11.13 both PASS. Tests: TestCheckVersionGates_ReportsEveryHandlerGate (published odata on 9.24.30 and 10.3), TestCheckVersionGates_UngatedFormsPass (10.4 passes); mdl-examples/bug-tests/1163-published-odata-below-10-4.mdl", "refs": ["#1163", "#1434"]} diff --git a/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-aggregate-function-floored-at-mendix10.json b/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-aggregate-function-floored-at-mendix10.json new file mode 100644 index 0000000000..bac85dc0a6 --- /dev/null +++ b/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-aggregate-function-floored-at-mendix10.json @@ -0,0 +1 @@ +{"area": "modelsdk", "date": "2026-10-28", "symptom": "On Mendix 9.x every count/sum/average/minimum/maximum in a microflow was refused: `this Mendix 9.24.30 project cannot store Microflows$AggregateAction.AggregateFunction (Mendix 10.0.0+)` (02-microflow-examples on 9.24)", "cause": "The vendored generated version data (modelsdk/gen/microflows/version.go) gave aggregateFunction the row of reduceReturnDataType ({Introduced 10.0.0, Required}) and had no row for reduceReturnDataType, so the floor refused the one key every aggregate has and let the fold's return type through unrefused", "file": "`modelsdk/gen/microflows/version.go` (Microflows$AggregateAction, fixed by hand), `modelsdk/version/metamodel_versions_gen.go` (go generate ./modelsdk/version)", "insight": "**A refusal can be the floor's data being wrong, not the writer's shape.** The prompt to look for an 'older shape' was a guess; evidence settled it in two steps: mendixmodelsdk 4.117.0 has `aggregateFunction: {}` (no version) and even sets its default differently either side of 9.24.0, and Studio Pro-authored 9.24.0 bytes (Workflow Commons 2.6.0 from the Marketplace, `mxcli marketplace download 117066 --version 2.6.0`, project.mpr inside the .mpk) store `AggregateFunction: \"Count\"` on 10 of 10 aggregates. A one-off script comparing every SDK StructureVersionInfo `introduced` with modelsdk/gen/*/version.go found only 4 mismatches in the whole table (this one; CustomWidgets$WidgetReturnType.type 9.20 vs none; Forms$GroupBox.caption 8.10 vs none; Workflows$CallMicroflowTask.boundaryEvents), so the generator's misattribution is rare, not systematic. **The cheapest 'which keys does version X declare' oracle is a floor-off build + `mx convert -p` on that version**: build mxcli with StripUndeclaredProperties stubbed, exec, dump units, convert, dump again, diff key sets per $Type — 9.24's convert kept all of AggregateAction's keys. Measured: 5 aggregates on a fresh 9.24.30 app → exec ok, mx check 0 errors, mx convert strips nothing (before: refused). Tests: TestNormalize_AggregateFunctionIsDeclaredOnMendix9, TestNormalize_ReduceFoldIsRefusedBelowMendix10; mdl-examples/bug-tests/1163-aggregate-on-mendix9.mdl", "refs": ["#1163"]} diff --git a/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-constant-override-flat-below-10-9.json b/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-constant-override-flat-below-10-9.json new file mode 100644 index 0000000000..27ca2e65ac --- /dev/null +++ b/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-constant-override-flat-below-10-9.json @@ -0,0 +1 @@ +{"area": "modelsdk", "date": "2026-10-28", "symptom": "On Mendix 9.x and 10.0–10.8 a new constant override was refused: `alter settings constant @M.C value '…' in configuration 'Default'` → `this Mendix 9.24.30 project cannot store Settings$ConstantValue.SharedOrPrivateValue (Mendix 10.9.0+)` (14-project-settings on 9.24)", "cause": "settingsoverlay writes a NEW override in the nested shape (Settings$SharedValue under SharedOrPrivateValue), which is the only shape 10.9+ reads; below 10.9 the override is `{$ID, $Type, ConstantId, Value}` with a flat string. The overlay already kept an existing flat override flat, so only a new one failed", "file": "`modelsdk/canon/versionshape.go` (LowerToProjectShape: lowerConstantValue), `mdl/settingsoverlay/settingsoverlay.go` (comment)", "insight": "**`mx convert` on the older version is a silent-data-loss detector, not just a key oracle**: given the nested form, 9.24.30 kept the override but emptied it (`Value: \"\"`) and still reported success — an unlowered write that got past the floor would have shipped an empty override with mx check at 0 errors. Measure the VALUE after convert, not only the key set. A private value (Settings$PrivateValue, workstation-local) has no flat form and is refused naming the constant. The reader (settings_read.go constantValueOf) already preferred the flat Value, so no reader change was needed — TestProjectSettings_Mendix9FlatConstantValue pins it. Measured: bug-test on a fresh 9.24.30 app → describe settings prints 'broker:9092', mx check 0 errors, mx convert keeps the value and changes no key in the settings unit. 14-project-settings 9.24 FAIL→PASS; 10.24.24 and 11.13 PASS. Tests: TestNormalize_ConstantValueIsFlatBelowMendix10_9, TestNormalize_RefusesAPrivateConstantValueBelowMendix10_9; mdl-examples/bug-tests/1163-constant-override-on-mendix9.mdl", "refs": ["#1163", "#801"]} diff --git a/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-flow-argument-expression-below-10-10.json b/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-flow-argument-expression-below-10-10.json new file mode 100644 index 0000000000..78c6b027a7 --- /dev/null +++ b/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-flow-argument-expression-below-10-10.json @@ -0,0 +1 @@ +{"area": "modelsdk", "date": "2026-10-28", "symptom": "On Mendix 9.x and 10.0–10.9 a page passing `$currentObject` to a microflow or nanoflow was refused: `this Mendix 9.24.30 project cannot store Forms$MicroflowParameterMapping.Expression (Mendix 10.10.0+)` (29-datagrid DG12 on 9.24)", "cause": "bindParameterMappingValue writes an argument that is not a page/snippet parameter or widget as Expression text with Variable null. Expression is 10.10.0; before it a mapping holds only Variable (a Forms$PageVariable, required before 10.10), and Studio Pro 9 does not store an argument that is the enclosing object at all — it infers it", "file": "`modelsdk/canon/versionshape.go` (LowerToProjectShape: lowerFlowArguments on any ParameterMappings list)", "insight": "**The floor's refusal hid a worse failure**: with the floor off, 9.24 does not load the project at all — `An error occurred when trying to set the 'Variable' property of a Microflow parameter mapping … ArgumentNullException` — so dropping only Expression would have shipped an unloadable page. **'Older shape' can be 'nothing stored'**: the 9.24 starter app and Workflow Commons 2.6.0 (9.24.0, from the Marketplace) have 82 flow calls taking 1–2 parameters and NOT ONE mapping (count calls against the called flow's parameters; an empty list alone proves nothing). So below 10.10 `$currentObject` (or an empty argument) is left out, a Variable-bound one keeps its Variable (the floor drops the empty Expression), and any other expression is refused naming the parameter. Readers needed nothing: describe already prints a call with no mappings as `call microflow M.F`, and re-executing that on 9.24 is Unchanged. Seen in passing: mxcli writes a populated Forms$CallNanoflowClientAction.ParameterMappings list with marker 3 (no RegisterListMarker for NanoflowParameterMapping), Studio Pro 2. Measured: bug-test on a fresh 9.24.30 app → exec ok, mx check 0 errors, mx convert changes no key; describe → exec Unchanged. 29-datagrid on 9.24 now stops at DG15 page variables (10.17, correct); 10.24.24 and 11.13 PASS. Tests: TestNormalize_FlowArgumentsBelowMendix10_10, TestNormalize_RefusesAnExpressionArgumentBelowMendix10_10; mdl-examples/bug-tests/1163-flow-arguments-on-mendix9.mdl", "refs": ["#1163", "#1140"], "ce": ["CE1571"]} diff --git a/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-workflow-persistentid-below-10-21.json b/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-workflow-persistentid-below-10-21.json new file mode 100644 index 0000000000..52ce2c428a --- /dev/null +++ b/.claude/skills/fix-issue/findings/modelsdk/2026-10-28-workflow-persistentid-below-10-21.json @@ -0,0 +1 @@ +{"area": "modelsdk", "date": "2026-10-28", "symptom": "On Mendix 9.x and 10.0–10.20 every workflow was refused: `this Mendix 9.24.30 project cannot store Workflows$Workflow.PersistentId (Mendix 10.21.0+)` (25-rename on 9.24)", "cause": "The workflow writer (and the ALTER WORKFLOW mutator, for the activities it inserts) mints a fresh PersistentId on the workflow and on every activity, outcome and boundary event. PersistentId is 10.21.0 on all four (Workflow, and the abstract WorkflowActivity, Outcome, BoundaryEvent). The floor refused the workflow's own and never saw the rest, because it does not look up abstract supertypes", "file": "`modelsdk/canon/versionshape.go` (LowerToProjectShape: dropPersistentID on every Workflows$ element below 10.21)", "insight": "**An identity the writer mints is not a value the statement said**, so below its version it is dropped, not refused (unlike ForceFullObjects, which a script can set). Lower it in canon.LowerToProjectShape rather than encodeWorkflow: wfmutator's raw inserts (mutator.go: PersistentId on inserted activities, outcomes, boundary events) never pass encodeWorkflow, but every write passes normalizePropertySetsFor. **Test with a non-zero GUID**: a zero GUID is an empty value the floor silently drops, so the first draft of the test passed on the unfixed code. **The floor-off build + `mx convert -p` 9.24 key diff** showed the whole picture at once: PersistentId on 6 element types, plus the empty keys the floor already handles. The same run found the next gap: a `parallel split` writes Workflows$EndOfParallelSplitPathActivity (a 10.21 type) and 9.24 does not load the project (TypeCacheUnknownTypeException) — not fixed here. Measured: bug-test on a fresh 9.24.30 app → exec ok, mx check 0 errors, mx convert strips no key (adds WorkflowOnStateChangeEvent/UsertaskOnStateChangeEvent null). 25-rename 9.24 FAIL→PASS; 25-rename and 24b on 10.24.24 and 11.13 PASS. Tests: TestNormalize_WorkflowPersistentIDsBelowMendix10_21; mdl-examples/bug-tests/1163-workflow-on-mendix9.mdl", "refs": ["#1163"]} diff --git a/.claude/skills/mendix/manage-security/SKILL.md b/.claude/skills/mendix/manage-security/SKILL.md index 9f294a4333..e75335f11b 100644 --- a/.claude/skills/mendix/manage-security/SKILL.md +++ b/.claude/skills/mendix/manage-security/SKILL.md @@ -13,11 +13,11 @@ description: "Configure Mendix security in MDL — module and user roles, access | 27-35 | When to Use This Skill | | 36-43 | Security Concepts | | 44-120 | The System-module ceiling — decide the role model around this FIRST | -| 121-558 | Syntax Reference | -| 559-573 | Starlark Lint Rule APIs | -| 574-626 | Common Workflow: Setting Up Module Security | -| 627-636 | Common Mistakes | -| 637-646 | Validation | +| 121-560 | Syntax Reference | +| 561-575 | Starlark Lint Rule APIs | +| 576-628 | Common Workflow: Setting Up Module Security | +| 629-638 | Common Mistakes | +| 639-648 | Validation | # Security Management Skill @@ -205,7 +205,9 @@ create module role MyModule.Viewer description 'Read-only access'; -- `or modify` updates an existing role's description instead of failing, so the -- whole security script stays re-runnable rather than needing a run-once file. +-- Without `description` it keeps the stored one; `description ''` clears it. create or modify module role MyModule.ApiUser description 'API consumer'; +create or modify module role MyModule.Admin; -- ensures it exists, keeps its description -- Remove a module role (`if exists` makes it a no-op when the role is gone) drop module role MyModule.Viewer; diff --git a/.claude/skills/steward/SKILL.md b/.claude/skills/steward/SKILL.md new file mode 100644 index 0000000000..76b305efe0 --- /dev/null +++ b/.claude/skills/steward/SKILL.md @@ -0,0 +1,121 @@ +--- +name: steward +description: Drive an mxcli pull request Claude opened to green and merge it. Covers when to open it, the local gate to pass before every push (make ci-local), which CI failure is a known runner flake, how to resolve the conflicts this repo keeps hitting, how to read the AI PR Review comment, and the merge-on-green default. Read before acting on a CI, review or conflict event on such a PR. +--- + +## Find it fast + +`sed -n ',p' SKILL.md` one section rather than reading the file; +`./mxcli syntax ` answers *how do I write this* for a fraction of either. + +| lines | section | +|---|---| +| 27-54 | Merge on green is the default | +| 55-72 | Before every push: `make ci-local` | +| 73-87 | Reading a red CI job | +| 88-103 | Conflicts this repo keeps hitting | +| 104-121 | The AI PR Review comment | + + +# Steward: Driving a PR to Merged + +This is a **contributor** skill (dev-only); it is not synced to user projects. +It holds this repository's conventions for the loop after a PR is opened. How +to make the change itself is `/mxcli-dev:fix-issue` and the CLAUDE.md working +rules; this file starts where they stop. + +## Merge on green is the default + +The user reviews a change **in the session, before the PR exists**. So: + +- Open a PR only after the user has seen the change summarized in the session + and asked for it. Never open one to get a review. +- Once it is open, merge it without asking again as soon as all of these hold + on the **current head**: + - every job of `Build, Test & Lint` (`.github/workflows/push-test.yml`) is + green, `integration-passed` included. A push to the PR branch runs that + workflow twice on the same commit, for the push and the pull_request + event; a job that passed in one run and was cancelled before it got a + runner in the other counts as green; + - GitHub reports no merge conflict; + - no human review thread or changes-requested review is open; + - no Critical item from the AI PR Review comment is unanswered (see below). +- Merge with a **merge commit** (not squash, not rebase), passing the head SHA + you checked as `expectedHeadSha`, so a push that lands in between is not + merged unseen. +- After the merge, unsubscribe from the PR, cancel its pending check-in, and + restart the working branch from the new main: + `git fetch origin main && git checkout -B origin/main`, then push. + The next PR starts from there; never stack new work on merged history. + +Ask instead of merging when a human reviewer has commented or requested +changes, or when the PR is someone else's. Never merge a red head, whatever +the reason it is red. + +## Before every push: `make ci-local` + +`make build test lint` is a subset of the gate CI applies. `make ci-local` +runs what the `build-and-test` job runs, in its order, minus `govulncheck`. +Run it before the first push and before every push that changes code, and +push only on a clean exit. It takes a while; run it in the background. + +- **Formatting:** `make fmt` is the fixer. `fmt-check` uses the toolchain's + gofmt (`go env GOROOT`), because the gofmt on `PATH` can be an older Go than + the `toolchain` line in `go.mod`, and its verdict differs from CI's. +- **Integration suites** need mxbuild, so `ci-local` does not run them. When + the change touches a package with integration tests, run that suite: + `make test-integration-`. +- **When the job gains a step,** add it to `ci-local` in the same PR. +- **A failure in code the change does not touch:** run the same target on + `origin/main`. If it fails there too, it is main's: fix it in its own PR + (it can go first), and never skip or disable the test to get past it. + +## Reading a red CI job + +The workflow is the source of truth for what runs; these are the jobs whose +failures need reading differently. + +- **`build-and-test`:** reproduce with `make ci-local`. +- **`integration ()`:** reproduce with `make test-integration-`. +- **`tunnel-seam-cross-platform (macos-latest | windows-latest)`:** the one + known runner flake. The hosted macOS queue sometimes cancels the job before + it gets a runner (the job shows runner id 0 and no step ran). That is a job + that died before any test body ran: re-run the failed jobs **once**. A test + failure in this job is real: it runs the tunnel seam's own tests on that + platform (ADR-0009). +- **`AI PR Review`** is always green: it posts a comment, it does not gate. + +## Conflicts this repo keeps hitting + +While the PR is open, merge `origin/main` into its head. Do not rebase or +force-push it. + +| File | Resolution | +|---|---| +| `CHANGELOG.md` `[Unreleased]` | Keep **both** sides: every entry is one line, so a conflict is two fixes landing at once. Afterwards check for a stray blank line or a duplicated entry. | +| A `SKILL.md` section index | Keep either side, then `make skill-index`. | +| `cmd/mxcli/skills/` | Never resolve by hand: `make sync-skills` regenerates it from `.claude/skills/mendix/`. | +| `docs-site/src/language/versions.md` tables | `make gen-migration-reference`. | +| `mdl/conformance/allowlist.txt` | Take the higher count on each conflicting line, then `make conformance-shrink`, which lowers every count to what the gate finds and never raises one. | +| Bug findings | They do not conflict: each finding is its own new file. | + +Run `make ci-local` on the merge result before pushing it. + +## The AI PR Review comment + +`ai-review.yml` posts **one PR comment** headed `## AI Code Review`, not review +threads. It has sections Critical Issues, Moderate Issues, Minor Issues, What +Looks Good and Recommendation. On a large PR the model sees only a selected +part of the diff, so a claim that something is missing may only mean that file +was outside the selection. + +| Section | How to treat it | +|---|---| +| Critical | Blocking. Verify each item against the code: fix and push it, or answer it with the path you traced and why it does not hold. | +| Moderate | A claim to verify, not an order. Fix it when a real caller reaches the failure and the fix is in proportion to it; otherwise say why not. | +| Minor | Optional. Never start a push for one. Carry the plainly correct ones into a push that already touches those files. | + +Answer it with **one** PR comment that gives the outcome for each Critical and +Moderate item. Sometimes nothing is posted at all: the workflow skips posting +when its API key is missing or the API call fails, and still reports success. +A missing comment is neither an approval nor a blocker. diff --git a/CHANGELOG.md b/CHANGELOG.md index ea7ab00ce5..3ff88359ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,12 +8,20 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added -- **`mxcli git commit -m … -p app.mpr` — commit the way Studio Pro does** (mendixlabs/mxcli#1669) — Studio Pro's *Commit* is more than `git commit`: it keeps the ContentsHash index in step, stages exactly the files a Mendix repository ships, and attaches the mx_metadata note. The new command is that sequence in one line: mismatched hashes are repaired from the files on disk first (`MISSING` and `ORPHAN` entries have no automatic repair and refuse the commit), everything is staged and the files Studio Pro never commits (`project-settings.user.json`, `*.mpr.lock`, `*.mpr.bak`, `.mendix-cache/`, `deployment/`, `theme-cache/`, `.mxcli/`) are unstaged again, `git commit` runs with the caller's message and identity, and the mx_metadata note is computed from the new commit and attached, so Studio Pro can version-check the revision and show what changed. `--push` pushes the branch and the notes ref together (`git push` alone loses the note the next time Studio Pro fetches); `--dry-run` stages, reports and unstages without committing — and without repairing anything, hash drift included, because a dry run writes nothing. The commit is refused while Studio Pro has the project open (`.mpr.lock`) — honouring the same `MXCLI_ALLOW_STUDIO_PRO_OPEN=1` override as every write — and during an unresolved merge, which `git add -A` would otherwise commit with its conflict markers. Docs: "Working Outside Studio Pro" › *One commit, all the steps*. -- **`mxcli git doctor [-p app.mpr] [--fix]` — audit a repository for everything a plain-git history misses, and repair it** (mendixlabs/mxcli#1669) — a repository committed only with git drifts from what Studio Pro expects in four ways, each invisible on its own: the local git config Studio Pro writes is absent (`core.autocrlf=False` keeps a checkout from rewriting line endings inside `.mxunit` files on Windows; `mendix.commits-since-gc` and `mendix.lineEndingResetDone` are its own bookkeeping), the remote is SSH which Studio Pro cannot use (HTTPS only), commits have no note under `refs/notes/mx_metadata` (history shows `(unknown)`, the version check fails), and a malformed note Studio Pro cannot read at all. `doctor` reports all four in one read-only pass and exits 1 while an issue remains, so it can gate CI. `--fix` sets the config keys, rewrites SSH remote URLs (`git@host:org/app.git`, `ssh://…`) to HTTPS, and backfills notes: computed from the model with `-p` (the same engine `git note` uses), or a template note with a `--version` ModelerVersion where the model at a commit cannot be read (MPR v1 history, a revision without the `.mpr`); with neither, the newest valid note in history supplies the version. Notes Studio Pro wrote are never touched, with or without `--fix`; `--fix` does not push. Docs: "Working Outside Studio Pro" › *Auditing a repository*. +- **`mxcli git commit -m … -p app.mpr` — commit the way Studio Pro does** (mendixlabs/mxcli#1669) — Studio Pro's *Commit* is more than `git commit`: it keeps the ContentsHash index in step, stages exactly the files a Mendix repository ships, and attaches the mx_metadata note. The new command is that sequence in one line: mismatched hashes are repaired from the files on disk first (`MISSING` and `ORPHAN` entries have no automatic repair and refuse the commit), everything is staged and the files Studio Pro never commits (`project-settings.user.json`, `*.mpr.lock`, `*.mpr.bak`, and the project's own `.mendix-cache/`, `deployment/`, `theme-cache/`, `.mxcli/` beside the `.mpr` — a folder of the same name further down, such as `resources/deployment/`, is committed) are unstaged again, `git commit` runs with the caller's message and identity, and the mx_metadata note is computed from the new commit and attached, so Studio Pro can version-check the revision and show what changed. `--push` pushes the branch and the notes ref together (`git push` alone loses the note the next time Studio Pro fetches); `--dry-run` reports what it would commit without committing — and without repairing anything, hash drift included, because a dry run writes nothing: it stages into a copy of the index, so what the user had staged is left as it was. The commit is refused while Studio Pro has the project open (`.mpr.lock`) — honouring the same `MXCLI_ALLOW_STUDIO_PRO_OPEN=1` override as every write — and during an unresolved merge, which `git add -A` would otherwise commit with its conflict markers. Docs: "Working Outside Studio Pro" › *One commit, all the steps*. +- **`mxcli git doctor [-p app.mpr] [--fix]` — audit a repository for everything a plain-git history misses, and repair it** (mendixlabs/mxcli#1669) — a repository committed only with git drifts from what Studio Pro expects in four ways, each invisible on its own: the local git config Studio Pro writes is absent (`core.autocrlf=False` keeps a checkout from rewriting line endings inside `.mxunit` files on Windows; `mendix.commits-since-gc` and `mendix.lineEndingResetDone` are its own bookkeeping), the remote is SSH which Studio Pro cannot use (HTTPS only), commits have no note under `refs/notes/mx_metadata` (history shows `(unknown)`, the version check fails), and a malformed note Studio Pro cannot read at all. `doctor` reports all four in one read-only pass and exits 1 while an issue remains, so it can gate CI. `--fix` sets the config keys, rewrites SSH remote URLs (`git@host:org/app.git`, `ssh://…`) to HTTPS — judged as configured, so a `url..insteadOf` rule in the user's git config (https → `git@` is a common one) is reported rather than "repaired" on every run — and backfills notes: computed from the model with `-p` (the same engine `git note` uses), or a template note with a `--version` ModelerVersion where the model at a commit cannot be read (MPR v1 history, a revision without the `.mpr`); with neither, the newest valid note in history supplies the version. Notes Studio Pro wrote are never touched, with or without `--fix`; `--fix` does not push. A clone whose notes ref lacks the remote's notes — `git clone` never fetches `refs/notes/*`, so every fresh clone and CI checkout — is reported as exactly that, with the fetch command, rather than as a history of unannotated commits; `--fix` fetches the notes first, as a fast-forward only. Docs: "Working Outside Studio Pro" › *Auditing a repository*. ### Fixed - **A navigation profile's application title and icon can be set and are described** (ako/mxcli#1756, mendixlabs/mxcli#1664) — the title (Studio Pro's *Application title*, `Mendix` in a new project) is what the runtime puts in every browser tab as `Mendix - `, and it had no MDL form: `create or modify navigation Responsive app title 'EWO' …` failed with "extraneous input 'app'", `describe navigation` printed neither it nor the *Application icon*, and `create or modify translations … for en_US` refuses the source language, so every generated app kept "Mendix" in the tab. `create or modify navigation` now takes `app title ''` and `app icon ` (`app icon none` clears it), in any order with the other clauses; `app` and `icon` stay usable as names. The title is written as the project default language's item, the title's other languages kept; the icon is resolved like any image reference, by `check --references` and before exec writes; an omitted clause keeps the stored value; describe prints both. A native profile has neither, so both are refused there. +- **A test whose earlier `@expect` fails is no longer reported PASS on the after-startup runner** (ako/mxcli#1753) — every Docker run and `--local --legacy-runner` logged a verdict line per assertion, and the result was taken from the last one: a failing first assertion followed by a passing second reported PASS, with the failure message erased, and `mxcli test` exited 0. A failure now sticks, and the test reports the first one, plus how many more failed. The endpoint runner (`--local`, `--attach`, `--watch`) was not affected. +- **Two tests that assign the same variable with a lower-case `call` or `create` run on the after-startup runner** (found with ako/mxcli#1753) — that runner compiles every test into one microflow and renames each test's variables, but it recognised only an upper-case `CALL` or `CREATE`. `$r = call microflow …`, the spelling mdl 1 and `fmt` write, was left as it was, so two tests assigning `$r` declared it twice and injection failed with MDL063 (CE0111 from mxbuild). 8 of the 16 test files in `mdl-examples/` hit this under Docker or `--legacy-runner`. A variable first assigned by `retrieve`, `declare` or a list operation is still not renamed there. +- **`create published odata service` on Mendix 9 and 10.0–10.3 is refused up front, naming the real reason** (ako/mxcli#1163) — it was refused at write time for "ODataPublish$PublishedODataService2.IncludeMetadataByDefault (Mendix 10.8.0+)". That property was not the problem: the whole document mxcli writes is a 10.4 type, and Mendix 9.24 cannot load a project that holds one. Before 10.4 a published OData service is a different document that mxcli does not write. `check -p` and `exec` now report MDL-VERSION01 before writing anything. +- **A page passing `$currentObject` to a microflow or nanoflow is written on Mendix 9 and on 10.0–10.9** (ako/mxcli#1163) — `call microflow M.F(P = $currentObject)` on a button or a data grid column was refused with "cannot store Forms$MicroflowParameterMapping.Expression (Mendix 10.10.0+)". An argument held as an expression arrived in 10.10. Before it, Studio Pro stores no argument for the enclosing object and infers it. Without the refusal, 9.24 could not even load the page, because the mapping had no `Variable`. Below 10.10 that argument is now left out, and a page parameter argument is stored as before. Any other expression argument is refused, naming the parameter. +- **A constant override in a configuration is written on Mendix 9 and on 10.0–10.8** (ako/mxcli#1163) — `alter settings constant @M.C value '…' in configuration 'Default'` was refused with "cannot store Settings$ConstantValue.SharedOrPrivateValue (Mendix 10.9.0+)". Before 10.9 an override's value is a flat `Value` string; Studio Pro 9.24 does not know the nested form and empties the override on load. A new override is now written flat there, and an existing one is updated in its own shape. A private value has no form before 10.9 and is refused, naming the constant. +- **A workflow written to Mendix 9.24 has Studio Pro's property set** (ako/mxcli#1163) — its two state-change event handlers (`WorkflowOnStateChangeEvent`, `UsertaskOnStateChangeEvent`, removed in 11.0) were completed only from 10.24, where they were measured. `mx convert` 9.24.30 adds both as well, and Mendix's merge engine refuses to compare documents whose property names differ (#1373). +- **Workflows are written on Mendix 9 and on 10.0–10.20** (ako/mxcli#1163) — every workflow was refused with "cannot store Workflows$Workflow.PersistentId (Mendix 10.21.0+)". Workflows exist from 9.0, but the persistent ID on a workflow, its activities, outcomes and boundary events arrived in 10.21. The activities' and outcomes' IDs were never refused, and `mx convert` 9.24 stripped them. mxcli mints these IDs itself, so below 10.21 it no longer writes them, for `create`, `create or modify` and the activities `alter workflow` inserts. A `parallel split` is still unwritable below 10.21: its paths end in an activity type that version does not have. +- **Aggregates in microflows are written on Mendix 9** (ako/mxcli#1163) — every `count`, `sum`, `average`, `minimum` and `maximum` on a 9.x project was refused with "cannot store Microflows$AggregateAction.AggregateFunction (Mendix 10.0.0+)". The version data had the aggregate's function introduced in 10.0, which belongs to the fold `reduce` added. Studio Pro 9.24 stores it on every aggregate. An aggregate over an expression, `reduce`, `all` and `any` are Mendix 10.0 and are still refused below it, now naming the fold. - **Pages on Mendix 9.24 and 10.x are read and written in the version's own shape** (ako/mxcli#1209) — several page properties are stored differently below a version: a design property's value before 10.2, a data view's page parameter before 10.0, its editability before 10.7, and a text box's placeholder before 10.11. `describe` read only the newer form, so on 9.24 a describe → exec round trip removed every design property and placeholder. Writing any page with a data view or a design property was refused, and `alter styling` on a stored design property reported success and changed nothing. Every write now converts to the project's form, and `describe` reads both. What the older form cannot hold is refused, naming the widget: a Spacing or colour design property, conditional editability, a placeholder with parameters. - **A text box's `KeyboardType` survives describe → exec** (ako/mxcli#1209) — the keyboard a native app opens had no MDL spelling and was written as `Default`, so a rewrite reset it. `textbox t (Attribute: Email, KeyboardType: EmailAddress)` (`Default`, `NumberPad`, `DecimalPad`, `EmailAddress`, `PhonePad`, `URL`); an unknown value is MDL-WIDGET18. - **A button's workflow action survives a rewrite** (ako/mxcli#832) — `describe` printed no action for Show user task page, Show workflow admin page or Call workflow, so re-executing a page turned them into no action (on ako/TestApp's TaskInbox and TaskDashboard, 2 of 6 user-task buttons each). They are now `open user task [with (AssignOnOpen, OpenWhenAssigned)]`, `open workflow with (DefaultPage: M.Page)` and `call workflow M.WF [with (…)]`. `complete task` takes `with (ClosePage, Commit)` instead of always writing both true. `open workflow` without a page is CE1834 and is refused. @@ -29,6 +37,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **`check -p --references` reports CE1406 for a retrieve, create or create list of a Hidden entity of a Protected module** (ako/mxcli#1699, mendixlabs/mxcli#1652) — a flow in an app module that ran `retrieve $l from AddOn.Internal`, `$o = create AddOn.Internal` or `$l = create list of AddOn.Internal` on a Hidden entity of a Protected (add-on) module passed `check` and `exec`, and mxbuild refused it: `[CE1406] "Retrieve object(s) activity 'Retrieve list of Internal from database' is not allowed to use entity 'AddOn.Internal' because app modules cannot reference hidden elements in add-on modules."` (likewise "Create object activity", "Create list activity"). The check judged an entity only as a parameter or return type; it now judges the entity these three activities name, in microflows and nanoflows, and `exec` refuses the script before writing it. Measured on Mendix 11.13.0: each shape refused against a Hidden entity, 0 errors against a Usable one and inside the add-on; an association retrieve that reaches a Hidden entity over a Usable association names only the association and is not refused. - **A pluggable widget's text-template parameter across an association builds, and describes so it re-executes** (mendixlabs/mxcli#1494) — `fieldset fs (legend: '{1}', legendParams: ({1} = M.Parent_Child/Label))` passed `check` and `exec` and failed the build with `[CE1365] "Attribute 'M.Child.Label' is not an attribute of entity 'M.Parent'."`: the parameter was written with its attribute but without the association hop, so mxbuild read the attribute against the widget's own entity. The same writer serves an object-list item's template (an Accordion group's `headerText`) and a Data grid 2 column's dynamic text, and all three failed; on Mendix 11.12.4 they now build with 0 errors. `describe page` printed such a parameter as the bare attribute (`{1} = Label`), which re-executed into CE1613 "The selected attribute 'M.Parent.Label' no longer exists"; it now prints `M.Parent_Child/Label`. The companion report, an expression parameter (`{1} = if 1 = 1 then 'A' else 'B'`) stored as an attribute name (mendixlabs/mxcli#1495), was fixed in v0.25.0 (#823) and is now pinned by a test. - **`alter entity … add event handler` adds a second handler for the same event** (mendixlabs/mxcli#1540) — `add event handler on before commit call M.Second($currentObject)` failed with "event handler already exists for Before Commit" when the entity already had one, though Studio Pro allows several handlers for one event and `create or modify entity` with two `on before commit` clauses stores both (measured on 11.12.4: two Before Commit handlers are 0 errors, a void-returning one is CE7247, so the check does look at them). A handler is now identified by its moment, event and microflow: another microflow adds a second handler, the same microflow again is still a duplicate. The same key had made `add event handler if not exists … call M.New` skip — reporting "already exists" — whenever any handler for the event existed, and `drop event handler on before commit` remove whichever handler was stored first. `drop event handler` now takes `call M.Mf` to say which one; without it, a drop that matches several handlers is refused and names them. +- **A bare `create or modify module role M.R;` keeps the role's description** (ako/mxcli#1218) — the "make sure this role exists" statement reported `Modified module role: M.R` and erased the stored description: a statement without `description` was written as an empty one. An omitted `description` now keeps what is stored, as an omitted user role property already did, and the run reports `Unchanged`; `description ''` clears it. Grants were never affected, and `describe` → `exec` was not, since `describe` prints the description. Measured on Mendix 11.13.0: `mx check` is 0 errors with and without the fault, so only the stored description showed it. ## [0.28.0] - 2026-10-23 diff --git a/Makefile b/Makefile index 12f80861ef..f64a174faa 100644 --- a/Makefile +++ b/Makefile @@ -8,6 +8,7 @@ # make test-integration - Run integration tests (requires mx/mxbuild) # make test-mdl - Run MDL integration tests (requires Docker) # make lint - Lint all code (Go + TypeScript) +# make ci-local - Run what CI's build-and-test job runs, before pushing # make lint-go - Lint Go code (fmt + vet) # make lint-ts - Lint TypeScript code (tsc --noEmit) # make grammar - Regenerate ANTLR parser @@ -35,7 +36,7 @@ GO_BUILD_FLAGS = -trimpath # Clean version for VS Code extension (must be valid semver: major.minor.patch) VSCE_VERSION = $(shell echo "$(VERSION)" | sed 's/^v//; s/-.*//' | grep -E '^[0-9]+\.[0-9]+\.[0-9]+$$' || echo "0.0.0") -.PHONY: build build-debug size release clean test test-mdl check-mdl check-skill-mdl check-conformance conformance-shrink gen-migration-reference check-migration-reference check-skill-pack-js check-findings check-wiki-pages digest-status check-tunnel-deps check-test-timeouts check-widget-versions test-integration test-integration-executor test-integration-roundtrip test-integration-parity test-integration-upgrade test-integration-other grammar completions sync-skills sync-skill-packs sync-commands sync-mods sync-lint-rules sync-changelog sync-all docs documentation docs-site docs-serve vscode-ext vscode-install source-tree sbom sbom-report lint lint-go lint-ts fmt fmt-check vet skill-index check-skill-index +.PHONY: build build-debug size release clean test ci-local test-mdl check-mdl check-skill-mdl check-conformance conformance-shrink gen-migration-reference check-migration-reference check-skill-pack-js check-findings check-wiki-pages digest-status check-tunnel-deps check-test-timeouts check-widget-versions test-integration test-integration-executor test-integration-roundtrip test-integration-parity test-integration-upgrade test-integration-other grammar completions sync-skills sync-skill-packs sync-commands sync-mods sync-lint-rules sync-changelog sync-all docs documentation docs-site docs-serve vscode-ext vscode-install source-tree sbom sbom-report lint lint-go lint-ts fmt fmt-check vet skill-index check-skill-index # Helper: copy file only if content differs (avoids mtime updates that invalidate go build cache) # Usage: $(call copy-if-changed,src,dst) @@ -453,6 +454,20 @@ check-widget-versions: build # Lint all code (Go + TypeScript) lint: lint-go lint-ts check-conformance check-test-timeouts +# What the build-and-test job in .github/workflows/push-test.yml runs, in its +# order, so a push is proven against the gate it will meet rather than against +# `build test lint`, which covers less of it: check-mdl, check-findings, +# check-skill-index and the rest run only in CI unless someone remembers them. +# check-skill-mdl is a superset of the job's three skill/docs MDL steps. +# +# Left out: govulncheck (the job installs it @latest, over the network) and the +# integration jobs, which need mxbuild -- `make test-integration` runs those. +# A step added to the job belongs here too. +ci-local: build test check-tunnel-deps check-mdl check-skill-pack-js check-findings \ + check-wiki-pages check-skill-index check-test-timeouts check-skill-mdl \ + check-conformance check-migration-reference lint-go + @echo "ci-local passed (build-and-test, without govulncheck)" + # Lint Go code # # Depends on fmt-check, which VERIFIES, not on fmt, which rewrites. `go fmt ./...` @@ -470,13 +485,21 @@ lint-go: fmt-check vet # Tracked files only: the ANTLR parser under mdl/grammar/parser is generated at # build time and deliberately not committed. The empty-list guard is a positive # control -- a check that inspected nothing must fail loudly rather than pass. +# +# The gofmt is the toolchain's, not the first one on PATH. go.mod's `toolchain` +# line makes `go` (and so `make fmt`) switch to that release, but a bare `gofmt` +# is whatever Go is installed, and gofmt's output changes between releases. With +# Go 1.24.7 installed and the toolchain at 1.27.3, the bare one failed main on a +# file CI accepts (mdl/backend/mfmutator/splice_handler_test.go) and passed a +# file CI then rejected (ako/mxcli#1759). +GOFMT ?= $(shell go env GOROOT)/bin/gofmt fmt-check: @files=$$(git ls-files '*.go' | grep -v '/parser/'); \ if [ -z "$$files" ]; then \ echo "fmt-check: found no Go files to check -- the file list is wrong"; \ exit 1; \ fi; \ - unformatted=$$(gofmt -l $$files); \ + unformatted=$$($(GOFMT) -l $$files); \ if [ -n "$$unformatted" ]; then \ echo "Not gofmt-clean (run 'make fmt'):"; \ echo "$$unformatted"; \ diff --git a/cmd/mxcli/cmd_git_commit.go b/cmd/mxcli/cmd_git_commit.go index cbdb52cbea..3942c276ce 100644 --- a/cmd/mxcli/cmd_git_commit.go +++ b/cmd/mxcli/cmd_git_commit.go @@ -32,8 +32,9 @@ var gitCommitCmd = &cobra.Command{ rewritten from the file, which is the source of truth; MISSING or ORPHAN entries have no automatic repair and refuse the commit; 2. everything is staged, then the files Studio Pro never commits are - unstaged again: project-settings.user.json, *.mpr.lock, *.mpr.bak, - .mendix-cache/, deployment/, theme-cache/, .mxcli/; + unstaged again: project-settings.user.json, *.mpr.lock, *.mpr.bak, and + the project's own .mendix-cache/, deployment/, theme-cache/ and .mxcli/ + beside the .mpr (a folder of the same name further down is committed); 3. git commit runs with your message and your git identity; 4. the mx_metadata note is computed from the new commit and attached, so Studio Pro can version-check it and show what changed. @@ -60,7 +61,7 @@ func init() { gitCommitCmd.Flags().StringP("project", "p", "", "path to the Mendix project (.mpr)") _ = gitCommitCmd.MarkFlagRequired("project") gitCommitCmd.Flags().Bool("push", false, "push the branch and the notes ref to their remote afterwards") - gitCommitCmd.Flags().Bool("dry-run", false, "stage and report, then unstage; commit nothing") + gitCommitCmd.Flags().Bool("dry-run", false, "report what would be committed; commit nothing and leave your staging as it is") gitCmd.AddCommand(gitCommitCmd) } @@ -115,21 +116,34 @@ func commitRun(mprPath, message string, push, dryRun bool, out io.Writer) error } } - // 2. Stage everything, then unstage what Studio Pro never commits. - if _, err := gitOutIn(root, nil, "add", "-A"); err != nil { + // 2. Stage everything, then unstage what Studio Pro never commits. A dry + // run stages into a copy of the index: resetting the real one afterwards + // unstaged whatever the user had staged before running it. + var env []string + if dryRun { + idx, cleanup, err := indexCopy(root) + if err != nil { + return fmt.Errorf("copy the index for the dry run: %w", err) + } + defer cleanup() + env = []string{"GIT_INDEX_FILE=" + idx} + } + git := func(args ...string) (string, error) { return gitOutInEnv(root, env, nil, args...) } + if _, err := git("add", "-A"); err != nil { return fmt.Errorf("stage changes: %w", err) } // -z: paths come back NUL-separated, so spaces and quotes stay inside one path. - staged := nulFields(gitOutOr(root, "diff", "--cached", "--name-only", "-z")) + cached, _ := git("diff", "--cached", "--name-only", "-z") + staged := nulFields(cached) + projDir := path.Dir(repo.MPRPath) // the project's folder in the repository; "." at its root var junk []string for _, f := range staged { - if isJunkPath(f) { + if isJunkPath(f, projDir) { junk = append(junk, f) } } if len(junk) > 0 { - args := append([]string{"reset", "-q", "--"}, junk...) - if _, err := gitOutIn(root, nil, args...); err != nil { + if _, err := git(append([]string{"reset", "-q", "--"}, junk...)...); err != nil { return fmt.Errorf("unstage files Studio Pro never commits: %w", err) } fmt.Fprintf(out, "Left out %d file(s) Studio Pro never commits (project-settings.user.json, *.mpr.lock, caches).\n", len(junk)) @@ -151,10 +165,7 @@ func commitRun(mprPath, message string, push, dryRun bool, out io.Writer) error } if dryRun { - if _, err := gitOutIn(root, nil, "reset", "-q"); err != nil { - return fmt.Errorf("unstage: %w", err) - } - fmt.Fprintln(out, "Dry run: staged nothing permanently, committed nothing.") + fmt.Fprintln(out, "Dry run: committed nothing; what you had staged is unchanged.") return nil } @@ -185,6 +196,47 @@ func commitRun(mprPath, message string, push, dryRun bool, out io.Writer) error return nil } +// indexCopy copies the repository's index to a temporary file, for a +// GIT_INDEX_FILE that a dry run can stage into without touching the real one. +// With no index yet, the path is left free for git to create it. +func indexCopy(root string) (string, func(), error) { + p, err := gitOut(root, "rev-parse", "--git-path", "index") + if err != nil { + return "", nil, err + } + src := strings.TrimSpace(p) + if !filepath.IsAbs(src) { + src = filepath.Join(root, src) + } + f, err := os.CreateTemp("", "mxcli-dry-run-index-*") + if err != nil { + return "", nil, err + } + tmp := f.Name() + cleanup := func() { _ = os.Remove(tmp) } + data, err := os.ReadFile(src) + switch { + case errors.Is(err, os.ErrNotExist): + f.Close() + cleanup() // an empty file is not a valid index + return tmp, cleanup, nil + case err != nil: + f.Close() + cleanup() + return "", nil, err + } + if _, err := f.Write(data); err != nil { + f.Close() + cleanup() + return "", nil, err + } + if err := f.Close(); err != nil { + cleanup() + return "", nil, err + } + return tmp, cleanup, nil +} + // nulFields splits git's -z output (NUL-separated, no quoting) into paths. func nulFields(s string) []string { var out []string @@ -197,9 +249,14 @@ func nulFields(s string) []string { } // isJunkPath reports whether a repo-relative path is one of the files Studio -// Pro never commits. Matched on any path level and without case, the way -// Studio Pro matches its own lock file (whose .gitignore entry is lowercase). -func isJunkPath(rel string) bool { +// Pro never commits. The files match by name at any level and without case, +// the way Studio Pro matches its own lock file (whose .gitignore entry is +// lowercase). The folders are the project's own, the ones Studio Pro, mxbuild +// and mxcli create directly in projDir (the project's folder relative to the +// repository root, "." at the root): matched at any depth, a user folder of +// the same name, resources/deployment/ say, was silently left out of the +// commit. +func isJunkPath(rel, projDir string) bool { f := filepath.ToSlash(rel) base := path.Base(f) switch { @@ -208,11 +265,21 @@ func isJunkPath(rel string) bool { case strings.HasSuffix(strings.ToLower(base), ".mpr.lock"), strings.HasSuffix(strings.ToLower(base), ".mpr.bak"): return true } - for _, seg := range strings.Split(f, "/") { - switch strings.ToLower(seg) { - case ".mendix-cache", "deployment", "theme-cache", ".mxcli": - return true + inProject := f + if projDir != "." && projDir != "" { + prefix := projDir + "/" + if len(f) <= len(prefix) || !strings.EqualFold(f[:len(prefix)], prefix) { + return false } + inProject = f[len(prefix):] + } + folder, rest, inFolder := strings.Cut(inProject, "/") + if !inFolder || rest == "" { + return false + } + switch strings.ToLower(folder) { + case ".mendix-cache", "deployment", "theme-cache", ".mxcli": + return true } return false } diff --git a/cmd/mxcli/cmd_git_doctor.go b/cmd/mxcli/cmd_git_doctor.go index 1e17d23f3c..06fb78f185 100644 --- a/cmd/mxcli/cmd_git_doctor.go +++ b/cmd/mxcli/cmd_git_doctor.go @@ -25,20 +25,27 @@ Checked: 1. Local git config: core.autocrlf=False (a checkout must not rewrite line endings inside .mxunit files), mendix.commits-since-gc and mendix.lineEndingResetDone (Studio Pro's own bookkeeping). --fix sets them. - 2. The remote URL. Studio Pro can only use HTTPS; an SSH remote - (git@host:org/app.git or ssh://…) is rewritten to HTTPS by --fix. Other - non-HTTPS URLs are reported without being changed. + 2. The remote URL, as configured. Studio Pro can only use HTTPS; an SSH + remote (git@host:org/app.git or ssh://…) is rewritten to HTTPS by --fix. + Other non-HTTPS URLs are reported without being changed. A + url..insteadOf rule in your git config that makes git use another + URL is reported too; --fix leaves it alone. 3. The remote's refs/notes/mx_metadata. Absent or unreachable is a hint: - push the notes ref together with the branch. + push the notes ref together with the branch. When the remote has notes + the local ref does not hold (git clone never fetches refs/notes/*), the + commits are not judged: that is the finding, and --fix fetches the ref, + as a fast-forward only. 4. Every commit's mx_metadata note. Missing notes make Studio Pro show "(unknown)"; a malformed note it cannot read at all. --fix computes real notes from the model (with -p), and falls back to a template note with a --version ModelerVersion where the model at a commit cannot be read. Notes Studio Pro wrote are never touched. -Exits 1 while an issue remains, so the audit can gate CI. --fix does not push: -push the branch and the notes ref together yourself. +Exits 1 while an issue remains, so the audit can gate CI; fetch the notes +first there. --fix does not push: push the branch and the notes ref together +yourself. + git fetch refs/notes/mx_metadata:refs/notes/mx_metadata git push refs/notes/mx_metadata`, Example: ` mxcli git doctor mxcli git doctor -p app.mpr --fix diff --git a/cmd/mxcli/gitdoctor.go b/cmd/mxcli/gitdoctor.go index 48f0457286..0d106e9f6f 100644 --- a/cmd/mxcli/gitdoctor.go +++ b/cmd/mxcli/gitdoctor.go @@ -76,11 +76,16 @@ func doctorRun(root, mprPath, remoteOverride, version string, fix bool, out io.W fmt.Fprintln(out, "Remote") remote := detectDoctorRemote(root, remoteOverride) + remoteNotes := "" // the remote's notes commit, when it has one and could be reached switch { case remote == "": fmt.Fprintln(out, " [i] no remote configured; skipping remote checks") default: - urlRaw, urlErr := gitOut(root, "remote", "get-url", remote) + // The configured URL, not `git remote get-url`: that one applies + // url..insteadOf, so a rewrite in the user's git config (https -> + // git@ is a common one) made an HTTPS remote read as SSH, and --fix set + // the configured URL on every run without changing what git uses. + urlRaw, urlErr := gitOut(root, "config", "--get", "remote."+remote+".url") if urlErr != nil { found++ fmt.Fprintf(out, " [ ] remote %s does not exist (git remote -v lists the names)\n", remote) @@ -105,6 +110,9 @@ func doctorRun(root, mprPath, remoteOverride, version string, fix bool, out io.W } else { fmt.Fprintf(out, " [i] %s: %s (not HTTPS; Studio Pro can only use HTTPS remotes)\n", remote, url) } + if eff, err := gitOut(root, "remote", "get-url", remote); err == nil && strings.TrimSpace(eff) != url { + fmt.Fprintf(out, " [i] a url..insteadOf rule in your git config makes git use %s instead; --fix leaves the rule alone\n", strings.TrimSpace(eff)) + } // Absent or unreachable notes ref is a hint, not an issue. ls, err := gitOut(root, "ls-remote", remote, mxMetadataRef) switch { @@ -112,12 +120,15 @@ func doctorRun(root, mprPath, remoteOverride, version string, fix bool, out io.W fmt.Fprintf(out, " [i] could not reach %s to check %s\n", remote, mxMetadataRef) case strings.Contains(ls, mxMetadataRef): fmt.Fprintf(out, " [x] %s has %s\n", remote, mxMetadataRef) + if f := strings.Fields(ls); len(f) > 0 { + remoteNotes = f[0] + } default: fmt.Fprintf(out, " [i] %s has no %s yet; push it together with the branch\n", remote, mxMetadataRef) } } - fn, fx, err := doctorNotes(root, mprPath, version, fix, out) + fn, fx, err := doctorNotes(root, mprPath, version, remote, remoteNotes, fix, out) if err != nil { return err } @@ -143,12 +154,35 @@ func doctorRun(root, mprPath, remoteOverride, version string, fix bool, out io.W // doctorNotes reports (and with fix, repairs) missing and malformed notes over // the whole current history. It returns how many it found and how many it fixed. -func doctorNotes(root, mprPath, version string, fix bool, out io.Writer) (found, fixed int, err error) { +// +// remoteNotes is the remote's notes commit ("" when it has none or could not +// be reached). Those are the notes Studio Pro reads: its fetch force-replaces +// the local ref with them. A local ref that does not contain that commit — a +// fresh clone, since git clone never fetches refs/notes/* — cannot be judged: +// every commit the remote annotates would look unannotated, and a note written +// over it could not be pushed (non-fast-forward). So the commits are not +// judged until the ref is fetched; --fix fetches it, as a fast-forward only, +// which never drops a note that exists only locally. +func doctorNotes(root, mprPath, version, remote, remoteNotes string, fix bool, out io.Writer) (found, fixed int, err error) { fmt.Fprintln(out, "mx_metadata notes") if _, err := gitOut(root, "rev-parse", "--verify", "--quiet", "HEAD"); err != nil { fmt.Fprintln(out, " [i] no commits yet") return 0, 0, nil } + if remoteNotes != "" && !hasNotesCommit(root, remoteNotes) { + found++ + fmt.Fprintf(out, " [ ] %s from %s is not fetched; the notes cannot be judged without it\n", mxMetadataRef, remote) + if !fix { + fmt.Fprintf(out, " git fetch %s %s:%s\n", remote, mxMetadataRef, mxMetadataRef) + return found, 0, nil + } + if _, err := gitOut(root, "fetch", "--quiet", remote, mxMetadataRef+":"+mxMetadataRef); err != nil { + fmt.Fprintf(out, " could not fetch it as a fast-forward (%v); if the local ref has notes the remote lacks, push or reconcile them first\n", err) + return found, 0, nil + } + fixed++ + fmt.Fprintf(out, " fetched from %s\n", remote) + } revs := strings.Fields(gitOutOr(root, "rev-list", "--reverse", "HEAD")) noted := map[string]bool{} for _, line := range strings.Split(strings.TrimRight(gitOutOr(root, "notes", "--ref="+mxMetadataRef, "list"), "\n"), "\n") { @@ -316,6 +350,21 @@ func sshToHTTPS(url string) (host, p string, ok bool) { return "", "", false } +// hasNotesCommit reports whether the local notes ref is, or contains, the +// notes commit c. False when the ref is absent or c is not in its history +// (including when c's object was never fetched). +func hasNotesCommit(root, c string) bool { + local, err := gitOut(root, "rev-parse", "--verify", "--quiet", mxMetadataRef) + if err != nil { + return false + } + if strings.TrimSpace(local) == c { + return true + } + _, err = gitOut(root, "merge-base", "--is-ancestor", c, strings.TrimSpace(local)) + return err == nil +} + func gitNoteText(root, c string) string { out, err := gitOut(root, "notes", "--ref="+mxMetadataRef, "show", c) if err != nil { diff --git a/cmd/mxcli/gitdoctor_test.go b/cmd/mxcli/gitdoctor_test.go index c0e0393f4f..5457a1b31b 100644 --- a/cmd/mxcli/gitdoctor_test.go +++ b/cmd/mxcli/gitdoctor_test.go @@ -23,6 +23,14 @@ import ( func doctorEnv(t *testing.T, dir string) { t.Helper() requireGit(t) + // The repository under test is the only git configuration these tests may + // see. `git remote get-url` applies url..insteadOf, so a rewrite in + // ~/.gitconfig (https -> git@, common on developer machines) or in the + // GIT_CONFIG_COUNT/KEY/VALUE environment (git@ -> https, set by hosted + // sandboxes) changed the URL the remote tests read back. + t.Setenv("GIT_CONFIG_GLOBAL", os.DevNull) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + t.Setenv("GIT_CONFIG_COUNT", "0") t.Setenv("GIT_CEILING_DIRECTORIES", filepath.Dir(dir)) t.Setenv("GIT_AUTHOR_NAME", "t") t.Setenv("GIT_AUTHOR_EMAIL", "t@example.com") @@ -135,6 +143,56 @@ func TestGitDoctor_SSHURLFormRewrittenByFix(t *testing.T) { } } +// A url..insteadOf rule rewrites the URL every git command uses, not the +// URL the repository is configured with. The doctor judged the rewritten one: +// with the common https -> git@ rule in ~/.gitconfig an HTTPS remote read as +// SSH, and --fix "repaired" the configured URL on every run. +func TestGitDoctor_InsteadOfRewriteIsNotAnSSHRemote(t *testing.T) { + dir := t.TempDir() + doctorEnv(t, dir) + t.Setenv("GIT_SSH_COMMAND", "false") // the rewritten URL is SSH: never reach the network + gitIn(t, dir, "init", "-q", "-b", "main") + for _, kv := range mxConfigKeys { + gitIn(t, dir, "config", "--local", kv[0], kv[1]) + } + gitIn(t, dir, "config", "--local", "url.git@git.example.com:.insteadOf", "https://git.example.com/") + gitIn(t, dir, "remote", "add", "origin", "https://git.example.com/org/app.git") + + var out bytes.Buffer + if err := doctorRun(dir, "", "", "", false, &out); err != nil { + t.Errorf("a configured HTTPS remote is not an issue, whatever a rewrite makes of it: %v\n%s", err, out.String()) + } + if !strings.Contains(out.String(), "insteadOf") { + t.Errorf("the report must explain the rewrite:\n%s", out.String()) + } + + out.Reset() + if err := doctorRun(dir, "", "", "", true, &out); err != nil { + t.Errorf("--fix: %v\n%s", err, out.String()) + } + if got := configLocal(t, dir, "remote.origin.url"); got != "https://git.example.com/org/app.git" { + t.Errorf("--fix changed the configured URL to %q:\n%s", got, out.String()) + } +} + +// The configured URL is what --fix rewrites, so an SSH remote is still found +// when a rule rewrites it to HTTPS (as hosted sandboxes do). +func TestGitDoctor_ConfiguredSSHRemoteFoundBehindRewrite(t *testing.T) { + dir := t.TempDir() + doctorEnv(t, dir) + gitIn(t, dir, "init", "-q", "-b", "main") + gitIn(t, dir, "config", "--local", "url.https://git.example.com/.insteadOf", "git@git.example.com:") + gitIn(t, dir, "remote", "add", "origin", "git@git.example.com:org/app.git") + + var out bytes.Buffer + if err := doctorRun(dir, "", "", "", true, &out); err != nil { + t.Fatalf("--fix: %v\n%s", err, out.String()) + } + if got := configLocal(t, dir, "remote.origin.url"); got != "https://git.example.com/org/app" { + t.Errorf("configured URL = %q, want https://git.example.com/org/app\n%s", got, out.String()) + } +} + // --- doctor: mx_metadata notes ---------------------------------------------- // doctorNoteRepo makes a two-commit repository holding an MPR v2 project: @@ -210,6 +268,48 @@ func TestGitDoctor_FixKeepsRealNotes(t *testing.T) { } } +// A fresh clone has no refs/notes/* (git clone never fetches them), while the +// remote carries the notes Studio Pro wrote. Judged against the empty local +// ref, every commit looked unannotated, and --fix wrote notes over the +// remote's that it then refused to accept (non-fast-forward). +func TestGitDoctor_UnfetchedNotesAreNotMissing(t *testing.T) { + src, _ := doctorNoteRepo(t) + doctorEnv(t, src) + gitIn(t, src, "notes", "--ref="+mxMetadataRef, "add", "-f", "-m", realStudioProNote, "HEAD") + remote := t.TempDir() + gitIn(t, remote, "init", "-q", "--bare") + gitIn(t, src, "remote", "add", "origin", remote) + gitIn(t, src, "push", "-q", "origin", "feature", mxMetadataRef) + + clone := filepath.Join(t.TempDir(), "clone") + gitIn(t, filepath.Dir(clone), "clone", "-q", "-b", "feature", remote, clone) + for _, kv := range mxConfigKeys { + gitIn(t, clone, "config", "--local", kv[0], kv[1]) + } + mprPath := filepath.Join(clone, "App.mpr") + head := strings.TrimSpace(gitOutOr(clone, "rev-parse", "HEAD")) + + var out bytes.Buffer + err := doctorRun(clone, mprPath, "", "", false, &out) + if strings.Contains(out.String(), shortCommit(head)+" without a note") { + t.Errorf("HEAD has a note on the remote; an unfetched notes ref must not report it missing:\n%s", out.String()) + } + if !errors.Is(err, errDoctorIssuesRemain) || !strings.Contains(out.String(), "git fetch origin "+mxMetadataRef) { + t.Errorf("the unfetched notes ref must be the finding, with the command that fetches it: %v\n%s", err, out.String()) + } + + out.Reset() + if err := doctorRun(clone, mprPath, "", "", true, &out); err != nil { + t.Fatalf("--fix must fetch the notes and annotate the rest: %v\n%s", err, out.String()) + } + if got := strings.TrimSpace(noteOn(t, clone, "HEAD")); got != realStudioProNote { + t.Fatalf("--fix replaced the remote's note on HEAD:\n got %s\nwant %s\n%s", got, realStudioProNote, out.String()) + } + if _, err := gitOut(clone, "push", "origin", "feature", mxMetadataRef); err != nil { + t.Errorf("the notes --fix leaves must push as a fast-forward: %v\n%s", err, out.String()) + } +} + func TestGitDoctor_TemplateWithoutProjectUsesVersionFlag(t *testing.T) { dir := t.TempDir() doctorEnv(t, dir) @@ -358,6 +458,34 @@ func TestGitCommit_RefusedWhileStudioProOpen(t *testing.T) { } } +// --dry-run staged everything to report it and then reset the whole index, so +// whatever the user had staged before running it was unstaged. +func TestGitCommit_DryRunKeepsTheUsersStaging(t *testing.T) { + dir, mprPath := doctorNoteRepo(t) + doctorEnv(t, dir) + for _, name := range []string{"staged.txt", "unstaged.txt"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte(name), 0o644); err != nil { + t.Fatal(err) + } + } + gitIn(t, dir, "add", "staged.txt") + + var out bytes.Buffer + if err := commitRun(mprPath, "dry", false, true, &out); err != nil { + t.Fatalf("dry run failed: %v\n%s", err, out.String()) + } + if !strings.Contains(out.String(), "unstaged.txt") { + t.Errorf("the dry run must still report every file it would commit:\n%s", out.String()) + } + staged, err := gitOut(dir, "diff", "--cached", "--name-only") + if err != nil { + t.Fatal(err) + } + if got := strings.TrimSpace(staged); got != "staged.txt" { + t.Errorf("staged after --dry-run = %q, want exactly what the user had staged (staged.txt)", got) + } +} + func TestGitCommit_DryRunCommitsNothing(t *testing.T) { dir, mprPath := doctorNoteRepo(t) doctorEnv(t, dir) @@ -464,6 +592,85 @@ func TestGitCommit_SpacedPathsStagedWholeAndJunkLeft(t *testing.T) { } } +// writeFiles creates each repo-relative path with its parent folders. +func writeFiles(t *testing.T, dir string, paths ...string) { + t.Helper() + for _, p := range paths { + full := filepath.Join(dir, filepath.FromSlash(p)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(p), 0o644); err != nil { + t.Fatal(err) + } + } +} + +// committedSet is the set of paths HEAD changed. +func committedSet(t *testing.T, dir string) map[string]bool { + t.Helper() + set := map[string]bool{} + for _, f := range strings.Split(strings.TrimSpace(committedFiles(t, dir)), "\n") { + set[f] = true + } + return set +} + +// The never-commit folders are the project's own, beside the .mpr, where +// Studio Pro's .gitignore anchors them (/deployment/). Matched at any depth, a +// user folder of the same name was silently left out of the commit. +func TestGitCommit_NestedFolderNamedLikeJunkIsCommitted(t *testing.T) { + dir, mprPath := doctorNoteRepo(t) + doctorEnv(t, dir) + writeFiles(t, dir, "resources/deployment/endpoints.xml", "javascriptsource/theme-cache/x.js", "deployment/run.log", "theme-cache/a.css") + + var out bytes.Buffer + if err := commitRun(mprPath, "nested", false, false, &out); err != nil { + t.Fatalf("commitRun: %v\n%s", err, out.String()) + } + files := committedSet(t, dir) + for _, keep := range []string{"resources/deployment/endpoints.xml", "javascriptsource/theme-cache/x.js"} { + if !files[keep] { + t.Errorf("%s is not the project's own folder and must be committed:\n%s", keep, out.String()) + } + } + for _, junk := range []string{"deployment/run.log", "theme-cache/a.css"} { + if files[junk] { + t.Errorf("%s is the project's own and must be left out", junk) + } + } +} + +// With the project in a subfolder of the repository, its folders are under +// that subfolder; the same names elsewhere in the repository are not its own. +func TestGitCommit_JunkAnchoredAtProjectInSubfolder(t *testing.T) { + root := t.TempDir() + doctorEnv(t, root) + gitIn(t, root, "init", "-q", "-b", "feature") + app := filepath.Join(root, "app") + if err := os.MkdirAll(app, 0o755); err != nil { + t.Fatal(err) + } + writeProject(t, app, "11.13.0", baseUnits(t)) + gitIn(t, root, "add", "-A") + gitIn(t, root, "commit", "-q", "-m", "base") + writeFiles(t, root, "app/deployment/run.log", "app/resources/deployment/endpoints.xml", "tools/deployment/deploy.sh") + + var out bytes.Buffer + if err := commitRun(filepath.Join(app, "App.mpr"), "sub", false, false, &out); err != nil { + t.Fatalf("commitRun: %v\n%s", err, out.String()) + } + files := committedSet(t, root) + if files["app/deployment/run.log"] { + t.Error("app/deployment/ is the project's own and must be left out") + } + for _, keep := range []string{"app/resources/deployment/endpoints.xml", "tools/deployment/deploy.sh"} { + if !files[keep] { + t.Errorf("%s must be committed:\n%s", keep, out.String()) + } + } +} + // An unresolved merge must refuse the commit: `git add -A` would otherwise // stage the conflict markers and commit them. func TestGitCommit_RefusedDuringUnresolvedMerge(t *testing.T) { diff --git a/cmd/mxcli/gitnote.go b/cmd/mxcli/gitnote.go index 24a5934d63..b9e55e1bd3 100644 --- a/cmd/mxcli/gitnote.go +++ b/cmd/mxcli/gitnote.go @@ -366,8 +366,14 @@ func gitOut(dir string, args ...string) (string, error) { } func gitOutIn(dir string, stdin []byte, args ...string) (string, error) { + return gitOutInEnv(dir, nil, stdin, args...) +} + +// gitOutInEnv is gitOutIn with extra environment entries, such as the +// GIT_INDEX_FILE a dry run stages into. +func gitOutInEnv(dir string, env []string, stdin []byte, args ...string) (string, error) { cmd := exec.Command("git", append([]string{"-C", dir}, args...)...) - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0", "GIT_PAGER=cat") + cmd.Env = append(append(os.Environ(), "GIT_TERMINAL_PROMPT=0", "GIT_PAGER=cat"), env...) if stdin != nil { cmd.Stdin = bytes.NewReader(stdin) } diff --git a/cmd/mxcli/syntax/features_security.go b/cmd/mxcli/syntax/features_security.go index 83372b3499..698f5a7d4a 100644 --- a/cmd/mxcli/syntax/features_security.go +++ b/cmd/mxcli/syntax/features_security.go @@ -22,7 +22,7 @@ func init() { "module role", "create role", "drop role", }, Syntax: "CREATE [OR MODIFY] MODULE ROLE . [DESCRIPTION ''];\nDROP MODULE ROLE .;", - Example: "mdl 1;\nCREATE MODULE ROLE Shop.Admin DESCRIPTION 'Full access';\n-- OR MODIFY makes a security script re-runnable:\nCREATE OR MODIFY MODULE ROLE Shop.User DESCRIPTION 'Read-only access';", + Example: "mdl 1;\nCREATE MODULE ROLE Shop.Admin DESCRIPTION 'Full access';\n-- OR MODIFY makes a security script re-runnable:\nCREATE OR MODIFY MODULE ROLE Shop.User DESCRIPTION 'Read-only access';\n-- Without DESCRIPTION it keeps the stored one; DESCRIPTION '' clears it:\nCREATE OR MODIFY MODULE ROLE Shop.Admin;", SeeAlso: []string{"security.user-role", "security.entity-access"}, }) diff --git a/cmd/mxcli/testrunner/generator.go b/cmd/mxcli/testrunner/generator.go index 674b81b3dc..0e347a7f9b 100644 --- a/cmd/mxcli/testrunner/generator.go +++ b/cmd/mxcli/testrunner/generator.go @@ -273,9 +273,15 @@ func writeExpectAssertion(b *strings.Builder, testID string, exp Expect, evaluat // varPattern matches $VariableName in MDL ($ followed by word characters). var varPattern = regexp.MustCompile(`\$([A-Za-z_][A-Za-z0-9_]*)`) -// assignPattern matches "$var = CALL" or "$var = CREATE" at the start of any +// assignPattern matches "$var = call …" or "$var = create …" at the start of any // statement line inside the test body. -var assignPattern = regexp.MustCompile(`(?m)^\s*\$([A-Za-z_][A-Za-z0-9_]*)\s*=\s*(?:CALL|CREATE)`) +// +// The keywords match in any case. MDL keywords are case-insensitive, and mdl 1 +// and `fmt` write them in lower case; matching only `CALL|CREATE` left +// `$r = call microflow …` unrenamed, so two tests that both assigned `$r` declared +// it twice in the one runner flow and injection failed with MDL063 (CE0111). +// The `\b` keeps a value that merely starts with those letters from matching. +var assignPattern = regexp.MustCompile(`(?mi)^\s*\$([A-Za-z_][A-Za-z0-9_]*)\s*=\s*(?:call|create)\b`) // extractVariableNames finds all user-defined variable names in the MDL body. // Returns the set of variable names (without $ prefix). diff --git a/cmd/mxcli/testrunner/on_error_coverage_test.go b/cmd/mxcli/testrunner/on_error_coverage_test.go index d190814ac8..669a898584 100644 --- a/cmd/mxcli/testrunner/on_error_coverage_test.go +++ b/cmd/mxcli/testrunner/on_error_coverage_test.go @@ -36,7 +36,7 @@ func TestThrowsCatchesEveryCallKind(t *testing.T) { t.Run(c.kind, func(t *testing.T) { tc := parseOne(t, "/**\n * @test it raises\n * @throws 'Only http and https csv urls are supported'\n */\n"+c.stmt+"\n/\n") assertHandled(t, "endpoint runner", GenerateTestFlows(&TestSuite{Name: "s", Tests: []TestCase{tc}}), c.stmt) - assertHandled(t, "legacy runner", GenerateTestRunner(&TestSuite{Name: "s", Tests: []TestCase{tc}}), c.stmt) + assertHandled(t, "legacy runner", GenerateTestRunner(&TestSuite{Name: "s", Tests: []TestCase{tc}}), legacyStmt(c.stmt)) }) } } @@ -53,7 +53,7 @@ func TestVerdictCatchesEveryCallKind(t *testing.T) { if !strings.Contains(mdl, verdictFailPrefix+"exception during execution: ") { t.Errorf("no FAIL verdict in the handler:\n%s", mdl) } - assertHandled(t, "legacy runner", GenerateTestRunner(&TestSuite{Name: "s", Tests: []TestCase{tc}}), c.stmt) + assertHandled(t, "legacy runner", GenerateTestRunner(&TestSuite{Name: "s", Tests: []TestCase{tc}}), legacyStmt(c.stmt)) }) } } @@ -78,3 +78,12 @@ func assertHandled(t *testing.T, runner, mdl, stmt string) { t.Errorf("%s: generated MDL does not parse: %v\n%s", runner, errs[0], mdl) } } + +// legacyStmt is stmt as the legacy runner writes it for the suite's first test: +// that runner compiles every test into one flow, so it renames the variable a +// call assigns with the test's suffix. These statements used to be matched +// unrenamed, which held only because a lower-case `call` was not renamed +// (TestLegacyRunnerRenamesLowerCaseAssignments). +func legacyStmt(stmt string) string { + return renameVariables(stmt, extractVariableNames(stmt), "_1") +} diff --git a/cmd/mxcli/testrunner/rename_keyword_case_test.go b/cmd/mxcli/testrunner/rename_keyword_case_test.go new file mode 100644 index 0000000000..ef63847244 --- /dev/null +++ b/cmd/mxcli/testrunner/rename_keyword_case_test.go @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/executor" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// TestLegacyRunnerRenamesLowerCaseAssignments: the after-startup runner compiles +// every test into one microflow, so a variable two tests both assign has to be +// renamed per test. Which variables are renamed was decided by a pattern that +// matched only upper-case `CALL` and `CREATE` — the spelling the runner's own +// examples used — so `$r = call microflow …`, how mdl 1 and `fmt` spell it, was +// not renamed. Two tests that both assign `$r` then declare it twice, and +// injection fails with MDL063 (CE0111 "Duplicate variable name" from mxbuild). +// Found alongside ako/mxcli#1753. +func TestLegacyRunnerRenamesLowerCaseAssignments(t *testing.T) { + cases := []struct { + name, header, body, renamed string + }{ + {"call microflow, mdl 1", "mdl 1;\n", + "$r = call microflow Probe.SUB_Make();", "$r_%d = call microflow"}, + {"call microflow, mdl 0", "", + "$r = call microflow Probe.SUB_Make();", "$r_%d = call microflow"}, + {"call java action", "mdl 1;\n", + "$r = call java action Probe.JA_Upper(Text = 'a');", "$r_%d = call java action"}, + {"create object", "mdl 1;\n", + "$r = create Probe.Item (Name = 'a');", "$r_%d = create Probe.Item"}, + {"create list", "mdl 1;\n", + "$r = create list of Probe.Item;", "$r_%d = create list of"}, + {"mixed case", "mdl 1;\n", + "$r = Call Microflow Probe.SUB_Make();", "$r_%d = Call Microflow"}, + // Control: the spelling the pattern always matched. + {"upper case (control)", "", + "$r = CALL MICROFLOW Probe.SUB_Make();", "$r_%d = CALL MICROFLOW"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + block := func(n string) string { + return "/**\n * @test " + n + "\n * @expect $r != empty\n */\n" + c.body + "\n/\n" + } + tests, err := parseMDLTests(c.header+block("one")+"\n"+block("two"), "rename.test.mdl") + if err != nil { + t.Fatalf("parse: %v", err) + } + src := GenerateTestRunner(&TestSuite{Name: "s", Tests: tests}) + + for i, want := range []string{ + strings.Replace(c.renamed, "%d", "1", 1), + strings.Replace(c.renamed, "%d", "2", 1), + } { + if !strings.Contains(src, want) { + t.Errorf("test %d: no %q — the assignment was not renamed", i+1, want) + } + } + if strings.Contains(src, "($r != empty)") { + t.Errorf("an assertion still reads the unrenamed $r") + } + + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("generated runner does not parse: %v\n%s", errs[0], src) + } + for _, v := range executor.ValidateProgram(prog, "") { + if v.RuleID == "MDL063" { + t.Errorf("%s: %s", v.RuleID, v.Message) + } + } + if t.Failed() { + t.Logf("generated runner:\n%s", src) + } + }) + } +} + +// TestAssignPatternNeedsAWholeKeyword: matching case-insensitively must not turn +// an assignment whose value merely starts with the letters of a keyword into one. +func TestAssignPatternNeedsAWholeKeyword(t *testing.T) { + for _, body := range []string{ + "$r = callback;", + "$r = createdDate;", + } { + if names := extractVariableNames(body); len(names) != 0 { + t.Errorf("%q: renamed %v, want nothing", body, names) + } + } + if names := extractVariableNames("$r = call microflow M.F();"); !names["r"] { + t.Errorf("control: $r = call microflow was not picked up: %v", names) + } +} diff --git a/cmd/mxcli/testrunner/results.go b/cmd/mxcli/testrunner/results.go index 905b232d91..593574229e 100644 --- a/cmd/mxcli/testrunner/results.go +++ b/cmd/mxcli/testrunner/results.go @@ -189,6 +189,8 @@ func ParseLogResults(logReader io.Reader, suite *TestSuite, requireAssertions bo // Track which tests we've seen results for resultMap := make(map[string]*TestResult) runTimes := make(map[string]time.Time) + // failures counts each test's FAIL and ERROR lines; see applyVerdictLine. + failures := make(map[string]int) scanner := bufio.NewScanner(logReader) for scanner.Scan() { @@ -233,6 +235,9 @@ func ParseLogResults(logReader io.Reader, suite *TestSuite, requireAssertions bo // verdict did not finish: an error left the runner flow, which // stops it here — a call whose own handler re-raises does exactly // that (ako/mxcli#1448). Its PASS or FAIL line replaces this. + // A RUN line starts the test's verdict afresh, so a failure that + // sticks (applyVerdictLine) cannot outlive the run it belongs to. + delete(failures, id) resultMap[id] = &TestResult{ ID: id, Name: name, @@ -240,77 +245,25 @@ func ParseLogResults(logReader io.Reader, suite *TestSuite, requireAssertions bo Message: "started and reported no verdict: an error left the test runner here, which stopped it (the runtime log has the error)", } - case "PASS": - if r, ok := resultMap[id]; ok { - r.Status = StatusPass - r.Message = "" - if t, ok := runTimes[id]; ok { - r.Duration = time.Since(t) - } - } else { - resultMap[id] = &TestResult{ - ID: id, - Name: id, - Status: StatusPass, - } - } - - case "FAIL": + case "PASS", "FAIL", "ERROR", "SKIP": + // ERROR is a setup that threw: the test never ran, so it is neither a + // pass nor a failure. Only the monolithic runner reports this way — + // the endpoint carries the same outcome back as a SETUP verdict. msg := "" if len(parts) >= 4 { msg = parts[3] } - if r, ok := resultMap[id]; ok { - r.Status = StatusFail - r.Message = msg + r, ok := resultMap[id] + if !ok { + r = &TestResult{ID: id, Name: id} + resultMap[id] = r + } + if action == "PASS" || action == "FAIL" { if t, ok := runTimes[id]; ok { r.Duration = time.Since(t) } - } else { - resultMap[id] = &TestResult{ - ID: id, - Name: id, - Status: StatusFail, - Message: msg, - } - } - - case "ERROR": - // A setup that threw: the test never ran, so it is neither a pass nor - // a failure. Only the monolithic runner reports this way — the - // endpoint carries the same outcome back as a SETUP verdict. - msg := "" - if len(parts) >= 4 { - msg = parts[3] - } - if r, ok := resultMap[id]; ok { - r.Status = StatusError - r.Message = msg - } else { - resultMap[id] = &TestResult{ - ID: id, - Name: id, - Status: StatusError, - Message: msg, - } - } - - case "SKIP": - msg := "" - if len(parts) >= 4 { - msg = parts[3] - } - if r, ok := resultMap[id]; ok { - r.Status = StatusSkip - r.Message = msg - } else { - resultMap[id] = &TestResult{ - ID: id, - Name: id, - Status: StatusSkip, - Message: msg, - } } + applyVerdictLine(r, verdictStatus[action], msg, failures) case "END": result.Duration = time.Since(result.Started) @@ -337,6 +290,13 @@ func ParseLogResults(logReader io.Reader, suite *TestSuite, requireAssertions bo } res.Status = r.Status res.Message = r.Message + if more := failures[tc.ID] - 1; more > 0 { + noun := "failures" + if more == 1 { + noun = "failure" + } + res.Message += fmt.Sprintf(" (and %d more %s in this test)", more, noun) + } res.Duration = r.Duration result.Tests = append(result.Tests, res) } else { @@ -351,6 +311,45 @@ func ParseLogResults(logReader io.Reader, suite *TestSuite, requireAssertions bo return result } +// verdictStatus maps a log protocol verdict action to the status it reports. +var verdictStatus = map[string]TestStatus{ + "PASS": StatusPass, + "FAIL": StatusFail, + "ERROR": StatusError, + "SKIP": StatusSkip, +} + +// applyVerdictLine records one verdict line of the after-startup runner's log +// against a test's result. failures counts each test's FAIL and ERROR lines. +// +// A test can log more than one verdict line: writeExpectAssertion logs PASS or +// FAIL for every @expect on its own, and the skipped-assertions guard logs a +// FAIL after them. A failure is therefore sticky — once a test has a FAIL or an +// ERROR, no later line clears it, and the first failure's message is the one +// reported. When every line simply replaced the last, a test whose first +// assertion failed and whose second held reported PASS with the failure message +// erased, and `mxcli test` exited 0 (ako/mxcli#1753). The rule lives here rather +// than in the generated flow so it also holds for a log written by a runner +// generated before it. +// +// This is the endpoint runner's semantics: there an assertion is evaluated only +// while $Verdict is still PASS, and the skipped-assertions guard does not +// replace an existing failure, so its first failure is what it returns. +// +// The ERROR a RUN line leaves as a placeholder is not a failure line; the +// test's own verdict replaces it. +func applyVerdictLine(r *TestResult, status TestStatus, msg string, failures map[string]int) { + failed := status == StatusFail || status == StatusError + if failed { + failures[r.ID]++ + } + if failures[r.ID] > 1 || (!failed && failures[r.ID] > 0) { + return + } + r.Status = status + r.Message = msg +} + // PrintResults writes a human-readable summary to the writer. func PrintResults(w io.Writer, result *SuiteResult, color bool) { fmt.Fprintf(w, "\n") diff --git a/cmd/mxcli/testrunner/sticky_failure_test.go b/cmd/mxcli/testrunner/sticky_failure_test.go new file mode 100644 index 0000000000..993c94a1f6 --- /dev/null +++ b/cmd/mxcli/testrunner/sticky_failure_test.go @@ -0,0 +1,135 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testrunner + +import ( + "strings" + "testing" +) + +// twoExpectSuite is one test with two @expect lines, the shape the after-startup +// runner reports as two verdict lines: writeExpectAssertion logs PASS or FAIL +// for each assertion on its own. +func twoExpectSuite(t *testing.T) *TestSuite { + t.Helper() + e1, err := ParseExpect("$r = 1") + if err != nil { + t.Fatalf("ParseExpect: %v", err) + } + e2, err := ParseExpect("$r = 2") + if err != nil { + t.Fatalf("ParseExpect: %v", err) + } + tc := TestCase{ID: "test_1", Name: "two expects", MDL: "$r = 2;", Expects: []Expect{e1, e2}} + return &TestSuite{Name: "S", Tests: []TestCase{tc}} +} + +// verdictLog is a runner log for test_1 with the given verdict lines between +// its RUN line and the suite's END. +func verdictLog(lines ...string) string { + var b strings.Builder + b.WriteString("MXTEST: MXTEST:START:S\n") + b.WriteString("MXTEST: MXTEST:RUN:test_1:two expects\n") + for _, l := range lines { + b.WriteString("MXTEST: MXTEST:" + l + "\n") + } + b.WriteString("MXTEST: MXTEST:END:S\n") + return b.String() +} + +// TestLaterPassDoesNotClearEarlierFailure is ako/mxcli#1753: on the after-startup +// runner a test with more than one @expect reported PASS when an earlier +// assertion failed and a later one held, and `mxcli test` exited 0. Each +// assertion logs its own verdict line, and the parser let every line overwrite +// the test's status — the last one won, and a PASS also cleared the failure +// message. The endpoint runner evaluates an assertion only while the verdict is +// still PASS, so there the first failure sticks; the log parser now does the +// same. +func TestLaterPassDoesNotClearEarlierFailure(t *testing.T) { + const want = "Expected $r = 1, actual: 2" + + got := ParseLogResults(strings.NewReader(verdictLog("FAIL:test_1:"+want, "PASS:test_1")), + twoExpectSuite(t), false) + r := got.Tests[0] + if r.Status != StatusFail { + t.Errorf("FAIL then PASS: status = %v, want FAIL", r.Status) + } + if r.Message != want { + t.Errorf("FAIL then PASS: message = %q, want %q", r.Message, want) + } + if got.AllPassed() { + t.Error("FAIL then PASS: the suite passed, so `mxcli test` exits 0") + } + + // Control: the order the old parser already got right. + got = ParseLogResults(strings.NewReader(verdictLog("PASS:test_1", "FAIL:test_1:"+want)), + twoExpectSuite(t), false) + if r := got.Tests[0]; r.Status != StatusFail || r.Message != want { + t.Errorf("control, PASS then FAIL: got %v %q, want FAIL %q", r.Status, r.Message, want) + } + + // Control: every assertion held. + got = ParseLogResults(strings.NewReader(verdictLog("PASS:test_1", "PASS:test_1")), + twoExpectSuite(t), false) + if r := got.Tests[0]; r.Status != StatusPass || r.Message != "" { + t.Errorf("control, PASS then PASS: got %v %q, want PASS", r.Status, r.Message) + } +} + +// TestFirstFailureMessageIsReported pins which message a test that failed more +// than once reports: the first, as the endpoint runner's $Verdict does, with a +// count of the rest so the reader knows the log holds more than the summary. +func TestFirstFailureMessageIsReported(t *testing.T) { + got := ParseLogResults(strings.NewReader(verdictLog( + "FAIL:test_1:Expected $r = 1, actual: 2", + "FAIL:test_1:Expected $r = 3, actual: 2", + "PASS:test_1", + "FAIL:test_1:Expected $r = 4, actual: 2", + )), twoExpectSuite(t), false) + r := got.Tests[0] + if r.Status != StatusFail { + t.Fatalf("status = %v, want FAIL", r.Status) + } + const want = "Expected $r = 1, actual: 2 (and 2 more failures in this test)" + if r.Message != want { + t.Errorf("message = %q, want %q", r.Message, want) + } +} + +// TestSkippedAssertionsFailureStillWins pins the guard writeTestBlock emits after +// the assertions: when a call's own handler resumed the flow past some of them, +// the assertions that did run log PASS and the guard logs a FAIL after them +// (ako/mxcli#1448). That FAIL is the first failure, so it is the verdict. +func TestSkippedAssertionsFailureStillWins(t *testing.T) { + got := ParseLogResults(strings.NewReader(verdictLog( + "PASS:test_1", + "FAIL:test_1:"+assertionsSkippedReason, + )), twoExpectSuite(t), false) + if r := got.Tests[0]; r.Status != StatusFail || r.Message != assertionsSkippedReason { + t.Errorf("got %v %q, want FAIL with the skipped-assertions reason", r.Status, r.Message) + } +} + +// TestLaterPassDoesNotClearEarlierError: a setup that threw logs ERROR, and no +// verdict line after it may turn the test into a pass or a skip either. +func TestLaterPassDoesNotClearEarlierError(t *testing.T) { + const msg = "Setup failed: Shop.Seed" + for _, later := range []string{"PASS:test_1", "SKIP:test_1:skipped"} { + got := ParseLogResults(strings.NewReader(verdictLog("ERROR:test_1:"+msg, later)), + twoExpectSuite(t), false) + if r := got.Tests[0]; r.Status != StatusError || r.Message != msg { + t.Errorf("ERROR then %s: got %v %q, want ERROR %q", later, r.Status, r.Message, msg) + } + } +} + +// TestRunLineStartsVerdictAfresh: a failure sticks for the run it was logged in, +// not for the rest of the log. A RUN line begins a test's verdict, so a log that +// holds the same test twice reports the later run. +func TestRunLineStartsVerdictAfresh(t *testing.T) { + log := verdictLog("FAIL:test_1:Expected $r = 1, actual: 2", "RUN:test_1:two expects", "PASS:test_1", "PASS:test_1") + got := ParseLogResults(strings.NewReader(log), twoExpectSuite(t), false) + if r := got.Tests[0]; r.Status != StatusPass { + t.Errorf("got %v %q, want the second run's PASS", r.Status, r.Message) + } +} diff --git a/docs-site/src/appendixes/version-compatibility.md b/docs-site/src/appendixes/version-compatibility.md index 67099ac20f..e6b4cd22a9 100644 --- a/docs-site/src/appendixes/version-compatibility.md +++ b/docs-site/src/appendixes/version-compatibility.md @@ -174,9 +174,9 @@ What *is* version-specific is the optional/default-value half: `PageParameter.Is Atlas UI v3 (Mendix 11.0+) introduced new design properties like "Card style" and "Disable row wrap" for containers. These don't exist in the Atlas v2 theme bundled with 10.x, causing CE6083. -### Page Properties Stored in an Older Shape (below 10.2, 10.7, 10.11) +### Properties Stored in an Older Shape -Some page properties kept their meaning and changed how they are stored. mxcli builds the current shape and, on every write, rewrites it into the shape the project's version stores. `describe` reads both shapes ([ako/mxcli#1209](https://github.com/ako/mxcli/issues/1209)). The older shapes were measured on Studio Pro-authored 9.24.30 pages: +Some properties kept their meaning and changed how they are stored. mxcli builds the current shape and, on every write, rewrites it into the shape the project's version stores. `describe` reads both shapes ([ako/mxcli#1209](https://github.com/ako/mxcli/issues/1209), [ako/mxcli#1163](https://github.com/ako/mxcli/issues/1163)). The older shapes were measured on Studio Pro-authored 9.24 documents and with `mx convert` 9.24.30: | Property | Below | Stored as | |---|---|---| @@ -185,15 +185,23 @@ Some page properties kept their meaning and changed how they are stored. mxcli b | A data view's editability | 10.7 | `Editable: true\|false`, instead of `Editability` | | A text box's, text area's or date picker's placeholder | 10.11 | `Placeholder` text, instead of a `PlaceholderTemplate` | | Any data source's `ForceFullObjects` | 10.8 | Not stored | +| The `PersistentId` of a workflow, activity, outcome or boundary event | 10.21 | Not stored | +| A constant's value in a configuration (`alter settings constant … in configuration`) | 10.9 | A flat `Value` string on the override, instead of a `Settings$SharedValue` under `SharedOrPrivateValue` | +| A page's flow call argument that is the enclosing object (`call microflow M.F(P = $currentObject)`, `call nanoflow …`) | 10.10 | Not stored: Studio Pro infers it. A page parameter, snippet parameter or widget argument is stored as before, as a `Variable` | -What an older shape cannot hold is refused, naming the widget, rather than dropped: +What an older shape cannot hold is refused, naming the element, rather than dropped: - a Spacing, colour-picker or toggle-button-group design property below 10.2; - conditional data view editability below 10.7; - a placeholder with parameters below 10.11; -- a data view fed by another widget below 10.0. +- a data view fed by another widget below 10.0; +- a private constant value in a configuration below 10.9; +- a page's flow call argument that is any other expression (a literal, an attribute path) below 10.10; +- an aggregate over an expression (`sum $L of …`), `reduce`, `all` or `any` below 10.0. The plain `count`, `sum`, `average`, `minimum` and `maximum` are written on every version ([ako/mxcli#1163](https://github.com/ako/mxcli/issues/1163)). Before this, `describe` on 9.24 printed none of these properties, so a describe → exec round trip removed them. Writing a data view on 9.24 was refused outright. +A published OData service is not an older shape of the same document. Before 10.4 it is a different document (`Rest$PublishedODataService`, not `ODataPublish$PublishedODataService2`), which mxcli does not write. `create published odata service` is refused below 10.4 with MDL-VERSION01, by `check -p` and by `exec` before anything is written. Mendix 9.24 cannot load a project that holds the 10.4 document. + ### Widget Type Definitions Pluggable widget PropertyTypes change between Mendix versions. A widget template extracted from 11.6 may have more or fewer properties than the widget installed in a 10.24 project. The [widget augmentation system](../internals/widget-templates.md) handles this by reconciling templates against the project's `.mpk` files. diff --git a/docs-site/src/reference/security/create-module-role.md b/docs-site/src/reference/security/create-module-role.md index 13ed8b972c..6a073d25cb 100644 --- a/docs-site/src/reference/security/create-module-role.md +++ b/docs-site/src/reference/security/create-module-role.md @@ -3,7 +3,7 @@ ## Synopsis ```sql -CREATE MODULE ROLE module.RoleName [ DESCRIPTION 'text' ] +CREATE [ OR MODIFY ] MODULE ROLE module.RoleName [ DESCRIPTION 'text' ] ``` ## Description @@ -12,7 +12,7 @@ Creates a new module role within a module. Module roles are the foundation of Me Each module role belongs to exactly one module. The role name must be unique within the module. By convention, common role names include `User`, `Admin`, `Viewer`, and `Manager`. -If the module does not exist, the statement returns an error. +If the module does not exist, the statement returns an error. If the role exists, `CREATE MODULE ROLE` returns an error; `CREATE OR MODIFY` updates it instead, so a security script can be re-run. Without `DESCRIPTION`, `CREATE OR MODIFY` keeps the role's stored description; `DESCRIPTION ''` clears it. ## Parameters @@ -20,7 +20,7 @@ If the module does not exist, the statement returns an error. : Qualified name consisting of the module name and the new role name, separated by a dot. The module must already exist. `DESCRIPTION 'text'` -: Optional human-readable description of the role's purpose. +: Optional human-readable description of the role's purpose. Left out, a new role has none and an existing role keeps its own. ## Examples @@ -45,6 +45,12 @@ CREATE MODULE ROLE HR.Employee DESCRIPTION 'Self-service access'; CREATE MODULE ROLE HR.Viewer DESCRIPTION 'Read-only access'; ``` +Make sure a role exists, keeping whatever description it has: + +```sql +CREATE OR MODIFY MODULE ROLE HR.Manager; +``` + ## See Also [CREATE USER ROLE](create-user-role.md), [GRANT](grant.md), [REVOKE](revoke.md) diff --git a/docs-site/src/tools/outside-studio-pro.md b/docs-site/src/tools/outside-studio-pro.md index 9cf80d53a1..8219bcf627 100644 --- a/docs-site/src/tools/outside-studio-pro.md +++ b/docs-site/src/tools/outside-studio-pro.md @@ -196,8 +196,10 @@ In order it: 1. repairs ContentsHash mismatches from the files on disk (`MISSING` and `ORPHAN` entries have no automatic repair and refuse the commit); 2. stages everything, then unstages the files Studio Pro never commits: - `project-settings.user.json`, `*.mpr.lock`, `*.mpr.bak`, `.mendix-cache/`, - `deployment/`, `theme-cache/`, `.mxcli/`; + `project-settings.user.json`, `*.mpr.lock`, `*.mpr.bak`, and the project's + own `.mendix-cache/`, `deployment/`, `theme-cache/` and `.mxcli/`, the + folders Studio Pro, mxbuild and mxcli create beside the `.mpr`. A folder of + the same name further down, such as `resources/deployment/`, is committed; 3. runs `git commit` with your message and your git identity; 4. computes the mx_metadata note from the new commit and attaches it, and prints the `git push origin refs/notes/mx_metadata` command — @@ -208,7 +210,8 @@ The commit is refused while Studio Pro has the project open (the `.mpr.lock` beside it), and during an unresolved merge — `git add -A` would otherwise commit the conflict markers. `--dry-run` verifies and reports (including ContentsHash drift, which it does not repair — a dry run writes nothing), -stages, reports what it would commit, and unstages again without committing. +and reports what it would commit without committing. It stages into a copy of +the index, so whatever you had staged is left exactly as it was. ## Auditing a repository: `mxcli git doctor` @@ -225,13 +228,24 @@ mxcli git doctor --fix --version 10.21.0 | Check | `--fix` | |-------|---------| | local git config: `core.autocrlf=False` (a checkout must not rewrite line endings inside `.mxunit` files), `mendix.commits-since-gc`, `mendix.lineEndingResetDone` | sets them | -| the remote URL: Studio Pro can only use HTTPS | rewrites SSH (`git@host:org/app.git`, `ssh://…`) to HTTPS; other non-HTTPS URLs are reported unchanged | +| the remote URL as configured: Studio Pro can only use HTTPS | rewrites SSH (`git@host:org/app.git`, `ssh://…`) to HTTPS; other non-HTTPS URLs are reported unchanged. A `url..insteadOf` rule in your git config that makes git use another URL is reported, and left alone | | the remote's `refs/notes/mx_metadata` | reported only — push the notes ref together with the branch | +| the local `refs/notes/mx_metadata` holds the remote's notes: `git clone` and a plain `git fetch` never fetch them, so a fresh clone or a CI checkout has none | fetches them, as a fast-forward only; refused when the local ref has notes the remote does not | | each commit on the checked-out branch and its mx_metadata note: missing notes make Studio Pro show `(unknown)`; a malformed note it cannot read at all | computes real notes from the model (`-p`), falling back to a template note with a `--version` ModelerVersion where the model at a commit cannot be read (MPR v1 history, a revision without the `.mpr`); when no version is known the newest valid note in history supplies it | Notes Studio Pro wrote are never touched, with or without `--fix`. `--fix` does not push. +The commits are judged only once the local notes ref holds the remote's. Until +then the report names the unfetched ref and the command that fetches it, rather +than calling every commit the remote annotates unannotated. In CI, fetch the +notes before the audit: + +```bash +git fetch origin refs/notes/mx_metadata:refs/notes/mx_metadata +mxcli git doctor -p app.mpr +``` + ## Commit metadata: `mxcli git note` Studio Pro attaches a git note under `refs/notes/mx_metadata` to every commit diff --git a/docs-site/src/tools/test-annotations.md b/docs-site/src/tools/test-annotations.md index 4cd744b272..65f0454486 100644 --- a/docs-site/src/tools/test-annotations.md +++ b/docs-site/src/tools/test-annotations.md @@ -77,6 +77,10 @@ FAIL the board is 81 squares expected length($result) = 81, actual: 27 ``` +A test with several `@expect` lines passes only when every one of them holds, and +it reports the first that does not. The Docker / `--legacy-runner` path evaluates +them all, and adds how many more failed — `(and 1 more failure in this test)`. + `@expect` cannot be combined with `@throws`: a body expected to fail produces no result to assert on, so the combination is refused rather than ignored. diff --git a/docs-wiki/bug-patterns/the-platform-ci-does-not-run.md b/docs-wiki/bug-patterns/the-platform-ci-does-not-run.md index 1b1116a8db..8e4c471e1e 100644 --- a/docs-wiki/bug-patterns/the-platform-ci-does-not-run.md +++ b/docs-wiki/bug-patterns/the-platform-ci-does-not-run.md @@ -91,6 +91,18 @@ every unit write passes (`canon.LowerToProjectShape`, before the floor), and to read both shapes. A starter app is the cheapest source of the older shape's real bytes: `mxcli new --version 9.24.30.51827` authored by Studio Pro. +A refusal is not always an older shape, though. In the next 9.24 run (#1163), one +of five refusals was the floor's own data being wrong: an aggregate's function had +been given the row of the fold `reduce` added in 10.0. One was a document type the +version does not have: a published OData service before 10.4. That needs a gate, +because lowering its property would have produced a project that does not load. +For one more, the older shape was "nothing stored": Studio Pro 9 infers a flow +argument that is the enclosing object. Three things settled each one: the SDK's +version data, Studio Pro-authored bytes (a Marketplace `.mpk` holds a whole +`project.mpr` of its Mendix version), and `mx convert -p` on the older version +run against a floor-off build. Convert strips undeclared keys and empties values +it does not understand, so compare values as well as key sets. + **mxcli sometimes generates the environment it then fails in.** The non-root devcontainer that could not provision PostgreSQL was scaffolded by `mxcli new` — the same code emits that base image, installs postgresql, and runs as `vscode`. diff --git a/mdl-examples/bug-tests/1163-aggregate-on-mendix9.mdl b/mdl-examples/bug-tests/1163-aggregate-on-mendix9.mdl new file mode 100644 index 0000000000..d13e79e3a1 --- /dev/null +++ b/mdl-examples/bug-tests/1163-aggregate-on-mendix9.mdl @@ -0,0 +1,36 @@ +mdl 1; +-- ako/mxcli#1163: on Mendix 9.x every count/sum/average/minimum/maximum in a +-- microflow was refused: +-- +-- this Mendix 9.24.30 project cannot store +-- Microflows$AggregateAction.AggregateFunction (Mendix 10.0.0+) +-- +-- The property floor came from the vendored generated version data, which had +-- given AggregateFunction the row of ReduceReturnDataType (the fold Reduce +-- added in 10.0) and left that one out. Studio Pro 9.24.0 stores +-- AggregateFunction on every aggregate (Workflow Commons 2.6.0), `mx convert` +-- 9.24.30 keeps it, and mendixmodelsdk gives it no version. +-- +-- Run it on a fresh 9.24.30 app (`mxcli new … --version 9.24.30.51827`): exec +-- writes all five, and `mxcli docker check` reports the blank app's error +-- count. An aggregate over an expression (`sum $L of …`) and `reduce`, `all` +-- and `any` are Mendix 10.0 and are still refused below it. + +create module Probe1163Agg; + +create persistent entity Probe1163Agg.Line ( + Amount: Decimal +); + +create microflow Probe1163Agg.Totals ( + $Lines: list of Probe1163Agg.Line +) +returns decimal as $Sum +begin + $Count = count $Lines; + $Sum = sum $Lines by Amount; + $Average = average $Lines by Amount; + $Min = minimum $Lines by Amount; + $Max = maximum $Lines by Amount; + return $Sum; +end; diff --git a/mdl-examples/bug-tests/1163-constant-override-on-mendix9.mdl b/mdl-examples/bug-tests/1163-constant-override-on-mendix9.mdl new file mode 100644 index 0000000000..a0d8db719a --- /dev/null +++ b/mdl-examples/bug-tests/1163-constant-override-on-mendix9.mdl @@ -0,0 +1,30 @@ +mdl 1; +-- ako/mxcli#1163: on Mendix 9.x and on 10.0–10.8 a new constant override in a +-- configuration was refused: +-- +-- this Mendix 9.24.30 project cannot store +-- Settings$ConstantValue.SharedOrPrivateValue (Mendix 10.9.0+) +-- +-- Before 10.9 an override's value is a flat Value string on the +-- Settings$ConstantValue; from 10.9 it is a Settings$SharedValue nested under +-- SharedOrPrivateValue (mendixmodelsdk). `mx convert` 9.24.30 turns the nested +-- form into `{$ID, $Type, ConstantId, Value: ""}`: it does not know it, and the +-- override silently becomes empty. mxcli builds the nested form and lowers it +-- on the write. A private value (stored on the developer's workstation) has no +-- flat form and is refused below 10.9. +-- +-- Run it on a fresh 9.24.30 app (`mxcli new … --version 9.24.30.51827`): exec, +-- then `describe settings` shows the override as 'broker:9092', and `mxcli +-- docker check` reports the blank app's error count. + +create module Probe1163Const; + +create constant Probe1163Const.ServerUrl ( Type: string, DefaultValue: 'localhost:9092' ); + +-- A new override: built nested, stored flat. +alter settings constant @Probe1163Const.ServerUrl value 'kafka:9092' + in configuration 'Default'; + +-- An existing flat override is updated in place. +alter settings constant @Probe1163Const.ServerUrl value 'broker:9092' + in configuration 'Default'; diff --git a/mdl-examples/bug-tests/1163-flow-arguments-on-mendix9.mdl b/mdl-examples/bug-tests/1163-flow-arguments-on-mendix9.mdl new file mode 100644 index 0000000000..8934125f3c --- /dev/null +++ b/mdl-examples/bug-tests/1163-flow-arguments-on-mendix9.mdl @@ -0,0 +1,50 @@ +mdl 1; +-- ako/mxcli#1163: on Mendix 9.x and on 10.0–10.9 a page passing +-- `$currentObject` to a microflow or nanoflow was refused: +-- +-- this Mendix 9.24.30 project cannot store +-- Forms$MicroflowParameterMapping.Expression (Mendix 10.10.0+) +-- +-- An argument stored as Expression text arrived in 10.10. Before it, a +-- mapping holds only a Variable (a Forms$PageVariable, required: a null one +-- makes 9.24 fail to load the page). An argument that is the enclosing object +-- is not stored at all, because Studio Pro 9.24 infers it: Workflow Commons +-- 2.6.0 (9.24.0) has 77 flow calls taking parameters and no mapping on any. +-- Below 10.10 mxcli therefore leaves `$currentObject` out and keeps a page +-- parameter as its Variable. Any other expression (a literal, an attribute +-- path) has no form there and is refused, naming the parameter. +-- +-- Run it on a fresh 9.24.30 app (`mxcli new … --version 9.24.30.51827`): exec, +-- then `mxcli docker check` reports the blank app's error count, and `mx +-- convert -p` changes no key on the page. + +create module Probe1163Args; + +create persistent entity Probe1163Args.Item ( + Name: String(100) +); + +create microflow Probe1163Args.ACT_Item ( $Item: Probe1163Args.Item ) +begin + log info node 'Probe1163Args' 'item'; +end; + +create nanoflow Probe1163Args.NF_Item ( $Item: Probe1163Args.Item ) +begin + log info node 'Probe1163Args' 'item'; +end; + +create page Probe1163Args.Item_Edit ( + Title: 'Item', + Layout: Atlas_Core.Atlas_Default, + Params: ( $Item: Probe1163Args.Item ) +) { + dataview dv (DataSource: $Item) { + actionbutton btnCurrent (Caption: 'Current', + Action: call microflow Probe1163Args.ACT_Item(Item = $currentObject)) + actionbutton btnNanoflow (Caption: 'Nanoflow', + Action: call nanoflow Probe1163Args.NF_Item(Item = $currentObject)) + } + actionbutton btnParam (Caption: 'Param', + Action: call microflow Probe1163Args.ACT_Item(Item = $Item)) +}; diff --git a/mdl-examples/bug-tests/1163-published-odata-below-10-4.mdl b/mdl-examples/bug-tests/1163-published-odata-below-10-4.mdl new file mode 100644 index 0000000000..d42e11fea8 --- /dev/null +++ b/mdl-examples/bug-tests/1163-published-odata-below-10-4.mdl @@ -0,0 +1,42 @@ +mdl 1; +-- ako/mxcli#1163: on Mendix 9.x a published OData service was refused with +-- a message naming the wrong thing: +-- +-- this Mendix 9.24.30 project cannot store +-- ODataPublish$PublishedODataService2.IncludeMetadataByDefault (Mendix 10.8.0+) +-- +-- That property is not the problem. The whole document mxcli writes, +-- ODataPublish$PublishedODataService2, was introduced in 10.4.0 +-- (mendixmodelsdk). Before 10.4 a published OData service is a +-- Rest$PublishedODataService, a different document that mxcli does not +-- write. With the floor off, mx check 9.24.30 cannot load the project +-- ("TypeCacheUnknownTypeException: ... ODataPublish$PublishedODataService2"), +-- so lowering the property would have produced an unloadable app. +-- +-- `mxcli check -p <9.24 app>` now reports MDL-VERSION01 "create +-- published odata service" before anything is written, and exec refuses the +-- same way. On 10.4 and later the service is written as before. + +create module Probe1163OData; + +create persistent entity Probe1163OData.Customer ( + Name: String(100), + Email: String(200) unique +); + +create published odata service Probe1163OData.CustomerAPI ( + Path: 'odata/customers/', + Version: '1.0.0', + ODataVersion: OData4, + Namespace: 'Probe1163OData.Customers', + ServiceName: 'Customer Service' +) +{ + publish entity Probe1163OData.Customer as 'Customers' ( + ReadMode: source + ) + expose ( + Email as 'customerId' (KEY, Filterable, Sortable), + Name + ); +}; diff --git a/mdl-examples/bug-tests/1163-workflow-on-mendix9.mdl b/mdl-examples/bug-tests/1163-workflow-on-mendix9.mdl new file mode 100644 index 0000000000..ecea673ad0 --- /dev/null +++ b/mdl-examples/bug-tests/1163-workflow-on-mendix9.mdl @@ -0,0 +1,55 @@ +mdl 1; +-- ako/mxcli#1163: on Mendix 9.x and on 10.0–10.20 every workflow was refused: +-- +-- this Mendix 9.24.30 project cannot store +-- Workflows$Workflow.PersistentId (Mendix 10.21.0+) +-- +-- Workflows exist from 9.0; PersistentId arrived in 10.21.0, on the workflow +-- and on every activity, outcome and boundary event (mendixmodelsdk). The +-- activities' and outcomes' were declared on abstract supertypes the property +-- floor does not look up, so they were never refused, and `mx convert` 9.24.30 +-- strips every one. A PersistentId is minted by the writer, never stated by a +-- script, so below 10.21 none is written: for CREATE, CREATE OR MODIFY and +-- the activities ALTER WORKFLOW inserts on its own. +-- +-- Run it on a fresh 9.24.30 app (`mxcli new … --version 9.24.30.51827`): exec, +-- then `mxcli docker check` reports the blank app's error count, and `mx +-- convert -p` strips no PersistentId. +-- +-- Not covered: a `parallel split`. Its paths end in a +-- Workflows$EndOfParallelSplitPathActivity, a type Mendix introduced in 10.21, +-- so below 10.21 the project does not load (TypeCacheUnknownTypeException). + +create module Probe1163Wf; + +create persistent entity Probe1163Wf.Request ( + Title: String(100), + Amount: Integer +); + +create microflow Probe1163Wf.ACT_Validate ($WorkflowContext: Probe1163Wf.Request) +begin + log info node 'Probe1163Wf' 'validate'; +end; + +create microflow Probe1163Wf.ACT_Notify ($WorkflowContext: Probe1163Wf.Request) +begin + log info node 'Probe1163Wf' 'notify'; +end; + +create workflow Probe1163Wf.WF_Request + parameter $WorkflowContext: Probe1163Wf.Request + display 'Request' +begin + call microflow Probe1163Wf.ACT_Validate; + decision $WorkflowContext/Amount > 1000 + outcomes + true -> { call microflow Probe1163Wf.ACT_Notify; } + false -> { end workflow; } + ; +end workflow; + +-- ALTER serializes the inserted activity on its own, outside CREATE's encoder. +alter workflow Probe1163Wf.WF_Request { + insert after ACT_Validate { call microflow Probe1163Wf.ACT_Notify; } +}; diff --git a/mdl-examples/bug-tests/1218-module-role-description-kept.mdl b/mdl-examples/bug-tests/1218-module-role-description-kept.mdl new file mode 100644 index 0000000000..4178383285 --- /dev/null +++ b/mdl-examples/bug-tests/1218-module-role-description-kept.mdl @@ -0,0 +1,34 @@ +mdl 1; +-- ako/mxcli#1218: a bare `create or modify module role M.R;` silently erased +-- the role's description. +-- +-- Reported: +-- $ mxcli exec probe.mdl -p Formula1Backend.mpr +-- Modified module role: Formula1Backend.ApiUser +-- and `show module roles` had the role's Description empty afterwards. The +-- executor handed the statement's empty description to the backend, which +-- overwrites; an omitted DESCRIPTION clause was read as "set it to ''". +-- +-- An omitted clause now keeps what is stored, as an omitted user role property +-- already did (ako/mxcli#707). `description ''` is the spelling that clears it. +-- +-- mxcli exec 1218-module-role-description-kept.mdl -p app.mpr +-- mxcli -p app.mpr -c "show module roles in Zz1218" +-- +-- After the run Zz1218.ApiUser has the description from statement 1, and +-- Zz1218.Cleared has none. Re-running the script reports `Unchanged` for the +-- bare statement and `Modified` only where it states a different description. +-- Measured on Mendix 11.13.0: `mx check` is 0 errors with the fault and +-- without it, so only the stored description shows the difference. +-- The unit test is mdl/executor/issue1218_module_role_description_test.go. + +create or modify module Zz1218; + +create or modify module role Zz1218.ApiUser description 'Read the published OData services. Nothing else.'; + +-- The "make sure this role exists" idiom: keeps the description above. +create or modify module role Zz1218.ApiUser; + +-- Stating an empty description is how a script clears one. +create or modify module role Zz1218.Cleared description 'Temporary'; +create or modify module role Zz1218.Cleared description ''; diff --git a/mdl-examples/bug-tests/test-runner-1448-own-error-handler.test.mdl b/mdl-examples/bug-tests/test-runner-1448-own-error-handler.test.mdl index 7bf7545c7e..1987183ff3 100644 --- a/mdl-examples/bug-tests/test-runner-1448-own-error-handler.test.mdl +++ b/mdl-examples/bug-tests/test-runner-1448-own-error-handler.test.mdl @@ -31,8 +31,11 @@ -- -- Under --legacy-runner a test whose error leaves the runner (05, 06, 11) ends -- the whole run; 05 was reported PASS there (a RUN line with no verdict read as --- the zero status) and is now an ERROR. That runner also needs a distinct --- variable name per test. +-- the zero status) and is now an ERROR. That runner also needed a distinct +-- variable name per test: it renamed a variable per test only after an +-- upper-case `CALL`, so this file's `$r` and `$Result` drew 14 MDL063 +-- duplicate-variable errors there. It renames one after a lower-case `call` too +-- since the sibling fix to ako/mxcli#1753. mdl 1; /** * @test 01 plain call that throws diff --git a/mdl-examples/bug-tests/test-runner-1753-earlier-expect-fails.test.mdl b/mdl-examples/bug-tests/test-runner-1753-earlier-expect-fails.test.mdl new file mode 100644 index 0000000000..02f7ab6845 --- /dev/null +++ b/mdl-examples/bug-tests/test-runner-1753-earlier-expect-fails.test.mdl @@ -0,0 +1,55 @@ +-- ako/mxcli#1753: on the after-startup runner (every Docker run, and --local +-- --legacy-runner) a test with more than one @expect was reported PASS when an +-- earlier assertion failed and a later one held, and `mxcli test` exited 0. +-- Each assertion logs its own PASS or FAIL line, and the log parser let the +-- last line be the test's verdict. A failure now sticks, and the first one is +-- the message reported. +-- +-- Setup: test-runner-1514-setup.mdl (Probe.SUB_Make returns 'a;b'). Then: +-- +-- mxcli test mdl-examples/bug-tests/test-runner-1753-earlier-expect-fails.test.mdl -p app.mpr --local --legacy-runner +-- +-- Expected: test 1 FAIL "Expected $a = 'zzz', actual: a;b" (before the fix: +-- PASS), test 2 FAIL with the same message for $b (the control — the order the +-- parser always got right), test 3 FAIL "Expected $c = 'zzz', actual: a;b (and +-- 1 more failure in this test)", test 4 PASS. The endpoint runner (--local +-- alone) evaluates an assertion only while the test is still passing, so it +-- reported test 1 correctly before the fix too. +-- +-- The log parser's verdicts are pinned by cmd/mxcli/testrunner/ +-- sticky_failure_test.go; this file is the runtime reproduction and was not run +-- against an app for the fix. Each test uses its own variable so the file does +-- not depend on the runner renaming a shared one. +mdl 1; +/** + * @test 1 earlier assertion fails, later one holds + * @expect $a = 'zzz' + * @expect $a = 'a;b' + */ +$a = call microflow Probe.SUB_Make(); +/ + +/** + * @test 2 earlier assertion holds, later one fails + * @expect $b = 'a;b' + * @expect $b = 'zzz' + */ +$b = call microflow Probe.SUB_Make(); +/ + +/** + * @test 3 two assertions fail, one holds + * @expect $c = 'zzz' + * @expect $c = 'a;b' + * @expect $c = 'yyy' + */ +$c = call microflow Probe.SUB_Make(); +/ + +/** + * @test 4 every assertion holds + * @expect $d = 'a;b' + * @expect length($d) = 3 + */ +$d = call microflow Probe.SUB_Make(); +/ diff --git a/mdl/ast/ast_security.go b/mdl/ast/ast_security.go index 18c3d38fc4..80e6f0328f 100644 --- a/mdl/ast/ast_security.go +++ b/mdl/ast/ast_security.go @@ -10,13 +10,26 @@ package ast type CreateModuleRoleStmt struct { CreateGuard // `create … if not exists` (ako/mxcli#731) Name QualifiedName - Description string + // Description is nil when the statement has no DESCRIPTION clause: a new + // role then has none, and `create or modify` keeps the stored one + // (ako/mxcli#1218), as CreateUserRoleStmt's properties do. `description ''` + // is the stated empty description, which clears it. + Description *string // CreateOrModify makes the statement idempotent: an existing role has its // description updated instead of the statement failing, so a security script // can be re-run. CreateOrModify bool } +// DescriptionText is the description the statement states, or "" when it +// states none. +func (s *CreateModuleRoleStmt) DescriptionText() string { + if s.Description == nil { + return "" + } + return *s.Description +} + func (s *CreateModuleRoleStmt) isStatement() {} // DropModuleRoleStmt represents: DROP MODULE ROLE [IF EXISTS] Module.RoleName diff --git a/mdl/backend/modelsdk/settings_mendix9_constant_value_test.go b/mdl/backend/modelsdk/settings_mendix9_constant_value_test.go new file mode 100644 index 0000000000..b574c42218 --- /dev/null +++ b/mdl/backend/modelsdk/settings_mendix9_constant_value_test.go @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/mdl/bsonutil" + "github.com/mendixlabs/mxcli/mdl/settingsoverlay" + "github.com/mendixlabs/mxcli/modelsdk/bsoncompat" + mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr" +) + +// seedFlatConstantValue gives the fixture's Default configuration one constant +// override in the shape Mendix stores below 10.9 (ako/mxcli#1163; `mx convert` +// 9.24.30): `{$ID, $Type, ConstantId, Value}`, the value a flat string. +func seedFlatConstantValue(t *testing.T, proj, constantID, value string) { + t.Helper() + r, err := mmpr.OpenWithOptions(proj, mmpr.OpenOptions{ReadOnly: false}) + if err != nil { + t.Fatalf("open fixture: %v", err) + } + defer r.Close() + refs, err := r.ListUnitsByType("Settings$ProjectSettings") + if err != nil || len(refs) != 1 { + t.Fatalf("ListUnitsByType(Settings$ProjectSettings) = %v, %v", refs, err) + } + raw, err := r.GetRawUnitBytes(refs[0].ID) + if err != nil { + t.Fatalf("GetRawUnitBytes: %v", err) + } + var doc bson.M + if err := bsoncompat.UnmarshalDocM(raw, &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + seeded := false + for _, part := range settingsoverlay.ArrayElements(doc["Settings"]) { + if part["$Type"] != "Settings$ConfigurationSettings" { + continue + } + cfgs := settingsoverlay.ArrayElements(part["Configurations"]) + if len(cfgs) == 0 { + t.Fatalf("fixture has no configurations to seed") + } + cfgs[0]["ConstantValues"] = bson.A{int32(3), bson.D{ + {Key: "$ID", Value: bsonutil.NewIDBsonBinary()}, + {Key: "$Type", Value: "Settings$ConstantValue"}, + {Key: "ConstantId", Value: constantID}, + {Key: "Value", Value: value}, + }} + seeded = true + } + if !seeded { + t.Fatalf("fixture has no Settings$ConfigurationSettings part") + } + contents, err := bson.Marshal(bsonutil.OrderStorageValue(doc)) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if err := mmpr.NewWriterWithReader(r).UpdateRawUnit(refs[0].ID, contents); err != nil { + t.Fatalf("UpdateRawUnit: %v", err) + } +} + +// A Mendix 9 constant override is read from its flat Value, and updating it +// keeps that shape rather than introducing the nested one 9.x does not have. +func TestProjectSettings_Mendix9FlatConstantValue(t *testing.T) { + proj := copyFixture(t) + const id = "MyFirstModule.ServerUrl" + seedFlatConstantValue(t, proj, id, "kafka:9092") + + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + ps, err := b.GetProjectSettings() + if err != nil { + t.Fatalf("GetProjectSettings: %v", err) + } + cvs := ps.Configuration.Configurations[0].ConstantValues + if len(cvs) != 1 || cvs[0].ConstantId != id || cvs[0].Value != "kafka:9092" { + t.Fatalf("flat override read as %+v, want %s = kafka:9092", cvs, id) + } + + cvs[0].Value = "broker:9092" + if err := b.UpdateProjectSettings(ps); err != nil { + t.Fatalf("UpdateProjectSettings: %v", err) + } + if err := b.Disconnect(); err != nil { + t.Fatalf("disconnect: %v", err) + } + after := settingsoverlay.ArrayElements(readConfiguration(t, proj)["ConstantValues"]) + if len(after) != 1 || after[0]["Value"] != "broker:9092" { + t.Fatalf("flat override after update = %#v, want Value broker:9092", after) + } + if _, nested := after[0]["SharedOrPrivateValue"]; nested { + t.Errorf("updating a flat override added the nested shape: %#v", after[0]) + } +} diff --git a/mdl/executor/cmd_security_write.go b/mdl/executor/cmd_security_write.go index 808f4354a4..3b8eccd8ad 100644 --- a/mdl/executor/cmd_security_write.go +++ b/mdl/executor/cmd_security_write.go @@ -44,7 +44,9 @@ func execCreateModuleRole(ctx *ExecContext, s *ast.CreateModuleRoleStmt) error { if mr.Description == autoDocumentRoleDescription { oldQualified := s.Name.Module + "." + mr.Name newQualified := s.Name.Module + "." + s.Name.Name - if err := ctx.Backend.AddModuleRole(ms.ID, s.Name.Name, s.Description); err != nil { + // Adopting the role takes it over: the marker description is not + // the author's documentation, so an unstated description clears it. + if err := ctx.Backend.AddModuleRole(ms.ID, s.Name.Name, s.DescriptionText()); err != nil { return mdlerrors.NewBackend("create module role", err) } // If the casing actually changed, propagate the rename across every @@ -64,8 +66,14 @@ func execCreateModuleRole(ctx *ExecContext, s *ast.CreateModuleRoleStmt) error { if s.CreateOrModify { // Re-running a security script must not fail on a role that is // already there. AddModuleRole overwrites, so this also adopts a new - // description and the caller's casing. - if err := ctx.Backend.AddModuleRole(ms.ID, s.Name.Name, s.Description); err != nil { + // description and the caller's casing. A statement without a + // description clause keeps the stored one: passing "" erased it, + // reported only as "Modified" (ako/mxcli#1218). + description := mr.Description + if s.Description != nil { + description = *s.Description + } + if err := ctx.Backend.AddModuleRole(ms.ID, s.Name.Name, description); err != nil { return mdlerrors.NewBackend("modify module role", err) } if !ctx.Quiet { @@ -76,7 +84,7 @@ func execCreateModuleRole(ctx *ExecContext, s *ast.CreateModuleRoleStmt) error { return mdlerrors.NewAlreadyExists("module role", s.Name.Module+"."+s.Name.Name) } - if err := ctx.Backend.AddModuleRole(ms.ID, s.Name.Name, s.Description); err != nil { + if err := ctx.Backend.AddModuleRole(ms.ID, s.Name.Name, s.DescriptionText()); err != nil { return mdlerrors.NewBackend("create module role", err) } diff --git a/mdl/executor/issue1218_module_role_description_test.go b/mdl/executor/issue1218_module_role_description_test.go new file mode 100644 index 0000000000..767bb64d09 --- /dev/null +++ b/mdl/executor/issue1218_module_role_description_test.go @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/security" +) + +// ako/mxcli#1218: "A bare `create or modify module role M.R;` silently erases +// the role's description". The statement reported "Modified module role" and +// the role's Description was gone: the executor handed the statement's empty +// description to AddModuleRole, which overwrites. An omitted clause keeps what +// is stored, as an omitted user role property already did (ako/mxcli#707); a +// description clause with an empty string is how a script clears it. +// +// The statements are parsed with the real visitor, so the test covers the +// whole chain from "the statement does not say" to the value written. +func TestIssue1218_CreateOrModifyModuleRole_KeepsUnstatedDescription(t *testing.T) { + const stored = "Read the published OData services. Nothing else." + cases := []struct { + name string + src string + stored *security.ModuleRole // nil: the role does not exist yet + want string // the description AddModuleRole writes + }{ + {"bare statement keeps the stored description", + "create or modify module role Shop.ApiUser;", + &security.ModuleRole{Name: "ApiUser", Description: stored}, stored}, + {"a stated description replaces it", + "create or modify module role Shop.ApiUser description 'Calls the API';", + &security.ModuleRole{Name: "ApiUser", Description: stored}, "Calls the API"}, + {"an empty description clears it", + "create or modify module role Shop.ApiUser description '';", + &security.ModuleRole{Name: "ApiUser", Description: stored}, ""}, + {"a casing change keeps the stored description", + "create or modify module role Shop.apiUser;", + &security.ModuleRole{Name: "ApiUser", Description: stored}, stored}, + {"or replace reads as or modify and keeps it too", + "mdl 1;\ncreate or replace module role Shop.ApiUser;", + &security.ModuleRole{Name: "ApiUser", Description: stored}, stored}, + // Adopting a role mxcli auto-provisioned takes it over: its marker + // description is not the author's documentation, so it is not kept. + {"adopting an auto-provisioned role drops its marker", + "create module role Shop.ApiUser;", + &security.ModuleRole{Name: "ApiUser", Description: autoDocumentRoleDescription}, ""}, + {"a new role without a description has none", + "create or modify module role Shop.ApiUser;", + nil, ""}, + {"a new role takes the stated description", + "create module role Shop.ApiUser description 'Calls the API';", + nil, "Calls the API"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + mod := &model.Module{BaseElement: model.BaseElement{ID: "mod-shop"}, Name: "Shop"} + ms := &security.ModuleSecurity{BaseElement: model.BaseElement{ID: "ms-shop"}, ContainerID: mod.ID} + if tc.stored != nil { + ms.ModuleRoles = []*security.ModuleRole{tc.stored} + } + var written []string + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + GetModuleSecurityFunc: func(model.ID) (*security.ModuleSecurity, error) { return ms, nil }, + AddModuleRoleFunc: func(_ model.ID, _ string, description string) error { + written = append(written, description) + return nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(mkHierarchy(mod))) + + prog, errs := visitor.Build(tc.src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + stmt := findStmt[*ast.CreateModuleRoleStmt](t, prog, tc.src) + if err := execCreateModuleRole(ctx, stmt); err != nil { + t.Fatalf("exec: %v\n%s", err, buf.String()) + } + if len(written) != 1 { + t.Fatalf("AddModuleRole called %d time(s), want 1\n%s", len(written), buf.String()) + } + if written[0] != tc.want { + t.Errorf("description written = %q, want %q\n%s", written[0], tc.want, buf.String()) + } + }) + } +} diff --git a/mdl/executor/issue707_describe_roundtrip_test.go b/mdl/executor/issue707_describe_roundtrip_test.go index 923c7868ac..296dcd9fbe 100644 --- a/mdl/executor/issue707_describe_roundtrip_test.go +++ b/mdl/executor/issue707_describe_roundtrip_test.go @@ -114,8 +114,8 @@ func TestIssue707_ModuleRoleDescriptionReparses(t *testing.T) { out := buf.String() stmt := findStmt[*ast.CreateModuleRoleStmt](t, reparse(t, out), out) - if stmt.Description != "Can't delete orders" { - t.Errorf("description: stored %q, re-exec stores %q\n%s", "Can't delete orders", stmt.Description, out) + if stmt.DescriptionText() != "Can't delete orders" { + t.Errorf("description: stored %q, re-exec stores %q\n%s", "Can't delete orders", stmt.DescriptionText(), out) } } diff --git a/mdl/executor/version_gates.go b/mdl/executor/version_gates.go index 8d6fe0ae5d..33514acad5 100644 --- a/mdl/executor/version_gates.go +++ b/mdl/executor/version_gates.go @@ -53,11 +53,18 @@ func statementVersionGates(stmt ast.Statement) []versionGate { return []versionGate{{"integration", "published_rest_grant_revoke", "grant access on published rest service", "upgrade your project to 10.0+"}} case *ast.CreateODataServiceStmt: + // The document mxcli writes, ODataPublish$PublishedODataService2, is + // 10.4+; before, a published OData service is a Rest$PublishedODataService, + // a different document mxcli does not write (ako/mxcli#1163). + gates := []versionGate{{"integration", "published_odata_service", "create published odata service", + "Before Mendix 10.4 a published OData service is a different document " + + "(Rest$PublishedODataService), which mxcli does not write. Publish it in Studio Pro, or upgrade the project."}} if s.SupportsGraphQL { - return []versionGate{{"integration", "odata_graphql", "SupportsGraphQL on a published OData service", + gates = append(gates, versionGate{"integration", "odata_graphql", "SupportsGraphQL on a published OData service", "Publishing a service as GraphQL as well arrived in Studio Pro 10.14. " + - "Remove SupportsGraphQL to publish OData only."}} + "Remove SupportsGraphQL to publish OData only."}) } + return gates case *ast.CreateMessageDefinitionStmt: return []versionGate{messageDefinitionDocumentGate("create message definition")} case *ast.AlterSettingsStmt: diff --git a/mdl/executor/version_gates_test.go b/mdl/executor/version_gates_test.go index 18b555be7b..7b320b2e05 100644 --- a/mdl/executor/version_gates_test.go +++ b/mdl/executor/version_gates_test.go @@ -32,6 +32,10 @@ func TestCheckVersionGates_ReportsEveryHandlerGate(t *testing.T) { {"alter published rest", &ast.AlterPublishedRestServiceStmt{}, [3]int{9, 24, 0}, "alter published rest service"}, {"grant published rest", &ast.GrantPublishedRestServiceAccessStmt{}, [3]int{9, 24, 0}, "grant access on published rest service"}, {"odata graphql", &ast.CreateODataServiceStmt{SupportsGraphQL: true}, [3]int{10, 13, 0}, "SupportsGraphQL"}, + // ako/mxcli#1163: the document mxcli writes is 10.4+; 9.24.30 does not + // load a project holding one (TypeCacheUnknownTypeException). + {"published odata", &ast.CreateODataServiceStmt{}, [3]int{9, 24, 30}, "create published odata service"}, + {"published odata 10.3", &ast.CreateODataServiceStmt{}, [3]int{10, 3, 0}, "create published odata service"}, {"message definition document", &ast.CreateMessageDefinitionStmt{}, [3]int{11, 14, 0}, "create message definition"}, {"workflow group", &ast.AlterSettingsStmt{Section: "WORKFLOWS", AddGroup: true, GroupName: "G"}, [3]int{11, 1, 0}, "add group"}, {"view entity", &ast.CreateViewEntityStmt{}, [3]int{10, 17, 0}, "create view entity"}, @@ -104,14 +108,13 @@ func TestCheckVersionGates_ReportsEveryHandlerGate(t *testing.T) { // Statements with no gate, or a gated statement without the gated clause, are // not reported — `synchronize all` on 9.0, a page without parameters, OData -// without GraphQL. +// without GraphQL from 10.4. func TestCheckVersionGates_UngatedFormsPass(t *testing.T) { prog := &ast.Program{Statements: []ast.Statement{ &ast.CreateModuleStmt{Name: "M"}, &ast.CreateMicroflowStmt{}, &ast.CreateNanoflowStmt{Body: []ast.MicroflowStatement{&ast.SynchronizeStmt{SyncType: "All"}}}, &ast.CreatePageStmtV3{}, - &ast.CreateODataServiceStmt{}, &ast.AlterSettingsStmt{Section: "MODEL"}, // No EXPORT LEVEL clause: the stored level is carried (#1498). &ast.CreateEntityStmt{Attributes: []ast.Attribute{{Name: "A"}}}, @@ -122,6 +125,10 @@ func TestCheckVersionGates_UngatedFormsPass(t *testing.T) { if vs := versionGateViolations(versionCtx(t, 9, 0, 0), prog); len(vs) != 0 { t.Errorf("ungated forms reported on 9.0: %+v", vs) } + odata := &ast.Program{Statements: []ast.Statement{&ast.CreateModuleStmt{Name: "M"}, &ast.CreateODataServiceStmt{}}} + if vs := versionGateViolations(versionCtx(t, 10, 4, 0), odata); len(vs) != 0 { + t.Errorf("a published OData service without GraphQL reported on 10.4: %+v", vs) + } } // One source of truth: a handler that called checkFeature with its own feature diff --git a/mdl/settingsoverlay/settingsoverlay.go b/mdl/settingsoverlay/settingsoverlay.go index a11bfa3757..1434bf9179 100644 --- a/mdl/settingsoverlay/settingsoverlay.go +++ b/mdl/settingsoverlay/settingsoverlay.go @@ -343,9 +343,11 @@ func SetWebUISettings(ws *model.WebUISettings, raw map[string]any) map[string]an // ConstantValues rebuilds a configuration's ConstantValues list, updating each // override in the slot it is already stored in so its value shape survives. -// Studio Pro and mxbuild only read the nested SharedOrPrivateValue; a flat "Value" -// is a legacy shape mxcli's reader tolerates but the platform ignores, so writing -// every override flat made all constant overrides look empty in Studio Pro. +// From Mendix 10.9 Studio Pro and mxbuild only read the nested +// SharedOrPrivateValue, so writing every override flat made all constant +// overrides look empty in Studio Pro. Below 10.9 the flat "Value" is the only +// shape: a new override is still built nested here, and the write lowers it +// (canon.LowerToProjectShape, ako/mxcli#1163). func ConstantValues(cvs []*model.ConstantValue, raw any) bson.A { rawValues := ArrayElements(raw) byConstant := make(map[string]map[string]any, len(rawValues)) diff --git a/mdl/visitor/module_role_create_statement_test.go b/mdl/visitor/module_role_create_statement_test.go index abdc78eabc..6aaaf42964 100644 --- a/mdl/visitor/module_role_create_statement_test.go +++ b/mdl/visitor/module_role_create_statement_test.go @@ -35,7 +35,7 @@ func TestCreateModuleRole_IsACreateStatementKind(t *testing.T) { if !ok { t.Fatalf("%s: got %T", tc.src, prog.Statements[0]) } - if s.Name.String() != "M.Admin" || s.CreateOrModify != tc.orModify || s.Description != tc.desc { + if s.Name.String() != "M.Admin" || s.CreateOrModify != tc.orModify || s.DescriptionText() != tc.desc { t.Errorf("%s: got %+v", tc.src, s) } if got := len(prog.Deprecations) > 0; got != tc.deprecated { diff --git a/mdl/visitor/visitor_security.go b/mdl/visitor/visitor_security.go index 0defad6da3..3f00edd350 100644 --- a/mdl/visitor/visitor_security.go +++ b/mdl/visitor/visitor_security.go @@ -27,7 +27,10 @@ func (b *Builder) ExitCreateModuleRoleStatement(ctx *parser.CreateModuleRoleStat } if ctx.DESCRIPTION() != nil { if sl := ctx.STRING_LITERAL(); sl != nil { - stmt.Description = unquoteStringLit(sl) + // A pointer, so `description ''` (clear it) and no clause at all + // (keep what is stored) stay different statements (ako/mxcli#1218). + d := unquoteStringLit(sl) + stmt.Description = &d } } b.statements = append(b.statements, stmt) diff --git a/mdl/visitor/visitor_security_or_modify_test.go b/mdl/visitor/visitor_security_or_modify_test.go index 22521e4043..f38cbed148 100644 --- a/mdl/visitor/visitor_security_or_modify_test.go +++ b/mdl/visitor/visitor_security_or_modify_test.go @@ -42,8 +42,8 @@ func TestCreateModuleRole_OrModify(t *testing.T) { if got.Name.Name != tc.wantName { t.Errorf("Name = %q, want %q", got.Name.Name, tc.wantName) } - if got.Description != tc.desc { - t.Errorf("Description = %q, want %q", got.Description, tc.desc) + if got.DescriptionText() != tc.desc { + t.Errorf("Description = %q, want %q", got.DescriptionText(), tc.desc) } }) } diff --git a/mdl/visitor/visitor_security_test.go b/mdl/visitor/visitor_security_test.go index 440717b87e..00e783c1ad 100644 --- a/mdl/visitor/visitor_security_test.go +++ b/mdl/visitor/visitor_security_test.go @@ -28,8 +28,25 @@ func TestCreateModuleRole(t *testing.T) { if stmt.Name.Module != "MyModule" || stmt.Name.Name != "Editor" { t.Errorf("Expected MyModule.Editor, got %s.%s", stmt.Name.Module, stmt.Name.Name) } - if stmt.Description != "Can edit content" { - t.Errorf("Expected description 'Can edit content', got %q", stmt.Description) + if stmt.DescriptionText() != "Can edit content" { + t.Errorf("Expected description 'Can edit content', got %q", stmt.DescriptionText()) + } +} + +// A description clause with an empty string states an empty description, which +// clears a stored one under `create or modify`; no clause states none, which +// keeps it (ako/mxcli#1218). +func TestCreateModuleRole_EmptyDescriptionIsStated(t *testing.T) { + prog, errs := Build(`create or modify module role MyModule.Viewer description '';`) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + stmt, ok := prog.Statements[0].(*ast.CreateModuleRoleStmt) + if !ok { + t.Fatalf("Expected CreateModuleRoleStmt, got %T", prog.Statements[0]) + } + if stmt.Description == nil || *stmt.Description != "" { + t.Errorf("Description = %v, want a stated empty string", stmt.Description) } } @@ -46,8 +63,8 @@ func TestCreateModuleRole_NoDescription(t *testing.T) { if !ok { t.Fatalf("Expected CreateModuleRoleStmt, got %T", prog.Statements[0]) } - if stmt.Description != "" { - t.Errorf("Expected empty description, got %q", stmt.Description) + if stmt.Description != nil { + t.Errorf("Expected no description clause, got %q", *stmt.Description) } } diff --git a/modelsdk/canon/studiopro_property_defaults.json b/modelsdk/canon/studiopro_property_defaults.json index 5ceeade35b..f05785e613 100644 --- a/modelsdk/canon/studiopro_property_defaults.json +++ b/modelsdk/canon/studiopro_property_defaults.json @@ -109,6 +109,6 @@ {"type":"Workflows$InterruptingTimerBoundaryEvent","key":"Annotation","since":"11.6.8","seen":"11.15.0","value":{"v":null}}, {"type":"Workflows$NonInterruptingTimerBoundaryEvent","key":"Annotation","since":"11.6.8","seen":"11.15.0","value":{"v":null}}, {"type":"Workflows$NonInterruptingTimerEventSubProcessStartActivity","key":"Recurrence","since":"11.15.0","seen":"11.15.0","value":{"v":null}}, - {"type":"Workflows$Workflow","key":"UsertaskOnStateChangeEvent","since":"10.24.24","until":"10.25.0","seen":"10.24.24","value":{"v":null}}, - {"type":"Workflows$Workflow","key":"WorkflowOnStateChangeEvent","since":"10.24.24","until":"10.25.0","seen":"10.24.24","value":{"v":null}} + {"type":"Workflows$Workflow","key":"UsertaskOnStateChangeEvent","since":"9.24.30","until":"10.25.0","seen":"10.24.24","value":{"v":null}}, + {"type":"Workflows$Workflow","key":"WorkflowOnStateChangeEvent","since":"9.24.30","until":"10.25.0","seen":"10.24.24","value":{"v":null}} ] diff --git a/modelsdk/canon/versionshape.go b/modelsdk/canon/versionshape.go index fb1efebde7..0ee2012481 100644 --- a/modelsdk/canon/versionshape.go +++ b/modelsdk/canon/versionshape.go @@ -4,6 +4,7 @@ package canon import ( "fmt" + "strings" "go.mongodb.org/mongo-driver/v2/bson" @@ -37,6 +38,21 @@ import ( // CE6050 "The selected entity must be configured with a page parameter". // - ForceFullObjects, declared on the abstract Forms$DataSource from 10.8.0: // every data source carried it, and the floor never saw it. +// - Settings$ConstantValue: a Settings$SharedValue under SharedOrPrivateValue +// from 10.9.0; before, the value as a flat Value string, `{$ID, $Type, +// ConstantId, Value}` (`mx convert` 9.24.30, which empties the nested form +// it does not know). +// - Forms$MicroflowParameterMapping / NanoflowParameterMapping: an argument +// as Expression text from 10.10.0; before, only a Variable (a required +// Forms$PageVariable), and an argument that is the enclosing object is not +// stored at all — Studio Pro 9.24 infers it (Workflow Commons 2.6.0: 77 +// flow calls taking parameters, no mapping on any). mxcli writes that +// argument as Expression `$currentObject` with Variable null, which 9.24 +// cannot even load. +// - Workflows$ PersistentId, from 10.21.0 on the workflow and on the abstract +// WorkflowActivity, Outcome and BoundaryEvent (ako/mxcli#1163). Workflows +// exist from 9.0; `mx convert` 9.24.30 strips every one. The workflow's +// own was refused; the others slipped past the floor. // // It runs on every unit write, before the floor, so no writer needs to know // these shapes. What the older shape cannot hold — a design property value @@ -53,8 +69,12 @@ func LowerToProjectShape(contents []byte, pv *version.Version) ([]byte, error) { textPlaceholder: !PropertyDeclared("Forms$AttributeWidgetWithPlaceholder", "PlaceholderTemplate", *pv), namedParameter: !PropertyDeclared("Forms$EntityPathSource", "SourceVariable", *pv), noFullObjects: !PropertyDeclared("Forms$DataSource", "ForceFullObjects", *pv), + noPersistentID: !PropertyDeclared("Workflows$Workflow", "PersistentId", *pv), + flatConstant: !PropertyDeclared("Settings$ConstantValue", "SharedOrPrivateValue", *pv), + variableArgs: !PropertyDeclared("Forms$MicroflowParameterMapping", "Expression", *pv), } - if !l.flatDesignProps && !l.boolEditable && !l.textPlaceholder && !l.namedParameter && !l.noFullObjects { + if !l.flatDesignProps && !l.boolEditable && !l.textPlaceholder && !l.namedParameter && !l.noFullObjects && + !l.noPersistentID && !l.flatConstant && !l.variableArgs { return contents, nil } var doc bson.D @@ -74,7 +94,8 @@ func LowerToProjectShape(contents []byte, pv *version.Version) ([]byte, error) { type shapeLowering struct { pv version.Version flatDesignProps, boolEditable, textPlaceholder bool - namedParameter, noFullObjects bool + namedParameter, noFullObjects, noPersistentID bool + flatConstant, variableArgs bool changed bool } @@ -106,12 +127,24 @@ func (l *shapeLowering) walk(v any) (any, error) { if err = l.lowerPlaceholder(x, typ); err != nil { return nil, err } + case l.flatConstant && typ == "Settings$ConstantValue": + if x, err = l.lowerConstantValue(x); err != nil { + return nil, err + } } if l.noFullObjects { if x, err = l.dropForceFullObjects(x, typ); err != nil { return nil, err } } + if l.noPersistentID && strings.HasPrefix(typ, "Workflows$") { + x = l.dropPersistentID(x) + } + if l.variableArgs { + if err = l.lowerFlowArguments(x); err != nil { + return nil, err + } + } for i := range x { if x[i].Value, err = l.walk(x[i].Value); err != nil { return nil, err @@ -261,6 +294,90 @@ func (l *shapeLowering) dropForceFullObjects(d bson.D, typ string) (bson.D, erro return append(append(bson.D{}, d[:i]...), d[i+1:]...), nil } +// lowerConstantValue replaces a constant override's nested SharedOrPrivateValue +// with the flat Value older versions store, in its place. A flat Value already +// beside it (the overlay clears one to "" when it writes the nested form) takes +// the nested value. A private value — kept on the developer's workstation, which +// the flat form cannot say — is refused. +func (l *shapeLowering) lowerConstantValue(d bson.D) (bson.D, error) { + i := shapeIndex(d, "SharedOrPrivateValue") + if i < 0 { + return d, nil + } + inner, _ := d[i].Value.(bson.D) + if t, _ := shapeString(inner, "$Type"); t == "Settings$PrivateValue" { + constant, _ := shapeString(d, "ConstantId") + return nil, fmt.Errorf("constant %q has a private value in this configuration, which Mendix introduced in 10.9 "+ + "(this project is %s): a constant value there is shared, stored in the model", constant, l.pv) + } + value, _ := shapeString(inner, "Value") + l.changed = true + if j := shapeIndex(d, "Value"); j >= 0 { + d[j].Value = value + return append(append(bson.D{}, d[:i]...), d[i+1:]...), nil + } + d[i] = bson.E{Key: "Value", Value: value} + return d, nil +} + +var flowParameterMappings = map[string]bool{ + "Forms$MicroflowParameterMapping": true, "Forms$NanoflowParameterMapping": true, +} + +// lowerFlowArguments rewrites the flow call arguments in d's ParameterMappings +// list, if it has one, into what a version before 10.10 stores: an argument +// bound through Variable keeps it (its empty Expression is the floor's to +// drop); an argument that is the enclosing object (`$currentObject`) — or one +// that binds nothing — is left out, which is how Studio Pro 9.24 stores it. +// Any other expression has no Variable form and is refused, naming the +// parameter. +func (l *shapeLowering) lowerFlowArguments(d bson.D) error { + i := shapeIndex(d, "ParameterMappings") + if i < 0 { + return nil + } + list, _ := d[i].Value.(bson.A) + out := bson.A{} + dropped := false + for _, el := range list { + m, ok := el.(bson.D) + typ, _ := shapeString(m, "$Type") + if !ok || !flowParameterMappings[typ] { + out = append(out, el) + continue + } + if v, _ := shapeValue(m, "Variable"); v != nil { + out = append(out, el) + continue + } + expr, _ := shapeString(m, "Expression") + if e := strings.TrimSpace(expr); e != "" && !strings.EqualFold(e, "$currentObject") { + param, _ := shapeString(m, "Parameter") + return fmt.Errorf("the argument for %s is the expression %s, which Mendix introduced in 10.10 (this project is %s): "+ + "an argument there is the enclosing object, or a page parameter, snippet parameter or widget", param, expr, l.pv) + } + dropped = true + } + if dropped { + d[i].Value = out + l.changed = true + } + return nil +} + +// dropPersistentID removes a workflow element's PersistentId below 10.21, where +// none declares it. Whatever it holds is dropped, not refused: a PersistentId is +// minted by the writer (a fresh GUID on every build), never stated by a script, +// and a workflow Studio Pro 9.24 stores has none. +func (l *shapeLowering) dropPersistentID(d bson.D) bson.D { + i := shapeIndex(d, "PersistentId") + if i < 0 { + return d + } + l.changed = true + return append(append(bson.D{}, d[:i]...), d[i+1:]...) +} + // listItems returns the elements of a stored list, without its type marker. func listItems(v any) bson.A { a, _ := v.(bson.A) diff --git a/modelsdk/gen/microflows/version.go b/modelsdk/gen/microflows/version.go index 01eea8f039..dcd1c81620 100644 --- a/modelsdk/gen/microflows/version.go +++ b/modelsdk/gen/microflows/version.go @@ -45,9 +45,15 @@ var VersionInfos = map[string]version.TypeVersionInfo{ }, "Microflows$AggregateAction": { Properties: map[string]version.PropertyVersionInfo{ - "aggregateFunction": {Introduced: "10.0.0", Required: true}, + // Fixed by hand (ako/mxcli#1163): the vendored generator gave + // aggregateFunction the row of reduceReturnDataType, which it then + // left out. mendixmodelsdk 4.117.0 has aggregateFunction with no + // version and reduceReturnDataType introduced 10.0.0 and required; + // Studio Pro 9.24 stores AggregateFunction (Workflow Commons 2.6.0). + "aggregateFunction": {}, "expression": {Introduced: "10.0.0"}, "reduceInitialValueExpression": {Introduced: "10.0.0"}, + "reduceReturnDataType": {Introduced: "10.0.0", Required: true}, "useExpression": {Introduced: "10.0.0"}, }, }, diff --git a/modelsdk/mpr/writer_version_shape_1163_test.go b/modelsdk/mpr/writer_version_shape_1163_test.go new file mode 100644 index 0000000000..3d33cfcb7b --- /dev/null +++ b/modelsdk/mpr/writer_version_shape_1163_test.go @@ -0,0 +1,389 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mpr + +import ( + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github.com/mendixlabs/mxcli/modelsdk/mpr/version" +) + +// ako/mxcli#1163: what the 9.24 doctype run found mxcli writing that Mendix 9 +// stores differently, or not at all. Each case is the document as mxcli builds +// it, normalized for the project's version the way every unit write is. + +// aggregateMicroflow is a microflow holding one Aggregate list activity as the +// writer builds it: Count/Sum/… carry no fold; Reduce, All and Any carry +// ReduceInitialValueExpression and ReduceReturnDataType. +func aggregateMicroflow(function string) bson.D { + action := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Microflows$AggregateAction"}, + {Key: "AggregateFunction", Value: function}, + {Key: "AggregateVariableName", Value: "Items"}, + {Key: "Attribute", Value: "M.Item.Price"}, + {Key: "ErrorHandlingType", Value: "Rollback"}} + switch function { + case "Reduce", "All", "Any": + initial := "" + if function == "Reduce" { + initial = "0" + } + action = append(action, + bson.E{Key: "ReduceInitialValueExpression", Value: initial}, + bson.E{Key: "ReduceReturnDataType", Value: bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "DataTypes$BooleanType"}}}) + } + action = append(action, bson.E{Key: "VariableName", Value: "Total"}) + activity := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Microflows$ActionActivity"}, {Key: "Action", Value: action}} + return bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Microflows$Microflow"}, {Key: "Name", Value: "Agg"}, + {Key: "ObjectCollection", Value: bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Microflows$MicroflowObjectCollection"}, + {Key: "Objects", Value: bson.A{int32(3), activity}}}}} +} + +// Microflows$AggregateAction.AggregateFunction is declared on every version +// mxcli writes. The generated version data floored it at 10.0.0, so every +// count/sum/average/minimum/maximum on 9.x was refused ("cannot store +// Microflows$AggregateAction.AggregateFunction (Mendix 10.0.0+)"). Studio Pro +// 9.24.0 stores it (Workflow Commons 2.6.0: 10 of 10 aggregates, `Count`), `mx +// convert` 9.24.30 keeps it, and mendixmodelsdk gives it no version at all; the +// 10.0.0 floor belongs to ReduceReturnDataType, the fold Reduce added. +func TestNormalize_AggregateFunctionIsDeclaredOnMendix9(t *testing.T) { + for _, v := range []*version.ProjectVersion{pv(9, 0, 5), pv(9, 24, 30), pv(10, 0, 0), pv(11, 13, 0)} { + d, err := normalizeShape(t, v, aggregateMicroflow("Sum")) + if err != nil { + t.Errorf("%d.%d: a sum aggregate is refused: %v", v.MajorVersion, v.MinorVersion, err) + continue + } + if f, _ := get(find(d, "Microflows$AggregateAction"), "AggregateFunction"); f != "Sum" { + t.Errorf("%d.%d: AggregateFunction = %v, want Sum", v.MajorVersion, v.MinorVersion, f) + } + } +} + +// The fold is what Mendix 10.0 added (mendixmodelsdk: ReduceReturnDataType +// introduced 10.0.0). `all` carries an empty initial value, so the fold's +// return type is the one key that says it: on 9.24 it is refused, naming it, +// rather than written into a document Studio Pro 9 cannot resolve. Before, the +// generated data had no row for it and it passed through. +func TestNormalize_ReduceFoldIsRefusedBelowMendix10(t *testing.T) { + for _, fn := range []string{"Reduce", "All"} { + if _, err := normalizeShape(t, pv(9, 24, 30), aggregateMicroflow(fn)); err == nil || + !strings.Contains(err.Error(), "Microflows$AggregateAction.ReduceReturnDataType (Mendix 10.0.0+)") { + t.Errorf("%s on 9.24 was not refused naming its fold's return type: %v", fn, err) + } + d, err := normalizeShape(t, pv(10, 0, 0), aggregateMicroflow(fn)) + if err != nil { + t.Fatalf("%s on 10.0 is refused: %v", fn, err) + } + if _, ok := get(find(d, "Microflows$AggregateAction"), "ReduceReturnDataType"); !ok { + t.Errorf("%s on 10.0 lost ReduceReturnDataType", fn) + } + } +} + +// pid is a fresh PersistentId: a non-zero GUID, as the writer mints one (a zero +// GUID is an empty value, which the floor would drop rather than refuse). +func pid() bson.Binary { + pidSeq++ + return bson.Binary{Subtype: 0, Data: []byte{pidSeq, 0x11, 2, 3, 4, 5, 0x46, 7, 0x88, 9, 10, 11, 12, 13, 14, 15}} +} + +var pidSeq byte + +// workflowWithPersistentIDs is a workflow as mxcli builds it: a fresh +// PersistentId on the workflow, on every activity and on every outcome. +func workflowWithPersistentIDs() bson.D { + outcome := func(value bool) bson.D { + return bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Workflows$BooleanConditionOutcome"}, + {Key: "Flow", Value: bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Workflows$Flow"}, {Key: "Activities", Value: bson.A{int32(3)}}}}, + {Key: "PersistentId", Value: pid()}, {Key: "Value", Value: value}} + } + split := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Workflows$ExclusiveSplitActivity"}, + {Key: "Caption", Value: "Active?"}, {Key: "Expression", Value: "$WorkflowContext/Active"}, {Key: "Name", Value: "split"}, + {Key: "Outcomes", Value: bson.A{int32(3), outcome(true), outcome(false)}}, + {Key: "PersistentId", Value: pid()}} + call := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Workflows$CallMicroflowTask"}, + {Key: "BoundaryEvents", Value: bson.A{int32(2)}}, {Key: "Caption", Value: "Validate"}, + {Key: "Microflow", Value: "M.Validate"}, {Key: "Name", Value: "call"}, {Key: "PersistentId", Value: pid()}} + return bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Workflows$Workflow"}, + {Key: "Flow", Value: bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Workflows$Flow"}, + {Key: "Activities", Value: bson.A{int32(3), call, split}}}}, + {Key: "Name", Value: "WF"}, + {Key: "PersistentId", Value: pid()}, + {Key: "Title", Value: "WF"}} +} + +// persistentIDHolders returns the $Type of every element in v carrying a +// PersistentId. +func persistentIDHolders(v any) []string { + var out []string + switch x := v.(type) { + case bson.D: + typ, _ := get(x, "$Type") + for _, e := range x { + if e.Key == "PersistentId" { + out = append(out, typ.(string)) + } + out = append(out, persistentIDHolders(e.Value)...) + } + case bson.A: + for _, el := range x { + out = append(out, persistentIDHolders(el)...) + } + } + return out +} + +// Workflows exist from 9.0, PersistentId from 10.21.0 — on the workflow and on +// WorkflowActivity, Outcome and BoundaryEvent (mendixmodelsdk). Below that a +// workflow was refused ("cannot store Workflows$Workflow.PersistentId (Mendix +// 10.21.0+)"), and the activities' and outcomes' were never seen by the floor, +// which does not look up the abstract supertypes that declare them: `mx +// convert` 9.24.30 strips all of them. A PersistentId is minted by the writer, +// never stated by a script, so below 10.21 it is not written. +func TestNormalize_WorkflowPersistentIDsBelowMendix10_21(t *testing.T) { + for _, tt := range []struct { + v *version.ProjectVersion + want int + }{ + {pv(9, 24, 30), 0}, + {pv(10, 20, 0), 0}, + {pv(10, 21, 0), 5}, + {pv(10, 24, 24), 5}, + {pv(11, 13, 0), 5}, // control + } { + d, err := normalizeShape(t, tt.v, workflowWithPersistentIDs()) + if err != nil { + t.Errorf("%d.%d: a workflow is refused: %v", tt.v.MajorVersion, tt.v.MinorVersion, err) + continue + } + if got := persistentIDHolders(d); len(got) != tt.want { + t.Errorf("%d.%d: PersistentId on %d elements %v, want %d", tt.v.MajorVersion, tt.v.MinorVersion, len(got), got, tt.want) + } + } +} + +// settingsWithConstantValue is the project settings unit as `alter settings +// constant … in configuration` writes a new override: the value nested in a +// Settings$SharedValue (or a Settings$PrivateValue, which holds none). flat is +// the empty flat Value the overlay leaves beside a nested one, "-" for none. +func settingsWithConstantValue(valueType, value, flat string) bson.D { + inner := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: valueType}} + if valueType == "Settings$SharedValue" { + inner = append(inner, bson.E{Key: "Value", Value: value}) + } + cv := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Settings$ConstantValue"}, + {Key: "ConstantId", Value: "M.ServerUrl"}, {Key: "SharedOrPrivateValue", Value: inner}} + if flat != "-" { + cv = append(cv, bson.E{Key: "Value", Value: flat}) + } + config := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Settings$ServerConfiguration"}, + {Key: "ConstantValues", Value: bson.A{int32(3), cv}}, {Key: "Name", Value: "Default"}} + part := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Settings$ConfigurationSettings"}, + {Key: "Configurations", Value: bson.A{int32(3), config}}} + return bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Settings$ProjectSettings"}, + {Key: "Settings", Value: bson.A{int32(2), part}}} +} + +// A configuration's constant override is a flat Value string below 10.9 and a +// Settings$SharedValue under SharedOrPrivateValue from it (mendixmodelsdk: +// Value deleted, SharedOrPrivateValue introduced 10.9.0). `mx convert` 9.24.30 +// turns the nested form into `{$ID, $Type, ConstantId, Value: ""}` — it does +// not know the nested value, so the override silently became empty, and mxcli +// refused it ("cannot store Settings$ConstantValue.SharedOrPrivateValue +// (Mendix 10.9.0+)"). +func TestNormalize_ConstantValueIsFlatBelowMendix10_9(t *testing.T) { + for _, tt := range []struct { + v *version.ProjectVersion + flat bool + }{ + {pv(9, 24, 30), true}, + {pv(10, 8, 0), true}, + {pv(10, 9, 0), false}, + {pv(11, 13, 0), false}, // control + } { + for _, flat := range []string{"-", ""} { + d, err := normalizeShape(t, tt.v, settingsWithConstantValue("Settings$SharedValue", "kafka:9092", flat)) + if err != nil { + t.Errorf("%d.%d: a constant override is refused: %v", tt.v.MajorVersion, tt.v.MinorVersion, err) + continue + } + cv := find(d, "Settings$ConstantValue") + if !tt.flat { + if _, ok := get(cv, "SharedOrPrivateValue"); !ok { + t.Errorf("%d.%d: SharedOrPrivateValue was lowered: %v", tt.v.MajorVersion, tt.v.MinorVersion, keysOf(cv)) + } + continue + } + if got := strings.Join(keysOf(cv), ","); got != "$ID,$Type,ConstantId,Value" { + t.Errorf("%d.%d: constant value keys = %s, want $ID,$Type,ConstantId,Value", tt.v.MajorVersion, tt.v.MinorVersion, got) + } + if v, _ := get(cv, "Value"); v != "kafka:9092" { + t.Errorf("%d.%d: Value = %v, want kafka:9092", tt.v.MajorVersion, tt.v.MinorVersion, v) + } + } + } +} + +// A private value lives on the developer's workstation, which a flat Value +// cannot say: refused, naming the constant, not written as an empty override. +func TestNormalize_RefusesAPrivateConstantValueBelowMendix10_9(t *testing.T) { + _, err := normalizeShape(t, pv(9, 24, 30), settingsWithConstantValue("Settings$PrivateValue", "", "-")) + if err == nil || !strings.Contains(err.Error(), "M.ServerUrl") { + t.Errorf("a private constant value on 9.24 was not refused naming the constant: %v", err) + } +} + +// flowMapping is one argument of a page's flow call as mxcli builds it +// (bindParameterMappingValue): a page-variable reference under Variable with an +// empty Expression, anything else as Expression text with Variable null. +func flowMapping(mappingType, param, expression, pageParam string) bson.D { + var variable any + if pageParam != "" { + variable = bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Forms$PageVariable"}, + {Key: "PageParameter", Value: pageParam}, {Key: "SnippetParameter", Value: ""}, + {Key: "UseAllPages", Value: false}, {Key: "Widget", Value: ""}} + } + return bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: mappingType}, + {Key: "Expression", Value: expression}, {Key: "Parameter", Value: param}, {Key: "Variable", Value: variable}} +} + +// pageWithFlowCalls is a page with a microflow button and a nanoflow button, +// each passing the given mappings. +func pageWithFlowCalls(microflowMappings, nanoflowMappings []bson.D) bson.D { + list := func(ms []bson.D) bson.A { + a := bson.A{int32(2)} + for _, m := range ms { + a = append(a, m) + } + return a + } + mfButton := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Forms$ActionButton"}, {Key: "Name", Value: "btnMf"}, + {Key: "Action", Value: bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Forms$MicroflowAction"}, + {Key: "MicroflowSettings", Value: bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Forms$MicroflowSettings"}, + {Key: "Microflow", Value: "M.ACT"}, {Key: "ParameterMappings", Value: list(microflowMappings)}}}}}} + nfButton := bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Forms$ActionButton"}, {Key: "Name", Value: "btnNf"}, + {Key: "Action", Value: bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Forms$CallNanoflowClientAction"}, + {Key: "Nanoflow", Value: "M.NF"}, {Key: "ParameterMappings", Value: list(nanoflowMappings)}}}} + return bson.D{{Key: "$ID", Value: bid()}, {Key: "$Type", Value: "Forms$Page"}, {Key: "Name", Value: "P"}, + {Key: "Widgets", Value: bson.A{int32(2), mfButton, nfButton}}} +} + +// mappingsOf returns the mappings stored under the first $Type holder in d. +func mappingsOf(t *testing.T, d bson.D, holder string) []bson.D { + t.Helper() + list, _ := get(find(d, holder), "ParameterMappings") + var out []bson.D + for _, el := range listItems(list) { + out = append(out, el.(bson.D)) + } + return out +} + +func listItems(v any) bson.A { + a, _ := v.(bson.A) + if len(a) > 0 { + if _, marker := a[0].(int32); marker { + return a[1:] + } + } + return a +} + +// A flow argument on a page is a Variable (a Forms$PageVariable, required) below +// 10.10, and Expression text only from 10.10 (mendixmodelsdk). mxcli writes +// `$currentObject` as Expression with Variable null: the floor refused it on +// 9.24 ("cannot store Forms$MicroflowParameterMapping.Expression (Mendix +// 10.10.0+)"), and without the floor 9.24 does not load the page at all ("An +// error occurred when trying to set the 'Variable' property of a Microflow +// parameter mapping … Value cannot be null"). Studio Pro 9.24 stores no mapping +// for an argument that is the enclosing object — it infers it: Workflow +// Commons 2.6.0 (9.24.0), 75 microflow and 2 nanoflow calls taking parameters, +// none with a mapping; the 9.24.30 starter app's ShowPasswordForm(Account) +// button likewise. So below 10.10 that mapping is not written; a page +// parameter keeps its Variable and loses only the empty Expression. +func TestNormalize_FlowArgumentsBelowMendix10_10(t *testing.T) { + build := func() bson.D { + return pageWithFlowCalls( + []bson.D{flowMapping("Forms$MicroflowParameterMapping", "M.ACT.Item", "$currentObject", ""), + flowMapping("Forms$MicroflowParameterMapping", "M.ACT.Order", "", "Order")}, + []bson.D{flowMapping("Forms$NanoflowParameterMapping", "M.NF.Item", "$currentObject", "")}) + } + for _, tt := range []struct { + v *version.ProjectVersion + lowered bool + mfCount int + nfCount int + mfParams string + }{ + {pv(9, 24, 30), true, 1, 0, "M.ACT.Order"}, + {pv(10, 9, 0), true, 1, 0, "M.ACT.Order"}, + {pv(10, 10, 0), false, 2, 1, "M.ACT.Item,M.ACT.Order"}, + {pv(11, 13, 0), false, 2, 1, "M.ACT.Item,M.ACT.Order"}, // control + } { + d, err := normalizeShape(t, tt.v, build()) + if err != nil { + t.Errorf("%d.%d: the page is refused: %v", tt.v.MajorVersion, tt.v.MinorVersion, err) + continue + } + mf := mappingsOf(t, d, "Forms$MicroflowSettings") + nf := mappingsOf(t, d, "Forms$CallNanoflowClientAction") + var params []string + for _, m := range mf { + p, _ := get(m, "Parameter") + params = append(params, p.(string)) + if _, ok := get(m, "Expression"); ok == tt.lowered { + t.Errorf("%d.%d: mapping %v keeps Expression %v", tt.v.MajorVersion, tt.v.MinorVersion, p, ok) + } + if v, _ := get(m, "Variable"); tt.lowered && v == nil { + t.Errorf("%d.%d: mapping %v has no Variable, which 9.x cannot load", tt.v.MajorVersion, tt.v.MinorVersion, p) + } + } + if len(mf) != tt.mfCount || len(nf) != tt.nfCount || strings.Join(params, ",") != tt.mfParams { + t.Errorf("%d.%d: %d microflow mappings (%s) and %d nanoflow mappings, want %d (%s) and %d", + tt.v.MajorVersion, tt.v.MinorVersion, len(mf), strings.Join(params, ","), len(nf), tt.mfCount, tt.mfParams, tt.nfCount) + } + } +} + +// Any other expression — a literal, an attribute path — has no Variable form: +// refused below 10.10, naming the parameter, rather than dropped. +func TestNormalize_RefusesAnExpressionArgumentBelowMendix10_10(t *testing.T) { + for _, m := range []bson.D{ + flowMapping("Forms$MicroflowParameterMapping", "M.ACT.Flag", "true", ""), + flowMapping("Forms$NanoflowParameterMapping", "M.NF.Name", "$Order/Name", ""), + } { + _, err := normalizeShape(t, pv(9, 24, 30), pageWithFlowCalls([]bson.D{m}, nil)) + p, _ := get(m, "Parameter") + if err == nil || !strings.Contains(err.Error(), p.(string)) || !strings.Contains(err.Error(), "10.10") { + t.Errorf("an expression argument for %v on 9.24 was not refused naming it: %v", p, err) + } + } +} + +// Studio Pro's property set for a workflow on 9.24 includes the two state-change +// event handlers Mendix removed in 11.0: `mx convert` 9.24.30 adds both as null +// to a workflow mxcli wrote, as 10.24.24 does — a property-name difference +// Mendix's merge engine refuses to compare (#1373). +func TestNormalize_WorkflowStateChangeEventsOnMendix9(t *testing.T) { + for _, tt := range []struct { + v *version.ProjectVersion + want bool + }{ + {pv(9, 24, 30), true}, + {pv(10, 24, 24), true}, + {pv(11, 13, 0), false}, // removed in 11.0 + } { + d, err := normalizeShape(t, tt.v, workflowWithPersistentIDs()) + if err != nil { + t.Fatal(err) + } + wf := find(d, "Workflows$Workflow") + for _, k := range []string{"WorkflowOnStateChangeEvent", "UsertaskOnStateChangeEvent"} { + if v, ok := get(wf, k); ok != tt.want || v != nil { + t.Errorf("%d.%d: %s present %v (%v), want present %v and null", tt.v.MajorVersion, tt.v.MinorVersion, k, ok, v, tt.want) + } + } + } +} diff --git a/modelsdk/version/metamodel_versions_gen.go b/modelsdk/version/metamodel_versions_gen.go index d5e02fa57d..37591339cd 100644 --- a/modelsdk/version/metamodel_versions_gen.go +++ b/modelsdk/version/metamodel_versions_gen.go @@ -1241,9 +1241,15 @@ var metamodelVersions = []map[string]TypeVersionInfo{ }, "Microflows$AggregateAction": { Properties: map[string]PropertyVersionInfo{ - "aggregateFunction": {Introduced: "10.0.0", Required: true}, + // Fixed by hand (ako/mxcli#1163): the vendored generator gave + // aggregateFunction the row of reduceReturnDataType, which it then + // left out. mendixmodelsdk 4.117.0 has aggregateFunction with no + // version and reduceReturnDataType introduced 10.0.0 and required; + // Studio Pro 9.24 stores AggregateFunction (Workflow Commons 2.6.0). + "aggregateFunction": {}, "expression": {Introduced: "10.0.0"}, "reduceInitialValueExpression": {Introduced: "10.0.0"}, + "reduceReturnDataType": {Introduced: "10.0.0", Required: true}, "useExpression": {Introduced: "10.0.0"}, }, }, diff --git a/sdk/versions/mendix-10.yaml b/sdk/versions/mendix-10.yaml index adebf0d35d..207443cabf 100644 --- a/sdk/versions/mendix-10.yaml +++ b/sdk/versions/mendix-10.yaml @@ -167,6 +167,17 @@ features: mdl: "CREATE BUSINESS EVENT SERVICE Module.Name ..." odata_client: min_version: "10.0.0" + published_odata_service: + # mxcli writes a published OData service as ODataPublish$PublishedODataService2, + # introduced in 10.4.0 (mendixmodelsdk); before, it is a + # Rest$PublishedODataService, a different document with its own children, + # which mxcli does not write. Measured: mx check 9.24.30 cannot load a + # project holding a PublishedODataService2 — + # "TypeCacheUnknownTypeException: ... ODataPublish$PublishedODataService2" + # (ako/mxcli#1163). Below this the property floor used to refuse + # IncludeMetadataByDefault instead, which named the wrong thing. + min_version: "10.4.0" + mdl: "CREATE PUBLISHED ODATA SERVICE Module.Name (...) { PUBLISH ENTITY ... }" odata_graphql: # A published OData service can also answer GraphQL — one boolean, same # resources, an extra endpoint. Introduced in Studio Pro 10.14 as an diff --git a/sdk/versions/mendix-11.yaml b/sdk/versions/mendix-11.yaml index 687a18b3b5..02a08812e9 100644 --- a/sdk/versions/mendix-11.yaml +++ b/sdk/versions/mendix-11.yaml @@ -179,6 +179,10 @@ features: mdl: "CREATE BUSINESS EVENT SERVICE Module.Name ..." odata_client: min_version: "10.0.0" + published_odata_service: + # ODataPublish$PublishedODataService2 is 10.4+; see mendix-10.yaml. + min_version: "10.4.0" + mdl: "CREATE PUBLISHED ODATA SERVICE Module.Name (...) { PUBLISH ENTITY ... }" odata_graphql: # A published OData service can also answer GraphQL — one boolean, same # resources, an extra endpoint. Introduced in Studio Pro 10.14 as an